Re: [PATCH v2 1/7] x86/sev: Prepare for using the RMPREAD instruction to access the RMP

[Tom Lendacky <thomas.lendacky@amd.com>]

> On Wed, 4 Sep 2024 12:47:28 +0200, Borislav Petkov wrote:

Sorry, this email seemed to get lost in our email quarantine. Trying to
reply using copy/paste and git send-email...

> > On Tue, Jul 30, 2024 at 02:40:01PM -0500, Tom Lendacky wrote:
> > The RMPREAD instruction returns an architecture defined format of an
> > RMP entry. This is the preferred method for examining RMP entries.
> > 
> > In preparation for using the RMPREAD instruction, convert the existing
> > code that directly accesses the RMP to map the raw RMP information into
> > the architecture defined format.
> > 
> > RMPREAD output returns a status bit for the 2MB region status. If the
> > input page address is 2MB aligned and any other pages within the 2MB
> > region are assigned, then 2MB region status will be set to 1. Otherwise,
> > the 2MB region status will be set to 0. For systems that do not support
> > RMPREAD, calculating this value would require looping over all of the RMP
> > table entries within that range until one is found with the assigned bit
> > set. Since this bit is not defined in the current format, and so not used
> > today, do not incur the overhead associated with calculating it.
> > 
> > Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> > ---
> >  arch/x86/virt/svm/sev.c | 141 ++++++++++++++++++++++++++++------------
> >  1 file changed, 98 insertions(+), 43 deletions(-)
> > 
> > diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> > index 0ce17766c0e5..103a2dd6e81d 100644
> > --- a/arch/x86/virt/svm/sev.c
> > +++ b/arch/x86/virt/svm/sev.c
> > @@ -30,11 +30,27 @@
> >  #include <asm/cmdline.h>
> >  #include <asm/iommu.h>
> >  
> > +/*
> > + * The RMP entry format as returned by the RMPREAD instruction.
> 
> I'm guessing this is the architectural variant...

Yes, this is the format returned by RMPREAD.

> 
> > + */
> > +struct rmpentry {
> > +	u64 gpa;
> > +	u8  assigned		:1,
> > +	    rsvd1		:7;
> > +	u8  pagesize		:1,
> > +	    hpage_region_status	:1,
> > +	    rsvd2		:6;
> > +	u8  immutable		:1,
> > +	    rsvd3		:7;
> > +	u8  rsvd4;
> > +	u32 asid;
> > +} __packed;
> > +
> >  /*
> >   * The RMP entry format is not architectural. The format is defined in PPR
> >   * Family 19h Model 01h, Rev B1 processor.
> 
> ... considering this thing?
> 
> >   */
> > -struct rmpentry {
> > +struct rmpentry_raw {
> 
> "raw"? Hm.
> 
> So what is the goal here?
> 
> Use rmpentry_raw for the kernel's representation and convert the format that
> gets returned by RMPREAD into rmpentry_raw?

No, just the opposite. Take the current systems that don't have RMPREAD support
and transform the "raw" RMP entry data obtained directly from the RMP table
into the architectural variant. This way, only the architectural variant is
used going forward.

> 
> Because if so, the _raw thing is what comes from RMPREAD.
> 
> Because as it is now, it is begging to confuse people.
> 
> Because if you call the *new* entry differently, it won't cause any of the
> churn you have to do below...

I can look at naming the new struct "rmpread" and see how that looks if you
prefer.

Thanks,
Tom

> 
> Hmmm?
> 
> -- 
> Regards/Gruss,
>     Boris.
>