Re: [git pull request] ACPI and driver patches for 2.6.33.merge

[Shaohua Li <shaohua.li@intel.com>]

On Wed, Dec 23, 2009 at 10:57:19AM +0800, Yinghai Lu wrote:
> Shaohua Li wrote:
> > On Wed, Dec 23, 2009 at 10:09:04AM +0800, Yinghai Lu wrote:
> >> On Tue, Dec 22, 2009 at 5:38 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> >>> Shaohua Li wrote:
> >>>> On Tue, Dec 22, 2009 at 06:08:57PM +0800, Yinghai Lu wrote:
> >>>>> Ingo Molnar wrote:
> >>>>>> * Shaohua Li <shaohua.li@intel.com> wrote:
> >>>>>>
> >>>>>>> On Fri, Dec 18, 2009 at 09:28:50AM +0800, Yinghai Lu wrote:
> >>>>>>>> On Wed, Dec 16, 2009 at 12:06 PM, Len Brown <lenb@kernel.org> wrote:
> >>>>>>>>> Hi Linus,
> >>>>>>>>>
> >>>>>>>>> please pull from:
> >>>>>>>>>
> >>>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git release
> >>>>>>>>>
> >>>>>>>> ;..
> >>>>>>>>> Shaohua Li (3):
> >>>>>>>>> ? ? ?ACPI: Add a generic API for _OSC -v2
> >>>>>>>>> ? ? ?ACPI: cleanup pci_root _OSC code.
> >>>>>>>>> ? ? ?ACPI: Add platform-wide _OSC support.
> >>>>>>>> it seems these three patches broke the _OSC on my intel new systems.
> >>>>>>>>
> >>>>>>>> revert them fix the problem with AER and pciehp and etc
> >>>>>>> can you give more details? I just cleaned up the _OSC code for AER and
> >>>>>>> pciehp, no function changes.
> >>>>>> Famous last words ;-)
> >>>>>>
> >>>>>> Yinghai, i suspect Shaohua needs the kind of info you'd need if you tried to
> >>>>>> fix it: acpidump, before/after debug boot log, a description of what goes bad,
> >>>>>> etc.
> >>>>> the so called clean up, change the ret length checking.
> >>>>>
> >>>>> -       if (!output.length)
> >>>>> -               return AE_NULL_OBJECT;
> >>>>> -
> >>>>>
> >>>>> +       /* return buffer should have the same length as cap buffer */
> >>>>> +       if (context->ret.length != context->cap.length)
> >>>>> +               return AE_NULL_OBJECT;
> >>>> Wield BIOS. ACPI spec does mention the return buffer have the same length.
> >>>> Does changing the check back make the issue go away?
> >>> change to
> >>> if (context->ret.length < context->cap.length)
> >>>
> >>> make AER work, but pciehp still fail.
> > Can you try below patch please? Looks the returned acpi buffer is a two-tiled buffer.
> > Strange is it doesn't fail at my hand.
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 65f7e33..0c1ad31 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -397,6 +397,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
> >  	union acpi_object *out_obj;
> >  	u8 uuid[16];
> >  	u32 errors;
> > +	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
> >  
> >  	if (!context)
> >  		return AE_ERROR;
> > @@ -419,16 +420,16 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
> >  	in_params[3].buffer.length 	= context->cap.length;
> >  	in_params[3].buffer.pointer 	= context->cap.pointer;
> >  
> > -	status = acpi_evaluate_object(handle, "_OSC", &input, &context->ret);
> > +	status = acpi_evaluate_object(handle, "_OSC", &input, &output);
> >  	if (ACPI_FAILURE(status))
> >  		return status;
> >  
> > -	/* return buffer should have the same length as cap buffer */
> > -	if (context->ret.length != context->cap.length)
> > +	if (!output.length)
> >  		return AE_NULL_OBJECT;
> >  
> > -	out_obj = context->ret.pointer;
> > -	if (out_obj->type != ACPI_TYPE_BUFFER) {
> > +	out_obj = output.pointer;
> > +	if (out_obj->type != ACPI_TYPE_BUFFER
> > +		|| out_obj->buffer.length != context->cap.length) {
> >  		acpi_print_osc_error(handle, context,
> >  			"_OSC evaluation returned wrong type");
> >  		status = AE_TYPE;
> > @@ -457,11 +458,20 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
> >  		goto out_kfree;
> >  	}
> >  out_success:
> > -	return AE_OK;
> > +	context->ret.length = out_obj->buffer.length;
> > +	context->ret.pointer = kmalloc(context->ret.length, GFP_KERNEL);
> > +	if (!context->ret.pointer) {
> > +		status =  AE_NO_MEMORY;
> > +		goto out_kfree;
> > +	}
> > +	memcpy(context->ret.pointer, out_obj->buffer.pointer,
> > +		context->ret.length);
> > +	status =  AE_OK;
> >  
> >  out_kfree:
> > -	kfree(context->ret.pointer);
> > -	context->ret.pointer = NULL;
> > +	kfree(output.pointer);
> > +	if (status != AE_OK)
> > +		context->ret.pointer = NULL;
> >  	return status;
> >  }
> >  EXPORT_SYMBOL(acpi_run_osc);
> 
> aer and pciehp work again with this patch.
Len, this is the patch with describtion, please check and apply.

Executing _OSC returns a buffer, which has an acpi object in it.
Don't directly returns the buffer, instead, we return the acpi object's
buffer. This fixes a regression since caller of acpi_run_osc expects
an acpi object's buffer returned.

Tested-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 65f7e33..0c1ad31 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -397,6 +397,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
 	union acpi_object *out_obj;
 	u8 uuid[16];
 	u32 errors;
+	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
 
 	if (!context)
 		return AE_ERROR;
@@ -419,16 +420,16 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
 	in_params[3].buffer.length 	= context->cap.length;
 	in_params[3].buffer.pointer 	= context->cap.pointer;
 
-	status = acpi_evaluate_object(handle, "_OSC", &input, &context->ret);
+	status = acpi_evaluate_object(handle, "_OSC", &input, &output);
 	if (ACPI_FAILURE(status))
 		return status;
 
-	/* return buffer should have the same length as cap buffer */
-	if (context->ret.length != context->cap.length)
+	if (!output.length)
 		return AE_NULL_OBJECT;
 
-	out_obj = context->ret.pointer;
-	if (out_obj->type != ACPI_TYPE_BUFFER) {
+	out_obj = output.pointer;
+	if (out_obj->type != ACPI_TYPE_BUFFER
+		|| out_obj->buffer.length != context->cap.length) {
 		acpi_print_osc_error(handle, context,
 			"_OSC evaluation returned wrong type");
 		status = AE_TYPE;
@@ -457,11 +458,20 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
 		goto out_kfree;
 	}
 out_success:
-	return AE_OK;
+	context->ret.length = out_obj->buffer.length;
+	context->ret.pointer = kmalloc(context->ret.length, GFP_KERNEL);
+	if (!context->ret.pointer) {
+		status =  AE_NO_MEMORY;
+		goto out_kfree;
+	}
+	memcpy(context->ret.pointer, out_obj->buffer.pointer,
+		context->ret.length);
+	status =  AE_OK;
 
 out_kfree:
-	kfree(context->ret.pointer);
-	context->ret.pointer = NULL;
+	kfree(output.pointer);
+	if (status != AE_OK)
+		context->ret.pointer = NULL;
 	return status;
 }
 EXPORT_SYMBOL(acpi_run_osc);