xen/arch/x86/mm/mem_sharing.c | 3 --- xen/drivers/passthrough/pci.c | 2 +- xen/include/asm-x86/hvm/domain.h | 3 +++ 3 files changed, 4 insertions(+), 4 deletions(-)
...in assign_device().
The check of arch.hvm.mem_sharing_enabled flag at the top of
drivers/passthrough/pci.c:assign_device() does not make sure that the
domain is actually an HVM domain.
This patch fixes the issue by moving the definition of the
mem_sharing_enabled() macro from mem_sharing.c into asm-86/hvm/domain.h and
then using that instead.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Tamas K Lengyel <tamas@tklengyel.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
---
xen/arch/x86/mm/mem_sharing.c | 3 ---
xen/drivers/passthrough/pci.c | 2 +-
xen/include/asm-x86/hvm/domain.h | 3 +++
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index f16a3f5324..db02f4dfd5 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -149,9 +149,6 @@ static inline shr_handle_t get_next_handle(void)
return x + 1;
}
-#define mem_sharing_enabled(d) \
- (is_hvm_domain(d) && (d)->arch.hvm.mem_sharing_enabled)
-
static atomic_t nr_saved_mfns = ATOMIC_INIT(0);
static atomic_t nr_shared_mfns = ATOMIC_INIT(0);
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index e88689425d..7697ec440d 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1450,7 +1450,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
/* Prevent device assign if mem paging or mem sharing have been
* enabled for this domain */
- if ( unlikely(d->arch.hvm.mem_sharing_enabled ||
+ if ( unlikely(mem_sharing_enabled(d) ||
vm_event_check_ring(d->vm_event_paging) ||
p2m_get_hostp2m(d)->global_logdirty) )
return -EXDEV;
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 6c7c4f5aa6..1e6e3ce078 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -197,8 +197,11 @@ struct hvm_domain {
#ifdef CONFIG_HVM
#define hap_enabled(d) (is_hvm_domain(d) && (d)->arch.hvm.hap_enabled)
+#define mem_sharing_enabled(d) \
+ (is_hvm_domain(d) && (d)->arch.hvm.mem_sharing_enabled)
#else
#define hap_enabled(d) ({(void)(d); false;})
+#define mem_sharing_enabled(d) ({(void)(d); false;})
#endif
#endif /* __ASM_X86_HVM_DOMAIN_H__ */
--
2.20.1.2.gb21ebb671
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-develOn 01.07.2019 15:17, Paul Durrant wrote: > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -1450,7 +1450,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag) > > /* Prevent device assign if mem paging or mem sharing have been > * enabled for this domain */ > - if ( unlikely(d->arch.hvm.mem_sharing_enabled || > + if ( unlikely(mem_sharing_enabled(d) || > vm_event_check_ring(d->vm_event_paging) || > p2m_get_hostp2m(d)->global_logdirty) ) > return -EXDEV; This change is redundant with the more extensive one by "x86/HVM: p2m_ram_ro is incompatible with device pass-through", of which I've sent v2 earlier today, and v1 of which has been pending for quite some time without having heard back from other than you. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message----- > From: Jan Beulich <JBeulich@suse.com> > Sent: 03 July 2019 14:20 > To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; George Dunlap > <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; WeiLiu <wl@xen.org> > Subject: Re: [PATCH] passthrough/pci: properly qualify the mem_sharing_enabled check... > > On 01.07.2019 15:17, Paul Durrant wrote: > > --- a/xen/drivers/passthrough/pci.c > > +++ b/xen/drivers/passthrough/pci.c > > @@ -1450,7 +1450,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 > flag) > > > > /* Prevent device assign if mem paging or mem sharing have been > > * enabled for this domain */ > > - if ( unlikely(d->arch.hvm.mem_sharing_enabled || > > + if ( unlikely(mem_sharing_enabled(d) || > > vm_event_check_ring(d->vm_event_paging) || > > p2m_get_hostp2m(d)->global_logdirty) ) > > return -EXDEV; > > This change is redundant with the more extensive one by > "x86/HVM: p2m_ram_ro is incompatible with device pass-through", > of which I've sent v2 earlier today, and v1 of which has been > pending for quite some time without having heard back from > other than you. Agreed, but I still think it's a good idea to move the mem_sharing_enabled() macro into domain.h, so maybe incorporate that into your patch? I'll take a look at v2 as soon as I can. Paul > > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
On 03.07.2019 15:25, Paul Durrant wrote: >> -----Original Message----- >> From: Jan Beulich <JBeulich@suse.com> >> Sent: 03 July 2019 14:20 >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; George Dunlap >> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; WeiLiu <wl@xen.org> >> Subject: Re: [PATCH] passthrough/pci: properly qualify the mem_sharing_enabled check... >> >> On 01.07.2019 15:17, Paul Durrant wrote: >>> --- a/xen/drivers/passthrough/pci.c >>> +++ b/xen/drivers/passthrough/pci.c >>> @@ -1450,7 +1450,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 >> flag) >>> >>> /* Prevent device assign if mem paging or mem sharing have been >>> * enabled for this domain */ >>> - if ( unlikely(d->arch.hvm.mem_sharing_enabled || >>> + if ( unlikely(mem_sharing_enabled(d) || >>> vm_event_check_ring(d->vm_event_paging) || >>> p2m_get_hostp2m(d)->global_logdirty) ) >>> return -EXDEV; >> >> This change is redundant with the more extensive one by >> "x86/HVM: p2m_ram_ro is incompatible with device pass-through", >> of which I've sent v2 earlier today, and v1 of which has been >> pending for quite some time without having heard back from >> other than you. > > Agreed, but I still think it's a good idea to move the > mem_sharing_enabled() macro into domain.h, so maybe incorporate > that into your patch? Hmm, that would actually have been a question of mine if I hadn't recognized the redundancy: Why domain.h rather than mem_sharing.h? Furthermore I'd rather not use it here anyway, to avoid two consecutive is_hvm_*() checks (which have become slightly more expensive with the LFENCE addition). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
> -----Original Message----- > From: Jan Beulich <JBeulich@suse.com> > Sent: 03 July 2019 14:30 > To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Roger Pau > Monne <roger.pau@citrix.com>; Tamas KLengyel <tamas@tklengyel.com>; WeiLiu <wl@xen.org> > Subject: Re: [PATCH] passthrough/pci: properly qualify the mem_sharing_enabled check... > > On 03.07.2019 15:25, Paul Durrant wrote: > >> -----Original Message----- > >> From: Jan Beulich <JBeulich@suse.com> > >> Sent: 03 July 2019 14:20 > >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org > >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; George > Dunlap > >> <George.Dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; WeiLiu <wl@xen.org> > >> Subject: Re: [PATCH] passthrough/pci: properly qualify the mem_sharing_enabled check... > >> > >> On 01.07.2019 15:17, Paul Durrant wrote: > >>> --- a/xen/drivers/passthrough/pci.c > >>> +++ b/xen/drivers/passthrough/pci.c > >>> @@ -1450,7 +1450,7 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 > >> flag) > >>> > >>> /* Prevent device assign if mem paging or mem sharing have been > >>> * enabled for this domain */ > >>> - if ( unlikely(d->arch.hvm.mem_sharing_enabled || > >>> + if ( unlikely(mem_sharing_enabled(d) || > >>> vm_event_check_ring(d->vm_event_paging) || > >>> p2m_get_hostp2m(d)->global_logdirty) ) > >>> return -EXDEV; > >> > >> This change is redundant with the more extensive one by > >> "x86/HVM: p2m_ram_ro is incompatible with device pass-through", > >> of which I've sent v2 earlier today, and v1 of which has been > >> pending for quite some time without having heard back from > >> other than you. > > > > Agreed, but I still think it's a good idea to move the > > mem_sharing_enabled() macro into domain.h, so maybe incorporate > > that into your patch? > > Hmm, that would actually have been a question of mine if I hadn't > recognized the redundancy: Why domain.h rather than mem_sharing.h? > Furthermore I'd rather not use it here anyway, to avoid two > consecutive is_hvm_*() checks (which have become slightly more > expensive with the LFENCE addition). Maybe mem_sharing.h is a more logical location, but I think providing a macro to make sure the is_hvm_domain() protection is there is a good thing. As for using it here, yes I guess the addition of the speculation barrier makes it more expensive but then is not really performance critical code so I'd go for tidiness. Paul > > Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
© 2016 - 2024 Red Hat, Inc.