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

[Blue Swirl <blauwirbel@xxxxxxxxx>]

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
>
> diff --git a/Makefile.target b/Makefile.target
> index 1984f58..6b390e6 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -188,6 +188,7 @@ obj-$(CONFIG_NO_XEN) += xen-stub.o
>
> Â# xen full virtualized machine
> Âobj-$(CONFIG_XEN) += xen_machine_fv.o
> +obj-$(CONFIG_XEN) += xen_platform.o
>
> Â# USB layer
> Âobj-$(CONFIG_USB_OHCI) += usb-ohci.o
> diff --git a/hw/hw.h b/hw/hw.h
> index 4405092..67f3369 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -653,6 +653,9 @@ extern const VMStateDescription vmstate_i2c_slave;
> Â#define VMSTATE_INT32_LE(_f, _s) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_le, int32_t)
>
> +#define VMSTATE_UINT8_TEST(_f, _s, _t) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> + Â ÂVMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_uint8, uint8_t)
> +
> Â#define VMSTATE_UINT16_TEST(_f, _s, _t) Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
> Â Â VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_uint16, uint16_t)
>
> diff --git a/hw/pci_ids.h b/hw/pci_ids.h
> index 39e9f1d..1f2e0dd 100644
> --- a/hw/pci_ids.h
> +++ b/hw/pci_ids.h
> @@ -105,3 +105,5 @@
> Â#define PCI_DEVICE_ID_INTEL_82371AB Â Â Â0x7111
> Â#define PCI_DEVICE_ID_INTEL_82371AB_2 Â Â0x7112
> Â#define PCI_DEVICE_ID_INTEL_82371AB_3 Â Â0x7113
> +
> +#define PCI_VENDOR_ID_XENSOURCE Â Â Â Â Â0x5853
> diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
> index 03683c7..65fd44a 100644
> --- a/hw/xen_machine_fv.c
> +++ b/hw/xen_machine_fv.c
> @@ -34,6 +34,7 @@
> Â#include "blockdev.h"
>
> Â#include "xen/hvm/hvm_info_table.h"
> +#include "xen_platform.h"
>
> Â#define MAX_IDE_BUS 2
>
> @@ -87,6 +88,8 @@ static void xen_init_fv(ram_addr_t ram_size,
>
> Â Â pc_vga_init(pci_bus);
>
> + Â Âpci_xen_platform_init(pci_bus);
> +
> Â Â /* init basic PC hardware */
> Â Â pc_basic_device_init(isa_irq, &floppy_controller, &rtc_state);
>
> diff --git a/hw/xen_platform.c b/hw/xen_platform.c
> new file mode 100644
> index 0000000..15b490a
> --- /dev/null
> +++ b/hw/xen_platform.c
> @@ -0,0 +1,455 @@
> +/*
> + * XEN platform pci device, formerly known as the event channel device
> + *
> + * Copyright (c) 2003-2004 Intel Corp.
> + * Copyright (c) 2006 XenSource
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw.h"
> +#include "pc.h"
> +#include "pci.h"
> +#include "irq.h"
> +#include "xen_common.h"
> +#include "net.h"
> +#include "xen_platform.h"
> +#include "xen_backend.h"
> +#include "qemu-log.h"
> +
> +#include <assert.h>
> +#include <xenguest.h>
> +
> +//#define PLATFORM_DEBUG
> +
> +#ifdef PLATFORM_DEBUG
> +#define DPRINTF(fmt, ...) do { \
> + Â Âfprintf(stderr, "xen_platform: " fmt, ## __VA_ARGS__); \
> +} while (0)
> +#else
> +#define DPRINTF(fmt, ...) do { } while (0)
> +#endif
> +
> +#define PFFLAG_ROM_LOCK 1 /* Sets whether ROM memory area is RW or RO */
> +
> +typedef struct PCIXenPlatformState {
> + Â ÂPCIDevice Âpci_dev;
> + Â Âuint8_t flags; /* used only for version_id == 2 */
> + Â Âint drivers_blacklisted;
> + Â Âuint16_t driver_product_version;
> +
> + Â Â/* Log from guest drivers */
> + Â Âint throttling_disabled;
> + Â Âchar log_buffer[4096];
> + Â Âint log_buffer_off;
> +} PCIXenPlatformState;
> +
> +#define XEN_PLATFORM_IOPORT 0x10
> +
> +/* 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?

> + Â Â Â Âif (available > BUCKET_MAX_SIZE)
> + Â Â Â Â Â Âavailable = BUCKET_MAX_SIZE;
> + Â Â Â Âlast_refil = now;
> + Â Â}
> +
> + Â Âassert(available >= count);

Is it possible to trigger this from the guest?

> +
> + Â Âavailable -= count;
> +}
> +
> +/* Xen Platform, Fixed IOPort */
> +
> +static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, 
> uint32_t val)
> +{
> + Â ÂPCIXenPlatformState *s = opaque;
> +
> + Â Âswitch (addr - XEN_PLATFORM_IOPORT) {
> + Â Âcase 0:
> + Â Â Â Â/* TODO: */
> + Â Â Â Â/* Unplug devices. ÂValue is a bitmask of which devices to
> + Â Â Â Â Â unplug, with bit 0 the IDE devices, bit 1 the network
> + Â Â Â Â Â devices, and bit 2 the non-primary-master IDE devices. */
> + Â Â Â Âbreak;
> + Â Âcase 2:
> + Â Â Â Âswitch (val) {
> + Â Â Â Âcase 1:
> + Â Â Â Â Â ÂDPRINTF("Citrix Windows PV drivers loaded in guest\n");
> + Â Â Â Â Â Âbreak;
> + Â Â Â Âcase 0:
> + Â Â Â Â Â ÂDPRINTF("Guest claimed to be running PV product 0?\n");
> + Â Â Â Â Â Âbreak;
> + Â Â Â Âdefault:
> + Â Â Â Â Â ÂDPRINTF("Unknown PV product %d loaded in guest\n", val);
> + Â Â Â Â Â Âbreak;
> + Â Â Â Â}
> + Â Â Â Âs->driver_product_version = val;
> + Â Â Â Âbreak;
> + Â Â}
> +}
> +
> +static void platform_fixed_ioport_writel(void *opaque, uint32_t addr,
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â uint32_t val)
> +{
> + Â Âswitch (addr - XEN_PLATFORM_IOPORT) {
> + Â Âcase 0:
> + Â Â Â Â/* PV driver version */
> + Â Â Â Âbreak;
> + Â Â}
> +}
> +
> +static void platform_fixed_ioport_writeb(void *opaque, uint32_t addr, 
> uint32_t val)
> +{
> + Â ÂPCIXenPlatformState *s = opaque;
> +
> + Â Âswitch (addr - XEN_PLATFORM_IOPORT) {
> + Â Âcase 0: /* Platform flags */ {
> + Â Â Â Âhvmmem_type_t mem_type = (val & PFFLAG_ROM_LOCK) ?
> + Â Â Â Â Â ÂHVMMEM_ram_ro : HVMMEM_ram_rw;
> + Â Â Â Âif (xc_hvm_set_mem_type(xen_xc, xen_domid, mem_type, 0xc0, 0x40))
> + Â Â Â Â Â ÂDPRINTF("unable to change ro/rw state of ROM memory area!\n");

braces

> + Â Â Â Âelse {
> + Â Â Â Â Â Âs->flags = val & PFFLAG_ROM_LOCK;
> + Â Â Â Â Â ÂDPRINTF("changed ro/rw state of ROM memory area. now is %s 
> state.\n",
> + Â Â Â Â Â Â Â Â Â Â(mem_type == HVMMEM_ram_ro ? "ro":"rw"));
> + Â Â Â Â}
> + Â Â Â Âbreak;
> + Â Â}
> + Â Âcase 2:
> + Â Â Â Â/* Send bytes to syslog */
> + Â Â Â Âif (val == '\n' || s->log_buffer_off == sizeof(s->log_buffer) - 1) {
> + Â Â Â Â Â Â/* Flush buffer */
> + Â Â Â Â Â Âs->log_buffer[s->log_buffer_off] = 0;
> + Â Â Â Â Â Âthrottle(s, s->log_buffer_off);
> + Â Â Â Â Â ÂDPRINTF("%s\n", s->log_buffer);
> + Â Â Â Â Â Âs->log_buffer_off = 0;
> + Â Â Â Â Â Âbreak;
> + Â Â Â Â}
> + Â Â Â Âs->log_buffer[s->log_buffer_off++] = val;
> + Â Â Â Âbreak;
> + Â Â}
> +}
> +
> +static uint32_t platform_fixed_ioport_readw(void *opaque, uint32_t addr)
> +{
> + Â ÂPCIXenPlatformState *s = opaque;
> +
> + Â Âswitch (addr - XEN_PLATFORM_IOPORT) {
> + Â Âcase 0:
> + Â Â Â Âif (s->drivers_blacklisted) {
> + Â Â Â Â Â Â/* The drivers will recognise this magic number and refuse
> + Â Â Â Â Â Â * to do anything. */
> + Â Â Â Â Â Âreturn 0xd249;
> + Â Â Â Â} else {
> + Â Â Â Â Â Â/* Magic value so that you can identify the interface. */
> + Â Â Â Â Â Âreturn 0x49d2;
> + Â Â Â Â}
> + Â Âdefault:
> + Â Â Â Âreturn 0xffff;
> + Â Â}
> +}
> +
> +static uint32_t platform_fixed_ioport_readb(void *opaque, uint32_t addr)
> +{
> + Â ÂPCIXenPlatformState *s = opaque;
> +
> + Â Âswitch (addr - XEN_PLATFORM_IOPORT) {
> + Â Âcase 0:
> + Â Â Â Â/* Platform flags */
> + Â Â Â Âreturn s->flags;
> + Â Âcase 2:
> + Â Â Â Â/* Version number */
> + Â Â Â Âreturn 1;
> + Â Âdefault:
> + Â Â Â Âreturn 0xff;
> + Â Â}
> +}
> +
> +static void platform_fixed_ioport_reset(void *opaque)
> +{
> + Â ÂPCIXenPlatformState *s = opaque;
> +
> + Â Âplatform_fixed_ioport_writeb(s, XEN_PLATFORM_IOPORT, 0);
> +}
> +
> +static void platform_fixed_ioport_init(PCIXenPlatformState* s)
> +{
> + Â Âregister_ioport_write(XEN_PLATFORM_IOPORT, 16, 4, 
> platform_fixed_ioport_writel, s);
> + Â Âregister_ioport_write(XEN_PLATFORM_IOPORT, 16, 2, 
> platform_fixed_ioport_writew, s);
> + Â Âregister_ioport_write(XEN_PLATFORM_IOPORT, 16, 1, 
> platform_fixed_ioport_writeb, s);
> + Â Âregister_ioport_read(XEN_PLATFORM_IOPORT, 16, 2, 
> platform_fixed_ioport_readw, s);
> + Â Âregister_ioport_read(XEN_PLATFORM_IOPORT, 16, 1, 
> platform_fixed_ioport_readb, s);
> +}
> +
> +/* Xen Platform PCI Device */
> +
> +static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr)
> +{
> + Â Âaddr &= 0xff;
> +
> + Â Âif (addr == 0)
> + Â Â Â Âreturn platform_fixed_ioport_readb(opaque, XEN_PLATFORM_IOPORT);

braces

> + Â Âelse
> + Â Â Â Âreturn ~0u;
> +}
> +
> +static void xen_platform_ioport_writeb(void *opaque, uint32_t addr, uint32_t 
> val)
> +{
> + Â ÂPCIXenPlatformState *s = opaque;
> +
> + Â Âaddr &= 0xff;
> + Â Âval Â&= 0xff;
> +
> + Â Âswitch (addr) {
> + Â Âcase 0: /* Platform flags */
> + Â Â Â Âplatform_fixed_ioport_writeb(opaque, XEN_PLATFORM_IOPORT, val);
> + Â Â Â Âbreak;
> + Â Âcase 8:
> + Â Â Â Â{
> + Â Â Â Â Â Âif (val == '\n' || s->log_buffer_off == sizeof(s->log_buffer) - 
> 1) {
> + Â Â Â Â Â Â Â Â/* Flush buffer */
> + Â Â Â Â Â Â Â Âs->log_buffer[s->log_buffer_off] = 0;
> + Â Â Â Â Â Â Â Âthrottle(s, s->log_buffer_off);
> + Â Â Â Â Â Â Â ÂDPRINTF("%s\n", s->log_buffer);
> + Â Â Â Â Â Â Â Âs->log_buffer_off = 0;
> + Â Â Â Â Â Â Â Âbreak;
> + Â Â Â Â Â Â}
> + Â Â Â Â Â Âs->log_buffer[s->log_buffer_off++] = val;
> + Â Â Â Â}
> + Â Â Â Âbreak;
> + Â Âdefault:
> + Â Â Â Âbreak;
> + Â Â}
> +}
> +
> +static void platform_ioport_map(PCIDevice *pci_dev, int region_num, pcibus_t 
> addr, pcibus_t size, int type)
> +{
> + Â ÂPCIXenPlatformState *d = DO_UPCAST(PCIXenPlatformState, pci_dev, 
> pci_dev);
> +
> + Â Âregister_ioport_write(addr, size, 1, xen_platform_ioport_writeb, d);
> + Â Âregister_ioport_read(addr, size, 1, xen_platform_ioport_readb, d);
> +}
> +
> +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.

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