Re: Directly reserve an interrupt IDT entry for Hyper-V
On Wednesday, August 20, 2014 9:31:54 am Roger Pau Monné wrote: On 20/08/14 11:19, Wei Hu wrote: Hello, Sending to Xen, drivers and virtualization mailing lists since this might be of interest to the folks on these aliases. I am working for Microsoft to improve the performance of FreeBSD running on Hyper-V. Right now I am adding a feature in the vmbus driver which could handle the host-guest channel communications on all vCPUs simultaneously. In order to achieve this, the hypervisor will send same interrupt concurrently on all the vCPUs. The traditional way on FreeBSD to set up interrupt handling for devicse, such as calling bus_alloc_resource() to reserve an IRQ line, and then calling bus_setup_intr() to create a vector, doesn't seem to work in this case. It seems if the interrupt is routed via legacy IRQ, it can only be active on one vCPU at a time. In order to allow the same interrupt to be handled on all vCPUs concurrently, all I need is an IDT entry, not an IRQ line. I checked current FreeBSD code. It looks to me Xen directly uses the vector number IDT_EVTCHN (0x93) to achieve the same purpose. I am proposing both Xen and Hyper-V share this same vector. Following is a little bit detail of my proposal for the changes in the current kernel. 1. In machdep.c: #ifdef XENHVM setidt(IDT_EVTCHN, IDTVEC(xen_intr_upcall), SDT_SYSIGT, SEL_UPL, 0); #else setidt(IDT_EVTCHN, IDTVEC(hv_vmbus_intr), SDT_SYSIGT, SEL_UPL, 0); #endif 2. Apic_vector.S Add IDTVEC(hv_vmbus_intr) to call Hyper-V vmbus interrupt service routine. Any thoughts, objections and feedbacks are all welcome. Hello, I don't think using the same IDT vector is the right approach, I would just pick a different IDT vector and use that for Hyper-V. Using the same IDT vector (like your suggestion above) would prevent shipping a kernel with with both Hyper-V and Xen support (like it's done now in GENERIC). Roger. Hmm, can't you make this a runtime check to only call setidt() if you detect you are under the appropriate hypervisor? Also, bhyve currently has a hackish way of requesting a free IDT slot. Perhaps it would be best if I added little API to reserve an IDT slot assuming that callers could accept a dynamic IDT vector rather than a static one. -- John Baldwin ___ 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: [PATCH RFC 10/13] xen: add ACPI bus to xen_nexus when running as Dom0
On Friday, February 14, 2014 5:38:15 am Roger Pau Monné wrote: On 08/02/14 22:50, John Baldwin wrote: On Tuesday, December 24, 2013 12:20:59 PM Roger Pau Monne wrote: Also disable a couple of ACPI devices that are not usable under Dom0. Hmm, setting debug.acpi.disabled in this way is a bit hacky. It might be fine however if there's no way for the user to set it before booting the kernel (as opposed to haing the relevant drivers explicitly disable themselves under Xen which I think would be cleaner, but would also make your patch larger) Thanks for the review, the user can pass parameters to FreeBSD when booted as Dom0, I just find it uncomfortable to force the user into always setting something on the command line in order to boot. Can the user set debug.acpi.disabled? If so, you are overriding their setting which would be bad. What do you mean with haing the relevant drivers explicitly disable themselves under Xen? Adding a gate on every one of those devices like if (xen_pv_domain()) return (ENXIO); in the identify/probe routine seems even worse. A check like this in probe() is what I had in mind, though I agree it's not perfect. -- John Baldwin ___ 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: [PATCH RFC 12/13] mca: disable cmc enable on Xen PV
On Tuesday, December 24, 2013 12:21:01 PM Roger Pau Monne wrote: Xen PV guests doesn't have a lapic, so disable the lapic call in mca initialization. I think this is fine, but I wonder if it wouldn't be cleaner to have lapic_enable_cmc() do the check instead. Where else do you check lapic_disabled? --- sys/x86/x86/mca.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/sys/x86/x86/mca.c b/sys/x86/x86/mca.c index f1369cd..e9d2c1d 100644 --- a/sys/x86/x86/mca.c +++ b/sys/x86/x86/mca.c @@ -897,7 +897,7 @@ _mca_init(int boot) } #ifdef DEV_APIC - if (PCPU_GET(cmci_mask) != 0 boot) + if (PCPU_GET(cmci_mask) != 0 boot !lapic_disabled) lapic_enable_cmc(); #endif } -- John Baldwin ___ 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: [PATCH RFC 11/13] pci: introduce a new event on PCI device detection
On Tuesday, December 24, 2013 12:21:00 PM Roger Pau Monne wrote:
Add a new event that will fire each time a PCI device is added to the
system, and allows us to register the device with Xen.
It's really hackish to make this PCI specific. OTOH, I can't think of a
good place to have a more generic new-bus callback. You could make the
eventhandler pass the 'device_t' instead of the dinfo. The dinfo isn't
really a public structure, and since the device_t's ivars are already set
you can use things like 'pci_get_domain()' and 'pci_get_bus()' of the
passed in device in your callback function.
---
sys/dev/pci/pci.c |1 +
sys/sys/eventhandler.h |5 +
sys/x86/xen/pv.c| 21 +
sys/x86/xen/xen_nexus.c |6 ++
sys/xen/pv.h|1 +
5 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c
index 4d8837f..2ee5093 100644
--- a/sys/dev/pci/pci.c
+++ b/sys/dev/pci/pci.c
@@ -3293,6 +3293,7 @@ pci_add_child(device_t bus, struct pci_devinfo *dinfo)
resource_list_init(dinfo-resources);
pci_cfg_save(dinfo-cfg.dev, dinfo, 0);
pci_cfg_restore(dinfo-cfg.dev, dinfo);
+ EVENTHANDLER_INVOKE(pci_add, dinfo);
pci_print_verbose(dinfo);
pci_add_resources(bus, dinfo-cfg.dev, 0, 0);
}
diff --git a/sys/sys/eventhandler.h b/sys/sys/eventhandler.h
index 111c21b..3201848 100644
--- a/sys/sys/eventhandler.h
+++ b/sys/sys/eventhandler.h
@@ -269,5 +269,10 @@ typedef void (*unregister_framebuffer_fn)(void *,
struct fb_info *); EVENTHANDLER_DECLARE(register_framebuffer,
register_framebuffer_fn); EVENTHANDLER_DECLARE(unregister_framebuffer,
unregister_framebuffer_fn);
+/* PCI events */
+struct pci_devinfo;
+typedef void (*pci_add_fn)(void *, struct pci_devinfo *);
+EVENTHANDLER_DECLARE(pci_add, pci_add_fn);
+
#endif /* _SYS_EVENTHANDLER_H_ */
diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
index e5ad200..a44f8ca 100644
--- a/sys/x86/xen/pv.c
+++ b/sys/x86/xen/pv.c
@@ -39,6 +39,9 @@ __FBSDID($FreeBSD$);
#include sys/rwlock.h
#include sys/mutex.h
#include sys/smp.h
+#include sys/reboot.h
+#include sys/pciio.h
+#include sys/eventhandler.h
#include vm/vm.h
#include vm/vm_extern.h
@@ -63,6 +66,8 @@ __FBSDID($FreeBSD$);
#include xen/interface/vcpu.h
+#include dev/pci/pcivar.h
+
/* Native initial function */
extern u_int64_t hammer_time(u_int64_t, u_int64_t);
/* Xen initial function */
@@ -384,6 +389,22 @@ xen_pv_ioapic_register_intr(struct ioapic_intsrc *pin)
xen_register_pirq(pin-io_irq, pin-io_activehi, pin-io_edgetrigger);
}
+void
+xen_pv_pci_device_add(void *arg, struct pci_devinfo *dinfo)
+{
+ struct physdev_pci_device_add add_pci;
+ int error;
+
+ bzero(add_pci, sizeof(add_pci));
+ add_pci.seg = dinfo-cfg.domain;
+ add_pci.bus = dinfo-cfg.bus;
+ add_pci.devfn = (dinfo-cfg.slot 3) | dinfo-cfg.func;
+ error = HYPERVISOR_physdev_op(PHYSDEVOP_pci_device_add, add_pci);
+ if (error)
+ printf(unable to add device bus %u devfn %u error: %d\n,
+add_pci.bus, add_pci.devfn, error);
+}
+
static void
xen_pv_set_init_ops(void)
{
diff --git a/sys/x86/xen/xen_nexus.c b/sys/x86/xen/xen_nexus.c
index 823b3bc..60c6c5d 100644
--- a/sys/x86/xen/xen_nexus.c
+++ b/sys/x86/xen/xen_nexus.c
@@ -34,6 +34,7 @@ __FBSDID($FreeBSD$);
#include sys/sysctl.h
#include sys/systm.h
#include sys/smp.h
+#include sys/eventhandler.h
#include contrib/dev/acpica/include/acpi.h
@@ -42,6 +43,7 @@ __FBSDID($FreeBSD$);
#include machine/nexusvar.h
#include xen/xen-os.h
+#include xen/pv.h
static const char *xen_devices[] =
{
@@ -87,6 +89,10 @@ nexus_xen_attach(device_t dev)
/* Disable some ACPI devices that are not usable by Dom0 */
setenv(debug.acpi.disabled, cpu hpet timer);
+ /* Register PCI add hook */
+ EVENTHANDLER_REGISTER(pci_add, xen_pv_pci_device_add, NULL,
+ EVENTHANDLER_PRI_FIRST);
+
acpi_dev = BUS_ADD_CHILD(dev, 10, acpi, 0);
if (acpi_dev == NULL)
panic(Unable to add ACPI bus to Xen Dom0);
diff --git a/sys/xen/pv.h b/sys/xen/pv.h
index a9d6eb0..ac737a7 100644
--- a/sys/xen/pv.h
+++ b/sys/xen/pv.h
@@ -24,5 +24,6 @@
#define __XEN_PV_H__
int xen_pv_start_all_aps(void);
+void xen_pv_pci_device_add(void *, struct pci_devinfo *);
#endif /* __XEN_PV_H__ */
--
John Baldwin
___
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: [PATCH RFC 07/13] xen: implement IO APIC support in Xen mptable parser
On Tuesday, December 24, 2013 12:20:56 PM Roger Pau Monne wrote: Use madt_setup_io (from madt.c) on Xen apic_enumerator, in order to parse the interrupt sources from the IO APIC. I would like to get opinions, but I think we should rename and move madt_setup_io to io_apic.c. It wouldn't be appropriate for io_apic.c as it isn't generic to I/O APICs but is specific to ACPI. However, mptable.c is really not a great name for this file in sys/x86/xen. I wonder if it should be xen_apic.c instead? Also, if Xen PV has an MADT table, why do you need a custom APIC enumerator at all? That is, what is preventing the code in madt.c from just working? Do you just not have 'device acpi' in the kernel config you are using? -- John Baldwin ___ 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: [PATCH RFC 04/13] xen: implement basic PIRQ support for Dom0
On Tuesday, December 24, 2013 6:20:53 am Roger Pau Monne wrote: This allows Dom0 to manage physical hardware, redirecting the physical interrupts to event channels. --- sys/x86/xen/xen_intr.c | 190 +-- sys/xen/xen_intr.h | 11 +++ 2 files changed, 192 insertions(+), 9 deletions(-) diff --git a/sys/x86/xen/xen_intr.c b/sys/x86/xen/xen_intr.c index bc0781e..340e5ed 100644 --- a/sys/x86/xen/xen_intr.c +++ b/sys/x86/xen/xen_intr.c @@ -104,6 +104,8 @@ DPCPU_DECLARE(struct vcpu_info *, vcpu_info); #define is_valid_evtchn(x) ((x) != 0) +#define EEXIST 17 /* Xen already exists error */ Is there a xen_errno.h header? Might be nice to have one and give these constants unique names (e.g. XEN_EEXIST or some such) to avoid confusion/conflicts with sys/errno.h. Other than that I think this looks fine. -- John Baldwin ___ 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: [PATCH RFC 02/13] ioapic: introduce hooks for some ioapic ops
On Tuesday, December 24, 2013 6:20:51 am Roger Pau Monne wrote: Create some hooks for IO APIC operations that will diverge from bare metal when implemented for Xen Dom0. This patch should not introduce any changes in functionality, it's a preparatory patch for the implementation of the Xen IO APIC hooks. I think this is fine. I should really create a sys/x86/include/apicvar.h as there is a lot shared between those two headers. -- John Baldwin ___ 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: [PATCH RFC 06/13] xen: Dom0 console fixes
On Tuesday, December 24, 2013 6:20:55 am Roger Pau Monne wrote: Minor fixes and workarounds to make the Xen Dom0 console work. Looks ok to me. -- John Baldwin ___ 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: [PATCH RFC 05/13] xen: implement Xen IO APIC ops
On Tuesday, December 24, 2013 6:20:54 am Roger Pau Monne wrote:
Implement a different set of hooks for IO APIC to use when running
under Xen Dom0.
---
sys/x86/xen/pv.c | 44
1 files changed, 44 insertions(+), 0 deletions(-)
diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
index ab4afba..e5ad200 100644
--- a/sys/x86/xen/pv.c
+++ b/sys/x86/xen/pv.c
@@ -49,6 +49,7 @@ __FBSDID($FreeBSD$);
#include vm/vm_pager.h
#include vm/vm_param.h
+#include x86/apicreg.h
#include machine/sysarch.h
#include machine/clock.h
#include machine/pc/bios.h
@@ -58,6 +59,7 @@ __FBSDID($FreeBSD$);
#include xen/xen-os.h
#include xen/hypervisor.h
#include xen/pv.h
+#include xen/xen_intr.h
#include xen/interface/vcpu.h
@@ -73,6 +75,11 @@ static caddr_t xen_pv_parse_preload_data(u_int64_t);
static void xen_pv_parse_memmap(caddr_t, vm_paddr_t *, int *);
static void xen_pv_set_init_ops(void);
+
+static u_int xen_pv_ioapic_read(volatile ioapic_t *, int);
+static void xen_pv_ioapic_write(volatile ioapic_t *, int, u_int);
+static void xen_pv_ioapic_register_intr(struct ioapic_intsrc *);
+
/* Extern Declarations
---*/
/* Variables used by amd64 mp_machdep to start APs */
extern struct mtx ap_boot_mtx;
@@ -92,6 +99,13 @@ struct init_ops xen_init_ops = {
.parse_memmap = xen_pv_parse_memmap,
};
+/* Xen ioapic_ops implementation */
+struct ioapic_ops xen_ioapic_ops = {
+ .read = xen_pv_ioapic_read,
+ .write =xen_pv_ioapic_write,
+ .register_intr =xen_pv_ioapic_register_intr,
+};
+
static struct
{
const char *ev;
@@ -342,6 +356,34 @@ xen_pv_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap,
int *physmap_idx)
bios_add_smap_entries(xen_smap, size, physmap, physmap_idx);
}
+static u_int
+xen_pv_ioapic_read(volatile ioapic_t *apic, int reg)
+{
+ struct physdev_apic apic_op;
+ int rc;
+
+ mtx_assert(icu_lock, MA_OWNED);
+
+ apic_op.apic_physbase = pmap_kextract((vm_offset_t) apic);
Seems a shame to have to do this. I wouldn't mind if you changed the
read/write callbacks to take 'struct ioapic *' instead and then use the
'io_paddr' member. I do think that would be cleaner.
+ apic_op.reg = reg;
+ rc = HYPERVISOR_physdev_op(PHYSDEVOP_apic_read, apic_op);
+ if (rc)
+ panic(apic_read operation failed);
+
+ return (apic_op.value);
+}
+
+static void
+xen_pv_ioapic_write(volatile ioapic_t *apic, int reg, u_int val)
+{
+}
I guess not allowing writes is on purpose?
+
+static void
+xen_pv_ioapic_register_intr(struct ioapic_intsrc *pin)
+{
+ xen_register_pirq(pin-io_irq, pin-io_activehi, pin-io_edgetrigger);
+}
+
static void
xen_pv_set_init_ops(void)
{
@@ -349,4 +391,6 @@ xen_pv_set_init_ops(void)
init_ops = xen_init_ops;
/* Disable lapic */
lapic_disabled = true;
+ /* IOAPIC ops for Xen PV */
+ ioapic_ops = xen_ioapic_ops;
}
--
1.7.7.5 (Apple Git-26)
--
John Baldwin
___
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: [PATCH RFC 01/13] xen: use the hardware e820 map on Dom0
On Tuesday, December 24, 2013 6:20:50 am Roger Pau Monne wrote:
We need to do some tweaking of the hardware e820 map, since the memory
layout provided by Xen and the e820 map doesn't match.
This consists in clamping the e820 map so that regions above max_pfn
are marked as unusuable.
---
sys/x86/xen/pv.c | 35 +--
1 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
index 4f7415e..ab4afba 100644
--- a/sys/x86/xen/pv.c
+++ b/sys/x86/xen/pv.c
@@ -297,17 +297,48 @@ static void
xen_pv_parse_memmap(caddr_t kmdp, vm_paddr_t *physmap, int *physmap_idx)
{
struct xen_memory_map memmap;
+ unsigned long max_pfn = HYPERVISOR_start_info-nr_pages;
+ u_int64_t mem_end = ptoa(max_pfn);
u_int32_t size;
- int rc;
+ int rc, mem_op, i;
One minor nit is that it is preferred to not initalize variables in
declarations style-wise. Aside from that, this looks fine to me.
/* Fetch the E820 map from Xen */
memmap.nr_entries = MAX_E820_ENTRIES;
set_xen_guest_handle(memmap.buffer, xen_smap);
- rc = HYPERVISOR_memory_op(XENMEM_memory_map, memmap);
+ mem_op = xen_initial_domain() ?
+ XENMEM_machine_memory_map :
+ XENMEM_memory_map;
+ rc = HYPERVISOR_memory_op(mem_op, memmap);
if (rc)
panic(unable to fetch Xen E820 memory map);
size = memmap.nr_entries * sizeof(xen_smap[0]);
+ /*
+ * This should be improved, Xen provides us with a single
+ * chunk of physical memory that goes from 0 to max_pfn,
+ * and what we do here is clamp the HW memory map to make
+ * sure regions above max_pfn are marked as reserved.
+ *
+ * TRTTD would be to move the memory not used because of
+ * the holes in the HW memory map to the now clamped regions
+ * using XENMEM_{decrease/increase}_reservation.
+ */
+ for (i = 0; i memmap.nr_entries; i++) {
+ u_int64_t end = xen_smap[i].base + xen_smap[i].length;
+ if (xen_smap[i].type == SMAP_TYPE_MEMORY) {
+ if (xen_smap[i].base mem_end) {
+ /* Mark as reserved */
+ xen_smap[i].type = SMAP_TYPE_RESERVED;
+ continue;
+ }
+ if (end mem_end) {
+ /* Truncate region */
+ xen_smap[i].length -= end - mem_end;
+ }
+ }
+ }
+
+
bios_add_smap_entries(xen_smap, size, physmap, physmap_idx);
}
--
1.7.7.5 (Apple Git-26)
--
John Baldwin
___
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: [PATCH v7 03/19] xen: add and enable Xen console for PVH guests
On Thursday, December 19, 2013 1:54:40 pm Roger Pau Monne wrote:
diff --git a/sys/dev/xen/console/console.c b/sys/dev/xen/console/console.c
index 23eaee2..e8079da 100644
--- a/sys/dev/xen/console/console.c
+++ b/sys/dev/xen/console/console.c
@@ -69,11 +69,14 @@ struct mtx cn_mtx;
static char wbuf[WBUF_SIZE];
static char rbuf[RBUF_SIZE];
static int rc, rp;
-static unsigned int cnsl_evt_reg;
+unsigned int cnsl_evt_reg;
static unsigned int wc, wp; /* write_cons, write_prod */
xen_intr_handle_t xen_intr_handle;
device_t xencons_dev;
+/* Virt address of the shared console page */
Tiny nit: I would expand Virt to Virtual
+char *console_page;
+
#ifdef KDB
static int xc_altbrk;
#endif
@@ -110,9 +113,28 @@ static struct ttydevsw xc_ttydevsw = {
.tsw_outwakeup = xcoutwakeup,
};
+/*- Debug function
---*/
+#define XC_PRINTF_BUFSIZE 1024
+void
+xc_printf(const char *fmt, ...)
+{
+__va_list ap;
+int retval;
+static char buf[XC_PRINTF_BUFSIZE];
+
+va_start(ap, fmt);
+retval = vsnprintf(buf, XC_PRINTF_BUFSIZE - 1, fmt, ap);
+va_end(ap);
+buf[retval] = 0;
+HYPERVISOR_console_write(buf, retval);
+}
+
vsnprintf() always nul-terminates, so you can simplify this slightly to:
retval = vsnprintf(buf, sizeof(buf), fmt, ap);
OTOH, you can't actually use 'retval' from vsnprintf as it returns the
number of characters that would have been output if the buffer were
infinitely long, not the number of characters output into the string.
From printf(3):
These functions return the number of characters printed (not including
the trailing `\0' used to end output to strings) or a negative value if
an output error occurs, except for snprintf() and vsnprintf(), which
return the number of characters that would have been printed if the size
were unlimited (again, not including the final `\0').
So I think what you actually want is this:
void
xc_printf(const char *fmt, ...)
{
static char buf[XC_PRINTF_BUFSIZE];
__va_list ap;
va_start(ap, fmt);
vsnprintf(buf, sizeof(buf), fmt, ap);
va_end(ap);
HYPERVISOR_console_write(buf, strlen(buf));
}
(And I realize this is an old bug, you were just moving an existing function)
-static void
-xc_identify(driver_t *driver, device_t parent)
-{
- device_t child;
- child = BUS_ADD_CHILD(parent, 0, driver_name, 0);
- device_set_driver(child, driver);
- device_set_desc(child, Xen Console);
-}
-
Hmm, how does the device get created without an identify routine?
--
John Baldwin
___
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: [PATCH v7 02/19] xen: add macro to detect if running as Dom0
On Thursday, December 19, 2013 1:54:39 pm Roger Pau Monne wrote:
---
sys/xen/xen-os.h |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/sys/xen/xen-os.h b/sys/xen/xen-os.h
index c7474d8..e8a5a99 100644
--- a/sys/xen/xen-os.h
+++ b/sys/xen/xen-os.h
@@ -82,6 +82,13 @@ xen_hvm_domain(void)
return (xen_domain_type == XEN_HVM_DOMAIN);
}
+static inline int
+xen_initial_domain(void)
+{
+ return (xen_domain() HYPERVISOR_start_info
+ HYPERVISOR_start_info-flags SIF_INITDOMAIN);
+}
+
#ifndef xen_mb
#define xen_mb() mb()
#endif
This looks fine to me.
--
John Baldwin
___
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: [PATCH v7 13/19] xen: introduce flag to disable the local apic
On Thursday, December 19, 2013 1:54:50 pm Roger Pau Monne wrote: PVH guests don't have an emulated lapic. Since the tests all use '!lapic_disabled' I think I would prefer to flip the name to 'lapic_valid' and have it default to true. It would just be a cosmetic change to the current patch, but I think it is more readable long- term. -- John Baldwin ___ 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: [PATCH v7 18/19] xen: changes to gnttab for PVH
On Thursday, December 19, 2013 1:54:55 pm Roger Pau Monne wrote: --- sys/xen/gnttab.c | 26 +- 1 files changed, 21 insertions(+), 5 deletions(-) I can't really speak to the correctness of this as I don't know the Xen internals well enough, but I don't see anything that looks wrong. :) -- John Baldwin ___ 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: [PATCH v7 15/19] xen: create a Xen nexus to use in PV/PVH
On Thursday, December 19, 2013 1:54:52 pm Roger Pau Monne wrote: Introduce a Xen specific nexus that is going to be in charge for attaching Xen specific devices. Remove the identify routine from Xen devices and instead attach them from the nexus (PV/PVH) or xenpci (HVM). As with my previous mail, I would encourage you to create a xenpv0 device and have the xen nexus be very minimal. This would let you leave the existing identify routines in place (which is cleaner / more extensible than a static table of devices that you explicitly add), but by having the PV-specific drivers attach as children of xenpv0, their identify routines would only be invoked when xenpv0 attached. You can even use an identify routine for CPUs (see cpu_identify in sys/x86/x86/legacy.c). -- John Baldwin ___ 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: [PATCH v7 06/19] xen: implement an early timer for Xen PVH
On Thursday, December 19, 2013 1:54:43 pm Roger Pau Monne wrote:
When running as a PVH guest, there's no emulated i8254, so we need to
use the Xen PV timer as the early source for DELAY. This change allows
for different implementations of the early DELAY function and
implements a Xen variant for it.
This mostly looks good to me. I would perhaps move DELAY() itself into
delay.c if it isn't too ugly to do so. I guess it would look something
like:
#if !(defined(__i386__) defined(XEN))
void
DELAY(int n)
{
if (delay_tc(n))
return;
#ifdef __amd64__
init_ops.early_delay(n);
#else
i8254_delay(n);
}
#endif
This would let you leave delay_tc() private to delay.c.
--
John Baldwin
___
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: [PATCH v7 11/19] xen: changes to hvm code in order to support PVH guests
On Thursday, December 19, 2013 1:54:48 pm Roger Pau Monne wrote: On PVH we don't need to init the shared info page, or disable emulated devices. Also, make sure PV IPIs are set before starting the APs. Looks ok to me. -- John Baldwin ___ 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: [PATCH v7 08/19] xen: use the same hypercall mechanism for XEN and XENHVM
On Thursday, December 19, 2013 1:54:45 pm Roger Pau Monne wrote: --- sys/amd64/include/xen/hypercall.h |7 --- sys/i386/i386/locore.s|9 + sys/i386/include/xen/hypercall.h |8 sys/x86/xen/hvm.c | 24 ++-- 4 files changed, 19 insertions(+), 29 deletions(-) Looks good to me. -- John Baldwin ___ 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: [PATCH v7 16/19] xen: add shutdown hook for PVH
On Thursday, December 19, 2013 1:54:53 pm Roger Pau Monne wrote: Add the PV shutdown hook to PVH. --- sys/dev/xen/control/control.c | 37 ++--- 1 files changed, 18 insertions(+), 19 deletions(-) This looks fine to me. -- John Baldwin ___ 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: [PATCH v7 17/19] xen: xenstore changes to support PVH
On Thursday, December 19, 2013 1:54:54 pm Roger Pau Monne wrote:
---
sys/xen/xenstore/xenstore.c | 21 ++---
1 files changed, 10 insertions(+), 11 deletions(-)
This looks fine modulo possibly making it a xenpv child instead of a nexus
child.
Note that even in the old XEN code in FreeBSD I would argue that attaching
directly to nexus is wrong and that even the old XEN code should create
some sort of top-level device below nexus0 that is XEN-specific for XEN-
specific drivers to attach to. I've no idea what shape the old XEN-specific
code is in or if it can be tested, but I'd really like PVH to do the right
thing if at all possible.
diff --git a/sys/xen/xenstore/xenstore.c b/sys/xen/xenstore/xenstore.c
index bcf6357..2893c84 100644
--- a/sys/xen/xenstore/xenstore.c
+++ b/sys/xen/xenstore/xenstore.c
@@ -229,13 +229,11 @@ struct xs_softc {
*/
struct sx xenwatch_mutex;
-#ifdef XENHVM
/**
* The HVM guest pseudo-physical frame number. This is Xen's mapping
* of the true machine frame number into our physical address space.
*/
unsigned long gpfn;
-#endif
/**
* The event channel for communicating with the
@@ -1141,13 +1139,15 @@ xs_attach(device_t dev)
/* Initialize the interface to xenstore. */
struct proc *p;
-#ifdef XENHVM
- xs.evtchn = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN);
- xs.gpfn = hvm_get_parameter(HVM_PARAM_STORE_PFN);
- xen_store = pmap_mapdev(xs.gpfn * PAGE_SIZE, PAGE_SIZE);
-#else
- xs.evtchn = xen_start_info-store_evtchn;
-#endif
+ if (xen_hvm_domain()) {
+ xs.evtchn = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN);
+ xs.gpfn = hvm_get_parameter(HVM_PARAM_STORE_PFN);
+ xen_store = pmap_mapdev(xs.gpfn * PAGE_SIZE, PAGE_SIZE);
+ } else if (xen_pv_domain()) {
+ xs.evtchn = HYPERVISOR_start_info-store_evtchn;
+ } else {
+ panic(Unknown domain type, cannot initialize xenstore\n);
+ }
TAILQ_INIT(xs.reply_list);
TAILQ_INIT(xs.watch_events);
@@ -1256,9 +1256,8 @@ static devclass_t xenstore_devclass;
#ifdef XENHVM
DRIVER_MODULE(xenstore, xenpci, xenstore_driver, xenstore_devclass, 0, 0);
-#else
-DRIVER_MODULE(xenstore, nexus, xenstore_driver, xenstore_devclass, 0, 0);
#endif
+DRIVER_MODULE(xenstore, nexus, xenstore_driver, xenstore_devclass, 0, 0);
/*--- Sysctl Data
*/
/* XXX Shouldn't the node be somewhere else? */
--
1.7.7.5 (Apple Git-26)
--
John Baldwin
___
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: [PATCH v7 12/19] xen: add a hook to perform AP startup
On Thursday, December 19, 2013 1:54:49 pm Roger Pau Monne wrote: AP startup on PVH follows the PV method, so we need to add a hook in order to diverge from bare metal. --- +int native_start_all_aps(void); + Please put this in a header instead of using an extern in the Xen PV code. machine/smp.h is probably the best header to use. -- John Baldwin ___ 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: [PATCH v7 04/19] amd64: introduce hook for custom preload metadata parsers
On Tuesday, December 24, 2013 11:47:53 am Roger Pau Monné wrote: On 24/12/13 16:47, John Baldwin wrote: On Thursday, December 19, 2013 1:54:41 pm Roger Pau Monne wrote: --- /dev/null +++ b/sys/xen/pv.h @@ -0,0 +1,28 @@ +/* + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. One non-technical question. This license statement doesn't actually say who the copyright belongs to which seems problematic. Would it be possible to use FreeBSD's preferred 2-clause BSD license on this file? I copied the license from sys/xen/hvm.h without realizing it was not the standard 2-clause BSD one, but I don't have any problem changing it (maybe we should also change hvm.h?) to match the license in pv.c. The license in pv.c is perfect. For hvm.h, if that could be changed to the 2-clause BSD license, that would also be great. I'll leave that up you to and Justin to determine if that change is ok to make. -- John Baldwin ___ 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
