| Message ID | 20190903161428.7159-5-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ioreq: add support for internal servers | expand |
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Roger Pau Monne > Sent: 03 September 2019 17:14 > To: xen-devel@lists.xenproject.org > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org>; Jan Beulich <jbeulich@suse.com>; > Roger Pau Monne <roger.pau@citrix.com> > Subject: [Xen-devel] [PATCH v2 04/11] ioreq: add fields to allow internal ioreq servers > > Internal ioreq servers are plain function handlers implemented inside > of the hypervisor. Note that most fields used by current (external) > ioreq servers are not needed for internal ones, and hence have been > placed inside of a struct and packed in an union together with the > only internal specific field, a function pointer to a handler. > > This is required in order to have PCI config accesses forwarded to > external ioreq servers or to internal ones (ie: QEMU emulated devices > vs vPCI passthrough), and is the first step in order to allow > unprivileged domains to use vPCI. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Paul Durrant <paul.durrant@citrix.com> > --- > Changes since v1: > - Do not add an internal field to the ioreq server struct, whether a > server is internal or external can already be inferred from the id. > - Add an extra parameter to the internal handler in order to pass > user-provided opaque data to the handler. > --- > xen/include/asm-x86/hvm/domain.h | 30 +++++++++++++++++++----------- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h > index bcc5621797..9fbe83f45a 100644 > --- a/xen/include/asm-x86/hvm/domain.h > +++ b/xen/include/asm-x86/hvm/domain.h > @@ -52,21 +52,29 @@ struct hvm_ioreq_vcpu { > #define MAX_NR_IO_RANGES 256 > > struct hvm_ioreq_server { > - struct domain *target, *emulator; > - > + struct domain *target; > /* Lock to serialize toolstack modifications */ > spinlock_t lock; > - > - struct hvm_ioreq_page ioreq; > - struct list_head ioreq_vcpu_list; > - struct hvm_ioreq_page bufioreq; > - > - /* Lock to serialize access to buffered ioreq ring */ > - spinlock_t bufioreq_lock; > - evtchn_port_t bufioreq_evtchn; > struct rangeset *range[NR_IO_RANGE_TYPES]; > bool enabled; > - uint8_t bufioreq_handling; > + > + union { > + struct { > + struct domain *emulator; > + struct hvm_ioreq_page ioreq; > + struct list_head ioreq_vcpu_list; > + struct hvm_ioreq_page bufioreq; > + > + /* Lock to serialize access to buffered ioreq ring */ > + spinlock_t bufioreq_lock; > + evtchn_port_t bufioreq_evtchn; > + uint8_t bufioreq_handling; > + }; > + struct { > + void *data; > + int (*handler)(struct vcpu *v, ioreq_t *, void *); > + }; > + }; > }; > > /* > -- > 2.22.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel
On 03.09.2019 18:14, Roger Pau Monne wrote: > --- a/xen/include/asm-x86/hvm/domain.h > +++ b/xen/include/asm-x86/hvm/domain.h > @@ -52,21 +52,29 @@ struct hvm_ioreq_vcpu { > #define MAX_NR_IO_RANGES 256 > > struct hvm_ioreq_server { > - struct domain *target, *emulator; > - > + struct domain *target; > /* Lock to serialize toolstack modifications */ > spinlock_t lock; > - > - struct hvm_ioreq_page ioreq; > - struct list_head ioreq_vcpu_list; > - struct hvm_ioreq_page bufioreq; > - > - /* Lock to serialize access to buffered ioreq ring */ > - spinlock_t bufioreq_lock; > - evtchn_port_t bufioreq_evtchn; > struct rangeset *range[NR_IO_RANGE_TYPES]; > bool enabled; > - uint8_t bufioreq_handling; > + > + union { > + struct { > + struct domain *emulator; > + struct hvm_ioreq_page ioreq; > + struct list_head ioreq_vcpu_list; > + struct hvm_ioreq_page bufioreq; > + > + /* Lock to serialize access to buffered ioreq ring */ > + spinlock_t bufioreq_lock; > + evtchn_port_t bufioreq_evtchn; > + uint8_t bufioreq_handling; > + }; > + struct { > + void *data; > + int (*handler)(struct vcpu *v, ioreq_t *, void *); If you omit the latter two parameter names, the first one should be omitted, too. And if there was to be any inconsistency in this regard, then the one parameter where the type doesn't immediately clarify the purpose would be the one to have a name. As to the struct vcpu * parameter - is there an expectation that the handler would be called with this being other than "current"? If not, the parameter would want to either be dropped, or be named "curr". Jan
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h index bcc5621797..9fbe83f45a 100644 --- a/xen/include/asm-x86/hvm/domain.h +++ b/xen/include/asm-x86/hvm/domain.h @@ -52,21 +52,29 @@ struct hvm_ioreq_vcpu { #define MAX_NR_IO_RANGES 256 struct hvm_ioreq_server { - struct domain *target, *emulator; - + struct domain *target; /* Lock to serialize toolstack modifications */ spinlock_t lock; - - struct hvm_ioreq_page ioreq; - struct list_head ioreq_vcpu_list; - struct hvm_ioreq_page bufioreq; - - /* Lock to serialize access to buffered ioreq ring */ - spinlock_t bufioreq_lock; - evtchn_port_t bufioreq_evtchn; struct rangeset *range[NR_IO_RANGE_TYPES]; bool enabled; - uint8_t bufioreq_handling; + + union { + struct { + struct domain *emulator; + struct hvm_ioreq_page ioreq; + struct list_head ioreq_vcpu_list; + struct hvm_ioreq_page bufioreq; + + /* Lock to serialize access to buffered ioreq ring */ + spinlock_t bufioreq_lock; + evtchn_port_t bufioreq_evtchn; + uint8_t bufioreq_handling; + }; + struct { + void *data; + int (*handler)(struct vcpu *v, ioreq_t *, void *); + }; + }; }; /*
Internal ioreq servers are plain function handlers implemented inside of the hypervisor. Note that most fields used by current (external) ioreq servers are not needed for internal ones, and hence have been placed inside of a struct and packed in an union together with the only internal specific field, a function pointer to a handler. This is required in order to have PCI config accesses forwarded to external ioreq servers or to internal ones (ie: QEMU emulated devices vs vPCI passthrough), and is the first step in order to allow unprivileged domains to use vPCI. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - Do not add an internal field to the ioreq server struct, whether a server is internal or external can already be inferred from the id. - Add an extra parameter to the internal handler in order to pass user-provided opaque data to the handler. --- xen/include/asm-x86/hvm/domain.h | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)