Re: [PATCH 7/7] qemu: delayed MMIO support (e1000)

[Laurent Vivier <Laurent.Vivier@bull.net>]

Le lundi 26 mai 2008 à 17:05 +0300, Avi Kivity a écrit :
> Laurent Vivier wrote:
> > This patch defines delayed MMIO zones for e1000 ethernet card.
> >
> > Signed-off-by: Laurent Vivier <Laurent.Vivier@bull.net>
> > ---
> >  qemu/hw/e1000.c |   17 +++++++++++++++++
> >  1 files changed, 17 insertions(+), 0 deletions(-)
> >
> > diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
> > index 01f8983..97f46f6 100644
> > --- a/qemu/hw/e1000.c
> > +++ b/qemu/hw/e1000.c
> > @@ -26,6 +26,7 @@
> >  #include "hw.h"
> >  #include "pci.h"
> >  #include "net.h"
> > +#include "qemu-kvm.h"
> >  
> >  #include "e1000_hw.h"
> >  
> > @@ -938,6 +939,22 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
> >  
> >      d->mmio_base = addr;
> >      cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
> > +
> > +    if (kvm_enabled()) {
> > +        /* delayed zones are:
> > +         * 0x00000 -> 0x000C0        exclude: E1000_ICR,4
> > +         * 0x000C4 -> 0x00400        exclude: E1000_TCTL,4
> > +         * 0x00404 -> 0x03818        exclude: E1000_TDT,4
> > +         * 0x0381C -> 0x20000        PNPMMIO_SIZE
> > +         */
> > +        qemu_kvm_register_delayed_mmio(addr, E1000_ICR);
> > +        qemu_kvm_register_delayed_mmio(addr + E1000_ICR + 4,
> > +                                       E1000_TCTL - (E1000_ICR + 4));
> > +        qemu_kvm_register_delayed_mmio(addr + E1000_TCTL + 4,
> > +                                       E1000_TDT - (E1000_TCTL + 4));
> > +        qemu_kvm_register_delayed_mmio(addr + E1000_TDT + 4,
> > +                                       PNPMMIO_SIZE - (E1000_TDT + 4));
> > +    }
> >  }
> >  
> 
> What about the calls to set_ics()?  They can cause interrupts to be 
> deasserted AFAICS.  So IMC (and maybe a few other registers) need to be 
> excluded as well.

Well, I don't know how I missed them...

> Instead of hardcoding this, how about a
> 
> uint32_t delayed_mmi_excluded_regs[] = {
>    E1000_ICR, E1000_TCTL, ...
> };
> 
> The code can sort them and generate the ranges automatically.
> 
> (alternatively, a boolmap:
> 
> bool delayed_mmi_excluded_regs[] = {
>   [E1000_ICR]: true, ...
> };
> )

I'll try...

Regards,
Laurent
-- 
------------- Laurent.Vivier@bull.net ---------------
"The best way to predict the future is to invent it."
- Alan Kay