WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [Qemu-devel] [PATCH RFC V3 04/12] xen: Add the Xen platf

On Fri, 17 Sep 2010, Blue Swirl wrote:

> On Fri, Sep 17, 2010 at 11:14 AM,  <anthony.perard@xxxxxxxxxx> wrote:
> > From: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> >
> > Introduce a new emulated PCI device, specific to fully virtualized Xen
> > guests.  The device is necessary for PV on HVM drivers to work.
> >
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > ---
> >  Makefile.target     |    1 +
> >  hw/hw.h             |    3 +
> >  hw/pci_ids.h        |    2 +
> >  hw/xen_machine_fv.c |    3 +
> >  hw/xen_platform.c   |  455 
> > +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/xen_platform.h   |    8 +
> >  6 files changed, 472 insertions(+), 0 deletions(-)
> >  create mode 100644 hw/xen_platform.c
> >  create mode 100644 hw/xen_platform.h

[...]

> > +/* We throttle access to dom0 syslog, to avoid DOS attacks.  This is
> > +   modelled as a token bucket, with one token for every byte of log.
> > +   The bucket size is 128KB (->1024 lines of 128 bytes each) and
> > +   refills at 256B/s.  It starts full.  The guest is blocked if no
> > +   tokens are available when it tries to generate a log message. */
> > +#define BUCKET_MAX_SIZE (128*1024)
> > +#define BUCKET_FILL_RATE 256
> > +
> > +static void throttle(PCIXenPlatformState *s, unsigned count)
> > +{
> > +    static unsigned available;
> > +    static struct timespec last_refil;
>
> last_refill
>
> > +    static int started;
> > +    static int warned;
> > +
> > +    struct timespec waiting_for, now;
> > +    double delay;
> > +    struct timespec ts;
> > +
> > +    if (s->throttling_disabled)
> > +        return;
>
> Braces should be added here and other places.
>
> > +
> > +    if (!started) {
> > +        clock_gettime(CLOCK_MONOTONIC, &last_refil);
> > +        available = BUCKET_MAX_SIZE;
> > +        started = 1;
> > +    }
> > +
> > +    if (count > BUCKET_MAX_SIZE) {
> > +        DPRINTF("tried to get %d tokens, but bucket size is %d\n",
>
> count is unsigned, so %u.
>
> > +                BUCKET_MAX_SIZE, count);
> > +        exit(1);
> > +    }
> > +
> > +    if (available < count) {
> > +        /* The bucket is empty.  Refil it */
> > +
> > +        /* When will it be full enough to handle this request? */
> > +        delay = (double)(count - available) / BUCKET_FILL_RATE;
> > +        waiting_for = last_refil;
> > +        waiting_for.tv_sec += delay;
> > +        waiting_for.tv_nsec += (delay - (int)delay) * 1e9;
> > +        if (waiting_for.tv_nsec >= 1000000000) {
> > +            waiting_for.tv_nsec -= 1000000000;
> > +            waiting_for.tv_sec++;
> > +        }
> > +
> > +        /* How long do we have to wait? (might be negative) */
> > +        clock_gettime(CLOCK_MONOTONIC, &now);
> > +        ts.tv_sec = waiting_for.tv_sec - now.tv_sec;
> > +        ts.tv_nsec = waiting_for.tv_nsec - now.tv_nsec;
> > +        if (ts.tv_nsec < 0) {
> > +            ts.tv_sec--;
> > +            ts.tv_nsec += 1000000000;
> > +        }
> > +
> > +        /* Wait for it. */
> > +        if (ts.tv_sec > 0 ||
> > +            (ts.tv_sec == 0 && ts.tv_nsec > 0)) {
> > +            if (!warned) {
> > +                DPRINTF("throttling guest access to syslog");
> > +                warned = 1;
> > +            }
> > +            while (nanosleep(&ts, &ts) < 0 && errno == EINTR)
> > +                ;
>
> braces
>
> > +        }
> > +
> > +        /* Refil */
>
> Refill
>
> > +        clock_gettime(CLOCK_MONOTONIC, &now);
> > +        delay = (now.tv_sec - last_refil.tv_sec) +
> > +            (now.tv_nsec - last_refil.tv_nsec) * 1.0e-9;
> > +        available += BUCKET_FILL_RATE * delay;
>
> We have muldiv64(), perhaps it could be used here?

Ok, I use it, and I also use qemu_get_clock_ns(rt_clock).

> > +        if (available > BUCKET_MAX_SIZE)
> > +            available = BUCKET_MAX_SIZE;
> > +        last_refil = now;
> > +    }
> > +
> > +    assert(available >= count);
>
> Is it possible to trigger this from the guest?

I don't think we can do that for every guest.

> > +
> > +    available -= count;
> > +}
> > +

[...]

> > +static uint32_t platform_mmio_read(void *opaque, target_phys_addr_t addr)
> > +{
> > +    static int warnings = 0;
> > +
> > +    if (warnings < 5) {
> > +        DPRINTF("Warning: attempted read from physical address "
> > +                "0x" TARGET_FMT_plx " in xen platform mmio space\n", addr);
> > +        warnings++;
>
> Since DPRINTF only works in a specially compiled version, I'd remove
> these checks. There could also be additional debug flags besides
> PLATFORM_DEBUG to enable these warnings if these are too noisy, like
> DEBUG_MMIO. I'd rename PLATFORM_DEBUG to DEBUG_PLATFORM.

I will fix the coding style issue.

Thanks,

-- 
Anthony PERARD

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

<Prev in Thread] Current Thread [Next in Thread>