Re: [Qemu-devel] [PATCH v3 5/9] target-arm: Add ARMMMUFaultInfo

[Peter Maydell <peter.maydell@linaro.org>]

On 8 October 2015 at 21:06, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Wed, Oct 07, 2015 at 05:24:27PM +0100, Alex Bennée wrote:
>>
>> Edgar E. Iglesias <edgar.iglesias@gmail.com> writes:
>>
>> > +/**
>> > + * ARMMMUFaultInfo: Information describing an ARM MMU Fault
>> > + * @s2addr: Address that caused a fault at stage 2
>> > + * @stage2: True if we faulted at stage 2
>> > + * @s1ptw: True if we faulted at stage 2 while doing a stage 1 page-table walk
>> > + */
>> > +typedef struct ARMMMUFaultInfo ARMMMUFaultInfo;
>> > +struct ARMMMUFaultInfo {
>> > +    target_ulong s2addr;
>> > +    bool stage2;
>> > +    bool s1ptw;
>>
>> I guess the compiler packs the bools down pretty well but why not just
>> encode the faulting stage in a single variable? Perhaps I'm
>> misunderstanding the potential combinations here.
>>
>
> Do you mean using bitfields?
> e.g:
>
> struct ARMMMUFaultInfo {
>     target_ulong s2addr;
>     unsigned int stage2 : 1;
>     unsigned int s1ptw : 1;
> }
> If so, I guess we could. If others agree, I can change to that.
> We could also maybe structure things differently. I didn't consider
> the encodings very much here to be honest...

I'm not a great fan of bitfields personally; I think a pair
of bool fields expresses things more clearly.

-- PMM