From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56756) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4fko-0000W9-8M for qemu-devel@nongnu.org; Mon, 15 Jun 2015 21:33:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4fkl-0000qE-0J for qemu-devel@nongnu.org; Mon, 15 Jun 2015 21:33:30 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:35211) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4fkk-0000q8-RL for qemu-devel@nongnu.org; Mon, 15 Jun 2015 21:33:26 -0400 Received: by pacyx8 with SMTP id yx8so2097811pac.2 for ; Mon, 15 Jun 2015 18:33:25 -0700 (PDT) Message-ID: <557F7CDF.5080203@linaro.org> Date: Tue, 16 Jun 2015 09:33:19 +0800 From: Shannon Zhao MIME-Version: 1.0 References: <1433929959-29530-1-git-send-email-drjones@redhat.com> <1433929959-29530-3-git-send-email-drjones@redhat.com> <20150615180904-mutt-send-email-mst@redhat.com> <20150615163221.GA30395@hawk.localdomain> <20150615201153-mutt-send-email-mst@redhat.com> In-Reply-To: <20150615201153-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/2] hw/arm/virt-acpi-build: Add SPCR table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , Peter Maydell Cc: Igor Mammedov , Andrew Jones , QEMU Developers On 2015/6/16 2:13, Michael S. Tsirkin wrote: > On Mon, Jun 15, 2015 at 05:59:06PM +0100, Peter Maydell wrote: >> On 15 June 2015 at 17:32, Andrew Jones 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 > > > We don't much. One can use build_append_int_noprefix and just avoid > structs altogether. But if we use build_append_int_noprefix, we have to bother about the unused fields of the struct and have lots of build_append_int_noprefix(table, 0, 1/2/4/8). > We did this for some structures and I'm thinking it's a good direction > generally. > -- Shannon