| Message ID | 20231018080242.1213-1-tamas@tklengyel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-4.18] x86/mem_sharing: add missing m2p entry when mapping shared_info page | expand |
On 18/10/2023 9:02 am, Tamas K Lengyel wrote: > When mapping in the shared_info page to a fork the m2p entry wasn't set > resulting in the shared_info being reset even when the fork reset was called > with only reset_state and not reset_memory. This results in an extra > unnecessary TLB flush. > > Fixes: 1a0000ac775 ("mem_sharing: map shared_info page to same gfn during fork") > Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> > --- > xen/arch/x86/mm/mem_sharing.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > index 94b6b782ef..142258f16a 100644 > --- a/xen/arch/x86/mm/mem_sharing.c > +++ b/xen/arch/x86/mm/mem_sharing.c > @@ -1847,6 +1847,8 @@ static int copy_special_pages(struct domain *cd, struct domain *d) > p2m_ram_rw, p2m->default_access, -1); > if ( rc ) > return rc; > + > + set_gpfn_from_mfn(mfn_x(new_mfn), gfn_x(old_gfn)); > } > } > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> CC Henry. This needs a view about a release ack. Cons: it's been broken since Xen 4.14 and we're very deep into the 4.18 code freeze. Pros: it's a bug and would clearly qualify for backport, and is in a niche feature so isn't plausibly going to adversely affect other users. ~Andrew
Hi all, > On Oct 20, 2023, at 07:48, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 18/10/2023 9:02 am, Tamas K Lengyel wrote: >> When mapping in the shared_info page to a fork the m2p entry wasn't set >> resulting in the shared_info being reset even when the fork reset was called >> with only reset_state and not reset_memory. This results in an extra >> unnecessary TLB flush. >> >> Fixes: 1a0000ac775 ("mem_sharing: map shared_info page to same gfn during fork") >> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> >> --- >> xen/arch/x86/mm/mem_sharing.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c >> index 94b6b782ef..142258f16a 100644 >> --- a/xen/arch/x86/mm/mem_sharing.c >> +++ b/xen/arch/x86/mm/mem_sharing.c >> @@ -1847,6 +1847,8 @@ static int copy_special_pages(struct domain *cd, struct domain *d) >> p2m_ram_rw, p2m->default_access, -1); >> if ( rc ) >> return rc; >> + >> + set_gpfn_from_mfn(mfn_x(new_mfn), gfn_x(old_gfn)); >> } >> } >> > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > > CC Henry. This needs a view about a release ack. > > Cons: it's been broken since Xen 4.14 and we're very deep into the 4.18 > code freeze. > > Pros: it's a bug and would clearly qualify for backport, and is in a > niche feature so isn't plausibly going to adversely affect other users. Given the fact that it will be backported anyway, I had the same discussion with Juergen in his thread [1]. So if we can bear the risk of delaying merging this patch for a week, would it be ok to wait for the release and backport this patch to the stable tree? Thanks! [1] https://lore.kernel.org/xen-devel/83E6DCF6-926C-43A6-94D2-EB3B2C444309@arm.com/ Kind regards, Henry > > ~Andrew
On 20/10/2023 2:14 am, Henry Wang wrote: > Hi all, > >> On Oct 20, 2023, at 07:48, Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> >> On 18/10/2023 9:02 am, Tamas K Lengyel wrote: >>> When mapping in the shared_info page to a fork the m2p entry wasn't set >>> resulting in the shared_info being reset even when the fork reset was called >>> with only reset_state and not reset_memory. This results in an extra >>> unnecessary TLB flush. >>> >>> Fixes: 1a0000ac775 ("mem_sharing: map shared_info page to same gfn during fork") >>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> >>> --- >>> xen/arch/x86/mm/mem_sharing.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c >>> index 94b6b782ef..142258f16a 100644 >>> --- a/xen/arch/x86/mm/mem_sharing.c >>> +++ b/xen/arch/x86/mm/mem_sharing.c >>> @@ -1847,6 +1847,8 @@ static int copy_special_pages(struct domain *cd, struct domain *d) >>> p2m_ram_rw, p2m->default_access, -1); >>> if ( rc ) >>> return rc; >>> + >>> + set_gpfn_from_mfn(mfn_x(new_mfn), gfn_x(old_gfn)); >>> } >>> } >>> >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> >> >> CC Henry. This needs a view about a release ack. >> >> Cons: it's been broken since Xen 4.14 and we're very deep into the 4.18 >> code freeze. >> >> Pros: it's a bug and would clearly qualify for backport, and is in a >> niche feature so isn't plausibly going to adversely affect other users. > Given the fact that it will be backported anyway, I had the same discussion with Juergen > in his thread [1]. So if we can bear the risk of delaying merging this patch for a week, > would it be ok to wait for the release and backport this patch to the stable tree? Thanks! > > [1] https://lore.kernel.org/xen-devel/83E6DCF6-926C-43A6-94D2-EB3B2C444309@arm.com/ > > Kind regards, > Henry That's fine. I'll pull this into my next branch for when the 4.19 tree opens. ~Andrew
On Fri, Oct 20, 2023 at 6:57 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 20/10/2023 2:14 am, Henry Wang wrote: > > Hi all, > > > >> On Oct 20, 2023, at 07:48, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > >> > >> On 18/10/2023 9:02 am, Tamas K Lengyel wrote: > >>> When mapping in the shared_info page to a fork the m2p entry wasn't set > >>> resulting in the shared_info being reset even when the fork reset was called > >>> with only reset_state and not reset_memory. This results in an extra > >>> unnecessary TLB flush. > >>> > >>> Fixes: 1a0000ac775 ("mem_sharing: map shared_info page to same gfn during fork") > >>> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> > >>> --- > >>> xen/arch/x86/mm/mem_sharing.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c > >>> index 94b6b782ef..142258f16a 100644 > >>> --- a/xen/arch/x86/mm/mem_sharing.c > >>> +++ b/xen/arch/x86/mm/mem_sharing.c > >>> @@ -1847,6 +1847,8 @@ static int copy_special_pages(struct domain *cd, struct domain *d) > >>> p2m_ram_rw, p2m->default_access, -1); > >>> if ( rc ) > >>> return rc; > >>> + > >>> + set_gpfn_from_mfn(mfn_x(new_mfn), gfn_x(old_gfn)); > >>> } > >>> } > >>> > >> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> > >> > >> CC Henry. This needs a view about a release ack. > >> > >> Cons: it's been broken since Xen 4.14 and we're very deep into the 4.18 > >> code freeze. > >> > >> Pros: it's a bug and would clearly qualify for backport, and is in a > >> niche feature so isn't plausibly going to adversely affect other users. > > Given the fact that it will be backported anyway, I had the same discussion with Juergen > > in his thread [1]. So if we can bear the risk of delaying merging this patch for a week, > > would it be ok to wait for the release and backport this patch to the stable tree? Thanks! > > > > [1] https://lore.kernel.org/xen-devel/83E6DCF6-926C-43A6-94D2-EB3B2C444309@arm.com/ > > > > Kind regards, > > Henry > > That's fine. I'll pull this into my next branch for when the 4.19 tree > opens. Thanks, I agree that backporting is perfectly fine for this one. Tamas
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c index 94b6b782ef..142258f16a 100644 --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -1847,6 +1847,8 @@ static int copy_special_pages(struct domain *cd, struct domain *d) p2m_ram_rw, p2m->default_access, -1); if ( rc ) return rc; + + set_gpfn_from_mfn(mfn_x(new_mfn), gfn_x(old_gfn)); } }
When mapping in the shared_info page to a fork the m2p entry wasn't set resulting in the shared_info being reset even when the fork reset was called with only reset_state and not reset_memory. This results in an extra unnecessary TLB flush. Fixes: 1a0000ac775 ("mem_sharing: map shared_info page to same gfn during fork") Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com> --- xen/arch/x86/mm/mem_sharing.c | 2 ++ 1 file changed, 2 insertions(+)