From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
To: "Zheng, Lv" <lv.zheng@intel.com>, Jan Beulich <jbeulich@suse.com>
Cc: "ian.jackson@eu.citrix.com" <ian.jackson@eu.citrix.com>,
"zetalog@gmail.com" <zetalog@gmail.com>,
"Brown, Len" <len.brown@intel.com>,
"Wysocki, Rafael J" <rafael.j.wysocki@intel.com>,
"Moore, Robert" <robert.moore@intel.com>,
"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
"rjw@rjwysocki.net" <rjw@rjwysocki.net>,
"linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 08/13] ACPICA: Hardware: Add optimized access bit width support
Date: Fri, 27 May 2016 13:31:36 -0400 [thread overview]
Message-ID: <57488478.1070406@oracle.com> (raw)
In-Reply-To: <1AE640813FDE7649BE1B193DEA596E883BBB1530@SHSMSX101.ccr.corp.intel.com>
On 05/27/2016 03:34 AM, Zheng, Lv wrote:
> Hi,
>
>> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
>> 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@oracle.com> 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]
Your patch from earlier does resolve both issues, thanks.
> Your solution could result in problems like:
> If a GAS is defined with bit_width not a power of 2, and access_width is any (0).
>
> So the correct fix here is to make sure if bit_width is exactly 8,16,32,64, which matches old register descriptors.
> I added address check here because I want to limit this regression safe check to old register descriptors.
> As since the old bit_width can actually reflect the register's access width, the address of the register should always be aligned.
>
> There might be cases that using the new GAS register descriptor format, it is possible to define a GAS that is not aligned, and it's bit_width is exactly 8,16,32,64.
> We shouldn't make a default access_width decision using bit_width here.
True. But OTOH switching to 32-bit accesses may result in us suddenly
trying to touch bytes we haven't accessed before.
-boris
> The address check here can help to avoid applying this workaround on such cases.
>
> Thanks and best regards
> -Lv
next prev parent reply other threads:[~2016-05-27 17:31 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-04 5:47 [PATCH 00/15] ACPICA: 20160422 Release Lv Zheng
2016-05-04 5:48 ` [PATCH 01/15] ACPICA: Linuxize: reduce divergences for 20160422 release Lv Zheng
2016-05-04 21:37 ` Rafael J. Wysocki
2016-05-04 21:38 ` Rafael J. Wysocki
2016-05-05 2:16 ` Zheng, Lv
2016-05-05 2:15 ` Zheng, Lv
2016-05-04 5:48 ` [PATCH 02/15] ACPICA: Divergence: remove unwanted spaces for typedef Lv Zheng
2016-05-04 5:48 ` [PATCH 03/15] ACPICA: Refactor evaluate_object to reduce nesting Lv Zheng
2016-05-04 5:48 ` [PATCH 04/15] ACPICA: ACPI 6.1: Support for new PCCT subtable Lv Zheng
2016-05-04 5:48 ` [PATCH 05/15] ACPICA: ACPI 6.0: Update _BIX support for new package element Lv Zheng
2016-05-04 5:48 ` [PATCH 06/15] ACPICA: ACPI 6.0, tools/iasl: Add support for new resource descriptors Lv Zheng
2016-05-04 5:48 ` [PATCH 07/15] ACPICA: Renamed some #defined flag constants for clarity Lv Zheng
2016-05-04 5:48 ` [PATCH 08/15] ACPICA: Dispatcher: Update thread ID for recursive method calls Lv Zheng
2016-05-04 15:10 ` Prarit Bhargava
2016-05-04 19:22 ` Rafael J. Wysocki
2016-05-04 20:30 ` Mario_Limonciello
2016-05-04 20:45 ` Rafael J. Wysocki
2016-05-04 5:49 ` [PATCH 09/15] ACPICA: Utilities: Add ACPI_IS_ALIGNED() macro Lv Zheng
2016-05-04 5:49 ` [PATCH 10/15] ACPICA: Hardware: Add optimized access bit width support Lv Zheng
2016-05-04 5:49 ` [PATCH 11/15] ACPICA: Executer: Introduce a set of macros to handle bit width mask generation Lv Zheng
2016-05-04 5:49 ` [PATCH 12/15] ACPICA: Hardware: Add access_width/bit_offset support in acpi_hw_read() Lv Zheng
2016-05-04 5:49 ` [PATCH 13/15] ACPICA: Hardware: Add access_width/bit_offset support for acpi_hw_write() Lv Zheng
2016-05-04 5:49 ` [PATCH 14/15] ACPICA: Move all ASCII utilities to a common file Lv Zheng
2016-05-04 5:49 ` [PATCH 15/15] ACPICA: Update version to 20160422 Lv Zheng
2016-05-05 4:57 ` [PATCH v2 00/13] ACPICA: 20160422 Release Lv Zheng
2016-05-05 4:57 ` [PATCH v2 01/13] ACPICA: Divergence: remove unwanted spaces for typedef Lv Zheng
2016-05-05 4:58 ` [PATCH v2 02/13] ACPICA: Refactor evaluate_object to reduce nesting Lv Zheng
2016-05-05 4:58 ` [PATCH v2 03/13] ACPICA: ACPI 6.1: Support for new PCCT subtable Lv Zheng
2016-05-05 4:58 ` [PATCH v2 04/13] ACPICA: ACPI 6.0: Update _BIX support for new package element Lv Zheng
2016-05-05 4:58 ` [PATCH v2 05/13] ACPICA: ACPI 6.0, tools/iasl: Add support for new resource descriptors Lv Zheng
2016-05-05 4:58 ` [PATCH v2 06/13] ACPICA: Renamed some #defined flag constants for clarity Lv Zheng
2016-05-05 4:58 ` [PATCH v2 07/13] ACPICA: Utilities: Add ACPI_IS_ALIGNED() macro Lv Zheng
2016-05-05 4:58 ` [PATCH v2 08/13] ACPICA: Hardware: Add optimized access bit width support Lv Zheng
2016-05-25 19:17 ` Boris Ostrovsky
2016-05-26 16:26 ` Jan Beulich
2016-05-26 16:55 ` Boris Ostrovsky
2016-05-26 21:59 ` Boris Ostrovsky
2016-05-27 3:24 ` Zheng, Lv
2016-05-27 7:34 ` Zheng, Lv
2016-05-27 17:31 ` Boris Ostrovsky [this message]
2016-05-27 3:14 ` Zheng, Lv
2016-05-05 4:58 ` [PATCH v2 09/13] ACPICA: Executer: Introduce a set of macros to handle bit width mask generation Lv Zheng
2016-05-05 4:58 ` [PATCH v2 10/13] ACPICA: ACPI 2.0, Hardware: Add access_width/bit_offset support in acpi_hw_read() Lv Zheng
2016-05-05 5:00 ` [PATCH v2 11/13] ACPICA: ACPI 2.0, Hardware: Add access_width/bit_offset support for acpi_hw_write() Lv Zheng
2016-05-05 5:00 ` [PATCH v2 12/13] ACPICA: Move all ASCII utilities to a common file Lv Zheng
2016-05-05 5:00 ` [PATCH v2 13/13] ACPICA: Update version to 20160422 Lv Zheng
2016-05-05 23:49 ` [PATCH v2 00/13] ACPICA: 20160422 Release Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=57488478.1070406@oracle.com \
--to=boris.ostrovsky@oracle.com \
--cc=ian.jackson@eu.citrix.com \
--cc=jbeulich@suse.com \
--cc=len.brown@intel.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lv.zheng@intel.com \
--cc=rafael.j.wysocki@intel.com \
--cc=rjw@rjwysocki.net \
--cc=robert.moore@intel.com \
--cc=xen-devel@lists.xen.org \
--cc=zetalog@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).