Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT

From: Alexandru Stefan ISAILA <aisaila@bitdefender.com>
To: "rcojocaru@bitdefender.com" <rcojocaru@bitdefender.com>,
	"JBeulich@suse.com" <JBeulich@suse.com>
Cc: "George.Dunlap@eu.citrix.com" <George.Dunlap@eu.citrix.com>,
	"andrew.cooper3@citrix.com" <andrew.cooper3@citrix.com>,
	"tamas@tklengyel.com" <tamas@tklengyel.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [PATCH v1 1/2] x86/mm: Add mem access rights to NPT
Date: Wed, 30 May 2018 09:04:51 +0000	[thread overview]
Message-ID: <1527671092.25123.9.camel@bitdefender.com> (raw)
In-Reply-To: <5B041ECB02000078001C4B3D@prv1-mh.provo.novell.com>

On Ma, 2018-05-22 at 07:44 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 22.05.18 at 15:35, <aisaila@bitdefender.com> wrote:
> > On Ma, 2018-05-22 at 03:49 -0600, Jan Beulich wrote:
> > >
> > > >
> > > > >
> > > > > >
> > > > > > On 18.05.18 at 20:30, <rcojocaru@bitdefender.com> wrote:
> > > > On 05/18/2018 06:30 PM, Jan Beulich wrote:
> > > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > On 11.05.18 at 13:11, <aisaila@bitdefender.com> wrote:
> > > > > > This patch adds access rights for the NPT pages. The access
> > > > > > rights are
> > > > > > saved in bits 59:56 of pte that are manipulated through
> > > > > > p2m_set_access()
> > > > > > and p2m_get_access() functions.
> > > > > You don't give any rationale for the choice of bits. Right
> > > > > now
> > > > > p2m-pt.c still
> > > > > assumes that CPU and IOMMU page tables might be shared,
> > > > > despite
> > > > > amd_iommu_init() unconditionally turning this functionality
> > > > > off.
> > > > > As long as the
> > > > > option for that mode hasn't been removed from p2m-pt.c, I
> > > > > think
> > > > > bits used
> > > > > by the IOMMU (here: bit 59) should not be used for software
> > > > > purposes. The
> > > > > alternative therefore is for you to supply a prereq patch
> > > > > purging
> > > > > the sharing
> > > > > functionality from p2m-pt.c and preferably also from the AMD
> > > > > IOMMU code.
> > > > > That's of course only an option if we don't foresee any means
> > > > > by
> > > > > which this
> > > > > mode may become usable again.
> > > > The choice of bits was our interpretation of Andrew's reply
> > > > here:
> > > >
> > > > https://lists.xenproject.org/archives/html/xen-devel/2018-05/ms
> > > > g005
> > > > 73.html
> > > >
> > > > Have we misread it?
> > > I don't think you have, but what Andrew has described was only
> > > the
> > > CPU side
> > > of considerations to make. Plus of course the patch description
> > > should explain
> > > whatever choice you make.
> > >
> > > >
> > > >
> > > > We've also thought about putting the information in a new field
> > > > of
> > > > struct page_info. Would that perhaps be preferable?
> > > I don't view this as a page property, but a mapping property. As
> > > such
> > > it can't validly go into struct page_info.
> > I will add the information in the patch description. Can you tell
> > us
> > what structure is best to use for the access rights?
> I may not correctly understand the question: I think everybody agrees
> on the bits to go into an _unused_ portion of the p2m entry.

Sorry for the misunderstanding, I wanted to clarify if the 59:56 bits
are fully ok to be used or if not then where should I use 4 bits to
store the mem access info?

Any thoughts on this matter are appreciated.

Thanks,
Alex

________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel