Re: [PATCH V7 1/2] xen/arm: Harden the P2M code in p2m_remove_mapping()

[Julien Grall <julien@xxxxxxx>]

Hi Oleksandr,

On 16/07/2022 16:29, Oleksandr Tyshchenko wrote:
> > On 16/07/2022 15:56, Oleksandr Tyshchenko wrote:
> > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > >
> > > Borrow the x86's check from p2m_remove_page() which was added
> > > by the following commit: c65ea16dbcafbe4fe21693b18f8c2a3c5d14600e
> > > "x86/p2m: don't assert that the passed in MFN matches for a remove"
> > > and adjust it to the Arm code base.
> > >
> > > Basically, this check will be strictly needed for the xenheap pages
> > > after applying a subsequent commit which will introduce xenheap based
> > > M2P approach on Arm. But, it will be a good opportunity to harden
> > > the P2M code for *every* RAM pages since it is possible to remove
> > > any GFN - MFN mapping currently on Arm (even with the wrong helpers).
> > >
> > > Suggested-by: Julien Grall <jgrall@xxxxxxxxxx>
> > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > ---
> > > You can find the corresponding discussion at:
> > > https://urldefense.com/v3/__https://lore.kernel.org/xen-devel/82d8bfe0-cb46-d303-6a60-2324dd76a1f7@xxxxxxx/__;!!GF_29dbcQIUBPA!3a2u-XL4NvAzSMfz72LARrdWVFvq2In5ZpUdxP2cSt7bM8PgV7P_ZclZG2R-rE9PcosUHyqsKRNfVG2TiM9Tlg$
> > > [lore[.]kernel[.]org]
> > > https://urldefense.com/v3/__https://lore.kernel.org/xen-devel/1652294845-13980-2-git-send-email-olekstysh@xxxxxxxxx/__;!!GF_29dbcQIUBPA!3a2u-XL4NvAzSMfz72LARrdWVFvq2In5ZpUdxP2cSt7bM8PgV7P_ZclZG2R-rE9PcosUHyqsKRNfVG0kg7IZSA$
> > > [lore[.]kernel[.]org]
> > >
> > > Changes V6 -> V7:
> > >      - make this commit to be the first
> > >      - update commit description and add a comment in code
> > > ---
> > >    xen/arch/arm/p2m.c | 28 ++++++++++++++++++++++++++++
> > >    1 file changed, 28 insertions(+)
> > >
> > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > > index d00c2e462a..2a0d383df4 100644
> > > --- a/xen/arch/arm/p2m.c
> > > +++ b/xen/arch/arm/p2m.c
> > > @@ -1308,11 +1308,39 @@ static inline int p2m_remove_mapping(struct
> > > domain *d,
> > >                                         mfn_t mfn)
> > >    {
> > >        struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > > +    unsigned long i;
> > >        int rc;
> > >          p2m_write_lock(p2m);
> > > +    /*
> > > +     * Before removing the GFN - MFN mapping for any RAM pages make
> > > sure
> > > +     * that there is no difference between what is already mapped
> > > and what
> > > +     * is requested to be unmapped.
> > > +     * If they don't match bail out early. For instance, this could
> > > happen
> > > +     * if two CPUs are requesting to unmap the same P2M concurrently.
> > Missing word: P2M *entry*
> Yes. May I please ask, could this be done on the commit if this appears
> to be the last version?
I have committed the series with the same typo.

Cheers,

--
Julien Grall