Re: [Qemu-arm] [Qemu-devel] [PATCH v17 02/10] ACPI: add some GHES structures and macros definition

[Igor Mammedov]

On Tue, 25 Jun 2019 17:56:00 +0800
gengdongjiu <address@hidden> wrote:

> On 2019/6/24 19:16, Igor Mammedov wrote:
> >>>> On 2019/6/20 20:10, Igor Mammedov wrote:    
> >>>>>> + */
> >>>>>> +struct AcpiGenericErrorStatus {
> >>>>>> +    /* It is a bitmask composed of ACPI_GEBS_xxx macros */
> >>>>>> +    uint32_t block_status;
> >>>>>> +    uint32_t raw_data_offset;
> >>>>>> +    uint32_t raw_data_length;
> >>>>>> +    uint32_t data_length;
> >>>>>> +    uint32_t error_severity;
> >>>>>> +} QEMU_PACKED;
> >>>>>> +typedef struct AcpiGenericErrorStatus AcpiGenericErrorStatus;      
> >>>>> there shouldn't be packed structures,
> >>>>> is it a leftover from previous version?      
> >>>> I remember some people suggest to add QEMU_PACKED before, anyway I will 
> >>>> remove it in my next version patch.    
> >>> Question is why it's  there and where it is used?    
> >> sorry, it is my carelessness. it should be packed structures.
> >>
> >> I used this structures to get its actual total size and member offset in 
> >> [PATCH v17 10/10].
> >> If it is not packed structures, the total size and member offset may be 
> >> not right.  
> > I'd suggest to drop these typedefs and use a macro with size for that 
> > purpose,
> > Also it might be good to make it local to the file that would use it.  
> so you mean we also use macro for the  member offset  in the structures?  
> such as the offset of data_length,
yes, but I hope there won't be need for data_length offset at all.

> may be there is many hardcode.
> 
> >   
> >>> BTW:
> >>> series doesn't apply to master anymore.  
>