Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()
On 23/04/18 15:39, Andrew Cooper wrote: > On 20/04/18 16:03, Jan Beulich wrote: >> hvm_domain_initialise() may call this with nr being zero, which triggers >> the "does not cross L3 boundary" check. >> >> Signed-off-by: Jan Beulich> > Acked-by: Andrew Cooper > Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()
On 20/04/18 16:03, Jan Beulich wrote: > hvm_domain_initialise() may call this with nr being zero, which triggers > the "does not cross L3 boundary" check. > > Signed-off-by: Jan BeulichAcked-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()
>>> On 20.04.18 at 17:42,wrote: > On 20/04/18 16:27, Jan Beulich wrote: > On 20.04.18 at 17:21, wrote: >>> On 20/04/18 16:03, Jan Beulich wrote: hvm_domain_initialise() may call this with nr being zero, which triggers the "does not cross L3 boundary" check. Signed-off-by: Jan Beulich >>> Is this the correct fix? >>> >>> Its unclear what the call to create_perdomain_mapping() is doing, but it >>> is the sole caller which passes 0. >>> >>> I presume it is to allocate d->arch.perdomain_l3_pg but >>> destroy_perdomain_mapping() doesn't free this page, so I've got no idea >>> what the destroy call is trying to achieve. >> Callers of this pair of functions should not be concerned about their >> inner workings: If the create call succeeded, the destroy function should >> be called for cleanup, even if that's _currently_ a no-op. > > Right, but its still not clear that passing 0 (even to create) is a > legitimate thing to do. Well, the create function specifically handles the nr == 0 case, so it's intended to be called that way (and, as you've said, exclusively for setting up d->arch.perdomain_l3_pg). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()
On 20/04/18 16:27, Jan Beulich wrote: On 20.04.18 at 17:21,wrote: >> On 20/04/18 16:03, Jan Beulich wrote: >>> hvm_domain_initialise() may call this with nr being zero, which triggers >>> the "does not cross L3 boundary" check. >>> >>> Signed-off-by: Jan Beulich >> Is this the correct fix? >> >> Its unclear what the call to create_perdomain_mapping() is doing, but it >> is the sole caller which passes 0. >> >> I presume it is to allocate d->arch.perdomain_l3_pg but >> destroy_perdomain_mapping() doesn't free this page, so I've got no idea >> what the destroy call is trying to achieve. > Callers of this pair of functions should not be concerned about their > inner workings: If the create call succeeded, the destroy function should > be called for cleanup, even if that's _currently_ a no-op. Right, but its still not clear that passing 0 (even to create) is a legitimate thing to do. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()
>>> On 20.04.18 at 17:21,wrote: > On 20/04/18 16:03, Jan Beulich wrote: >> hvm_domain_initialise() may call this with nr being zero, which triggers >> the "does not cross L3 boundary" check. >> >> Signed-off-by: Jan Beulich > > Is this the correct fix? > > Its unclear what the call to create_perdomain_mapping() is doing, but it > is the sole caller which passes 0. > > I presume it is to allocate d->arch.perdomain_l3_pg but > destroy_perdomain_mapping() doesn't free this page, so I've got no idea > what the destroy call is trying to achieve. Callers of this pair of functions should not be concerned about their inner workings: If the create call succeeded, the destroy function should be called for cleanup, even if that's _currently_ a no-op. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86: correct assertion in destroy_perdomain_mapping()
hvm_domain_initialise() may call this with nr being zero, which triggers the "does not cross L3 boundary" check. Signed-off-by: Jan Beulich--- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -5475,7 +5475,7 @@ void destroy_perdomain_mapping(struct do ASSERT(va >= PERDOMAIN_VIRT_START && va < PERDOMAIN_VIRT_SLOT(PERDOMAIN_SLOTS)); -ASSERT(!l3_table_offset(va ^ (va + nr * PAGE_SIZE - 1))); +ASSERT(!nr || !l3_table_offset(va ^ (va + nr * PAGE_SIZE - 1))); if ( !d->arch.perdomain_l3_pg ) return; ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel