Re: [Xen-devel] [PATCH 3/7] x86/pagewalk: Helpers for reserved bit handling

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 01/03/17 15:57, Jan Beulich wrote:
>>>> On 27.02.17 at 15:03, <andrew.cooper3@xxxxxxxxxx> wrote:
>> -static inline int
>> -guest_supports_1G_superpages(struct vcpu *v)
>> +static inline bool guest_has_pse36(const struct vcpu *v)
>> +{
>> +     /* No support for 2-level PV guests. */
>> +    return is_pv_vcpu(v) ? 0 : paging_mode_hap(v->domain);
> Considering the return type ITYM "false" instead of "0" here.

Yes, sorry.  (This series was what caused me to introduce bool to the
hypervisor in the first place, and I clearly haven't rebased it forwards
properly.)

> But then why use a conditional expression here at all?
>
>     return !is_pv_vcpu(v) && paging_mode_hap(v->domain);

Can do.

> Furthermore there's no point in passing in a struct vcpu here -
> both checked constructs are per-domain, and passing in
> struct domain is also a better fit with the guest_ prefix of the
> function name.

At the moment, all the guest_* functions consistently take a vcpu.

However, they are inconsistent with other terms, and some of the
static-configuration properties probably should take a domain.

I will split out this cleanup into an earlier patch, adjusting some
naming and parameters.

>
>> +static inline bool guest_l2e_rsvd_bits(const struct vcpu *v, guest_l2e_t 
>> l2e)
>> +{
>> +    uint64_t rsvd_bits = guest_rsvd_bits(v);
>> +
>> +    return ((l2e.l2 & (rsvd_bits | GUEST_L2_PAGETABLE_RSVD |
>> +                       (guest_supports_superpages(v) ? 0 : _PAGE_PSE))) ||
>> +            ((l2e.l2 & _PAGE_PSE) &&
>> +             (l2e.l2 & ((GUEST_PAGING_LEVELS == 2 && guest_has_pse36(v))
>> +                        ? (fold_pse36(rsvd_bits | (1ULL << 40)))
> While one can look it up in the doc, I strongly think this 40 needs a
> comment.

Thinking about it, having GUEST_L2_PSE36_RSVD is probably a good move.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel