Re: [PATCHv6 05/30] x86/tdx: Exclude shared bit from __PHYSICAL_MASK

Kirill A. Shutemov
2022-03-17T13:58:16Z

On Thu, Mar 17, 2022 at 01:16:00AM +0100, Thomas Gleixner wrote:
> On Wed, Mar 16 2022 at 05:08, Kirill A. Shutemov wrote:
> > @@ -82,6 +82,14 @@ void __init tdx_early_init(void)
> >
> > cc_set_vendor(CC_VENDOR_INTEL);
> >
> > + /*
> > + * All bits above GPA width are reserved and kernel treats shared bit
> > + * as flag, not as part of physical address.
> > + *
> > + * Adjust physical mask to only cover valid GPA bits.
> > + */
> > + physical_mask &= GENMASK_ULL(gpa_width - 2, 0);
> > +
>
> Hrm. I forgot about the second use case for gpa_width, but my comment
> about ordering still stands. OTOH:
>
> GENMASK_ULL(gpa_width - 2, 0) == BIT_UL(gpa_width - 1) - 1
>
> right? So you really can consolidate on the fact that cc_mask is a
> single bit which is above the guests physical address space boundary.
>
> I.e. make the code tell the story instead of adding lengthy comments
> explaining the obfuscation.

So it will looks something like this:

cc_set_vendor(CC_VENDOR_INTEL);
cc_mask = get_cc_mask();
cc_set_mask(cc_mask);

/*
* All bits above GPA width are reserved and kernel treats shared bit
* as flag, not as part of physical address.
*
* Adjust physical mask to only cover valid GPA bits.
*/
physical_mask &= cc_mask - 1;

I still think these comments are useful. I hided comment for cc_mask
calclulation inside get_cc_mask().

Does it look fine to you?

--
Kirill A. Shutemov