Re: [Xen-devel] [PATCH v5 02/29] Replace all occurances of __FUNCTION__ with __func__
On Mon, Nov 13, 2017 at 02:34:42PM -0800, Alistair Francis wrote: > Replace all occurs of __FUNCTION__ except for the check in checkpatch > with the non GCC specific __func__. > > One line in hcd-musb.c was manually tweaked to pass checkpatch. > > Signed-off-by: Alistair Francis > Cc: Gerd Hoffmann > Cc: Andrzej Zaborowski > Cc: Stefano Stabellini > Cc: Anthony Perard > Cc: John Snow > Cc: Aurelien Jarno > Cc: Yongbok Kim > Cc: Peter Crosthwaite > Cc: Stefan Hajnoczi > Cc: Fam Zheng > Cc: Juan Quintela > Cc: "Dr. David Alan Gilbert" > Cc: qemu-...@nongnu.org > Cc: qemu-bl...@nongnu.org > Cc: xen-de...@lists.xenproject.org > Reviewed-by: Eric Blake > Reviewed-by: Stefan Hajnoczi > Reviewed-by: Anthony PERARD > Reviewed-by: Juan Quintela Reviewed-by: Gerd Hoffmann ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v5 0/4] xenfb: Enablement for Windows PV HID frontend
On Fri, Nov 03, 2017 at 11:56:27AM +, Owen Smith wrote: > Improve the input device model in xenfb, by updating the > Qemu input handlers and adding a feature to allow for > raw (unscaled) absolute coordinates to be represented. Reviewed-by: Gerd Hoffmann Will this be merged via xen queue? If not I can queue it up in my input queue, but I'd like to have a review from the xen folks then. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/3 v4] xenfb: Add [feature|request]-raw-pointer
Hi, > It's probably OS specific though. I guess the behaviour changed > because the OS favours absolute pointing devices over relative ones > and how it has two absolute ones to choose from. How it reconciles > those, who knows? Typically hid emulation calls qemu_input_handler_activate() when the guest initializes the device, which moves the device to the top of the priority list. Visible effect on a typical guest with ps/2 mouse and usb-tablet is that qemu switches from relative mode (mouse) to absolute mode (tablet) when the guest loads the usb hid driver. I suspect pvmouse is doing the same thing. So it may simply depend on guest driver load order whenever pvmouse or usb-tablet is used. Simplest fix is probably to only attach the device you plan to use to the guest. If you can't turn off pvmouse for xen guests then you might want drop the qemu_input_handler_activate() call, so it behaves simliar to the ps/2 mouse (is used in case no other pointer device is present). HTH, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/3 v4] xenfb: Enablement for Windows PV HID frontend
On Tue, 2017-09-26 at 14:43 +, Owen Smith wrote: > Improve the input device model in xenfb, by updating the > Qemu input handlers and adding a feature to allow for > raw (unscaled) absolute coordinates to be represented. > > By using a reverse mapping call, the Linux input.h #defines > are not pulled into xenfb, and so should remove the compiler > warnings reported. Reviewed-by: Gerd Hoffmann What is the plan with this? Merge through xen queue? I can merge it via ui queue too, but in that case I'd like to get an review from the xen guys. Note: conflicts with keycodemapdb patch series, might need a rebase depending on merge order. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/4] ui/input: Add activate/remove for keyboard handlers
diff --git a/ui/input-legacy.c b/ui/input-legacy.c > index 7159747..fbe1ce7 100644 > --- a/ui/input-legacy.c > +++ b/ui/input-legacy.c > @@ -142,6 +142,18 @@ QEMUPutKbdEntry > *qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque) > return entry; > } > > +void qemu_activate_kbd_event_handler(QEMUPutKbdEntry *entry) Please don't add new code to input-legacy.c please. Switch your code to use the new qemu_input_handler_*() functions directly instead. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen: add qemu device for each pvusb backend
Hi, > > Hmm, I think the xen core needs better QOM support ... > > > > struct XenDevice should have a DeviceState element, so it can be used as > > device object directly instead of attaching a device object like > > this ... > > Hmm, interesting idea. The device object could even be added in > Xen common code if the backend is indicating the need for it via a > special flag/field. I'll have a try. No, not optional. Just turn *all* xen devices into QOM objects. XenDevice should probably a subclass of the base device object (DeviceState), and all Xen backends (block, net, fb, pvusb, ...) should be subclasses of XenDevice. The latter is probably how things are modeled already, just the QOM object stuff is missing (register classes, macros to cast objects, ...) because qdev (the QOM predecessor) didn't have that. Once this is in place you can simply use DEVICE(xendevice) to get the DeviceState pointer. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen: add qemu device for each pvusb backend
Hi, > struct usbback_info { > struct XenDevice xendev; /* must be first */ > +char id[24]; > +struct USBBACKDevice *dev; > USBBus bus; > void *urb_sring; > void *conn_sring; > @@ -116,6 +124,10 @@ struct usbback_info { > QEMUBH *bh; > }; > > +typedef struct USBBACKDevice { > +DeviceState qdev; > +} USBBACKDevice; Hmm, I think the xen core needs better QOM support ... struct XenDevice should have a DeviceState element, so it can be used as device object directly instead of attaching a device object like this ... cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] xen: add an own bus for xen backend devices
On Mo, 2016-09-26 at 14:43 +0200, Juergen Gross wrote: > Add a bus for Xen backend devices in order to be able to establish a > dedicated device path for pluggable devices. Looks sane to me. Can take this through the usb queue if I get an ack from xen. > +#define TYPE_XENSYSDEV "xensysdev" > +#define TYPE_XENSYSBUS "xen-sysbus" I'd make this consistent and use the dash either for both or not at all. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/2] Xen pvUSB correction
On Mo, 2016-09-26 at 14:43 +0200, Juergen Gross wrote: > Trying to use pvUSB in a Xen guest with a qemu emulated USB controller > will crash qemu as it tries to attach a pvUSB device to the emulated > controller. Hmm. --verbose please. While this clearly doesn't do what you intended I think it should not have crashed qemu. pvUSB devices should work on emulated controller (and emulated devices should work on the pvUSB controller). If they don't you probably taking shortcuts somewhere which work only for the pvUSB device on pvUSB controller case. Please check. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [RFC for 2.8 0/3] Drop support for 64 bit guests on 32 bit hosts
On Di, 2016-08-09 at 16:55 +0100, Alex Bennée wrote: > Hi, > > I'm proposing for the 2.8 cycle we officially drop supporting 64 bit > guests on 32 bit hosts. For most of the KVM targets it doesn't make > any sense anyway and for TCG it makes things harder (e.g. supporting > 64 bit atomics on a 32 bit platform). I'm not actually convinced > things actually work if built or that anyone relies on these > combinations. Consider these patches a way of flushing any such users > out ;-) Adding xen-devel to Cc. 64bit xen hypervisor with 32bit dom0 running 32bit qemu, providing device emulation for 64bit dumUs at least used to be a common setup years ago. I have my doubts this is still the case in recent xen versions, but I guess we better ask the Xen guys ... Can anyone clarify? thanks, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PULL 4/6] xen: when removing a backend don't remove many of them
From: Juergen Gross When a Xenstore watch fires indicating a backend has to be removed don't remove all backends for that domain with the specified device index, but just the one which has the correct type. The easiest way to achieve this is to use the already determined xendev as parameter for xen_be_del_xendev() instead of only the domid and device index. This at once removes the open coded QTAILQ_FOREACH_SAVE() in xen_be_del_xendev() as there is no need to search for the correct xendev any longer. Signed-off-by: Juergen Gross Reviewed-by: Stefano Stabellini Message-id: 1470140044-16492-2-git-send-email-jgr...@suse.com Signed-off-by: Gerd Hoffmann --- hw/xen/xen_backend.c | 58 +--- 1 file changed, 19 insertions(+), 39 deletions(-) diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index bab79b1..3ceb778 100644 --- a/hw/xen/xen_backend.c +++ b/hw/xen/xen_backend.c @@ -321,48 +321,28 @@ static struct XenDevice *xen_be_get_xendev(const char *type, int dom, int dev, /* * release xen backend device. */ -static struct XenDevice *xen_be_del_xendev(int dom, int dev) +static void xen_be_del_xendev(struct XenDevice *xendev) { -struct XenDevice *xendev, *xnext; - -/* - * This is pretty much like QTAILQ_FOREACH(xendev, &xendevs, next) but - * we save the next pointer in xnext because we might free xendev. - */ -xnext = xendevs.tqh_first; -while (xnext) { -xendev = xnext; -xnext = xendev->next.tqe_next; - -if (xendev->dom != dom) { -continue; -} -if (xendev->dev != dev && dev != -1) { -continue; -} - -if (xendev->ops->free) { -xendev->ops->free(xendev); -} - -if (xendev->fe) { -char token[XEN_BUFSIZE]; -snprintf(token, sizeof(token), "fe:%p", xendev); -xs_unwatch(xenstore, xendev->fe, token); -g_free(xendev->fe); -} +if (xendev->ops->free) { +xendev->ops->free(xendev); +} -if (xendev->evtchndev != NULL) { -xenevtchn_close(xendev->evtchndev); -} -if (xendev->gnttabdev != NULL) { -xengnttab_close(xendev->gnttabdev); -} +if (xendev->fe) { +char token[XEN_BUFSIZE]; +snprintf(token, sizeof(token), "fe:%p", xendev); +xs_unwatch(xenstore, xendev->fe, token); +g_free(xendev->fe); +} -QTAILQ_REMOVE(&xendevs, xendev, next); -g_free(xendev); +if (xendev->evtchndev != NULL) { +xenevtchn_close(xendev->evtchndev); } -return NULL; +if (xendev->gnttabdev != NULL) { +xengnttab_close(xendev->gnttabdev); +} + +QTAILQ_REMOVE(&xendevs, xendev, next); +g_free(xendev); } /* @@ -682,7 +662,7 @@ static void xenstore_update_be(char *watch, char *type, int dom, if (xendev != NULL) { bepath = xs_read(xenstore, 0, xendev->be, &len); if (bepath == NULL) { -xen_be_del_xendev(dom, dev); +xen_be_del_xendev(xendev); } else { free(bepath); xen_be_backend_changed(xendev, path); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PULL 6/6] xen: use a common function for pv and hvm guest backend register calls
From: Juergen Gross Instead of calling xen_be_register() for each supported backend type for hvm and pv guests in their machine init functions use a common function in order not to have to add new backends twice. This at once fixes the error that hvm domains couldn't use the qusb backend. Signed-off-by: Juergen Gross Acked-by: Anthony PERARD Message-id: 1470119552-16170-1-git-send-email-jgr...@suse.com Signed-off-by: Gerd Hoffmann --- hw/xen/xen_backend.c | 10 ++ hw/xenpv/xen_machine_pv.c| 7 +-- include/hw/xen/xen_backend.h | 1 + xen-hvm.c| 4 +--- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index 3ceb778..69a2388 100644 --- a/hw/xen/xen_backend.c +++ b/hw/xen/xen_backend.c @@ -780,6 +780,16 @@ int xen_be_register(const char *type, struct XenDevOps *ops) return xenstore_scan(type, xen_domid, ops); } +void xen_be_register_common(void) +{ +xen_be_register("console", &xen_console_ops); +xen_be_register("vkbd", &xen_kbdmouse_ops); +xen_be_register("qdisk", &xen_blkdev_ops); +#ifdef CONFIG_USB_LIBUSB +xen_be_register("qusb", &xen_usb_ops); +#endif +} + int xen_be_bind_evtchn(struct XenDevice *xendev) { if (xendev->local_port != -1) { diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c index 48f725c..79aef4e 100644 --- a/hw/xenpv/xen_machine_pv.c +++ b/hw/xenpv/xen_machine_pv.c @@ -67,14 +67,9 @@ static void xen_init_pv(MachineState *machine) break; } -xen_be_register("console", &xen_console_ops); -xen_be_register("vkbd", &xen_kbdmouse_ops); +xen_be_register_common(); xen_be_register("vfb", &xen_framebuffer_ops); -xen_be_register("qdisk", &xen_blkdev_ops); xen_be_register("qnic", &xen_netdev_ops); -#ifdef CONFIG_USB_LIBUSB -xen_be_register("qusb", &xen_usb_ops); -#endif /* configure framebuffer */ if (xenfb_enabled) { diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h index 754c0a4..0df282a 100644 --- a/include/hw/xen/xen_backend.h +++ b/include/hw/xen/xen_backend.h @@ -87,6 +87,7 @@ void xen_be_check_state(struct XenDevice *xendev); /* xen backend driver bits */ int xen_be_init(void); +void xen_be_register_common(void); int xen_be_register(const char *type, struct XenDevOps *ops); int xen_be_set_state(struct XenDevice *xendev, enum xenbus_state state); int xen_be_bind_evtchn(struct XenDevice *xendev); diff --git a/xen-hvm.c b/xen-hvm.c index eb57792..3b0343a 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -1318,9 +1318,7 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory) error_report("xen backend core setup failed"); goto err; } -xen_be_register("console", &xen_console_ops); -xen_be_register("vkbd", &xen_kbdmouse_ops); -xen_be_register("qdisk", &xen_blkdev_ops); +xen_be_register_common(); xen_read_physmap(state); return; -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: use a common function for pv and hvm guest backend register calls
On Di, 2016-08-02 at 08:32 +0200, Juergen Gross wrote: > Instead of calling xen_be_register() for each supported backend type > for hvm and pv guests in their machine init functions use a common > function in order not to have to add new backends twice. > > This at once fixes the error that hvm domains couldn't use the qusb > backend. Looks good to me. Should I take this through the usb patch queue, together with the other xen-usb fixes (once codestyle issues are fixed)? If so, can I get an ack from xen please, preferably fast enough for -rc2? thanks, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen: drain submit queue in xen-usb before removing device
On Fr, 2016-07-29 at 13:17 +0200, Juergen Gross wrote: > When unplugging a device in the Xen pvusb backend drain the submit > queue before deallocation of the control structures. Otherwise there > will be bogus memory accesses when I/O contracts are finished. > > Correlated to this issue is the handling of cancel requests: a packet > cancelled will still lead to the call of complete, so add a flag > A === checkpatch complains === WARNING: braces {} are necessary for all arms of this statement #105: FILE: hw/usb/xen-usb.c:696: +if (sched) [...] WARNING: braces {} are necessary for all arms of this statement #111: FILE: hw/usb/xen-usb.c:702: +if (!usbif->ports[port - 1].attached) [...] WARNING: braces {} are necessary for all arms of this statement #152: FILE: hw/usb/xen-usb.c:847: +if (usbif->ports[i].dev) [...] cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Regression with commit 095497ffc66b7f031
On Fr, 2016-07-15 at 12:02 +0200, Paolo Bonzini wrote: > > On 15/07/2016 10:47, Juergen Gross wrote: > > Nothing scaring and no real difference between working and not working > > variant. > > > > Meanwhile I've been digging a little bit deeper and found the reason: > > libxenstore is setting up a reader thread which is waiting for the > > watch to fire. With above commit the stack size of that thread (16kB) > > is too small. Setting it to 32kB made qemu work again. > > This makes very little sense (not your fault)... The commit doesn't > change stack usage at all, TLS should not be on the stack. > > Can you capture a backtrace where the 16K stack is exceeded? Perhaps > it's only due to inlining decision on the compiler, in which case > Peter's patch from today is only a bandaid. Hmm, I guess I hold off the vnc pull request for now ... cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xen: fix ram init regression
Commit "8156d48 pc: allow raising low memory via max-ram-below-4g option" causes a regression on xen, because it uses a different memory split. This patch initializes max-ram-below-4g to zero and leaves the initialization to the memory initialization functions. That way they can pick different default values (max-ram-below-4g is zero still) or use the user supplied value (max-ram-below-4g is non-zero). Also skip the whole ram split calculation on Xen. xen_ram_init() does its own split calculation anyway so it is superfluous, also this way xen_ram_init can actually see whenever max-ram-below-4g is zero or not. Reported-by: Anthony PERARD Tested-by: Anthony PERARD Signed-off-by: Gerd Hoffmann --- hw/i386/pc.c | 2 +- hw/i386/pc_piix.c | 52 +--- hw/i386/pc_q35.c | 3 +++ xen-hvm.c | 3 +++ 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 7198ed5..66e1dae 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1886,7 +1886,7 @@ static void pc_machine_initfn(Object *obj) pc_machine_get_hotplug_memory_region_size, NULL, NULL, NULL, &error_abort); -pcms->max_ram_below_4g = 0xe000; /* 3.5G */ +pcms->max_ram_below_4g = 0; /* use default */ object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size", pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g, diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 53bc968..f51fa77 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -108,37 +108,43 @@ static void pc_init1(MachineState *machine, *so legacy non-PAE guests can get as much memory as possible in *the 32bit address space below 4G. * + * - Note that Xen has its own ram setp code in xen_ram_init(), + *called via xen_hvm_init(). + * * Examples: *qemu -M pc-1.7 -m 4G(old default)-> 3584M low, 512M high *qemu -M pc -m 4G(new default)-> 3072M low, 1024M high *qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high *qemu -M pc,max-ram-below-4g=4G -m 3968M -> 3968M low (=4G-128M) */ -lowmem = pcms->max_ram_below_4g; -if (machine->ram_size >= pcms->max_ram_below_4g) { -if (pcmc->gigabyte_align) { -if (lowmem > 0xc000) { -lowmem = 0xc000; -} -if (lowmem & ((1ULL << 30) - 1)) { -error_report("Warning: Large machine and max_ram_below_4g " - "(%" PRIu64 ") not a multiple of 1G; " - "possible bad performance.", - pcms->max_ram_below_4g); -} -} -} - -if (machine->ram_size >= lowmem) { -pcms->above_4g_mem_size = machine->ram_size - lowmem; -pcms->below_4g_mem_size = lowmem; -} else { -pcms->above_4g_mem_size = 0; -pcms->below_4g_mem_size = machine->ram_size; -} - if (xen_enabled()) { xen_hvm_init(pcms, &ram_memory); +} else { +if (!pcms->max_ram_below_4g) { +pcms->max_ram_below_4g = 0xe000; /* default: 3.5G */ +} +lowmem = pcms->max_ram_below_4g; +if (machine->ram_size >= pcms->max_ram_below_4g) { +if (pcmc->gigabyte_align) { +if (lowmem > 0xc000) { +lowmem = 0xc000; +} +if (lowmem & ((1ULL << 30) - 1)) { +error_report("Warning: Large machine and max_ram_below_4g " + "(%" PRIu64 ") not a multiple of 1G; " + "possible bad performance.", + pcms->max_ram_below_4g); +} +} +} + +if (machine->ram_size >= lowmem) { +pcms->above_4g_mem_size = machine->ram_size - lowmem; +pcms->below_4g_mem_size = lowmem; +} else { +pcms->above_4g_mem_size = 0; +pcms->below_4g_mem_size = machine->ram_size; +} } pc_cpus_init(pcms); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index e4b541f..1b653e2 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -93,6 +93,9 @@ static void pc_q35_init(MachineState *machine) /* Handle the machine opt max-ram-below-4g. It is basically doing * min(qemu limit, user limit). */ +if (!pcms->max_ram_below_4g) { +pcms->max_ram_below_4g = 1ULL << 32; /* default: 4G */; +} if (lowmem > pcms->max_ram_below_4g) { lowmem
Re: [Xen-devel] Change of max-ram-below-4g initial value breaks Xen
On Do, 2016-06-23 at 17:18 +0100, Anthony PERARD wrote: > On Thu, Jun 23, 2016 at 04:57:54PM +0200, Gerd Hoffmann wrote: > > Hi, > > > > > How could xen_ram_init() find out if the value of max-ram-below-4g is > > > the default or if a user have set it? Is there another way we could fix > > > this? > > > > Attached patch should fix it. Patch survived a quick smoke test on kvm > > so far, need to do some more testing tomorrow. Can you give it a spin > > on xen? > > Thanks. Unfortunately, it does not work :(. > > In this patch, max_ram_below_4g is set before the call to xen_ram_init() > and xen_ram_init read it back (via object_property_get_int()). So, in > xen_ram_init, user_lowmem is not 0. Ah, I see. We do the split calculation twice on xen. That is pretty pointless. New patch attached. cheers, Gerd From a1bb0d4f7a94e97102e7ea72d0a65de2a17b1160 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 23 Jun 2016 16:49:03 +0200 Subject: [PATCH] xen: fix ram init regression Commit "8156d48 pc: allow raising low memory via max-ram-below-4g option" causes a regression on xen, because it uses a different memory split. This patch initializes max-ram-below-4g to zero and leaves the initialization to the memory initialization functions. That way they can pick different default values (max-ram-below-4g is zero still) or use the user supplied value (max-ram-below-4g is non-zero). Also skip the whole ram split calculation on Xen. xen_ram_init() does its own split calculation anyway so it is superfluous, also this way xen_ram_init can actually see whenever max-ram-below-4g is zero or not. Signed-off-by: Gerd Hoffmann --- hw/i386/pc.c | 2 +- hw/i386/pc_piix.c | 52 +--- hw/i386/pc_q35.c | 3 +++ xen-hvm.c | 3 +++ 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 7198ed5..66e1dae 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1886,7 +1886,7 @@ static void pc_machine_initfn(Object *obj) pc_machine_get_hotplug_memory_region_size, NULL, NULL, NULL, &error_abort); -pcms->max_ram_below_4g = 0xe000; /* 3.5G */ +pcms->max_ram_below_4g = 0; /* use default */ object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size", pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g, diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 53bc968..f51fa77 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -108,37 +108,43 @@ static void pc_init1(MachineState *machine, *so legacy non-PAE guests can get as much memory as possible in *the 32bit address space below 4G. * + * - Note that Xen has its own ram setp code in xen_ram_init(), + *called via xen_hvm_init(). + * * Examples: *qemu -M pc-1.7 -m 4G(old default)-> 3584M low, 512M high *qemu -M pc -m 4G(new default)-> 3072M low, 1024M high *qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high *qemu -M pc,max-ram-below-4g=4G -m 3968M -> 3968M low (=4G-128M) */ -lowmem = pcms->max_ram_below_4g; -if (machine->ram_size >= pcms->max_ram_below_4g) { -if (pcmc->gigabyte_align) { -if (lowmem > 0xc000) { -lowmem = 0xc000; -} -if (lowmem & ((1ULL << 30) - 1)) { -error_report("Warning: Large machine and max_ram_below_4g " - "(%" PRIu64 ") not a multiple of 1G; " - "possible bad performance.", - pcms->max_ram_below_4g); -} -} -} - -if (machine->ram_size >= lowmem) { -pcms->above_4g_mem_size = machine->ram_size - lowmem; -pcms->below_4g_mem_size = lowmem; -} else { -pcms->above_4g_mem_size = 0; -pcms->below_4g_mem_size = machine->ram_size; -} - if (xen_enabled()) { xen_hvm_init(pcms, &ram_memory); +} else { +if (!pcms->max_ram_below_4g) { +pcms->max_ram_below_4g = 0xe000; /* default: 3.5G */ +} +lowmem = pcms->max_ram_below_4g; +if (machine->ram_size >= pcms->max_ram_below_4g) { +if (pcmc->gigabyte_align) { +if (lowmem > 0xc000) { +lowmem = 0xc000; +} +if (lowmem & ((1ULL << 30) - 1)) { +error_report("Warning: Large machine and max_ram_below_4g " + "(%" PRIu64 ") not a multiple of
Re: [Xen-devel] Change of max-ram-below-4g initial value breaks Xen
Hi, > How could xen_ram_init() find out if the value of max-ram-below-4g is > the default or if a user have set it? Is there another way we could fix > this? Attached patch should fix it. Patch survived a quick smoke test on kvm so far, need to do some more testing tomorrow. Can you give it a spin on xen? thanks, Gerd From d45a95861def18a02e1c26d3717693432517107a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 23 Jun 2016 16:49:03 +0200 Subject: [PATCH] xen: fix ram init regression Commit "8156d48 pc: allow raising low memory via max-ram-below-4g option" causes a regression on xen, because it uses a different memory split. This patch initializes max-ram-below-4g to zero and leaves the initialization to the memory initialization functions. That way they can pick different default values (max-ram-below-4g is zero still) or use the user supplied value (max-ram-below-4g is non-zero). Signed-off-by: Gerd Hoffmann --- hw/i386/pc.c | 2 +- hw/i386/pc_piix.c | 3 +++ hw/i386/pc_q35.c | 3 +++ xen-hvm.c | 3 +++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 7198ed5..66e1dae 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1886,7 +1886,7 @@ static void pc_machine_initfn(Object *obj) pc_machine_get_hotplug_memory_region_size, NULL, NULL, NULL, &error_abort); -pcms->max_ram_below_4g = 0xe000; /* 3.5G */ +pcms->max_ram_below_4g = 0; /* use default */ object_property_add(obj, PC_MACHINE_MAX_RAM_BELOW_4G, "size", pc_machine_get_max_ram_below_4g, pc_machine_set_max_ram_below_4g, diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 53bc968..78e3d44 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -114,6 +114,9 @@ static void pc_init1(MachineState *machine, *qemu -M pc,max-ram-below-4g=2G -m 4G -> 2048M low, 2048M high *qemu -M pc,max-ram-below-4g=4G -m 3968M -> 3968M low (=4G-128M) */ +if (!pcms->max_ram_below_4g) { +pcms->max_ram_below_4g = 0xe000; /* default: 3.5G */ +} lowmem = pcms->max_ram_below_4g; if (machine->ram_size >= pcms->max_ram_below_4g) { if (pcmc->gigabyte_align) { diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index e4b541f..1b653e2 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -93,6 +93,9 @@ static void pc_q35_init(MachineState *machine) /* Handle the machine opt max-ram-below-4g. It is basically doing * min(qemu limit, user limit). */ +if (!pcms->max_ram_below_4g) { +pcms->max_ram_below_4g = 1ULL << 32; /* default: 4G */; +} if (lowmem > pcms->max_ram_below_4g) { lowmem = pcms->max_ram_below_4g; if (machine->ram_size - lowmem > lowmem && diff --git a/xen-hvm.c b/xen-hvm.c index 98ea44f..eb57792 100644 --- a/xen-hvm.c +++ b/xen-hvm.c @@ -190,6 +190,9 @@ static void xen_ram_init(PCMachineState *pcms, /* Handle the machine opt max-ram-below-4g. It is basically doing * min(xen limit, user limit). */ +if (!user_lowmem) { +user_lowmem = HVM_BELOW_4G_RAM_END; /* default */ +} if (HVM_BELOW_4G_RAM_END <= user_lowmem) { user_lowmem = HVM_BELOW_4G_RAM_END; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] Change of max-ram-below-4g initial value breaks Xen
Hi, > How could xen_ram_init() find out if the value of max-ram-below-4g is > the default or if a user have set it? Is there another way we could fix > this? I guess we'll need a separate variable for that then, something along the lines of "max-ram-below-4g-default". I'll have a look tomorrow (unless you are faster with a patch ;) cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PULL 3/4] xen: add pvUSB backend
On Di, 2016-06-07 at 10:35 +0200, Olaf Hering wrote: > On Mon, May 23, Gerd Hoffmann wrote: > > > +++ b/hw/usb/Makefile.objs > > +common-obj-$(CONFIG_XEN_BACKEND) += xen-usb.o > > +++ b/hw/usb/xen-usb.c > > +usb_bus_new(&usbif->bus, sizeof(usbif->bus), &xen_usb_bus_ops, > > xen_sysdev); > > xen_sysdev is in an i386-only file, as a result qemu fails to link. Ping. Juergen? Xen folks? Can anyone have a look please? I think the fix would be s/xen_sysdev/xendev/, but I can't test that myself. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PULL 2/4] xen: write information about supported backends
From: Juergen Gross Add a Xenstore directory for each supported pv backend. This will allow Xen tools to decide which backend type to use in case there are multiple possibilities. The information is added under /local/domain//device-model//backends before the "running" state is written to Xenstore. Using a directory for each backend enables us to add parameters for specific backends in the future. This interface is documented in the Xen source repository in the file docs/misc/qemu-backends.txt In order to reuse the Xenstore directory creation already present in hw/xen/xen_devconfig.c move the related functions to hw/xen/xen_backend.c where they fit better. Signed-off-by: Juergen Gross Acked-by: Anthony PERARD Reviewed-by: Wei Liu Message-id: 1463062421-613-3-git-send-email-jgr...@suse.com Signed-off-by: Gerd Hoffmann --- hw/xen/xen_backend.c | 63 hw/xen/xen_devconfig.c | 52 ++-- include/hw/xen/xen_backend.h | 2 ++ 3 files changed, 67 insertions(+), 50 deletions(-) diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c index 60575ad..c63f9df 100644 --- a/hw/xen/xen_backend.c +++ b/hw/xen/xen_backend.c @@ -42,11 +42,36 @@ struct xs_handle *xenstore = NULL; const char *xen_protocol; /* private */ +struct xs_dirs { +char *xs_dir; +QTAILQ_ENTRY(xs_dirs) list; +}; +static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = +QTAILQ_HEAD_INITIALIZER(xs_cleanup); + static QTAILQ_HEAD(XenDeviceHead, XenDevice) xendevs = QTAILQ_HEAD_INITIALIZER(xendevs); static int debug = 0; /* - */ +static void xenstore_cleanup_dir(char *dir) +{ +struct xs_dirs *d; + +d = g_malloc(sizeof(*d)); +d->xs_dir = dir; +QTAILQ_INSERT_TAIL(&xs_cleanup, d, list); +} + +void xen_config_cleanup(void) +{ +struct xs_dirs *d; + +QTAILQ_FOREACH(d, &xs_cleanup, list) { +xs_rm(xenstore, 0, d->xs_dir); +} +} + int xenstore_write_str(const char *base, const char *node, const char *val) { char abspath[XEN_BUFSIZE]; @@ -75,6 +100,30 @@ char *xenstore_read_str(const char *base, const char *node) return ret; } +int xenstore_mkdir(char *path, int p) +{ +struct xs_permissions perms[2] = { +{ +.id= 0, /* set owner: dom0 */ +}, { +.id= xen_domid, +.perms = p, +} +}; + +if (!xs_mkdir(xenstore, 0, path)) { +xen_be_printf(NULL, 0, "xs_mkdir %s: failed\n", path); +return -1; +} +xenstore_cleanup_dir(g_strdup(path)); + +if (!xs_set_permissions(xenstore, 0, path, perms, 2)) { +xen_be_printf(NULL, 0, "xs_set_permissions %s: failed\n", path); +return -1; +} +return 0; +} + int xenstore_write_int(const char *base, const char *node, int ival) { char val[12]; @@ -726,6 +775,20 @@ err: int xen_be_register(const char *type, struct XenDevOps *ops) { +char path[50]; +int rc; + +if (ops->backend_register) { +rc = ops->backend_register(); +if (rc) { +return rc; +} +} + +snprintf(path, sizeof(path), "device-model/%u/backends/%s", xen_domid, + type); +xenstore_mkdir(path, XS_PERM_NONE); + return xenstore_scan(type, xen_domid, ops); } diff --git a/hw/xen/xen_devconfig.c b/hw/xen/xen_devconfig.c index 1f30fe4..b7d290d 100644 --- a/hw/xen/xen_devconfig.c +++ b/hw/xen/xen_devconfig.c @@ -5,54 +5,6 @@ /* - */ -struct xs_dirs { -char *xs_dir; -QTAILQ_ENTRY(xs_dirs) list; -}; -static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = QTAILQ_HEAD_INITIALIZER(xs_cleanup); - -static void xen_config_cleanup_dir(char *dir) -{ -struct xs_dirs *d; - -d = g_malloc(sizeof(*d)); -d->xs_dir = dir; -QTAILQ_INSERT_TAIL(&xs_cleanup, d, list); -} - -void xen_config_cleanup(void) -{ -struct xs_dirs *d; - -QTAILQ_FOREACH(d, &xs_cleanup, list) { - xs_rm(xenstore, 0, d->xs_dir); -} -} - -/* - */ - -static int xen_config_dev_mkdir(char *dev, int p) -{ -struct xs_permissions perms[2] = {{ -.id= 0, /* set owner: dom0 */ -},{ -.id= xen_domid, -.perms = p, -}}; - -if (!xs_mkdir(xenstore, 0, dev)) { - xen_be_printf(NULL, 0, "xs_mkdir %s: failed\n", dev); - return -1; -} -xen_config_cleanup_dir(g_strdup(dev)); - -if (!xs_set_permissions(xenstore, 0, dev, perms, 2)) { - xen_be_printf(NULL, 0, "xs_set_permissions %s: failed\n", dev); - return -1; -} -return 0; -} - static int xen_config_dev_dirs(const char *ftype, const char *btype, int vdev, char *fe, char *
[Xen-devel] [PULL 1/4] xen: introduce dummy system device
From: Juergen Gross Introduce a new dummy system device serving as parent for virtual buses. This will enable new pv backends to introduce virtual buses which are removable again opposed to system buses which are meant to stay once added. Signed-off-by: Juergen Gross Acked-by: Anthony PERARD Reviewed-by: Wei Liu Message-id: 1463062421-613-2-git-send-email-jgr...@suse.com Signed-off-by: Gerd Hoffmann --- hw/xenpv/xen_machine_pv.c| 40 include/hw/xen/xen_backend.h | 1 + 2 files changed, 41 insertions(+) diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c index fc13535..48d5bc6 100644 --- a/hw/xenpv/xen_machine_pv.c +++ b/hw/xenpv/xen_machine_pv.c @@ -25,10 +25,15 @@ #include "qemu/osdep.h" #include "hw/hw.h" #include "hw/boards.h" +#include "hw/sysbus.h" #include "hw/xen/xen_backend.h" #include "xen_domainbuild.h" #include "sysemu/block-backend.h" +#define TYPE_XENSYSDEV "xensysdev" + +DeviceState *xen_sysdev; + static void xen_init_pv(MachineState *machine) { DriveInfo *dinfo; @@ -67,6 +72,9 @@ static void xen_init_pv(MachineState *machine) break; } +xen_sysdev = qdev_create(NULL, TYPE_XENSYSDEV); +qdev_init_nofail(xen_sysdev); + xen_be_register("console", &xen_console_ops); xen_be_register("vkbd", &xen_kbdmouse_ops); xen_be_register("vfb", &xen_framebuffer_ops); @@ -101,6 +109,38 @@ static void xen_init_pv(MachineState *machine) xen_init_display(xen_domid); } +static int xen_sysdev_init(SysBusDevice *dev) +{ +return 0; +} + +static Property xen_sysdev_properties[] = { +{/* end of property list */}, +}; + +static void xen_sysdev_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + +k->init = xen_sysdev_init; +dc->props = xen_sysdev_properties; +} + +static const TypeInfo xensysdev_info = { +.name = TYPE_XENSYSDEV, +.parent= TYPE_SYS_BUS_DEVICE, +.instance_size = sizeof(SysBusDevice), +.class_init= xen_sysdev_class_init, +}; + +static void xenpv_register_types(void) +{ +type_register_static(&xensysdev_info); +} + +type_init(xenpv_register_types); + static void xenpv_machine_init(MachineClass *mc) { mc->desc = "Xen Para-virtualized PC"; diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h index c839eeb..b4b4ff0 100644 --- a/include/hw/xen/xen_backend.h +++ b/include/hw/xen/xen_backend.h @@ -60,6 +60,7 @@ extern xc_interface *xen_xc; extern xenforeignmemory_handle *xen_fmem; extern struct xs_handle *xenstore; extern const char *xen_protocol; +extern DeviceState *xen_sysdev; /* xenstore helper functions */ int xenstore_write_str(const char *base, const char *node, const char *val); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PULL 3/4] xen: add pvUSB backend
From: Juergen Gross Add a backend for para-virtualized USB devices for xen domains. The backend is using host-libusb to forward USB requests from a domain via libusb to the real device(s) passed through. Signed-off-by: Juergen Gross Acked-by: Anthony PERARD Message-id: 1463062421-613-4-git-send-email-jgr...@suse.com Signed-off-by: Gerd Hoffmann --- hw/usb/Makefile.objs |4 + hw/usb/xen-usb.c | 1080 ++ hw/xenpv/xen_machine_pv.c|3 + include/hw/xen/xen_backend.h |3 + include/hw/xen/xen_common.h |2 + 5 files changed, 1092 insertions(+) create mode 100644 hw/usb/xen-usb.c diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs index 2717027..98b5c9d 100644 --- a/hw/usb/Makefile.objs +++ b/hw/usb/Makefile.objs @@ -38,3 +38,7 @@ common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o # usb pass-through common-obj-y += $(patsubst %,host-%.o,$(HOST_USB)) + +ifeq ($(CONFIG_USB_LIBUSB),y) +common-obj-$(CONFIG_XEN_BACKEND) += xen-usb.o +endif diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c new file mode 100644 index 000..664df04 --- /dev/null +++ b/hw/usb/xen-usb.c @@ -0,0 +1,1080 @@ +/* + * xen paravirt usb device backend + * + * (c) Juergen Gross + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; under version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, see <http://www.gnu.org/licenses/>. + * + * Contributions after 2012-01-13 are licensed under the terms of the + * GNU GPL, version 2 or (at your option) any later version. + */ + +#include +#include +#include +#include +#include + +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qemu/config-file.h" +#include "hw/sysbus.h" +#include "hw/usb.h" +#include "hw/xen/xen_backend.h" +#include "monitor/qdev.h" +#include "qapi/qmp/qbool.h" +#include "qapi/qmp/qint.h" +#include "qapi/qmp/qstring.h" +#include "sys/user.h" + +#include +#include + +/* + * Check for required support of usbif.h: USBIF_SHORT_NOT_OK was the last + * macro added we rely on. + */ +#ifdef USBIF_SHORT_NOT_OK + +#define TR(xendev, lvl, fmt, args...) \ +{ \ +struct timeval tv; \ +\ +gettimeofday(&tv, NULL);\ +xen_be_printf(xendev, lvl, "%8ld.%06ld xen-usb(%s):" fmt, \ + tv.tv_sec, tv.tv_usec, __func__, ##args); \ +} +#define TR_BUS(xendev, fmt, args...) TR(xendev, 2, fmt, ##args) +#define TR_REQ(xendev, fmt, args...) TR(xendev, 3, fmt, ##args) + +#define USBBACK_MAXPORTSUSBIF_PIPE_PORT_MASK +#define USB_DEV_ADDR_SIZE (USBIF_PIPE_DEV_MASK + 1) + +/* USB wire protocol: structure describing control request parameter. */ +struct usbif_ctrlrequest { +uint8_tbRequestType; +uint8_tbRequest; +uint16_t wValue; +uint16_t wIndex; +uint16_t wLength; +}; + +struct usbback_info; +struct usbback_req; + +struct usbback_stub { +USBDevice *dev; +USBPort port; +unsigned int speed; +bool attached; +QTAILQ_HEAD(submit_q_head, usbback_req) submit_q; +}; + +struct usbback_req { +struct usbback_info *usbif; +struct usbback_stub *stub; +struct usbif_urb_request req; +USBPacketpacket; + +unsigned int nr_buffer_segs; /* # of transfer_buffer segments */ +unsigned int nr_extra_segs; /* # of iso_frame_desc segments */ + +QTAILQ_ENTRY(usbback_req) q; + +void *buffer; +void *isoc_buffer; +struct libusb_transfer *xfer; +}; + +struct usbback_hotplug { +QSIMPLEQ_ENTRY(usbback_hotplug) q; +unsigned port; +}; + +struct usbback_info { +struct XenDevice xendev; /* must be first */ +USBBus bus; +void *urb_sring; +void *conn_sring; +struct usbif_urb_back_ring urb_ring; +struct usbif_conn_back_ring conn_ring; +int num_ports; +int usb_ver; +bool ring_error; +QTAILQ_HEAD(req_free_q_head, usbback_req) req_free_q; +QSIMPLE
Re: [Xen-devel] [PATCH v5 0/3] usb, xen: add pvUSB backend
On Do, 2016-05-12 at 16:13 +0200, Juergen Gross wrote: > This series adds a Xen pvUSB backend driver to qemu. USB devices > connected to the host can be passed through to a Xen guest. The > devices are specified via Xenstore. Access to the devices is done > via host-libusb.c > > I've tested the backend with various USB devices (memory sticks, > keyboard, ...). > > Changes in V5: > - patch 2: silence checkpatch errors/warnings > Added to usb patch queue. thanks, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 0/3] usb, xen: add pvUSB backend
On Do, 2016-05-12 at 07:47 +0200, Juergen Gross wrote: > This series adds a Xen pvUSB backend driver to qemu. USB devices > connected to the host can be passed through to a Xen guest. The > devices are specified via Xenstore. Access to the devices is done > via host-libusb.c > > I've tested the backend with various USB devices (memory sticks, > keyboard, ...). Doesn't pass scripts/checkpatch.pl: Applying: xen: write information about supported backends === checkpatch complains === WARNING: line over 80 characters #14: FILE: hw/xen/xen_backend.c:49: +static QTAILQ_HEAD(xs_dirs_head, xs_dirs) xs_cleanup = QTAILQ_HEAD_INITIALIZER(xs_cleanup); ERROR: space required after that close brace '}' #53: FILE: hw/xen/xen_backend.c:109: +}}; WARNING: space prohibited between function name and open parenthesis '(' #85: FILE: hw/xen/xen_backend.c:785: +snprintf(path, sizeof (path), "device-model/%u/backends/%s", xen_domid, total: 1 errors, 2 warnings, 161 lines checked cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [SeaBIOS] Xen PV block device support in Seabios
Hi, > > Ian, thanks for your reply! It looks like the problem is how and when to > > clear PV resources in seabios before handing over to guest. But I wonder > > why virtio works in seabios. Does seabios using virtio need to clear > > things like vrings? Or seabios doesn't clear the things and guest just > > covers the configuration with new values? > > I think virtio covered this use case from day 1 by having the reset, That is correct. First thing the kernel driver does as part of the initialization sequence is a reset, to put the device into a known state, no matter where seabios left it off. Oh, and of course seabios does the same. So even in case the virtio device is in some weird state due to kernel crashing and rebooting seabios will be able to boot from it without any problems. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] usb, xen: add pvUSB backend
On Do, 2016-03-10 at 16:19 +0100, Juergen Gross wrote: > This series adds a Xen pvUSB backend driver to qemu. USB devices > connected to the host can be passed through to a Xen guest. The > devices are specified via Xenstore. Access to the devices is done > via host-libusb.c > I've tested the backend with various USB devices (memory sticks, > keyboard, ...). Patches look sane to me. Have you tested both virtual and physical devices? Given how it is written devices such as the virtual usb tablet should work just fine too. I can take that through the usb queue, but I'd like to see someone from xen have a look at this too. Reviews anyone? thanks, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 2/8] pc: move igd support code to igd.c
Hi, > +/* Here we just expose minimal host bridge offset subset. */ > +static const IGDHostInfo igd_host_bridge_infos[] = { > +{0x08, 2}, /* revision id */ > +{0x2c, 2}, /* sybsystem vendor id */ > +{0x2e, 2}, /* sybsystem id */ Can anyone clarify where this comes from? Setting the subsystem id without also setting the pci id looks wrong, given that each pci id has its own subsystem id namespace. Testing (with alex vfio patches) shows that dropping this seems to have no bad effects. Things are still working fine of we only set these ... > +{0x50, 2}, /* SNB: processor graphics control register */ > +{0x52, 2}, /* processor graphics control register */ > +{0xa4, 4}, /* SNB: graphics base of stolen memory */ > +{0xa8, 4}, /* SNB: base of GTT stolen memory */ ... gfx registers in host bridge pci config space. thanks, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 0/8] igd passthrough chipset tweaks
Hi, Next version of the patches, after a longish break. Meanwhile it is more clear how we are going to handle the igd passthrough quirks with kvm: vfio will get support for device-specific regions, and we will use that for the opregion and also to provide unpriviledged read access to host bridge (00:00.0) and isa bridge (00:1f,0) pci config space. That implies we wouldn't share the code for pci config space access and the existing xen code wouldn't be reused for kvm, except maybe for the struct IGDHostInfo tables. Separating out the igd support code into its own file and the cleanups + bugfixes on top of that still make sense though. So here we go with a stripped down patch series ... cheers, Gerd Gerd Hoffmann (8): pc: remove has_igd_gfx_passthru global pc: move igd support code to igd.c igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize igd: use defines for standard pci config space offsets igd: revamp host config read igd: move igd-passthrough-isa-bridge to igd.c too igd: handle igd-passthrough-isa-bridge setup in realize() default-configs/x86_64-softmmu.mak | 1 + hw/i386/pc_piix.c | 115 +-- hw/pci-host/Makefile.objs | 3 + hw/pci-host/igd.c | 157 + hw/pci-host/piix.c | 90 - hw/xen/xen_pt.c| 4 +- hw/xen/xen_pt.h| 5 +- include/hw/i386/pc.h | 2 +- vl.c | 10 --- 9 files changed, 167 insertions(+), 220 deletions(-) create mode 100644 hw/pci-host/igd.c -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 2/8] pc: move igd support code to igd.c
Pure code motion, except for dropping instance_size for TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE (no need to set, we can inherit it from TYPE_I440FX_PCI_DEVICE). Signed-off-by: Gerd Hoffmann Acked-by: Stefano Stabellini --- default-configs/x86_64-softmmu.mak | 1 + hw/pci-host/Makefile.objs | 3 ++ hw/pci-host/igd.c | 99 ++ hw/pci-host/piix.c | 90 -- 4 files changed, 103 insertions(+), 90 deletions(-) create mode 100644 hw/pci-host/igd.c diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 6e3b312..cd3340e 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -58,3 +58,4 @@ CONFIG_IOH3420=y CONFIG_I82801B11=y CONFIG_SMBIOS=y CONFIG_HYPERV_TESTDEV=$(CONFIG_KVM) +CONFIG_PCI_IGD=y diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs index 45f1f0e..c03210b 100644 --- a/hw/pci-host/Makefile.objs +++ b/hw/pci-host/Makefile.objs @@ -16,3 +16,6 @@ common-obj-$(CONFIG_FULONG) += bonito.o common-obj-$(CONFIG_PCI_PIIX) += piix.o common-obj-$(CONFIG_PCI_Q35) += q35.o common-obj-$(CONFIG_PCI_GENERIC) += gpex.o + +# igd passthrough support +common-obj-$(CONFIG_PCI_IGD) += igd.o diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c new file mode 100644 index 000..331e9e1 --- /dev/null +++ b/hw/pci-host/igd.c @@ -0,0 +1,99 @@ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "hw/pci/pci.h" +#include "hw/i386/pc.h" + +/* IGD Passthrough Host Bridge. */ +typedef struct { +uint8_t offset; +uint8_t len; +} IGDHostInfo; + +/* Here we just expose minimal host bridge offset subset. */ +static const IGDHostInfo igd_host_bridge_infos[] = { +{0x08, 2}, /* revision id */ +{0x2c, 2}, /* sybsystem vendor id */ +{0x2e, 2}, /* sybsystem id */ +{0x50, 2}, /* SNB: processor graphics control register */ +{0x52, 2}, /* processor graphics control register */ +{0xa4, 4}, /* SNB: graphics base of stolen memory */ +{0xa8, 4}, /* SNB: base of GTT stolen memory */ +}; + +static int host_pci_config_read(int pos, int len, uint32_t *val) +{ +char path[PATH_MAX]; +int config_fd; +ssize_t size = sizeof(path); +/* Access real host bridge. */ +int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", + 0, 0, 0, 0, "config"); +int ret = 0; + +if (rc >= size || rc < 0) { +return -ENODEV; +} + +config_fd = open(path, O_RDWR); +if (config_fd < 0) { +return -ENODEV; +} + +if (lseek(config_fd, pos, SEEK_SET) != pos) { +ret = -errno; +goto out; +} + +do { +rc = read(config_fd, (uint8_t *)val, len); +} while (rc < 0 && (errno == EINTR || errno == EAGAIN)); +if (rc != len) { +ret = -errno; +} + +out: +close(config_fd); +return ret; +} + +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +{ +uint32_t val = 0; +int rc, i, num; +int pos, len; + +num = ARRAY_SIZE(igd_host_bridge_infos); +for (i = 0; i < num; i++) { +pos = igd_host_bridge_infos[i].offset; +len = igd_host_bridge_infos[i].len; +rc = host_pci_config_read(pos, len, &val); +if (rc) { +return -ENODEV; +} +pci_default_write_config(pci_dev, pos, val, len); +} + +return 0; +} + +static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + +k->init = igd_pt_i440fx_initfn; +dc->desc = "IGD Passthrough Host bridge"; +} + +static const TypeInfo igd_passthrough_i440fx_info = { +.name = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, +.parent= TYPE_I440FX_PCI_DEVICE, +.class_init= igd_passthrough_i440fx_class_init, +}; + +static void igd_register_types(void) +{ +type_register_static(&igd_passthrough_i440fx_info); +} + +type_init(igd_register_types) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 41aa66f..6eb8bff 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -747,95 +747,6 @@ static const TypeInfo i440fx_info = { .class_init= i440fx_class_init, }; -/* IGD Passthrough Host Bridge. */ -typedef struct { -uint8_t offset; -uint8_t len; -} IGDHostInfo; - -/* Here we just expose minimal host bridge offset subset. */ -static const IGDHostInfo igd_host_bridge_infos[] = { -{0x08, 2}, /* revision id */ -{0x2c, 2}, /* sybsystem vendor id */ -{0x2e, 2}, /* sybsystem id */ -{0x50, 2}, /* SNB: processor graphics control register */ -{0x52, 2}, /* processor graphics control register */ -{0xa4, 4}, /* SNB: graphics base of stolen memory */ -{0xa8, 4}, /* SNB: base of GTT stolen m
[Xen-devel] [PATCH v4 3/8] igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
Signed-off-by: Gerd Hoffmann Reviewed-by: Stefano Stabellini --- hw/pci-host/igd.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 331e9e1..93b86ca 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -56,7 +56,7 @@ out: return ret; } -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { uint32_t val = 0; int rc, i, num; @@ -68,12 +68,11 @@ static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) len = igd_host_bridge_infos[i].len; rc = host_pci_config_read(pos, len, &val); if (rc) { -return -ENODEV; +error_setg(errp, "failed to read host config"); +return; } pci_default_write_config(pci_dev, pos, val, len); } - -return 0; } static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) @@ -81,7 +80,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); -k->init = igd_pt_i440fx_initfn; +k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 6/8] igd: revamp host config read
Move all work to the host_pci_config_copy helper function, which we can easily reuse when adding q35 support. Open sysfs file only once for all values. Use pread. Proper error handling. Fix bug: Update config space directly (writing via pci_default_write_config only works for registers whitelisted in wmask). Hmm, this code can hardly ever worked before, /me wonders what test coverage it had. With this patch in place igd-passthru=on actually works, although it still requires root priviledges because linux refuses to allow non-root users access pci config space above offset 0x50. Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 65 ++- 1 file changed, 26 insertions(+), 39 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 8a8b37b..5c4a008 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -20,40 +20,33 @@ static const IGDHostInfo igd_host_bridge_infos[] = { {0xa8, 4}, /* SNB: base of GTT stolen memory */ }; -static int host_pci_config_read(int pos, int len, uint32_t *val) +static void host_pci_config_copy(PCIDevice *guest, const char *host, + const IGDHostInfo *list, int len, Error **errp) { -char path[PATH_MAX]; -int config_fd; -ssize_t size = sizeof(path); -/* Access real host bridge. */ -int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", - 0, 0, 0, 0, "config"); -int ret = 0; +char *path; +int config_fd, rc, i; -if (rc >= size || rc < 0) { -return -ENODEV; -} - -config_fd = open(path, O_RDWR); +path = g_strdup_printf("/sys/bus/pci/devices/%s/config", host); +config_fd = open(path, O_RDONLY); if (config_fd < 0) { -return -ENODEV; +error_setg_file_open(errp, errno, path); +goto out_free; } -if (lseek(config_fd, pos, SEEK_SET) != pos) { -ret = -errno; -goto out; +for (i = 0; i < len; i++) { +rc = pread(config_fd, guest->config + list[i].offset, + list[i].len, list[i].offset); +if (rc != list[i].len) { +error_setg_errno(errp, errno, "read %s, offset 0x%x", + path, list[i].offset); +goto out_close; +} } -do { -rc = read(config_fd, (uint8_t *)val, len); -} while (rc < 0 && (errno == EINTR || errno == EAGAIN)); -if (rc != len) { -ret = -errno; -} - -out: +out_close: close(config_fd); -return ret; +out_free: +g_free(path); } #define IGD_PT_I440FX_CLASS(class) \ @@ -72,9 +65,6 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { IGDPtI440fxClass *k = IGD_PT_I440FX_GET_CLASS(pci_dev); Error *err = NULL; -uint32_t val = 0; -int rc, i, num; -int pos, len; k->parent_realize(pci_dev, &err); if (err != NULL) { @@ -82,16 +72,13 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) return; } -num = ARRAY_SIZE(igd_host_bridge_infos); -for (i = 0; i < num; i++) { -pos = igd_host_bridge_infos[i].offset; -len = igd_host_bridge_infos[i].len; -rc = host_pci_config_read(pos, len, &val); -if (rc) { -error_setg(errp, "failed to read host config"); -return; -} -pci_default_write_config(pci_dev, pos, val, len); +host_pci_config_copy(pci_dev, ":00:00.0", + igd_host_bridge_infos, + ARRAY_SIZE(igd_host_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; } } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 7/8] igd: move igd-passthrough-isa-bridge to igd.c too
Signed-off-by: Gerd Hoffmann --- hw/i386/pc_piix.c | 113 -- hw/pci-host/igd.c | 108 +++ 2 files changed, 108 insertions(+), 113 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 40c58a5..43964dc 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -910,119 +910,6 @@ static void pc_i440fx_0_10_machine_options(MachineClass *m) DEFINE_I440FX_MACHINE(v0_10, "pc-0.10", pc_compat_0_13, pc_i440fx_0_10_machine_options); -typedef struct { -uint16_t gpu_device_id; -uint16_t pch_device_id; -uint8_t pch_revision_id; -} IGDDeviceIDInfo; - -/* In real world different GPU should have different PCH. But actually - * the different PCH DIDs likely map to different PCH SKUs. We do the - * same thing for the GPU. For PCH, the different SKUs are going to be - * all the same silicon design and implementation, just different - * features turn on and off with fuses. The SW interfaces should be - * consistent across all SKUs in a given family (eg LPT). But just same - * features may not be supported. - * - * Most of these different PCH features probably don't matter to the - * Gfx driver, but obviously any difference in display port connections - * will so it should be fine with any PCH in case of passthrough. - * - * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell) - * scenarios, 0x9cc3 for BDW(Broadwell). - */ -static const IGDDeviceIDInfo igd_combo_id_infos[] = { -/* HSW Classic */ -{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */ -{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */ -{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */ -{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */ -{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */ -/* HSW ULT */ -{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */ -{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */ -{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */ -{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */ -{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */ -{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */ -/* HSW CRW */ -{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */ -{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */ -/* HSW Server */ -{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */ -/* HSW SRVR */ -{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */ -/* BSW */ -{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */ -{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */ -{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */ -{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */ -{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */ -{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */ -{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */ -{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */ -{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */ -{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */ -{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */ -}; - -static void isa_bridge_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - -dc->desc= "ISA bridge faked to support IGD PT"; -k->vendor_id= PCI_VENDOR_ID_INTEL; -k->class_id = PCI_CLASS_BRIDGE_ISA; -}; - -static TypeInfo isa_bridge_info = { -.name = "igd-passthrough-isa-bridge", -.parent= TYPE_PCI_DEVICE, -.instance_size = sizeof(PCIDevice), -.class_init = isa_bridge_class_init, -}; - -static void pt_graphics_register_types(void) -{ -type_register_static(&isa_bridge_info); -} -type_init(pt_graphics_register_types) - -void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id) -{ -struct PCIDevice *bridge_dev; -int i, num; -uint16_t pch_dev_id = 0x; -uint8_t pch_rev_id; - -num = ARRAY_SIZE(igd_combo_id_infos); -for (i = 0; i < num; i++) { -if (gpu_dev_id == igd_combo_id_infos[i].gpu_device_id) { -pch_dev_id = igd_combo_id_infos[i].pch_device_id; -pch_rev_id = igd_combo_id_infos[i].pch_revision_id; -} -} - -if (pch_dev_id == 0x) { -return; -} - -/* Currently IGD drivers always need to access PCH by 1f.0. */ -bridge_dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0), - "igd-passthrough-isa-bridge"); - -/* - * Note that vendor id is always PCI_VENDOR_ID_INTEL. - */ -if (!bridge_dev) { -fprintf(stderr, "set igd-passthrough-isa-bridge failed!\n"); -return; -} -pci_config_set_device_id(bridge_dev->config, pch_dev_id); -pci_config_set_revision(bridge_dev->config, pch_rev_id); -} - static void isapc_machine_options(MachineClass *m) { PCMachineClass *pcmc = PC_MACHINE_CLASS(m); dif
[Xen-devel] [PATCH v4 8/8] igd: handle igd-passthrough-isa-bridge setup in realize()
That way a simple '-device igd-passthrough-isa-bridge,addr=1f' will do the setup. Also instead of looking up reasonable PCI IDs based on the graphic device id simply copy over the ids from the host, thereby reusing the infrastructure we have in place for the igd host bridges. Less code, and should be more robust as we don't have to maintain the id table to keep things going. Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c| 115 +-- hw/xen/xen_pt.c | 4 +- include/hw/i386/pc.h | 2 +- 3 files changed, 30 insertions(+), 91 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index e7183a3..0513c55 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -100,111 +100,52 @@ static const TypeInfo igd_passthrough_i440fx_info = { .class_size= sizeof(IGDPtI440fxClass), }; -typedef struct { -uint16_t gpu_device_id; -uint16_t pch_device_id; -uint8_t pch_revision_id; -} IGDDeviceIDInfo; - -/* In real world different GPU should have different PCH. But actually - * the different PCH DIDs likely map to different PCH SKUs. We do the - * same thing for the GPU. For PCH, the different SKUs are going to be - * all the same silicon design and implementation, just different - * features turn on and off with fuses. The SW interfaces should be - * consistent across all SKUs in a given family (eg LPT). But just same - * features may not be supported. - * - * Most of these different PCH features probably don't matter to the - * Gfx driver, but obviously any difference in display port connections - * will so it should be fine with any PCH in case of passthrough. - * - * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell) - * scenarios, 0x9cc3 for BDW(Broadwell). - */ -static const IGDDeviceIDInfo igd_combo_id_infos[] = { -/* HSW Classic */ -{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */ -{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */ -{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */ -{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */ -{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */ -/* HSW ULT */ -{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */ -{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */ -{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */ -{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */ -{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */ -{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */ -/* HSW CRW */ -{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */ -{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */ -/* HSW Server */ -{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */ -/* HSW SRVR */ -{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */ -/* BSW */ -{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */ -{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */ -{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */ -{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */ -{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */ -{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */ -{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */ -{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */ -{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */ -{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */ -{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */ +static const IGDHostInfo igd_isa_bridge_infos[] = { +{PCI_VENDOR_ID, 2}, +{PCI_DEVICE_ID, 2}, +{PCI_REVISION_ID, 2}, +{PCI_SUBSYSTEM_VENDOR_ID, 2}, +{PCI_SUBSYSTEM_ID,2}, }; +static void igd_pt_isa_bridge_realize(PCIDevice *pci_dev, Error **errp) +{ +Error *err = NULL; + +if (pci_dev->devfn != PCI_DEVFN(0x1f, 0)) { +error_setg(errp, "igd isa bridge must have address 1f.0"); +return; +} + +host_pci_config_copy(pci_dev, ":00:1f.0", + igd_isa_bridge_infos, + ARRAY_SIZE(igd_isa_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} +} + static void isa_bridge_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); dc->desc= "ISA bridge faked to support IGD PT"; -k->vendor_id= PCI_VENDOR_ID_INTEL; +k->realize = igd_pt_isa_bridge_realize; k->class_id = PCI_CLASS_BRIDGE_ISA; }; static TypeInfo igd_passthrough_isa_bridge_info = { .name = "igd-passthrough-isa-bridge", .parent= TYPE_PCI_DEVICE, -.instance_size = sizeof(PCIDevice), .class_init = isa_bridge_class_init, }; -void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id) +void igd_passthrough_isa_bridge_create(PCIBus *bus) { -struct PCIDevice *bridge_dev; -int i, num; -
[Xen-devel] [PATCH v4 4/8] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 93b86ca..3654298 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -56,12 +56,32 @@ out: return ret; } +#define IGD_PT_I440FX_CLASS(class) \ +OBJECT_CLASS_CHECK(IGDPtI440fxClass, (class), \ + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE) +#define IGD_PT_I440FX_GET_CLASS(obj)\ +OBJECT_GET_CLASS(IGDPtI440fxClass, (obj), \ + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE) + +typedef struct IGDPtI440fxClass { +PCIDeviceClass parent_class; +void (*parent_realize)(PCIDevice *dev, Error **errp); +} IGDPtI440fxClass; + static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { +IGDPtI440fxClass *k = IGD_PT_I440FX_GET_CLASS(pci_dev); +Error *err = NULL; uint32_t val = 0; int rc, i, num; int pos, len; +k->parent_realize(pci_dev, &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} + num = ARRAY_SIZE(igd_host_bridge_infos); for (i = 0; i < num; i++) { pos = igd_host_bridge_infos[i].offset; @@ -77,17 +97,20 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) { +IGDPtI440fxClass *k = IGD_PT_I440FX_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); +PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass); -k->realize = igd_pt_i440fx_realize; -dc->desc = "IGD Passthrough Host bridge"; +k->parent_realize = pc->realize; +pc->realize = igd_pt_i440fx_realize; +dc->desc = "IGD Passthrough Host bridge (i440fx)"; } static const TypeInfo igd_passthrough_i440fx_info = { .name = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, .parent= TYPE_I440FX_PCI_DEVICE, .class_init= igd_passthrough_i440fx_class_init, +.class_size= sizeof(IGDPtI440fxClass), }; static void igd_register_types(void) -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 5/8] igd: use defines for standard pci config space offsets
Signed-off-by: Gerd Hoffmann Reviewed-by: Stefano Stabellini --- hw/pci-host/igd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 3654298..8a8b37b 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -11,9 +11,9 @@ typedef struct { /* Here we just expose minimal host bridge offset subset. */ static const IGDHostInfo igd_host_bridge_infos[] = { -{0x08, 2}, /* revision id */ -{0x2c, 2}, /* sybsystem vendor id */ -{0x2e, 2}, /* sybsystem id */ +{PCI_REVISION_ID, 2}, +{PCI_SUBSYSTEM_VENDOR_ID, 2}, +{PCI_SUBSYSTEM_ID,2}, {0x50, 2}, /* SNB: processor graphics control register */ {0x52, 2}, /* processor graphics control register */ {0xa4, 4}, /* SNB: graphics base of stolen memory */ -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 1/8] pc: remove has_igd_gfx_passthru global
Signed-off-by: Gerd Hoffmann Reviewed-by: Stefano Stabellini Reviewed-by: Eduardo Habkost --- hw/i386/pc_piix.c | 2 +- hw/xen/xen_pt.h | 5 +++-- vl.c | 10 -- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 6f8c2cd..40c58a5 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -364,7 +364,7 @@ static void pc_init_isa(MachineState *machine) #ifdef CONFIG_XEN static void pc_xen_hvm_init_pci(MachineState *machine) { -const char *pci_type = has_igd_gfx_passthru ? +const char *pci_type = machine->igd_gfx_passthru ? TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE; pc_init1(machine, diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index c2f8e1f..4048a5a 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -4,6 +4,7 @@ #include "qemu-common.h" #include "hw/xen/xen_common.h" #include "hw/pci/pci.h" +#include "hw/boards.h" #include "xen-host-pci-device.h" void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3); @@ -322,10 +323,10 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev, unsigned int domain, unsigned int bus, unsigned int slot, unsigned int function); -extern bool has_igd_gfx_passthru; static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev) { -return (has_igd_gfx_passthru +MachineState *machine = MACHINE(qdev_get_machine()); +return (machine->igd_gfx_passthru && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA)); } int xen_pt_register_vga_regions(XenHostPCIDevice *dev); diff --git a/vl.c b/vl.c index adeddd9..bdc2879 100644 --- a/vl.c +++ b/vl.c @@ -1374,13 +1374,6 @@ static inline void semihosting_arg_fallback(const char *file, const char *cmd) } } -/* Now we still need this for compatibility with XEN. */ -bool has_igd_gfx_passthru; -static void igd_gfx_passthru(void) -{ -has_igd_gfx_passthru = current_machine->igd_gfx_passthru; -} - /***/ /* USB devices */ @@ -4524,9 +4517,6 @@ int main(int argc, char **argv, char **envp) exit(1); } -/* Check if IGD GFX passthrough. */ -igd_gfx_passthru(); - /* init generic devices */ if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, NULL)) { -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [SeaBIOS] [SEABIOS] Plans for either 1.9.1 or 1.10.0?
Hi, > 1.9-stable created now, with the patch above cherry-picked. 1.9.1 tagged & tarball uploaded now. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks
Hi, > > I'd have qemu copy the data on 0xfc write then, so things continue to > > work without updating seabios. So, the firmware has to allocate space, > > reserve it etc., and programming the 0xfc register. Qemu has to make > > sure the opregion appears at the address written by the firmware, by > > whatever method it prefers. > > Yup. It's Qemu's responsibility to expose opregion content. > > btw, prefer to do copying here. It's pointless to allow write from guest > side. One write example is SWSCI mailbox, thru which gfx driver can > trigger some SCI event to communicate with BIOS (specifically ACPI > methods here), mostly for some monitor operations. However it's > not a right thing for guest to trigger host SCI and thus kick host > ACPI methods. Thanks. So, question again how we do that best. Option one being the mmap way, i.e. basically what the patches posted by alex are doing. Option two being the fw_cfg way, i.e. place a opregion copy in fw_cfg and have seabios not only set 0xfc, but also store the opregion there by copying from fw_cfg. Advantage of option one is that we'll keep the option to do things in a different way in the future, without breaking the guest/qemu interface. Disadvantage is that it'll cause hugepage mappings to be splitted. Hmm. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks
Hi, > +realloc: > +opregion = malloc_high(size * 1024); memalign_high(PAGE_SIZE, size * 1024); > > I'd have qemu copy the data on 0xfc write then, so things continue to > > work without updating seabios. So, the firmware has to allocate space, > > reserve it etc., and programming the 0xfc register. Qemu has to make > > sure the opregion appears at the address written by the firmware, by > > whatever method it prefers. > > Ah, so here is where we'd clobber data in firmware. I currently do > this in vfio's pci config write in QEMU: > > orig = pci_get_long(pdev->config + IGD_OPREGION); > pci_default_write_config(pdev, addr, val, len); > cur = pci_get_long(pdev->config + IGD_OPREGION); > > if (cur != orig) { > if (orig) { > memory_region_del_subregion(get_system_memory(), > vdev->igd_opregion->mem); > } > > if (cur) { > memory_region_add_subregion(get_system_memory(), > cur, vdev->igd_opregion->mem); > } > } Ok, so we avoid the clobber and qemu sill has the choice to implement the opregion in different ways, by simply changing how vdev->igd_opregion->mem is backed. Good. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks
Hi, > Thanks for the tip that seabios allocated pages automatically become > e820 reserved, that simplifies things a bit. It's common practice for all firmware. The e820 table from qemu is just a starting point, it is not passed on to the guest os as-is. All permanent allocations (acpi tables, smbios tables, seabios driver data such as virtio rings, ...) are taken away from RAM and added to RESERVED, and IIRC seabios also takes care to reserve the bios and option rom regions in real mode address space. > > Maybe we should define the interface as "guest writes 0xfc to pick > > address, qemu takes care to place opregion there". That gives us the > > freedom to change the qemu implementation (either copy host opregion or > > map the host opregion) without breaking things. > > Ok, so seabios allocates two pages, writes the base address of those > pages to 0xfc and looks to see whether the signature appears at that > address due to qemu mapping. It verifies the size and does a > free/realloc if not the right size. I think seabios first needs to reserve something big enough for a temporary mapping, to check signature + size, otherwise the opregion might scratch data structures beyond opregion in case it happens to be larger than 8k. How likely is it that the opregion size ever changes? Should we better be prepared to handle it? Or would it be ok to have a ... if (opregion_size > 8k) panic(); ... style sanity check? > If the graphics signature does not > appear, free those pages and assume no opregion support. Yes. > If we later > decide to use a copy, we'd need to disable the 0xfc automagic mapping > and probably pass the data via fw_cfg. Sound right? I'd have qemu copy the data on 0xfc write then, so things continue to work without updating seabios. So, the firmware has to allocate space, reserve it etc., and programming the 0xfc register. Qemu has to make sure the opregion appears at the address written by the firmware, by whatever method it prefers. > > lpc bridge is no problem, only pci id fields are copied over and > > unprivileged access is allowed for them. > > > > Copying the gfx registers of the host bridge is a problem indeed. > > I would argue that both are really a problem, libvirt wants to put QEMU > in a container that prevents access to any host system files other than > those explicitly allowed. Therefore libvirt needs to grant the process > access to the lpc sysfs config file even though it only needs user > visible register values. Yes, correct. We want svirt be as strict as possible. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks
Hi, > 1) The OpRegion MemoryRegion is mapped into system_memory through > programming of the 0xFC config space register. > a) vfio-pci could pick an address to do this as it is realized. > b) SeaBIOS/OVMF could program this. > > Discussion: 1.a) Avoids any BIOS dependency, but vfio-pci would need to > pick an address and mark it as e820 reserved. I'm not sure how to pick > that address. Because of that I'd let the firmware pick the address and program 0xfc accordingly, i.e. (b). seabios can simply malloc two pages and be done with it (any ram allocated by seabios will be tagged as e820 reserved). > 2) Read-only mappings version of 1) > > Discussion: Really nothing changes from the issues above, just prevents > any possibility of the guest modifying anything in the host. Xen > apparently allows write access to the host page already. I think read-only is out. Probably xen allows write access because guest drivers expect they have write access to the opregion, so the question is ... > 3) Copy OpRegion contents into buffer and do either 1) or 2) above. whenever we give the guest a copy of the host opregion or direct access. > 4) Copy contents into a guest RAM location, mark it reserved, point to > it via 0xFC config as scratch register. > a) Done by QEMU (vfio-pci) > b) Done by SeaBIOS/OVMF > > Discussion: This is the most like real hardware. 4.a) has the usual > issue of how to pick an address, but the benefit of not requiring BIOS > changes (simply mark the RAM reserved via existing methods). 4.b) would > require passing a buffer containing the contents of the OpRegion via > fw_cfg and letting the BIOS do the setup. The latter of course requires > modifying each BIOS for this support. Maybe we should define the interface as "guest writes 0xfc to pick address, qemu takes care to place opregion there". That gives us the freedom to change the qemu implementation (either copy host opregion or map the host opregion) without breaking things. > Of course none of these support hotplug nor really can they since > reserved memory regions are not dynamic in the architecture. igd is chipset graphics and therefore not hotpluggable anyway (on physical hardware), I'd be very surprised if the guest drivers are prepared to handle hotplug. > Another thing I notice in this series is the access to PCI config space > of both the host bridge and the LPC bridge. This prevents unprivileged > use cases lpc bridge is no problem, only pci id fields are copied over and unprivileged access is allowed for them. Copying the gfx registers of the host bridge is a problem indeed. > Should vfio add > additional device specific regions to expose the config space of these > other devices? That is an option. It is not clear yet which route we have to take though. Testing shows that newer linux drivers work fine even without igd-passthru=on tweaks, whereas older linux kernels and windows drivers don't work even with this series applied and igd-passthru=on. I'll go look at this as soon as I have test hardware (getting some is wip atm). cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Spice-devel] Developers for virgl 3d windows guest support
Hi, > About virtio-gpu using virgl3d project for 3d hw acceleration support > and is what I mainly watching for its large gpu choice/support, seems > any gpu that have a drm driver in host kernel is supported by virgl, or > I'm wrong? You need a mesa driver too. But, yes, pretty much any modern hardware with opengl supported by open source drivers should do. > Intel cpus with integrated gpu that support igvt-g don't seems powerful > enough for major of servers I need to build (about xeon only workstation > series seems support it). FYI: Any broadwell + newer should do for intel-vgpu, long-term. current experimental code releases include haswell too, but intel doesn't plan to upstream that. > I'm wondering what solution is more long-term reliable and approceable > to have such 2d/3d support and FWIK ity seems virgl is the way to go but > i really need some advice from some expert out there in order to make > the right choice. I expect intel-vgpu and virgl both will be upstreamed at some point and should work fine without too much hassle. Intel has the advantage that windows drivers exist already, for virgl they need to be written. Intel has the disadvantage that it only works on newer intel hardware, virgl works pretty much anywhere. If you havn't yet ordered the hardware this might not be much of a problem though. But also note that being hardware-independent doesn't come for free, there is some translation overhead involved. When comparing intel-vgpu and virgl on the same hardware intel will most likely deliver higher performance. > available (virgl or other that I'm not aware of). Well, there is https://github.com/espes/xqemu/blob/xbox/hw/xbox/nv2a.c Didn't have the time yet to look at this in detail, it's on my TODO list though. It is opengl-accelerated geforce nv2a emulation. For an older qemu version. Plumbing that into opengl infrastructure qemu got recently for virgl shouldn't be too hard. Advantage of emulating something existing is that the guest driver problem goes away. Disavdantage of course is that you are limited to what the emulated hardware is able to do (not sure what level of opengl the nv2a is able to support). Could be useful for desktop workloads nevertheless. Bottom line: lots of tradeoffs here, and also alot of work-in-progress stuff ... cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v3 04/11] igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
Hi, > > static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void > > *data) > > @@ -78,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass > > *klass, void *data) > > DeviceClass *dc = DEVICE_CLASS(klass); > > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > > > > -k->init = igd_pt_i440fx_initfn; > > +k->realize = igd_pt_i440fx_realize; > > I am trying to understand how this have ever worked before: > > * PCIDeviceClass::init is called by pci_default_realize() > (default value for PCIDeviceClass::realize) > * i440fx_class_init() overrides PCIDeviceClass::realize > to i440fx_realize() > > So, when exactly was igd_pt_i440fx_realize() being called, before > this series? It simply didn't? I suspect this got ported over from the qemu-xen tree, but wasn't really tested and also not adapted to commit "9af21db pci: Trivial device model conversions to realize". So this patch actually is yet another bugfix ... Current test status of this series (by Hao) is this: * newer linux intel drivers work just fine without chipset tweaks. * older linux intel drivers and windows guests don't work even with all the fixes in this series. So applying the series doesn't improve things at all (code cleanups aside). My current plan to go forward is: (a) get test hardware (wip atm). (b) go figure what is really needed, lots of testing. (c) rework and repost series. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
Hi, > > > > +i440fx_realize = k->realize; > > > > k->realize = igd_pt_i440fx_realize; > > > > ... because we are overriding it right here. > > Many device classes have a parent_realize field so they can keep > a pointer to the original realize function. It's better than a > static variable. How does the attached patch (incremental fix, not tested yet) look like? cheers, Gerd From 3d110e297b5182107e055db3ab69092affdef5bb Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 20 Jan 2016 10:08:19 +0100 Subject: [PATCH] [fixup] TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE realize_parent --- hw/pci-host/igd.c | 24 +++- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 160828f..2d51745 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -49,12 +49,24 @@ out_free: g_free(path); } -static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); +#define IGD_PT_I440FX_CLASS(class) \ +OBJECT_CLASS_CHECK(IGDPtI440fxClass, (class), \ + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE) +#define IGD_PT_I440FX_GET_CLASS(obj)\ +OBJECT_GET_CLASS(IGDPtI440fxClass, (obj), \ + TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE) + +typedef struct IGDPtI440fxClass { +PCIDeviceClass parent_class; +void (*parent_realize)(PCIDevice *dev, Error **errp); +} IGDPtI440fxClass; + static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { +IGDPtI440fxClass *k = IGD_PT_I440FX_GET_CLASS(pci_dev); Error *err = NULL; -i440fx_realize(pci_dev, &err); +k->parent_realize(pci_dev, &err); if (err != NULL) { error_propagate(errp, err); return; @@ -72,11 +84,12 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) { +IGDPtI440fxClass *k = IGD_PT_I440FX_CLASS(klass); DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); +PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass); -i440fx_realize = k->realize; -k->realize = igd_pt_i440fx_realize; +k->parent_realize = pc->realize; +pc->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge (i440fx)"; } @@ -84,6 +97,7 @@ static const TypeInfo igd_passthrough_i440fx_info = { .name = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, .parent= TYPE_I440FX_PCI_DEVICE, .class_init= igd_passthrough_i440fx_class_init, +.class_size= sizeof(IGDPtI440fxClass), }; static void (*q35_realize)(PCIDevice *pci_dev, Error **errp); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [SeaBIOS] [SEABIOS] Plans for either 1.9.1 or 1.10.0?
Hi, > It's been suggested (by you :)) that > 76327b9f32a009245c215f4a3c5d58a01b5310ae be cherry-picked into 1.9.1 as > well, perhaps. Yes, right. Thanks for the reminder. Picked up. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [SeaBIOS] [SEABIOS] Plans for either 1.9.1 or 1.10.0?
On Do, 2016-01-14 at 14:50 +, Ian Campbell wrote: > Hello, > > The xen.git development branch currently points to SeaBIOS rel-1.9.0, but > Roger has tripped over a build issue which is fixed by 3b8c5378dfe2 "build: > fix typo in buildversion.py". > > Is there any plan for either a 1.9.1 or a 1.10.0 in the near future which > would include this, so I can roll forward to that? 1.10 will take a while ... > Or if not a 1.9.1 release perhaps for now just a 1.9-stable branch could be > created? 1.9-stable created now, with the patch above cherry-picked. Doing 1.9.1 soonish (next week maybe) is fine with me. If someone would like to see other patches backported to 1.9-stable please speak up now. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize()
On Di, 2016-01-12 at 09:50 +, Hao, Xudong wrote: > With latest qemu 7b8a354d4716, RHEL7.2 (with default kernel) VM still can't > boot up with IGD. There is another bug, using pci_default_write_config() doesn't fly as this checks writes against wmask and the registers in question are not whitelisted ... I've just rebased my igd patch series which (among other stuff) fixes that bug: https://www.kraxel.org/cgit/qemu/log/?h=work/igd git://git.kraxel.org/qemu work/igd Can you give it a spin? Also: what does "can't boot up" mean exactly? Guest doesn't boot at all? Guest boots, but igd/console/Xorg doesn't work? In case of the latter: Any chance to login via network and get a kernel log? thanks, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v4] igd-passthrough-i440FX: convert to realize()
Hi, > I can boot up Linux VM with IGD pass-through with latest qemu (without > any additional patch), guest run 3D "nexuiz" and get 180fps. That is a pretty recent linux guest I assume? Tried older kernels too, possibly even the old userspace xorg driver? Do windows guest work as well? cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v3 11/11] igd: move igd-passthrough-isa-bridge creation to machine init
Hi, > > That is true. Given that the only qemu-xen codebase with igd support is > > 4.7 and 4.7 hasn't been released yet, I am OK with changing the guest > > visible PCI layout. I might ask for your help in backporting the patches > > ;-) What are the 4.7 release plans btw? > One thing that I forgot to consider is that QEMU 2.5 has been released > with igd passthrough too and Xen 4.6 + QEMU 2.5 is a combination we > should support. > > However QEMU 2.5 has a serious bug > (http://marc.info/?l=qemu-devel&m=145172165010604) which probably > prevents igd passthrough from working at all. Stumbled over that one too, so my series has a (different) fix for it as well. > I asked Xudong to investigate. I am thinking that if the feature > works in 2.5, we need to support it, therefore we cannot break > migration by changing the PCI layout. I'd expect it is broken (at least for older guests). In case 2.5 works fine as-is we should be able to ditch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE altogether b/c it is a no-op in 2.5 because of the bug. But lets wait for the test results ... > Otherwise if the feature doesn't work, we could take the liberty to > make the change. Do you agree? Yes. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 10/11] igd: handle igd-passthrough-isa-bridge setup in realize()
On Mi, 2016-01-06 at 15:29 +, Stefano Stabellini wrote: > On Tue, 5 Jan 2016, Gerd Hoffmann wrote: > > That way a simple '-device igd-passthrough-isa-bridge,addr=1f' will > > do the setup. > > Is this going to change the QEMU command line arguments to use it? See patch 11 ;) cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 07/11] igd: revamp host config read
> > +for (i = 0; i < len; i++) { > > +rc = pread(config_fd, guest->config + list[i].offset, > > + list[i].len, list[i].offset); > > +if (rc != list[i].len) { > > pread is allowed to return early, returning the number of bytes read. > This is a sysfs file though, not a socket or pipe where a partial read makes sense and will actually happen. If we can't read something that'll be because the kernel denies access. So IMHO it should be fine to treat anything which doesn't give us the amount of bytes we asked for as an error condition. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
> > > > +static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); > > static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) > > { > > +Error *err = NULL; > > uint32_t val = 0; > > int rc, i, num; > > int pos, len; > > Can't we get the parent PCIDeviceClass realize function from pci_dev? So > that we don't have to introduce i440fx_realize? I don't think so ... > > > > +i440fx_realize = k->realize; > > k->realize = igd_pt_i440fx_realize; ... because we are overriding it right here. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 08/11] igd: add q35 support
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 41 - hw/pci-host/q35.c | 6 +- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index ec48875..f6e3f7a 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -1,5 +1,6 @@ #include "qemu-common.h" #include "hw/pci/pci.h" +#include "hw/pci-host/q35.h" #include "hw/i386/pc.h" /* IGD Passthrough Host Bridge. */ @@ -76,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) i440fx_realize = k->realize; k->realize = igd_pt_i440fx_realize; -dc->desc = "IGD Passthrough Host bridge"; +dc->desc = "IGD Passthrough Host bridge (i440fx)"; } static const TypeInfo igd_passthrough_i440fx_info = { @@ -85,9 +86,47 @@ static const TypeInfo igd_passthrough_i440fx_info = { .class_init= igd_passthrough_i440fx_class_init, }; +static void (*q35_realize)(PCIDevice *pci_dev, Error **errp); +static void igd_pt_q35_realize(PCIDevice *pci_dev, Error **errp) +{ +Error *err = NULL; + +q35_realize(pci_dev, &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} + +host_pci_config_copy(pci_dev, ":00:00.0", + igd_host_bridge_infos, + ARRAY_SIZE(igd_host_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} +} + +static void igd_passthrough_q35_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + +q35_realize = k->realize; +k->realize = igd_pt_q35_realize; +dc->desc = "IGD Passthrough Host bridge (q35)"; +} + +static const TypeInfo igd_passthrough_q35_info = { +.name = "igd-passthrough-q35-mch", +.parent= TYPE_MCH_PCI_DEVICE, +.class_init= igd_passthrough_q35_class_init, +}; + static void igd_register_types(void) { type_register_static(&igd_passthrough_i440fx_info); +type_register_static(&igd_passthrough_q35_info); } type_init(igd_register_types) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 1fb4707..07dc595 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -151,7 +151,11 @@ static void q35_host_initfn(Object *obj) memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, "pci-conf-data", 4); -object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE); +if (object_property_get_bool(qdev_get_machine(), "igd-passthru", NULL)) { +object_initialize(&s->mch, sizeof(s->mch), "igd-passthrough-q35-mch"); +} else { +object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE); +} object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); qdev_prop_set_uint32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0)); qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 9 + 1 file changed, 9 insertions(+) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index d1eeafb..6f52ab1 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -53,12 +53,20 @@ out: return ret; } +static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { +Error *err = NULL; uint32_t val = 0; int rc, i, num; int pos, len; +i440fx_realize(pci_dev, &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} + num = ARRAY_SIZE(igd_host_bridge_infos); for (i = 0; i < num; i++) { pos = igd_host_bridge_infos[i].offset; @@ -77,6 +85,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); +i440fx_realize = k->realize; k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 11/11] igd: move igd-passthrough-isa-bridge creation to machine init
This patch moves igd-passthrough-isa-bridge creation out of the xen passthrough code into machine init. It is triggered by the igd-passthru=on machine option. Advantages: * This works for on both xen and kvm. * It is activated for the pc machine type only, q35 has a real isa bridge on 1f.0 and must be handled differently. The q35 plan is https://lkml.org/lkml/2015/11/26/183 (should land in the next merge window, i.e. linux 4.5). * If we don't need it any more some day (intel is busy removing chipset dependencies from the guest driver) we have a single machine switch to just turn off all igd passthru chipset tweaks. Signed-off-by: Gerd Hoffmann --- hw/i386/pc_piix.c | 6 ++ hw/xen/xen_pt.c | 14 -- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index f36222e..2afbbd3 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -281,6 +281,12 @@ static void pc_init1(MachineState *machine, if (pcmc->pci_enabled) { pc_pci_device_init(pci_bus); } + +#ifdef CONFIG_LINUX +if (machine->igd_gfx_passthru) { +igd_passthrough_isa_bridge_create(pci_bus); +} +#endif } /* Looking for a pc_compat_2_4() function? It doesn't exist. diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 18a7f72..5f626c9 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -685,17 +685,6 @@ static const MemoryListener xen_pt_io_listener = { .priority = 10, }; -static void -xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s, - XenHostPCIDevice *dev) -{ -uint16_t gpu_dev_id; -PCIDevice *d = &s->dev; - -gpu_dev_id = dev->device_id; -igd_passthrough_isa_bridge_create(d->bus); -} - /* destroy. */ static void xen_pt_destroy(PCIDevice *d) { @@ -810,9 +799,6 @@ static int xen_pt_initfn(PCIDevice *d) xen_host_pci_device_put(&s->real_device); return -1; } - -/* Register ISA bridge for passthrough GFX. */ -xen_igd_passthrough_isa_bridge_create(s, &s->real_device); } /* Handle real device's MMIO/PIO BARs */ -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 01/11] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen
rename pc_xen_hvm_init_pci to pc_i440fx_init_pci, use it for both xen and non-xen init. That changes behavior of all pc-i440fx-$version machine types where specifying -machine igd-passthru=on used to have no effect and now it has. It is unlikely to cause any trouble though as there used to be no reason to add that option to kvm guests in the first place. Signed-off-by: Gerd Hoffmann Reviewed-by: Eduardo Habkost Reviewed-by: Stefano Stabellini --- hw/i386/pc_piix.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 438cdae..6532e32 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -368,10 +368,9 @@ static void pc_init_isa(MachineState *machine) pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE); } -#ifdef CONFIG_XEN -static void pc_xen_hvm_init_pci(MachineState *machine) +static void pc_i440fx_init_pci(MachineState *machine) { -const char *pci_type = has_igd_gfx_passthru ? +const char *pci_type = machine->igd_gfx_passthru ? TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE; pc_init1(machine, @@ -379,6 +378,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine) pci_type); } +#ifdef CONFIG_XEN static void pc_xen_hvm_init(MachineState *machine) { PCIBus *bus; @@ -388,7 +388,7 @@ static void pc_xen_hvm_init(MachineState *machine) exit(1); } -pc_xen_hvm_init_pci(machine); +pc_i440fx_init_pci(machine); bus = pci_find_primary_bus(); if (bus != NULL) { @@ -404,8 +404,7 @@ static void pc_xen_hvm_init(MachineState *machine) if (compat) { \ compat(machine); \ } \ -pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \ - TYPE_I440FX_PCI_DEVICE); \ +pc_i440fx_init_pci(machine); \ } \ DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn) -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 04/11] igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index ef0273b..d1eeafb 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -53,7 +53,7 @@ out: return ret; } -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { uint32_t val = 0; int rc, i, num; @@ -65,12 +65,11 @@ static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) len = igd_host_bridge_infos[i].len; rc = host_pci_config_read(pos, len, val); if (rc) { -return -ENODEV; +error_setg(errp, "failed to read host config"); +return; } pci_default_write_config(pci_dev, pos, val, len); } - -return 0; } static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) @@ -78,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); -k->init = igd_pt_i440fx_initfn; +k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 03/11] pc: move igd support code to igd.c
Pure code motion, except for dropping instance_size for TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE (no need to set, we can inherit it from TYPE_I440FX_PCI_DEVICE). Signed-off-by: Gerd Hoffmann Acked-by: Stefano Stabellini --- hw/pci-host/Makefile.objs | 3 ++ hw/pci-host/igd.c | 96 +++ hw/pci-host/piix.c| 88 --- 3 files changed, 99 insertions(+), 88 deletions(-) create mode 100644 hw/pci-host/igd.c diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs index 45f1f0e..e341a49 100644 --- a/hw/pci-host/Makefile.objs +++ b/hw/pci-host/Makefile.objs @@ -11,6 +11,9 @@ common-obj-$(CONFIG_PPCE500_PCI) += ppce500.o # ARM devices common-obj-$(CONFIG_VERSATILE_PCI) += versatile.o +# igd passthrough support +common-obj-$(CONFIG_LINUX) += igd.o + common-obj-$(CONFIG_PCI_APB) += apb.o common-obj-$(CONFIG_FULONG) += bonito.o common-obj-$(CONFIG_PCI_PIIX) += piix.o diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c new file mode 100644 index 000..ef0273b --- /dev/null +++ b/hw/pci-host/igd.c @@ -0,0 +1,96 @@ +#include "qemu-common.h" +#include "hw/pci/pci.h" +#include "hw/i386/pc.h" + +/* IGD Passthrough Host Bridge. */ +typedef struct { +uint8_t offset; +uint8_t len; +} IGDHostInfo; + +/* Here we just expose minimal host bridge offset subset. */ +static const IGDHostInfo igd_host_bridge_infos[] = { +{0x08, 2}, /* revision id */ +{0x2c, 2}, /* sybsystem vendor id */ +{0x2e, 2}, /* sybsystem id */ +{0x50, 2}, /* SNB: processor graphics control register */ +{0x52, 2}, /* processor graphics control register */ +{0xa4, 4}, /* SNB: graphics base of stolen memory */ +{0xa8, 4}, /* SNB: base of GTT stolen memory */ +}; + +static int host_pci_config_read(int pos, int len, uint32_t val) +{ +char path[PATH_MAX]; +int config_fd; +ssize_t size = sizeof(path); +/* Access real host bridge. */ +int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", + 0, 0, 0, 0, "config"); +int ret = 0; + +if (rc >= size || rc < 0) { +return -ENODEV; +} + +config_fd = open(path, O_RDWR); +if (config_fd < 0) { +return -ENODEV; +} + +if (lseek(config_fd, pos, SEEK_SET) != pos) { +ret = -errno; +goto out; +} +do { +rc = read(config_fd, (uint8_t *)&val, len); +} while (rc < 0 && (errno == EINTR || errno == EAGAIN)); +if (rc != len) { +ret = -errno; +} +out: +close(config_fd); +return ret; +} + +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +{ +uint32_t val = 0; +int rc, i, num; +int pos, len; + +num = ARRAY_SIZE(igd_host_bridge_infos); +for (i = 0; i < num; i++) { +pos = igd_host_bridge_infos[i].offset; +len = igd_host_bridge_infos[i].len; +rc = host_pci_config_read(pos, len, val); +if (rc) { +return -ENODEV; +} +pci_default_write_config(pci_dev, pos, val, len); +} + +return 0; +} + +static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + +k->init = igd_pt_i440fx_initfn; +dc->desc = "IGD Passthrough Host bridge"; +} + +static const TypeInfo igd_passthrough_i440fx_info = { +.name = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, +.parent= TYPE_I440FX_PCI_DEVICE, +.class_init= igd_passthrough_i440fx_class_init, +}; + +static void igd_register_types(void) +{ +type_register_static(&igd_passthrough_i440fx_info); +} + +type_init(igd_register_types) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 715208b..ccacb57 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -744,93 +744,6 @@ static const TypeInfo i440fx_info = { .class_init= i440fx_class_init, }; -/* IGD Passthrough Host Bridge. */ -typedef struct { -uint8_t offset; -uint8_t len; -} IGDHostInfo; - -/* Here we just expose minimal host bridge offset subset. */ -static const IGDHostInfo igd_host_bridge_infos[] = { -{0x08, 2}, /* revision id */ -{0x2c, 2}, /* sybsystem vendor id */ -{0x2e, 2}, /* sybsystem id */ -{0x50, 2}, /* SNB: processor graphics control register */ -{0x52, 2}, /* processor graphics control register */ -{0xa4, 4}, /* SNB: graphics base of stolen memory */ -{0xa8, 4}, /* SNB: base of GTT stolen memory */ -}; - -static int host_pci_config_read(int pos, int len, uint32_t val) -{ -char path[PATH_MAX]; -int config_fd; -ssize_t size = sizeof(path); -/* Access real host bridge. */ -int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", - 0, 0, 0, 0, "config"); -
[Xen-devel] [PATCH v3 00/11] igd passthrough chipset tweaks
Hi, We have some code in our tree to support pci passthrough of intel graphics devices (igd) on xen, which requires some chipset tweaks for (a) the host bridge and (b) the lpc/isa-bridge to meat the expectations of the guest driver. For kvm we need pretty much the same, also the requirements for vgpu (xengt/kvmgt) are very simliar. This patch wires up the existing support for kvm. It also brings a bunch of bugfixes and cleanups. Unfortunaly the oldish laptop I had planned to use for testing turned out to have no working iommu support for igd, so this patch series still has seen very light testing only. Any testing feedback is very welcome. Testing with kvm/i440fx: Add '-M pc,igd-passthru=on' to turn on the chipset tweaks. Passthrough the igd using vfio. Testing with kvm/q35: Add '-M q35,igd-passthru=on' to turn on the the chipset tweaks. Pick up the linux kernel patch referenced in patch #11, build a custom kernel with it. Passthrough the igd using vfio. Testing with xen: Existing setups should continue working ;) Changes in v3: * Handle igd-passthrough-isa-bridge creation in machine init. * Fix xen build failure. Changes in v2: * Added igd-passthrough-isa-bridge support form kvm. * Added patch to drop has_igd_gfx_passthru. cheers, Gerd Gerd Hoffmann (11): pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen pc: remove has_igd_gfx_passthru global pc: move igd support code to igd.c igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize igd: use defines for standard pci config space offsets igd: revamp host config read igd: add q35 support igd: move igd-passthrough-isa-bridge to igd.c too igd: handle igd-passthrough-isa-bridge setup in realize() igd: move igd-passthrough-isa-bridge creation to machine init hw/i386/pc_piix.c | 130 +++-- hw/pci-host/Makefile.objs | 3 + hw/pci-host/igd.c | 181 ++ hw/pci-host/piix.c| 88 -- hw/pci-host/q35.c | 6 +- hw/xen/xen_pt.c | 14 hw/xen/xen_pt.h | 5 +- include/hw/i386/pc.h | 2 +- vl.c | 10 --- 9 files changed, 204 insertions(+), 235 deletions(-) create mode 100644 hw/pci-host/igd.c -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 09/11] igd: move igd-passthrough-isa-bridge to igd.c too
Signed-off-by: Gerd Hoffmann --- hw/i386/pc_piix.c | 113 -- hw/pci-host/igd.c | 108 +++ 2 files changed, 108 insertions(+), 113 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 6532e32..f36222e 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -914,119 +914,6 @@ static void pc_i440fx_0_10_machine_options(MachineClass *m) DEFINE_I440FX_MACHINE(v0_10, "pc-0.10", pc_compat_0_13, pc_i440fx_0_10_machine_options); -typedef struct { -uint16_t gpu_device_id; -uint16_t pch_device_id; -uint8_t pch_revision_id; -} IGDDeviceIDInfo; - -/* In real world different GPU should have different PCH. But actually - * the different PCH DIDs likely map to different PCH SKUs. We do the - * same thing for the GPU. For PCH, the different SKUs are going to be - * all the same silicon design and implementation, just different - * features turn on and off with fuses. The SW interfaces should be - * consistent across all SKUs in a given family (eg LPT). But just same - * features may not be supported. - * - * Most of these different PCH features probably don't matter to the - * Gfx driver, but obviously any difference in display port connections - * will so it should be fine with any PCH in case of passthrough. - * - * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell) - * scenarios, 0x9cc3 for BDW(Broadwell). - */ -static const IGDDeviceIDInfo igd_combo_id_infos[] = { -/* HSW Classic */ -{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */ -{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */ -{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */ -{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */ -{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */ -/* HSW ULT */ -{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */ -{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */ -{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */ -{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */ -{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */ -{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */ -/* HSW CRW */ -{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */ -{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */ -/* HSW Server */ -{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */ -/* HSW SRVR */ -{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */ -/* BSW */ -{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */ -{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */ -{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */ -{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */ -{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */ -{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */ -{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */ -{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */ -{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */ -{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */ -{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */ -}; - -static void isa_bridge_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - -dc->desc= "ISA bridge faked to support IGD PT"; -k->vendor_id= PCI_VENDOR_ID_INTEL; -k->class_id = PCI_CLASS_BRIDGE_ISA; -}; - -static TypeInfo isa_bridge_info = { -.name = "igd-passthrough-isa-bridge", -.parent= TYPE_PCI_DEVICE, -.instance_size = sizeof(PCIDevice), -.class_init = isa_bridge_class_init, -}; - -static void pt_graphics_register_types(void) -{ -type_register_static(&isa_bridge_info); -} -type_init(pt_graphics_register_types) - -void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id) -{ -struct PCIDevice *bridge_dev; -int i, num; -uint16_t pch_dev_id = 0x; -uint8_t pch_rev_id; - -num = ARRAY_SIZE(igd_combo_id_infos); -for (i = 0; i < num; i++) { -if (gpu_dev_id == igd_combo_id_infos[i].gpu_device_id) { -pch_dev_id = igd_combo_id_infos[i].pch_device_id; -pch_rev_id = igd_combo_id_infos[i].pch_revision_id; -} -} - -if (pch_dev_id == 0x) { -return; -} - -/* Currently IGD drivers always need to access PCH by 1f.0. */ -bridge_dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0), - "igd-passthrough-isa-bridge"); - -/* - * Note that vendor id is always PCI_VENDOR_ID_INTEL. - */ -if (!bridge_dev) { -fprintf(stderr, "set igd-passthrough-isa-bridge failed!\n"); -return; -} -pci_config_set_device_id(bridge_dev->config, pch_dev_id); -pci_config_set_revision(bridge_dev->config, pch_rev_id); -} - static void isapc_machine_options(MachineClass *m) { PCMachineClass *pcmc = PC_MACHINE_CLASS(m); dif
[Xen-devel] [PATCH v3 02/11] pc: remove has_igd_gfx_passthru global
Signed-off-by: Gerd Hoffmann --- hw/xen/xen_pt.h | 5 +++-- vl.c| 10 -- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 3749711..cdd73ff 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -4,6 +4,7 @@ #include "qemu-common.h" #include "hw/xen/xen_common.h" #include "hw/pci/pci.h" +#include "hw/boards.h" #include "xen-host-pci-device.h" void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3); @@ -322,10 +323,10 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev, unsigned int domain, unsigned int bus, unsigned int slot, unsigned int function); -extern bool has_igd_gfx_passthru; static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev) { -return (has_igd_gfx_passthru +MachineState *machine = MACHINE(qdev_get_machine()); +return (machine->igd_gfx_passthru && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA)); } int xen_pt_register_vga_regions(XenHostPCIDevice *dev); diff --git a/vl.c b/vl.c index 5aaea77..d4e51ec 100644 --- a/vl.c +++ b/vl.c @@ -1365,13 +1365,6 @@ static inline void semihosting_arg_fallback(const char *file, const char *cmd) } } -/* Now we still need this for compatibility with XEN. */ -bool has_igd_gfx_passthru; -static void igd_gfx_passthru(void) -{ -has_igd_gfx_passthru = current_machine->igd_gfx_passthru; -} - /***/ /* USB devices */ @@ -4550,9 +4543,6 @@ int main(int argc, char **argv, char **envp) exit(1); } -/* Check if IGD GFX passthrough. */ -igd_gfx_passthru(); - /* init generic devices */ if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, NULL)) { -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 06/11] igd: use defines for standard pci config space offsets
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 6f52ab1..0784128 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -10,9 +10,9 @@ typedef struct { /* Here we just expose minimal host bridge offset subset. */ static const IGDHostInfo igd_host_bridge_infos[] = { -{0x08, 2}, /* revision id */ -{0x2c, 2}, /* sybsystem vendor id */ -{0x2e, 2}, /* sybsystem id */ +{PCI_REVISION_ID, 2}, +{PCI_SUBSYSTEM_VENDOR_ID, 2}, +{PCI_SUBSYSTEM_ID,2}, {0x50, 2}, /* SNB: processor graphics control register */ {0x52, 2}, /* processor graphics control register */ {0xa4, 4}, /* SNB: graphics base of stolen memory */ -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 10/11] igd: handle igd-passthrough-isa-bridge setup in realize()
That way a simple '-device igd-passthrough-isa-bridge,addr=1f' will do the setup. Also instead of looking up reasonable PCI IDs based on the graphic device id simply copy over the ids from the host, thereby reusing the infrastructure we have in place for the igd host bridges. Less code, and should be more robust as we don't have to maintain the id table to keep things going. Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c| 115 +-- hw/xen/xen_pt.c | 2 +- include/hw/i386/pc.h | 2 +- 3 files changed, 30 insertions(+), 89 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 96b679d..8f32c39 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -123,111 +123,52 @@ static const TypeInfo igd_passthrough_q35_info = { .class_init= igd_passthrough_q35_class_init, }; -typedef struct { -uint16_t gpu_device_id; -uint16_t pch_device_id; -uint8_t pch_revision_id; -} IGDDeviceIDInfo; - -/* In real world different GPU should have different PCH. But actually - * the different PCH DIDs likely map to different PCH SKUs. We do the - * same thing for the GPU. For PCH, the different SKUs are going to be - * all the same silicon design and implementation, just different - * features turn on and off with fuses. The SW interfaces should be - * consistent across all SKUs in a given family (eg LPT). But just same - * features may not be supported. - * - * Most of these different PCH features probably don't matter to the - * Gfx driver, but obviously any difference in display port connections - * will so it should be fine with any PCH in case of passthrough. - * - * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell) - * scenarios, 0x9cc3 for BDW(Broadwell). - */ -static const IGDDeviceIDInfo igd_combo_id_infos[] = { -/* HSW Classic */ -{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */ -{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */ -{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */ -{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */ -{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */ -/* HSW ULT */ -{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */ -{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */ -{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */ -{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */ -{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */ -{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */ -/* HSW CRW */ -{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */ -{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */ -/* HSW Server */ -{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */ -/* HSW SRVR */ -{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */ -/* BSW */ -{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */ -{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */ -{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */ -{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */ -{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */ -{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */ -{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */ -{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */ -{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */ -{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */ -{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */ +static const IGDHostInfo igd_isa_bridge_infos[] = { +{PCI_VENDOR_ID, 2}, +{PCI_DEVICE_ID, 2}, +{PCI_REVISION_ID, 2}, +{PCI_SUBSYSTEM_VENDOR_ID, 2}, +{PCI_SUBSYSTEM_ID,2}, }; +static void igd_pt_isa_bridge_realize(PCIDevice *pci_dev, Error **errp) +{ +Error *err = NULL; + +if (pci_dev->devfn != PCI_DEVFN(0x1f, 0)) { +error_setg(errp, "igd isa bridge must have address 1f.0"); +return; +} + +host_pci_config_copy(pci_dev, ":00:1f.0", + igd_isa_bridge_infos, + ARRAY_SIZE(igd_isa_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} +} + static void isa_bridge_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); dc->desc= "ISA bridge faked to support IGD PT"; -k->vendor_id= PCI_VENDOR_ID_INTEL; +k->realize = igd_pt_isa_bridge_realize; k->class_id = PCI_CLASS_BRIDGE_ISA; }; static TypeInfo igd_passthrough_isa_bridge_info = { .name = "igd-passthrough-isa-bridge", .parent= TYPE_PCI_DEVICE, -.instance_size = sizeof(PCIDevice), .class_init = isa_bridge_class_init, }; -void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id) +void igd_passthrough_isa_bridge_create(PCIBus *bus) { -struct PCIDevice *bridge_dev; -int i, num; -
[Xen-devel] [PATCH v3 07/11] igd: revamp host config read
Move all work to the host_pci_config_copy helper function, which we can easily reuse when adding q35 support. Open sysfs file only once for all values. Use pread. Proper error handling. Fix bugs: * Don't throw away results (like old host_pci_config_read did because val was passed by value not reference). * Update config space directly (writing via pci_default_write_config only works for registers whitelisted in wmask). Hmm, this code can hardly ever worked before, /me wonders what test coverage it had. With this patch in place igd-passthru=on actually works, although it still requires root priviledges because linux refuses to allow non-root users access pci config space above offset 0x50. Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 65 +++ 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 0784128..ec48875 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -19,47 +19,39 @@ static const IGDHostInfo igd_host_bridge_infos[] = { {0xa8, 4}, /* SNB: base of GTT stolen memory */ }; -static int host_pci_config_read(int pos, int len, uint32_t val) +static void host_pci_config_copy(PCIDevice *guest, const char *host, + const IGDHostInfo *list, int len, Error **errp) { -char path[PATH_MAX]; -int config_fd; -ssize_t size = sizeof(path); -/* Access real host bridge. */ -int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", - 0, 0, 0, 0, "config"); -int ret = 0; +char *path; +int config_fd, rc, i; -if (rc >= size || rc < 0) { -return -ENODEV; -} - -config_fd = open(path, O_RDWR); +path = g_strdup_printf("/sys/bus/pci/devices/%s/config", host); +config_fd = open(path, O_RDONLY); if (config_fd < 0) { -return -ENODEV; +error_setg_file_open(errp, errno, path); +goto out_free; } -if (lseek(config_fd, pos, SEEK_SET) != pos) { -ret = -errno; -goto out; +for (i = 0; i < len; i++) { +rc = pread(config_fd, guest->config + list[i].offset, + list[i].len, list[i].offset); +if (rc != list[i].len) { +error_setg_errno(errp, errno, "read %s, offset 0x%x", + path, list[i].offset); +goto out_close; +} } -do { -rc = read(config_fd, (uint8_t *)&val, len); -} while (rc < 0 && (errno == EINTR || errno == EAGAIN)); -if (rc != len) { -ret = -errno; -} -out: + +out_close: close(config_fd); -return ret; +out_free: +g_free(path); } static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { Error *err = NULL; -uint32_t val = 0; -int rc, i, num; -int pos, len; i440fx_realize(pci_dev, &err); if (err != NULL) { @@ -67,16 +59,13 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) return; } -num = ARRAY_SIZE(igd_host_bridge_infos); -for (i = 0; i < num; i++) { -pos = igd_host_bridge_infos[i].offset; -len = igd_host_bridge_infos[i].len; -rc = host_pci_config_read(pos, len, val); -if (rc) { -error_setg(errp, "failed to read host config"); -return; -} -pci_default_write_config(pci_dev, pos, val, len); +host_pci_config_copy(pci_dev, ":00:00.0", + igd_host_bridge_infos, + ARRAY_SIZE(igd_host_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; } } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 02/10] pc: remove has_igd_gfx_passthru global
> > +return (qdev_get_machine->igd_gfx_passthru > > && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA)); > > } > > Doesn't compile: > > qemu/hw/xen/xen_pt.h: In function ‘is_igd_vga_passthrough’: > qemu/hw/xen/xen_pt.h:325:29: error: request for member ‘igd_gfx_passthru’ in > something not a structure or union > return (qdev_get_machine->igd_gfx_passthru Incremental fix attached (will squash into v2). cheers, Gerd From b30226140f80202c4d2dda23acae9533aba6136b Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 18 Dec 2015 08:44:48 +0100 Subject: [PATCH] [fixup] build on xen --- hw/xen/xen_pt.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index 6d8702b..680fe6e 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -4,6 +4,7 @@ #include "qemu-common.h" #include "hw/xen/xen_common.h" #include "hw/pci/pci.h" +#include "hw/boards.h" #include "xen-host-pci-device.h" void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3); @@ -322,7 +323,8 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev, unsigned int function); static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev) { -return (qdev_get_machine->igd_gfx_passthru +MachineState *machine = MACHINE(qdev_get_machine()); +return (machine->igd_gfx_passthru && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA)); } int xen_pt_register_vga_regions(XenHostPCIDevice *dev); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 05/10] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 9 + 1 file changed, 9 insertions(+) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index d1eeafb..6f52ab1 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -53,12 +53,20 @@ out: return ret; } +static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { +Error *err = NULL; uint32_t val = 0; int rc, i, num; int pos, len; +i440fx_realize(pci_dev, &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} + num = ARRAY_SIZE(igd_host_bridge_infos); for (i = 0; i < num; i++) { pos = igd_host_bridge_infos[i].offset; @@ -77,6 +85,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); +i440fx_realize = k->realize; k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 07/10] igd: revamp host config read
Move all work to the host_pci_config_copy helper function, which we can easily reuse when adding q35 support. Open sysfs file only once for all values. Use pread. Proper error handling. Fix bugs: * Don't throw away results (like old host_pci_config_read did because val was passed by value not reference). * Update config space directly (writing via pci_default_write_config only works for registers whitelisted in wmask). Hmm, this code can hardly ever worked before, /me wonders what test coverage it had. With this patch in place igd-passthru=on actually works, although it still requires root priviledges because linux refuses to allow non-root users access pci config space above offset 0x50. Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 65 +++ 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 0784128..ec48875 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -19,47 +19,39 @@ static const IGDHostInfo igd_host_bridge_infos[] = { {0xa8, 4}, /* SNB: base of GTT stolen memory */ }; -static int host_pci_config_read(int pos, int len, uint32_t val) +static void host_pci_config_copy(PCIDevice *guest, const char *host, + const IGDHostInfo *list, int len, Error **errp) { -char path[PATH_MAX]; -int config_fd; -ssize_t size = sizeof(path); -/* Access real host bridge. */ -int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", - 0, 0, 0, 0, "config"); -int ret = 0; +char *path; +int config_fd, rc, i; -if (rc >= size || rc < 0) { -return -ENODEV; -} - -config_fd = open(path, O_RDWR); +path = g_strdup_printf("/sys/bus/pci/devices/%s/config", host); +config_fd = open(path, O_RDONLY); if (config_fd < 0) { -return -ENODEV; +error_setg_file_open(errp, errno, path); +goto out_free; } -if (lseek(config_fd, pos, SEEK_SET) != pos) { -ret = -errno; -goto out; +for (i = 0; i < len; i++) { +rc = pread(config_fd, guest->config + list[i].offset, + list[i].len, list[i].offset); +if (rc != list[i].len) { +error_setg_errno(errp, errno, "read %s, offset 0x%x", + path, list[i].offset); +goto out_close; +} } -do { -rc = read(config_fd, (uint8_t *)&val, len); -} while (rc < 0 && (errno == EINTR || errno == EAGAIN)); -if (rc != len) { -ret = -errno; -} -out: + +out_close: close(config_fd); -return ret; +out_free: +g_free(path); } static void (*i440fx_realize)(PCIDevice *pci_dev, Error **errp); static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { Error *err = NULL; -uint32_t val = 0; -int rc, i, num; -int pos, len; i440fx_realize(pci_dev, &err); if (err != NULL) { @@ -67,16 +59,13 @@ static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) return; } -num = ARRAY_SIZE(igd_host_bridge_infos); -for (i = 0; i < num; i++) { -pos = igd_host_bridge_infos[i].offset; -len = igd_host_bridge_infos[i].len; -rc = host_pci_config_read(pos, len, val); -if (rc) { -error_setg(errp, "failed to read host config"); -return; -} -pci_default_write_config(pci_dev, pos, val, len); +host_pci_config_copy(pci_dev, ":00:00.0", + igd_host_bridge_infos, + ARRAY_SIZE(igd_host_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; } } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 09/10] igd: move igd-passthrough-isa-bridge to igd.c too
Signed-off-by: Gerd Hoffmann --- hw/i386/pc_piix.c | 113 -- hw/pci-host/igd.c | 108 +++ 2 files changed, 108 insertions(+), 113 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index ce6c3c5..656bc39 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -921,119 +921,6 @@ static void pc_i440fx_0_10_machine_options(MachineClass *m) DEFINE_I440FX_MACHINE(v0_10, "pc-0.10", pc_compat_0_13, pc_i440fx_0_10_machine_options); -typedef struct { -uint16_t gpu_device_id; -uint16_t pch_device_id; -uint8_t pch_revision_id; -} IGDDeviceIDInfo; - -/* In real world different GPU should have different PCH. But actually - * the different PCH DIDs likely map to different PCH SKUs. We do the - * same thing for the GPU. For PCH, the different SKUs are going to be - * all the same silicon design and implementation, just different - * features turn on and off with fuses. The SW interfaces should be - * consistent across all SKUs in a given family (eg LPT). But just same - * features may not be supported. - * - * Most of these different PCH features probably don't matter to the - * Gfx driver, but obviously any difference in display port connections - * will so it should be fine with any PCH in case of passthrough. - * - * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell) - * scenarios, 0x9cc3 for BDW(Broadwell). - */ -static const IGDDeviceIDInfo igd_combo_id_infos[] = { -/* HSW Classic */ -{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */ -{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */ -{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */ -{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */ -{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */ -/* HSW ULT */ -{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */ -{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */ -{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */ -{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */ -{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */ -{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */ -/* HSW CRW */ -{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */ -{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */ -/* HSW Server */ -{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */ -/* HSW SRVR */ -{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */ -/* BSW */ -{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */ -{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */ -{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */ -{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */ -{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */ -{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */ -{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */ -{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */ -{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */ -{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */ -{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */ -}; - -static void isa_bridge_class_init(ObjectClass *klass, void *data) -{ -DeviceClass *dc = DEVICE_CLASS(klass); -PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); - -dc->desc= "ISA bridge faked to support IGD PT"; -k->vendor_id= PCI_VENDOR_ID_INTEL; -k->class_id = PCI_CLASS_BRIDGE_ISA; -}; - -static TypeInfo isa_bridge_info = { -.name = "igd-passthrough-isa-bridge", -.parent= TYPE_PCI_DEVICE, -.instance_size = sizeof(PCIDevice), -.class_init = isa_bridge_class_init, -}; - -static void pt_graphics_register_types(void) -{ -type_register_static(&isa_bridge_info); -} -type_init(pt_graphics_register_types) - -void igd_passthrough_isa_bridge_create(PCIBus *bus, uint16_t gpu_dev_id) -{ -struct PCIDevice *bridge_dev; -int i, num; -uint16_t pch_dev_id = 0x; -uint8_t pch_rev_id; - -num = ARRAY_SIZE(igd_combo_id_infos); -for (i = 0; i < num; i++) { -if (gpu_dev_id == igd_combo_id_infos[i].gpu_device_id) { -pch_dev_id = igd_combo_id_infos[i].pch_device_id; -pch_rev_id = igd_combo_id_infos[i].pch_revision_id; -} -} - -if (pch_dev_id == 0x) { -return; -} - -/* Currently IGD drivers always need to access PCH by 1f.0. */ -bridge_dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0), - "igd-passthrough-isa-bridge"); - -/* - * Note that vendor id is always PCI_VENDOR_ID_INTEL. - */ -if (!bridge_dev) { -fprintf(stderr, "set igd-passthrough-isa-bridge failed!\n"); -return; -} -pci_config_set_device_id(bridge_dev->config, pch_dev_id); -pci_config_set_revision(bridge_dev->config, pch_rev_id); -} - static void isapc_machine_options(MachineClass *m) { m->desc = "ISA-only PC"; diff --g
[Xen-devel] [PATCH v2 02/10] pc: remove has_igd_gfx_passthru global
Signed-off-by: Gerd Hoffmann --- hw/xen/xen_pt.h | 3 +-- vl.c| 10 -- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index c545280..6d8702b 100644 --- a/hw/xen/xen_pt.h +++ b/hw/xen/xen_pt.h @@ -320,10 +320,9 @@ extern void *pci_assign_dev_load_option_rom(PCIDevice *dev, unsigned int domain, unsigned int bus, unsigned int slot, unsigned int function); -extern bool has_igd_gfx_passthru; static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev) { -return (has_igd_gfx_passthru +return (qdev_get_machine->igd_gfx_passthru && ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA)); } int xen_pt_register_vga_regions(XenHostPCIDevice *dev); diff --git a/vl.c b/vl.c index 4211ff1..e45a1da 100644 --- a/vl.c +++ b/vl.c @@ -1365,13 +1365,6 @@ static inline void semihosting_arg_fallback(const char *file, const char *cmd) } } -/* Now we still need this for compatibility with XEN. */ -bool has_igd_gfx_passthru; -static void igd_gfx_passthru(void) -{ -has_igd_gfx_passthru = current_machine->igd_gfx_passthru; -} - /***/ /* USB devices */ @@ -4550,9 +4543,6 @@ int main(int argc, char **argv, char **envp) exit(1); } -/* Check if IGD GFX passthrough. */ -igd_gfx_passthru(); - /* init generic devices */ if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, NULL)) { -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 08/10] igd: add q35 support
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 41 - hw/pci-host/q35.c | 6 +- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index ec48875..f6e3f7a 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -1,5 +1,6 @@ #include "qemu-common.h" #include "hw/pci/pci.h" +#include "hw/pci-host/q35.h" #include "hw/i386/pc.h" /* IGD Passthrough Host Bridge. */ @@ -76,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) i440fx_realize = k->realize; k->realize = igd_pt_i440fx_realize; -dc->desc = "IGD Passthrough Host bridge"; +dc->desc = "IGD Passthrough Host bridge (i440fx)"; } static const TypeInfo igd_passthrough_i440fx_info = { @@ -85,9 +86,47 @@ static const TypeInfo igd_passthrough_i440fx_info = { .class_init= igd_passthrough_i440fx_class_init, }; +static void (*q35_realize)(PCIDevice *pci_dev, Error **errp); +static void igd_pt_q35_realize(PCIDevice *pci_dev, Error **errp) +{ +Error *err = NULL; + +q35_realize(pci_dev, &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} + +host_pci_config_copy(pci_dev, ":00:00.0", + igd_host_bridge_infos, + ARRAY_SIZE(igd_host_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} +} + +static void igd_passthrough_q35_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + +q35_realize = k->realize; +k->realize = igd_pt_q35_realize; +dc->desc = "IGD Passthrough Host bridge (q35)"; +} + +static const TypeInfo igd_passthrough_q35_info = { +.name = "igd-passthrough-q35-mch", +.parent= TYPE_MCH_PCI_DEVICE, +.class_init= igd_passthrough_q35_class_init, +}; + static void igd_register_types(void) { type_register_static(&igd_passthrough_i440fx_info); +type_register_static(&igd_passthrough_q35_info); } type_init(igd_register_types) diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index 1fb4707..07dc595 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -151,7 +151,11 @@ static void q35_host_initfn(Object *obj) memory_region_init_io(&phb->data_mem, obj, &pci_host_data_le_ops, phb, "pci-conf-data", 4); -object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE); +if (object_property_get_bool(qdev_get_machine(), "igd-passthru", NULL)) { +object_initialize(&s->mch, sizeof(s->mch), "igd-passthrough-q35-mch"); +} else { +object_initialize(&s->mch, sizeof(s->mch), TYPE_MCH_PCI_DEVICE); +} object_property_add_child(OBJECT(s), "mch", OBJECT(&s->mch), NULL); qdev_prop_set_uint32(DEVICE(&s->mch), "addr", PCI_DEVFN(0, 0)); qdev_prop_set_bit(DEVICE(&s->mch), "multifunction", false); -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 10/10] igd: handle igd-passthrough-isa-bridge setup in realize()
That way a simple '-device igd-passthrough-isa-bridge,addr=1f' will do the setup. Also instead of looking up reasonable PCI IDs based on the graphic device id simply copy over the ids from the host, thereby reusing the infrastructure we have in place for the igd host bridges. Less code, and should be more robust as we don't have to maintain the id table to keep things going. Note that igd-passthrough-isa-bridge will be needed for '-machine pc' only. For q35 the plan is https://lkml.org/lkml/2015/11/26/183 (should land in the next merge window, i.e. linux 4.5). TODO: Figure if and how we are going to add this to the virtual machine automatically. The options I see are: (1) Nothing automatic, users must add the device manually. This is what you get with this patch, except when running on xen. (2) Do it the xen way, let the pci pass-thru code add it when it finds a igd device (i.e. vfio-pci for kvm). It's a bit ugly though, and it also has the problem that pc and q35 machine types have different needs here. (3) Let machine init do it in case igd-passthru=on is set. Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 115 +- 1 file changed, 28 insertions(+), 87 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 96b679d..2887b31 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -123,111 +123,52 @@ static const TypeInfo igd_passthrough_q35_info = { .class_init= igd_passthrough_q35_class_init, }; -typedef struct { -uint16_t gpu_device_id; -uint16_t pch_device_id; -uint8_t pch_revision_id; -} IGDDeviceIDInfo; - -/* In real world different GPU should have different PCH. But actually - * the different PCH DIDs likely map to different PCH SKUs. We do the - * same thing for the GPU. For PCH, the different SKUs are going to be - * all the same silicon design and implementation, just different - * features turn on and off with fuses. The SW interfaces should be - * consistent across all SKUs in a given family (eg LPT). But just same - * features may not be supported. - * - * Most of these different PCH features probably don't matter to the - * Gfx driver, but obviously any difference in display port connections - * will so it should be fine with any PCH in case of passthrough. - * - * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell) - * scenarios, 0x9cc3 for BDW(Broadwell). - */ -static const IGDDeviceIDInfo igd_combo_id_infos[] = { -/* HSW Classic */ -{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */ -{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */ -{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */ -{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */ -{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */ -/* HSW ULT */ -{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */ -{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */ -{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */ -{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */ -{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */ -{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */ -/* HSW CRW */ -{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */ -{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */ -/* HSW Server */ -{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */ -/* HSW SRVR */ -{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */ -/* BSW */ -{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */ -{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */ -{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */ -{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */ -{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */ -{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */ -{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */ -{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */ -{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */ -{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */ -{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */ +static const IGDHostInfo igd_isa_bridge_infos[] = { +{PCI_VENDOR_ID, 2}, +{PCI_DEVICE_ID, 2}, +{PCI_REVISION_ID, 2}, +{PCI_SUBSYSTEM_VENDOR_ID, 2}, +{PCI_SUBSYSTEM_ID,2}, }; +static void igd_pt_isa_bridge_realize(PCIDevice *pci_dev, Error **errp) +{ +Error *err = NULL; + +if (pci_dev->devfn != PCI_DEVFN(0x1f, 0)) { +error_setg(errp, "igd isa bridge must have address 1f.0"); +return; +} + +host_pci_config_copy(pci_dev, ":00:1f.0", + igd_isa_bridge_infos, + ARRAY_SIZE(igd_isa_bridge_infos), + &err); +if (err != NULL) { +error_propagate(errp, err); +return; +} +} + static void isa_bridge_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceCla
[Xen-devel] [PATCH v2 06/10] igd: use defines for standard pci config space offsets
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index 6f52ab1..0784128 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -10,9 +10,9 @@ typedef struct { /* Here we just expose minimal host bridge offset subset. */ static const IGDHostInfo igd_host_bridge_infos[] = { -{0x08, 2}, /* revision id */ -{0x2c, 2}, /* sybsystem vendor id */ -{0x2e, 2}, /* sybsystem id */ +{PCI_REVISION_ID, 2}, +{PCI_SUBSYSTEM_VENDOR_ID, 2}, +{PCI_SUBSYSTEM_ID,2}, {0x50, 2}, /* SNB: processor graphics control register */ {0x52, 2}, /* processor graphics control register */ {0xa4, 4}, /* SNB: graphics base of stolen memory */ -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 03/10] pc: move igd support code to igd.c
Pure code motion, except for dropping instance_size for TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE (no need to set, we can inherit it from TYPE_I440FX_PCI_DEVICE). Signed-off-by: Gerd Hoffmann Acked-by: Stefano Stabellini --- hw/pci-host/Makefile.objs | 3 ++ hw/pci-host/igd.c | 96 +++ hw/pci-host/piix.c| 88 --- 3 files changed, 99 insertions(+), 88 deletions(-) create mode 100644 hw/pci-host/igd.c diff --git a/hw/pci-host/Makefile.objs b/hw/pci-host/Makefile.objs index 45f1f0e..e341a49 100644 --- a/hw/pci-host/Makefile.objs +++ b/hw/pci-host/Makefile.objs @@ -11,6 +11,9 @@ common-obj-$(CONFIG_PPCE500_PCI) += ppce500.o # ARM devices common-obj-$(CONFIG_VERSATILE_PCI) += versatile.o +# igd passthrough support +common-obj-$(CONFIG_LINUX) += igd.o + common-obj-$(CONFIG_PCI_APB) += apb.o common-obj-$(CONFIG_FULONG) += bonito.o common-obj-$(CONFIG_PCI_PIIX) += piix.o diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c new file mode 100644 index 000..ef0273b --- /dev/null +++ b/hw/pci-host/igd.c @@ -0,0 +1,96 @@ +#include "qemu-common.h" +#include "hw/pci/pci.h" +#include "hw/i386/pc.h" + +/* IGD Passthrough Host Bridge. */ +typedef struct { +uint8_t offset; +uint8_t len; +} IGDHostInfo; + +/* Here we just expose minimal host bridge offset subset. */ +static const IGDHostInfo igd_host_bridge_infos[] = { +{0x08, 2}, /* revision id */ +{0x2c, 2}, /* sybsystem vendor id */ +{0x2e, 2}, /* sybsystem id */ +{0x50, 2}, /* SNB: processor graphics control register */ +{0x52, 2}, /* processor graphics control register */ +{0xa4, 4}, /* SNB: graphics base of stolen memory */ +{0xa8, 4}, /* SNB: base of GTT stolen memory */ +}; + +static int host_pci_config_read(int pos, int len, uint32_t val) +{ +char path[PATH_MAX]; +int config_fd; +ssize_t size = sizeof(path); +/* Access real host bridge. */ +int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", + 0, 0, 0, 0, "config"); +int ret = 0; + +if (rc >= size || rc < 0) { +return -ENODEV; +} + +config_fd = open(path, O_RDWR); +if (config_fd < 0) { +return -ENODEV; +} + +if (lseek(config_fd, pos, SEEK_SET) != pos) { +ret = -errno; +goto out; +} +do { +rc = read(config_fd, (uint8_t *)&val, len); +} while (rc < 0 && (errno == EINTR || errno == EAGAIN)); +if (rc != len) { +ret = -errno; +} +out: +close(config_fd); +return ret; +} + +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +{ +uint32_t val = 0; +int rc, i, num; +int pos, len; + +num = ARRAY_SIZE(igd_host_bridge_infos); +for (i = 0; i < num; i++) { +pos = igd_host_bridge_infos[i].offset; +len = igd_host_bridge_infos[i].len; +rc = host_pci_config_read(pos, len, val); +if (rc) { +return -ENODEV; +} +pci_default_write_config(pci_dev, pos, val, len); +} + +return 0; +} + +static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); + +k->init = igd_pt_i440fx_initfn; +dc->desc = "IGD Passthrough Host bridge"; +} + +static const TypeInfo igd_passthrough_i440fx_info = { +.name = TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE, +.parent= TYPE_I440FX_PCI_DEVICE, +.class_init= igd_passthrough_i440fx_class_init, +}; + +static void igd_register_types(void) +{ +type_register_static(&igd_passthrough_i440fx_info); +} + +type_init(igd_register_types) diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index 715208b..ccacb57 100644 --- a/hw/pci-host/piix.c +++ b/hw/pci-host/piix.c @@ -744,93 +744,6 @@ static const TypeInfo i440fx_info = { .class_init= i440fx_class_init, }; -/* IGD Passthrough Host Bridge. */ -typedef struct { -uint8_t offset; -uint8_t len; -} IGDHostInfo; - -/* Here we just expose minimal host bridge offset subset. */ -static const IGDHostInfo igd_host_bridge_infos[] = { -{0x08, 2}, /* revision id */ -{0x2c, 2}, /* sybsystem vendor id */ -{0x2e, 2}, /* sybsystem id */ -{0x50, 2}, /* SNB: processor graphics control register */ -{0x52, 2}, /* processor graphics control register */ -{0xa4, 4}, /* SNB: graphics base of stolen memory */ -{0xa8, 4}, /* SNB: base of GTT stolen memory */ -}; - -static int host_pci_config_read(int pos, int len, uint32_t val) -{ -char path[PATH_MAX]; -int config_fd; -ssize_t size = sizeof(path); -/* Access real host bridge. */ -int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s", - 0, 0, 0, 0, "config"); -
[Xen-devel] [PATCH v2 04/10] igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize
Signed-off-by: Gerd Hoffmann --- hw/pci-host/igd.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/hw/pci-host/igd.c b/hw/pci-host/igd.c index ef0273b..d1eeafb 100644 --- a/hw/pci-host/igd.c +++ b/hw/pci-host/igd.c @@ -53,7 +53,7 @@ out: return ret; } -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) +static void igd_pt_i440fx_realize(PCIDevice *pci_dev, Error **errp) { uint32_t val = 0; int rc, i, num; @@ -65,12 +65,11 @@ static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) len = igd_host_bridge_infos[i].len; rc = host_pci_config_read(pos, len, val); if (rc) { -return -ENODEV; +error_setg(errp, "failed to read host config"); +return; } pci_default_write_config(pci_dev, pos, val, len); } - -return 0; } static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) @@ -78,7 +77,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); -k->init = igd_pt_i440fx_initfn; +k->realize = igd_pt_i440fx_realize; dc->desc = "IGD Passthrough Host bridge"; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 00/10] igd passthrough chipset tweaks
Hi, We have some code in our tree to support pci passthrough of intel graphics devices (igd) on xen, which requires some chipset tweaks for (a) the host bridge and (b) the lpc/isa-bridge to meat the expectations of the guest driver. For kvm we need pretty much the same, also the requirements for vgpu (xengt/kvmgt) are very simliar. This patch wires up the existing support for kvm. It also brings a bunch of bugfixes and cleanups. Unfortunaly the oldish laptop I had planned to use for testing turned out to have no working iommu support for igd, so this patch series still has seen very light testing only. Any testing feedback is very welcome. Testing with kvm/i440fx: Add '-M pc,igd-passthru=on -device igd-passthrough-isa-bridge,addr=1f' to turn on the chipset tweaks. Passthrough the igd using vfio. Testing with kvm/q35: Add '-M q35,igd-passthru=on' to turn on the the chipset tweaks. Pick up the linux kernel patch referenced in patch #10, build a custom kernel with it. Passthrough the igd using vfio. Testing with xen: Existing setups should continue working ;) Changes in v2: * Added igd-passthrough-isa-bridge support form kvm. * Added patch to drop has_igd_gfx_passthru. TODO: * Possibly handle igd-passthrough-isa-bridge automatically (see patch 10). * Figure a way to handle the opregion, probably via vfio extension. Beyond the scope of this patch series, but probably needed to make laptop panels work correctly. Gerd Hoffmann (10): pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen pc: remove has_igd_gfx_passthru global pc: move igd support code to igd.c igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize igd: use defines for standard pci config space offsets igd: revamp host config read igd: add q35 support igd: move igd-passthrough-isa-bridge to igd.c too igd: handle igd-passthrough-isa-bridge setup in realize() hw/i386/pc_piix.c | 124 ++- hw/pci-host/Makefile.objs | 3 + hw/pci-host/igd.c | 181 ++ hw/pci-host/piix.c| 88 -- hw/pci-host/q35.c | 6 +- hw/xen/xen_pt.h | 3 +- vl.c | 10 --- 7 files changed, 195 insertions(+), 220 deletions(-) create mode 100644 hw/pci-host/igd.c -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 01/10] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen
rename pc_xen_hvm_init_pci to pc_i440fx_init_pci, use it for both xen and non-xen init. That changes behavior of all pc-i440fx-$version machine types where specifying -machine igd-passthru=on used to have no effect and now it has. It is unlikely to cause any trouble though as there used to be no reason to add that option to kvm guests in the first place. Signed-off-by: Gerd Hoffmann Reviewed-by: Eduardo Habkost Reviewed-by: Stefano Stabellini --- hw/i386/pc_piix.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 2e41efe..ce6c3c5 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -419,10 +419,9 @@ static void pc_init_isa(MachineState *machine) pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE); } -#ifdef CONFIG_XEN -static void pc_xen_hvm_init_pci(MachineState *machine) +static void pc_i440fx_init_pci(MachineState *machine) { -const char *pci_type = has_igd_gfx_passthru ? +const char *pci_type = machine->igd_gfx_passthru ? TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : TYPE_I440FX_PCI_DEVICE; pc_init1(machine, @@ -430,6 +429,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine) pci_type); } +#ifdef CONFIG_XEN static void pc_xen_hvm_init(MachineState *machine) { PCIBus *bus; @@ -439,7 +439,7 @@ static void pc_xen_hvm_init(MachineState *machine) exit(1); } -pc_xen_hvm_init_pci(machine); +pc_i440fx_init_pci(machine); bus = pci_find_primary_bus(); if (bus != NULL) { @@ -455,8 +455,7 @@ static void pc_xen_hvm_init(MachineState *machine) if (compat) { \ compat(machine); \ } \ -pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \ - TYPE_I440FX_PCI_DEVICE); \ +pc_i440fx_init_pci(machine); \ } \ DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn) -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH] ehci: make idt processing more robust
Make ehci_process_itd return an error in case we didn't do any actual iso transfer because we've found no active transaction. That'll avoid ehci happily run in circles forever if the guest builds a loop out of idts. Signed-off-by: Gerd Hoffmann --- hw/usb/hcd-ehci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 4e2161b..d07f228 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1389,7 +1389,7 @@ static int ehci_process_itd(EHCIState *ehci, { USBDevice *dev; USBEndpoint *ep; -uint32_t i, len, pid, dir, devaddr, endp; +uint32_t i, len, pid, dir, devaddr, endp, xfers = 0; uint32_t pg, off, ptr1, ptr2, max, mult; ehci->periodic_sched_active = PERIODIC_ACTIVE; @@ -1479,9 +1479,10 @@ static int ehci_process_itd(EHCIState *ehci, ehci_raise_irq(ehci, USBSTS_INT); } itd->transact[i] &= ~ITD_XACT_ACTIVE; +xfers++; } } -return 0; +return xfers ? 0 : -1; } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Intel-gfx] [Announcement] 2015-Q3 release of XenGT - a Mediated Graphics Passthrough Solution from Intel
Hi, > btw some questions here: > > for non-gl and gl rendering in Qemu, are they based on dma-buf already? > once we can export guest framebuffer in dma-buf, is there additional work > required or just straightforward to integrate with SPICE? Right now we are busy integrating dma-buf support into spice, which will be used for the gl rendering path, for virtio-gpu. For intel-vgpu the wireup inside qemu will be slightly different: We'll get a dma-buf handle from the igd driver, whereas virtio-gpu renders into a texture, then exports that texture as dma-buf. But in both cases we'll go pass the dma-buf with the guest framebuffer (and meta-data such as fourcc and size) to spice-server, which in turn will pass on the dma-buf to spice-client for (local) display. So we have a common code path in spice for both virtio-gpu and intel-vgpu, based on dma-bufs. spice-server even doesn't need to know what kind of graphics device the guest has, it'll go just process the dma-bufs. longer-term we also plan to support video-encoding for a remote display. Again based on dma-bufs, by sending them to the gpu video encoder. The non-gl rendering path needs to be figured out. With virtio-gpu we'll go simply turn off 3d support, so the guest will fallback to do software rendering, we'll get a classic DisplaySurface and the vnc server can work with that. That isn't going to fly with intel-vgpu though, so we need something else. Import dma-buf, then glReadPixels into a DisplaySurface would work. But as mentioned before I'd prefer a code path which doesn't require opengl support in qemu, and one option for that would be the special vfio region. I've written up a quick draft meanwhile: diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 751b69f..91b928d 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -596,6 +596,28 @@ struct vfio_iommu_spapr_tce_remove { }; #define VFIO_IOMMU_SPAPR_TCE_REMOVE_IO(VFIO_TYPE, VFIO_BASE + 20) +/* Additional API for vGPU */ + +/* + * framebuffer meta data + * subregion located at the end of the framebuffer region + */ +struct vfio_framebuffer { + __u32 argsz; + + /* out */ + __u32 format;/* drm fourcc */ + __u32 offset;/* relative to region start */ + __u32 width; /* in pixels */ + __u32 height;/* in pixels */ + __u32 stride;/* in bytes */ + + /* in+out */ +#define VFIO_FB_STATE_REQUEST_UPDATE 1 /* userspace requests update */ +#define VFIO_FB_STATE_UPDATE_COMPLETE 2 /* kernel signals completion */ + __u32 state; /* VFIO_FB_STATE_ */ +}; + /* * */ #endif /* _UAPIVFIO_H */ cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH for-2.5] vnc: fix segfault
Commit "c7628bf vnc: only alloc server surface with clients connected" missed one rarely used codepath (cirrus with guest drivers using 2d accel) where we have to check for the server surface being present, to avoid qemu crashing with a NULL pointer dereference. Add the check. Reported-by: Anthony PERARD Signed-off-by: Gerd Hoffmann --- ui/vnc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/ui/vnc.c b/ui/vnc.c index c9f2fed..7538405 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -931,6 +931,11 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl, int i, x, y, pitch, inc, w_lim, s; int cmp_bytes; +if (!vd->server) { +/* no client connected */ +return; +} + vnc_refresh_server_surface(vd); QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) { if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) { -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Intel-gfx] [Announcement] 2015-Q3 release of XenGT - a Mediated Graphics Passthrough Solution from Intel
Hi, > But there's some work to add generic mmap support to dma-bufs, and for > really simple case (where we don't have a gl driver to handle the dma-buf > specially) for untiled framebuffers that would be all we need? Not requiring gl is certainly a bonus, people might want build qemu without opengl support to reduce the attach surface and/or package dependency chain. And, yes, requirements for the non-gl rendering path are pretty low. qemu needs something it can mmap, and which it can ask pixman to handle. Preferred format is PIXMAN_x8r8g8b8 (qemu uses that internally in alot of places so this avoids conversions). Current plan is to have a special vfio region (not visible to the guest) where the framebuffer lives, with one or two pages at the end for meta data (format and size). Status field is there too and will be used by qemu to request updates and the kernel to signal update completion. Guess I should write that down as vfio rfc patch ... I don't think it makes sense to have fields to notify qemu about which framebuffer regions have been updated, I'd expect with full-screen composing we have these days this information isn't available anyway. Maybe a flag telling whenever there have been updates or not, so qemu can skip update processing in case we have the screensaver showing a black screen all day long. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Intel-gfx] [Announcement] 2015-Q3 release of XenGT - a Mediated Graphics Passthrough Solution from Intel
Hi, > > Yes, vGPU may have additional features, like a framebuffer area, that > > aren't present or optional for direct assignment. Obviously we support > > direct assignment of GPUs for some vendors already without this feature. > > For exposing framebuffers for spice/vnc I highly recommend against > anything that looks like a bar/fixed mmio range mapping. First this means > the kernel driver needs to internally fake remapping, which isn't fun. Sure. I don't think we should remap here. More below. > My recoomendation is to build the actual memory access for underlying > framebuffers on top of dma-buf, so that it can be vacuumed up by e.g. the > host gpu driver again for rendering. We want that too ;) Some more background: OpenGL support in qemu is still young and emerging, and we are actually building on dma-bufs here. There are a bunch of different ways how guest display output is handled. At the end of the day it boils down to only two fundamental cases though: (a) Where qemu doesn't need access to the guest framebuffer - qemu directly renders via opengl (works today with virtio-gpu and will be in the qemu 2.5 release) - qemu passed on the dma-buf to spice client for local display (experimental code exists). - qemu feeds the guest display into gpu-assisted video encoder to send a stream over the network (no code yet). (b) Where qemu must read the guest framebuffer. - qemu's builtin vnc server. - qemu writing screenshots to file. - (non-opengl legacy code paths for local display, will hopefully disappear long-term though ...) So, the question is how to support (b) best. Even with OpenGL support in qemu improving over time I don't expect this going away completely anytime soon. I think it makes sense to have a special vfio region for that. I don't think remapping makes sense there. It doesn't need to be "live", it doesn't need support high refresh rates. Placing a copy of the guest framebuffer there on request (and convert from tiled to linear while being at it) is perfectly fine. qemu has a adaptive update rate and will stop doing frequent update requests when the vnc client disconnects, so there will be nothing to do if nobody wants actually see the guest display. Possible alternative approach would be to import a dma-buf, then use glReadPixels(). I suspect when doing the copy in the kernel the driver could ask just the gpu to blit the guest framebuffer. Don't know gfx hardware good enough to be sure though, comments are welcome. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Intel-gfx] [Announcement] 2015-Q3 release of XenGT - a Mediated Graphics Passthrough Solution from Intel
Hi, > > iGVT-g_Setup_Guide.txt mentions a "Indirect Display Mode", but doesn't > > explain how the guest framebuffer can be accessed then. > > You can check "fb_decoder.h". One thing to clarify. Its format is > actually based on drm definition, instead of OpenGL. Sorry for > that. drm is fine. That header explains the format, but not how it can be accessed. Is the guest fb exported as dma-buf? > > So, for non-opengl rendering qemu needs the guest framebuffer data so it > > can feed it into the vnc server. The vfio framebuffer region is meant > > to support this use case. > > what's the format requirement on that framebuffer? If you are familiar > with Intel Graphics, there's a so-called tiling feature applied on frame > buffer so it can't be used as a raw input to vnc server. w/o opengl you > need do some conversion on CPU first. Yes, that conversion needs to happen, qemu can't deal with tiled graphics. Anything which pixman can handle will work. Prefered would be PIXMAN_x8r8g8b8 (aka DRM_FORMAT_XRGB on little endian host) which is the format used by the vnc server (and other places in qemu) internally. qemu can also use the opengl texture for the guest fb, then fetch the data with glReadPixels(). Which will probably do exactly the same conversion. But it'll add a opengl dependency to the non-opengl rendering path in qemu, would be nice if we can avoid that. While being at it: When importing a dma-buf with a tiled framebuffer into opengl (via eglCreateImageKHR + EGL_LINUX_DMA_BUF_EXT) I suspect we have to pass in the tile size as attribute to make it work. Is that correct? cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Intel-gfx] [Announcement] 2015-Q3 release of XenGT - a Mediated Graphics Passthrough Solution from Intel
Hi, > > Another area of extension is how to expose a framebuffer to QEMU for > > seamless integration into a SPICE/VNC channel. For this I believe we > > could use a new region, much like we've done to expose VGA access > > through a vfio device file descriptor. An area within this new > > framebuffer region could be directly mappable in QEMU while a > > non-mappable page, at a standard location with standardized format, > > provides a description of framebuffer and potentially even a > > communication channel to synchronize framebuffer captures. This would > > be new code for QEMU, but something we could share among all vGPU > > implementations. > > Now GVT-g already provides an interface to decode framebuffer information, > w/ an assumption that the framebuffer will be further composited into > OpenGL APIs. Can I have a pointer to docs / code? iGVT-g_Setup_Guide.txt mentions a "Indirect Display Mode", but doesn't explain how the guest framebuffer can be accessed then. > So the format is defined according to OpenGL definition. > Does that meet SPICE requirement? Yes and no ;) Some more background: We basically have two rendering paths in qemu. The classic one, without opengl, and a new, still emerging one, using opengl and dma-bufs (gtk support merged for qemu 2.5, sdl2 support will land in 2.6, spice support still WIP, hopefully 2.6 too). For best performance you probably want use the new opengl-based rendering whenever possible. However I do *not* expect the classic rendering path disappear, we'll continue to need that in various cases, most prominent one being vnc support. So, for non-opengl rendering qemu needs the guest framebuffer data so it can feed it into the vnc server. The vfio framebuffer region is meant to support this use case. > Another thing to be added. Framebuffers are frequently switched in > reality. So either Qemu needs to poll or a notification mechanism is required. The idea is to have qemu poll (and adapt poll rate, i.e. without vnc client connected qemu will poll alot less frequently). > And since it's dynamic, having framebuffer page directly exposed in the > new region might be tricky. We can just expose framebuffer information > (including base, format, etc.) and let Qemu to map separately out of VFIO > interface. Allocate some memory, ask gpu to blit the guest framebuffer there, i.e. provide a snapshot of the current guest display instead of playing mapping tricks? > And... this works fine with vGPU model since software knows all the > detail about framebuffer. However in pass-through case, who do you expect > to provide that information? Is it OK to introduce vGPU specific APIs in > VFIO? It will only be used in the vgpu case, not for pass-though. We think it is better to extend the vfio interface to improve vgpu support rather than inventing something new while vfio can satisfy 90% of the vgpu needs already. We want avoid vendor-specific extensions though, the vgpu extension should work across vendors. > Now there is no standard. We expose vGPU life-cycle mgmt. APIs through > sysfs (under i915 node), which is very Intel specific. In reality different > vendors have quite different capabilities for their own vGPUs, so not sure > how standard we can define such a mechanism. Agree when it comes to create vGPU instances. > But this code should be > minor to be maintained in libvirt. As far I know libvirt only needs to discover those devices. If they look like sr/iov devices in sysfs this might work without any changes to libvirt. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Qemu-devel] Question about xen disk unplug support for ahci missed in qemu
Hi, > > I'm trying to follow this discussion as best as I am able, but my lack > > of experience with Xen prevents me from really participating in a > > meaningful way. > > > > (I see that Laszlo is still discussing some CD-ROM issues with Fabio > > which may be of interest to me...) > > > > At any rate, I won't be authoring any Xen-specific hacks to the AHCI > > device, but I do have plans to implement hot-plugging emulation as per > > the AHCI spec. Perhaps this is sufficient for the Xen layer, but someone > > else will need to author the appropriate glue code. > > > > If "real" hot-plugging is not sufficient, we'll need to discuss further, > > preferably over some RFC patches. > > That's fine. AHCI hot-plugging would go a long way and once we have > that, the rest is easy. Can we get some more background on this? IIRC the IDE bits are needed to boot hvm guests, which goes like this: (1) boot disk is hooked up using both xenbus and ide. (2) seabios boots using ide. (3) linux kernel activates xenbus, at which point qemu zaps the ide disks to avoid the disk being present twice in the system. Correct? Do we really want repeat this exercise for AHCI? Alot has changed since this boot hack for ide was added ... As far I know OVMF has xenbus drivers, so OVMF should already boot xen guests just fine without this, correct? Can we just have xenbus drivers for seabios too? seabios can run disk drivers in 32bit mode meanwhile, so this should not be as difficult any more as it used to be. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices
> > Why multiple small iovecs instead of one big iovec? > > The guest buffer might span multiple physical non contiguous pages. Sure, thats why we have iovecs in the first place. > I > don't want to copy data to a new buffer due to performance reasons > (there is already at least one copy operation done by qemu). We can walk the iovec and fill the libusb struct with the pointers and lengths instead of using usb_copy_packet ... > > usb_host_req_complete_iso_xen() returns a single status for the whole > > USBPacket anyway ... > > I need status per iso request, and libusb does deliver that. So one usbpacket per iso request should make this work. And of you enable pipelining for the endpoint qemu allows multiple outstanding async packages, so you can queue up a batch of requests ... cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices
Hi, > > So, the signaling needs to be different. The host adapter needs to > > signal somehow that it can handle async iso packets. One way would be > > to flag this per usb bus, another one per usb packet. Also all xen > > naming and the xen inlude should go away. BTW: does this build without > > xen-devel installed? > > Okay, I'll try to make it more generic. I think the async iso capability > should be a bus attribute. Makes sense. > > Can we get rid of the callbacks? By filling the USBPacket iovec with > > the iso request chunks for example? > > Difficult. One iso request chunk could require multiple iovec entries. Why multiple small iovecs instead of one big iovec? usb_host_req_complete_iso_xen() returns a single status for the whole USBPacket anyway ... > The RFC version tried to avoid the callbacks and there you didn't like > exposing the additional structures. -ENOPATCH. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch V1 2/3] xen/usb: add capability for passing through isoc jobs to host devices
On Do, 2015-09-03 at 12:45 +0200, Juergen Gross wrote: > When Xen is using the qemu usb framework for pure passthrough of I/Os > to host devices the handling of isoc jobs is rather complicated if > multiple isoc frames are transferred with one call. > > Instead of calling the framework with each frame individually, using > timers to avoid polling in a loop and sampling all responses to > construct a sum response for the user, just add a capability to > use the libusb isoc framework instead. This capability is selected > via a device specific property. > > When the property is selected the host usb driver will use xen specific > callbacks to signal the end of isoc I/Os. For now these callbacks will > just be nops, they'll be filled with sensible actions when the xen > pv-usb backend is being added. So you basically add support for async isoc requests. Fine. There is nothing xen specific in this though, except that xen is (so far) the only user. It isn't going to work for uhci and ehci, put possibly xhci can join the party. So, the signaling needs to be different. The host adapter needs to signal somehow that it can handle async iso packets. One way would be to flag this per usb bus, another one per usb packet. Also all xen naming and the xen inlude should go away. BTW: does this build without xen-devel installed? Can we get rid of the callbacks? By filling the USBPacket iovec with the iso request chunks for example? cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch RFC 1/4] usb: support device specification via -
Hi, > > device_add monitor command. > > Okay. I guess the USBDevice for the added device can be obtained > in USBPortOps->attach via USBPort->dev ? Yes. > Doing a quick search through the sources I couldn't find a way to > issue a monitor command from inside qemu. I assume I'd have to use > do_device_add() directly? qdev_device_add() should do it, yes. Why do you need to call this from inside qemu? Watching on xenbus and responding to new devices created there? cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch RFC 2/4] usb: add flag to USBPacket to request complete callback after isoc transfer
> > --- a/hw/usb/host-libusb.c > > +++ b/hw/usb/host-libusb.c > > @@ -451,6 +451,7 @@ static void usb_host_req_complete_iso(struct > > libusb_transfer *transfer) > > } > > if (xfer->ring->ep->pid == USB_TOKEN_IN) { > > QTAILQ_INSERT_TAIL(&xfer->ring->copy, xfer, next); > > +usb_wakeup(xfer->ring->ep, 0); > > } else { > > QTAILQ_INSERT_TAIL(&xfer->ring->unused, xfer, next); > > } > > Hmm, I can see the benefit of this call to avoid polling. > > OTOH I don't see how to find the packages already processed via this > mechanism. To help in my case I'd need: > > - the call being made in the else clause Hmm. This is for IN transfers, notifying the host adapter "I have data for you, please hand me one (or more) USBPacket which I can fill". Why do you need it for OUT transfers too? usb-host has copyed and queued up the data already, there is nothing to pass back ... > - some way to have a package reference in the endpoint (assuming >to use the bus .endpoint_wakeup callback which is called by >usb_wakeup(), too). Yes, endpoint callback would be more useful for this. PortOps needs this for remote wakeup implementation. >The problem here is that host-libusb.c would call usb_wakeup() >not for each packet, but for each libusb I/O, which is combining >multiple packets given to usb_handle_packet(). You can call just call usb_handle_packet() multiple times, either calculate how often based on time and bandwidth, or continue calling until you get no more data back. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch RFC 1/4] usb: support device specification via -
On Fr, 2015-07-17 at 09:32 +0200, Juergen Gross wrote: > On 07/17/2015 08:59 AM, Gerd Hoffmann wrote: > > On Do, 2015-07-16 at 17:47 +0200, Juergen Gross wrote: > >> Today a host usb device can be specified either via : > >> or via . syntax. Add the possibility to specify it via > >> - as this is needed for the support of xen pvusb backend. > > > > -device usb-host,hostbus=,hostport= should already do what > > you want. > > The problem is I have to add the device while qemu is already running, > so I'm using usb_host_device_open(). To be able to specify the device > via - I have to add the capability to find the device by > those parameters. > > I haven't found another way to achieve this. device_add monitor command. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch RFC 2/4] usb: add flag to USBPacket to request complete callback after isoc transfer
On Do, 2015-07-16 at 17:47 +0200, Juergen Gross wrote: > In order to avoid having to poll for the result of an iso transfer > add the possibility to request the "complete" callback which is being > used for bulk transfers as well. Sorry for the late notice (didn't do much usb coding recently and forgot about it): We actually _have_ a notification mechanism already: usb_wakeup(USBEndpoint *ep, int streamid). That will trigger a USBPortOps->wakeup callback in the host adapter emulation. So, the usb-host change should be as simple as this: --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -451,6 +451,7 @@ static void usb_host_req_complete_iso(struct libusb_transfer *transfer) } if (xfer->ring->ep->pid == USB_TOKEN_IN) { QTAILQ_INSERT_TAIL(&xfer->ring->copy, xfer, next); +usb_wakeup(xfer->ring->ep, 0); } else { QTAILQ_INSERT_TAIL(&xfer->ring->unused, xfer, next); } cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [Patch RFC 1/4] usb: support device specification via -
On Do, 2015-07-16 at 17:47 +0200, Juergen Gross wrote: > Today a host usb device can be specified either via : > or via . syntax. Add the possibility to specify it via > - as this is needed for the support of xen pvusb backend. -device usb-host,hostbus=,hostport= should already do what you want. > diff --git a/hw/usb/host-legacy.c b/hw/usb/host-legacy.c > index 3cc9c42..526108c 100644 > --- a/hw/usb/host-legacy.c > +++ b/hw/usb/host-legacy.c I don't think we should extend this. This exists purely for backward compatibility reasons. > +out: > qdev_prop_set_uint32(&dev->qdev, "hostbus", filter.bus_num); > qdev_prop_set_uint32(&dev->qdev, "hostaddr", filter.addr); > +if (filter.port) { > +qdev_prop_set_string(&dev->qdev, "port", filter.port); Hmm? This should have been "hostport", right? cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC][PATCH 1/1] libxl: add one machine property to support IGD GFX passthrough
On Mi, 2015-01-21 at 11:37 +, Ian Jackson wrote: > Tiejun Chen writes ("[RFC][PATCH 1/1] libxl: add one machine property to > support IGD GFX passthrough"): > > When we're working to support IGD GFX passthrough with qemu > > upstream, instead of "-gfx_passthru" we'd like to make that > > a machine option, "-machine xxx,gfx_passthru=on". This need > > to bring several changes on tool side. > > Has the corresponding patch to qemu-upstream been accepted yet ? > > I'd like to see a confirmation from the qemu side that this is going > into their tree and that the command-line option syntax has been > agreed. Suggested at patch review, not merged (guess thats why it is tagged 'rfc', to get both qemu+xen on the same page). While being at it: Should we name this 'igd-passthru' instead of 'gfx-passthru'? The hostbridge / isabridge quirks needed are actually specific to igd and are not needed for -- say -- nvidia gfx cards. cheers, Gerd ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel