Re: [Xen-devel] [PATCH 05/11] xen pci platform device driver

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 10 May 2010, Jeremy Fitzhardinge wrote:
> On 05/10/2010 07:20 AM, Stefano Stabellini wrote:
> > Add the xen pci platform device driver that is responsible
> > for initializing the grant table and xenbus in PV on HVM mode.
> > Few changes to xenbus and grant table are necessary to allow the delayed
> > initialization in HVM mode.
> > Grant table needs few additional modifications to work in HVM mode.
> >
> > When running on HVM the event channel upcall is never called while in
> > progress because it is a normal Linux irq handler, therefore we cannot
> > be sure that evtchn_upcall_pending is 0 when returning.
> >   
> 
> Won't the event (and the interrupt it raises) still be pending when the
> handler returns?
> 

Reading handle_fasteoi_irq it doesn't seem like it.

> > For this reason if evtchn_upcall_pending is set by Xen we need to loop
> > again on the event channels set pending otherwise we might loose some
> > event channel deliveries.
> >   
> 
> See below...
> 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > Signed-off-by: Sheng Yang <sheng@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/xen/Kconfig                  |   11 ++-
> >  drivers/xen/Makefile                 |    3 +-
> >  drivers/xen/events.c                 |    5 +-
> >  drivers/xen/grant-table.c            |   70 +++++++++-
> >  drivers/xen/platform-pci.c           |  236 
> > ++++++++++++++++++++++++++++++++++
> >  drivers/xen/xenbus/xenbus_probe.c    |   20 ++-
> >  include/xen/grant_table.h            |    1 +
> >  include/xen/interface/grant_table.h  |    1 +
> >  include/xen/interface/platform_pci.h |   45 +++++++
> >  include/xen/platform_pci.h           |   41 ++++++
> >  include/xen/xenbus.h                 |    1 +
> >  11 files changed, 417 insertions(+), 17 deletions(-)
> >  create mode 100644 drivers/xen/platform-pci.c
> >  create mode 100644 include/xen/interface/platform_pci.h
> >  create mode 100644 include/xen/platform_pci.h
> >
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index cab100a..3e02457 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -60,4 +60,13 @@ config XEN_SYS_HYPERVISOR
> >           Create entries under /sys/hypervisor describing the Xen
> >      hypervisor environment.  When running native or in another
> >      virtual environment, /sys/hypervisor will still be present,
> > -    but will have no xen contents.
> > \ No newline at end of file
> > +    but will have no xen contents.
> > +
> > +config XEN_PLATFORM_PCI
> > +   tristate "xen platform pci device driver"
> > +   depends on XEN
> > +   help
> > +     Driver for the Xen PCI Platform device: it is responsible for
> > +     initializing xenbus and grant_table when running in a Xen HVM
> > +     domain. As a consequence this driver is required to run any Xen PV
> > +     frontend on Xen HVM.
> > diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> > index 7c28434..2a502d2 100644
> > --- a/drivers/xen/Makefile
> > +++ b/drivers/xen/Makefile
> > @@ -9,4 +9,5 @@ obj-$(CONFIG_XEN_XENCOMM)   += xencomm.o
> >  obj-$(CONFIG_XEN_BALLOON)  += balloon.o
> >  obj-$(CONFIG_XEN_DEV_EVTCHN)       += evtchn.o
> >  obj-$(CONFIG_XENFS)                += xenfs/
> > -obj-$(CONFIG_XEN_SYS_HYPERVISOR)   += sys-hypervisor.o
> > \ No newline at end of file
> > +obj-$(CONFIG_XEN_SYS_HYPERVISOR)   += sys-hypervisor.o
> > +obj-$(CONFIG_XEN_PLATFORM_PCI)             += platform-pci.o
> > diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> > index cd609f4..197ccbc 100644
> > --- a/drivers/xen/events.c
> > +++ b/drivers/xen/events.c
> > @@ -662,7 +662,7 @@ void __xen_evtchn_do_upcall(struct pt_regs *regs)
> >  
> >             count = __get_cpu_var(xed_nesting_count);
> >             __get_cpu_var(xed_nesting_count) = 0;
> > -   } while(count != 1);
> > +   } while(count != 1 || vcpu_info->evtchn_upcall_pending);
> >   
> 
> So the intention here is to pick up newly posted events while the event
> processing loop is running?  But if this is necessary it won't work,
> because it is still racy.  For example, what happens if a new event is
> posted right here, just after the loop has exited?  Will it get
> ignored?  Or if it does need to be processed, why is the test in the
> loop necessary?
> 

Your example is not going to cause any trouble because in that case Xen
will also send an interrupt to the guest that is going to have local irq
disable at the time.
When the first interrupt handling routing is finished, local irq will be
enabled and the second interrupt will be served, therefore no event
channels will be lost.

The problem that I am trying to solve is making sure that when
evtchn_upcall_pending is set by Xen and an interrupt is sent by Xen to
the guest, we (the guest) handle the event channel.
In particular the problem was that during event channel handling we
re-enable irqs so a second irq may be served but the second handler
wouldn't go as far as resetting evtchn_upcall_pending because as soon as
it realizes that another one is in progress the fasteoi handler will exits.
Therefore we have to reset evtchn_upcall_pending and handle the event
channel directly in the first handler. Since this only happens when we
re-enable irqs we can make sure that we'll be able to handle the new
event channel in the same loop.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel