| Message ID | 1502310866-10450-4-git-send-email-tianyu.lan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Aug 09, 2017 at 04:34:04PM -0400, Lan Tianyu wrote: > This patch is to add get_irq_info callback for platform implementation > to convert irq remapping request to irq info (E,G vector, dest, dest_mode > and so on). > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > --- > xen/common/viommu.c | 16 ++++++++++++++++ > xen/include/asm-x86/viommu.h | 8 ++++++++ > xen/include/xen/viommu.h | 9 +++++++++ > 3 files changed, 33 insertions(+) > > diff --git a/xen/common/viommu.c b/xen/common/viommu.c > index f4d34e6..03c879d 100644 > --- a/xen/common/viommu.c > +++ b/xen/common/viommu.c > @@ -213,6 +213,22 @@ int viommu_handle_irq_request(struct domain *d, u32 viommu_id, > return info->viommu[viommu_id]->ops->handle_irq_request(d, request); > } > > +int viommu_get_irq_info(struct domain *d, u32 viommu_id, Again, uint32_t please. Please fix all these in this series. > + struct irq_remapping_request *request, > + struct irq_remapping_info *irq_info) > +{ > + struct viommu_info *info = &d->viommu; Having skimmed the rest of this series, there is no addition of viommu to struct domain (no change to sched.h). Did I miss something obvious? > + > + if ( viommu_id >= info->nr_viommu > + || !info->viommu[viommu_id] ) > + return -EINVAL; > + > + if ( !info->viommu[viommu_id]->ops->get_irq_info ) > + return -EINVAL; > + > + return info->viommu[viommu_id]->ops->get_irq_info(d, request, irq_info); Same comments in previous patch apply here, too. > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h [...] > +static inline int viommu_get_irq_info(struct domain *d, u32 viommu_id, > + struct irq_remapping_request *request, > + struct irq_remapping_info *irq_info) > +{ return 0 }; This should fail, too. > #endif > > #endif /* __XEN_VIOMMU_H__ */ > -- > 1.8.3.1 >
On Wed, Aug 09, 2017 at 04:34:04PM -0400, Lan Tianyu wrote: > This patch is to add get_irq_info callback for platform implementation > to convert irq remapping request to irq info (E,G vector, dest, dest_mode > and so on). > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> > --- > xen/common/viommu.c | 16 ++++++++++++++++ > xen/include/asm-x86/viommu.h | 8 ++++++++ > xen/include/xen/viommu.h | 9 +++++++++ > 3 files changed, 33 insertions(+) > > diff --git a/xen/common/viommu.c b/xen/common/viommu.c > index f4d34e6..03c879d 100644 > --- a/xen/common/viommu.c > +++ b/xen/common/viommu.c > @@ -213,6 +213,22 @@ int viommu_handle_irq_request(struct domain *d, u32 viommu_id, > return info->viommu[viommu_id]->ops->handle_irq_request(d, request); > } > > +int viommu_get_irq_info(struct domain *d, u32 viommu_id, > + struct irq_remapping_request *request, > + struct irq_remapping_info *irq_info) The definition of this struct seems to be arch-specific, in which case IMHO it should be called arch_irq_remapping_info, in order to denote it's arch-specific. > +{ > + struct viommu_info *info = &d->viommu; > + > + if ( viommu_id >= info->nr_viommu > + || !info->viommu[viommu_id] ) Unneeded line break. Roger.
On 2017年08月22日 23:38, Roger Pau Monné wrote: > On Wed, Aug 09, 2017 at 04:34:04PM -0400, Lan Tianyu wrote: >> This patch is to add get_irq_info callback for platform implementation >> to convert irq remapping request to irq info (E,G vector, dest, dest_mode >> and so on). >> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> >> --- >> xen/common/viommu.c | 16 ++++++++++++++++ >> xen/include/asm-x86/viommu.h | 8 ++++++++ >> xen/include/xen/viommu.h | 9 +++++++++ >> 3 files changed, 33 insertions(+) >> >> diff --git a/xen/common/viommu.c b/xen/common/viommu.c >> index f4d34e6..03c879d 100644 >> --- a/xen/common/viommu.c >> +++ b/xen/common/viommu.c >> @@ -213,6 +213,22 @@ int viommu_handle_irq_request(struct domain *d, u32 viommu_id, >> return info->viommu[viommu_id]->ops->handle_irq_request(d, request); >> } >> >> +int viommu_get_irq_info(struct domain *d, u32 viommu_id, >> + struct irq_remapping_request *request, >> + struct irq_remapping_info *irq_info) > > The definition of this struct seems to be arch-specific, in which case > IMHO it should be called arch_irq_remapping_info, in order to denote > it's arch-specific. OK. Will update. > >> +{ >> + struct viommu_info *info = &d->viommu; >> + >> + if ( viommu_id >= info->nr_viommu >> + || !info->viommu[viommu_id] ) > > Unneeded line break. > > Roger. >
>>> On 22.08.17 at 17:38, <roger.pau@citrix.com> wrote: > On Wed, Aug 09, 2017 at 04:34:04PM -0400, Lan Tianyu wrote: >> --- a/xen/common/viommu.c >> +++ b/xen/common/viommu.c >> @@ -213,6 +213,22 @@ int viommu_handle_irq_request(struct domain *d, u32 viommu_id, >> return info->viommu[viommu_id]->ops->handle_irq_request(d, request); >> } >> >> +int viommu_get_irq_info(struct domain *d, u32 viommu_id, >> + struct irq_remapping_request *request, >> + struct irq_remapping_info *irq_info) > > The definition of this struct seems to be arch-specific, in which case > IMHO it should be called arch_irq_remapping_info, in order to denote > it's arch-specific. In which case it also wouldn't belong in this file. Jan
diff --git a/xen/common/viommu.c b/xen/common/viommu.c index f4d34e6..03c879d 100644 --- a/xen/common/viommu.c +++ b/xen/common/viommu.c @@ -213,6 +213,22 @@ int viommu_handle_irq_request(struct domain *d, u32 viommu_id, return info->viommu[viommu_id]->ops->handle_irq_request(d, request); } +int viommu_get_irq_info(struct domain *d, u32 viommu_id, + struct irq_remapping_request *request, + struct irq_remapping_info *irq_info) +{ + struct viommu_info *info = &d->viommu; + + if ( viommu_id >= info->nr_viommu + || !info->viommu[viommu_id] ) + return -EINVAL; + + if ( !info->viommu[viommu_id]->ops->get_irq_info ) + return -EINVAL; + + return info->viommu[viommu_id]->ops->get_irq_info(d, request, irq_info); +} + /* * Local variables: * mode: C diff --git a/xen/include/asm-x86/viommu.h b/xen/include/asm-x86/viommu.h index 51bda72..1e8d4be 100644 --- a/xen/include/asm-x86/viommu.h +++ b/xen/include/asm-x86/viommu.h @@ -27,6 +27,14 @@ #define VIOMMU_REQUEST_IRQ_MSI 0 #define VIOMMU_REQUEST_IRQ_APIC 1 +struct irq_remapping_info +{ + u8 vector; + u32 dest; + u32 dest_mode:1; + u32 delivery_mode:3; +}; + struct irq_remapping_request { union { diff --git a/xen/include/xen/viommu.h b/xen/include/xen/viommu.h index 0be1b3a..0badeae 100644 --- a/xen/include/xen/viommu.h +++ b/xen/include/xen/viommu.h @@ -32,6 +32,8 @@ struct viommu_ops { int (*destroy)(struct viommu *viommu); int (*handle_irq_request)(struct domain *d, struct irq_remapping_request *request); + int (*get_irq_info)(struct domain *d, struct irq_remapping_request *request, + struct irq_remapping_info *info); }; struct viommu { @@ -58,6 +60,9 @@ int viommu_domctl(struct domain *d, struct xen_domctl_viommu_op *op, int viommu_setup(void); int viommu_handle_irq_request(struct domain *d, u32 viommu_id, struct irq_remapping_request *request); +int viommu_get_irq_info(struct domain *d, u32 viommu_id, + struct irq_remapping_request *request, + struct irq_remapping_info *irq_info); #else static inline int viommu_init_domain(struct domain *d) { return 0; } static inline int viommu_register_type(u64 type, struct viommu_ops * ops) @@ -71,6 +76,10 @@ static inline int viommu_domctl(struct domain *d, static inline int viommu_handle_irq_request(struct domain *d, u32 viommu_id, struct irq_remapping_request *request) { return 0 }; +static inline int viommu_get_irq_info(struct domain *d, u32 viommu_id, + struct irq_remapping_request *request, + struct irq_remapping_info *irq_info) +{ return 0 }; #endif #endif /* __XEN_VIOMMU_H__ */
This patch is to add get_irq_info callback for platform implementation to convert irq remapping request to irq info (E,G vector, dest, dest_mode and so on). Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> --- xen/common/viommu.c | 16 ++++++++++++++++ xen/include/asm-x86/viommu.h | 8 ++++++++ xen/include/xen/viommu.h | 9 +++++++++ 3 files changed, 33 insertions(+)