Re: [Xen-devel] [PATCH v9 14/19] xen: introduce xenpv bus and a dummy pvcpu device
On 06/01/14 12:28, Julien Grall wrote: > > > On 01/06/2014 09:46 AM, Roger Pau Monné wrote: >> On 05/01/14 22:52, Julien Grall wrote: >>> >>> >>> On 01/02/2014 03:43 PM, Roger Pau Monne wrote: Since Xen PVH guests doesn't have ACPI, we need to create a dummy bus so top level Xen devices can attach to it (instead of attaching directly to the nexus) and a pvcpu device that will be used to fill the pcpu->pc_device field. --- sys/conf/files.amd64 |1 + sys/conf/files.i386 |1 + sys/x86/xen/xenpv.c | 155 ++ >>> >>> I think it makes more sense to have 2 files: one for xenpv bus and one >>> for a dummy pvcpu device. It would allow us to move xenpv bus to common >>> code (sys/xen or sys/dev/xen). >> >> Ack. I wasn't thinking other arches will probably use the xenpv bus but >> not the dummy cpu device. Would you agree to leave xenpv bus inside >> x86/xen for now and move the dummy PV cpu device to dev/xen/pvcpu/? > > As we will attach every xen device to xenpv, it makes more sense to have > xenpv bus used on ARM. It will avoid duplication code and keep it nicer. > > I'm fine with this solution for now. I will update/move the code when I > will send the patch series to support FreeBSD on Xen on ARM. > >>> >>> [..] >>> + +static int +xenpv_probe(device_t dev) +{ + +device_set_desc(dev, "Xen PV bus"); +device_quiet(dev); +return (0); >>> >>> As I understand, 0 means I can "handle" the current device, in this case >>> if a device is probing, because it doesn't have yet a driver, we will >>> use xenpv and end up with 2 (or even more) xenpv buses. >>> >>> As we only want to probe xenpv bus once, when the bus was added >>> manually, returning BUS_PROBE_NO_WILDCARD would suit better. >>> >>> [..] >>> +static int +xenpvcpu_probe(device_t dev) +{ + +device_set_desc(dev, "Xen PV CPU"); +return (0); >>> >>> Same here: BUS_PROBE_NOWILDCARD. >> >> Ack for both, will change it to BUS_PROBE_NOWILDCARD. While at it, we >> should also change xenstore probe function to return >> BUS_PROBE_NOWILDCARD. >> > > Right, I have a patch for xenstore. Do you want me to send it? Sure, send it! ___ freebsd-xen@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-xen To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: [Xen-devel] [PATCH v9 14/19] xen: introduce xenpv bus and a dummy pvcpu device
On 01/06/2014 09:46 AM, Roger Pau Monné wrote: On 05/01/14 22:52, Julien Grall wrote: On 01/02/2014 03:43 PM, Roger Pau Monne wrote: Since Xen PVH guests doesn't have ACPI, we need to create a dummy bus so top level Xen devices can attach to it (instead of attaching directly to the nexus) and a pvcpu device that will be used to fill the pcpu->pc_device field. --- sys/conf/files.amd64 |1 + sys/conf/files.i386 |1 + sys/x86/xen/xenpv.c | 155 ++ I think it makes more sense to have 2 files: one for xenpv bus and one for a dummy pvcpu device. It would allow us to move xenpv bus to common code (sys/xen or sys/dev/xen). Ack. I wasn't thinking other arches will probably use the xenpv bus but not the dummy cpu device. Would you agree to leave xenpv bus inside x86/xen for now and move the dummy PV cpu device to dev/xen/pvcpu/? As we will attach every xen device to xenpv, it makes more sense to have xenpv bus used on ARM. It will avoid duplication code and keep it nicer. I'm fine with this solution for now. I will update/move the code when I will send the patch series to support FreeBSD on Xen on ARM. [..] + +static int +xenpv_probe(device_t dev) +{ + +device_set_desc(dev, "Xen PV bus"); +device_quiet(dev); +return (0); As I understand, 0 means I can "handle" the current device, in this case if a device is probing, because it doesn't have yet a driver, we will use xenpv and end up with 2 (or even more) xenpv buses. As we only want to probe xenpv bus once, when the bus was added manually, returning BUS_PROBE_NO_WILDCARD would suit better. [..] +static int +xenpvcpu_probe(device_t dev) +{ + +device_set_desc(dev, "Xen PV CPU"); +return (0); Same here: BUS_PROBE_NOWILDCARD. Ack for both, will change it to BUS_PROBE_NOWILDCARD. While at it, we should also change xenstore probe function to return BUS_PROBE_NOWILDCARD. Right, I have a patch for xenstore. Do you want me to send it? -- Julien Grall ___ freebsd-xen@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-xen To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: [Xen-devel] [PATCH v9 14/19] xen: introduce xenpv bus and a dummy pvcpu device
On 05/01/14 22:52, Julien Grall wrote: > > > On 01/02/2014 03:43 PM, Roger Pau Monne wrote: >> Since Xen PVH guests doesn't have ACPI, we need to create a dummy >> bus so top level Xen devices can attach to it (instead of >> attaching directly to the nexus) and a pvcpu device that will be used >> to fill the pcpu->pc_device field. >> --- >> sys/conf/files.amd64 |1 + >> sys/conf/files.i386 |1 + >> sys/x86/xen/xenpv.c | 155 >> ++ > > I think it makes more sense to have 2 files: one for xenpv bus and one > for a dummy pvcpu device. It would allow us to move xenpv bus to common > code (sys/xen or sys/dev/xen). Ack. I wasn't thinking other arches will probably use the xenpv bus but not the dummy cpu device. Would you agree to leave xenpv bus inside x86/xen for now and move the dummy PV cpu device to dev/xen/pvcpu/? > > [..] > >> + >> +static int >> +xenpv_probe(device_t dev) >> +{ >> + >> +device_set_desc(dev, "Xen PV bus"); >> +device_quiet(dev); >> +return (0); > > As I understand, 0 means I can "handle" the current device, in this case > if a device is probing, because it doesn't have yet a driver, we will > use xenpv and end up with 2 (or even more) xenpv buses. > > As we only want to probe xenpv bus once, when the bus was added > manually, returning BUS_PROBE_NO_WILDCARD would suit better. > > [..] > >> +static int >> +xenpvcpu_probe(device_t dev) >> +{ >> + >> +device_set_desc(dev, "Xen PV CPU"); >> +return (0); > > Same here: BUS_PROBE_NOWILDCARD. Ack for both, will change it to BUS_PROBE_NOWILDCARD. While at it, we should also change xenstore probe function to return BUS_PROBE_NOWILDCARD. ___ freebsd-xen@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-xen To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
Re: [Xen-devel] [PATCH v9 14/19] xen: introduce xenpv bus and a dummy pvcpu device
On 01/02/2014 03:43 PM, Roger Pau Monne wrote: Since Xen PVH guests doesn't have ACPI, we need to create a dummy bus so top level Xen devices can attach to it (instead of attaching directly to the nexus) and a pvcpu device that will be used to fill the pcpu->pc_device field. --- sys/conf/files.amd64 |1 + sys/conf/files.i386 |1 + sys/x86/xen/xenpv.c | 155 ++ I think it makes more sense to have 2 files: one for xenpv bus and one for a dummy pvcpu device. It would allow us to move xenpv bus to common code (sys/xen or sys/dev/xen). [..] + +static int +xenpv_probe(device_t dev) +{ + + device_set_desc(dev, "Xen PV bus"); + device_quiet(dev); + return (0); As I understand, 0 means I can "handle" the current device, in this case if a device is probing, because it doesn't have yet a driver, we will use xenpv and end up with 2 (or even more) xenpv buses. As we only want to probe xenpv bus once, when the bus was added manually, returning BUS_PROBE_NO_WILDCARD would suit better. [..] +static int +xenpvcpu_probe(device_t dev) +{ + + device_set_desc(dev, "Xen PV CPU"); + return (0); Same here: BUS_PROBE_NOWILDCARD. -- Julien Grall ___ freebsd-xen@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-xen To unsubscribe, send any mail to "freebsd-xen-unsubscr...@freebsd.org"
[PATCH v9 14/19] xen: introduce xenpv bus and a dummy pvcpu device
Since Xen PVH guests doesn't have ACPI, we need to create a dummy bus so top level Xen devices can attach to it (instead of attaching directly to the nexus) and a pvcpu device that will be used to fill the pcpu->pc_device field. --- sys/conf/files.amd64 |1 + sys/conf/files.i386 |1 + sys/x86/xen/xenpv.c | 155 ++ 3 files changed, 157 insertions(+), 0 deletions(-) create mode 100644 sys/x86/xen/xenpv.c diff --git a/sys/conf/files.amd64 b/sys/conf/files.amd64 index a3491da..d7c98cc 100644 --- a/sys/conf/files.amd64 +++ b/sys/conf/files.amd64 @@ -570,3 +570,4 @@ x86/xen/hvm.c optionalxenhvm x86/xen/xen_intr.c optionalxen | xenhvm x86/xen/pv.c optionalxenhvm x86/xen/pvcpu_enum.c optionalxenhvm +x86/xen/xenpv.coptionalxenhvm diff --git a/sys/conf/files.i386 b/sys/conf/files.i386 index 790296d..81142e3 100644 --- a/sys/conf/files.i386 +++ b/sys/conf/files.i386 @@ -603,3 +603,4 @@ x86/x86/tsc.c standard x86/x86/delay.cstandard x86/xen/hvm.c optional xenhvm x86/xen/xen_intr.c optional xen | xenhvm +x86/xen/xenpv.coptional xen | xenhvm diff --git a/sys/x86/xen/xenpv.c b/sys/x86/xen/xenpv.c new file mode 100644 index 000..41d674f --- /dev/null +++ b/sys/x86/xen/xenpv.c @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2013 Roger Pau Monné + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include +__FBSDID("$FreeBSD$"); + +#include +#include +#include +#include +#include +#include +#include + +#include + +static int +xenpv_probe(device_t dev) +{ + + device_set_desc(dev, "Xen PV bus"); + device_quiet(dev); + return (0); +} + +static int +xenpv_attach(device_t dev) +{ + device_t child; + + /* +* Let our child drivers identify any child devices that they +* can find. Once that is done attach any devices that we +* found. +*/ + bus_generic_probe(dev); + bus_generic_attach(dev); + + if (!devclass_get_device(devclass_find("isa"), 0)) { + child = BUS_ADD_CHILD(dev, 0, "isa", 0); + if (child == NULL) + panic("xenpv_attach isa"); + device_probe_and_attach(child); + } + + return 0; +} + +static device_method_t xenpv_methods[] = { + /* Device interface */ + DEVMETHOD(device_probe, xenpv_probe), + DEVMETHOD(device_attach,xenpv_attach), + DEVMETHOD(device_suspend, bus_generic_suspend), + DEVMETHOD(device_resume,bus_generic_resume), + + /* Bus interface */ + DEVMETHOD(bus_add_child,bus_generic_add_child), + + DEVMETHOD_END +}; + +static driver_t xenpv_driver = { + "xenpv", + xenpv_methods, + 1, /* no softc */ +}; +static devclass_t xenpv_devclass; + +DRIVER_MODULE(xenpv, nexus, xenpv_driver, xenpv_devclass, 0, 0); + +/* + * Dummy Xen cpu device + * + * Since there's no ACPI on PVH guests, we need to create a dummy + * CPU device in order to fill the pcpu->pc_device field. + */ + +static void +xenpvcpu_identify(driver_t *driver, device_t parent) +{ + device_t child; + int i; + + /* Only attach to PV guests, HVM guests use the ACPI CPU devices */ + if (!xen_pv_domain()) + return; + + CPU_FOREACH(i) { + child = BUS_ADD_CHILD(parent, 0, "pvcpu", i); + if (child == NULL) + panic("xenpvcpu_identify add pvcpu");