Re: [Xen-devel] [PATCH v9 3/3] VT-d: Fix vt-d Device-TLB flush timeout issue
On April 05, 2016 5:48pm, Jan Beulich wrote:
> >>> On 01.04.16 at 16:47, wrote:
> > +static void dev_invalidate_iotlb_timeout(struct iommu *iommu, u16 did,
> > + u16 seg, u8 bus, u8 devfn)
> {
> > +struct domain *d = NULL;
> > +struct pci_dev *pdev;
> > +
> > +if ( test_bit(did, iommu->domid_bitmap) )
> > +d = rcu_lock_domain_by_id(iommu->domid_map[did]);
> > +
> > +if ( d == NULL )
> > +return;
>
> This should be accompanied by a comment explaining why returning here
> without indicating any error is okay.
>
I'd say:
"The domain has been freed, and the device no longer belongs to this domain."
> > +{
> > +ASSERT(pdev->domain);
> > +list_del(&pdev->domain_list);
> > +pdev->domain = NULL;
> > +pci_hide_existing_device(pdev);
> > +break;
> > +}
> > +}
> > +
> > +pcidevs_unlock();
> > +
> > +if ( !is_hardware_domain(d) )
> > +domain_crash(d);
>
> else printk();
>
As I mentioned in commit log "If impacted domain is hardware domain, just throw
out a warning (done in queue_invalidate_wait).",
I am not sure whether we really need a printk() at this point or not.
> > @@ -251,11 +303,13 @@ int qinval_device_iotlb(struct iommu *iommu, }
> >
> > int qinval_device_iotlb_sync(struct iommu *iommu,
> > -u32 max_invs_pend, u16 sid, u16 size, u64 addr)
> > +u32 max_invs_pend, u16 did, u16 seg, u8 bus, u8 devfn, u16 size,
> > + u64 addr)
> > {
> > +u16 sid = PCI_BDF2(bus, devfn);
>
> Pointless local variable (but being a matter of taste, the VT-d maintainers
> will
> have the final say).
>
I will drop this local variable.
> > --- a/xen/include/xen/pci.h
> > +++ b/xen/include/xen/pci.h
> > @@ -116,6 +116,7 @@ const unsigned long *pci_get_ro_map(u16 seg); int
> > pci_add_device(u16 seg, u8 bus, u8 devfn,
> > const struct pci_dev_info *, nodeid_t node); int
> > pci_remove_device(u16 seg, u8 bus, u8 devfn);
> > +void pci_hide_existing_device(struct pci_dev *pdev);
> > int pci_ro_device(int seg, int bus, int devfn); int
> > pci_hide_device(int bus, int devfn);
>
> Please move the addition here.
>
Agreed.
Quan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] R: Xen crash in cpupool_assign_cpu_locked spinlock
Thanks for reporting, and thanks Andrew for Cc-ing me. I saw what the issue is already, I'll patch it first thing Monday morning. I did some last minute refactoring during which I wrongly replaced one pcpu_schedule_lock_irq(), with just pcpu_schedule_lock(), and did the last round of testing with 'debug=n'... sorry. Regards, Dario (Sorry also for top posting, and in general, the formatting of this email) Inviato da Samsung Mobile Messaggio originale Da: Andrew Cooper Data:09/04/2016 20:36 (GMT+01:00) A: Aaron Cornelius ,xen-de...@lists.xenproject.org,Dario Faggioli Oggetto: Re: [Xen-devel] Xen crash in cpupool_assign_cpu_locked spinlock On 09/04/16 19:33, Aaron Cornelius wrote: > I am not really sure where bugs on the staging branch should be reported. I > just found this one and couldn't find anywhere that it had been reported yet. This is the correct place. CC'ing Dario who is the author of the recent scheduler changes in staging. ~Andrew > > I have a Xen/Ubuntu 14.04 VM (in virtualbox) with 2 CPUs allocated to it, when > it boots I remove a CPU from the default pool (because it's allocated to a new > credit schedule in the new cpuppol config file), and when I create the cpupool > Xen crashes. > > I just pulled the staging branch from git://xenbits.xen.org/xen.git a few > hours > ago. I ran this with a fresh pull of master yesterday and I did not > experience > this issue. > > Here are the commands: > $ sudo xl cpupool-cpu-remove Pool-0 1 > $ sudo xl cpupool-create /etc/xen/newpool > > > Here is the new cpupool config file: > $ cat /etc/xen/newpool > name = "newpool" > sched = "credit" > cpus = ["1"] > > And here is the xen console output: > (XEN) Xen BUG at spinlock.c:48 > (XEN) [ Xen-4.7-unstable x86_64 debug=y Not tainted ] > (XEN) CPU:0 > (XEN) RIP:e008:[] spinlock.c#check_lock+0x3c/0x40 > (XEN) RFLAGS: 00010246 CONTEXT: hypervisor (d0v1) > (XEN) rax: 0001 rbx: 83007ffe4148 rcx: > (XEN) rdx: 0001 rsi: 82cfb300 rdi: 83007ffe414e > (XEN) rbp: 83007fd07c90 rsp: 83007fd07c90 r8: 830072629ed0 > (XEN) r9: deadbeef r10: 82d08025edc0 r11: 0286 > (XEN) r12: 830072629c60 r13: 830072629840 r14: 82d0802e4a40 > (XEN) r15: 82d08033bd40 cr0: 80050033 cr4: 000406a0 > (XEN) cr3: 4c536000 cr2: 7fc1e682bab5 > (XEN) ds: es: fs: gs: ss: e010 cs: e008 > (XEN) Xen code around (spinlock.c#check_lock+0x3c/0x40): > (XEN) 00 98 83 f2 01 39 d0 75 <02> 0b 5d c3 55 48 89 e5 f0 ff 05 49 4d 1b 00 > 5d > (XEN) Xen stack trace from rsp=83007fd07c90: > (XEN)83007fd07ca8 82d08012fe1b 0001 83007fd07d18 > (XEN)82d08012ec04 0001 830072629ed0 830072629e70 > (XEN)0001 83007ffe6000 83007ffe4148 830072629840 > (XEN)0001 830072629840 82d0802f961c 82d080312980 > (XEN) 83007fd07d38 82d080101b77 83007fd07e48 > (XEN)0001 83007fd07d88 82d080102385 0006c0dd > (XEN)fffe 83007fd07d80 83007fd0 880025cb82f8 > (XEN)82d0802f961c 82d080312980 83007fd07ef8 > (XEN)82d0801312e8 0001 0001 > (XEN) 83007fd07ef8 82d080184d21 83007faee0c0 > (XEN)7fc1e6eec004 83007fd07e48 82d08010acf7 07000206 > (XEN)83007ffea010 83007fd07ea8 83007fd07ea4 0006c0dd > (XEN)83007ffb6000 0006c0dd 820040005000 > (XEN)820040005178 0003 000d0012 00010004 > (XEN)0001 0001 00e2 7fc1e59b67e8 > (XEN) 7ffc3a785990 00408000 7fc1e6ce3515 > (XEN)00da09e0 0001 0001 0001 > (XEN)00da09e0 ffc5ac7667a4 0033 > (XEN)83007ffb6000 880025cb82f8 7ffc3a785760 00305000 > (XEN)7ffc3a785760 7cff802f80c7 82d0802418a2 8100146a > (XEN) Xen call trace: > (XEN)[] spinlock.c#check_lock+0x3c/0x40 > (XEN)[] _spin_lock+0x11/0x52 > (XEN)[] schedule_cpu_switch+0x17f/0x24a > (XEN)[] cpupool.c#cpupool_assign_cpu_locked+0x2b/0x113 > (XEN)[] cpupool_do_sysctl+0x1e2/0x6b4 > (XEN)[] do_sysctl+0x625/0x1088 > (XEN)[] lstar_enter+0xe2/0x13c > (XEN) > (XEN) > (XEN) > (XEN) Panic on CPU 0: > (XEN) Xen BUG at spinlock.c:48 > (XEN) > > - Aaron Cornelius > > ___ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC PATCH v6 00/28] libxl: Deprivilege qemu
I take that this series is going to miss 4.7 at this stage, right? On Tue, 22 Dec 2015, Ian Jackson wrote: > This is a new version of Stefano Stabellini's series > [PATCH v5 0/6] libxl: xs_restrict QEMU > > I took Stefano's code as a spec for how to interact with qemu, and > have reworked the whole series. In particular, I have > - rebased onto staging > - split up some of the larger patches > - restructured the patches so that every intervening version should work > - redone the async task handling > - provided what seem to me to be appropriate configuration options > - shaved a few yaks (although I tried to limit this!) > - fixed the cross-version compatibility > - reduced the new permission grant for the domain to only .../physmap > > I have compiled this but I haven't tested it yet. > > I think it would be very useful at this stage for others to read the > commit messages, internal docs, and important structural elements. > > Detailed code review should probably wait until after testing which in > turn ought to wait unil we decide what (if any) design and interface > changes are needed. > > And I don't expect to get any feedback until the new year :-). > > Happy holidays all. > > Regards, > Ian. > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] Is: ARM maintainers advice ..Was:Re: [PATCH v5 11/28] xsplice: Implement support for applying/reverting/replacing patches.
On Sat, Apr 09, 2016 at 10:36:02PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 07, 2016 at 09:43:38AM -0600, Jan Beulich wrote:
> > >>> On 07.04.16 at 05:09, wrote:
> > >> > +uint8_t *old_ptr;
> > >> > +
> > >> > +BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->undo));
> > >> > +BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof val));
> > >> > +
> > >> > +old_ptr = (uint8_t *)func->old_addr;
> > >>
> > >> (Considering this cast, the "old_addr" member should be
> > >> unsigned long (or void *), not uint64_t: The latest on ARM32
> > >> such would otherwise cause problems.)
> > >
> > > I has to be uint8_t to make the single byte modifications. Keep
> > > also in mind that this file is only for x86.
> >
> > old_addr can't reasonably be uint8_t, so I can only assume you're
> > mixing up things here. (And yes, I do realize this is x86 code, but
> > my reference to ARM32 was only mean to say that there you'll
> > need to do something similar, and casting uint64_t to whatever
> > kind of pointer type is not going to work without compiler warning.)
>
> Way back .. when we spoke about the .xsplice.funcs structure
> you recommended to make the types be either uintXX specific
> or Elf types. I choose Elf types but then we realized that
> ARM32 hypervisor would be involved which of course would have
> a different size of the structure. So I went with uintXXX
> to save a bit of headache (specifically those BUILD_BUG_ON
> checks).
>
> I can't see making the old_addr be unsigned long or void *,
> which means going back to Elf types. And for ARM32 I will
> have to deal with a different structure size.
CC-ing Stefano and Julien here to advise. I ended up
exposing the ABI part in sysctl.h (and design document as):
#define XSPLICE_PAYLOAD_VERSION 1
/*
* .xsplice.funcs structure layout defined in the `Payload format`
* section in the xSplice design document.
*
* The size should be exactly 64 bytes.
*/
struct xsplice_patch_func {
const char *name; /* Name of function to be patched. */
uint64_t new_addr;
uint64_t old_addr; /* Can be zero and name will be looked up. */
uint32_t new_size;
uint32_t old_size;
uint8_t version;/* MUST be XSPLICE_PAYLOAD_VERSION. */
uint8_t pad[31];/* MUST be zero filled. */
};
typedef struct xsplice_patch_func xsplice_patch_func_t;
Which looks nice.
When the ELF file is loaded we load it as this structure:
[x86]
#ifndef CONFIG_ARM
struct xsplice_patch_func_internal {
const char *name;
void *new_addr;
void *old_addr;
uint32_t new_size;
uint32_t old_size;
uint8_t version;
union {
uint8_t undo[8];
uint8_t pad[31];
} u;
};
#else
[ARM]
struct xsplice_patch_func_internal {
const char *name;
uint32_t _pad0;
void *new_addr;
uint32_t _pad1;
void *old_addr;
uint32_t _pad2;
uint32_t new_size;
uint32_t old_size;
uint8_t version;
union {
uint8_t pad[31];
} u;
};
#endif
That allows the size and offsets to be the same. I checked under ARM32
builds:
struct xsplice_patch_func_internal {
const char * name; /* 0 4 */
uint32_t _pad0;/* 4 4 */
void * new_addr; /* 8 4 */
uint32_t _pad1;/*12 4 */
void * old_addr; /*16 4 */
uint32_t _pad2;/*20 4 */
uint32_t new_size; /*24 4 */
uint32_t old_size; /*28 4 */
uint8_tversion; /*32 1 */
union {
uint8_tpad[31]; /* 31 */
} u; /*3331 */
/* --- cacheline 1 boundary (64 bytes) --- */
/* size: 64, cachelines: 1, members: 10 */
};
So it all looks correct. (and I can cast the old_addr to uint8_t).
Naturally we can expand the pad[] to hold whatever is needed
when implementing xSplice under ARM
However I would appreciate advise from ARM folks if I made some
wrong assumptions..
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 28/28] libxl: xsrestrict QEMU
On Tue, 22 Dec 2015, Ian Jackson wrote: > If QEMU supports xsrestrict, pass xsrestrict=on to it (by default). > > XXX We need to do this only if xenstored supports it, and AFAICT there > is not a particularly easy way to test this. Should we open a new > test xenstore connection to query this information ? We could do this > once per libxl ctx. > > Allow the user to specify that xsrestrict should be on, in which case > if it qemu cannot be depriv'd, we fail. > > When qemu is depriv'd it still needs write access to the physmap > records in xenstore. It will be running with the privilege of the > domain, so we need to allow the domain write access to > /local/domain/$LIBXL_TOOLSTACK_DOMID/device-model/$DOMID/physmap QEMU also needs to be able to write to "state" under /local/domain/$LIBXL_TOOLSTACK_DOMID/device-model/$DOMID I guess theoretically it might be possible to restrict the xenstore connection on the QEMU side only after switching to the "running" state, however that's not how it was done in the early implementation at least: http://marc.info/?i=1433173614-19716-2-git-send-email-stefano.stabellini%40eu.citrix.com ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] docs: add misc/qemu-backends.txt
On Thu, 7 Apr 2016, Juergen Gross wrote: > Document the interface between qemu and libxl regarding backends > supported by qemu. > > Signed-off-by: Juergen Gross > --- > docs/misc/qemu-backends.txt | 19 +++ > 1 file changed, 19 insertions(+) > create mode 100644 docs/misc/qemu-backends.txt > > diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt > new file mode 100644 > index 000..f28755e > --- /dev/null > +++ b/docs/misc/qemu-backends.txt > @@ -0,0 +1,19 @@ > +In order to know whether qemu supports a specific backend type libxl > +needs a way to obtain this information. > + > +As each qemu instance owns a path (named "" from now on) in > +Xenstore the backend information is presented there. is built > +from the domain id where the qemu instance is running > +and the domain id of the target domain of the qemu process : > + > + = /local/domain//device-model/ > + > +Before signalling qemu is running by writing "running" to /state > +qemu will create a Xenstore node for each supported backend under > +/backends with the backend type as name (e.g. > +/backends/qdisk for the qdisk backend). > + > +libxl can assume a backend of a specific type is supported if: > +- /backends/ is existing in Xenstore > +- or /backends is not existing and is one of: > + "console", "vkbd", "vfb", "qdisk", "qnic" The thing to be careful about is that the plan just a few months ago was to have QEMU restrict its own xenstore connection to the privilege level of the guest VM it was servicing. Libxl would relax the xenstore access rights to allow QEMU (and the gueest VM) access to /local/domain//device-model//physmap, but nothing else. See: [1] http://marc.info/?l=qemu-devel&m=143317363104584&w=2 [2] http://marc.info/?l=xen-devel&m=145081000327541 what that means is that QEMU wouldn't be able to write to /local/domain//device-model//backends, unless the writing was done before calling xsrestrict, which should be doable, but not what was done in [1]. Maybe we could add a note saying that these paths need to be written by QEMU before dropping xenstore privileges? ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] pv-grub guest booting fail with recent qemu-xen
Hello,
> > > > +if (prod - out_cons >= XENFB_OUT_RING_LEN) {
> > > > +return;
> > > > +}
This test seems overzealous to me: AIUI, the producer can produce
XENFB_OUT_RING_LEN events, and thus prod - out_cons is exactly
XENFB_OUT_RING_LEN, i.e. there is no room left at all.
The frontend part is:
while (page->out_prod - page->out_cons == XENFB_OUT_RING_LEN)
schedule();
I.e. it waits while the buffer is exactly full.
So it seems to me the bug is at the backend side.
Samuel
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [for-4.7 2/5] xen/arm: acpi: The boot CPU does not always match the first entry in the MADT
On Thu, 7 Apr 2016, Julien Grall wrote:
> Since the ACPI 6.0 errata document [1], the first entry in the MADT
> does not have to correspond to the boot CPU.
>
> Introduce a new variable to know if a MADT entry matching the boot CPU
> is found. Furthermore, it's not necessary to check if the MPIDR is
> duplicated for the boot CPU. So the rest of the function can be skipped.
>
> [1] 1380 Unnecessary restrictions to FW vendors in ordering of GIC structures
> in MADT
>
> Signed-off-by: Julien Grall
> ---
> xen/arch/arm/acpi/boot.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 859aa86..2a71660 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -37,7 +37,8 @@
> #include
>
> /* Processors with enabled flag and sane MPIDR */
> -static unsigned int enabled_cpus;
> +static unsigned int enabled_cpus = 1;
> +static bool __initdata bootcpu_valid;
>
> /* total number of cpus in this system */
> static unsigned int __initdata total_cpus;
> @@ -71,10 +72,15 @@ acpi_map_gic_cpu_interface(struct
> acpi_madt_generic_interrupt *processor)
> }
>
> /* Check if GICC structure of boot CPU is available in the MADT */
> -if ( (enabled_cpus == 0) && (cpu_logical_map(0) != mpidr) )
> +if ( cpu_logical_map(0) == mpidr )
> {
> -printk("Firmware bug, invalid CPU MPIDR for cpu0: 0x%"PRIx64" in
> MADT\n",
> - mpidr);
> +if ( bootcpu_valid )
> +{
> +printk("Firmware bug, duplicate boot CPU MPIDR: 0x%"PRIx64" in
> MADT\n",
> + mpidr);
> +return;
> +}
> +bootcpu_valid = true;
> return;
> }
In that case you can start the other loop below from i = 1, instead of
i = 0.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [for-4.7 3/5] xen/arm: acpi: Fix SMP support when booting with ACPI
On Thu, 7 Apr 2016, Julien Grall wrote:
> The variable enabled_cpus is used to know the number of CPU enabled in
> the MADT.
>
> Currently this variable is used to check the validity of the boot CPU.
> It will be considered invalid when "enabled_cpus > 1".
>
> However, this condition also means that multiple CPUs are present on the
> system. So secondary will never be brought up.
>
> The correct way to check the validity of the boot CPU is to use the
> variable bootcpu_valid.
>
> Signed-off-by: Julien Grall
Reviewed-by: Stefano Stabellini
> xen/arch/arm/acpi/boot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 2a71660..fd29bdc 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -149,7 +149,7 @@ void __init acpi_smp_init_cpus(void)
> return;
> }
>
> -if ( enabled_cpus > 1 )
> +if ( !bootcpu_valid )
> {
> printk("MADT missing boot CPU MPIDR, not enabling secondaries\n");
> return;
> --
> 1.9.1
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [for-4.7 4/5] xen/arm: acpi: Remove uncessary check in acpi_map_gic_cpu_interface
On Thu, 7 Apr 2016, Julien Grall wrote:
> This part of the code will never be executed when the entry
> corresponds to the boot CPU.
>
> Also print an error message rather when arch_cpu_init has failed.
>
> Signed-off-by: Julien Grall
Reviewed-by: Stefano Stabellini
> xen/arch/arm/acpi/boot.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index fd29bdc..602ab39 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -51,6 +51,7 @@ static void __init
> acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
> {
> int i;
> +int rc;
> u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK;
> bool_t enabled = !!(processor->flags & ACPI_MADT_ENABLED);
>
> @@ -102,16 +103,16 @@ acpi_map_gic_cpu_interface(struct
> acpi_madt_generic_interrupt *processor)
> if ( !acpi_psci_present() )
> return;
>
> -/* CPU 0 was already initialized */
> -if ( enabled_cpus )
> +if ( (rc = arch_cpu_init(enabled_cpus, NULL)) < 0 )
> {
> -if ( arch_cpu_init(enabled_cpus, NULL) < 0 )
> -return;
> -
> -/* map the logical cpu id to cpu MPIDR */
> -cpu_logical_map(enabled_cpus) = mpidr;
> +printk("cpu%d: init failed (0x%"PRIx64" MPIDR): %d\n",
> + enabled_cpus, mpidr, rc);
> +return;
> }
>
> +/* map the logical cpu id to cpu MPIDR */
> +cpu_logical_map(enabled_cpus) = mpidr;
> +
> enabled_cpus++;
> }
>
> --
> 1.9.1
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [for-4.7 5/5] xen/arm: acpi: Print more error messages in acpi_map_gic_cpu_interface
On Thu, 7 Apr 2016, Julien Grall wrote:
> It's helpful to spot any error without having to modify the hypervisor
> code.
>
> Signed-off-by: Julien Grall
Reviewed-by: Stefano Stabellini
> xen/arch/arm/acpi/boot.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 602ab39..23285f7 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -63,7 +63,10 @@ acpi_map_gic_cpu_interface(struct
> acpi_madt_generic_interrupt *processor)
>
> total_cpus++;
> if ( !enabled )
> +{
> +printk("Skipping disabled CPU entry with 0x%"PRIx64" MPIDR\n",
> mpidr);
> return;
> +}
>
> if ( enabled_cpus >= NR_CPUS )
> {
> @@ -101,7 +104,11 @@ acpi_map_gic_cpu_interface(struct
> acpi_madt_generic_interrupt *processor)
> }
>
> if ( !acpi_psci_present() )
> +{
> +printk("PSCI not present, skipping CPU MPIDR 0x%"PRIx64"\n",
> + mpidr);
> return;
> +}
>
> if ( (rc = arch_cpu_init(enabled_cpus, NULL)) < 0 )
> {
> --
> 1.9.1
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] Is: ARM maintainers advice ..Was:Re: [PATCH v5 11/28] xsplice: Implement support for applying/reverting/replacing patches.
On Sun, 10 Apr 2016, Konrad Rzeszutek Wilk wrote:
> On Sat, Apr 09, 2016 at 10:36:02PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Thu, Apr 07, 2016 at 09:43:38AM -0600, Jan Beulich wrote:
> > > >>> On 07.04.16 at 05:09, wrote:
> > > >> > +uint8_t *old_ptr;
> > > >> > +
> > > >> > +BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->undo));
> > > >> > +BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof val));
> > > >> > +
> > > >> > +old_ptr = (uint8_t *)func->old_addr;
> > > >>
> > > >> (Considering this cast, the "old_addr" member should be
> > > >> unsigned long (or void *), not uint64_t: The latest on ARM32
> > > >> such would otherwise cause problems.)
> > > >
> > > > I has to be uint8_t to make the single byte modifications. Keep
> > > > also in mind that this file is only for x86.
> > >
> > > old_addr can't reasonably be uint8_t, so I can only assume you're
> > > mixing up things here. (And yes, I do realize this is x86 code, but
> > > my reference to ARM32 was only mean to say that there you'll
> > > need to do something similar, and casting uint64_t to whatever
> > > kind of pointer type is not going to work without compiler warning.)
> >
> > Way back .. when we spoke about the .xsplice.funcs structure
> > you recommended to make the types be either uintXX specific
> > or Elf types. I choose Elf types but then we realized that
> > ARM32 hypervisor would be involved which of course would have
> > a different size of the structure. So I went with uintXXX
> > to save a bit of headache (specifically those BUILD_BUG_ON
> > checks).
> >
> > I can't see making the old_addr be unsigned long or void *,
> > which means going back to Elf types. And for ARM32 I will
> > have to deal with a different structure size.
>
> CC-ing Stefano and Julien here to advise. I ended up
> exposing the ABI part in sysctl.h (and design document as):
>
>
> #define XSPLICE_PAYLOAD_VERSION 1
> /*
> * .xsplice.funcs structure layout defined in the `Payload format`
> * section in the xSplice design document.
> *
> * The size should be exactly 64 bytes.
> */
> struct xsplice_patch_func {
> const char *name; /* Name of function to be patched. */
> uint64_t new_addr;
> uint64_t old_addr; /* Can be zero and name will be looked up. */
> uint32_t new_size;
> uint32_t old_size;
> uint8_t version;/* MUST be XSPLICE_PAYLOAD_VERSION. */
> uint8_t pad[31];/* MUST be zero filled. */
> };
> typedef struct xsplice_patch_func xsplice_patch_func_t;
>
> Which looks nice.
>
> When the ELF file is loaded we load it as this structure:
>
> [x86]
> #ifndef CONFIG_ARM
> struct xsplice_patch_func_internal {
> const char *name;
> void *new_addr;
> void *old_addr;
> uint32_t new_size;
> uint32_t old_size;
> uint8_t version;
> union {
> uint8_t undo[8];
> uint8_t pad[31];
> } u;
> };
> #else
> [ARM]
> struct xsplice_patch_func_internal {
> const char *name;
> uint32_t _pad0;
> void *new_addr;
> uint32_t _pad1;
> void *old_addr;
> uint32_t _pad2;
> uint32_t new_size;
> uint32_t old_size;
> uint8_t version;
> union {
> uint8_t pad[31];
> } u;
> };
> #endif
>
> That allows the size and offsets to be the same. I checked under ARM32
> builds:
>
>
> struct xsplice_patch_func_internal {
> const char * name; /* 0 4 */
> uint32_t _pad0;/* 4 4 */
> void * new_addr; /* 8 4 */
> uint32_t _pad1;/*12 4 */
> void * old_addr; /*16 4 */
> uint32_t _pad2;/*20 4 */
> uint32_t new_size; /*24 4 */
> uint32_t old_size; /*28 4 */
> uint8_tversion; /*32 1 */
> union {
> uint8_tpad[31]; /* 31 */
> } u; /*3331 */
> /* --- cacheline 1 boundary (64 bytes) --- */
>
> /* size: 64, cachelines: 1, members: 10 */
> };
>
> So it all looks correct. (and I can cast the old_addr to uint8_t).
> Naturally we can expand the pad[] to hold whatever is needed
> when implementing xSplice under ARM
>
> However I would appreciate advise from ARM folks if I made some
> wrong assumptions..
It looks good. I take that ARM64 will be like x86. In that case, instead
of #ifdef'ing x86 or ARM, I would #ifdef BITS_PER_LONG or POINTER_ALIGN.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 06/24] arm/x86/vmap: Add vmalloc_xen, vfree_xen and vm_init_type
For those users who want to use the virtual addresses that
are in the hypervisor's virtual address space - these threej new
functions allow that. Along with providing the underlaying
MFNs for the user's (such as changing page table permissions).
Implementation wise the vmap API keeps track of two virtual
address regions now:
a) VMAP_VIRT_START
b) Any provided virtual address space (need start and end).
The a) one is the default one and the existing behavior
for users of vmalloc, vmap, etc is the same.
If however one wishes to use the b) one only has to use
the vm_init_type to initialize and the vmalloc_type to utilize it.
This allows users (such as xSplice) to provide their own
mechanism to change the the page flags, and also use virtual
addresses closer to the hypervisor virtual addresses (at least
on x86) while not having to deal with the allocation of
pages.
For example of users, see patch titled "xsplice: Implement payload
loading", where we parse the payload's ELF relocations - which
is defined to be signed 32-bit (so max displacement is 2GB virtual
spacE). The displacement of the hypervisor virtual addresses to the
vmalloc (on x86) is more than 32-bits - which means that ELF relocations
would truncate the 34 and 33th bit. Hence this alternate API.
We also add add extra checks in case the b) range has not been initialized.
Signed-off-by: Konrad Rzeszutek Wilk
Suggested-by: Jan Beulich
Acked-by: Julien Grall [ARM]
---
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Keir Fraser
Cc: Tim Deegan
Cc: Stefano Stabellini
Cc: Julien Grall
v4: New patch.
v5: Update per Jan's comments.
v6: Drop the stray parentheses on typedefs.
Ditch the vunmap callback. Stash away the virtual addresses in lists.
Ditch the vmap callback. Just provide virtual address.
Ditch the vmalloc_range. Require users of alternative virtual address
to call vmap_init_type first.
v7: Don't expose the vmalloc_type and such. Instead provide an wrapper
called vmalloc_xen for those.
Rename the enum, change one of the names.
Moved the vunmap_type around in c file so we don't have to declare
it in the header.
---
---
xen/arch/arm/kernel.c | 2 +-
xen/arch/arm/mm.c | 2 +-
xen/arch/x86/mm.c | 2 +-
xen/common/vmap.c | 202 ++---
xen/drivers/acpi/osl.c | 2 +-
xen/include/xen/vmap.h | 21 -
6 files changed, 148 insertions(+), 83 deletions(-)
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 61808ac..9871bd9 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -299,7 +299,7 @@ static __init int kernel_decompress(struct bootmodule *mod)
return -ENOMEM;
}
mfn = _mfn(page_to_mfn(pages));
-output = __vmap(&mfn, 1 << kernel_order_out, 1, 1, PAGE_HYPERVISOR);
+output = __vmap(&mfn, 1 << kernel_order_out, 1, 1, PAGE_HYPERVISOR,
VMAP_DEFAULT);
rc = perform_gunzip(output, input, size);
clean_dcache_va_range(output, output_size);
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7065c3e..94ea054 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -807,7 +807,7 @@ void *ioremap_attr(paddr_t pa, size_t len, unsigned int
attributes)
mfn_t mfn = _mfn(PFN_DOWN(pa));
unsigned int offs = pa & (PAGE_SIZE - 1);
unsigned int nr = PFN_UP(offs + len);
-void *ptr = __vmap(&mfn, nr, 1, 1, attributes);
+void *ptr = __vmap(&mfn, nr, 1, 1, attributes, VMAP_DEFAULT);
if ( ptr == NULL )
return NULL;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index bca7532..ca2d0bb 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -6124,7 +6124,7 @@ void __iomem *ioremap(paddr_t pa, size_t len)
unsigned int offs = pa & (PAGE_SIZE - 1);
unsigned int nr = PFN_UP(offs + len);
-va = __vmap(&mfn, nr, 1, 1, PAGE_HYPERVISOR_NOCACHE) + offs;
+va = __vmap(&mfn, nr, 1, 1, PAGE_HYPERVISOR_NOCACHE, VMAP_DEFAULT) +
offs;
}
return (void __force __iomem *)va;
diff --git a/xen/common/vmap.c b/xen/common/vmap.c
index 134eda0..eb477a4 100644
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -10,40 +10,43 @@
#include
static DEFINE_SPINLOCK(vm_lock);
-static void *__read_mostly vm_base;
-#define vm_bitmap ((unsigned long *)vm_base)
+static void *__read_mostly vm_base[VMAP_REGION_NR];
+#define vm_bitmap(x) ((unsigned long *)vm_base[x])
/* highest allocated bit in the bitmap */
-static unsigned int __read_mostly vm_top;
+static unsigned int __read_mostly vm_top[VMAP_REGION_NR];
/* total number of bits in the bitmap */
-static unsigned int __read_mostly vm_end;
+static unsigned int __read_mostly vm_end[VMAP_REGION_NR];
/* lowest known clear bit in the bitmap */
-static unsigned int vm_low;
+static unsigned int vm_low[VMAP_REGION_NR];
-void __init vm_init(void)
+void __init vm_init_type(enum vmap_region type, void *start, void *end)
{
unsigned int i, nr;
unsigned long va;
-vm_base = (void *)VMAP_VI
[Xen-devel] [PATCH v7 21/24] xsplice: Stacking build-id dependency checking.
We now expect that the ELF payloads be built with the
--build-id.
Also the .xsplice.deps section has to have the contents
of the hypervisor (or a preceding payload) build-id.
We already have the code to verify the Elf_Note build-id
so export parts of it.
This dependency means the hypervisor MUST be compiled with
--build-id - so we gate the build of xSplice on the availability
of said functionality.
This does not impact the ordering of how the payloads can
be loaded, but it does enforce an STRICT ordering when the
payloads are applied. Also the REPLACE is special - we need
to check that its dependency against the hypervisor - not
the last applied patch.
To make this easier to test we also add an extra test-case
to be used - which can only be applied on top of the
xen_hello_world payload.
As in, one can apply xen_hello_world and then xen_bye_world
on top of that. Not the other way.
We also print the dependency and payloads build_in the keyhandler.
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
---
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v3: First time included.
v4: Andrew fix against the build_id.o mutilations.
Andrew fix to not include extra symbols in binary.id
v5: s/ssize_t/unsigned int/
v6: s/an NT_GNU../a NT_GNU/
- Squash "xsplice: Print dependency and payloads build_id in the keyhandler"
in this patch.
- Add in xen_build_id_check size of section for better checking.
v7: Added Andrew's reviewed-by.
Change the .name in test-case to adhere to spec.
Dropped NT_GNU_BUILD_ID and moved that to earlier patch
(build_id: Provide ld-embedded build-ids)
Amended spec and code to only have one of .xsplice.depends and
.note.gnu.build-id
Expanded comment about note.o and why we don't use arch/x86/note.o.bin
Moved xen_build_id_check definition to xsplice.h from version.h
(and dropping the #include's in version.h)
Sort header files in tests.
---
---
.gitignore | 1 +
Config.mk | 1 +
docs/misc/xsplice.markdown | 99 ++--
xen/arch/x86/test/Makefile | 45 +++--
xen/arch/x86/test/xen_bye_world.c | 34 ++
xen/arch/x86/test/xen_bye_world_func.c | 24 +++
xen/common/Kconfig | 6 +-
xen/common/version.c | 42
xen/common/xsplice.c | 116 -
xen/include/xen/xsplice.h | 2 +
10 files changed, 316 insertions(+), 54 deletions(-)
create mode 100644 xen/arch/x86/test/xen_bye_world.c
create mode 100644 xen/arch/x86/test/xen_bye_world_func.c
diff --git a/.gitignore b/.gitignore
index 4a81f43..88cec1d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -248,6 +248,7 @@ xen/arch/x86/efi/disabled
xen/arch/x86/efi/mkreloc
xen/arch/x86/test/config.h
xen/arch/x86/test/xen_hello_world.xsplice
+xen/arch/x86/test/xen_bye_world.xsplice
xen/arch/*/efi/boot.c
xen/arch/*/efi/compat.c
xen/arch/*/efi/efi.h
diff --git a/Config.mk b/Config.mk
index db70638..4b6f3f5 100644
--- a/Config.mk
+++ b/Config.mk
@@ -134,6 +134,7 @@ ifeq ($(call ld-ver-build-id,$(LD)),n)
build_id_linker :=
else
CFLAGS += -DBUILD_ID
+export XEN_HAS_BUILD_ID=y
build_id_linker := --build-id=sha1
endif
diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown
index c9c32b7..2ab4ca0 100644
--- a/docs/misc/xsplice.markdown
+++ b/docs/misc/xsplice.markdown
@@ -283,8 +283,17 @@ The xSplice core code loads the payload as a standard ELF
binary, relocates it
and handles the architecture-specifc sections as needed. This process is much
like what the Linux kernel module loader does.
-The payload contains a section (xsplice_patch_func) with an array of structures
-describing the functions to be patched:
+The payload contains at least three sections:
+
+ * `.xsplice.funcs` - which is an array of xsplice_patch_func structures.
+ * `.xsplice.depends` - which is an ELF Note that describes what the payload
+depends on. **MUST** have one.
+ * `.note.gnu.build-id` - the build-id of this payload. **MUST** have one.
+
+### .xsplice.funcs
+
+The `.xsplice.funcs` contains an array of xsplice_patch_func structures
+which describe the functions to be patched:
struct xsplice_patch_func {
@@ -367,6 +376,23 @@ struct xsplice_patch_func xsplice_hello_world = {
Code must be compiled with -fPIC.
+### .xsplice.depends and .note.gnu.build-id
+
+To support dependencies checking and safe loading (to load the
+appropiate payload against the right hypervisor) there is a need
+to embbed an build-id dependency.
+
+This is done by the payload containing an section `.xsplice.depends`
+which follows the format of an ELF Note. The contents of this
+(name, and description) are specific to the linker utilized to
+build the hypevisor and payload.
+
+If GNU linker is used then the name is `GNU` and the description
+is a NT_GNU_BUILD_ID type ID
[Xen-devel] [PATCH v7 14/24] xsplice: Add support for bug frames.
From: Ross Lagerwall
Add support for handling bug frames contained with xsplice modules. If a
trap occurs search either the kernel bug table or an applied payload's
bug table depending on the instruction pointer.
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
---
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v2:- s/module/payload/
- add build time check in case amount of bug frames expands.
- add define for the number of bug-frames.
v3:
- add missing BUGFRAME_NR, squash s/core_size/core/ in earlier patch.
- Moved code around.
- Changed per Andrew's recommendation.
- Fixed style changes.
- Made it compile under ARM (PRIu32,PRIu64)
v4: Use 'struct virtual_region'
- Rip more of the is_active_text code.
- Use one function for the ->skip
- Include test-case
v5: Rip out the ->skip function.
v7: Add a text check as well.
Add Andrew's Reviewed-by.
---
---
xen/arch/x86/traps.c | 5 +++--
xen/common/xsplice.c | 50 +++
xen/include/xen/xsplice.h | 5 +
3 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index f73f7f3..8384158 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -50,6 +50,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1287,7 +1288,7 @@ void do_invalid_op(struct cpu_user_regs *regs)
/* WARN, BUG or ASSERT: decode the filename pointer and line number. */
filename = bug_ptr(bug);
-if ( !is_kernel(filename) )
+if ( !is_kernel(filename) && !is_patch(filename) )
goto die;
fixup = strlen(filename);
if ( fixup > 50 )
@@ -1314,7 +1315,7 @@ void do_invalid_op(struct cpu_user_regs *regs)
case BUGFRAME_assert:
/* ASSERT: decode the predicate string pointer. */
predicate = bug_msg(bug);
-if ( !is_kernel(predicate) )
+if ( !is_kernel(predicate) && !is_patch(predicate) )
predicate = "";
printk("Assertion '%s' failed at %s%s:%d\n",
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index a92a39b..8424c0f 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -122,6 +122,32 @@ static int verify_payload(const
xen_sysctl_xsplice_upload_t *upload, char *n)
return 0;
}
+bool_t is_patch(const void *ptr)
+{
+struct payload *data;
+
+/*
+ * No locking since this list is only ever changed during apply or revert
+ * context.
+ */
+list_for_each_entry ( data, &applied_list, applied_list )
+{
+if ( ptr >= data->rw_addr &&
+ ptr < (data->rw_addr + data->rw_size) )
+return 1;
+
+if ( ptr >= data->ro_addr &&
+ ptr < (data->ro_addr + data->ro_size) )
+return 1;
+
+if ( ptr >= data->text_addr &&
+ ptr < (data->text_addr + data->text_size) )
+return 1;
+}
+
+return 0;
+}
+
unsigned long xsplice_symbols_lookup_by_name(const char *symname)
{
struct payload *data;
@@ -479,6 +505,30 @@ static int prepare_payload(struct payload *payload,
region->start = (unsigned long)payload->text_addr;
region->end = (unsigned long)(payload->text_addr + payload->text_size);
+/* Optional sections. */
+for ( i = 0; i < BUGFRAME_NR; i++ )
+{
+char str[14];
+
+snprintf(str, sizeof(str), ".bug_frames.%u", i);
+sec = xsplice_elf_sec_by_name(elf, str);
+if ( !sec )
+continue;
+
+if ( sec->sec->sh_size &&
+ (sec->sec->sh_size % sizeof(*region->frame[i].bugs)) )
+{
+dprintk(XENLOG_DEBUG, XSPLICE "%s: Wrong size of
.bug_frames.%u!\n",
+elf->name, i);
+return -EINVAL;
+}
+
+region->frame[i].bugs = sec->load_addr;
+if ( sec->sec->sh_size)
+region->frame[i].n_bugs = sec->sec->sh_size /
+ sizeof(*region->frame[i].bugs);
+}
+
return 0;
}
diff --git a/xen/include/xen/xsplice.h b/xen/include/xen/xsplice.h
index a0436da..3ab3911 100644
--- a/xen/include/xen/xsplice.h
+++ b/xen/include/xen/xsplice.h
@@ -66,6 +66,7 @@ struct xsplice_symbol {
int xsplice_op(struct xen_sysctl_xsplice_op *);
void check_for_xsplice_work(void);
+bool_t is_patch(const void *addr);
unsigned long xsplice_symbols_lookup_by_name(const char *symname);
/* Arch hooks. */
@@ -118,6 +119,10 @@ static inline int xsplice_op(struct xen_sysctl_xsplice_op
*op)
}
static inline void check_for_xsplice_work(void) { };
+static inline bool_t is_patch(const void *addr)
+{
+return 0;
+}
#endif /* CONFIG_XSPLICE */
#endif /* __XEN_XSPLICE_H__ */
--
2.5.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 15/24] xsplice: Add support for exception tables.
From: Ross Lagerwall
Add support for exception tables contained within xSplice payloads. If an
exception occurs search either the main exception table or a particular
active payload's exception table depending on the instruction pointer.
Also we add an test-case to make sure we have an exception that
is handled.
To not grow the code-base if xSplice is not compiled in we add
certain #define to help in determining if code needs to be __init
or not.
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
---
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v3:
- s/module/payload/
- sanity checks.
- Move code around.
- s/module/payload/
v4: Use 'struct virtual_region'
v5:
- Expand test-case.
- Deal with struct exception_table_entry being const.
v6:
- Make the code have __init if not compiled with xSplice
- Remove not needed declarations.
v7:
- Make the non_canonical_addr be 0xdeadULL
- Remove casts
- Add Reviewed-by from Andrew
- Change ifdef to be !ARM
---
---
xen/arch/x86/extable.c | 31 ++-
xen/arch/x86/test/xen_hello_world_func.c | 11 +++
xen/common/xsplice.c | 25 +
xen/include/asm-x86/uaccess.h| 2 ++
xen/include/xen/xsplice.h| 17 +
5 files changed, 73 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 2a06cca..e0ba35f 100644
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#define EX_FIELD(ptr, field) ((unsigned long)&(ptr)->field + (ptr)->field)
@@ -20,7 +21,7 @@ static inline unsigned long ex_cont(const struct
exception_table_entry *x)
return EX_FIELD(x, cont);
}
-static int __init cmp_ex(const void *a, const void *b)
+static int __INIT cmp_ex(const void *a, const void *b)
{
const struct exception_table_entry *l = a, *r = b;
unsigned long lip = ex_addr(l);
@@ -35,7 +36,7 @@ static int __init cmp_ex(const void *a, const void *b)
}
#ifndef swap_ex
-static void __init swap_ex(void *a, void *b, int size)
+static void __INIT swap_ex(void *a, void *b, int size)
{
struct exception_table_entry *l = a, *r = b, tmp;
long delta = b - a;
@@ -48,19 +49,23 @@ static void __init swap_ex(void *a, void *b, int size)
}
#endif
-void __init sort_exception_tables(void)
+void __INIT sort_exception_table(struct exception_table_entry *start,
+ struct exception_table_entry *stop)
{
-sort(__start___ex_table, __stop___ex_table - __start___ex_table,
- sizeof(struct exception_table_entry), cmp_ex, swap_ex);
-sort(__start___pre_ex_table,
- __stop___pre_ex_table - __start___pre_ex_table,
+sort(start, stop - start,
sizeof(struct exception_table_entry), cmp_ex, swap_ex);
}
-static inline unsigned long
-search_one_table(const struct exception_table_entry *first,
- const struct exception_table_entry *last,
- unsigned long value)
+void __init sort_exception_tables(void)
+{
+sort_exception_table(__start___ex_table, __stop___ex_table);
+sort_exception_table(__start___pre_ex_table, __stop___pre_ex_table);
+}
+
+unsigned long
+search_one_extable(const struct exception_table_entry *first,
+ const struct exception_table_entry *last,
+ unsigned long value)
{
const struct exception_table_entry *mid;
long diff;
@@ -85,7 +90,7 @@ search_exception_table(unsigned long addr)
const struct virtual_region *region = find_text_region(addr);
if ( region && region->ex )
-return search_one_table(region->ex, region->ex_end - 1, addr);
+return search_one_extable(region->ex, region->ex_end - 1, addr);
return 0;
}
@@ -94,7 +99,7 @@ unsigned long
search_pre_exception_table(struct cpu_user_regs *regs)
{
unsigned long addr = (unsigned long)regs->eip;
-unsigned long fixup = search_one_table(
+unsigned long fixup = search_one_extable(
__start___pre_ex_table, __stop___pre_ex_table-1, addr);
if ( fixup )
{
diff --git a/xen/arch/x86/test/xen_hello_world_func.c
b/xen/arch/x86/test/xen_hello_world_func.c
index 1ad002a..432954f 100644
--- a/xen/arch/x86/test/xen_hello_world_func.c
+++ b/xen/arch/x86/test/xen_hello_world_func.c
@@ -5,9 +5,20 @@
#include
+static unsigned long *non_canonical_addr = (unsigned long
*)(0xdeadULL);
+
/* Our replacement function for xen_extra_version. */
const char *xen_hello_world(void)
{
+unsigned long tmp;
+int rc;
+/*
+ * Any BUG, or WARN_ON will contain symbol and payload name. Furthermore
+ * exceptions will be caught and processed properly.
+ */
+rc = __get_user(tmp, non_canonical_addr);
+BUG_ON(rc != -EFAULT);
+
return "Hello World";
}
diff --git a/xen/common/xsplic
[Xen-devel] [PATCH v7 13/24] x86, xsplice: Print payload's symbol name and payload name in backtraces
From: Ross Lagerwall
Naturally the backtrace is presented when an instruction
hits an bug_frame or %p is used.
The payloads do not support bug_frames yet - however the functions
the payloads call could hit an BUG() or WARN().
The traps.c has logic to scan for it this - and eventually it will
find the correct bug_frame and the walk the stack using %p to print
the backtrace. For %p and symbols to print a string - the
'is_active_kernel_text' is consulted which uses an 'struct virtual_region'.
Therefore we register our start->end addresses so that
'is_active_kernel_text' will include our payload address.
We also register our symbol lookup table function so that it can
scan the list of payloads and retrieve the correct name.
Lastly we change vsprintf to take into account s and namebuf.
For core code they are the same, but for payloads they are different.
This gets us:
Xen call trace:
[] revert_hook+0x31/0x35 [xen_hello_world]
[] xsplice.c#revert_payload+0x86/0xc6
[] check_for_xsplice_work+0x233/0x3cd
[] domain.c#continue_idle_domain+0x9/0x1f
Which is great if payloads have similar or same symbol names.
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
---
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Keir Fraser
Cc: Tim Deegan
v2: Add missing full stop.
v3: s/module/payload/
v4: Expand comment and include registration of 'virtual_region'
Redo the vsprintf handling of payload name.
Drop the ->skip function
v6: Add comment explaining the purpose behind the strcmp.
Redid per Jan's review.
v7: Add Andrew's Review-by
Drop the strcmp and just do pointer checks.
---
---
xen/common/vsprintf.c | 19 +++--
xen/common/xsplice.c | 59 +++
2 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index 18d2634..acabb3a 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include
#include
@@ -331,16 +332,23 @@ static char *pointer(char *str, char *end, const char
**fmt_ptr,
{
unsigned long sym_size, sym_offset;
char namebuf[KSYM_NAME_LEN+1];
+bool_t payload = 0;
/* Advance parents fmt string, as we have consumed 's' or 'S' */
++*fmt_ptr;
s = symbols_lookup((unsigned long)arg, &sym_size, &sym_offset,
namebuf);
-
-/* If the symbol is not found, fall back to printing the address */
+/* If the symbol is not found, fall back to printing the address. */
if ( !s )
break;
+/*
+ * namebuf contents and s for core hypervisor are same but for xSplice
+ * payloads they differ (namebuf contains the name of the payload).
+ */
+if ( namebuf != s )
+payload = 1;
+
/* Print symbol name */
str = string(str, end, s, -1, -1, 0);
@@ -354,6 +362,13 @@ static char *pointer(char *str, char *end, const char
**fmt_ptr,
str = number(str, end, sym_size, 16, -1, -1, SPECIAL);
}
+if ( payload )
+{
+str = string(str, end, " [", -1, -1, 0);
+str = string(str, end, namebuf, -1, -1, 0);
+str = string(str, end, "]", -1, -1, 0);
+}
+
return str;
}
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index 27f4822..a92a39b 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -14,7 +14,9 @@
#include
#include
#include
+#include
#include
+#include
#include
#include
#include
@@ -52,6 +54,8 @@ struct payload {
struct list_head applied_list; /* Linked to 'applied_list'. */
struct xsplice_patch_func_internal *funcs;/* The array of functions to
patch. */
unsigned int nfuncs; /* Nr of functions to patch. */
+struct virtual_region region;/* symbol, bug.frame patching and
+exception table (x86). */
struct xsplice_symbol *symtab; /* All symbols. */
char *strtab;/* Pointer to .strtab. */
unsigned int nsyms; /* Nr of entries in .strtab and
symbols. */
@@ -140,6 +144,51 @@ unsigned long xsplice_symbols_lookup_by_name(const char
*symname)
return 0;
}
+static const char *xsplice_symbols_lookup(unsigned long addr,
+ unsigned long *symbolsize,
+ unsigned long *offset,
+ char *namebuf)
+{
+struct payload *data;
+unsigned int i;
+int best;
+
+/*
+ * No locking since this list is only ever changed during apply or revert
+ * context.
+ */
+list_for_each_entry ( data, &applied_list, applied_list )
+{
+if ( (void *)addr < data->text_addr &&
+ (void *)addr >= (data->text_ad
[Xen-devel] [PATCH v7 24/24] MAINTAINERS/xsplice: Add myself and Ross as the maintainers.
If you have a patch for xSplice send it our way! Signed-off-by: Ross Lagerwall Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Andrew Cooper --- Cc: Ian Jackson Cc: Jan Beulich Cc: Keir Fraser Cc: Tim Deegan v5: Sort them F: fields (Jan) v7: Added Andrew's Reviewed-by --- --- MAINTAINERS | 10 ++ 1 file changed, 10 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a34685d..2182c24 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -436,6 +436,16 @@ F: xen/include/xsm/ F: xen/xsm/ F: docs/misc/xsm-flask.txt +XSPLICE +M: Konrad Rzeszutek Wilk +M: Ross Lagerwall +S: Supported +F: docs/misc/xsplice.markdown +F: tools/misc/xen-xsplice.c +F: xen/arch/*/xsplice* +F: xen/common/xsplice* +F: xen/include/xen/xsplice* + THE REST M: Ian Jackson M: Jan Beulich -- 2.5.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7] xSplice v1 design and implementation.
Hey! Changelog: v6: http://lists.xen.org/archives/html/xen-devel/2016-04/msg00871.html - Acted on all comments from Andrew, Julien, and Jan. - Got help from Andrew on one of them over the weekend. - Finally got my EFI box working (tests passed). v5: http://lists.xen.org/archives/html/xen-devel/2016-03/msg03286.html - Acted at all comments from Jan. v4: http://lists.xen.org/archives/html/xen-devel/2016-03/msg01776.html - Lots of review. Lots of rework. Some patches checked in. v3: http://www.gossamer-threads.com/lists/xen/devel/418262 and http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg04106.html - Act on all reviews. - Redo the flow of patches v2: http://lists.xen.org/archives/html/xen-devel/2016-01/msg01597.html - Updated code/docs/design with review comments. - Make xen also have an PT_NOTE - Added more of Ross's patches - Combined build-id patchset with this. (since the RFC and the Seattle Xen presentation) - Finished off some of the work around the build-id. - Settled on the preemption mechanism. - Cleaned the patches a lot up, broke them up to easy review for maintainers. v1: http://lists.xenproject.org/archives/html/xen-devel/2015-09/msg02116.html - Put all the design comments in the code Prototype: http://lists.xenproject.org/archives/html/xen-devel/2015-10/msg02595.html [Posting by Ross] - Took all reviews into account. - Redid the patches *Tools Maintainers* All patches acked. The patch #3: "libxc: Implementation of XEN_XSPLICE_op in libxc" has errno=-EINVAL sprinkled on some return paths. *Hypervisor Maintainers* Jan, the hypervisor patches #2, #5-#17, #21-#23 need your Ack. All hypervisor patches have been Reviewed by Andrew. There is a new patch thanks to Andrew's help over the weekend! [PATCH v7 07/24] x86/mm: Introduce modify_xen_mappings() All the XSM parts have been Acked. The patch related to VERSION_build_id has been Acked: HYPERCALL_version_op: Add VERSION_build_id to retrieve build-id. Patchset has been tested on ARM (tweaking the Kconfig to build xSplice and then booting and using it under ARM CubieTruck - granted there is no patching but the hypercalls work), ARM64 (only built it), and x86 (legacy and EFI) *Are there any TODOs left from v5 or v6 reviews?* One I hope can be deferred - that is xensyms_read which we use in "[PATCH v7 12/24] xsplice,symbols: Implement symbol name resolution on address." is not the fastest. It will need some tweaking (or a new function will have to be written) and I hope that this can be done in v4.8. The other is to test this on a 8 socket machine with tons of CPUs. Somebody else is using this beast right now. The impact is whether the default timeout of 30ms to quisce the CPUs should be increased on those beasts. *What is xSplice?* A mechanism to binarily patch the running hypervisor with new opcodes that have come about due to primarily security updates. *What will this patchset do once I've it* Patch the hypervisor. *Why are you emailing me?* Please please review as many patches as possible. *OK, what do you have?* They are located at a git tree: git://xenbits.xen.org/people/konradwilk/xen.git xsplice.v7 (Copying from Ross's email): Much of the work is implementing a basic version of the Linux kernel module loader. The code: * Loading of xSplice ELF payloads. * Copying allocated sections into a new executable region of memory. * Resolving symbols. * Applying relocations. * Patching of altinstructions. * Special handling of bug frames and exception tables. * Unloading of xSplice ELF payloads. * Compiling a sample xSplice ELF payload * Resolving symbols * Using build-id dependencies * Support for shadow variable framework * Support for executing ELF payload functions on load/unload. The other main bit of this work is applying and reverting the patches safely. As implemented, the code is patched with each CPU waiting in the return-to-guest path (i.e. with no stack) or on the cpu-idle path which appears to be the safest way of patching. While it is safe we should still (in the next wave of patches) to verify to not patch cetain critical sections (say the code doing the patching) All of the following should work: * Applying patches safely. * Reverting patches safely. * Replacing patches safely (e.g. reverting any applied patches and applying a new patch). * Bug frames as part of modules. This means adding or changing WARN, ASSERT, BUG, and run_in_exception_handler works correctly. Line number only changes _are ignored_. * Exception tables as part of modules. E.g. wrmsr_safe and copy_to_user work correctly when used in a patch module. * Stacking of patches on top of each other * Resolving symbols (even of patches) *Limitations* The above is enough to fully implement an update system where multiple source patches are combined (using combinediff) and built into a single binary which then atomically replaces any existing loaded patches (this is why Ross added a REPLACE op
[Xen-devel] [PATCH v7 12/24] xsplice, symbols: Implement symbol name resolution on address.
From: Ross Lagerwall If in the payload we do not have the old_addr we can resolve the virtual address based on the UNDEFined symbols. We also use an boolean flag: new_symbol to track symbols. The usual case this is used is by: * A payload may introduce a new symbol * A payload may override an existing symbol (introduced in Xen or another payload) * Overriding symbols must exist in the symtab for backtraces. * A payload must always link against the object which defines the new symbol. Considering that payloads may be loaded in any order it would be incorrect to link against a payload which simply overrides a symbol because you could end up with a chain of jumps which is inefficient and may result in the expected function not being executed. Also we include a local definition block in the symbols.c file. Signed-off-by: Konrad Rzeszutek Wilk Signed-off-by: Ross Lagerwall Reviewed-by: Andrew Cooper --- Cc: Keir Fraser Cc: Jan Beulich Cc: Andrew Cooper v1: Ross original version. v2: Include test-case and document update. v2: s/size_t/ssize_t/ Include core_text_size, core_text calculation v4: Cast on dprintk to uint64_t to make ELF 32bit build. v6: Rebase where the spinlock is no more recursive. Drop the spinlock usage in xsplice_symbols_lookup_by_name v7: Add Andrew's Reviewed-by Initialize addr and symname to zero as xensyms_read uses it. --- --- xen/arch/x86/Makefile | 16 +++- xen/arch/x86/test/Makefile | 4 +- xen/arch/x86/test/xen_hello_world.c | 5 +- xen/common/symbols.c| 34 xen/common/xsplice.c| 156 +++- xen/common/xsplice_elf.c| 19 - xen/include/xen/symbols.h | 2 + xen/include/xen/xsplice.h | 8 ++ 8 files changed, 232 insertions(+), 12 deletions(-) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 6c2d5fa..57c93e1 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -72,6 +72,12 @@ efi-y := $(shell if [ ! -r $(BASEDIR)/include/xen/compile.h -o \ -O $(BASEDIR)/include/xen/compile.h ]; then \ echo '$(TARGET).efi'; fi) +ifdef CONFIG_XSPLICE +all_symbols = --all-symbols +else +all_symbols = +endif + $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32 ./boot/mkelf32 $(TARGET)-syms $(TARGET) 0x10 \ `$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'` @@ -111,12 +117,14 @@ $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0 $(NM) -pa --format=sysv $(@D)/.$(@F).0 \ - | $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort \ + >$(@D)/.$(@F).0.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1 $(NM) -pa --format=sysv $(@D)/.$(@F).1 \ - | $(BASEDIR)/tools/symbols --sysv --sort --warn-dup >$(@D)/.$(@F).1.S + | $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort --warn-dup \ + >$(@D)/.$(@F).1.S $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o $(LD) $(LDFLAGS) -T xen.lds -N prelink.o \ $(@D)/.$(@F).1.o -o $@ @@ -140,14 +148,14 @@ $(TARGET).efi: prelink-efi.o efi.lds efi/relocs-dummy.o $(BASEDIR)/common/symbol $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).$(base).0 &&) : $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).0) >$(@D)/.$(@F).0r.S $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).0 \ - | $(guard) $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0s.S + | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0s.S $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o $(foreach base, $(VIRT_BASE) $(ALT_BASE), \ $(guard) $(LD) $(call EFI_LDFLAGS,$(base)) -T efi.lds -N $< \ $(@D)/.$(@F).0r.o $(@D)/.$(@F).0s.o -o $(@D)/.$(@F).$(base).1 &&) : $(guard) efi/mkreloc $(foreach base,$(VIRT_BASE) $(ALT_BASE),$(@D)/.$(@F).$(base).1) >$(@D)/.$(@F).1r.S $(guard) $(NM) -pa --format=sysv $(@D)/.$(@F).$(VIRT_BASE).1 \ - | $(guard) $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1s.S + | $(guard) $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1s.S $(guard) $(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o $(guard) $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T efi.lds -N $< \ $(@D)/.$(@F).1r.o $(@D)/.$(@F).1s.o -o $@ diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/
[Xen-devel] [PATCH v7 11/24] x86/xen_hello_world.xsplice: Test payload for patching 'xen_extra_version'.
This change demonstrates how to generate an xSplice ELF payload.
The idea here is that we want to patch in the hypervisor
the 'xen_version_extra' function with an function that will
return 'Hello World'. The 'xl info | grep extraversion'
will reflect the new value after the patching.
To generate this ELF payload file we need:
- C code of the new code (xen_hello_world_func.c).
- C code generating the .xsplice.funcs structure
(xen_hello_world.c)
- The address of the old code (xen_extra_version). We
retrieve it by using 'nm --defined' on xen-syms.
- The size of the new and old code for which we use
nm --defined -S on our code and xen-syms respectively.
There are two C files and one header files generated
during build. One could make this one C file if the
size of the newly patched function size was known in
advance (or an random value was choosen).
There is also a strict order of compiling:
1) xen_hello_world_func.c
2) config.h - extract the size of the new function,
the old function and the old function address.
3) xen_hello_world.c - which contains the .xsplice.funcs
structure.
4) Link the object files in an xen_hello_world.xsplice file.
The use-case is simple:
$xen-xsplice load /usr/lib/debug/xen_hello_world.xsplice
$xen-xsplice list
ID | status
+
xen_hello_world APPLIED
$xl info | grep extra
xen_extra : Hello World
$xen-xsplice revert xen_hello_world
Performing revert: completed
$xen-xsplice unload xen_hello_world
Performing unload: completed
$xl info | grep extra
xen_extra : -unstable
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
Acked-by: Julien Grall [ARM]
---
Cc: Stefano Stabellini
Cc: Julien Grall
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v2: Do it using hypervisor Makefiles
v3: Remove the stale linker file.
Add Copyright and local definition block
s/name/xen_hello_world_name/
v6: Remove the 'install', and 'uninstall' destinations.
Remove xen/config.h from files.
v7: Made the build target be called 'tests'.
Changed the .name to have 'xen_extra_version' to be consistent
with the spec.
Add Julien's Ack and Andrew's Reviewed-by.
---
---
.gitignore | 2 ++
docs/misc/xsplice.markdown | 37 +++
xen/Makefile | 8 --
xen/arch/arm/Makefile| 2 ++
xen/arch/x86/Makefile| 4 +++
xen/arch/x86/test/Makefile | 44
xen/arch/x86/test/xen_hello_world.c | 30 ++
xen/arch/x86/test/xen_hello_world_func.c | 22
8 files changed, 147 insertions(+), 2 deletions(-)
create mode 100644 xen/arch/x86/test/Makefile
create mode 100644 xen/arch/x86/test/xen_hello_world.c
create mode 100644 xen/arch/x86/test/xen_hello_world_func.c
diff --git a/.gitignore b/.gitignore
index 39eb779..4a81f43 100644
--- a/.gitignore
+++ b/.gitignore
@@ -246,6 +246,8 @@ xen/arch/x86/efi.lds
xen/arch/x86/efi/check.efi
xen/arch/x86/efi/disabled
xen/arch/x86/efi/mkreloc
+xen/arch/x86/test/config.h
+xen/arch/x86/test/xen_hello_world.xsplice
xen/arch/*/efi/boot.c
xen/arch/*/efi/compat.c
xen/arch/*/efi/efi.h
diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown
index d4e7d75..c9c32b7 100644
--- a/docs/misc/xsplice.markdown
+++ b/docs/misc/xsplice.markdown
@@ -330,6 +330,43 @@ When reverting a patch, the hypervisor iterates over each
`xsplice_patch_func`
and the core code copies the data from the undo buffer (private internal copy)
to `old_addr`.
+### Example of .xsplice.funcs
+
+A simple example of what a payload file can be:
+
+
+/* MUST be in sync with hypervisor. */
+struct xsplice_patch_func {
+const char *name;
+uint64_t new_addr;
+uint64_t old_addr;
+uint32_t new_size;
+uint32_t old_size;
+uint8_t version;
+uint8_t pad[31];
+};
+
+/* Our replacement function for xen_extra_version. */
+const char *xen_hello_world(void)
+{
+return "Hello World";
+}
+
+static unsigned char patch_this_fnc[] = "xen_extra_version";
+
+struct xsplice_patch_func xsplice_hello_world = {
+.version = XSPLICE_PAYLOAD_VERSION,
+.name = patch_this_fnc,
+.new_addr = (unsigned long)(xen_hello_world),
+.old_addr = 0x82d08013963c, /* Extracted from xen-syms. */
+.new_size = 13, /* To be be computed by scripts. */
+.old_size = 13, /* ---""--- */
+} __attribute__((__section__(".xsplice.funcs")));
+
+
+
+Code must be compiled with -fPIC.
+
## Hypercalls
We will employ the sub operations of the system management hypercall (sysctl).
diff --git a/xen/Makefile b/xen/Makefile
index c908544..eb0482e 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -39,8 +39,8 @@ dist: install
build
[Xen-devel] [PATCH v7 01/24] xsplice: Design document
A mechanism is required to binarily patch the running hypervisor with new opcodes that have come about due to primarily security updates. This document describes the design of the API that would allow us to upload to the hypervisor binary patches. This document has been shaped by the input from: Martin Pohlack Jan Beulich Thank you! Input-from: Martin Pohlack Input-from: Jan Beulich Signed-off-by: Konrad Rzeszutek Wilk Signed-off-by: Ross Lagerwall Acked-by: Ian Jackson --- Cc: Ian Jackson Cc: Jan Beulich Cc: Keir Fraser Cc: Tim Deegan v1-2: review v3: Split document in v1 and v2 (todo) to simplify implementation goals. - Add const on some structures. Truncate size to uint16_t where it makes sense. - Convert 'id' to 'name', Add Ross's comments about what is implemented. - Wei's and Ross's reviews. - Jan's review comments. - Jan's review comments. s/int32_t state/uint32_t state/ now that return code is in seperate field (rc). Add various other types, such as R_X86_64_PC64 in the list. Mention the need for compiler check. v4: - Drop the LOADED->CHECKED state and go directly to CHECKED state. Drop LOADED. v5: Julien mentioned ARM 32-bit would not use ELF64, so make the .xsplice.func use uintXX_t types instead of ELF ones. Remove the OUT on idx subfield. Mention that 'nr' being zero can be used for probing the number of payloads. Update what 'idx' means. v6: Update what 'idx' means again! Move the "Interdependencies section" to make it easier to in the design doc the movement of text (when the patch implements it). Add also 'version' field to payload. --- --- docs/misc/xsplice.markdown | 1044 1 file changed, 1044 insertions(+) create mode 100644 docs/misc/xsplice.markdown diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown new file mode 100644 index 000..d4e7d75 --- /dev/null +++ b/docs/misc/xsplice.markdown @@ -0,0 +1,1044 @@ +# xSplice Design v1 + +## Rationale + +A mechanism is required to binarily patch the running hypervisor with new +opcodes that have come about due to primarily security updates. + +This document describes the design of the API that would allow us to +upload to the hypervisor binary patches. + +The document is split in four sections: + + * Detailed descriptions of the problem statement. + * Design of the data structures. + * Design of the hypercalls. + * Implementation notes that should be taken into consideration. + + +## Glossary + + * splice - patch in the binary code with new opcodes + * trampoline - a jump to a new instruction. + * payload - telemetries of the old code along with binary blob of the new + function (if needed). + * reloc - telemetries contained in the payload to construct proper trampoline. + +## History + +The document has gone under various reviews and only covers v1 design. + +The end of the document has a section titled `Not Yet Done` which +outlines ideas and design for the future version of this work. + +## Multiple ways to patch + +The mechanism needs to be flexible to patch the hypervisor in multiple ways +and be as simple as possible. The compiled code is contiguous in memory with +no gaps - so we have no luxury of 'moving' existing code and must either +insert a trampoline to the new code to be executed - or only modify in-place +the code if there is sufficient space. The placement of new code has to be done +by hypervisor and the virtual address for the new code is allocated dynamically. + +This implies that the hypervisor must compute the new offsets when splicing +in the new trampoline code. Where the trampoline is added (inside +the function we are patching or just the callers?) is also important. + +To lessen the amount of code in hypervisor, the consumer of the API +is responsible for identifying which mechanism to employ and how many locations +to patch. Combinations of modifying in-place code, adding trampoline, etc +has to be supported. The API should allow read/write any memory within +the hypervisor virtual address space. + +We must also have a mechanism to query what has been applied and a mechanism +to revert it if needed. + +## Workflow + +The expected workflows of higher-level tools that manage multiple patches +on production machines would be: + + * The first obvious task is loading all available / suggested + hotpatches when they are available. + * Whenever new hotpatches are installed, they should be loaded too. + * One wants to query which modules have been loaded at runtime. + * If unloading is deemed safe (see unloading below), one may want to + support a workflow where a specific hotpatch is marked as bad and + unloaded. + +## Patching code + +The first mechanism to patch that comes in mind is in-place replacement. +That is replace the affected code with new code. Unfortunately the x86 +ISA is variable size which places limits on how much space we have available +to replace the instructions. T
[Xen-devel] [PATCH v7 18/24] HYPERCALL_version_op: Add VERSION_build_id to retrieve build-id.
The VERSION hypercall provides the flexibility to expose
the size of the build-id (so the callers can allocate the
proper size before trying to retrieve it). It also allows
in one nice swoop to retrieve the hypervisor build-id in the
provided buffer.
Signed-off-by: Konrad Rzeszutek Wilk
Acked-by: Daniel De Graaf
Acked-by: Jan Beulich
Reviewed-by: Andrew Cooper
---
Cc: Daniel De Graaf
Cc: Ian Jackson
Cc: Stefano Stabellini
Cc: Wei Liu
v4: New patch.
v5: Rebase - s/VERSION_OP/VERSION/
v6: Add Jan's Ack [provided the version_ops replacement does not need to
be reverted].
---
---
tools/flask/policy/policy/modules/xen/xen.te | 1 +
xen/common/kernel.c | 4
xen/include/public/version.h | 3 +++
xen/xsm/flask/hooks.c| 3 +++
xen/xsm/flask/policy/access_vectors | 2 ++
5 files changed, 13 insertions(+)
diff --git a/tools/flask/policy/policy/modules/xen/xen.te
b/tools/flask/policy/policy/modules/xen/xen.te
index 34fcfb9..28da3bc 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -83,6 +83,7 @@ allow dom0_t xen_t:version {
xen_extraversion xen_compile_info xen_capabilities
xen_changeset xen_pagesize xen_guest_handle xen_commandline
extraversion capabilities changeset pagesize guest_handle commandline
+build_id
};
allow dom0_t xen_t:mmu memorymap;
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index af2674d..14e14ad 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -474,6 +474,10 @@ DO(version_op)(unsigned int cmd,
XEN_GUEST_HANDLE_PARAM(void) arg,
ptr = saved_cmdline;
break;
+case XEN_VERSION_build_id:
+rc = xen_build_id(&ptr, &sz);
+break;
+
default:
rc = -ENOSYS;
}
diff --git a/xen/include/public/version.h b/xen/include/public/version.h
index 78961c9..3f3238f 100644
--- a/xen/include/public/version.h
+++ b/xen/include/public/version.h
@@ -157,6 +157,9 @@ DEFINE_XEN_GUEST_HANDLE(xen_version_op_val_t);
/* arg = char[]. Contains NUL terminated utf-8 string. */
#define XEN_VERSION_commandline 9
+/* arg = void. Contains binary value of hypervisor build-id. */
+#define XEN_VERSION_build_id10
+
#endif /* __XEN_PUBLIC_VERSION_H__ */
/*
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 14ca163..e199d57 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1697,6 +1697,9 @@ static int flask_version_op (uint32_t op)
case XEN_VERSION_commandline:
return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION,
VERSION__COMMANDLINE, NULL);
+case XEN_VERSION_build_id:
+return avc_has_perm(dsid, SECINITSID_XEN, SECCLASS_VERSION,
+VERSION__BUILD_ID, NULL);
default:
return -EPERM;
}
diff --git a/xen/xsm/flask/policy/access_vectors
b/xen/xsm/flask/policy/access_vectors
index 6c0ce7c..a1e40e6 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -540,4 +540,6 @@ class version
guest_handle
# Xen command line.
commandline
+# Build id of the hypervisor
+build_id
}
--
2.5.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 07/24] x86/mm: Introduce modify_xen_mappings()
From: Andrew Cooper
To simply change the permissions on existing Xen mappings. The existing
destroy_xen_mappings() is altered to support a change the PTE permissions.
A new destroy_xen_mappings() is introduced, as the special case of not passing
_PAGE_PRESENT to modify_xen_mappings().
As cleanup (and an ideal functional test), the boot logic which remaps Xen's
code and data with reduced permissions is altered to use
modify_xen_mappings(), rather than map_pages_to_xen() passing the same mfn's
back in.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: George Dunlap
---
xen/arch/x86/mm.c| 69
xen/arch/x86/setup.c | 22 +++--
xen/include/xen/mm.h | 2 ++
3 files changed, 70 insertions(+), 23 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index ca2d0bb..ccef946 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5956,7 +5956,17 @@ int populate_pt_range(unsigned long virt, unsigned long
mfn,
return map_pages_to_xen(virt, mfn, nr_mfns, MAP_SMALL_PAGES);
}
-void destroy_xen_mappings(unsigned long s, unsigned long e)
+/*
+ * Alter the permissions of a range of Xen virtual address space.
+ *
+ * Does not create new mappings, and does not modify the mfn in existing
+ * mappings, but will shatter superpages if necessary, and will destroy
+ * mappings if not passed _PAGE_PRESENT.
+ *
+ * It is an error to call with present flags over an unpopulated range. It is
+ * an error to try to modify flags other than NX, RW and PRESENT.
+ */
+void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
{
bool_t locking = system_state > SYS_STATE_boot;
l2_pgentry_t *pl2e;
@@ -5964,8 +5974,12 @@ void destroy_xen_mappings(unsigned long s, unsigned long
e)
unsigned int i;
unsigned long v = s;
+/* Set of valid PTE bits which may be altered. */
+#define FLAGS_MASK (_PAGE_NX|_PAGE_RW|_PAGE_PRESENT)
+
ASSERT(IS_ALIGNED(s, PAGE_SIZE));
ASSERT(IS_ALIGNED(e, PAGE_SIZE));
+ASSERT(!(nf & ~FLAGS_MASK));
while ( v < e )
{
@@ -5973,6 +5987,9 @@ void destroy_xen_mappings(unsigned long s, unsigned long
e)
if ( !(l3e_get_flags(*pl3e) & _PAGE_PRESENT) )
{
+/* Confirm the caller isn't trying to create new mappings. */
+ASSERT(!(nf & _PAGE_PRESENT));
+
v += 1UL << L3_PAGETABLE_SHIFT;
v &= ~((1UL << L3_PAGETABLE_SHIFT) - 1);
continue;
@@ -5984,8 +6001,12 @@ void destroy_xen_mappings(unsigned long s, unsigned long
e)
l1_table_offset(v) == 0 &&
((e - v) >= (1UL << L3_PAGETABLE_SHIFT)) )
{
-/* PAGE1GB: whole superpage is destroyed. */
-l3e_write_atomic(pl3e, l3e_empty());
+/* PAGE1GB: whole superpage is modified. */
+l3_pgentry_t nl3e = !(nf & _PAGE_PRESENT) ? l3e_empty()
+: l3e_from_pfn(l3e_get_pfn(*pl3e),
+ (l3e_get_flags(*pl3e) & ~FLAGS_MASK) | nf);
+
+l3e_write_atomic(pl3e, nl3e);
v += 1UL << L3_PAGETABLE_SHIFT;
continue;
}
@@ -6016,6 +6037,9 @@ void destroy_xen_mappings(unsigned long s, unsigned long
e)
if ( !(l2e_get_flags(*pl2e) & _PAGE_PRESENT) )
{
+/* Confirm the caller isn't trying to create new mappings. */
+ASSERT(!(nf & _PAGE_PRESENT));
+
v += 1UL << L2_PAGETABLE_SHIFT;
v &= ~((1UL << L2_PAGETABLE_SHIFT) - 1);
continue;
@@ -6026,8 +6050,12 @@ void destroy_xen_mappings(unsigned long s, unsigned long
e)
if ( (l1_table_offset(v) == 0) &&
((e-v) >= (1UL << L2_PAGETABLE_SHIFT)) )
{
-/* PSE: whole superpage is destroyed. */
-l2e_write_atomic(pl2e, l2e_empty());
+/* PSE: whole superpage is modified. */
+l2_pgentry_t nl2e = !(nf & _PAGE_PRESENT) ? l2e_empty()
+: l2e_from_pfn(l2e_get_pfn(*pl2e),
+ (l2e_get_flags(*pl2e) & ~FLAGS_MASK) | nf);
+
+l2e_write_atomic(pl2e, nl2e);
v += 1UL << L2_PAGETABLE_SHIFT;
}
else
@@ -6055,13 +6083,23 @@ void destroy_xen_mappings(unsigned long s, unsigned
long e)
}
else
{
+l1_pgentry_t nl1e;
+
/* Ordinary 4kB mapping. */
pl1e = l2e_to_l1e(*pl2e) + l1_table_offset(v);
-l1e_write_atomic(pl1e, l1e_empty());
+
+nl1e = !(nf & _PAGE_PRESENT) ? l1e_empty()
+: l1e_from_pfn(l1e_get_pfn(*pl1e),
+ (l1e_get_flags(*pl1e) & ~FLAGS_MASK) | nf);
+
+l1e_write_atomic(pl1e, nl1e);
v += PAGE_SIZE;
-/* If we are done with the L2E, check if it is now empty. */
-if
[Xen-devel] [PATCH v7 03/24] libxc: Implementation of XEN_XSPLICE_op in libxc
The underlaying toolstack code to do the basic
operations when using the XEN_XSPLICE_op syscalls:
- upload the payload,
- get status of an payload,
- list all the payloads,
- apply, check, replace, and revert the payload.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Ross Lagerwall
Acked-by: Wei Liu
Reviewed-by: Andrew Cooper
---
Cc: Ian Jackson
Cc: Stefano Stabellini
Cc: Wei Liu
v2: Actually set zero for the _pad entries.
v3: Split status into state and error code.
Add REPLACE action.
- Use timeout and utilize pads.
- Update per Wei's review.
- Extra space slipped in, remove it
v4: Add Wei's review, update comment and Ack.
v7: Sprinkle errno=-EINVAL on all the 'if (!len)', etc checks.
Added Reviewed-by from Andrew.
---
---
tools/libxc/include/xenctrl.h | 62
tools/libxc/xc_misc.c | 355 ++
2 files changed, 417 insertions(+)
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index f5a034a..a7937fa 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2640,6 +2640,68 @@ const uint32_t *xc_get_feature_deep_deps(uint32_t
feature);
#endif
+int xc_xsplice_upload(xc_interface *xch,
+ char *name, unsigned char *payload, uint32_t size);
+
+int xc_xsplice_get(xc_interface *xch,
+ char *name,
+ xen_xsplice_status_t *status);
+
+/*
+ * The heart of this function is to get an array of xen_xsplice_status_t.
+ *
+ * However it is complex because it has to deal with the hypervisor
+ * returning some of the requested data or data being stale
+ * (another hypercall might alter the list).
+ *
+ * The parameters that the function expects to contain data from
+ * the hypervisor are: 'info', 'name', and 'len'. The 'done' and
+ * 'left' are also updated with the number of entries filled out
+ * and respectively the number of entries left to get from hypervisor.
+ *
+ * It is expected that the caller of this function will take the
+ * 'left' and use the value for 'start'. This way we have an
+ * cursor in the array. Note that the 'info','name', and 'len' will
+ * be updated at the subsequent calls.
+ *
+ * The 'max' is to be provided by the caller with the maximum
+ * number of entries that 'info', 'name', and 'len' arrays can
+ * be filled up with.
+ *
+ * Each entry in the 'name' array is expected to be of XEN_XSPLICE_NAME_SIZE
+ * length.
+ *
+ * Each entry in the 'info' array is expected to be of xen_xsplice_status_t
+ * structure size.
+ *
+ * Each entry in the 'len' array is expected to be of uint32_t size.
+ *
+ * The return value is zero if the hypercall completed successfully.
+ * Note that the return value is _not_ the amount of entries filled
+ * out - that is saved in 'done'.
+ *
+ * If there was an error performing the operation, the return value
+ * will contain an negative -EXX type value. The 'done' and 'left'
+ * will contain the number of entries that had been succesfully
+ * retrieved (if any).
+ */
+int xc_xsplice_list(xc_interface *xch, unsigned int max, unsigned int start,
+xen_xsplice_status_t *info, char *name,
+uint32_t *len, unsigned int *done,
+unsigned int *left);
+
+/*
+ * The operations are asynchronous and the hypervisor may take a while
+ * to complete them. The `timeout` offers an option to expire the
+ * operation if it could not be completed within the specified time
+ * (in ms). Value of 0 means let hypervisor decide the best timeout.
+ */
+int xc_xsplice_apply(xc_interface *xch, char *name, uint32_t timeout);
+int xc_xsplice_revert(xc_interface *xch, char *name, uint32_t timeout);
+int xc_xsplice_unload(xc_interface *xch, char *name, uint32_t timeout);
+int xc_xsplice_check(xc_interface *xch, char *name, uint32_t timeout);
+int xc_xsplice_replace(xc_interface *xch, char *name, uint32_t timeout);
+
/* Compat shims */
#include "xenctrl_compat.h"
diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
index 7d997d9..8cd398b 100644
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -696,6 +696,361 @@ int xc_hvm_inject_trap(
return rc;
}
+int xc_xsplice_upload(xc_interface *xch,
+ char *name,
+ unsigned char *payload,
+ uint32_t size)
+{
+int rc;
+DECLARE_SYSCTL;
+DECLARE_HYPERCALL_BUFFER(char, local);
+DECLARE_HYPERCALL_BOUNCE(name, 0 /* later */,
XC_HYPERCALL_BUFFER_BOUNCE_IN);
+xen_xsplice_name_t def_name = { .pad = { 0, 0, 0 } };
+
+if ( !name || !payload )
+{
+errno = EINVAL;
+return -1;
+}
+
+def_name.size = strlen(name) + 1;
+if ( def_name.size > XEN_XSPLICE_NAME_SIZE )
+{
+errno = EINVAL;
+return -1;
+}
+
+HYPERCALL_BOUNCE_SET_SIZE(name, def_name.size);
+
+if ( xc_hypercall_bounce_pre(xch, name) )
+return -1;
+
+local = xc_hypercall_buffer_allo
[Xen-devel] [PATCH v7 09/24] xsplice: Implement payload loading
From: Ross Lagerwall
Add support for loading xsplice payloads. This is somewhat similar to
the Linux kernel module loader, implementing the following steps:
- Verify the elf file.
- Parse the elf file.
- Allocate a region of memory mapped within a free area of
[xen_virt_end, XEN_VIRT_END].
- Copy allocated sections into the new region. Split them in three
regions - .text, .data, and .rodata. MUST have at least .text.
- Resolve section symbols. All other symbols must be absolute addresses.
(Note that patch titled "xsplice,symbols: Implement symbol name resolution
on address" implements that)
- Perform relocations.
- Secure the the regions (.text,.data,.rodata) with proper permissions.
We capitalize on the vmalloc callback API (see patch titled:
"rm/x86/vmap: Add vmalloc_xen, vfree_xen and vm_init_type") to allocate
a region of memory within the [xen_virt_end, XEN_VIRT_END] for the code.
We also use the "x86/mm: Introduce modify_xen_mappings()"
to change the virtual address page-table permissions.
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
Acked-by: Julien Grall
---
Cc: Stefano Stabellini
Cc: Julien Grall
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v2: - Change the 'xsplice_patch_func' structure layout/size.
- Add more error checking. Fix memory leak.
- Move elf_resolve and elf_perform relocs in elf file.
- Print the payload address and pages in keyhandler.
v3:
- Make it build under ARM
- Build it without using the return_ macro.
- Add fixes from Ross.
- Add the _return macro back - but only use it during debug builds.
- Remove the macro, prefix arch_ on arch specific calls.
v4:
- Move alloc_payload to arch specific file.
- Use void* instead of uint8_t, use const
- Add copyrights
- Unroll the vmap code to add ASSERT. Change while to not incur
potential long error loop
- Use vmalloc/vfree cb APIs
- Secure .text pages to be RX instead of RWX.
v5:
- Fix allocation of virtual addresses only allowing one page to be allocated.
- Create .text, .data, and .rodata regions with different permissions.
- Make the find_space_t not a typedef to pointer to a function.
- Allocate memory in here.
v6: Drop parentheses on typedefs.
- s/an xSplice/a xSplice/
- Rebase on "vmap: Add vmalloc_cb"
- Rebase on "vmap: Add vmalloc_type and vm_init_type"
- s/uint8_t/void/ on load_addr
- Set xsplice_elf on stack without using memset.
v7:
- Changed the check on delta = elf->hdr->e_shoff + elf->hdr->e_shnum *
elf->hdr->e_shentsize;
The sections can be right at the back of the file (different linker!), so
the failing conditional
for 'if (delta >= elf->len)' is incorrect and should have been '>'.
- Changed dprintk(XENLOG_DEBUG to XENLOG_ERR, then back to DEBUG. Converted
some of the printk to dprintk.
- Rebase on " arm/x86/vmap: Add vmalloc_xen, vfree_xen and vm_init_type"
- Changed some of the printk XENLOG_ERR to XENLOG_DEBUG
- Check the idx in the relocation to make sure it is within bounds and
implemented.
- Use "x86/mm: Introduce modify_xen_mappings()"
- Introduce PRIxElfAddr
- Check for overflow in R_X86_64_PC32
- Return -EOPNOTSUPP if we don't support types in ELF64_R_TYPE
---
---
xen/arch/arm/Makefile | 1 +
xen/arch/arm/xsplice.c| 54 ++
xen/arch/x86/Makefile | 1 +
xen/arch/x86/xsplice.c| 188 +++
xen/common/xsplice.c | 226 +-
xen/common/xsplice_elf.c | 124 ++-
xen/include/xen/elfstructs.h | 4 +
xen/include/xen/xsplice.h | 29 ++
xen/include/xen/xsplice_elf.h | 9 +-
9 files changed, 631 insertions(+), 5 deletions(-)
create mode 100644 xen/arch/arm/xsplice.c
create mode 100644 xen/arch/x86/xsplice.c
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 0328b50..eae5cb3 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -40,6 +40,7 @@ obj-y += device.o
obj-y += decode.o
obj-y += processor.o
obj-y += smc.o
+obj-$(CONFIG_XSPLICE) += xsplice.o
#obj-bin-y += o
diff --git a/xen/arch/arm/xsplice.c b/xen/arch/arm/xsplice.c
new file mode 100644
index 000..4465244
--- /dev/null
+++ b/xen/arch/arm/xsplice.c
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2016 Citrix Systems R&D Ltd.
+ */
+#include
+#include
+#include
+#include
+
+int arch_xsplice_verify_elf(const struct xsplice_elf *elf)
+{
+return -ENOSYS;
+}
+
+int arch_xsplice_perform_rel(struct xsplice_elf *elf,
+ const struct xsplice_elf_sec *base,
+ const struct xsplice_elf_sec *rela)
+{
+return -ENOSYS;
+}
+
+int arch_xsplice_perform_rela(struct xsplice_elf *elf,
+ const struct xsplice_elf_sec *base,
+ const struct xsplice_elf_sec *rela)
+{
+return -ENOSYS;
+}
+
+
[Xen-devel] [PATCH v7 17/24] build_id: Provide ld-embedded build-ids
This patch enables the Elf to be built with the build-id and provide in the Xen hypervisor the code to extract it. The man-page for ld --build-id says it is: "Request the creation of a ".note.gnu.build-id" ELF note section or a ".build-id" COFF section. The contents of the note are unique bits identifying this linked file. style can be "uuid" to use 128 random bits, "sha1" to use a 160-bit SHA1 hash on the normative parts of the output contents, ..." One can also retrieve the value of the build-id by doing 'readelf -n xen-syms'. For EFI builds we re-use the same build-id that the xen-syms was built with. The version of ld that first implemented --build-id is v2.18. We check for to see if the linker supports the --build-id parameter and if so use it. For x86 we have two binaries - the xen-syms and the xen - an smaller version with lots of sections removed. To make it possible for readelf -n xen we also modify mkelf32 and xen.lds.S to include the PT_NOTE ELF section. The EFI binary is more complicated. We only build one type of binary and expanding the amount of sections the EFI binary has to include an .note one is pointless - as there is no concept of PT_NOTE. The best we can do is move this .note in the .rodata section. Further development wise should move it to .buildid section so that DataDirectory debug data nor CodeView can view it. (The author has no clue what those are). Note that in earlier patches the linker script had: __note_gnu_build_id_start = .; *(.rodata.note.gnu.build-id) __note_gnu_build_id_end = .; *(.note) *(.note.*) Which meant you could have different ELF notes _outside_ the __note_gnu_build_id_end. However for EFI builds we take the whole .note* section and jam it in the EFI to be between __note_gnu_build_id_start and __note_gnu_build_id_end. To not make this happend we make on the ELF build the section be called .note.gnu.build-id (instead of just .note). If there is a need for a different type of note other folks can add it as a different section name. Note that we do call --binary-id=sha1 on all linker invocations. We have to do to enforce that the symbol offsets don't changes (the side effect is that we we would have multiple binary ids - except that the last one is the final one). Without this working the symbol table embedded in Xen ends up incorrect - some of the values it contains would be offset by the size of the included build id. This obviously causes problems when resolving symbols. We also define the NT_GNU_BUILD_ID in the elfstructs.h as we need to use it in various places. Suggested-by: Andrew Cooper Signed-off-by: Martin Pohlack Signed-off-by: Konrad Rzeszutek Wilk Acked-by: Julien Grall --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Keir Fraser Cc: Jan Beulich Cc: Andrew Cooper v1: Rebase it on Martin's initial patch v2: Move it to XENVER hypercall v3: Fix EFI building (Ross's fix) Don't use the third argument for length. Use new structure for XENVER_build_id with variable buf. Include Ross's fix. Include detection of bin-utils for build-id support, add probing for size, and return -EPERM for XSM denied calls. Build xen_build_id under ARM, required adding ELFSIZE in proper file. Rebase on top XSM version class. v4: Include the build-id .note in the xen ELF binary. s/build_id/build_id_linker/ For EFI build, moved the --build-id values in .data section Rebase on staging. Split patch in two. Always do --build-id call. Include the .note in .rodata. USe const void * and ssize_t Use -S to make build_id.o and objcopy differently (Andrew suggested) v5: Put back the #ifdef LOCK_PROFILE on ARM. (Bad change). Move the _erodata around. s/ssize_t/unsigned int/ v6: Redid it per Jan's review. v7: Move build-id note in .rodata.note for EFI builds only. Move build-id note in .rodata for EFI builds only. Retain it in .note. Change name of object file used by EFI builds to notes.o Make on ELF builds the PT_NOTE section name be .note.gnu.build-id and ingest that in ELF build. Define NT_GNU_BUILD_ID in elfstructs.h --- --- Config.mk| 11 xen/arch/arm/Makefile| 2 +- xen/arch/arm/xen.lds.S | 15 - xen/arch/x86/Makefile| 30 -- xen/arch/x86/boot/mkelf32.c | 129 ++- xen/arch/x86/xen.lds.S | 28 ++ xen/common/version.c | 50 + xen/include/xen/elfstructs.h | 3 + xen/include/xen/version.h| 1 + 9 files changed, 247 insertions(+), 22 deletions(-) diff --git a/Config.mk b/Config.mk index 79eb2bd..db70638 100644 --- a/Config.mk +++ b/Config.mk @@ -126,6 +126,17 @@ endef check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1") $(eval $(check-y)) +ld-ver-build-id = $(shell $(1) --build-id 2>&1 | \ + grep -q unrecognized && echo n || echo y) + +export XEN_HAS_BUILD_ID
[Xen-devel] [PATCH v7 08/24] xsplice: Add helper elf routines
From: Ross Lagerwall
Add Elf routines and data structures in preparation for loading an
xSplice payload.
We make an assumption that the max number of sections an ELF payload
can have is 64. We can in future make this be dependent on the
names of the sections and verifying against a list, but for right now
this suffices.
Also we a whole lot of checks to make sure that the ELF payload
file is not corrupted nor that the offsets point past the file.
For most of the checks we print an message if the hypervisor is built
with debug enabled.
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Acked-by: Ian Jackson
Reviewed-by: Andrew Cooper
---
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Keir Fraser
Cc: Tim Deegan
v2: - With the #define ELFSIZE in the ARM file we can use the common
#defines instead of using #ifdef CONFIG_ARM_32. Moved to another
patch.
- Add checks for ELF file.
- Add name to be printed.
- Add len for easier ELF checks.
- Expand on the checks. Add macro.
v3: Remove the return_ macro
- Add return_ macro back but make it depend on debug=y
- Per Andrew review: add local variable. Fix memory leak in
elf_resolve_sections, Remove macro and use dprintk. Fix alignment.
Use void* instead of uint8_t to handle raw payload.
v4 - Fix memory leak in elf_get_sym
- Add XSPLICE to printk/dprintk
v5: Sprinkle newlines.
v6: Squash the ELF header checks from 'xsplice: Implement payload loading' here,
Do better job at checking string sections and the users of them (sh_size),
Use XSPLICE as a string literal,
Move some checks outside the loop,
Make sure that SHT_STRTAB are really what they say
Sprinkle consts.
v7:
Check sh_entsize and sh_offset.
Added Andrew's Reviewed-by and Ian's Acked-by
Redo check on sh_entsize to not be !=
---
---
xen/common/Makefile | 1 +
xen/common/xsplice_elf.c | 360 ++
xen/include/xen/xsplice.h | 3 +
xen/include/xen/xsplice_elf.h | 51 ++
4 files changed, 415 insertions(+)
create mode 100644 xen/common/xsplice_elf.c
create mode 100644 xen/include/xen/xsplice_elf.h
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 1e4bc70..afd84b6 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -59,6 +59,7 @@ obj-y += wait.o
obj-$(CONFIG_XENOPROF) += xenoprof.o
obj-y += xmalloc_tlsf.o
obj-$(CONFIG_XSPLICE) += xsplice.o
+obj-$(CONFIG_XSPLICE) += xsplice_elf.o
obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo
unlz4 earlycpio,$(n).init.o)
diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
new file mode 100644
index 000..ffa5c95
--- /dev/null
+++ b/xen/common/xsplice_elf.c
@@ -0,0 +1,360 @@
+/*
+ * Copyright (C) 2016 Citrix Systems R&D Ltd.
+ */
+
+#include
+#include
+#include
+#include
+
+const struct xsplice_elf_sec *xsplice_elf_sec_by_name(const struct xsplice_elf
*elf,
+ const char *name)
+{
+unsigned int i;
+
+for ( i = 1; i < elf->hdr->e_shnum; i++ )
+{
+if ( !strcmp(name, elf->sec[i].name) )
+return &elf->sec[i];
+}
+
+return NULL;
+}
+
+static int elf_verify_strtab(const struct xsplice_elf_sec *sec)
+{
+const Elf_Shdr *s;
+const uint8_t *contents;
+
+s = sec->sec;
+
+if ( s->sh_type != SHT_STRTAB )
+return -EINVAL;
+
+if ( !s->sh_size )
+return -EOPNOTSUPP;
+
+contents = (const uint8_t *)sec->data;
+
+if ( contents[0] || contents[s->sh_size - 1] )
+return -EINVAL;
+
+return 0;
+}
+
+static int elf_resolve_sections(struct xsplice_elf *elf, const void *data)
+{
+struct xsplice_elf_sec *sec;
+unsigned int i;
+Elf_Off delta;
+int rc;
+
+/* xsplice_elf_load sanity checked e_shnum. */
+sec = xmalloc_array(struct xsplice_elf_sec, elf->hdr->e_shnum);
+if ( !sec )
+{
+printk(XENLOG_ERR XSPLICE"%s: Could not allocate memory for section
table!\n",
+ elf->name);
+return -ENOMEM;
+}
+
+elf->sec = sec;
+
+delta = elf->hdr->e_shoff + elf->hdr->e_shnum * elf->hdr->e_shentsize;
+if ( delta > elf->len )
+{
+dprintk(XENLOG_DEBUG, XSPLICE "%s: Section table is past end of
payload!\n",
+elf->name);
+return -EINVAL;
+}
+
+for ( i = 1; i < elf->hdr->e_shnum; i++ )
+{
+delta = elf->hdr->e_shoff + i * elf->hdr->e_shentsize;
+
+sec[i].sec = (void *)data + delta;
+
+delta = sec[i].sec->sh_offset;
+
+/*
+ * N.B. elf_resolve_section_names, elf_get_sym skip this check as
+ * we do it here.
+ */
+if ( delta && (delta + sec[i].sec->sh_size > elf->len) )
+{
+dprintk(XENLOG_DEBUG, XSPLICE "%s: Section [%u] data is past end
of payload!\n",
+elf->name, i);
+return -EINVAL;
+}
+
+sec[i].
[Xen-devel] [PATCH v7 20/24] xsplice: Print build_id in keyhandler and on bootup.
As it should be an useful debug mechanism.
Signed-off-by: Konrad Rzeszutek Wilk
Acked-by: Jan Beulich
Reviewed-by: Andrew Cooper
--
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Keir Fraser
Cc: Tim Deegan
v2: s/char */const void *
v5: s/ssize_t/unsigned int/
v6: Remove pointless initializers, use string literal instead of %s,
add Jan's Ack.
v7: Add Andrew's Reviewed-by
---
---
xen/common/xsplice.c | 12
1 file changed, 12 insertions(+)
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index cbbb23d..21e56a5 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1329,10 +1330,15 @@ static const char *state2str(uint32_t state)
static void xsplice_printall(unsigned char key)
{
struct payload *data;
+const void *binary_id = NULL;
+unsigned int len = 0;
unsigned int i;
printk("'%u' pressed - Dumping all xsplice patches\n", key);
+if ( !xen_build_id(&binary_id, &len) )
+printk("build-id: %*phN\n", len, binary_id);
+
if ( !spin_trylock(&payload_lock) )
{
printk("Lock held. Try again.\n");
@@ -1365,11 +1371,17 @@ static void xsplice_printall(unsigned char key)
static int __init xsplice_init(void)
{
+const void *binary_id;
+unsigned int len;
+
BUILD_BUG_ON( sizeof(struct xsplice_patch_func) != 64 );
BUILD_BUG_ON( sizeof(struct xsplice_patch_func_internal) != 64 );
BUILD_BUG_ON( offsetof(struct xsplice_patch_func, new_addr) != 8 );
BUILD_BUG_ON( offsetof(struct xsplice_patch_func, new_size) != 24 );
+if ( !xen_build_id(&binary_id, &len) )
+printk(XENLOG_INFO XSPLICE ": build-id: %*phN\n", len, binary_id);
+
register_keyhandler('x', xsplice_printall, "print xsplicing info", 1);
arch_xsplice_init();
--
2.5.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 05/24] arm/x86: Use struct virtual_region to do bug, symbol, and (x86) exception tables lookup.
During execution of the hypervisor we have two regions of
executable code - stext -> _etext, and _sinittext -> _einitext.
The later is not needed after bootup.
We also have various built-in macros and functions to search
in between those two swaths depending on the state of the system.
That is either for bug_frames, exceptions (x86) or symbol
names for the instruction.
With xSplice in the picture - we need a mechanism for new payloads
to searched as well for all of this.
Originally we had extra 'if (xsplice)...' but that gets
a bit tiring and does not hook up nicely.
This 'struct virtual_region' and virtual_region_list provide a
mechanism to search for the bug_frames, exception table,
and symbol names entries without having various calls in
other sub-components in the system.
Code which wishes to participate in bug_frames and exception table
entries search has to only use two public APIs:
- register_virtual_region
- unregister_virtual_region
to let the core code know.
If the ->lookup_symbol is not then the default internal symbol lookup
mechanism is used.
Suggested-by: Andrew Cooper
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
Acked-by: Julien Grall [ARM]
---
Cc: Stefano Stabellini
Cc: Julien Grall
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v4: New patch.
v5:
- Rename to virtual_region.
- Ditch the 'skip' function.
- Remove the _stext.
- Use RCU lists.
- Add a search function.
- Remove extern, add rcu_read_lock. remove __ from name.
v6: s/search_for_text/find_text_region/.
- Drop the uaccess.h need. Make setup_virtual_regions accept two parameters.
Remove #ifdef.
- Constify struct exception_table_entry.
- Remove some newlines.
- Change header file guard #define to proper name.
---
---
xen/arch/arm/setup.c | 4 ++
xen/arch/arm/traps.c | 39 +++
xen/arch/x86/extable.c | 12 +++-
xen/arch/x86/setup.c | 6 ++
xen/arch/x86/traps.c | 40 ++-
xen/common/Makefile | 1 +
xen/common/symbols.c | 11 ++-
xen/common/virtual_region.c | 148 +++
xen/include/xen/symbols.h| 9 +++
xen/include/xen/virtual_region.h | 47 +
10 files changed, 280 insertions(+), 37 deletions(-)
create mode 100644 xen/common/virtual_region.c
create mode 100644 xen/include/xen/virtual_region.h
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 6d205a9..09ff1ea 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -860,6 +861,9 @@ void __init start_xen(unsigned long boot_phys_offset,
system_state = SYS_STATE_active;
+/* Must be done past setting system_state. */
+unregister_init_virtual_region();
+
domain_unpause_by_systemcontroller(dom0);
/* Switch on to the dynamically allocated stack for the idle vcpu
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 1516abd..4c423de 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -101,6 +102,8 @@ integer_param("debug_stack_lines", debug_stack_lines);
void init_traps(void)
{
+setup_virtual_regions(NULL, NULL);
+
/* Setup Hyp vector base */
WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
@@ -1116,27 +1119,33 @@ void do_unexpected_trap(const char *msg, struct
cpu_user_regs *regs)
int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc)
{
-const struct bug_frame *bug;
+const struct bug_frame *bug = NULL;
const char *prefix = "", *filename, *predicate;
unsigned long fixup;
-int id, lineno;
-static const struct bug_frame *const stop_frames[] = {
-__stop_bug_frames_0,
-__stop_bug_frames_1,
-__stop_bug_frames_2,
-NULL
-};
+int id = -1, lineno;
+const struct virtual_region *region;
-for ( bug = __start_bug_frames, id = 0; stop_frames[id]; ++bug )
+region = find_text_region(pc);
+if ( region )
{
-while ( unlikely(bug == stop_frames[id]) )
-++id;
+for ( id = 0; id < BUGFRAME_NR; id++ )
+{
+const struct bug_frame *b;
+unsigned int i;
-if ( ((vaddr_t)bug_loc(bug)) == pc )
-break;
+for ( i = 0, b = region->frame[id].bugs;
+ i < region->frame[id].n_bugs; b++, i++ )
+{
+if ( ((vaddr_t)bug_loc(b)) == pc )
+{
+bug = b;
+goto found;
+}
+}
+}
}
-
-if ( !stop_frames[id] )
+ found:
+if ( !bug )
return -ENOENT;
/* WARN, BUG or ASSERT: decode the filename pointer and line number. */
diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 89b5bcb..2a06cca
[Xen-devel] [PATCH v7 04/24] xen-xsplice: Tool to manipulate xsplice payloads
A simple tool that allows an system admin to perform
basic xsplice operations:
- Upload a xsplice file (with an unique name)
- List all the xsplice payloads loaded.
- Apply, revert, replace, or unload the payload using the
unique name.
- Do all two - upload, and apply the payload in one go (load).
Also will use the name of the file as the
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Ross Lagerwall
Acked-by: Wei Liu
---
Cc: Ian Jackson
Cc: Stefano Stabellini
Cc: Wei Liu
v2:
- Removed REVERTED state.
- Fixed bugs handling XSPLICE_STATUS_PROGRESS.
- Split status into state and error.
Add REPLACE action.
v3:
- Utilize the timeout and use the default one (let the hypervisor
pick it).
- Change the s/all/load and infer the from name of file.
- s/id/name/
- Don't use hypercall buffer in upload_func, instead do it in libxc
- Remove the debug printk.
- Remove goto's (per Wei's review)
- Use fprintf(stderr in error paths.
- Add local variable block.
- Syntax, expand comment, and don't overwrite rc if xc_xsplice_upload failed.
v4:
- Remove LOADED state. Only have CHECKED state.
---
---
.gitignore | 1 +
tools/misc/Makefile | 4 +
tools/misc/xen-xsplice.c | 463 +++
3 files changed, 468 insertions(+)
create mode 100644 tools/misc/xen-xsplice.c
diff --git a/.gitignore b/.gitignore
index 20ffa2d..39eb779 100644
--- a/.gitignore
+++ b/.gitignore
@@ -182,6 +182,7 @@ tools/misc/xen_cpuperf
tools/misc/xen-cpuid
tools/misc/xen-detect
tools/misc/xen-tmem-list-parse
+tools/misc/xen-xsplice
tools/misc/xenperf
tools/misc/xenpm
tools/misc/xen-hvmctx
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index a94dad9..3a5f842 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -32,6 +32,7 @@ INSTALL_SBIN += xenlockprof
INSTALL_SBIN += xenperf
INSTALL_SBIN += xenpm
INSTALL_SBIN += xenwatchdogd
+INSTALL_SBIN += xen-xsplice
INSTALL_SBIN += $(INSTALL_SBIN-y)
# Everything to be installed in a private bin/
@@ -103,6 +104,9 @@ xen-mfndump: xen-mfndump.o
xenwatchdogd: xenwatchdogd.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+xen-xsplice: xen-xsplice.o
+ $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+
xen-lowmemd: xen-lowmemd.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl)
$(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
diff --git a/tools/misc/xen-xsplice.c b/tools/misc/xen-xsplice.c
new file mode 100644
index 000..fb9228e
--- /dev/null
+++ b/tools/misc/xen-xsplice.c
@@ -0,0 +1,463 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static xc_interface *xch;
+
+void show_help(void)
+{
+fprintf(stderr,
+"xen-xsplice: Xsplice test tool\n"
+"Usage: xen-xsplice [args]\n"
+" An unique name of payload. Up to %d characters.\n"
+"Commands:\n"
+" help display this help\n"
+" upload upload file with name\n"
+" list list payloads uploaded.\n"
+" applyapply patch.\n"
+" revert revert name patch.\n"
+" replace apply patch and revert all
others.\n"
+" unload unload name patch.\n"
+" load upload, check and apply .\n"
+" name is the name\n",
+XEN_XSPLICE_NAME_SIZE);
+}
+
+/* wrapper function */
+static int help_func(int argc, char *argv[])
+{
+show_help();
+return 0;
+}
+
+#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0]))
+
+static const char *state2str(unsigned int state)
+{
+#define STATE(x) [XSPLICE_STATE_##x] = #x
+static const char *const names[] = {
+STATE(CHECKED),
+STATE(APPLIED),
+};
+#undef STATE
+if (state >= ARRAY_SIZE(names) || !names[state])
+return "unknown";
+
+return names[state];
+}
+
+/* This value was choosen adhoc. It could be 42 too. */
+#define MAX_LEN 11
+static int list_func(int argc, char *argv[])
+{
+unsigned int idx, done, left, i;
+xen_xsplice_status_t *info = NULL;
+char *name = NULL;
+uint32_t *len = NULL;
+int rc = ENOMEM;
+
+if ( argc )
+{
+show_help();
+return -1;
+}
+idx = left = 0;
+info = malloc(sizeof(*info) * MAX_LEN);
+if ( !info )
+return rc;
+name = malloc(sizeof(*name) * XEN_XSPLICE_NAME_SIZE * MAX_LEN);
+if ( !name )
+{
+free(info);
+return rc;
+}
+len = malloc(sizeof(*len) * MAX_LEN);
+if ( !len ) {
+free(name);
+free(info);
+return rc;
+}
+
+fprintf(st
[Xen-devel] [PATCH v7 10/24] xsplice: Implement support for applying/reverting/replacing patches.
From: Ross Lagerwall
Implement support for the apply, revert and replace actions.
To perform and action on a payload, the hypercall sets up a data
structure to schedule the work. A hook is added in the reset_stack_and_jump
to check for work and execute it if needed (specifically we check an
per-cpu flag to make this as quick as possible).
In this way, patches can be applied with all CPUs idle and without
stacks. The first CPU to run check_for_xsplice_work() becomes the
master and triggers a reschedule softirq to trigger all the other CPUs
to enter check_for_xsplice_work() with no stack. Once all CPUs
have rendezvoused, all CPUs disable IRQs and NMIs are ignored.
The system is then quiscient and the master performs the action.
After this, all CPUs enable IRQs and NMIs are re-enabled.
Note that it is unsafe to patch do_nmi and the xSplice internal functions.
Patching functions on NMI/MCE path is liable to end in disaster.
This is not addressed in this patch and is mentioned in the
design doc as a further TODO.
The action to perform is one of:
- APPLY: For each function in the module, store the first 5 bytes of the
old function and replace it with a jump to the new function.
- REVERT: Copy the previously stored bytes into the first 5 bytes of the
old function.
- REPLACE: Revert each applied module and then apply the new module.
To prevent a deadlock with any other barrier in the system, the master
will wait for up to 30ms before timing out.
Measurements found that the patch application to take about 100 μs on a
72 CPU system, whether idle or fully loaded.
We also add an BUILD_ON to make sure that the size of the structure
of the payload is not inadvertly changed and that the offsets are
correct on both 32 and 64-bit hypervisor (ARM32 and ARM64).
To make this work, we make the public header use architecture
neutral members (xen_ulong_t) but for per-architecture have to
fiddle with padding.
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Acked-by: Julien Grall [ARM bits]
--
Cc: Stefano Stabellini
Cc: Julien Grall
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
Cc: Boris Ostrovsky
Cc: Suravee Suthikulpanit
Cc: Jun Nakajima
Cc: Kevin Tian
v2: - Pluck the 'struct xsplice_patch_func' in this patch.
- Modify code per review comments.
- Add more data in the keyboard handler.
- Redo the patching code, split it in functions.
v3: - Add return_ macro for debug builds.
- Move s/payload_list_lock/payload_list/ to earlier patch
- Remove const and use ELF types for xsplice_patch_func
- Add check routine to do simple sanity checks for various
sections.
- s/%p/PRIx64/ as ARM builds complain.
- Move code around. Add more dprintk. Add XSPLICE in front of all
printks/dprintk.
Put the NMIs back if we fail patching.
Add per-cpu to lessen contention for global structure.
Extract from xsplice_do_single patching code into xsplice_do_action
Squash xsplice_do_single and check_for_xsplice_work together to
have all rendezvous in one place.
Made XSPLICE_ACTION_REPLACE work again (wrong list iterator)
s/find_special_sections/prepare_payload/
Use list_del_init and INIT_LIST_HEAD for applied_list
v4:
- Add comment, adjust spacing for "Timed out on CPU semaphore"
- Added CR0.WP manipulations when altering the .text of hypervisor.
- Added fix from Andrew for CR0.WP manipulation.
v5: - Made xsplice_patch_func use uintXX_t instead of ELF_ types to easy
making it work under ARM (32bit). Add more BUILD-BUG-ON checks.
- Add more BUILD_ON checks. Sprinkle newlines.
v6: Rebase on "arm/x86: Alter nmi_callback_t typedef"
- Drop the recursive spinlock usage.
- Move NMI callbacks in arch specific.
- Fold the 'check_for_xsplice_work' in reset_stack_and_jump
- Add arch specific check for .xsplice.funcs.
- Seperate external and internal structure of .xsplice.funcs.
- Changed per Jan's review
- Modified the .xsplice.funcs checks
v7:
- Modified old_ptr to void* instead of uint8_t*
- Modified the xsplice_patch_func_internal for ARM32 to have padding.
- Used #if BITS_PER_LONG == 64 for the xsplice_patch_func_internal along
with ifndef CONFIG_ARM for the undo (which may be different size on ARM64)
---
---
xen/arch/arm/xsplice.c| 33 +++
xen/arch/x86/domain.c | 2 +
xen/arch/x86/xsplice.c| 76 +++
xen/common/xsplice.c | 461 +-
xen/include/asm-x86/current.h | 10 +-
xen/include/public/sysctl.h | 18 ++
xen/include/xen/xsplice.h | 58 ++
7 files changed, 648 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/xsplice.c b/xen/arch/arm/xsplice.c
index 4465244..5491317 100644
--- a/xen/arch/arm/xsplice.c
+++ b/xen/arch/arm/xsplice.c
@@ -6,6 +6,39 @@
#include
#include
+void arch_xsplice_patching_enter(void)
+{
+}
+
+void arch_xsplice_patching_leave(void)
+{
+}
+
+int arc
[Xen-devel] [PATCH v7 02/24] xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op
The implementation does not actually do any patching.
It just adds the framework for doing the hypercalls,
keeping track of ELF payloads, and the basic operations:
- query which payloads exist,
- query for specific payloads,
- check*1, apply*1, replace*1, and unload payloads.
*1: Which of course in this patch are nops.
The functionality is disabled on ARM until all arch
components are implemented.
Also by default it is disabled until the implementation
is in place.
We also use recursive spinlocks to so that the find_payload
function does not need to have a 'lock' and 'non-lock' variant.
Signed-off-by: Konrad Rzeszutek Wilk
Signed-off-by: Ross Lagerwall
Reviewed-by: Andrew Cooper
Acked-by: Daniel De Graaf
---
Cc: Daniel De Graaf
Cc: Ian Jackson
Cc: Stefano Stabellini
Cc: Wei Liu
v2: Rebased on keyhandler: rework keyhandler infrastructure
v3: Fixed XSM.
- Removed REVERTED state.
Split status and error code.
Add REPLACE action.
Separate payload data from the payload structure.
s/XSPLICE_ID_../XSPLICE_NAME_../
- Add xsplice and CONFIG_XSPLICE build toption.
Fix code per Jan's review.
Update the sysctl.h (change bits to enum like)
- Rebase on Kconfig changes.
- Add missing pad checks. Re-order keyhandler.h to build on ARM.
- Rebase on build: hook the schedulers into Kconfig
- s/id/name/; s/payload_list_lock/payload_lock/
- Put #ifdef CONFIG_XSPLICE in header file per Doug review.
- Andrew review:
- use recursive spinlocks, change name to xsplice_op,
sprinkle new-lines, add local variable block, include
state diagram, squash two goto labels, use vzalloc instead of
alloc_xenheap_pages.
- change 'state' from int32 to uint32_t
- remove the err label out of xsplice_upload
- use void* instaed of uint8_t
- move code around to make it easier to read.
- Add vmap.h to compiler under ARM.
- Add missing Copyright in header file
- Dropped LOADED state, make the payload go in CHECKED.
v4: Made it only work on x86 per Julien's (ARM) maintainer request.
v5: Dropped the load->check state example in sysctl.h
Made the ->nr=0 call work. Remove rc=0 in lots of cases. Update
header from design doc.
v6: Update what 'idx' means. Don't drop lock in find_payload. Make
find_name copy data.
v7: Don't print -EINVAL when payload_cnt is zero (and toolstack provides
idx as zero). Change return code to -ENOSYS, so change callback setting
based on -ENOSYS and -EOPNOTSUPP.
Add extra printk in keyhandler.
Use if (..) else in xsplice_upload instead of two 'if'.
Remove #ifdef in XSM machinery.
Add Andrew's Reviewed-by.
Rebase on x86/cpu: Sysctl and common infrastructure for levelling context
switching
and xen+tools: Export maximum host and guest cpu featuresets via SYSCTL
---
---
tools/flask/policy/policy/modules/xen/xen.te | 1 +
xen/common/Kconfig | 12 +
xen/common/Makefile | 1 +
xen/common/sysctl.c | 6 +
xen/common/xsplice.c | 403 +++
xen/include/public/sysctl.h | 166 +++
xen/include/xen/xsplice.h| 35 +++
xen/xsm/flask/hooks.c| 4 +
xen/xsm/flask/policy/access_vectors | 2 +
9 files changed, 630 insertions(+)
create mode 100644 xen/common/xsplice.c
create mode 100644 xen/include/xen/xsplice.h
diff --git a/tools/flask/policy/policy/modules/xen/xen.te
b/tools/flask/policy/policy/modules/xen/xen.te
index a551756..34fcfb9 100644
--- a/tools/flask/policy/policy/modules/xen/xen.te
+++ b/tools/flask/policy/policy/modules/xen/xen.te
@@ -74,6 +74,7 @@ allow dom0_t xen_t:xen2 {
get_symbol
get_cpu_levelling_caps
get_cpu_featureset
+xsplice_op
};
# Allow dom0 to use all XENVER_ subops and VERSION subops that have checks.
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index ad9f7bf..fea33d3 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -188,4 +188,16 @@ config SCHED_DEFAULT
endmenu
+# Enable/Disable xsplice support
+config XSPLICE
+ bool "xSplice live patching support"
+ default n
+ depends on X86
+ ---help---
+ Allows a running Xen hypervisor to be dynamically patched using
+ binary patches without rebooting. This is primarily used to binarily
+ patch in the field an hypervisor with XSA fixes.
+
+ If unsure, say Y.
+
endmenu
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 77de27e..910ac69 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -57,6 +57,7 @@ obj-y += vsprintf.o
obj-y += wait.o
obj-$(CONFIG_XENOPROF) += xenoprof.o
obj-y += xmalloc_tlsf.o
+obj-$(CONFIG_XSPLICE) += xsplice.o
obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo
unlz4 earlycpio,$(n).init.o)
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 253b7c8..d04d529 1
[Xen-devel] [PATCH v7 23/24] xsplice: Prevent duplicate payloads from being loaded.
From: Ross Lagerwall
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
---
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Keir Fraser
Cc: Tim Deegan
v6: Drop recursive lock - also now the caller is holding the lock
Move the code up in the code above.
v7: Add Andrew's Reviewed-by
---
---
xen/common/xsplice.c | 16
1 file changed, 16 insertions(+)
diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
index a0cc345..ab306d4 100644
--- a/xen/common/xsplice.c
+++ b/xen/common/xsplice.c
@@ -514,7 +514,9 @@ static int prepare_payload(struct payload *payload,
sec = xsplice_elf_sec_by_name(elf, ".note.gnu.build-id");
if ( sec )
{
+struct payload *data;
n = sec->load_addr;
+
if ( sec->sec->sh_size <= sizeof(*n) )
return -EINVAL;
@@ -524,6 +526,20 @@ static int prepare_payload(struct payload *payload,
if ( !payload->id.len || !payload->id.p )
return -EINVAL;
+
+/* Make sure it is not a duplicate. */
+list_for_each_entry ( data, &payload_list, list )
+{
+/* No way _this_ payload is on the list. */
+ASSERT(data != payload);
+if ( data->id.len &&
+ !memcmp(data->id.p, payload->id.p, data->id.len) )
+{
+dprintk(XENLOG_DEBUG, XSPLICE "%s: Already loaded as %s!\n",
+elf->name, data->name);
+return -EEXIST;
+}
+}
}
sec = xsplice_elf_sec_by_name(elf, ".xsplice.depends");
--
2.5.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 19/24] libxl: info: Display build_id of the hypervisor using XEN_VERSION_build_id
If the hypervisor is built with we will display it.
Signed-off-by: Konrad Rzeszutek Wilk
Acked-by: Wei Liu
---
Cc: Ian Jackson
Cc: Stefano Stabellini
Cc: Wei Liu
v2: Include HAVE_*, use libxl_zalloc, s/rc/ret/
v3: Retry with different size if 1020 is not enough.
v4: Use VERSION_OP subops instead of the XENVER_ subops
v5: Change it per Wei's review. s/VERSION_OP/VERSION/
And actually use the proper Style!
---
---
tools/libxl/libxl.c | 18 --
tools/libxl/libxl.h | 6 ++
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c| 1 +
4 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index d232473..ab6acac 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5374,6 +5374,7 @@ const libxl_version_info*
libxl_get_version_info(libxl_ctx *ctx)
GC_INIT(ctx);
char *buf;
xen_version_op_val_t val = 0;
+int r;
libxl_version_info *info = &ctx->version_info;
if (info->xen_version_extra != NULL)
@@ -5416,8 +5417,21 @@ const libxl_version_info*
libxl_get_version_info(libxl_ctx *ctx)
info->virt_start = val;
-(void)libxl__xc_version_wrapper(gc, XEN_VERSION_commandline, buf,
-info->pagesize, &info->commandline);
+if (libxl__xc_version_wrapper(gc, XEN_VERSION_commandline, buf,
+ info->pagesize, &info->commandline) < 0)
+goto out;
+
+r = xc_version(ctx->xch, XEN_VERSION_build_id, buf, info->pagesize);
+if (r < 0) {
+info->build_id = libxl__strdup(NOGC, "");
+} else if (r > 0) {
+unsigned int i;
+
+info->build_id = libxl__zalloc(NOGC, (r * 2) + 1);
+
+for (i = 0; i < r; i++)
+snprintf(&info->build_id[i * 2], 3, "%02hhx", buf[i]);
+}
out:
GC_FREE;
return info;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 3fc5c48..a68f698 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -247,6 +247,12 @@
#define LIBXL_HAVE_APIC_ASSIST 1
/*
+ * LIBXL_HAVE_BUILD_ID means that libxl_version_info has the extra
+ * field for the hypervisor build_id.
+ */
+#define LIBXL_HAVE_BUILD_ID 1
+
+/*
* libxl ABI compatibility
*
* The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index c3161f3..9840f3b 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -365,6 +365,7 @@ libxl_version_info = Struct("version_info", [
("virt_start",uint64),
("pagesize", integer),
("commandline", string),
+("build_id", string),
], dir=DIR_OUT)
libxl_domain_create_info = Struct("domain_create_info",[
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 6346017..ac7d759 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5920,6 +5920,7 @@ static void output_xeninfo(void)
printf("cc_compile_by : %s\n", info->compile_by);
printf("cc_compile_domain : %s\n", info->compile_domain);
printf("cc_compile_date: %s\n", info->compile_date);
+printf("build_id : %s\n", info->build_id);
return;
}
--
2.5.0
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v7 16/24] xsplice: Add support for alternatives
From: Ross Lagerwall
Add support for applying alternative sections within xsplice payload.
At payload load time, apply an alternative sections that are found.
Also we add an test-case exercising a rather useless alternative
(patching a NOP with a NOP) - but it does exercise the code-path.
Signed-off-by: Ross Lagerwall
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
---
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v2: Make a new alternative function that does not ASSERT on IRQs and
don't disable IRQs in the code when loading payload.
v4: Include test-case
Include check for size of alternatives and that it is not a 0 size
section.
v6: Add #define INIT to preserve __initness on alternative code.
Double check that alt_instr are only patching payload code.
v7: Move cr0 manipulation in apply_alternatives.
ifdef around alternative.o in Makefile
Pick X86_FEATURE_LM in test-case
Drop casting from load_addr
It is alternative.init.o, not alternative_init.o (thanks Andrew!)
---
---
xen/arch/x86/Makefile| 4 +++
xen/arch/x86/alternative.c | 42
xen/arch/x86/test/xen_hello_world_func.c | 5
xen/common/xsplice.c | 34 ++
xen/include/asm-x86/alternative.h| 4 +++
5 files changed, 74 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 57c93e1..d210bb7 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -6,7 +6,11 @@ subdir-y += mm
subdir-$(CONFIG_XENOPROF) += oprofile
subdir-y += x86_64
+ifdef CONFIG_XSPLICE
+obj-y += alternative.o
+else
obj-bin-y += alternative.init.o
+endif
obj-y += apic.o
obj-y += bitops.o
obj-bin-y += bzimage.init.o
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index f735ff8..366ad86 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -22,13 +22,14 @@
#include
#include
#include
+#include
#define MAX_PATCH_LEN (255-1)
extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
#ifdef K8_NOP1
-static const unsigned char k8nops[] __initconst = {
+static const unsigned char k8nops[] __INITCONST = {
K8_NOP1,
K8_NOP2,
K8_NOP3,
@@ -52,7 +53,7 @@ static const unsigned char * const k8_nops[ASM_NOP_MAX+1]
__initconstrel = {
#endif
#ifdef P6_NOP1
-static const unsigned char p6nops[] __initconst = {
+static const unsigned char p6nops[] __INITCONST = {
P6_NOP1,
P6_NOP2,
P6_NOP3,
@@ -75,7 +76,7 @@ static const unsigned char * const p6_nops[ASM_NOP_MAX+1]
__initconstrel = {
};
#endif
-static const unsigned char * const *ideal_nops __initdata = k8_nops;
+static const unsigned char * const *ideal_nops __INITDATA = k8_nops;
static int __init mask_nmi_callback(const struct cpu_user_regs *regs, int cpu)
{
@@ -100,7 +101,7 @@ static void __init arch_init_ideal_nops(void)
}
/* Use this to add nops to a buffer, then text_poke the whole buffer. */
-static void __init add_nops(void *insns, unsigned int len)
+static void __INIT add_nops(void *insns, unsigned int len)
{
while ( len > 0 )
{
@@ -114,7 +115,7 @@ static void __init add_nops(void *insns, unsigned int len)
}
/*
- * text_poke_early - Update instructions on a live kernel at boot time
+ * text_poke - Update instructions on a live kernel or non-executed code.
* @addr: address to modify
* @opcode: source of the copy
* @len: length to copy
@@ -125,9 +126,10 @@ static void __init add_nops(void *insns, unsigned int len)
* instructions. And on the local CPU you need to be protected again NMI or MCE
* handlers seeing an inconsistent instruction while you patch.
*
- * This routine is called with local interrupt disabled.
+ * You should run this with interrupts disabled or on code that has never
+ * been executed.
*/
-static void *__init text_poke_early(void *addr, const void *opcode, size_t len)
+static void *__INIT text_poke(void *addr, const void *opcode, size_t len)
{
memcpy(addr, opcode, len);
sync_core();
@@ -142,20 +144,14 @@ static void *__init text_poke_early(void *addr, const
void *opcode, size_t len)
* APs have less capabilities than the boot processor are not handled.
* Tough. Make sure you disable such features by hand.
*/
-static void __init apply_alternatives(struct alt_instr *start, struct
alt_instr *end)
+void __INIT apply_alternatives_nocheck(struct alt_instr *start, struct
alt_instr *end)
{
struct alt_instr *a;
u8 *instr, *replacement;
u8 insnbuf[MAX_PATCH_LEN];
-unsigned long cr0 = read_cr0();
-
-ASSERT(!local_irq_is_enabled());
printk(KERN_INFO "alt table %p -> %p\n", start, end);
-/* Disable WP to allow application of alternatives to read-only pages. */
-write_cr0(cr0 & ~X86_CR0_WP);
-
/*
* The scan order should be from start to end. A later scanned
* alternative code can overw
[Xen-devel] [PATCH v7 22/24] xsplice/xen_replace_world: Test-case for XSPLICE_ACTION_REPLACE
With this third payload one can do:
-bash-4.1# xen-xsplice load xen_hello_world.xsplice
Uploading xen_hello_world.xsplice (10148 bytes)
Performing check: completed
Performing apply:. completed
[xen_hello_world depends on hypervisor build-id]
-bash-4.1# xen-xsplice load xen_bye_world.xsplice
Uploading xen_bye_world.xsplice (7076 bytes)
Performing check: completed
Performing apply:. completed
[xen_bye_world depends on xen_hello_world build-id]
-bash-4.1# xen-xsplice upload xen_replace_world xen_replace_world.xsplice
Uploading xen_replace_world.xsplice (7148 bytes)
-bash-4.1# xen-xsplice list
ID | status
+
xen_hello_world | APPLIED
xen_bye_world | APPLIED
xen_replace_world | CHECKED
-bash-4.1# xen-xsplice replace xen_replace_world
Performing replace:. completed
-bash-4.1# xl info | grep extra
xen_extra : Hello Again World!
-bash-4.1# xen-xsplice list
ID | status
+
xen_hello_world | CHECKED
xen_bye_world | CHECKED
xen_replace_world | APPLIED
and revert both of the previous payloads and apply
the xen_replace_world.
All the magic of this is in the Makefile - we extract
the build-id from the hypervisor (xen-syms) and jam it
in the xen_replace_world as .xsplice.depends.
Signed-off-by: Konrad Rzeszutek Wilk
Reviewed-by: Andrew Cooper
---
Cc: Keir Fraser
Cc: Jan Beulich
Cc: Andrew Cooper
v4: New. Make the objcopy use -S to strip the name.
v7: Added Andrew's Reviewed-by
---
---
.gitignore | 1 +
xen/arch/x86/test/Makefile | 8 +++
xen/arch/x86/test/xen_replace_world.c | 34 ++
xen/arch/x86/test/xen_replace_world_func.c | 24 +
4 files changed, 67 insertions(+)
create mode 100644 xen/arch/x86/test/xen_replace_world.c
create mode 100644 xen/arch/x86/test/xen_replace_world_func.c
diff --git a/.gitignore b/.gitignore
index 88cec1d..1b73293 100644
--- a/.gitignore
+++ b/.gitignore
@@ -249,6 +249,7 @@ xen/arch/x86/efi/mkreloc
xen/arch/x86/test/config.h
xen/arch/x86/test/xen_hello_world.xsplice
xen/arch/x86/test/xen_bye_world.xsplice
+xen/arch/x86/test/xen_replace_world.xsplice
xen/arch/*/efi/boot.c
xen/arch/*/efi/compat.c
xen/arch/*/efi/efi.h
diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile
index 7d73184..5310b94 100644
--- a/xen/arch/x86/test/Makefile
+++ b/xen/arch/x86/test/Makefile
@@ -7,15 +7,18 @@ CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{
print "0x"$$2}')
XSPLICE := xen_hello_world.xsplice
XSPLICE_BYE := xen_bye_world.xsplice
+XSPLICE_REPLACE := xen_replace_world.xsplice
default: xsplice
install: xsplice
$(INSTALL_DATA) $(XSPLICE) $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE)
$(INSTALL_DATA) $(XSPLICE_BYE) $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_BYE)
+ $(INSTALL_DATA) $(XSPLICE_REPLACE)
$(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_REPLACE)
uninstall:
rm -f $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE)
rm -f $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_BYE)
+ rm -f $(DESTDIR)$(DEBUG_DIR)/$(XSPLICE_REPLACE)
.PHONY: clean
clean::
@@ -75,3 +78,8 @@ xsplice: config.h note.o
$(MAKE) -f $(BASEDIR)/Rules.mk hello_world_note.o
$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE_BYE) \
xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+ $(MAKE) -f $(BASEDIR)/Rules.mk xen_replace_world_func.o
+ $(MAKE) -f $(BASEDIR)/Rules.mk xen_replace_world.o
+ $(LD) $(LDFLAGS) $(build_id_linker) -r -o $(XSPLICE_REPLACE) \
+xen_replace_world_func.o \
+xen_replace_world.o note.o
diff --git a/xen/arch/x86/test/xen_replace_world.c
b/xen/arch/x86/test/xen_replace_world.c
new file mode 100644
index 000..70516bc
--- /dev/null
+++ b/xen/arch/x86/test/xen_replace_world.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include
+#include
+#include
+
+static char xen_replace_world_name[] = "xen_extra_version";
+extern const char *xen_replace_world(void);
+
+/* External symbol. */
+extern const char *xen_extra_version(void);
+
+struct xsplice_patch_func __section(".xsplice.funcs")
xsplice_xen_replace_world = {
+.version = XSPLICE_PAYLOAD_VERSION,
+.name = xen_replace_world_name,
+.new_addr = (unsigned long)(xen_replace_world),
+.old_addr = (unsigned long)(xen_extra_version),
+.new_size = NEW_CODE_SZ,
+.old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/test/xen_replace_world_func.c
b/xen/arch/x86/test/xen_replace_worl
[Xen-devel] [xen-unstable-smoke test] 90828: tolerable FAIL - PUSHED
flight 90828 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/90828/
Failures :-/ but no regressions.
Regressions which are regarded as allowable (not blocking):
build-amd64-libvirt 5 libvirt-buildfail like 89421
Tests which did not succeed, but are not blocking:
test-amd64-amd64-libvirt 1 build-check(1) blocked n/a
test-armhf-armhf-xl 12 migrate-support-checkfail never pass
test-armhf-armhf-xl 13 saverestore-support-checkfail never pass
version targeted for testing:
xen 3dac42fef86a79be209e585ae323fccb240bfc2e
baseline version:
xen a35dc6ccbb5c77f83dd03f6de0e58e71f805debf
Last test of basis89421 2016-04-07 23:05:13 Z3 days
Testing same since90828 2016-04-10 22:25:33 Z0 days1 attempts
People who touched revisions under test:
Andrew Cooper
Chunyan Liu
Daniel De Graaf
Dario Fagggioli
Dario Faggioli
Dasaratharaman Chandramouli
David Scott
Fu Wei
George Dunlap
Harmandeep Kaur
Ian Campbell
Ian Jackson
Jan Beulich
Julien Grall
Justin Weaver
Konrad Rzeszutek Wilk
Len Brown
Micah Barany
Olaf Hering
Robert VanVossen
Roger Pau Monne
Roger Pau Monné
Shanker Donthineni
Shannon Zhao
Uma Sharma
Vikram Sethi
Wei Liu
jobs:
build-amd64 pass
build-armhf pass
build-amd64-libvirt fail
test-armhf-armhf-xl pass
test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
test-amd64-amd64-libvirt blocked
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Pushing revision :
+ branch=xen-unstable-smoke
+ revision=3dac42fef86a79be209e585ae323fccb240bfc2e
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke
3dac42fef86a79be209e585ae323fccb240bfc2e
+ branch=xen-unstable-smoke
+ revision=3dac42fef86a79be209e585ae323fccb240bfc2e
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
getconfig Repos
perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-4.6-testing
+ '[' x3dac42fef86a79be209e585ae323fccb240bfc2e = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo h
[Xen-devel] [PATCH] Fix cpumap setting before passing to XEN
It's tool's duty to pass a correct cpumap to XEN. On a host with less
than 64
CPUS, it just shows below error.
[root@localhost /]# xm vcpu-pin 3 all all
Error: Cannot pin vcpu: 0 to cpu: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50,
51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63] - (22, 'Invalid
argument')
The fix make it same as in xl code.
Signed-off-by: Zhenzhong Duan
---
tools/python/xen/lowlevel/xc/xc.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index c40a4e9..2f4898d 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -243,13 +243,15 @@ static PyObject *pyxc_vcpu_setaffinity(XcObject *self,
for ( i = 0; i < PyList_Size(cpulist); i++ )
{
long cpu = PyInt_AsLong(PyList_GetItem(cpulist, i));
-if ( cpu < 0 || cpu >= nr_cpus )
+if ( cpu < 0 )
{
free(cpumap);
errno = EINVAL;
PyErr_SetFromErrno(xc_error_obj);
return NULL;
}
+if ( cpu >= nr_cpus )
+continue;
cpumap[cpu / 8] |= 1 << (cpu % 8);
}
}
--
1.7.3
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] IOMMU/MMU: Adjust low level functions for VT-d Device-TLB flush error.
On March 29, 2016 3:37pm, Jan Beulich wrote:
> >>> On 25.03.16 at 10:27, wrote:
> > On March 18, 2016 6:20pm, wrote:
> >> >>> On 17.03.16 at 07:54, wrote:
> >> > --- a/xen/drivers/passthrough/iommu.c
> >> > +++ b/xen/drivers/passthrough/iommu.c
> >> > @@ -554,11 +555,24 @@ static void iommu_flush_all(void)
> >> > iommu = drhd->iommu;
> >> > iommu_flush_context_global(iommu, 0);
> >> > flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> >> > -iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> >> > +rc = iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> >> > +
> >> > +if ( rc > 0 )
> >> > +{
> >> > +iommu_flush_write_buffer(iommu);
> >>
> >> Why is this needed all of the sudden?
> >
> > As there may be multiple IOMMUs. .e.g, there are 2 IOMMUs in my
> > machine, and I can find the following log message:
> > """
> > (XEN) Intel VT-d iommu 0 supported page sizes: 4kB, 2MB, 1GB.
> > (XEN) Intel VT-d iommu 1 supported page sizes: 4kB, 2MB, 1GB.
> > """
> > __iiuc__, iommu_flush_write_buffer() is per IOMMU, so It should be
> > called to flush every IOMMU.
>
> For one what you say suggests that right now this is being done for some
> (one?)
> IOMMU(s), which I don't see being the case. And then what you say _still_
> doesn't say _why_ this is now needed all of the sudden. If, in the course of
> doing your re-work here, you find pre-existing issues with the code, please
> split
> the necessary fixes out of your re-work and submit them separately with proper
> explanations in their commit messages.
>
I find out it is no need modification for this function.
I overlooked the parameter 'flush_non_present_entry', which is '0' to call
iommu_flush_iotlb_global().
At the bottom of the call tree,
(Existing code)
>>
In flush->iotlb():
{
if ( flush_non_present_entry )
{
if ( !cap_caching_mode(iommu->cap) )
return 1;
else
did = 0;
}
}
<<
it is impossible to return '1', which is used to indicate caller needs to flush
cache.
Quan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] IOMMU/MMU: Adjust low level functions for VT-d Device-TLB flush error.
On April 11, 2016 11:10am, wrote:
> On March 29, 2016 3:37pm, Jan Beulich wrote:
> > >>> On 25.03.16 at 10:27, wrote:
> > > On March 18, 2016 6:20pm, wrote:
> > >> >>> On 17.03.16 at 07:54, wrote:
> > >> > --- a/xen/drivers/passthrough/iommu.c
> > >> > +++ b/xen/drivers/passthrough/iommu.c
> > >> > @@ -554,11 +555,24 @@ static void iommu_flush_all(void)
> > >> > iommu = drhd->iommu;
> > >> > iommu_flush_context_global(iommu, 0);
> > >> > flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> > >> > -iommu_flush_iotlb_global(iommu, 0, flush_dev_iotlb);
> > >> > +rc = iommu_flush_iotlb_global(iommu, 0,
> > >> > + flush_dev_iotlb);
> > >> > +
> > >> > +if ( rc > 0 )
> > >> > +{
> > >> > +iommu_flush_write_buffer(iommu);
> > >>
> > >> Why is this needed all of the sudden?
> > >
> > > As there may be multiple IOMMUs. .e.g, there are 2 IOMMUs in my
> > > machine, and I can find the following log message:
> > > """
> > > (XEN) Intel VT-d iommu 0 supported page sizes: 4kB, 2MB, 1GB.
> > > (XEN) Intel VT-d iommu 1 supported page sizes: 4kB, 2MB, 1GB.
> > > """
> > > __iiuc__, iommu_flush_write_buffer() is per IOMMU, so It should be
> > > called to flush every IOMMU.
> >
> > For one what you say suggests that right now this is being done for
> > some (one?) IOMMU(s), which I don't see being the case. And then what
> > you say _still_ doesn't say _why_ this is now needed all of the
> > sudden. If, in the course of doing your re-work here, you find
> > pre-existing issues with the code, please split the necessary fixes
> > out of your re-work and submit them separately with proper explanations in
> their commit messages.
> >
>
> I find out it is no need modification for this function.
Sorry, this modification refers to as:
"
+if ( rc > 0 )
+{
+iommu_flush_write_buffer(iommu);
+rc = 0;
+}
"
at least errors need to be propagated.
Quan
> I overlooked the parameter 'flush_non_present_entry', which is '0' to call
> iommu_flush_iotlb_global().
> At the bottom of the call tree,
> (Existing code)
> >>
> In flush->iotlb():
> {
>
> if ( flush_non_present_entry )
> {
> if ( !cap_caching_mode(iommu->cap) )
> return 1;
> else
> did = 0;
> }
>
>
> }
> <<
>
>
> it is impossible to return '1', which is used to indicate caller needs to
> flush
> cache.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [libvirt bisection] complete build-amd64-libvirt
branch xen-unstable xenbranch xen-unstable job build-amd64-libvirt testid libvirt-build Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib git://git.sv.gnu.org/gnulib.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: libvirt git://libvirt.org/libvirt.git Bug introduced: ad7520e83f70892dd7ae6cd0cbc66d0759733e90 Bug not present: 6af73f53c6e40e80244bdacafeb3d0525a0c4189 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/90894/ commit ad7520e83f70892dd7ae6cd0cbc66d0759733e90 Author: John Ferlan Date: Tue Mar 29 18:22:46 2016 -0400 qemu: Create domain master key Add a masterKey and masterKeyLen to _qemuDomainObjPrivate to store a random domain master key and its length in order to support the ability to encrypt/decrypt sensitive data shared between libvirt and qemu. The key will be base64 encoded and written to a file to be used by the command line building code to share with qemu. New API's from this patch: qemuDomainGetMasterKeyFilePath: Return a path to where the key is located qemuDomainWriteMasterKeyFile: (private) Open (create/trunc) the masterKey path and write the masterKey qemuDomainMasterKeyReadFile: Using the master key path, open/read the file, and store the masterKey and masterKeyLen. Expected use only from qemuProcessReconnect qemuDomainGenerateRandomKey: (private) Generate a random key using available algorithms The key is generated either from the gnutls_rnd function if it exists or a less cryptographically strong mechanism using virGenerateRandomBytes qemuDomainMasterKeyRemove: Remove traces of the master key, remove the *KeyFilePath qemuDomainMasterKeyCreate: Generate the domain master key and save the key in the location returned by qemuDomainGetMasterKeyFilePath. This API will first ensure the QEMU_CAPS_OBJECT_SECRET is set in the capabilities. If not, then there's no need to generate the secret or file. The creation of the key will be attempted from qemuProcessPrepareHost once the libDir directory structure exists. The removal of the key will handled from qemuProcessStop just prior to deleting the libDir tree. Since the key will not be written out to the domain object XML file, the qemuProcessReconnect will read the saved file and restore the masterKey and masterKeyLen. For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/libvirt/build-amd64-libvirt.libvirt-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/libvirt/build-amd64-libvirt.libvirt-build --summary-out=tmp/90894.bisection-summary --basis-template=88483 --blessings=real,real-bisect libvirt build-amd64-libvirt libvirt-build Searching for failure / basis pass: 89302 fail [host=italia1] / 88483 [host=godello0] 88359 [host=godello1] 88247 [host=elbling1] 88091 [host=elbling1] 87976 [host=godello1] 87831 [host=godello1] 87702 [host=godello0] 87404 [host=godello0] 87264 [host=godello1] 87134 [host=godello0] 86994 [host=godello0] 86883 ok. Failure / basis pass flights: 89302 / 86883 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib git://git.sv.gnu.org/gnulib.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest 2844de6f4060dde5afa02b9c21d14b54b249f961 6cc32c63e80bc1a30c521b2f07f2b54909b59892 21f6526d1da331611ac5fe12967549d1a04e149b 316a862e5534249a6e6d876b4e203342d3fb870e a6f2cdb633bf519244a16674031b8034b581ba7f Basis pass dc56475f72b2c220443154b33fee221099eeec7f 6cc32c63e80bc1a30c521b2f07f2b54909b59892 21f6526d1da331611ac5fe12967549d1a04e149b 316a862e5534249a6e6d876b4e203342d3fb870e a6f2cdb633bf519244a16674031b8034b581ba7f Generating revisions with ./adhoc-revtuple-generator git://libvirt.org/libvirt.git#dc56475f72b2c220443154b33fee221099eeec7f-2844de6f4060dde5afa02b9c21d14b54b249f961 git://git.sv.gnu.org/gnulib.git#6cc32c63e80bc1a30c521b2f07f2b54909b59892-6cc32c63e80bc1a30c521b2f07f2b54909b59892 git://xenbits.xen.org/qemu-xen-traditional.git#21f6526d1da331611ac5fe12967549d1a04e149b-21f6526d1da331611ac5fe12967549d1a04e149b git://xenbits.xen.org/qemu-xen.git#316a862e5534249a6e6d876b4e203342d3fb870e-316a862e5534249a6e6d876b4e203342d3fb870e git://xenbits.xen.org/
Re: [Xen-devel] [PATCH v4 07/14] tools/lguest: force disable tboot and apm
"Luis R. Rodriguez" writes: > The paravirt_enabled() check is going away, the area tossed to > the kernel on lguest is not zerored out, so ensure lguest force > disables tboot and apm just in case the kernel file being read might > have this set for whatever reason. > > Signed-off-by: Luis R. Rodriguez Nice, thanks! Acked-by: Rusty Russell Cheers, Rusty. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] docs: add misc/qemu-backends.txt
On 10/04/16 22:00, Stefano Stabellini wrote: > On Thu, 7 Apr 2016, Juergen Gross wrote: >> Document the interface between qemu and libxl regarding backends >> supported by qemu. >> >> Signed-off-by: Juergen Gross >> --- >> docs/misc/qemu-backends.txt | 19 +++ >> 1 file changed, 19 insertions(+) >> create mode 100644 docs/misc/qemu-backends.txt >> >> diff --git a/docs/misc/qemu-backends.txt b/docs/misc/qemu-backends.txt >> new file mode 100644 >> index 000..f28755e >> --- /dev/null >> +++ b/docs/misc/qemu-backends.txt >> @@ -0,0 +1,19 @@ >> +In order to know whether qemu supports a specific backend type libxl >> +needs a way to obtain this information. >> + >> +As each qemu instance owns a path (named "" from now on) in >> +Xenstore the backend information is presented there. is built >> +from the domain id where the qemu instance is running >> +and the domain id of the target domain of the qemu process : >> + >> + = /local/domain//device-model/ >> + >> +Before signalling qemu is running by writing "running" to /state >> +qemu will create a Xenstore node for each supported backend under >> +/backends with the backend type as name (e.g. >> +/backends/qdisk for the qdisk backend). >> + >> +libxl can assume a backend of a specific type is supported if: >> +- /backends/ is existing in Xenstore >> +- or /backends is not existing and is one of: >> + "console", "vkbd", "vfb", "qdisk", "qnic" > > The thing to be careful about is that the plan just a few months ago was > to have QEMU restrict its own xenstore connection to the privilege level > of the guest VM it was servicing. Libxl would relax the xenstore access > rights to allow QEMU (and the gueest VM) access to > /local/domain//device-model//physmap, but nothing > else. See: > > [1] http://marc.info/?l=qemu-devel&m=143317363104584&w=2 > [2] http://marc.info/?l=xen-devel&m=145081000327541 > > what that means is that QEMU wouldn't be able to write to > /local/domain//device-model//backends, unless the > writing was done before calling xsrestrict, which should be > doable, but not what was done in [1]. > > Maybe we could add a note saying that these paths need to be written by > QEMU before dropping xenstore privileges? Okay. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] docs: add misc/qemu-backends.txt
On 08/04/16 20:27, Andrew Cooper wrote: > +Xenstore the backend information is presented there. is built >>> That looks to be missing an verb, no it has a verb, something is off with >>> that. >>> >>> XenStore presents the backend information there? >> No, qemu is presenting the information in Xenstore. > > "Xenstore" is not the start of the sentence - it follows on from the > previous line, but even as a native English speaker it took me a while > to figure out. > > A comma should be present (i.e. "As $FOO, $BAR relating to $FOO"), and > it would aid clarity. > > +As each qemu instance owns a path (named "" from now on) in > +Xenstore, the backend information is presented there. is built Will do it. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry
On 08/04/16 22:40, Luis R. Rodriguez wrote: > On Wed, Apr 06, 2016 at 10:40:08AM +0100, David Vrabel wrote: >> On 06/04/16 03:40, Luis R. Rodriguez wrote: >>> >>> * You don't need full EFI emulation >> >> I think needing any EFI emulation inside Xen (which is where it would >> need to be for dom0) is not suitable because of the increase in >> hypervisor ABI. > > Is this because of timing on architecture / design of HVMLite, or > a general position that the complexity to deal with EFI emulation > is too much for Xen's taste ? The Xen hypervisor should be as small as possible. Adding an EFI emulator will be adding quite some code. This should be done after a very thorough evaluation only. > ARM already went the EFI entry way for domU -- it went the OVMF route, > would such a possibility be possible for x86 domU HVMLite ? If not why > not, I mean it would seem to make sense to at least mimic the same type > of early boot environment, and perhaps there are some lessons to be > learned from that effort too. The final solution must be appropriate for dom0, too. So don't try to limit the discussion to domU. If dom0 isn't going to be acceptable there will no need to discuss domU. > Are there some lessons to be learned with ARM's effort? What are they? > If that could be re-done again with any type of cleaner path, what > could that be that could help the x86 side ? > > Although emulating EFI may require work, some folks have pointed out > that the amount of work may not be that much. If that is done can > we instead rely on the same code to replace OVMF to support both > Xen ARM and Xen HVMLite on x86 ? What would be the pros / cons of > this ? > >> I also still do not understand your objection to the current tiny stub. > > Its more of a hypothetical -- can an EFI entry be used instead given > it already does exactly what the new small entry does ? Its also rather > odd to add a new entry without evaluating fully a possible alternative > that would provide the same exact mechanism. The interface isn't the new entry only. It should be evaluated how much of the early EFI boot path would be common to the HVMlite one. What would be gained by using the same entry but having two different boot paths after it? You still need a way to distinguish between bare metal EFI and HVMlite. And Xen needs a way to find out whether a kernel is supporting HVMlite to boot it in the correct mode. > A full technical unbiased evaluation of the different approaches is what I'd > hope we could strive to achieve through discussion and peer review, thinking > and prioritizing ultimately what is best to minimize the impact on Linux > and also help take advantage of the best features possible through both > means. Thinking long term, not immediate short term. Sure. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [linux-3.16 test] 90844: regressions - trouble: blocked/broken/fail/pass
flight 90844 linux-3.16 real [real] http://logs.test-lab.xenproject.org/osstest/logs/90844/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 85048 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-arndale 3 host-install(3) broken pass in 89328 test-armhf-armhf-xl 16 guest-start.2 fail in 89328 pass in 90844 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail in 89328 pass in 90844 test-amd64-amd64-pair 9 xen-boot/src_host fail pass in 89328 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail pass in 89328 test-armhf-armhf-xl-multivcpu 6 xen-boot fail pass in 89328 Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 20 leak-check/check fail blocked in 85048 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat fail in 89328 blocked in 85048 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail in 89328 like 85048 build-i386-rumpuserxen6 xen-buildfail like 85048 build-amd64-rumpuserxen 6 xen-buildfail like 85048 test-amd64-amd64-xl-credit2 17 guest-localmigrate/x10 fail like 85048 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 85048 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 85048 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 85048 test-armhf-armhf-xl-rtds 11 guest-start fail like 85048 Tests which did not succeed, but are not blocking: test-amd64-i386-rumpuserxen-i386 1 build-check(1) blocked n/a test-amd64-amd64-rumpuserxen-amd64 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 13 saverestore-support-check fail in 89328 never pass test-armhf-armhf-xl-multivcpu 12 migrate-support-check fail in 89328 never pass test-armhf-armhf-xl-rtds 13 saverestore-support-check fail in 89328 never pass test-armhf-armhf-xl-rtds 12 migrate-support-check fail in 89328 never pass test-armhf-armhf-xl-arndale 12 migrate-support-check fail in 89328 never pass test-armhf-armhf-xl-arndale 13 saverestore-support-check fail in 89328 never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-armhf-armhf-libvirt 14 guest-saverestorefail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-xl-multivcpu 17 guest-localmigrate/x10 fail never pass test-amd64-amd64-xl-rtds 17 guest-localmigrate/x10 fail never pass test-armhf-armhf-libvirt-raw 13 guest-saverestorefail never pass test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 11 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 saverestore-support-checkfail never pass version targeted for testing: linux3a96c6601b6fc47baa6d296f9111ba7be4dad6fc baseline version: linux7f2a8840d127c8d5c59a5d79235
Re: [Xen-devel] [PATCH v5 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk
On 09/04/16 01:40, Luis R. Rodriguez wrote:
> We have 4 types of x86 platforms that disable RTC:
>
> * Intel MID
> * Lguest - uses paravirt
> * Xen dom-U - uses paravirt
> * x86 on legacy systems annotated with an ACPI legacy flag
>
> We can consolidate all of these into a platform specific legacy
> quirk set early in boot through i386_start_kernel() and through
> x86_64_start_reservations(). This deals with the RTC quirks which
> we can rely on through the hardware subarch, the ACPI check can
> be dealt with separately.
>
> For Xen things are bit more complex given that the @X86_SUBARCH_XEN
> x86_hardware_subarch is shared on for Xen which uses the PV path for
> both domU and dom0. Since the semantics for differentiating between
> the two are Xen specific we provide a platform helper to help override
> default legacy features -- x86_platform.set_legacy_features(). Use
> of this helper is highly discouraged, its only purpose should be
> to account for the lack of semantics available within your given
> x86_hardware_subarch.
>
> As per 0-day, this bumps the vmlinux size using i386-tinyconfig as
> follows:
>
> TOTAL TEXT init.textx86_early_init_platform_quirks()
> +70 +62+62 +43
>
> Only 8 bytes overhead total, as the main increase in size is
> all removed via __init.
I think this could be even less (see comment below).
>
> v2: split the subarch check from the ACPI check, clarify
> on the ACPI change commit log why ordering works
> v3: add x86_platform.set_legacy_features() to account for dom0,
> add also size impact on vmlinux as per 0-day report
You missed the v5 changes here.
>
> Suggested-by: Ingo Molnar
> Signed-off-by: Luis R. Rodriguez
> ---
> arch/x86/Makefile | 1 +
> arch/x86/include/asm/paravirt.h | 6 --
> arch/x86/include/asm/paravirt_types.h | 5 -
> arch/x86/include/asm/processor.h | 1 -
> arch/x86/include/asm/x86_init.h | 21 +
> arch/x86/kernel/Makefile | 6 +-
> arch/x86/kernel/head32.c | 2 ++
> arch/x86/kernel/head64.c | 1 +
> arch/x86/kernel/platform-quirks.c | 21 +
> arch/x86/kernel/rtc.c | 7 ++-
> arch/x86/lguest/boot.c| 1 -
> arch/x86/xen/enlighten.c | 10 +++---
> 12 files changed, 60 insertions(+), 22 deletions(-)
> create mode 100644 arch/x86/kernel/platform-quirks.c
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 4086abca0b32..f9ed8a7ce2b6 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -209,6 +209,7 @@ endif
> head-y := arch/x86/kernel/head_$(BITS).o
> head-y += arch/x86/kernel/head$(BITS).o
> head-y += arch/x86/kernel/head.o
> +head-y += arch/x86/kernel/platform-quirks.o
>
> libs-y += arch/x86/lib/
>
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index 601f1b8f9961..6c7a4a192032 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -20,12 +20,6 @@ static inline int paravirt_enabled(void)
> return pv_info.paravirt_enabled;
> }
>
> -static inline int paravirt_has_feature(unsigned int feature)
> -{
> - WARN_ON_ONCE(!pv_info.paravirt_enabled);
> - return (pv_info.features & feature);
> -}
> -
> static inline void load_sp0(struct tss_struct *tss,
>struct thread_struct *thread)
> {
> diff --git a/arch/x86/include/asm/paravirt_types.h
> b/arch/x86/include/asm/paravirt_types.h
> index e8c2326478c8..6acc1b26cf40 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -70,14 +70,9 @@ struct pv_info {
> #endif
>
> int paravirt_enabled;
> - unsigned int features;/* valid only if paravirt_enabled is set */
> const char *name;
> };
>
> -#define paravirt_has(x) paravirt_has_feature(PV_SUPPORTED_##x)
> -/* Supported features */
> -#define PV_SUPPORTED_RTC(1<<0)
> -
> struct pv_init_ops {
> /*
>* Patch may replace one of the defined code sequences with
> diff --git a/arch/x86/include/asm/processor.h
> b/arch/x86/include/asm/processor.h
> index 9264476f3d57..0c70c7daa6b8 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -474,7 +474,6 @@ static inline unsigned long current_top_of_stack(void)
> #else
> #define __cpuid native_cpuid
> #define paravirt_enabled() 0
> -#define paravirt_has(x) 0
>
> static inline void load_sp0(struct tss_struct *tss,
> struct thread_struct *thread)
> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> index 1ae89a2721d6..8bb8c1a4615a 100644
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -142,6 +142,15 @@ struct x86_cpuinit_ops {
> struct timespec;
>
> /**
> + * struct x86_legacy_features - le
Re: [Xen-devel] [PATCH v5 14/14] x86/paravirt: remove paravirt_enabled()
On 09/04/16 01:40, Luis R. Rodriguez wrote:
> That that paravirt_enabled() is replaced with proper
> x86 semantics we can remove it.
>
> Signed-off-by: Luis R. Rodriguez
> ---
> arch/x86/include/asm/paravirt.h | 5 -
> arch/x86/include/asm/paravirt_types.h | 1 -
> arch/x86/include/asm/processor.h | 1 -
> arch/x86/kernel/kvm.c | 8
> arch/x86/kernel/paravirt.c| 1 -
> arch/x86/lguest/boot.c| 2 --
> arch/x86/xen/enlighten.c | 1 -
> 7 files changed, 19 deletions(-)
Xen parts:
Acked-by: Juergen Gross
Juergen
>
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index 6c7a4a192032..dff26bc91b17 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -15,11 +15,6 @@
> #include
> #include
>
> -static inline int paravirt_enabled(void)
> -{
> - return pv_info.paravirt_enabled;
> -}
> -
> static inline void load_sp0(struct tss_struct *tss,
>struct thread_struct *thread)
> {
> diff --git a/arch/x86/include/asm/paravirt_types.h
> b/arch/x86/include/asm/paravirt_types.h
> index 6acc1b26cf40..7fedf24bd811 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -69,7 +69,6 @@ struct pv_info {
> u16 extra_user_64bit_cs; /* __USER_CS if none */
> #endif
>
> - int paravirt_enabled;
> const char *name;
> };
>
> diff --git a/arch/x86/include/asm/processor.h
> b/arch/x86/include/asm/processor.h
> index 0c70c7daa6b8..8d326e822cb8 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -473,7 +473,6 @@ static inline unsigned long current_top_of_stack(void)
> #include
> #else
> #define __cpuid native_cpuid
> -#define paravirt_enabled() 0
>
> static inline void load_sp0(struct tss_struct *tss,
> struct thread_struct *thread)
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index dc1207e2f193..eea2a6f72b31 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -285,14 +285,6 @@ static void __init paravirt_ops_setup(void)
> {
> pv_info.name = "KVM";
>
> - /*
> - * KVM isn't paravirt in the sense of paravirt_enabled. A KVM
> - * guest kernel works like a bare metal kernel with additional
> - * features, and paravirt_enabled is about features that are
> - * missing.
> - */
> - pv_info.paravirt_enabled = 0;
> -
> if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
> pv_cpu_ops.io_delay = kvm_io_delay;
>
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index f08ac28b8136..71a2d8a05a66 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -294,7 +294,6 @@ enum paravirt_lazy_mode paravirt_get_lazy_mode(void)
>
> struct pv_info pv_info = {
> .name = "bare hardware",
> - .paravirt_enabled = 0,
> .kernel_rpl = 0,
> .shared_kernel_pmd = 1, /* Only used when CONFIG_X86_PAE is set */
>
> diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
> index f5497ee5fd2f..3847e736702e 100644
> --- a/arch/x86/lguest/boot.c
> +++ b/arch/x86/lguest/boot.c
> @@ -1408,8 +1408,6 @@ __init void lguest_init(void)
> {
> /* We're under lguest. */
> pv_info.name = "lguest";
> - /* Paravirt is enabled. */
> - pv_info.paravirt_enabled = 1;
> /* We're running at privilege level 1, not 0 as normal. */
> pv_info.kernel_rpl = 1;
> /* Everyone except Xen runs with this set. */
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index e066fcf87c3d..7c1da39623f4 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1186,7 +1186,6 @@ static unsigned xen_patch(u8 type, u16 clobbers, void
> *insnbuf,
> }
>
> static const struct pv_info xen_info __initconst = {
> - .paravirt_enabled = 1,
> .shared_kernel_pmd = 0,
>
> #ifdef CONFIG_X86_64
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v9 2/3] VT-d: wrap a _sync version for all VT-d flush interfaces
> From: Xu, Quan > Sent: Thursday, April 07, 2016 3:45 PM > > On April 05, 2016 5:35pm, Jan Beulich wrote: > > >>> On 01.04.16 at 16:47, wrote: > > > The dev_invalidate_iotlb() scans ats_devices list to flush ATS > > > devices, and the invalidate_sync() is put after dev_invalidate_iotlb() > > > to synchronize with hardware for flush status. If we assign multiple > > > ATS devices to a domain, the flush status is about all these multiple > > > ATS devices. Once flush timeout expires, we couldn't find out which > > > one is the buggy ATS device. > > > > Is that true? Or is that just a limitation of our implementation? > > > > IMO, both. > I hope vt-d maintainers help me double check it. Just a limitation of our implementation. Now dev_invalidate_iotlb puts all possible IOTLB flush requests to the queue, and then invalidate_sync pushes a wait descriptor w/ timeout to detect error. VT-d spec says one or more descriptors may be fetched together by hardware. So when a timeout is triggered, we cannot tell which flush request actually has problem by reading queue head register. If we change the implementation to one-invalidation-sync-per-request, then we can tell. I discussed with Quan not to go that complexity though. Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
