On 15 June 2015 at 17:32, Andrew Jones <drjo...@redhat.com> wrote:
> On Mon, Jun 15, 2015 at 06:10:25PM +0200, Michael S. Tsirkin wrote:
>> On Mon, Jun 15, 2015 at 04:45:58PM +0100, Peter Maydell wrote:
>> > I'm still confused about when fields in these ACPI structs
>> > need to be converted to little-endian, and when they don't.
>> > Is there a rule-of-thumb I can use when I'm looking at patches?
>> Normally it's all LE unless it's a single byte value.
>> Did not check this specific table.
>> We really need to add sparse support to check
>> endian-ness matches, or re-write it
>> all using byte_add so there's no duplication of info.
> Everything used in the table is either a single byte, or I used le32,
> Well, I didn't bother for the pci_{device,vendor}_id assignments, as
> they're 0xffff anyway. I can change those two to make them more explicit,
> if that's preferred.
Yep, I just looked over the struct definition, so since this
has been reviewed I'll apply it to target-arm.next.
You could probably make it easier to review and write
code that has to do these endianness swaps with something
like
#define acpi_struct_assign(FIELD, VAL) \
((FIELD) = \
__builtin_choose_expr(sizeof(FIELD) == 1, VAL, \
__builtin_choose_expr(sizeof(FIELD) == 2, cpu_to_le16(VAL), \
__builtin_choose_expr(sizeof(FIELD) == 4, cpu_to_le32(VAL), \
__builtin_choose_expr(sizeof(FIELD) == 8, cpu_to_le64(VAL), \
abort))))
(untested, but based on some code in linux-user/qemu.h).
Then it's always
acpi_struct_assign(spcr->field, value);
whether the field is 1, 2, 4 or 8 bytes.
Not my bit of the codebase though, so I'll leave it to the
ACPI maintainers to decide how much they like magic macros :-)
thanks
-- PMM
