Hi, > From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx] > Sent: Friday, May 27, 2016 12:56 AM > Subject: Re: [PATCH v2 08/13] ACPICA: Hardware: Add optimized access > bit width support > > On 05/26/2016 12:26 PM, Jan Beulich wrote: > >>>> Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> 05/25/16 9:17 > PM >>> > >> On 05/05/2016 12:58 AM, Lv Zheng wrote: > >>> +static u8 > >>> +acpi_hw_get_access_bit_width(struct acpi_generic_address *reg, u8 > max_bit_width) > >>> +{ > >>> + u64 address; > >>> + > >>> + if (!reg->access_width) { > >>> + /* > >>> + * Detect old register descriptors where only the bit_width field > >>> + * makes senses. The target address is copied to handle possible > >>> + * alignment issues. > >>> + */ > >>> + ACPI_MOVE_64_TO_64(&address, ®->address); > >>> + if (!reg->bit_offset && reg->bit_width && > >>> + ACPI_IS_POWER_OF_TWO(reg->bit_width) && > >>> + ACPI_IS_ALIGNED(reg->bit_width, 8) && > >>> + ACPI_IS_ALIGNED(address, reg->bit_width)) { > >>> + return (reg->bit_width); > >>> + } else { > >>> + if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) { > >>> + return (32); > >> This (together with "... Add access_width/bit_offset support in > >> acpi_hw_write") breaks Xen guests using older QEMU which doesn't > support > >> 4-byte IO accesses. > >> > >> Why not return "reg->bit_width?:max_bit_width" ? This will preserve > >> original behavior. > > Did you figure out why we get here in the first place, instead of taking > the > > first "return"? I.e. isn't the issue the apparently wrong use of the second > > ACPI_IS_ALIGNED() above? Afaict it ought to be > > ACPI_IS_ALIGNED(address, reg->bit_width / 8)... > > We are trying to access address 0x...b004 (PM1a control) so yes, fixing > alignment check would probably resolve the problem that we are seeing > now. > > However, for compatibility purposes we may consider not doing any > checks > and simply return bit_width if access_width is not available. [Lv Zheng] There might be GAS defined with AccessWidth = 0. And its BitOffset can still make senses. That's why I put the check here, making it a tuning rather than a regression safe check. Thanks for the information, I'll submit the fix of the address check. To convert it to: ACPI_IS_ALIGNED(address, reg->bit_width / 8) This is my fault. Thanks -Lv > > -boris > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
