Re: [Xen-devel] [PATCH 2/3] Fix acpi_set_register

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

Agreed. Linux has the same double-read by the way, as recently as
2.6.26-rc4. I hope it's not for a particular reason, and is just a typo. You
might send your patch upstream to Linux if it is still not fixed there.

 -- Keir

On 3/9/08 11:44, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:

> acpi_set_register() has already done a read before switch-case block, this
> read with in switch-case is a unnecessary repeat.
> 
> The non-continuous PIO/MMIO access will spent far longer time than imagination
> because of the chipset link ASPM features. One single PIO/MMIO read may spent
> up to tens of thousands cpu cycles, although the fastest read may only cause
> less than one thousand cycles. I tried to measure the cost via TSC.
> 
> Jimmy
> 
> On Wednesday, September 03, 2008 4:43 PM, Keir Fraser wrote:
>> acpi_set_register() is doing a read-modify-write of PM2_CONTROL. It's only
>> safe to skip the read if the modification is going to obliterate all old
>> bits. Is the read really that expensive (and equally, in patch 3, is the
>> write of ARB_DIS really that expensive)?
>> 
>>  -- Keir
>> 
>> On 3/9/08 02:52, "Wei, Gang" <gang.wei@xxxxxxxxx> wrote:
>> 
>>> ACPI: Remove a redundant call to acpi_hw_register_read().
>>> 
>>> Signed-off-by: Wei Gang <gang.wei@xxxxxxxxx>
>>> 
>>> diff -r fc0b0c64246d xen/drivers/acpi/hwregs.c
>>> --- a/xen/drivers/acpi/hwregs.c Thu Aug 21 16:15:30 2008 +0800
>>> +++ b/xen/drivers/acpi/hwregs.c Mon Aug 25 15:24:32 2008 +0800
>>> @@ -238,12 +238,6 @@ acpi_status acpi_set_register(u32 regist
>>> break;
>>> 
>>>         case ACPI_REGISTER_PM2_CONTROL:
>>> -
>>> -               status = acpi_hw_register_read(ACPI_REGISTER_PM2_CONTROL,
>>> -                                              &register_value);
>>> -               if (ACPI_FAILURE(status)) {
>>> -                       goto unlock_and_exit;
>>> -               }
>>> 
>>>                 ACPI_DEBUG_PRINT((ACPI_DB_IO,
>>>                                   "PM2 control: Read %X from %8.8X%8.8X\n",
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel