|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] xen: Introduce PHYSDEVOP_pirq_eoi_gmfn_new
On 26/01/2012 15:49, "Stefano Stabellini" <Stefano.Stabellini@xxxxxxxxxxxxx>
wrote:
> PHYSDEVOP_pirq_eoi_gmfn changes the semantics of PHYSDEVOP_eoi.
> Introduce PHYSDEVOP_pirq_eoi_gmfn_new, that is like
> PHYSDEVOP_pirq_eoi_gmfn but it doesn't modify the behaviour of another
> hypercall.
It's nasty that pirq_eoi_gmfn has the side effect. I suggest add a PHYSDEVOP
to explicitly enable/disable unmask-on-eoi (i.e., the command accepts a
boolean parameter). Once it is explicitly enabled/disabled in this way,
pirq_eoi_gmfn no longer has the side effect (regardless of whether it is
called before or after the explicit setting). So e.g., pv_domain.auto_unmask
becomes an int where 0/1 means no/yes, and -1 means default (i.e., old
behavour where it depends on whether PHYSDEVOP_pirq_eoi_gmfn has been
called).
This seems to me to move a bad interface in a better direction.
-- Keir
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> ---
> xen/arch/ia64/xen/domain.c | 1 +
> xen/arch/ia64/xen/hypercall.c | 5 ++++-
> xen/arch/x86/domain.c | 1 +
> xen/arch/x86/physdev.c | 6 +++++-
> xen/include/asm-ia64/domain.h | 3 +++
> xen/include/asm-x86/domain.h | 3 +++
> xen/include/public/physdev.h | 8 ++++++++
> 7 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/ia64/xen/domain.c b/xen/arch/ia64/xen/domain.c
> index 1ea5a90..a31bd32 100644
> --- a/xen/arch/ia64/xen/domain.c
> +++ b/xen/arch/ia64/xen/domain.c
> @@ -1731,6 +1731,7 @@ int domain_relinquish_resources(struct domain *d)
> if (d->arch.pirq_eoi_map != NULL) {
> put_page(virt_to_page(d->arch.pirq_eoi_map));
> d->arch.pirq_eoi_map = NULL;
> + d->arch.auto_unmask = 0;
> }
>
> /* Tear down shadow mode stuff. */
> diff --git a/xen/arch/ia64/xen/hypercall.c b/xen/arch/ia64/xen/hypercall.c
> index 130675e..44c3407 100644
> --- a/xen/arch/ia64/xen/hypercall.c
> +++ b/xen/arch/ia64/xen/hypercall.c
> @@ -65,7 +65,7 @@ static long __do_pirq_guest_eoi(struct domain *d, int pirq)
> {
> if ( pirq < 0 || pirq >= NR_IRQS )
> return -EINVAL;
> - if ( d->arch.pirq_eoi_map ) {
> + if ( d->arch.pirq_eoi_map && d->arch.auto_unmask ) {
> spin_lock(&d->event_lock);
> evtchn_unmask(pirq_to_evtchn(d, pirq));
> spin_unlock(&d->event_lock);
> @@ -508,6 +508,7 @@ long do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> break;
> }
>
> + case PHYSDEVOP_pirq_eoi_gmfn_new:
> case PHYSDEVOP_pirq_eoi_gmfn: {
> struct physdev_pirq_eoi_gmfn info;
> unsigned long mfn;
> @@ -531,6 +532,8 @@ long do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> }
>
> current->domain->arch.pirq_eoi_map = mfn_to_virt(mfn);
> + if ( cmd == PHYSDEVOP_pirq_eoi_gmfn )
> + current->domain->arch.auto_unmask = 1;
> ret = 0;
> break;
> }
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index 61d83c8..a540af7 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -2125,6 +2125,7 @@ int domain_relinquish_resources(struct domain *d)
> put_page_and_type(
> mfn_to_page(d->arch.pv_domain.pirq_eoi_map_mfn));
> d->arch.pv_domain.pirq_eoi_map = NULL;
> + d->arch.pv_domain.auto_unmask = 0;
> }
> }
>
> diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
> index f280c28..efd517f 100644
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -271,7 +271,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> break;
> }
> if ( !is_hvm_domain(v->domain) &&
> - v->domain->arch.pv_domain.pirq_eoi_map )
> + v->domain->arch.pv_domain.pirq_eoi_map &&
> + v->domain->arch.pv_domain.auto_unmask )
> evtchn_unmask(pirq->evtchn);
> if ( !is_hvm_domain(v->domain) ||
> domain_pirq_to_irq(v->domain, eoi.irq) > 0 )
> @@ -293,6 +294,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> break;
> }
>
> + case PHYSDEVOP_pirq_eoi_gmfn_new:
> case PHYSDEVOP_pirq_eoi_gmfn: {
> struct physdev_pirq_eoi_gmfn info;
> unsigned long mfn;
> @@ -329,6 +331,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE(void) arg)
> ret = -ENOSPC;
> break;
> }
> + if ( cmd == PHYSDEVOP_pirq_eoi_gmfn )
> + v->domain->arch.pv_domain.auto_unmask = 1;
>
> put_gfn(current->domain, info.gmfn);
> ret = 0;
> diff --git a/xen/include/asm-ia64/domain.h b/xen/include/asm-ia64/domain.h
> index 12dc3bd..8163d67 100644
> --- a/xen/include/asm-ia64/domain.h
> +++ b/xen/include/asm-ia64/domain.h
> @@ -186,6 +186,9 @@ struct arch_domain {
> /* Shared page for notifying that explicit PIRQ EOI is required. */
> unsigned long *pirq_eoi_map;
> unsigned long pirq_eoi_map_mfn;
> + /* set auto_unmask to 1 if you want PHYSDEVOP_eoi to automatically
> + * unmask the event channel */
> + unsigned int auto_unmask;
>
> /* Address of efi_runtime_services_t (placed in domain memory) */
> void *efi_runtime;
> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
> index 00bbaeb..b0cbe65 100644
> --- a/xen/include/asm-x86/domain.h
> +++ b/xen/include/asm-x86/domain.h
> @@ -231,6 +231,9 @@ struct pv_domain
> /* Shared page for notifying that explicit PIRQ EOI is required. */
> unsigned long *pirq_eoi_map;
> unsigned long pirq_eoi_map_mfn;
> + /* set auto_unmask to 1 if you want PHYSDEVOP_eoi to automatically
> + * unmask the event channel */
> + unsigned int auto_unmask;
>
> /* Pseudophysical e820 map (XENMEM_memory_map). */
> spinlock_t e820_lock;
> diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
> index 6e23295..d555719 100644
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -50,6 +50,14 @@ DEFINE_XEN_GUEST_HANDLE(physdev_eoi_t);
> * array indexed by Xen's PIRQ value.
> */
> #define PHYSDEVOP_pirq_eoi_gmfn 17
> +/*
> + * Register a shared page for the hypervisor to indicate whether the
> + * guest must issue PHYSDEVOP_eoi. This hypercall is very similar to
> + * PHYSDEVOP_pirq_eoi_gmfn but it doesn't change the semantics of
> + * PHYSDEVOP_eoi. The page registered is used as a bit array indexed by
> + * Xen's PIRQ value.
> + */
> +#define PHYSDEVOP_pirq_eoi_gmfn_new 28
> struct physdev_pirq_eoi_gmfn {
> /* IN */
> xen_pfn_t gmfn;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |