Re: [Xen-devel] [PATCH v2 3/3] x86: Make the GDT remapping read-only on 64 bit
* Andy Lutomirskiwrote: > On Wed, Feb 1, 2017 at 1:15 AM, Ingo Molnar wrote: > > > > * Thomas Garnier wrote: > > > >> This patch makes the GDT remapped pages read-only to prevent corruption. > >> This change is done only on 64 bit. > > > > > >> > >> - table_base = gdt->address; > >> + table_base = (unsigned long)get_current_direct_gdt(); > > > > Instead of spreading these type casts far and wide please introduce another > > accessor the returns 'unsigned long': > > > > get_cpu_gdt_rw_vaddr() > > > > That whole function is an abomination. How about replacing 'unsigned > long table_base' with 'struct desc_struct *table'? If you're feeling > really adventurous, *delete* that function and replace all of its > users with something sane. Yeah, even better! Thanks, Ingo ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] x86: Make the GDT remapping read-only on 64 bit
On Thu, Jan 26, 2017 at 8:59 AM, Thomas Garnierwrote: > This patch makes the GDT remapped pages read-only to prevent corruption. > This change is done only on 64 bit. > > The native_load_tr_desc function was adapted to correctly handle a > read-only GDT. The LTR instruction always writes to the GDT TSS entry. > This generates a page fault if the GDT is read-only. This change checks > if the current GDT is a remap and swap GDTs as needed. This function was > tested by booting multiple machines and checking hibernation works > properly. > > KVM SVM and VMX were adapted to use the writeable GDT. On VMX, the > per-cpu variable was removed for functions to fetch the original GDT. > Instead of reloading the previous GDT, VMX will reload the fixmap GDT as > expected. For testing, VMs were started and restored on multiple > configurations. > > Signed-off-by: Thomas Garnier > --- > Based on next-20170125 > --- > arch/x86/include/asm/desc.h | 46 > +++- > arch/x86/include/asm/processor.h | 1 + > arch/x86/kernel/cpu/common.c | 28 ++-- > arch/x86/kvm/svm.c | 4 +--- > arch/x86/kvm/vmx.c | 15 + > 5 files changed, 70 insertions(+), 24 deletions(-) > > diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h > index 4cc176f57b78..ca7b2224fcb4 100644 > --- a/arch/x86/include/asm/desc.h > +++ b/arch/x86/include/asm/desc.h > @@ -52,6 +52,12 @@ static inline struct desc_struct > *get_cpu_direct_gdt(unsigned int cpu) > return per_cpu(gdt_page, cpu).gdt; > } > > +/* Provide the current original GDT */ > +static inline struct desc_struct *get_current_direct_gdt(void) > +{ > + return this_cpu_ptr(_page)->gdt; > +} I'm assuming that the reason that this isn't part of patch 2 and used instead of the version that takes cpu as a parameter is that TLS doesn't work until the GDT is set up. If so, perhaps that's worthy of a comment in patch 2. But give this_cpu_read(gdt_page.gdt) a try, please. > +/* > + * The LTR instruction marks the TSS GDT entry as busy. In 64bit, the GDT is > + * a read-only remapping. To prevent a page fault, the GDT is switched to the > + * original writeable version when needed. > + */ > +#ifdef CONFIG_X86_64 > +static inline void native_load_tr_desc(void) > +{ > + struct desc_ptr gdt; > + int cpu = raw_smp_processor_id(); > + bool restore = false; > + struct desc_struct *fixmap_gdt; > + > + native_store_gdt(); Off the top of my head, this is something like 10 cycles. IMO that's fast enough not to worry about the regression this will cause to KVM exits. In any event, we'll get that back and *much* more when we do the optimizations that this series enables. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 3/3] x86: Make the GDT remapping read-only on 64 bit
On Wed, Feb 1, 2017 at 1:15 AM, Ingo Molnarwrote: > > * Thomas Garnier wrote: > >> This patch makes the GDT remapped pages read-only to prevent corruption. >> This change is done only on 64 bit. > >> >> - table_base = gdt->address; >> + table_base = (unsigned long)get_current_direct_gdt(); > > Instead of spreading these type casts far and wide please introduce another > accessor the returns 'unsigned long': > > get_cpu_gdt_rw_vaddr() > That whole function is an abomination. How about replacing 'unsigned long table_base' with 'struct desc_struct *table'? If you're feeling really adventurous, *delete* that function and replace all of its users with something sane. --Andy ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable test] 105224: tolerable FAIL - PUSHED
flight 105224 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/105224/ Failures :-/ but no regressions. Regressions which are regarded as allowable (not blocking): test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 104681 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 104681 test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail like 104681 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 104681 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 104681 test-armhf-armhf-libvirt 13 saverestore-support-checkfail like 104681 test-amd64-amd64-xl-rtds 9 debian-install fail like 104681 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail like 104681 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail never pass build-arm64 5 xen-buildfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass build-arm64-pvops 5 kernel-build 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 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 11 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 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-xl-xsm 13 saverestore-support-checkfail never pass version targeted for testing: xen 8844ed299a88848da88b23e6db43b5bcc4ad4bee baseline version: xen c13f0f9a331153a57eedfe8c80f1e2f6d4f01dcc Last test of basis 104681 2017-01-26 03:50:00 Z6 days Failing since104728 2017-01-26 19:45:05 Z6 days 14 attempts Testing same since 105224 2017-02-01 19:44:38 Z0 days1 attempts People who touched revisions under test: Andrew CooperDaniel De Graaf Dario Faggioli Edgar E. Iglesias Elena Ufimtseva George Dunlap George Dunlap Ian Jackson
Re: [Xen-devel] [PATCH] xen-netfront: Delete rx_refill_timer in xennet_disconnect_backend()
On Wed, 2017-02-01 at 18:29 -0500, Boris Ostrovsky wrote: > > I could not convince myself that napi_synchronize() is sufficient here > (mostly because I am not familiar with napi flow). At the same time I > would rather not make changes in anticipation of possible disappearance > of netif_carrier_ok() in the future so I'd like this patch to go in as is. > > Unless there are other problems with the patch or if Eric (or others) > feel strongly about usage of netif_carrier_ok() here. > No strong feelings from me. We probably have more serious issues to fix anyway. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [linux-linus test] 105211: regressions - FAIL
flight 105211 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/105211/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-xsm 17 guest-localmigrate/x10fail REGR. vs. 59254 test-amd64-i386-xl 17 guest-localmigrate/x10fail REGR. vs. 59254 test-armhf-armhf-libvirt 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-xsm 17 guest-localmigrate/x10fail REGR. vs. 59254 test-amd64-amd64-xl-multivcpu 17 guest-localmigrate/x10 fail REGR. vs. 59254 test-armhf-armhf-xl-arndale 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-credit2 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 15 guest-localmigrate/x10 fail REGR. vs. 59254 test-armhf-armhf-xl 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-libvirt-xsm 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-multivcpu 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-xsm 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-credit2 17 guest-localmigrate/x10 fail in 105176 REGR. vs. 59254 test-amd64-amd64-xl 17 guest-localmigrate/x10 fail in 105176 REGR. vs. 59254 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-multivcpu 16 guest-saverestore.2 fail in 105176 pass in 105211 test-amd64-i386-pair 21 guest-migrate/src_host/dst_host fail in 105176 pass in 105211 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail in 105176 pass in 105211 test-amd64-amd64-xl-credit2 14 guest-saverestore fail pass in 105176 test-amd64-amd64-xl 15 guest-localmigrate fail pass in 105176 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 13 guest-localmigrate fail REGR. vs. 59254 test-armhf-armhf-xl-rtds 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-rtds 9 debian-installfail REGR. vs. 59254 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail baseline untested test-armhf-armhf-libvirt-raw 6 xen-bootfail baseline untested test-armhf-armhf-xl-vhd 6 xen-bootfail baseline untested test-amd64-i386-libvirt-xsm 15 guest-saverestore.2 fail blocked in 59254 test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail in 105176 baseline untested test-amd64-i386-libvirt-xsm 14 guest-saverestore fail in 105176 blocked in 59254 test-amd64-amd64-libvirt-xsm 14 guest-saverestore fail in 105176 blocked in 59254 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 59254 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 59254 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64 5 xen-buildfail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 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 version targeted for testing: linux
Re: [Xen-devel] [PATCH] xen-netfront: Delete rx_refill_timer in xennet_disconnect_backend()
On 01/31/2017 12:47 PM, Boris Ostrovsky wrote: > On 01/30/2017 02:31 PM, Boris Ostrovsky wrote: >> On 01/30/2017 02:06 PM, Eric Dumazet wrote: >>> On Mon, 2017-01-30 at 13:23 -0500, Boris Ostrovsky wrote: >>> We do netif_carrier_off() first thing in xennet_disconnect_backend() and the only place where the timer is rearmed is xennet_alloc_rx_buffers(), which is guarded by netif_carrier_ok() check. >>> Oh well, testing netif_carrier_ok() in packet processing fast path looks >>> unusual and a waste of cpu cycles. I've never seen that pattern before. >>> >>> If one day, we remove this netif_carrier_ok() test during a cleanup, >>> then the race window will open again. >> I don't know much about napi but I wonder whether I can indeed disable >> it in xennet_disconnect_backend(). I don't see how anything can happen >> after disconnect since it unmaps the rings. And then napi is re-enabled >> during reconnection in xennet_create_queues(). In which case am not sure >> there is any need for xennet_destroy_queues() as everything there could >> be folded into xennet_disconnect_backend(). > While this does work, there was a reason why napi_disable() was not > called in xennet_disconnect_backend() and it is explained in commit > ce58725fec6e --- napi_disable() may sleep and that's why it is called in > xennet_destroy_queues(). > > OTOH, there is a napi_synchronize() call in xennet_destroy_queues(). > Will destroying the timer after it guarantee that all preceding RX have > been completed? RX interrupt is disabled prior to napi_synchronize() so > presumably nothing new can be received. I could not convince myself that napi_synchronize() is sufficient here (mostly because I am not familiar with napi flow). At the same time I would rather not make changes in anticipation of possible disappearance of netif_carrier_ok() in the future so I'd like this patch to go in as is. Unless there are other problems with the patch or if Eric (or others) feel strongly about usage of netif_carrier_ok() here. -boris ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] xen/arm: fix rank/vgic lock inversion bug
On Wed, 1 Feb 2017, Julien Grall wrote: > Hi Stefano, > > CC Andre to get more feedback. > > On 31/01/2017 23:49, Stefano Stabellini wrote: > > On Fri, 27 Jan 2017, Julien Grall wrote: > > > On 03/01/17 23:29, Stefano Stabellini wrote: > > > > Always set the new physical irq affinity at the beginning of > > > > vgic_migrate_irq, in all cases. > > > > > > > > No need to set physical irq affinity in gic_update_one_lr anymore, > > > > solving the lock inversion problem. > > > > > > > > After migrating an interrupt from vcpu/pcpu 0 to vcpu/pcpu 1, it is > > > > possible to receive a physical interrupt on pcpu 1, which Xen is > > > > supposed to inject into vcpu 1, before the LR on pcpu 0 has been > > > > cleared. In this case the irq is still marked as > > > > GIC_IRQ_GUEST_MIGRATING, and struct pending_irq is still "inflight" on > > > > vcpu 0. As the irq cannot be "inflight" on vcpu 0 and vcpu 1 > > > > simultaneously, drop the interrupt. > > > > > > I am not sure to understand how this is related to the fix of the > > > rank/vgic > > > lock inversion bug. Am I right to think that this bug was already present? > > > > Yes, it is related: without this patch, this situation cannot happen, > > because irq_set_affinity is called from gic_update_one_lr. > > I am not convinced about that. gic_update_one_lr will take the lock of the > current vCPU whilst vgic_vcpu_inject_irq is taking the lock of the vCPU where > the vIRQ has been routed. > > The function the memory access of the list_del_init could potentially be > re-ordered with irq_set_affinity. > > However, what is saving us is the memory barrier hidden in spin_lock call in > gicv{2,3}_irq_set_affinity and the other one in vgic_vcpu_inject_irq. > > So I agree this is currently working, but I would argue it is by luck. I agree we should have a memory barrier after list_del_init, I think I even mentioned it in one of the past emails on related threads. One way or another, we'll make the problem go away with this patch (or other implementations of it). > > > > -vgic_vcpu_inject_irq(new, irq); > > > > > > The comment on top of the if says: "If the IRQ is still lr_pending, > > > re-inject > > > it to the new vCPU". However, you remove interrupt from the list but never > > > re-inject it. > > > > > > Looking at the context, I think we should keep the call to > > > vgic_vcpu_inject_irq otherwise we lost the interrupt for ever. > > > > It looks like I made a mistake while generating the patch: I meant to > > remove the redundant irq_set_affinity call, instead of the necessary > > vgic_vcpu_inject_irq call. I don't know how it happened, sorry. Thanks > > for catching the error. > > > > > > > > return; > > > > } > > > > /* if the IRQ is in a GICH_LR register, set GIC_IRQ_GUEST_MIGRATING > > > > @@ -495,6 +491,13 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned > > > > int > > > > virq) > > > > return; > > > > } > > > > > > > > +if ( test_bit(GIC_IRQ_GUEST_MIGRATING, >status) ) > > > > +{ > > > > +/* Drop the interrupt, because it is still inflight on another > > > > vcpu > > > > */ > > > > +spin_unlock_irqrestore(>arch.vgic.lock, flags); > > > > > > If I understand correctly, the problem arise because LRs are cleared > > > lazily > > > and the other vCPU is still running. It is a short window and I don't > > > think > > > should happen often. > > > > > > I would suggest to kick the other vCPU with an SGI to get the LRs cleared. > > > Any > > > opinions? > > > > I am fine with sending an SGI, but we still need > > http://marc.info/?l=xen-devel=148237295020488 to know the destination > > of the SGI. > > Hmmm correct. > > > > > The problem is what happens next: is it safe to busy-wait until the LR > > is cleared? > > Technically we already have this problem as LRs are cleared with the vgic lock > taken. > > So if by mistake you receive the interrupt on the wrong pCPU, you may busy > wait on the spinlock for some time. > > Note, I am not saying we should do the same here :). I would argue it would > depends how much time you expect to be taken for clearing the LRs. It can take an arbitrarily long time, I think. We cannot rely on it being short, unless below. > > It is safe only if we cannot receive a second interrupt > > notification while the first one has not been deactivated yet by the > > guest: > > > > - guest vcpu0 reads GICC_IAR for virq0 > > - guest vcpu1 change target for virq0 to vcpu1 > > - Xen traps the write and changes the physical affinity of virq0 (pirq0) > > to vcpu1 (pcpu1) > > - Xen receives a second virq0 (pirq0) interrupt notification in pcpu1, > > but virq0 is still MIGRATING and guest vcpu0 has not EOIed it yet > > - Xen sends an SGI to vcpu0 (pcpu0), but the interrupt has not been > > EOIed yet, thus, the LR doesn't get cleared > > > > Can this happen? I read 3.2.3 and 3.2.4a few times but I am still > > unsure, it looks like it can. If it
Re: [Xen-devel] [PATCH] libxl: fix dom0 autoballooning with Xen 4.8
Jim Fehlig wrote: > xen.git commit 57f8b13c changed several of the libxl memory > get/set functions to take 64 bit parameters. The libvirt > libxl driver still uses uint32_t variables for these various > parameters, which is particularly problematic for the > libxl_set_memory_target() function. > > When dom0 autoballooning is enabled, libvirt (like xl) determines > the memory needed to start a domain and the memory available. If > memory available is less than memory needed, dom0 is ballooned > down by passing a negative value to libxl_set_memory_target() > 'target_memkb' parameter. Prior to xen.git commit 57f8b13c, > 'target_memkb' was an int32_t. Subtracting a larger uint32 from > a smaller uint32 and assigning it to int32 resulted in a negative > number. After commit 57f8b13c, the same subtraction is widened > to a int64, resulting in a large positive number. The simple > fix taken by this patch is to assign the difference of the > uint32 values to a temporary int32 variable, which is then > passed to 'target_memkb' parameter of libxl_set_memory_target(). > > Note that it is undesirable to change libvirt to use 64 bit > variables since it requires setting LIBXL_API_VERSION to 0x040800. > Currently libvirt supports LIBXL_API_VERSION >= 0x040400, > essentially Xen >= 4.4. Any comments on this patch? xen-devel is already in cc, but I'll add Wei (one of the Xen tools maintainers) and Juergen (author of commit 57f8b13c) for some additional exposure :-). Currently, "out-of-the-box" Xen >= 4.8 + libvirt setup (dom0 autoballooning enabled) is broken without this fix. Regards, Jim > > Signed-off-by: Jim Fehlig> --- > src/libxl/libxl_domain.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index a5314b0..ed73cd2 100644 > --- a/src/libxl/libxl_domain.c > +++ b/src/libxl/libxl_domain.c > @@ -909,6 +909,7 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config > *d_config) > { > uint32_t needed_mem; > uint32_t free_mem; > +int32_t target_mem; > int tries = 3; > int wait_secs = 10; > > @@ -922,7 +923,8 @@ libxlDomainFreeMem(libxl_ctx *ctx, libxl_domain_config > *d_config) > if (free_mem >= needed_mem) > return 0; > > -if (libxl_set_memory_target(ctx, 0, free_mem - needed_mem, > +target_mem = free_mem - needed_mem; > +if (libxl_set_memory_target(ctx, 0, target_mem, > /* relative */ 1, 0) < 0) > goto error; > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document
Hi Stefano, On 01/02/2017 19:31, Stefano Stabellini wrote: On Wed, 1 Feb 2017, Julien Grall wrote: On 31/01/2017 19:06, Edgar E. Iglesias wrote: On Tue, Jan 31, 2017 at 05:09:53PM +, Julien Grall wrote: On 31/01/17 16:53, Edgar E. Iglesias wrote: On Wed, Jan 25, 2017 at 06:53:20PM +, Julien Grall wrote: On 24/01/17 20:07, Stefano Stabellini wrote: On Tue, 24 Jan 2017, Julien Grall wrote: From my understanding your MSI controller is embedded in the hostbridge, right? If so, the MSIs would need to be handled where the host bridge will be initialized (e.g either Xen or DOM0). From a design point of view, it would make more sense to have the MSI controller driver in Xen as the hostbridge emulation for guest will also live there. So if we receive MSI in Xen, we need to figure out a way for DOM0 and guest to receive MSI. The same way would be the best, and I guess non-PV if possible. I know you are looking to boot unmodified OS in a VM. This would mean we need to emulate the MSI controller and potentially xilinx PCI controller. How much are you willing to modify the OS? Regarding the MSI doorbell, I have seen it is configured by the software using a physical address of a page allocated in the RAM. When the PCI devices is writing into the doorbell does the access go through the SMMU? Regardless the answer, I think we would need to map the MSI doorbell page in the guest. Why? We should be able to handle the case by trapping and emulating PCI config accesses. Xen can force the real MSI descriptors to use whatever Xen wants them to use. With an SMMU, we need to find a way to map the MSI doorbell in the SMMU pagetable to allow the device to write to it. Without SMMU, it's unneeded. My point was using guest with SMMU, if you want to support PCI passthrough without SMMU then it is another subject and I would rather postpone this discussion. Meaning that even if we trap MSI configuration access, a guess could DMA in the page. So if I am not mistaken, MSI would be insecure in this case :/. That's right: if a device capable of DMA to an arbitrary address in memory is assigned to the guest, the guest can write to the MSI doorbell if an SMMU is present, otherwise, the guest can write to any address in memory without SMMU. Completely insecure. It is the same security compromised offered by PV PCI passthrough today with no VT-D on the platform. I think it's still usable in some cases, but we need to be very clear about its security properties. The guest would have to be mapped 1:1 in order to do DMA. And this is not supported today. Or maybe we could avoid mapping the doorbell in the guest and let Xen receive an SMMU abort. When receiving the SMMU abort, Xen could sanitize the value and write into the real MSI doorbell. Not sure if it would works thought. I thought that SMMU aborts are too slow for this? I got no idea here. However, I think it would be better than a security hole. So this could be an option for the user. Cheers, -- Julien grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Xen Security Advisory 154 (CVE-2016-2270) - x86: inconsistent cachability flags on guest mappings
On Wed, 2017-01-25 at 07:21 -0700, Jan Beulich wrote: > > Well, in the context of this XSA we've asked both of them, and iirc > we've got a vague reply from Intel and none from AMD. In fact we > did defer the XSA for quite a bit waiting for any useful feedback. > To AMD's advantage I'd like to add though that iirc they're a little > more clear in their PM about the specific question of UC and WC > you raise: They group the various cacheabilities into two groups > (cacheable and uncacheable) and require there to only not be > any mixture between groups. Iirc Intel's somewhat vague reply > allowed us to conclude we're likely safe that way on their side too. It would be good to get a definitive answer from Intel, to match AMD's. That's basically why I added hpa to CC, in fact. Peter, is there any possibility of a clarification here, please? Thanks. smime.p7s Description: S/MIME cryptographic signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: use qdev_unplug() instead of g_free() in xen_pv_find_xendev()
On 1 February 2017 at 19:37, Stefano Stabelliniwrote: > Hi Peter, > > do you think this is acceptable? The set of operations here is basically what I suggested in review of v1, so I think it is the right thing. OTOH this is a bit of an odd corner of the QOM model so it might be worth doing some testing to make sure the reference counts are doing what you (I) expect and that the object does get correctly freed both in the error-handling path here and when the device is unplugged via xen_pv_del_xendev(). thanks -- PMM ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document
Hi Stefano, On 31/01/2017 21:58, Stefano Stabellini wrote: On Wed, 25 Jan 2017, Julien Grall wrote: whilst for Device Tree the segment number is not available. So Xen needs to rely on DOM0 to discover the host bridges and notify Xen with all the relevant informations. This will be done via a new hypercall PHYSDEVOP_pci_host_bridge_add. The layout of the structure will be: I understand that the main purpose of this hypercall is to get Xen and Dom0 to agree on the segment numbers, but why is it necessary? If Dom0 has an emulated contoller like any other guest, do we care what segment numbers Dom0 will use? I was not planning to have a emulated controller for DOM0. The physical one is not necessarily ECAM compliant so we would have to either emulate the physical one (meaning multiple different emulation) or an ECAM compliant. The latter is not possible because you don't know if there is enough free MMIO space for the emulation. In the case on ARM, I don't see much the point to emulate the host bridge for DOM0. The only thing we need in Xen is to access the configuration space, we don't have about driving the host bridge. So I would let DOM0 dealing with that. Also, I don't see any reason for ARM to trap DOM0 configuration space access. The MSI will be configured using the interrupt controller and it is a trusted Domain. These last you sentences raise a lot of questions. Maybe I am missing something. You might want to clarify the strategy for Dom0 and DomUs, and how they differ, in the next version of the doc. At some point you wrote "Instantiation of a specific driver for the host controller can be easily done if Xen has the information to detect it. However, those drivers may require resources described in ASL." Does it mean you plan to drive the physical host bridge from Xen and Dom0 simultaneously? I may miss some bits, so feel free to correct me if I am wrong. My understanding is host bridge can be divided in 2 parts: - Initialization of the host bridge - Access the configuration space For generic host bridge, the initialization is inexistent. However some host bridge (e.g xgene, xilinx) may require some specific setup and also configuring clocks. Given that Xen only requires to access the configuration space, I was thinking to let DOM0 initialization the host bridge. This would avoid to import a lot of code in Xen, however this means that we need to know when the host bridge has been initialized before accessing the configuration space. I prefer to avoid a split-mind approach, where some PCI things are initialized/owned by one component and some others are initialized/owned by another component. It creates complexity. Of course, we have to face the reality that the alternatives might be worse, but let's take a look at the other options first. How hard would it be to bring the PCI host bridge initialization in Xen, for example in the case of the Xilinx ZynqMP? Traditionally, PCI host bridges have not required any initialization on x86. PCI is still new to the ARM ecosystems. I think it is reasonable to expect that going forward, as the ARM ecosystem matures, PCI host bridges will require little to no initialization on ARM too. I would agree for servers but I am less sure for embedded systems. You may want to save address space or even power potentially requiring a custom hostbridge. I hope I am wrong here. I think the xilinx host bridge is the simplest case. I am trying to understand better in a separate e-mail (see). There are more complex hostbridge such as x-gene [1] and R-Car [2]. If we take the example of the renesas salvator board been used on automotive (Globallogic and Bosh are working on support for Xen [3]), it contains an R-Car PCI root complex, below a part of the DTS /* External PCIe clock - can be overridden by the board */ pcie_bus_clk: pcie_bus { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <0>; }; pciec0: pcie@fe00 { compatible = "renesas,pcie-r8a7795"; reg = <0 0xfe00 0 0x8>; #address-cells = <3>; #size-cells = <2>; bus-range = <0x00 0xff>; device_type = "pci"; ranges = <0x0100 0 0x 0 0xfe10 0 0x0010 0x0200 0 0xfe20 0 0xfe20 0 0x0020 0x0200 0 0x3000 0 0x3000 0 0x0800 0x4200 0 0x3800 0 0x3800 0 0x0800>; /* Map all possible DDR as inbound ranges */ dma-ranges = <0x4200 0 0x4000 0 0x4000 0 0x4000>; interrupts = , , ; #interrupt-cells = <1>; interrupt-map-mask = <0 0 0 0>; interrupt-map = <0 0 0 0 GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
Re: [Xen-devel] [PATCH v4 14/16] kprobes: move kprobe declarations to asm-generic/kprobes.h
On Mon, Aug 29, 2016 at 11:04:18PM +0900, Masami Hiramatsu wrote: > On Tue, 23 Aug 2016 18:31:05 +0200 > "Luis R. Rodriguez"wrote: > > > On Tue, Aug 23, 2016 at 12:11:40AM +0900, Masami Hiramatsu wrote: > > > On Fri, 19 Aug 2016 14:34:12 -0700 > > > mcg...@kernel.org wrote: > > > > > > > From: "Luis R. Rodriguez" > > > > > > > > Often all is needed is these small helpers, instead of compiler.h > > > > or a full kprobes.h. This is important for asm helpers, in fact even > > > > some asm/kprobes.h make use of these helpers... instead just keep a > > > > generic asm file with helpers useful for asm code with the least amount > > > > of clutter as possible. > > > > > > > > Likewise we need now to also address what to do about this file for both > > > > when architectures have CONFIG_HAVE_KPROBES, and when they do not. Then > > > > for when architectures have CONFIG_HAVE_KPROBES but have disabled > > > > CONFIG_KPROBES. > > > > > > > > Right now most asm/kprobes.h do not have guards against CONFIG_KPROBES, > > > > this means most architecture code cannot include asm/kprobes.h safely. > > > > Correct this and add guards for architectures missing them. Additionally > > > > provide architectures that not have kprobes support with the default > > > > asm-generic solution. This lets us force asm/kprobes.h on the header > > > > include/linux/kprobes.h always, but most importantly we can now safely > > > > include just asm/kprobes.h on architecture code without bringing > > > > the full kitchen sink of header files. > > > > > > > > Two architectures already provided a guard against CONFIG_KPROBES on > > > > its kprobes.h: sh, arch. The rest of the architectures needed gaurds > > > > added. We avoid including any not-needed headers on asm/kprobes.h > > > > unless kprobes have been enabled. > > > > > > > > In a subsequent atomic change we can try now to remove compiler.h from > > > > include/linux/kprobes.h. > > > > > > Hmm, this looks a bit overkill... I rather like move it into > > > linux/table.h. > > > > That's the thing, we can't reasonably expect every table to add an entry > > into > > table.h, this should be up to each user. Moving it to tables.h just prolongs > > what needs to be done. In this case the change is justifiable given kprobe > > annotations are required for some architectures early in architecture code, > > and > > including compiler.h on early architecture code blows up. There is no easy > > fix > > to this, and this this was *actually* the cleanest solution I could devise > > without much changes. > > Aah, I misread your patch, it is asm-generic/kprobes.h, not asm/kprobes.h. > I think it is OK now. > > Acked-by: Masami Hiramatsu I'm going to send this patch out separately shortly as its independent. Luis ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen: use qdev_unplug() instead of g_free() in xen_pv_find_xendev()
Hi Peter, do you think this is acceptable? Thanks, Stefano On Wed, 1 Feb 2017, Juergen Gross wrote: > The error exits of xen_pv_find_xendev() free the new xen-device via > g_free() which is wrong. > > As the xen-device has been initialized as qdev it must be removed > via qdev_unplug(). > > This bug has been introduced with commit 3a6c9172ac5951e6dac2b3f6 > ("xen: create qdev for each backend device"). > > Reported-by: Roger Pau Monné> Tested-by: Roger Pau Monné > Signed-off-by: Juergen Gross > --- > V2: set free method to avoid memory leak (Peter Maydell) > use DEVICE(xendev) instead of >qdev (Peter Maydell) > --- > hw/xen/xen_backend.c | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c > index d119004..6c21c37 100644 > --- a/hw/xen/xen_backend.c > +++ b/hw/xen/xen_backend.c > @@ -124,10 +124,11 @@ static struct XenDevice *xen_be_get_xendev(const char > *type, int dom, int dev, > /* init new xendev */ > xendev = g_malloc0(ops->size); > object_initialize(>qdev, ops->size, TYPE_XENBACKEND); > -qdev_set_parent_bus(>qdev, xen_sysbus); > -qdev_set_id(>qdev, g_strdup_printf("xen-%s-%d", type, dev)); > -qdev_init_nofail(>qdev); > -object_unref(OBJECT(>qdev)); > +OBJECT(xendev)->free = g_free; > +qdev_set_parent_bus(DEVICE(xendev), xen_sysbus); > +qdev_set_id(DEVICE(xendev), g_strdup_printf("xen-%s-%d", type, dev)); > +qdev_init_nofail(DEVICE(xendev)); > +object_unref(OBJECT(xendev)); > > xendev->type = type; > xendev->dom = dom; > @@ -145,7 +146,7 @@ static struct XenDevice *xen_be_get_xendev(const char > *type, int dom, int dev, > xendev->evtchndev = xenevtchn_open(NULL, 0); > if (xendev->evtchndev == NULL) { > xen_pv_printf(NULL, 0, "can't open evtchn device\n"); > -g_free(xendev); > +qdev_unplug(DEVICE(xendev), NULL); > return NULL; > } > fcntl(xenevtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC); > @@ -155,7 +156,7 @@ static struct XenDevice *xen_be_get_xendev(const char > *type, int dom, int dev, > if (xendev->gnttabdev == NULL) { > xen_pv_printf(NULL, 0, "can't open gnttab device\n"); > xenevtchn_close(xendev->evtchndev); > -g_free(xendev); > +qdev_unplug(DEVICE(xendev), NULL); > return NULL; > } > } else { > -- > 2.10.2 > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document
On Wed, 1 Feb 2017, Julien Grall wrote: > Hi Edgar, > > On 31/01/2017 19:06, Edgar E. Iglesias wrote: > > On Tue, Jan 31, 2017 at 05:09:53PM +, Julien Grall wrote: > > > On 31/01/17 16:53, Edgar E. Iglesias wrote: > > > > On Wed, Jan 25, 2017 at 06:53:20PM +, Julien Grall wrote: > > > > > On 24/01/17 20:07, Stefano Stabellini wrote: > > > > > > On Tue, 24 Jan 2017, Julien Grall wrote: > > > > > For generic host bridge, the initialization is inexistent. However > > > > > some host > > > > > bridge (e.g xgene, xilinx) may require some specific setup and also > > > > > configuring clocks. Given that Xen only requires to access the > > > > > configuration > > > > > space, I was thinking to let DOM0 initialization the host bridge. This > > > > > would > > > > > avoid to import a lot of code in Xen, however this means that we need > > > > > to > > > > > know when the host bridge has been initialized before accessing the > > > > > configuration space. > > > > > > > > > > > > Yes, that's correct. > > > > There's a sequence on the ZynqMP that involves assiging Gigabit > > > > Transceivers > > > > to PCI (GTs are shared among PCIe, USB, SATA and the Display Port), > > > > enabling clocks and configuring a few registers to enable ECAM and MSI. > > > > > > > > I'm not sure if this could be done prior to starting Xen. Perhaps. > > > > If so, bootloaders would have to know a head of time what devices > > > > the GTs are supposed to be configured for. > > > > > > I've got further questions regarding the Gigabit Transceivers. You mention > > > they are shared, do you mean that multiple devices can use a GT at the > > > same > > > time? Or the software is deciding at startup which device will use a given > > > GT? If so, how does the software make this decision? > > > > Software will decide at startup. AFAIK, the allocation is normally done > > once but I guess that in theory you could design boards that could switch > > at runtime. I'm not sure we need to worry about that use-case though. > > > > The details can be found here: > > https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf > > > > I suggest looking at pages 672 and 733. > > Thank you for the documentation. I am trying to understand if we could move > initialization in Xen as suggested by Stefano. I looked at the driver in Linux > and the code looks simple not many dependencies. However, I was not able to > find where the Gigabit Transceivers are configured. Do you have any link to > the code for that? > > This would also mean that the MSI interrupt controller will be moved in Xen. > Which I think is a more sensible design (see more below). > > > > > > > > > - For all other host bridges => I don't know if there are host > > > > > bridges > > > > > falling under this category. I also don't have any idea how to handle > > > > > this. > > > > > > > > > > > > > > > > > Otherwise, if Dom0 is the only one to drive the physical host > > > > > > bridge, > > > > > > and Xen is the one to provide the emulated host bridge, how are DomU > > > > > > PCI > > > > > > config reads and writes supposed to work in details? > > > > > > > > > > I think I have answered to this question with my explanation above. > > > > > Let me > > > > > know if it is not the case. > > > > > > > > > > > How is MSI configuration supposed to work? > > > > > > > > > > For GICv3 ITS, the MSI will be configured with the eventID (it is uniq > > > > > per-device) and the address of the doorbell. The linkage between the > > > > > LPI and > > > > > "MSI" will be done through the ITS. > > > > > > > > > > For GICv2m, the MSI will be configured with an SPIs (or offset on some > > > > > GICv2m) and the address of the doorbell. Note that for DOM0 SPIs are > > > > > mapped > > > > > 1:1. > > > > > > > > > > So in both case, I don't think it is necessary to trap MSI > > > > > configuration for > > > > > DOM0. This may not be true if we want to handle other MSI controller. > > > > > > > > > > I have in mind the xilinx MSI controller (embedded in the host bridge? > > > > > [4]) > > > > > and xgene MSI controller ([5]). But I have no idea how they work and > > > > > if we > > > > > need to support them. Maybe Edgar could share details on the Xilinx > > > > > one? > > > > > > > > > > > > The Xilinx controller has 2 dedicated SPIs and pages for MSIs. AFAIK, > > > > there's no > > > > way to protect the MSI doorbells from mal-configured end-points raising > > > > malicious EventIDs. > > > > So perhaps trapped config accesses from domUs can help by adding this > > > > protection > > > > as drivers configure the device. > > > > > > > > On Linux, Once MSI's hit, the kernel takes the SPI interrupts, reads > > > > out the EventID from a FIFO in the controller and injects a new IRQ into > > > > the kernel. > > > > > > It might be early to ask, but how do you expect MSI to work with DOMU on > > > your hardware? Does your MSI controller supports
[Xen-devel] [xen-unstable test] 105201: regressions - FAIL
flight 105201 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/105201/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl 15 guest-start/debian.repeat fail REGR. vs. 104654 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 104681 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 104681 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 104681 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 104681 test-amd64-i386-qemut-rhel6hvm-intel 9 redhat-install fail REGR. vs. 104681 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. vs. 104681 test-amd64-i386-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 104681 test-amd64-i386-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 104681 test-amd64-i386-qemut-rhel6hvm-amd 9 redhat-install fail REGR. vs. 104681 test-amd64-i386-qemuu-rhel6hvm-amd 9 redhat-install fail REGR. vs. 104681 test-amd64-i386-qemuu-rhel6hvm-intel 9 redhat-install fail REGR. vs. 104681 test-amd64-i386-xl-qemut-winxpsp3 9 windows-install fail REGR. vs. 104681 test-amd64-i386-xl-qemuu-winxpsp3 9 windows-install fail REGR. vs. 104681 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 104681 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 104681 test-amd64-i386-xl-qemuu-win7-amd64 9 windows-install fail REGR. vs. 104681 test-amd64-i386-xl-qemut-win7-amd64 9 windows-install fail REGR. vs. 104681 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-libvirt-raw 12 saverestore-support-checkfail like 104681 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail like 104681 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stopfail like 104681 test-armhf-armhf-libvirt 13 saverestore-support-checkfail like 104681 test-amd64-amd64-xl-rtds 9 debian-install fail like 104681 test-armhf-armhf-libvirt-xsm 13 saverestore-support-checkfail like 104681 Tests which did not succeed, but are not blocking: build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 11 guest-start fail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail never pass build-arm64 5 xen-buildfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass build-arm64-pvops 5 kernel-build 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 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 12 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 11 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 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 12 migrate-support-checkfail never pass
[Xen-devel] [xtf test] 105216: all pass - PUSHED
flight 105216 xtf real [real] http://logs.test-lab.xenproject.org/osstest/logs/105216/ Perfect :-) All tests in this flight passed as required version targeted for testing: xtf c92015f8ab00026f85d187d0090fc02e8ab4cfaf baseline version: xtf 013927f81afb08e06314b66c8c8cd8549d5711c1 Last test of basis 105019 2017-01-30 14:14:51 Z2 days Testing same since 105216 2017-02-01 16:45:36 Z0 days1 attempts People who touched revisions under test: Wei Liujobs: build-amd64-xtf pass build-amd64 pass build-amd64-pvopspass test-xtf-amd64-amd64-1 pass test-xtf-amd64-amd64-2 pass test-xtf-amd64-amd64-3 pass test-xtf-amd64-amd64-4 pass test-xtf-amd64-amd64-5 pass 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=xtf + revision=c92015f8ab00026f85d187d0090fc02e8ab4cfaf + . ./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 xtf c92015f8ab00026f85d187d0090fc02e8ab4cfaf + branch=xtf + revision=c92015f8ab00026f85d187d0090fc02e8ab4cfaf + . ./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=xtf + xenbranch=xen-unstable + '[' xxtf = xlinux ']' + linuxbranch= + '[' x = x ']' + qemuubranch=qemu-upstream-unstable + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable + prevxenbranch=xen-4.8-testing + '[' xc92015f8ab00026f85d187d0090fc02e8ab4cfaf = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ : git://git.seabios.org/seabios.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git ++ : git://xenbits.xen.org/osstest/seabios.git ++ : https://github.com/tianocore/edk2.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/linux-firmware.git ++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git ++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git ++ : osst...@xenbits.xen.org:/home/xen/git/linux-pvops.git ++ : git://xenbits.xen.org/linux-pvops.git ++ : tested/linux-3.14 ++ : tested/linux-arm-xen ++ '['
Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document
Hi Edgar, On 31/01/2017 19:06, Edgar E. Iglesias wrote: On Tue, Jan 31, 2017 at 05:09:53PM +, Julien Grall wrote: On 31/01/17 16:53, Edgar E. Iglesias wrote: On Wed, Jan 25, 2017 at 06:53:20PM +, Julien Grall wrote: On 24/01/17 20:07, Stefano Stabellini wrote: On Tue, 24 Jan 2017, Julien Grall wrote: For generic host bridge, the initialization is inexistent. However some host bridge (e.g xgene, xilinx) may require some specific setup and also configuring clocks. Given that Xen only requires to access the configuration space, I was thinking to let DOM0 initialization the host bridge. This would avoid to import a lot of code in Xen, however this means that we need to know when the host bridge has been initialized before accessing the configuration space. Yes, that's correct. There's a sequence on the ZynqMP that involves assiging Gigabit Transceivers to PCI (GTs are shared among PCIe, USB, SATA and the Display Port), enabling clocks and configuring a few registers to enable ECAM and MSI. I'm not sure if this could be done prior to starting Xen. Perhaps. If so, bootloaders would have to know a head of time what devices the GTs are supposed to be configured for. I've got further questions regarding the Gigabit Transceivers. You mention they are shared, do you mean that multiple devices can use a GT at the same time? Or the software is deciding at startup which device will use a given GT? If so, how does the software make this decision? Software will decide at startup. AFAIK, the allocation is normally done once but I guess that in theory you could design boards that could switch at runtime. I'm not sure we need to worry about that use-case though. The details can be found here: https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf I suggest looking at pages 672 and 733. Thank you for the documentation. I am trying to understand if we could move initialization in Xen as suggested by Stefano. I looked at the driver in Linux and the code looks simple not many dependencies. However, I was not able to find where the Gigabit Transceivers are configured. Do you have any link to the code for that? This would also mean that the MSI interrupt controller will be moved in Xen. Which I think is a more sensible design (see more below). - For all other host bridges => I don't know if there are host bridges falling under this category. I also don't have any idea how to handle this. Otherwise, if Dom0 is the only one to drive the physical host bridge, and Xen is the one to provide the emulated host bridge, how are DomU PCI config reads and writes supposed to work in details? I think I have answered to this question with my explanation above. Let me know if it is not the case. How is MSI configuration supposed to work? For GICv3 ITS, the MSI will be configured with the eventID (it is uniq per-device) and the address of the doorbell. The linkage between the LPI and "MSI" will be done through the ITS. For GICv2m, the MSI will be configured with an SPIs (or offset on some GICv2m) and the address of the doorbell. Note that for DOM0 SPIs are mapped 1:1. So in both case, I don't think it is necessary to trap MSI configuration for DOM0. This may not be true if we want to handle other MSI controller. I have in mind the xilinx MSI controller (embedded in the host bridge? [4]) and xgene MSI controller ([5]). But I have no idea how they work and if we need to support them. Maybe Edgar could share details on the Xilinx one? The Xilinx controller has 2 dedicated SPIs and pages for MSIs. AFAIK, there's no way to protect the MSI doorbells from mal-configured end-points raising malicious EventIDs. So perhaps trapped config accesses from domUs can help by adding this protection as drivers configure the device. On Linux, Once MSI's hit, the kernel takes the SPI interrupts, reads out the EventID from a FIFO in the controller and injects a new IRQ into the kernel. It might be early to ask, but how do you expect MSI to work with DOMU on your hardware? Does your MSI controller supports virtualization? Or are you looking for a different way to inject MSI? MSI support in HW is quite limited to support domU and will require SW hacks :-( Anyway, something along the lines of this might work: * Trap domU CPU writes to MSI descriptors in config space. Force real MSI descriptors to the address of the door bell area. Force real MSI descriptors to use a specific device unique Event ID allocated by Xen. Remember what EventID domU requested per device and descriptor. * Xen or Dom0 take the real SPI generated when device writes into the doorbell area. At this point, we can read out the EventID from the MSI FIFO and map it to the one requested from domU. Xen or Dom0 inject the expected EventID into domU Do you have any good ideas? :-) From my understanding your MSI controller is embedded in the hostbridge, right? If so, the MSIs would need
[Xen-devel] [xen-unstable-smoke test] 105218: tolerable trouble: broken/fail/pass - PUSHED
flight 105218 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/105218/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64 5 xen-buildfail never pass build-arm64-pvops 5 kernel-build fail never pass 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 8844ed299a88848da88b23e6db43b5bcc4ad4bee baseline version: xen ad5808d9057248e7879cf375662f0a449fff7005 Last test of basis 105212 2017-02-01 15:01:10 Z0 days Testing same since 105218 2017-02-01 17:01:00 Z0 days1 attempts People who touched revisions under test: Andrew Cooperjobs: build-amd64 pass build-arm64 fail build-armhf pass build-amd64-libvirt pass build-arm64-pvopsfail test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass 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=8844ed299a88848da88b23e6db43b5bcc4ad4bee + . ./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 8844ed299a88848da88b23e6db43b5bcc4ad4bee + branch=xen-unstable-smoke + revision=8844ed299a88848da88b23e6db43b5bcc4ad4bee + . ./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.8-testing + '[' x8844ed299a88848da88b23e6db43b5bcc4ad4bee = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ :
Re: [Xen-devel] [PATCH] xen-netfront: Improve error handling during initialization
On 02/01/2017 10:50 AM, Ross Lagerwall wrote: > Improve error handling during initialization. This fixes a crash when > running out of grant refs when creating many queues across many netdevs. > > * Delay timer creation so that if initializing a queue fails, the timer > has not been setup yet. > * If creating queues fails (i.e. there are no grant refs available), > call xenbus_dev_fatal() to ensure that the xenbus device is set to the > closed state. > * If no queues are created, don't call xennet_disconnect_backend as > netdev->real_num_tx_queues will not have been set correctly. > * If setup_netfront() fails, ensure that all the queues created are > cleaned up, not just those that have been set up. > * If any queues were set up and an error occurs, call > xennet_destroy_queues() to stop the timer and clean up the napi context. We need to stop the timer in xennet_disconnect_backend(). I sent a patch a couple of day ago https://lists.xenproject.org/archives/html/xen-devel/2017-01/msg03269.html but was about to resend it with del_timer_sync() moved after napi_synchronize(). -boris > * If any fatal error occurs, unregister and destroy the netdev to avoid > leaving around a half setup network device. > > Signed-off-by: Ross Lagerwall> --- > drivers/net/xen-netfront.c | 39 ++- > 1 file changed, 18 insertions(+), 21 deletions(-) > > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c > index 8315fe7..8ca85af 100644 > --- a/drivers/net/xen-netfront.c > +++ b/drivers/net/xen-netfront.c > @@ -1596,9 +1596,6 @@ static int xennet_init_queue(struct netfront_queue > *queue) > spin_lock_init(>tx_lock); > spin_lock_init(>rx_lock); > > - setup_timer(>rx_refill_timer, rx_refill_timeout, > - (unsigned long)queue); > - > snprintf(queue->name, sizeof(queue->name), "%s-q%u", >queue->info->netdev->name, queue->id); > > @@ -1632,6 +1629,9 @@ static int xennet_init_queue(struct netfront_queue > *queue) > goto exit_free_tx; > } > > + setup_timer(>rx_refill_timer, rx_refill_timeout, > + (unsigned long)queue); > + > return 0; > > exit_free_tx: > @@ -1822,27 +1822,23 @@ static int talk_to_netback(struct xenbus_device *dev, > xennet_destroy_queues(info); > > err = xennet_create_queues(info, _queues); > - if (err < 0) > - goto destroy_ring; > + if (err < 0) { > + xenbus_dev_fatal(dev, err, "creating queues"); > + if (num_queues > 0) { > + goto destroy_ring; > + } else { > + kfree(info->queues); > + info->queues = NULL; > + goto out; > + } > + } > > /* Create shared ring, alloc event channel -- for each queue */ > for (i = 0; i < num_queues; ++i) { > queue = >queues[i]; > err = setup_netfront(dev, queue, feature_split_evtchn); > - if (err) { > - /* setup_netfront() will tidy up the current > - * queue on error, but we need to clean up > - * those already allocated. > - */ > - if (i > 0) { > - rtnl_lock(); > - netif_set_real_num_tx_queues(info->netdev, i); > - rtnl_unlock(); > - goto destroy_ring; > - } else { > - goto out; > - } > - } > + if (err) > + goto destroy_ring; > } > > again: > @@ -1932,9 +1928,10 @@ static int talk_to_netback(struct xenbus_device *dev, > xenbus_transaction_end(xbt, 1); > destroy_ring: > xennet_disconnect_backend(info); > - kfree(info->queues); > - info->queues = NULL; > + xennet_destroy_queues(info); > out: > + unregister_netdev(info->netdev); > + xennet_free_netdev(info->netdev); > return err; > } > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [early RFC] ARM PCI Passthrough design document
On Wed, 1 Feb 2017, Roger Pau Monné wrote: > On Wed, Jan 25, 2017 at 06:53:20PM +, Julien Grall wrote: > > Hi Stefano, > > > > On 24/01/17 20:07, Stefano Stabellini wrote: > > > On Tue, 24 Jan 2017, Julien Grall wrote: > > > > > > whilst for Device Tree the segment number is not available. > > > > > > > > > > > > So Xen needs to rely on DOM0 to discover the host bridges and > > > > > > notify Xen > > > > > > with all the relevant informations. This will be done via a new > > > > > > hypercall > > > > > > PHYSDEVOP_pci_host_bridge_add. The layout of the structure will be: > > > > > > > > > > I understand that the main purpose of this hypercall is to get Xen > > > > > and Dom0 > > > > > to > > > > > agree on the segment numbers, but why is it necessary? If Dom0 has an > > > > > emulated contoller like any other guest, do we care what segment > > > > > numbers > > > > > Dom0 will use? > > > > > > > > I was not planning to have a emulated controller for DOM0. The physical > > > > one is > > > > not necessarily ECAM compliant so we would have to either emulate the > > > > physical > > > > one (meaning multiple different emulation) or an ECAM compliant. > > > > > > > > The latter is not possible because you don't know if there is enough > > > > free MMIO > > > > space for the emulation. > > > > > > > > In the case on ARM, I don't see much the point to emulate the host > > > > bridge for > > > > DOM0. The only thing we need in Xen is to access the configuration > > > > space, we > > > > don't have about driving the host bridge. So I would let DOM0 dealing > > > > with > > > > that. > > > > > > > > Also, I don't see any reason for ARM to trap DOM0 configuration space > > > > access. > > > > The MSI will be configured using the interrupt controller and it is a > > > > trusted > > > > Domain. > > > > > > These last you sentences raise a lot of questions. Maybe I am missing > > > something. You might want to clarify the strategy for Dom0 and DomUs, > > > and how they differ, in the next version of the doc. > > > > > > At some point you wrote "Instantiation of a specific driver for the host > > > controller can be easily done if Xen has the information to detect it. > > > However, those drivers may require resources described in ASL." Does it > > > mean you plan to drive the physical host bridge from Xen and Dom0 > > > simultaneously? > > > > I may miss some bits, so feel free to correct me if I am wrong. > > > > My understanding is host bridge can be divided in 2 parts: > > - Initialization of the host bridge > > - Access the configuration space > > > > For generic host bridge, the initialization is inexistent. However some host > > bridge (e.g xgene, xilinx) may require some specific setup and also > > configuring clocks. Given that Xen only requires to access the configuration > > space, I was thinking to let DOM0 initialization the host bridge. This would > > avoid to import a lot of code in Xen, however this means that we need to > > know when the host bridge has been initialized before accessing the > > configuration space. > > Can the bridge be initialized without Dom0 having access to the ECAM area? If > that's possible I would do: > > 1. Dom0 initializes the bridge (whatever that involves). > 2. Dom0 calls PHYSDEVOP_pci_mmcfg_reserved to register the bridge with Xen: > 2.1 Xen scans the bridge and detects the devices. > 2.2 Xen maps the ECAM area into Dom0 stage-2 p2m. > 3. Dom0 scans the bridge (whatever is done on native). This doesn't seem too bad. We could live with it. But I would still consider doing the bridge initialization in Xen if it is not too inconvenient. It would be great to have dom0 be just like any other domain in regards to PCI. > > Now regarding the configuration space, I think we can divide in 2 category: > > - indirect access, the configuration space are multiplexed. An example > > would be the legacy method on x86 (e.g 0xcf8 and 0xcfc). A similar method is > > used for x-gene PCI driver ([1]). > > - ECAM like access, where each PCI configuration space will have it is > > own > > address space. I said "ECAM like" because some host bridge will require some > > bits fiddling when accessing register (see thunder-ecam [2]) > > > > There are also host bridges that mix both indirect access and ECAM like > > access depending on the device configuration space accessed (see thunder-pem > > [3]). > > Hay! Sounds like fun... > > > When using ECAM like host bridge, I don't think it will be an issue to have > > both DOM0 and Xen accessing configuration space at the same time. Although, > > we need to define who is doing what. In general case, DOM0 should not > > touched an assigned PCI device. The only possible interaction would be > > resetting a device (see my answer below). > > Iff Xen is really going to perform the reset of passthrough devices, then I > don't see any reason to expose those devices to Dom0 at all, IMHO you should > hide
Re: [Xen-devel] [PATCH] x86/dmop: Fix compat_dm_op() ABI
On Wed, 1 Feb 2017, Jan Beulich wrote: > >>> On 31.01.17 at 20:59,wrote: > > The parameter to compat_dm_op() is a pointer to an array of > > compat_dm_op_buf_t's in guest RAM. > > > > Signed-off-by: Andrew Cooper > > Reviewed-by: Jan Beulich > with one question (see below). > > > What is the pupose of COMPAT_HANDLE_PARAM()? It is a packed structure of one > > and a half pointers, so isn't safe at all for use in the hypercall function > > APIs (depsite its naming making it look deceptively like it is the correct > > thing to use). > > Stefano, you've added this in e7a527e100 ("xen: replace > XEN_GUEST_HANDLE with XEN_GUEST_HANDLE_PARAM when > appropriate"), without any user. I think it should simply be > removed. I think I added it for completeness: hypercalls parameters which are pointers are supposed to be declared using _PARAM macros, because on arm they have a different size compared to their corresponding XEN_GUEST_HANDLE type (4 bytes vs 8 bytes). Unfortunately, forgetting to convert an hypercall parameter type to _PARAM doesn't result in a build failure, but in a much less obvious runtime failure when the hypercall is called. Compat hypercalls are not used on ARM, but for consistency I thought it would be a good idea to mark their hypercall parameters as "_PARAM". To do that, I introduced the COMPAT_HANDLE_PARAM macro, whose only purpose is to be have "_PARAM" in its name. It is not necessary in compat_dm_op from an ABI point of view, but it properly marks the third parameter as an hypercall parameter. I am OK with removing COMPAT_HANDLE_PARAM, but please carefully use the appropriate XEN_GUEST_HANDLE_PARAM type (avoid directly using XEN_GUEST_HANDLE) in its stead. > > On a more serious note, why do we have all this macro infrastrucutre in the > > first place? Having spent rather longer debugging this than I to admit > > (almost mainly from the userspace side) I have concluded that it is actively > > dangerous to use; all it does is hide what is going on. > > > > What does it actually give us that the Linux route of a real C pointers and > > a > > __user attribute doesn't, other than obfuscating the code on the hypercall > > boundary? > > I think you refer to the whole handle machinery here, the main goal > of which is to avoid anyone mistakenly directly de-referencing a > pointer coming from a guest. Linux'es __user attribute doesn't > achieve this during the normal compilation stage, afaik. Additionally, XEN_GUEST_HANDLE_PARAM and XEN_GUEST_HANDLE differ on ARM. > > @@ -525,7 +525,7 @@ CHECK_dm_op_inject_msi; > > > > int compat_dm_op(domid_t domid, > > unsigned int nr_bufs, > > - COMPAT_HANDLE_PARAM(compat_dm_op_buf_t) bufs) > > + XEN_GUEST_HANDLE_PARAM(void) bufs) > > Why the change to void? I'd prefer if we kept correct types, > even if that's just for documentation purposes. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] xen/arm: Domain not fully destroyed when using credit2
On Tue, Jan 31, 2017 at 04:30:50PM +, Julien Grall wrote: > Hi Dario, > > On 25/01/17 16:00, Dario Faggioli wrote: > > On Wed, 2017-01-25 at 12:38 +, Julien Grall wrote: > > > On 25/01/17 11:10, Dario Faggioli wrote: > > And a good one. I may be wrong (I certainly wasn't around at the time), > > but ISTR out RCU code is imported/inspired by Linux... Looking there > > again may help, but, nowadays, Linux RCU subsystem is a Lernaean Hydra > > monster, with 100 heads and sharpen claws! :-O > > > > And, while, in there, it has to be like that, I don't think we need all > > such complexity, and hence we can't just re-sync. :-/ > > Yeah, even the tiny RCU code is quite complex :/. I've looked at our RCU > code and noticed there is a link in the header to [1]. > > It seems to be a documentation about the RCU code we used. From my > understanding of the "RCU Implementations", the authors are expecting a > timer to kick periodically pCPU and check if there is some RCU work pending. > > We could add this timer but it would prevent an idle pCPU to stay in low > power mode for a long time. Another solution would be to send an interrupt > to each pCPU when call_rcu is called rather depending on a mark. Although > this would still wake-up the pCPU even it was doing nothing. > > Any better ideas? > Worth checking all the RCU docs in Linux (Documentation/RCU). I think there are descriptions about idle or no-tick variants. It would be useful to know how Linux handles this. I suspect RCU in Linux is more capable than the one in Xen... Wei. > Cheers, > > [1] http://lse.sourceforge.net/locking/rcupdate.html > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] libs/gnttab: add FreeBSD handlers for the grant-table user-space device
On Wed, Feb 01, 2017 at 05:44:55PM +, Roger Pau Monne wrote: > This patch adds the headers and helpers for the FreeBSD gntdev, used in order > to map grants from remote domains and to allocate grants on behalf of the > current domain. > > Current code has been tested with the QEMU/Qdisk backend. > > Signed-off-by: Akshay Jaggi> [ added dummy stub for osdep_gnttab_grant_copy ] > Signed-off-by: Roger Pau Monné Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 7/8] golang/xenlight: Implement libxl_scheduler enumeration
On 23/01/17 16:43, Ronald Rojas wrote: > Include both constants and a Stringification for libxl_scheduler. > > Signed-off-by: George Dunlap> Signed-off-by: Ronald Rojas > --- > tools/golang/xenlight/xenlight.go | 62 > +++ > 1 file changed, 62 insertions(+) > > diff --git a/tools/golang/xenlight/xenlight.go > b/tools/golang/xenlight/xenlight.go > index 54692fd..0990f07 100644 > --- a/tools/golang/xenlight/xenlight.go > +++ b/tools/golang/xenlight/xenlight.go > @@ -211,6 +211,68 @@ type Dominfo struct { > > } > > +// # Consistent with values defined in domctl.h > +// # Except unknown which we have made up > +// libxl_scheduler = Enumeration("scheduler", [ > +// (0, "unknown"), > +// (4, "sedf"), > +// (5, "credit"), > +// (6, "credit2"), > +// (7, "arinc653"), > +// (8, "rtds"), > +// ]) > +type Scheduler int > + > +var ( > + SchedulerUnknown Scheduler = C.LIBXL_SCHEDULER_UNKNOWN > + SchedulerSedf Scheduler = C.LIBXL_SCHEDULER_SEDF > + SchedulerCredit Scheduler = C.LIBXL_SCHEDULER_CREDIT > + SchedulerCredit2 Scheduler = C.LIBXL_SCHEDULER_CREDIT2 > + SchedulerArinc653 Scheduler = C.LIBXL_SCHEDULER_ARINC653 > + SchedulerRTDS Scheduler = C.LIBXL_SCHEDULER_RTDS > +) > + > +// const char *libxl_scheduler_to_string(libxl_scheduler p); > +func (s Scheduler) String() string { > + cs := C.libxl_scheduler_to_string(C.libxl_scheduler(s)) > + // No need to free const return value > + > + return C.GoString(cs) > +} > + > +// int libxl_scheduler_from_string(const char *s, libxl_scheduler *e); > +func (s *Scheduler) FromString(gstr string) (err error) { > + cstr := C.CString(gstr) > + defer C.free(unsafe.Pointer(cstr)) > + > + var cs C.libxl_scheduler > + ret := C.libxl_scheduler_from_string(cstr, ) > + if ret != 0 { > + err = Error(-ret) > + return > + } > + > + *s = Scheduler(cs) > + return > +} > + > +func SchedulerFromString(name string) (s Scheduler, err error) { > + cname := C.CString(name) > + defer C.free(unsafe.Pointer(cname)) > + > + var cs C.libxl_scheduler > + > + ret := C.libxl_scheduler_from_string(cname, ) > + if ret != 0 { > + err = Error(-ret) > + return > + } > + > + s = Scheduler(cs) > + > + return > +} I don't think we need both of these functions. In my code I just used the method one (FromString). What do you think? BTW this file is an obvious template for the enumerations you need for DomInfo. :-) -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 6/8] golang/xenlight: Implement libxl_bitmap and helper operations
On 23/01/17 16:43, Ronald Rojas wrote: > Implement Bitmap type, along with helper functions. > > The Bitmap type is implemented interllay in a way which makes it > easy to copy into and out of the C libxl_bitmap type. > > Signed-off-by: George Dunlap> Signed-off-by: Ronald Rojas > --- > tools/golang/xenlight/xenlight.go | 166 > ++ > 1 file changed, 166 insertions(+) > > diff --git a/tools/golang/xenlight/xenlight.go > b/tools/golang/xenlight/xenlight.go > index dd6893c..54692fd 100644 > --- a/tools/golang/xenlight/xenlight.go > +++ b/tools/golang/xenlight/xenlight.go > @@ -131,6 +131,19 @@ func (chwcap C.libxl_hwcap)CToGo() (ghwcap Hwcap) { > return > } > > +// typedef struct { > +// uint32_t size; /* number of bytes in map */ > +// uint8_t *map; > +// } libxl_bitmap; > + > +// Implement the Go bitmap type such that the underlying data can > +// easily be copied in and out. NB that we still have to do copies > +// both directions, because cgo runtime restrictions forbid passing to > +// a C function a pointer to a Go-allocated structure which contains a > +// pointer. > +type Bitmap struct { > + bitmap []C.uint8_t > +} > > /* > * Types: IDL > @@ -199,6 +212,159 @@ type Dominfo struct { > } > > /* > + * Bitmap operations > + */ > + > +// Return a Go bitmap which is a copy of the referred C bitmap. > +func (cbm C.libxl_bitmap) CToGo() (gbm Bitmap) { Probably just toGo()... > + // Alloc a Go slice for the bytes > + size := int(cbm.size) > + gbm.bitmap = make([]C.uint8_t, size) > + > + // Make a slice pointing to the C array > + mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size] > + > + // And copy the C array into the Go array > + copy(gbm.bitmap, mapslice) > + > + return > +} > + > +// Must be C.libxl_bitmap_dispose'd of afterwards > +func (gbm Bitmap)GoToC() (cbm C.libxl_bitmap) { ...and toC (lower case so they're not visible outside the package). Also 'go fmt'. > + C.libxl_bitmap_init() > + > + size := len(gbm.bitmap) > + cbm._map = (*C.uint8_t)(C.malloc(C.size_t(size))) > + cbm.size = C.uint32_t(size) > + if cbm._map == nil { > + panic("C.calloc failed!") malloc (This is probably my typo). Other than that, looks good. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] VT-d/RMRR: Adjust the return values of register_one_rmrr()
Adjust/manage the return values of register_one_rmrr() such that new callers log errors for non-debug builds too, while not affecting the behavior of original callers. Signed-off-by: Venu Busireddy--- xen/drivers/passthrough/vtd/dmar.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c index 7c12b17..b5baa7f 100644 --- a/xen/drivers/passthrough/vtd/dmar.c +++ b/xen/drivers/passthrough/vtd/dmar.c @@ -619,6 +619,7 @@ static int register_one_rmrr(struct acpi_rmrr_unit *rmrru) rmrru->base_address, rmrru->end_address); scope_devices_free(>scope); xfree(rmrru); +return 1; } else if ( rmrru->base_address > rmrru->end_address ) { @@ -840,13 +841,13 @@ static int __init acpi_parse_dmar(struct acpi_table_header *table) entry_header->type); break; } -if ( ret ) +if ( ret < 0 ) break; entry_header = ((void *)entry_header + entry_header->length); } -if ( ret ) +if ( ret < 0 ) { printk(XENLOG_WARNING "Failed to parse ACPI DMAR. Disabling VT-d.\n"); @@ -856,7 +857,7 @@ static int __init acpi_parse_dmar(struct acpi_table_header *table) out: /* Zap ACPI DMAR signature to prevent dom0 using vt-d HW. */ acpi_dmar_zap(); -return ret; +return ret < 0 ? ret : 0; } #define MAX_USER_RMRR_PAGES 16 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] libs/gnttab: add FreeBSD handlers for the grant-table user-space device
This patch adds the headers and helpers for the FreeBSD gntdev, used in order to map grants from remote domains and to allocate grants on behalf of the current domain. Current code has been tested with the QEMU/Qdisk backend. Signed-off-by: Akshay Jaggi[ added dummy stub for osdep_gnttab_grant_copy ] Signed-off-by: Roger Pau Monné --- Cc: Wei Liu Cc: Ian Jackson --- Changes since v1: - Move some shared defines/macros to the private.h header. - Coding style fixes. --- tools/include/xen-sys/FreeBSD/gntdev.h | 191 tools/libs/gnttab/Makefile | 2 +- tools/libs/gnttab/freebsd.c| 315 + tools/libs/gnttab/linux.c | 9 - tools/libs/gnttab/private.h| 10 ++ 5 files changed, 517 insertions(+), 10 deletions(-) create mode 100644 tools/include/xen-sys/FreeBSD/gntdev.h create mode 100644 tools/libs/gnttab/freebsd.c diff --git a/tools/include/xen-sys/FreeBSD/gntdev.h b/tools/include/xen-sys/FreeBSD/gntdev.h new file mode 100644 index 000..f3af9a4 --- /dev/null +++ b/tools/include/xen-sys/FreeBSD/gntdev.h @@ -0,0 +1,191 @@ +/*- + * Copyright (c) 2016 Akshay Jaggi + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + *notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + *notice, this list of conditions and the following disclaimer in the + *documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * + * gntdev.h + * + * Interface to /dev/xen/gntdev. + * + * This device provides the user with two kinds of functionalities: + * 1. Grant Allocation + *Allocate a page of our own memory, and share it with a foreign domain. + * 2. Grant Mapping + *Map a grant allocated by a foreign domain, into our own memory. + * + * + * Grant Allocation + * + * Steps to allocate a grant: + * 1. Do an `IOCTL_GNTDEV_ALLOC_GREF ioctl`, with + * - `domid`, as the domain-id of the foreign domain + * - `flags`, ORed with GNTDEV_ALLOC_FLAG_WRITABLE if you want the foreign + * domain to have write access to the shared memory + * - `count`, with the number of pages to share with the foreign domain + * + *Ensure that the structure you allocate has enough memory to store + *all the allocated grant-refs, i.e., you need to allocate + *(sizeof(struct ioctl_gntdev_alloc_gref) + (count - 1)*sizeof(uint32_t)) + *bytes of memory. + * + * 2. Mmap the address given in `index` after a successful ioctl. + *This will give you access to the granted pages. + * + * Note: + * 1. The grant is not removed until all three of the following conditions + *are met + * - The region is not mmaped. That is, munmap() has been called if + * the region was mmapped previously. + * - IOCTL_GNTDEV_DEALLOC_GREF ioctl has been performed. After you + * perform this ioctl, you can no longer mmap or set notify on + * the grant. + * - The foreign domain has stopped using the grant. + * 2. Granted pages can only belong to one mmap region. + * 3. Every page of granted memory is a unit in itself. What this means + *is that you can set a unmap notification for each of the granted + *pages, individually; you can mmap and dealloc-ioctl a contiguous + *range of allocated grants (even if alloc-ioctls were performed + *individually), etc. + * + * + * Grant Mapping + * + * Steps to map a grant: + * 1. Do a `IOCTL_GNTDEV_MAP_GRANT_REF` ioctl, with + * - `count`, as the number of foreign grants to map + * - `refs[i].domid`, as the domain id of the foreign domain + * - `refs[i].ref`, as the grant-ref for the grant to be mapped + * + * 2. Mmap the address given in `index` after a successful ioctl. + *This will give you access to the mapped pages. + * + * Note: + * 1. The map
Re: [Xen-devel] [PATCH RFC 5/8] golang/xenlight: Add tests host related functinality functions
On 23/01/17 16:43, Ronald Rojas wrote: > Create tests for the following functions: > - GetVersionInfo > - GetPhysinfo > - GetDominfo > - GetMaxCpus > - GetOnlineCpus > - GetMaxNodes > - GetFreeMemory > > Signed-off-by: Ronald RojasHey Ronald, Sorry this has taken a long time to get to. I think building the test cases has been really good for testing out the package *as a package*; and I think the general approach is a really good one. One big comment first: Rather than having a separate makefile for each of these, what about putting all the tests of a similar type -- like these, which are primarily comparing the output of the functions -- in the same directory, and using a makefile like the attached? (You may have to modify the XEN_ROOT variable at the top to reflect where you end up putting it.) Then, as promised, a whole slew of nitpicks. Quick heads-up: It's easy the first time you have your code nitpicked like this to hear an angry or annoyed "tone", because the comments are really short. That's not what's intended; it's just to save time. > diff --git a/tools/golang/xenlight/test/dominfo/dominfo.c > b/tools/golang/xenlight/test/dominfo/dominfo.c > new file mode 100644 > index 000..ebe18c3 > --- /dev/null > +++ b/tools/golang/xenlight/test/dominfo/dominfo.c > @@ -0,0 +1,29 @@ > +#include > +#include > +#include > + > +char * bool_to_string(bool b); Need a newline here. And I suspect you'll be using this function again -- maybe you could put it in a .h file as a static inline? > +int main(){ > + > +libxl_ctx *context; > +libxl_ctx_alloc(,LIBXL_VERSION, 0, NULL); > +libxl_dominfo info; > +int err = libxl_domain_info(context, , 0); In the Xen tools style you normally declare all the variables together at the top of the function, and then put a newline between them. It's probably a good idea to read tools/libxl/CODING_STYLE, particularly the "Formating and naming" section. Also don't forget to call libxl_dominfo_init() (which will set some defaults). > +if(err != 0){ > +return err; > +} The tools style says there should be spaces around the parentheses, and that single-line blocks don't need braces; i.e.: if (err != 0) return err; > + printf("%d\n%d\n", info.domid, info.ssidref); > + printf("%s\n%s\n%s\n%s\n%s\n%s\n", bool_to_string(info.running), > bool_to_string(info.blocked ), bool_to_string(info.paused > ),bool_to_string(info.shutdown ),bool_to_string(info.dying), > bool_to_string(info.never_stop)); > + long cpu_time = info.cpu_time/((long)1<<35); Why are we clipping this value? > + printf("%d\n%lu\n%lu\n%lu\n%lu\n%lu\n%lu\n%d\n%d\n%d\n", > info.shutdown_reason, info.outstanding_memkb, info.current_memkb, > info.shared_memkb, info.paged_memkb, info.max_memkb, cpu_time, > info.vcpu_max_id, info.vcpu_online, info.cpupool); > + printf("%d\n", info.domain_type); These lines need to be wrapped to 80 columns or less. > + > + > +libxl_ctx_free(context); > + > +} > +char * bool_to_string(bool b){ > +return b ? "true": "false"; > +} > diff --git a/tools/golang/xenlight/test/dominfo/dominfo.go > b/tools/golang/xenlight/test/dominfo/dominfo.go > new file mode 100644 > index 000..e6c6a64 > --- /dev/null > +++ b/tools/golang/xenlight/test/dominfo/dominfo.go > @@ -0,0 +1,48 @@ > + > +package main > + > +/* > +#cgo LDFLAGS: -lxenlight -lyajl > +#include > +#include > +*/ > +import "C" This stanza isn't necessary -- all that should be hidden behind the package. (Same for all the other .go files.) > +import ( > + "fmt" > + "xenproject.org/xenlight" > + "os" > +) > + > +func main() { > + ctx := xenlight.Ctx I think this should probably be "var ctx Xenlight.Context". Copying a struct which has a pointer in it is usually a bad idea; if you're going to use your own copy, you should just use your own copy. :-) (Same for all the other files.) I also think we want a newline here. > + err := ctx.Open() > + if err != nil{ Run 'go fmt' to put spaces where they need to be. (Same for all the other files.) > + os.Exit(-1) > + } > + defer ctx.Close() Newline. > + info, err:= ctx.DomainInfo(0) > + if err != nil{ > + os.Exit(-1) > + } > + > + fmt.Printf("%d\n%d\n", info.Domid, info.Ssidref) > + //fmt.Printf("%s\n", info.SsidLabel) Why are we not printing this (both here and in the .c file)? > + fmt.Printf("%t\n%t\n%t\n%t\n%t\n%t\n", info.Running, info.Blocked, > info.Paused, info.Shutdown, info.Dying, info.NeverStop) These lines should be wrapped to 80 characters. > + cpuTime := info.CpuTime/(1<<35) > + fmt.Printf("%d\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n", > info.ShutdownReason, info.OutstandingMemkb, info.CurrentMemkb, > info.SharedMemkb, info.PagedMemkb, info.MaxMemkb, cpuTime, info.VcpuMaxId, > info.VcpuOnline, info.Cpupool) > + fmt.Printf("%d\n", info.DomainType) > + > +} > + > +func
Re: [Xen-devel] [PATCH v3 09/11] fuzz/x86emul: update fuzzer
This patch will be squashed in. From 4990d760f900223e32cb800e7374ee45d357081b Mon Sep 17 00:00:00 2001 From: Wei LiuDate: Wed, 1 Feb 2017 16:54:58 + Subject: [PATCH] fixup! fuzz/x86emul: update fuzzer --- .../x86-insn-emulator-fuzzer.c | 18 -- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c index edc5130b3b..ed43935600 100644 --- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c @@ -294,17 +294,12 @@ static int fuzz_read_cr( unsigned long *val, struct x86_emulate_ctxt *ctxt) { -int rc; - if ( reg >= ARRAY_SIZE(input.cr) ) return X86EMUL_UNHANDLEABLE; -rc = maybe_fail("read_cr", true); +*val = input.cr[reg]; -if ( rc == X86EMUL_OKAY ) -*val = input.cr[reg]; - -return rc; +return X86EMUL_OKAY; } static int fuzz_write_cr( @@ -312,17 +307,12 @@ static int fuzz_write_cr( unsigned long val, struct x86_emulate_ctxt *ctxt) { -int rc; - if ( reg >= ARRAY_SIZE(input.cr) ) return X86EMUL_UNHANDLEABLE; -rc = maybe_fail("write_cr", true); +input.cr[reg] = val; -if ( rc == X86EMUL_OKAY ) -input.cr[reg] = val; - -return rc; +return X86EMUL_OKAY; } enum { -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 105212: tolerable trouble: broken/fail/pass - PUSHED
flight 105212 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/105212/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64 5 xen-buildfail never pass build-arm64-pvops 5 kernel-build fail never pass 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 ad5808d9057248e7879cf375662f0a449fff7005 baseline version: xen 9d3226aa060916f21df3be98abb39f11496ab8ce Last test of basis 105207 2017-02-01 12:01:13 Z0 days Testing same since 105212 2017-02-01 15:01:10 Z0 days1 attempts People who touched revisions under test: Dario Faggiolijobs: build-amd64 pass build-arm64 fail build-armhf pass build-amd64-libvirt pass build-arm64-pvopsfail test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass 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=ad5808d9057248e7879cf375662f0a449fff7005 + . ./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 ad5808d9057248e7879cf375662f0a449fff7005 + branch=xen-unstable-smoke + revision=ad5808d9057248e7879cf375662f0a449fff7005 + . ./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.8-testing + '[' xad5808d9057248e7879cf375662f0a449fff7005 = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ :
Re: [Xen-devel] [PATCH v3] xen/arm: fix rank/vgic lock inversion bug
Hi Stefano, CC Andre to get more feedback. On 31/01/2017 23:49, Stefano Stabellini wrote: On Fri, 27 Jan 2017, Julien Grall wrote: On 03/01/17 23:29, Stefano Stabellini wrote: Always set the new physical irq affinity at the beginning of vgic_migrate_irq, in all cases. No need to set physical irq affinity in gic_update_one_lr anymore, solving the lock inversion problem. After migrating an interrupt from vcpu/pcpu 0 to vcpu/pcpu 1, it is possible to receive a physical interrupt on pcpu 1, which Xen is supposed to inject into vcpu 1, before the LR on pcpu 0 has been cleared. In this case the irq is still marked as GIC_IRQ_GUEST_MIGRATING, and struct pending_irq is still "inflight" on vcpu 0. As the irq cannot be "inflight" on vcpu 0 and vcpu 1 simultaneously, drop the interrupt. I am not sure to understand how this is related to the fix of the rank/vgic lock inversion bug. Am I right to think that this bug was already present? Yes, it is related: without this patch, this situation cannot happen, because irq_set_affinity is called from gic_update_one_lr. I am not convinced about that. gic_update_one_lr will take the lock of the current vCPU whilst vgic_vcpu_inject_irq is taking the lock of the vCPU where the vIRQ has been routed. The function the memory access of the list_del_init could potentially be re-ordered with irq_set_affinity. However, what is saving us is the memory barrier hidden in spin_lock call in gicv{2,3}_irq_set_affinity and the other one in vgic_vcpu_inject_irq. So I agree this is currently working, but I would argue it is by luck. [...] -vgic_vcpu_inject_irq(new, irq); The comment on top of the if says: "If the IRQ is still lr_pending, re-inject it to the new vCPU". However, you remove interrupt from the list but never re-inject it. Looking at the context, I think we should keep the call to vgic_vcpu_inject_irq otherwise we lost the interrupt for ever. It looks like I made a mistake while generating the patch: I meant to remove the redundant irq_set_affinity call, instead of the necessary vgic_vcpu_inject_irq call. I don't know how it happened, sorry. Thanks for catching the error. return; } /* if the IRQ is in a GICH_LR register, set GIC_IRQ_GUEST_MIGRATING @@ -495,6 +491,13 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int virq) return; } +if ( test_bit(GIC_IRQ_GUEST_MIGRATING, >status) ) +{ +/* Drop the interrupt, because it is still inflight on another vcpu */ +spin_unlock_irqrestore(>arch.vgic.lock, flags); If I understand correctly, the problem arise because LRs are cleared lazily and the other vCPU is still running. It is a short window and I don't think should happen often. I would suggest to kick the other vCPU with an SGI to get the LRs cleared. Any opinions? I am fine with sending an SGI, but we still need http://marc.info/?l=xen-devel=148237295020488 to know the destination of the SGI. Hmmm correct. The problem is what happens next: is it safe to busy-wait until the LR is cleared? Technically we already have this problem as LRs are cleared with the vgic lock taken. So if by mistake you receive the interrupt on the wrong pCPU, you may busy wait on the spinlock for some time. Note, I am not saying we should do the same here :). I would argue it would depends how much time you expect to be taken for clearing the LRs. > It is safe only if we cannot receive a second interrupt notification while the first one has not been deactivated yet by the guest: - guest vcpu0 reads GICC_IAR for virq0 - guest vcpu1 change target for virq0 to vcpu1 - Xen traps the write and changes the physical affinity of virq0 (pirq0) to vcpu1 (pcpu1) - Xen receives a second virq0 (pirq0) interrupt notification in pcpu1, but virq0 is still MIGRATING and guest vcpu0 has not EOIed it yet - Xen sends an SGI to vcpu0 (pcpu0), but the interrupt has not been EOIed yet, thus, the LR doesn't get cleared Can this happen? I read 3.2.3 and 3.2.4a few times but I am still unsure, it looks like it can. If it can happen, we cannot busy-wait. With 3.2.3 and 3.2.4, you are referring to the spec IHI 0048B, right? I will leave aside PPIs and SGIs as they target one specific CPU and migration will never happen. So we have to worry about SPIs and LPIs (thought they are not yet supported). From the note in 3.2.3: "For any processor, if an interrupt is active and pending, the GIC does not signal an interrupt exception request for the interrupt to any processor until the active status is cleared." Given that SPIs have an activate state and will be shared, this scenario cannot happen. For LPIs, there is no activate state. So as soon as they are EOIed, they might come up again. Depending on how will we handle irq migration, your scenario will become true. I am not sure if we should take into account LPIs right now. To be honest, I don't much like the idea of kicking the other vCPU.
Re: [Xen-devel] [XTF PATCH] build: disable PIE during linking if necessary
On 01/02/17 16:27, Jan Beulich wrote: On 01.02.17 at 17:21,wrote: >> --- a/build/common.mk >> +++ b/build/common.mk >> @@ -50,6 +50,15 @@ obj-perarch := >> obj-perenv := >> include $(ROOT)/build/files.mk >> >> + >> +cc-option = $(shell if [ -z "`echo 'int p=1;' | $(CC) $(1) -S -o /dev/null >> -x c - 2>&1`" ]; \ >> +then echo y; else echo n; fi) >> + >> +# Disable PIE, but need to check if compiler supports it >> +ifeq ($(call cc-option,-no-pie),y) >> +DISABLE_PIE_LDFLAGS := -no-pie >> +endif > How about > > LDFLAGS-$(call cc-option,-no-pie) += -no-pie > > and ... > >> @@ -61,7 +70,7 @@ CFLAGS_$(1) := $$(CFLAGS_$($(1)_arch)) >> $$(COMMON_CFLAGS-$(1)) -DCONFIG_ENV_$(1) >> >> link-$(1) := $(ROOT)/arch/x86/link-$(1).lds >> >> -LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib >> +LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib $(DISABLE_PIE_LDFLAGS) > LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib $(LDFLAGS-y) > > (or something along those lines)? Ok - will fix up to this suggestion. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [XTF PATCH] build: disable PIE during linking if necessary
>>> On 01.02.17 at 17:21,wrote: > --- a/build/common.mk > +++ b/build/common.mk > @@ -50,6 +50,15 @@ obj-perarch := > obj-perenv := > include $(ROOT)/build/files.mk > > + > +cc-option = $(shell if [ -z "`echo 'int p=1;' | $(CC) $(1) -S -o /dev/null > -x c - 2>&1`" ]; \ > + then echo y; else echo n; fi) > + > +# Disable PIE, but need to check if compiler supports it > +ifeq ($(call cc-option,-no-pie),y) > +DISABLE_PIE_LDFLAGS := -no-pie > +endif How about LDFLAGS-$(call cc-option,-no-pie) += -no-pie and ... > @@ -61,7 +70,7 @@ CFLAGS_$(1) := $$(CFLAGS_$($(1)_arch)) > $$(COMMON_CFLAGS-$(1)) -DCONFIG_ENV_$(1) > > link-$(1) := $(ROOT)/arch/x86/link-$(1).lds > > -LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib > +LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib $(DISABLE_PIE_LDFLAGS) LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib $(LDFLAGS-y) (or something along those lines)? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [XTF PATCH] build: disable PIE during linking if necessary
On Wed, Feb 01, 2017 at 04:23:53PM +, Andrew Cooper wrote: > On 01/02/17 16:21, Wei Liu wrote: > > Starting from ee3e265688, $(CC) is used for linking. That means all > > default $(CC) flags coming from distro takes effect. > > > > On Debian Stretch, gcc contains -pie by default, which makes the final > > object fail to link. We need to explicitly disable PIE when linking. > > Since not all versions of gcc support -no-pie, test its availability > > before adding. > > > > Example error message: > > > > /usr/bin/ld: /local/work/xtf.git/arch/x86/boot/head_pv64.o: relocation > > R_X86_64_32S against symbol `start_info' can not be used when making a > > shared object; > > recompile with -fPIC > > /usr/bin/ld: /local/work/xtf.git/arch/x86/entry_64-pv64.o: relocation > > R_X86_64_32S against `.text' can not be used when making a shared object; > > recompile with > > -fPIC > > /usr/bin/ld: final link failed: Nonrepresentable section on output > > collect2: error: ld returned 1 exit status > > > > Signed-off-by: Wei Liu> > LGTM. Any objection if I s/DISABLE_PIE_LDFLAGS/COMMON_LDFLAGS/ on > commit? I don't think the name needs to be that specific. NP. Do whatever you like. I just want to get it fixed. ;-) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [XTF PATCH] build: disable PIE during linking if necessary
On 01/02/17 16:21, Wei Liu wrote: > Starting from ee3e265688, $(CC) is used for linking. That means all > default $(CC) flags coming from distro takes effect. > > On Debian Stretch, gcc contains -pie by default, which makes the final > object fail to link. We need to explicitly disable PIE when linking. > Since not all versions of gcc support -no-pie, test its availability > before adding. > > Example error message: > > /usr/bin/ld: /local/work/xtf.git/arch/x86/boot/head_pv64.o: relocation > R_X86_64_32S against symbol `start_info' can not be used when making a shared > object; > recompile with -fPIC > /usr/bin/ld: /local/work/xtf.git/arch/x86/entry_64-pv64.o: relocation > R_X86_64_32S against `.text' can not be used when making a shared object; > recompile with > -fPIC > /usr/bin/ld: final link failed: Nonrepresentable section on output > collect2: error: ld returned 1 exit status > > Signed-off-by: Wei LiuLGTM. Any objection if I s/DISABLE_PIE_LDFLAGS/COMMON_LDFLAGS/ on commit? I don't think the name needs to be that specific. ~Andrew > --- > build/common.mk | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/build/common.mk b/build/common.mk > index 7c4870e..8e08c7e 100644 > --- a/build/common.mk > +++ b/build/common.mk > @@ -50,6 +50,15 @@ obj-perarch := > obj-perenv := > include $(ROOT)/build/files.mk > > + > +cc-option = $(shell if [ -z "`echo 'int p=1;' | $(CC) $(1) -S -o /dev/null > -x c - 2>&1`" ]; \ > + then echo y; else echo n; fi) > + > +# Disable PIE, but need to check if compiler supports it > +ifeq ($(call cc-option,-no-pie),y) > +DISABLE_PIE_LDFLAGS := -no-pie > +endif > + > # Run once per environment to set up some common bits & pieces > define PERENV_setup > > @@ -61,7 +70,7 @@ CFLAGS_$(1) := $$(CFLAGS_$($(1)_arch)) > $$(COMMON_CFLAGS-$(1)) -DCONFIG_ENV_$(1) > > link-$(1) := $(ROOT)/arch/x86/link-$(1).lds > > -LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib > +LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib $(DISABLE_PIE_LDFLAGS) > > # Needs to pick up test-provided obj-perenv and obj-perarch > DEPS-$(1) = $(head-$(1)) \ ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [XTF PATCH] build: disable PIE during linking if necessary
Starting from ee3e265688, $(CC) is used for linking. That means all default $(CC) flags coming from distro takes effect. On Debian Stretch, gcc contains -pie by default, which makes the final object fail to link. We need to explicitly disable PIE when linking. Since not all versions of gcc support -no-pie, test its availability before adding. Example error message: /usr/bin/ld: /local/work/xtf.git/arch/x86/boot/head_pv64.o: relocation R_X86_64_32S against symbol `start_info' can not be used when making a shared object; recompile with -fPIC /usr/bin/ld: /local/work/xtf.git/arch/x86/entry_64-pv64.o: relocation R_X86_64_32S against `.text' can not be used when making a shared object; recompile with -fPIC /usr/bin/ld: final link failed: Nonrepresentable section on output collect2: error: ld returned 1 exit status Signed-off-by: Wei Liu--- build/common.mk | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/build/common.mk b/build/common.mk index 7c4870e..8e08c7e 100644 --- a/build/common.mk +++ b/build/common.mk @@ -50,6 +50,15 @@ obj-perarch := obj-perenv := include $(ROOT)/build/files.mk + +cc-option = $(shell if [ -z "`echo 'int p=1;' | $(CC) $(1) -S -o /dev/null -x c - 2>&1`" ]; \ + then echo y; else echo n; fi) + +# Disable PIE, but need to check if compiler supports it +ifeq ($(call cc-option,-no-pie),y) +DISABLE_PIE_LDFLAGS := -no-pie +endif + # Run once per environment to set up some common bits & pieces define PERENV_setup @@ -61,7 +70,7 @@ CFLAGS_$(1) := $$(CFLAGS_$($(1)_arch)) $$(COMMON_CFLAGS-$(1)) -DCONFIG_ENV_$(1) link-$(1) := $(ROOT)/arch/x86/link-$(1).lds -LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib +LDFLAGS_$(1) := -Wl,-T,$$(link-$(1)) -nostdlib $(DISABLE_PIE_LDFLAGS) # Needs to pick up test-provided obj-perenv and obj-perarch DEPS-$(1) = $(head-$(1)) \ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 4/8] golang/xenlight: Implement libxl_domain_info and libxl_domain_unpause
On 23/01/17 16:43, Ronald Rojas wrote: > Add calls for the following host-related functionality: > - libxl_domain_info > - libxl_domain_unpause > > Include Golang version for the libxl_domain_info as > DomainInfo. > > Signed-off-by: George Dunlap> Signed-off-by: Ronald Rojas > --- > tools/golang/xenlight/xenlight.go | 92 > +++ > 1 file changed, 92 insertions(+) > > diff --git a/tools/golang/xenlight/xenlight.go > b/tools/golang/xenlight/xenlight.go > index 92410a8..dd6893c 100644 > --- a/tools/golang/xenlight/xenlight.go > +++ b/tools/golang/xenlight/xenlight.go > @@ -36,6 +36,7 @@ import "C" > import ( > "fmt" > "unsafe" > + "time" > ) > > /* > @@ -104,6 +105,12 @@ var errors = [...]string{ > * Types: Builtins > */ > > +type Domid uint32 > + > +type MemKB uint64 > + > +type Uuid C.libxl_uuid > + > type Context struct { > ctx *C.libxl_ctx > } > @@ -165,6 +172,32 @@ type VersionInfo struct { > BuildId string > } > > +type Dominfo struct { > + Uuid Uuid > + Domid Domid > + Ssidref uint32 > + SsidLabel string > + Runningbool > + Blockedbool > + Paused bool > + Shutdown bool > + Dying bool > + NeverStop bool Run 'go fmt' to align these. > + > + ShutdownReason int32 // FIXME shutdown_reason enumeration > + OutstandingMemkb MemKB > + CurrentMemkb MemKB > + SharedMemkb MemKB > + PagedMemkb MemKB > + MaxMemkb MemKB > + CpuTime time.Duration > + VcpuMaxId uint32 > + VcpuOnline uint32 > + Cpupool uint32 > + DomainType int32 //FIXME libxl_domain_type enumeration We need to have these enumerations actually created as consts, like we do with the errors. We should also make a type for these enums, and a String method that calls the libxl__to_string() function to convert it to a string. > + > +} > + > /* > * Context > */ > @@ -343,3 +376,62 @@ func (Ctx *Context) GetVersionInfo() (info *VersionInfo, > err error) { > > return > } > + > +func (Ctx *Context) DomainInfo(Id Domid) (di *Dominfo, err error) { > + err = Ctx.CheckOpen() > + if err != nil { > + return > + } > + > + var cdi C.libxl_dominfo > + > + ret := C.libxl_domain_info(Ctx.ctx, unsafe.Pointer(), > C.uint32_t(Id)) > + > + if ret != 0 { > + err = Error(-ret) > + return > + } > + > + // We could consider having this boilerplate generated by the > + // idl, in a function like this: > + // > + // di = translateCdomaininfoToGoDomaininfo(cdi) Stale comment; you could remove this, or make it cdi.toGo(). > + di = {} > + di.Uuid = Uuid(cdi.uuid) > + di.Domid = Domid(cdi.domid) > + di.Ssidref = uint32(cdi.ssidref) > + di.SsidLabel = C.GoString(cdi.ssid_label) And here we'll probably leak memory if we don't call libxl_dominfo_dispose() before returning. Everything else looks good, thanks! -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 3/8] golang/xenlight: Add host-related functionality
On 23/01/17 16:43, Ronald Rojas wrote: > Add calls for the following host-related functionality: > - libxl_get_max_cpus > - libxl_get_online_cpus > - libxl_get_max_nodes > - libxl_get_free_memory > - libxl_get_physinfo > - libxl_get_version_info > > Include Golang versions of the following structs: > - libxl_physinfo as Physinfo > - libxl_version_info as VersionInfo > - libxl_hwcap as Hwcap > > Signed-off-by: Ronald Rojas> --- > tools/golang/xenlight/xenlight.go | 186 > ++ > 1 file changed, 186 insertions(+) > > diff --git a/tools/golang/xenlight/xenlight.go > b/tools/golang/xenlight/xenlight.go > index eee2254..92410a8 100644 > --- a/tools/golang/xenlight/xenlight.go > +++ b/tools/golang/xenlight/xenlight.go > @@ -108,6 +108,63 @@ type Context struct { > ctx *C.libxl_ctx > } > > +type Hwcap []C.uint32_t > + > +func (chwcap C.libxl_hwcap)CToGo() (ghwcap Hwcap) { Sorry... I'm coming back to this after a bit of time and remembering a bunch of things as I'm going along. I don't think we want this method to be available publicly, so this should probably be 'cToGo' (Or perhaps just 'toGo', since it's already a C structure.) -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 3/8] golang/xenlight: Add host-related functionality
On 23/01/17 16:43, Ronald Rojas wrote: > Add calls for the following host-related functionality: > - libxl_get_max_cpus > - libxl_get_online_cpus > - libxl_get_max_nodes > - libxl_get_free_memory > - libxl_get_physinfo > - libxl_get_version_info > > Include Golang versions of the following structs: > - libxl_physinfo as Physinfo > - libxl_version_info as VersionInfo > - libxl_hwcap as Hwcap > > Signed-off-by: Ronald RojasMostly looks good! One final comment: [snip] > +//int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo) > +func (Ctx *Context) GetPhysinfo() (physinfo *Physinfo, err error) { > + err = Ctx.CheckOpen() > + if err != nil { > + return > + } > + var cphys C.libxl_physinfo > + > + ret := C.libxl_get_physinfo(Ctx.ctx, ) So I actually forgot that you're actually supposed to always call "libxl__init()" on IDL-generated types before using them, and "libxl__dispose()" when you're done with them. As it happens in this case, the struct contains no pointers to sub-structs; but if that changes in the future, this would leak the memory. Everything else is good, thanks. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH RFC 3/8] golang/xenlight: Add host-related functionality
On 23/01/17 16:43, Ronald Rojas wrote: > Add calls for the following host-related functionality: > - libxl_get_max_cpus > - libxl_get_online_cpus > - libxl_get_max_nodes > - libxl_get_free_memory > - libxl_get_physinfo > - libxl_get_version_info > > Include Golang versions of the following structs: > - libxl_physinfo as Physinfo > - libxl_version_info as VersionInfo > - libxl_hwcap as Hwcap > > Signed-off-by: Ronald RojasOh, last thing: It's probably worth running "go fmt xenlight.go" for each patch. That will catch little things like lack of spaces between things, and aligning the structures nicely. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xen-netfront: Improve error handling during initialization
Improve error handling during initialization. This fixes a crash when running out of grant refs when creating many queues across many netdevs. * Delay timer creation so that if initializing a queue fails, the timer has not been setup yet. * If creating queues fails (i.e. there are no grant refs available), call xenbus_dev_fatal() to ensure that the xenbus device is set to the closed state. * If no queues are created, don't call xennet_disconnect_backend as netdev->real_num_tx_queues will not have been set correctly. * If setup_netfront() fails, ensure that all the queues created are cleaned up, not just those that have been set up. * If any queues were set up and an error occurs, call xennet_destroy_queues() to stop the timer and clean up the napi context. * If any fatal error occurs, unregister and destroy the netdev to avoid leaving around a half setup network device. Signed-off-by: Ross Lagerwall--- drivers/net/xen-netfront.c | 39 ++- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 8315fe7..8ca85af 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -1596,9 +1596,6 @@ static int xennet_init_queue(struct netfront_queue *queue) spin_lock_init(>tx_lock); spin_lock_init(>rx_lock); - setup_timer(>rx_refill_timer, rx_refill_timeout, - (unsigned long)queue); - snprintf(queue->name, sizeof(queue->name), "%s-q%u", queue->info->netdev->name, queue->id); @@ -1632,6 +1629,9 @@ static int xennet_init_queue(struct netfront_queue *queue) goto exit_free_tx; } + setup_timer(>rx_refill_timer, rx_refill_timeout, + (unsigned long)queue); + return 0; exit_free_tx: @@ -1822,27 +1822,23 @@ static int talk_to_netback(struct xenbus_device *dev, xennet_destroy_queues(info); err = xennet_create_queues(info, _queues); - if (err < 0) - goto destroy_ring; + if (err < 0) { + xenbus_dev_fatal(dev, err, "creating queues"); + if (num_queues > 0) { + goto destroy_ring; + } else { + kfree(info->queues); + info->queues = NULL; + goto out; + } + } /* Create shared ring, alloc event channel -- for each queue */ for (i = 0; i < num_queues; ++i) { queue = >queues[i]; err = setup_netfront(dev, queue, feature_split_evtchn); - if (err) { - /* setup_netfront() will tidy up the current -* queue on error, but we need to clean up -* those already allocated. -*/ - if (i > 0) { - rtnl_lock(); - netif_set_real_num_tx_queues(info->netdev, i); - rtnl_unlock(); - goto destroy_ring; - } else { - goto out; - } - } + if (err) + goto destroy_ring; } again: @@ -1932,9 +1928,10 @@ static int talk_to_netback(struct xenbus_device *dev, xenbus_transaction_end(xbt, 1); destroy_ring: xennet_disconnect_backend(info); - kfree(info->queues); - info->queues = NULL; + xennet_destroy_queues(info); out: + unregister_netdev(info->netdev); + xennet_free_netdev(info->netdev); return err; } -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] acpi: Switch to dynamic mapping at SYS_STATE_boot
>>> On 01.02.17 at 15:31,wrote: > We can switch ACPI from using fixmap to dynamic mapping as soon as > the system enters SYS_STATE_boot. This will allow us, for example, > to map MADT on systems with large number of processors where the > table might not fit into NUM_FIXMAP_ACPI_PAGES (currently set to 4). > > To avoid having a window between system entering SYS_STATE_boot and > vmap area being initialized move vm_init() a little higher. > > Signed-off-by: Boris Ostrovsky Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback
On Wed, Feb 01, 2017 at 02:30:53PM +, Julien Grall wrote: > Hi Konrad, > > On 01/02/2017 14:25, Konrad Rzeszutek Wilk wrote: > > On Wed, Feb 01, 2017 at 12:46:32PM +0200, Andrii Anisov wrote: > > > Yup, > > > > > > Following is wrong: > > > > DEBUG macro could be globally across drivers defined by > > > > CONFIG_DEBUG_DRIVERS. See drivers/base/Makefile. > > > DEBUG is not defined globally. And there is no debug option to enable > > > DEBUG in drivers/xen/Makefile. > > > Should it be added? > > > > I am not exactly sure why you guys insist on having this, but > > the lets leave it as #if 0 along with a comment saying why and what > > the purpose of this is. > > From my understanding, the function has been implemented with the assumption > of the page will always belongs to the domain and not foreign. > > A user will unlikely know that it needs to remove #if 0 in order to check > the driver is doing the right thing. So I think we should have an easy way > to enable this code in kernel build. > > If you are not happy with the Kconfig, what would be the other way to have > this enabled without requiring the user to modify the code? Just leave the #if 0 in the code and add a comment explaining why it is there. > > Cheers, > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle()
On 26/01/17 01:00, Dario Faggioli wrote: > On Wed, 2017-01-18 at 03:30 -0700, Jan Beulich wrote: > On 18.01.17 at 11:21,wrote: >>> On 18/01/17 00:30, Dario Faggioli wrote: index ef8e0d8..d086264 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -985,7 +985,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now) cpumask_andnot(, >active, >idle); cpumask_andnot(, , >tickled); cpumask_and(, , new->vcpu->cpu_hard_affinity); -if ( cpumask_test_cpu(cpu, ) ) +if ( __cpumask_test_and_clear_cpu(cpu, ) ) >>> >>> Since we're micro-optimizing -- isn't test-and-clear a locked >>> operation? >>> Would that be more expensive than the if() statement below? >> >> cpumask_test_and_clear_cpu() is, but __cpumask_test_and_clear_cpu() >> isn't. >> > George, ping? Yes, this looks fine then. But it didn't apply cleanly when I tried to apply it -- please re-send it with the other patches you have outstanding. Thanks. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: sched: improve debug dump output.
On 26/01/17 16:52, Dario Faggioli wrote: > Scheduling information debug dump for Credit2 is hard > to read as it contains the same information repeated > multiple time in different ways. > > In fact, in Credit2, CPUs are grouped in runqueus. > Here's the current debug output: > > CPU[00] sibling=,0003, core=,00ff > run: [32767.0] flags=0 cpu=0 credit=-1073741824 [w=0] load=0 (~0%) > 1: [0.3] flags=0 cpu=2 credit=3273410 [w=256] load=262144 (~100%) > 2: [0.4] flags=0 cpu=2 credit=2974954 [w=256] load=262144 (~100%) > CPU[01] sibling=,0003, core=,00ff > run: [32767.1] flags=0 cpu=1 credit=-1073741824 [w=0] load=0 (~0%) > 1: [0.3] flags=0 cpu=2 credit=3273410 [w=256] load=262144 (~100%) > 2: [0.4] flags=0 cpu=2 credit=2974954 [w=256] load=262144 (~100%) > CPU[02] sibling=,000c, core=,00ff > run: [0.2] flags=2 cpu=2 credit=3556909 [w=256] load=262144 (~100%) > 1: [0.3] flags=0 cpu=2 credit=3273410 [w=256] load=262144 (~100%) > 2: [0.4] flags=0 cpu=2 credit=2974954 [w=256] load=262144 (~100%) > > Here, CPUs 0, 1 and 2, are all part of runqueue 0, > the content of which (which, BTW, is d0v3 and d0v4) > is printed 3 times! It is also not very useful to > see the details of the idle vcpus, as they're always > the same (except for the vCPU ids). > > With this change, we print: > - pCPUs details and, for non idle ones, what vCPU >they're running; > - the runqueue content, once and for all. > > Runqueue 0: > CPU[00] runq=0, sibling=,0003, core=,00ff > run: [0.15] flags=2 cpu=0 credit=5804742 [w=256] load=3655 (~1%) > CPU[01] runq=0, sibling=,0003, core=,00ff > CPU[02] runq=0, sibling=,000c, core=,00ff > run: [0.3] flags=2 cpu=2 credit=6674856 [w=256] load=262144 (~100%) > CPU[03] runq=0, sibling=,000c, core=,00ff > RUNQ: > 0: [0.1] flags=0 cpu=2 credit=6561215 [w=256] load=262144 (~100%) > 1: [0.2] flags=0 cpu=2 credit=5812356 [w=256] load=262144 (~100%) > > Stop printing details of idle vCPUs also in Credit1 > and RTDS (they're pretty useless in there too). > > Signed-off-by: Dario FaggioliHang on -- how does this relate to the credit2 dumping patch you sent the previous week? I'm all in favor of not saving up a massive queue of patches, but if the patches have conflicts with existing patches, I think it would be better if you re-sent a full series with a new version number, so I wouldn't have to try to figure out what order they needed to be applied in and which ones weren't necessary anymore. There's no problem, I don't think, of sending version n+1 of a series that has no changes other than adding new patches. I've checked in and pushed the three patches that I reviewed today; can you rebase your outstanding changes and send another series? Thanks! And sorry it took so long to get to. -George ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3] xenstore: remove XS_RESTRICT support
On 27/01/17 12:47, Juergen Gross wrote: > XS_RESTRICT and the xenstore library function xs_restrict() have never > been usable in all configurations and there are no known users. > > This functionality was thought to limit access rights of device models > to xenstore in order to avoid affecting other domains in case of a > security breech. Unfortunately XS_RESTRICT won't help as current > qemu is requiring access to dom0 only accessible xenstore paths to > work correctly. So this command is useless and should be removed. > > In order to avoid problems in the future remove all support for > XS_RESTRICT from xenstore. > > Signed-off-by: Juergen GrossAdding Dave Scott to Cc: list. Juergen > --- > V3: remove restrict functions in ocaml as requested by Wei Liu > > V2: don't replace XS_RESTRICT enum member with a dummy one as suggested > by Andrew Cooper > --- > tools/ocaml/libs/xb/op.ml | 5 ++--- > tools/ocaml/libs/xb/xb.mli | 1 - > tools/ocaml/xenstored/connection.ml | 3 --- > tools/ocaml/xenstored/logging.ml| 1 - > tools/ocaml/xenstored/perms.ml | 5 - > tools/ocaml/xenstored/process.ml| 16 > tools/xenstore/include/xenstore.h | 9 - > tools/xenstore/xenstored_core.c | 1 - > tools/xenstore/xs.c | 8 > xen/include/public/io/xs_wire.h | 4 ++-- > 10 files changed, 4 insertions(+), 49 deletions(-) > > diff --git a/tools/ocaml/libs/xb/op.ml b/tools/ocaml/libs/xb/op.ml > index 69346d8..d4f1f08 100644 > --- a/tools/ocaml/libs/xb/op.ml > +++ b/tools/ocaml/libs/xb/op.ml > @@ -19,7 +19,7 @@ type operation = Debug | Directory | Read | Getperms | > Transaction_end | Introduce | Release | > Getdomainpath | Write | Mkdir | Rm | > Setperms | Watchevent | Error | Isintroduced | > - Resume | Set_target | Restrict | Reset_watches | > + Resume | Set_target | Reset_watches | > Invalid > > let operation_c_mapping = > @@ -28,7 +28,7 @@ let operation_c_mapping = > Transaction_end; Introduce; Release; > Getdomainpath; Write; Mkdir; Rm; > Setperms; Watchevent; Error; Isintroduced; > - Resume; Set_target; Restrict; Reset_watches |] > + Resume; Set_target; Reset_watches |] > let size = Array.length operation_c_mapping > > let array_search el a = > @@ -68,6 +68,5 @@ let to_string ty = > | Isintroduced -> "IS_INTRODUCED" > | Resume-> "RESUME" > | Set_target-> "SET_TARGET" > - | Restrict -> "RESTRICT" > | Reset_watches -> "RESET_WATCHES" > | Invalid -> "INVALID" > diff --git a/tools/ocaml/libs/xb/xb.mli b/tools/ocaml/libs/xb/xb.mli > index 6c242da..b4d7052 100644 > --- a/tools/ocaml/libs/xb/xb.mli > +++ b/tools/ocaml/libs/xb/xb.mli > @@ -22,7 +22,6 @@ module Op : >| Isintroduced >| Resume >| Set_target > - | Restrict >| Reset_watches >| Invalid > val operation_c_mapping : operation array > diff --git a/tools/ocaml/xenstored/connection.ml > b/tools/ocaml/xenstored/connection.ml > index 3ffd35b..27fa778 100644 > --- a/tools/ocaml/xenstored/connection.ml > +++ b/tools/ocaml/xenstored/connection.ml > @@ -122,9 +122,6 @@ let close con = > let get_perm con = > con.perm > > -let restrict con domid = > - con.perm <- Perms.Connection.restrict con.perm domid > - > let set_target con target_domid = > con.perm <- Perms.Connection.set_target (get_perm con) > ~perms:[Perms.READ; Perms.WRITE] target_domid > > diff --git a/tools/ocaml/xenstored/logging.ml > b/tools/ocaml/xenstored/logging.ml > index c52f03d..0c0d03d 100644 > --- a/tools/ocaml/xenstored/logging.ml > +++ b/tools/ocaml/xenstored/logging.ml > @@ -241,7 +241,6 @@ let string_of_access_type = function > | Xenbus.Xb.Op.Mkdir -> "mkdir" > | Xenbus.Xb.Op.Rm-> "rm " > | Xenbus.Xb.Op.Setperms -> "setperms " > - | Xenbus.Xb.Op.Restrict -> "restrict " > | Xenbus.Xb.Op.Reset_watches -> "reset watches" > | Xenbus.Xb.Op.Set_target-> "settarget" > > diff --git a/tools/ocaml/xenstored/perms.ml b/tools/ocaml/xenstored/perms.ml > index 19bf44c..3ea193e 100644 > --- a/tools/ocaml/xenstored/perms.ml > +++ b/tools/ocaml/xenstored/perms.ml > @@ -119,11 +119,6 @@ let is_owner (connection:t) id = > let is_dom0 (connection:t) = > is_owner connection 0 > > -let restrict (connection:t) domid = > - match connection.target, connection.main with > - | None, (0, perms) -> { connection with main = (domid, perms) } > - | _-> raise Define.Permission_denied > - > let elt_to_string (i,p) = > Printf.sprintf "%i%S" i (String.concat "" (List.map String.of_char > (List.map char_of_permty p))) > >
Re: [Xen-devel] [PATCH] xen: credit2: non Credit2 pCPUs are ok during shutdown/suspend.
On 28/01/17 01:42, Dario Faggioli wrote: > Commit 7478ebe1602e6 ("xen: credit2: fix shutdown/suspend > when playing with cpupools"), while doing the right thing > for actual code, forgot to update the ASSERT()s accordingly, > in csched2_vcpu_migrate(). > > In fact, as stated there already, during shutdown/suspend, > we must allow a Credit2 vCPU to temporarily migrate to a > non Credit2 BSP, without any ASSERT() triggering. > > Move them down, after the check for whether or not we are > shutting down, where the assumption that the pCPU must be > valid Credit2 ones, is valid. > > Signed-off-by: Dario Faggioli> --- > Cc: George Dunlap > Cc: Anshul Makkar > Cc: Jan Beulich > --- > If 7478ebe1602e is backported, this one should be as well. > > Sorry for this. I'm sure I tested, so I don't know how it could happened... I > must have forgotten debug=n when testing this case. :-( Indeed, and sorry I missed this on review as well. Reviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/tools: tracing: credits can go negative, so use int.
On 18/01/17 11:32, Dario Faggioli wrote: > For Credit2, in both the trace records, inside Xen, > and in their parsing, in xenalyze. > > In fact, as it is quite a bit better, in order to > understand how much negative credits have gone for > a certain vCPU, to see an actual negative number, > as compared to a wrapped around unsigned! > > Signed-off-by: Dario FaggioliReviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2] acpi: Switch to dynamic mapping at SYS_STATE_boot
We can switch ACPI from using fixmap to dynamic mapping as soon as the system enters SYS_STATE_boot. This will allow us, for example, to map MADT on systems with large number of processors where the table might not fit into NUM_FIXMAP_ACPI_PAGES (currently set to 4). To avoid having a window between system entering SYS_STATE_boot and vmap area being initialized move vm_init() a little higher. Signed-off-by: Boris Ostrovsky--- Changes in v2: * Swap vm_init() and system_state assignment. xen/arch/x86/setup.c | 3 ++- xen/drivers/acpi/osl.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 0ccef1d..ccfd6dd 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1376,9 +1376,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) else end_boot_allocator(); +vm_init(); + system_state = SYS_STATE_boot; -vm_init(); console_init_ring(); vesa_init(); diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c index 3616dfd..7199047 100644 --- a/xen/drivers/acpi/osl.c +++ b/xen/drivers/acpi/osl.c @@ -89,7 +89,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void) void __iomem * acpi_os_map_memory(acpi_physical_address phys, acpi_size size) { - if (system_state >= SYS_STATE_active) { + if (system_state >= SYS_STATE_boot) { mfn_t mfn = _mfn(PFN_DOWN(phys)); unsigned int offs = phys & (PAGE_SIZE - 1); @@ -104,7 +104,7 @@ acpi_os_map_memory(acpi_physical_address phys, acpi_size size) void acpi_os_unmap_memory(void __iomem * virt, acpi_size size) { - if (system_state >= SYS_STATE_active) + if (system_state >= SYS_STATE_boot) vunmap((void *)((unsigned long)virt & PAGE_MASK)); } -- 1.8.3.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback
Hi Konrad, On 01/02/2017 14:25, Konrad Rzeszutek Wilk wrote: On Wed, Feb 01, 2017 at 12:46:32PM +0200, Andrii Anisov wrote: Yup, Following is wrong: DEBUG macro could be globally across drivers defined by CONFIG_DEBUG_DRIVERS. See drivers/base/Makefile. DEBUG is not defined globally. And there is no debug option to enable DEBUG in drivers/xen/Makefile. Should it be added? I am not exactly sure why you guys insist on having this, but the lets leave it as #if 0 along with a comment saying why and what the purpose of this is. From my understanding, the function has been implemented with the assumption of the page will always belongs to the domain and not foreign. A user will unlikely know that it needs to remove #if 0 in order to check the driver is doing the right thing. So I think we should have an easy way to enable this code in kernel build. If you are not happy with the Kconfig, what would be the other way to have this enabled without requiring the user to modify the code? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: credit2: improve debug dump output.
On 18/01/17 01:10, Dario Faggioli wrote: > Scheduling information debug dump for Credit2 is hard > to read as it contains the same information repeated > multiple time in different ways. > > In fact, in Credit2, CPUs are grouped in runqueus. Before > this change, for each CPU, we were printing the while > content of the runqueue, as shown below: > > CPU[00] sibling=03, core=ff > run: [32767.0] flags=0 cpu=0 credit=-1073741824 [w=0] load=0 (~0%) > 1: [0.0] flags=0 cpu=2 credit=3860932 [w=256] load=262144 (~100%) > 2: [0.1] flags=0 cpu=2 credit=3859906 [w=256] load=262144 (~100%) > CPU[01] sibling=03, core=ff > run: [32767.1] flags=0 cpu=1 credit=-1073741824 [w=0] load=0 (~0%) > 1: [0.0] flags=0 cpu=2 credit=2859840 [w=256] load=262144 (~100%) > 2: [0.3] flags=0 cpu=2 credit=-17466062 [w=256] load=262144 (~100%) > CPU[02] sibling=0c, core=ff > run: [0.0] flags=2 cpu=2 credit=1858628 [w=256] load=262144 (~100%) > 1: [0.3] flags=0 cpu=2 credit=-17466062 [w=256] load=262144 (~100%) > 2: [0.1] flags=0 cpu=2 credit=-23957055 [w=256] load=262144 (~100%) > CPU[03] sibling=0c, core=ff > run: [32767.3] flags=0 cpu=3 credit=-1073741824 [w=0] load=0 (~0%) > 1: [0.1] flags=0 cpu=2 credit=-3957055 [w=256] load=262144 (~100%) > 2: [0.0] flags=0 cpu=2 credit=-6216254 [w=256] load=262144 (~100%) > CPU[04] sibling=30, core=ff > run: [32767.4] flags=0 cpu=4 credit=-1073741824 [w=0] load=0 (~0%) > 1: [0.1] flags=0 cpu=2 credit=3782667 [w=256] load=262144 (~100%) > 2: [0.3] flags=0 cpu=2 credit=-16287483 [w=256] load=262144 (~100%) > > As it can be seen, all the CPUs print the whole content > of the runqueue they belong to, at the time of their > sampling, and this is cumbersome and hard to interpret! > > In new output format we print, for each CPU, only the vCPU > that is running there (if that's not the idle vCPU, in which > case, nothing is printed), while the runqueues content > is printed only once, in a dedicated section. > > An example: > > CPUs info: > CPU[02] runq=0, sibling=0c, core=ff > run: [0.3] flags=2 cpu=2 credit=8054391 [w=256] load=262144 (~100%) > CPU[14] runq=1, sibling=00c000, core=00ff00 > run: [0.4] flags=2 cpu=14 credit=8771420 [w=256] load=262144 (~100%) > ... ... ... ... ... ... ... ... ... > Runqueue info: > runqueue 0: > 0: [0.1] flags=0 cpu=2 credit=7869771 [w=256] load=262144 (~100%) > 1: [0.0] flags=0 cpu=2 credit=7709649 [w=256] load=262144 (~100%) > runqueue 1: > 0: [0.5] flags=0 cpu=14 credit=-1188 [w=256] load=262144 (~100%) > > Note that there still is risk of inconsistency between > what is printed in the 'Runqueue info:' and in 'CPUs info:' > sections. That is unavoidable, as the relevant locks are > released and re-acquired, around each single operation. > > At least, the inconsistency is less severe than before. > > Signed-off-by: Dario FaggioliReviewed-by: George Dunlap ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback
On Wed, Feb 01, 2017 at 12:46:32PM +0200, Andrii Anisov wrote: > Yup, > > Following is wrong: > > DEBUG macro could be globally across drivers defined by > > CONFIG_DEBUG_DRIVERS. See drivers/base/Makefile. > DEBUG is not defined globally. And there is no debug option to enable > DEBUG in drivers/xen/Makefile. > Should it be added? I am not exactly sure why you guys insist on having this, but the lets leave it as #if 0 along with a comment saying why and what the purpose of this is. > > Sincerely, > Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/2] swiotlb-xen: implement xen_swiotlb_dma_mmap callback
On Wed, Feb 01, 2017 at 01:46:48PM +0200, Andrii Anisov wrote: > Konrad, Stefano, > > >> What does 'local pages' mean? > > > > A page that belongs to this domain, rather than a foreign page that has > > been mapped. > I guess it should be clarified, but not sure in the commit message or > comment to the code? in the code pls. > > Sincerely, > Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: Kconfig: Add DEBUG_XEN config option
On Wed, Feb 01, 2017 at 02:12:13PM +0100, Juergen Gross wrote: > On 01/02/17 13:55, Julien Grall wrote: > > Hi Juergen, > > > > On 01/02/2017 12:17, Juergen Gross wrote: > >> On 01/02/17 12:19, Andrii Anisov wrote: > >>> From: Andrii Anisov> >>> > >>> Add a debug option to enable xen drivers debug code. > >>> > >>> Signed-off-by: Andrii Anisov > >> > >> In future you might want to add the maintainers to the recipient list. > >> > >>> --- > >>> drivers/xen/Kconfig | 8 > >>> drivers/xen/Makefile | 2 ++ > >>> 2 files changed, 10 insertions(+) > >>> > >>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig > >>> index f15bb3b7..733c433 100644 > >>> --- a/drivers/xen/Kconfig > >>> +++ b/drivers/xen/Kconfig > >>> @@ -298,4 +298,12 @@ config XEN_SYMS > >>> config XEN_HAVE_VPMU > >>> bool > >>> > >>> +config DEBUG_XEN > >>> +bool "XEN Drivers debug" > >>> +depends on DEBUG_KERNEL > >>> +help > >>> + Say Y here if you want to enable XEN drivers debug code. > >>> + > >>> + If you are unsure about this, say N here. > >> > >> So is this really a sensible thing to do? I don't see the value for > >> such a global config option possibly switching so many different > >> drivers to debug mode. > >> > >> In reality you want to do something like "debug" for a single driver > >> only during time of development. This won't need a global config > >> option but just a "#define" in the driver which is active while > >> developing and testing and which should be removed or commented out > >> when the final submission of the driver is done. > > > > This Kconfig was suggested in the context of [1]. The number of people > > working on swiotlb is very limited, but the check added is really useful > > in debug build to catch potential misuse for anyone. > > > > Do you have any other idea to turn this check on for debug build?? > > I think for this use case we would want either a more specific config > option name (e.g. DEBUG_XEN_SWIOTLB) or a more specific description > what it is doing, like e.g.: I really don't want to add this in the Xen SWIOTLB. It is kind of pointless - I would prefer if the debug code had just #if 0 with a comment explaining why it is such and what it can be used for. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-smoke test] 105207: tolerable trouble: broken/fail/pass - PUSHED
flight 105207 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/105207/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass build-arm64 5 xen-buildfail never pass build-arm64-pvops 5 kernel-build fail never pass 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 9d3226aa060916f21df3be98abb39f11496ab8ce baseline version: xen 5a44ff111d2e1ba9f375936294117ff4e263c4ac Last test of basis 105158 2017-01-31 23:01:03 Z0 days Testing same since 105207 2017-02-01 12:01:13 Z0 days1 attempts People who touched revisions under test: Wei Liujobs: build-amd64 pass build-arm64 fail build-armhf pass build-amd64-libvirt pass build-arm64-pvopsfail test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm broken test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass 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=9d3226aa060916f21df3be98abb39f11496ab8ce + . ./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 9d3226aa060916f21df3be98abb39f11496ab8ce + branch=xen-unstable-smoke + revision=9d3226aa060916f21df3be98abb39f11496ab8ce + . ./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.8-testing + '[' x9d3226aa060916f21df3be98abb39f11496ab8ce = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ :
Re: [Xen-devel] [PATCH v3 04/11] x86emul/test: use x86-vendors.h
On Wed, Feb 01, 2017 at 01:13:44PM +, Andrew Cooper wrote: > On 01/02/17 13:09, Jan Beulich wrote: > On 01.02.17 at 13:02,wrote: > >> --- a/tools/fuzz/x86_instruction_emulator/Makefile > >> +++ b/tools/fuzz/x86_instruction_emulator/Makefile > >> @@ -11,10 +11,15 @@ endif > >> x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > >>[ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > >> > >> +asm/x86-vendors.h: > >> + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm > > Hmm, that's making all headers available. I don't really like it to > > be that broad. Andrew, what do you think > > This is just a test utility. I think we can trust ourselves to only > include appropriate headers. > > This is simpler and easier than keeping a whitelist of permitted > headers, which doesn't require any modification when we add a new header. > Yes, this is exactly the reason why I did it like this. Wei. > ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 04/11] x86emul/test: use x86-vendors.h
>>> On 01.02.17 at 14:13,wrote: > On 01/02/17 13:09, Jan Beulich wrote: > On 01.02.17 at 13:02, wrote: >>> --- a/tools/fuzz/x86_instruction_emulator/Makefile >>> +++ b/tools/fuzz/x86_instruction_emulator/Makefile >>> @@ -11,10 +11,15 @@ endif >>> x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: >>> [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . >>> >>> +asm/x86-vendors.h: >>> + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm >> Hmm, that's making all headers available. I don't really like it to >> be that broad. Andrew, what do you think > > This is just a test utility. I think we can trust ourselves to only > include appropriate headers. > > This is simpler and easier than keeping a whitelist of permitted > headers, which doesn't require any modification when we add a new header. Well, okay then (albeit this "doesn't require any modification" doesn't hold with the dependency the patch also adds; perhaps midterm we should arrange for the compiler to generate dependencies for us). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [linux-linus test] 105176: regressions - FAIL
flight 105176 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/105176/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-credit2 17 guest-localmigrate/x10fail REGR. vs. 59254 test-amd64-i386-xl-xsm 17 guest-localmigrate/x10fail REGR. vs. 59254 test-amd64-i386-xl 17 guest-localmigrate/x10fail REGR. vs. 59254 test-amd64-amd64-xl-multivcpu 16 guest-saverestore.2 fail REGR. vs. 59254 test-amd64-i386-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 59254 test-armhf-armhf-libvirt 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-xsm 17 guest-localmigrate/x10fail REGR. vs. 59254 test-amd64-amd64-xl 17 guest-localmigrate/x10fail REGR. vs. 59254 test-armhf-armhf-xl-arndale 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-credit2 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 15 guest-localmigrate/x10 fail REGR. vs. 59254 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 59254 test-armhf-armhf-libvirt-xsm 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-multivcpu 6 xen-boot fail REGR. vs. 59254 test-armhf-armhf-xl-xsm 6 xen-boot fail REGR. vs. 59254 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 13 guest-localmigrate fail REGR. vs. 59254 test-armhf-armhf-xl-rtds 6 xen-boot fail REGR. vs. 59254 test-amd64-amd64-xl-rtds 9 debian-installfail REGR. vs. 59254 test-armhf-armhf-libvirt-raw 6 xen-bootfail baseline untested test-armhf-armhf-xl-vhd 6 xen-bootfail baseline untested test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline untested test-amd64-i386-libvirt-xsm 14 guest-saverestorefail blocked in 59254 test-amd64-amd64-libvirt-xsm 14 guest-saverestorefail blocked in 59254 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop fail like 59254 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail like 59254 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-xl-rtds 1 build-check(1) blocked n/a test-arm64-arm64-xl-multivcpu 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64 5 xen-buildfail never pass test-amd64-amd64-xl-pvh-amd 11 guest-start fail never pass test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail never pass test-amd64-i386-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 12 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail never pass build-arm64-xsm 5 xen-buildfail never pass test-amd64-amd64-libvirt 12 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2 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 version targeted for testing: linuxa2ca3d617944417e9dd5f09fc8a4549cda115f4f baseline version: linux45820c294fe1b1a9df495d57f40585ef2d069a39 Last test of basis59254 2015-07-09 04:20:48 Z 573 days Failing since 59348 2015-07-10 04:24:05 Z 572 days 239 attempts Testing same since 105176 2017-02-01 01:47:46 Z0 days1 attempts 7548 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm fail
Re: [Xen-devel] [PATCH v3 04/11] x86emul/test: use x86-vendors.h
On 01/02/17 13:09, Jan Beulich wrote: On 01.02.17 at 13:02,wrote: >> --- a/tools/fuzz/x86_instruction_emulator/Makefile >> +++ b/tools/fuzz/x86_instruction_emulator/Makefile >> @@ -11,10 +11,15 @@ endif >> x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: >> [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . >> >> +asm/x86-vendors.h: >> +[ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm > Hmm, that's making all headers available. I don't really like it to > be that broad. Andrew, what do you think This is just a test utility. I think we can trust ourselves to only include appropriate headers. This is simpler and easier than keeping a whitelist of permitted headers, which doesn't require any modification when we add a new header. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: Kconfig: Add DEBUG_XEN config option
On 01/02/17 13:55, Julien Grall wrote: > Hi Juergen, > > On 01/02/2017 12:17, Juergen Gross wrote: >> On 01/02/17 12:19, Andrii Anisov wrote: >>> From: Andrii Anisov>>> >>> Add a debug option to enable xen drivers debug code. >>> >>> Signed-off-by: Andrii Anisov >> >> In future you might want to add the maintainers to the recipient list. >> >>> --- >>> drivers/xen/Kconfig | 8 >>> drivers/xen/Makefile | 2 ++ >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig >>> index f15bb3b7..733c433 100644 >>> --- a/drivers/xen/Kconfig >>> +++ b/drivers/xen/Kconfig >>> @@ -298,4 +298,12 @@ config XEN_SYMS >>> config XEN_HAVE_VPMU >>> bool >>> >>> +config DEBUG_XEN >>> +bool "XEN Drivers debug" >>> +depends on DEBUG_KERNEL >>> +help >>> + Say Y here if you want to enable XEN drivers debug code. >>> + >>> + If you are unsure about this, say N here. >> >> So is this really a sensible thing to do? I don't see the value for >> such a global config option possibly switching so many different >> drivers to debug mode. >> >> In reality you want to do something like "debug" for a single driver >> only during time of development. This won't need a global config >> option but just a "#define" in the driver which is active while >> developing and testing and which should be removed or commented out >> when the final submission of the driver is done. > > This Kconfig was suggested in the context of [1]. The number of people > working on swiotlb is very limited, but the check added is really useful > in debug build to catch potential misuse for anyone. > > Do you have any other idea to turn this check on for debug build?? I think for this use case we would want either a more specific config option name (e.g. DEBUG_XEN_SWIOTLB) or a more specific description what it is doing, like e.g.: "Say Y here if you want to enable XEN drivers debug code adding more sanity checks to Xen drivers eventually crashing the kernel in case of detected inconsistencies. Enabling this option might slow down the kernel. It is not appropriate for production. If you are unsure about this, say N here." This would make clear that this option should not be used to spam the console with thousands of messages by dozens of different drivers, but for catching potential bugs early, and shipping a kernel with this option enabled is not a good idea. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 02/11] x86: extract macros to x86-defns.h
>>> On 01.02.17 at 13:02,wrote: > --- a/xen/include/asm-x86/processor.h > +++ b/xen/include/asm-x86/processor.h > @@ -16,6 +16,8 @@ > #include > #endif > > +#include "x86-defns.h" This should be . With that adjusted, Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o
>>> On 01.02.17 at 13:02,wrote: > --- a/tools/tests/x86_emulator/Makefile > +++ b/tools/tests/x86_emulator/Makefile > @@ -45,7 +45,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > > HOSTCFLAGS += $(CFLAGS_xeninclude) > > -x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c > x86_emulate/x86_emulate.h > +x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c > x86_emulate/x86_emulate.h > $(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $< > > test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h I think this latter one should be updated too. Which then calls for macroizing: x86_emulate.h := x86_emulate.h x86_emulate/x86_emulate.h Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: Kconfig: Add DEBUG_XEN config option
Dear Juergen, > In future you might want to add the maintainers to the recipient list. Yep, I would take care to not miss this point. And +1 to Julien's comment. Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: Kconfig: Add DEBUG_XEN config option
Hi Juergen, On 01/02/2017 12:17, Juergen Gross wrote: On 01/02/17 12:19, Andrii Anisov wrote: From: Andrii AnisovAdd a debug option to enable xen drivers debug code. Signed-off-by: Andrii Anisov In future you might want to add the maintainers to the recipient list. --- drivers/xen/Kconfig | 8 drivers/xen/Makefile | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index f15bb3b7..733c433 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -298,4 +298,12 @@ config XEN_SYMS config XEN_HAVE_VPMU bool +config DEBUG_XEN + bool "XEN Drivers debug" + depends on DEBUG_KERNEL + help + Say Y here if you want to enable XEN drivers debug code. + + If you are unsure about this, say N here. So is this really a sensible thing to do? I don't see the value for such a global config option possibly switching so many different drivers to debug mode. In reality you want to do something like "debug" for a single driver only during time of development. This won't need a global config option but just a "#define" in the driver which is active while developing and testing and which should be removed or commented out when the final submission of the driver is done. This Kconfig was suggested in the context of [1]. The number of people working on swiotlb is very limited, but the check added is really useful in debug build to catch potential misuse for anyone. Do you have any other idea to turn this check on for debug build?? Cheers, [1] https://lists.xen.org/archives/html/xen-devel/2017-01/msg03448.html -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 10/11] fuzz/x86emul: print out minimal input size
... so that users can know how big the initial input should be. Signed-off-by: Wei LiuReviewed-by: Jan Beulich --- .../fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c | 8 tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c| 5 + 2 files changed, 13 insertions(+) diff --git a/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c index 494c23ba2e..16edbd6bab 100644 --- a/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c +++ b/tools/fuzz/x86_instruction_emulator/afl-x86-insn-emulator-fuzzer.c @@ -2,8 +2,10 @@ #include #include #include +#include extern int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size); +extern unsigned int fuzz_minimal_input_size(void); #define INPUT_SIZE 4096 static uint8_t input[INPUT_SIZE]; @@ -21,6 +23,12 @@ int main(int argc, char **argv) exit(-1); } +if ( !strcmp(argv[1], "--min-input-size") ) +{ +printf("%u\n", fuzz_minimal_input_size()); +exit(0); +} + fp = fopen(argv[1], "rb"); if ( fp == NULL ) { diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c index 4c404ceb81..edc5130b3b 100644 --- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c @@ -717,6 +717,11 @@ int LLVMFuzzerTestOneInput(const uint8_t *data_p, size_t size) return 0; } +unsigned int fuzz_minimal_input_size(void) +{ +return DATA_OFFSET + 1; +} + /* * Local variables: * mode: C -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 11/11] fuzz: update README.afl example
Signed-off-by: Wei LiuAcked-by: Jan Beulich --- tools/fuzz/README.afl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/fuzz/README.afl b/tools/fuzz/README.afl index 431b4a8cbf..68e0fa396f 100644 --- a/tools/fuzz/README.afl +++ b/tools/fuzz/README.afl @@ -20,9 +20,10 @@ Use the x86 instruction emulator fuzzer as an example. $ make distclean $ make CC=$AFLPATH/afl-gcc afl # produces afl-x86-insn-emulator-fuzzer -3. provide initial test case: +3. provide initial test case (fuzzer dependent, see afl-*.c): $ mkdir testcase_dir - $ echo -n -e '\xc3' > testcase_dir/ret.bin + $ dd if=/dev/urandom of=testcase_dir/rand.bin \ + bs=`./afl-x86-insn-emulator-fuzzer --min-input-size` count=1 4. run the fuzzer with AFL: $ $AFLPATH/afl-fuzz -m none -t 1000 -i testcase_dir -o findings_dir -- \ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen: Kconfig: Add DEBUG_XEN config option
On 01/02/17 12:19, Andrii Anisov wrote: > From: Andrii Anisov> > Add a debug option to enable xen drivers debug code. > > Signed-off-by: Andrii Anisov In future you might want to add the maintainers to the recipient list. > --- > drivers/xen/Kconfig | 8 > drivers/xen/Makefile | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig > index f15bb3b7..733c433 100644 > --- a/drivers/xen/Kconfig > +++ b/drivers/xen/Kconfig > @@ -298,4 +298,12 @@ config XEN_SYMS > config XEN_HAVE_VPMU > bool > > +config DEBUG_XEN > + bool "XEN Drivers debug" > + depends on DEBUG_KERNEL > + help > + Say Y here if you want to enable XEN drivers debug code. > + > + If you are unsure about this, say N here. So is this really a sensible thing to do? I don't see the value for such a global config option possibly switching so many different drivers to debug mode. In reality you want to do something like "debug" for a single driver only during time of development. This won't need a global config option but just a "#define" in the driver which is active while developing and testing and which should be removed or commented out when the final submission of the driver is done. Juergen ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 00/11] fuzz: update x86emul fuzzer
The first few patches refactor x86emul code so that more code can be shared between xen and userspace tools. I have run XTF suite (tests subject to availability on the testbox I use, and xsa-195 was skipped because qemu segfault -- a known issue) against this series, no issue is found. Please see individual patch for changelog. Wei. --- Cc: Ian JacksonCc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap Wei Liu (11): x86emul/test: add missing dependency for x86_emulate.o x86: extract macros to x86-defns.h x86: extract vendor numeric id to x86-vendors.h x86emul/test: use x86-vendors.h x86emul: use eflags definitions in x86-defns.h x86emul: use msr definitions in msr-index.h x86: add UMIP CR4 bit x86emul: use CR definitions in x86-defns.h fuzz/x86emul: update fuzzer fuzz/x86emul: print out minimal input size fuzz: update README.afl example tools/fuzz/README.afl | 5 +- tools/fuzz/x86_instruction_emulator/Makefile | 9 +- .../afl-x86-insn-emulator-fuzzer.c | 8 + .../x86-insn-emulator-fuzzer.c | 666 +++-- tools/tests/x86_emulator/Makefile | 11 +- tools/tests/x86_emulator/test_x86_emulator.c | 9 - tools/tests/x86_emulator/x86_emulate.c | 3 - tools/tests/x86_emulator/x86_emulate.h | 9 +- xen/arch/x86/x86_emulate/x86_emulate.c | 403 ++--- xen/include/asm-x86/processor.h| 73 +-- xen/include/asm-x86/x86-defns.h| 69 +++ xen/include/asm-x86/x86-vendors.h | 13 + 12 files changed, 900 insertions(+), 378 deletions(-) create mode 100644 xen/include/asm-x86/x86-defns.h create mode 100644 xen/include/asm-x86/x86-vendors.h -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 04/11] x86emul/test: use x86-vendors.h
No functional change. Signed-off-by: Wei Liu--- Cc: Jan Beulich Cc: Andrew Cooper V3: link asm-x86 to working directory --- tools/fuzz/x86_instruction_emulator/Makefile | 9 +++-- tools/tests/x86_emulator/Makefile| 9 +++-- tools/tests/x86_emulator/x86_emulate.h | 7 ++- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile index f2bb12e871..d5c5bba0b0 100644 --- a/tools/fuzz/x86_instruction_emulator/Makefile +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -11,10 +11,15 @@ endif x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . +asm/x86-vendors.h: + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm + x86_emulate.c x86_emulate.h: %: [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* -CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ +CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I. + +x86_emulate.h: asm/x86-vendors.h x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h @@ -31,7 +36,7 @@ all: x86-instruction-emulator-fuzzer-all .PHONY: distclean distclean: clean - rm -f x86_emulate x86_emulate.c x86_emulate.h + rm -f x86_emulate x86_emulate.c x86_emulate.h asm .PHONY: clean clean: diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile index 883daf30a3..bd1c4d664e 100644 --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -32,7 +32,7 @@ $(TARGET): x86_emulate.o test_x86_emulator.o .PHONY: clean clean: - rm -rf $(TARGET) *.o *~ core blowfish.h blowfish.bin x86_emulate + rm -rf $(TARGET) *.o *~ core blowfish.h blowfish.bin x86_emulate asm .PHONY: distclean distclean: clean @@ -43,7 +43,12 @@ install: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . -HOSTCFLAGS += $(CFLAGS_xeninclude) +asm/x86-vendors.h: + [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm + +x86_emulate.h: asm/x86-vendors.h + +HOSTCFLAGS += $(CFLAGS_xeninclude) -I. x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h $(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $< diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h index 3a6badee46..db287004e6 100644 --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -6,6 +6,8 @@ #include #include +#include + #define BUG() abort() #define ASSERT assert #define ASSERT_UNREACHABLE() assert(!__LINE__) @@ -38,11 +40,6 @@ #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63)) -/* There's no strict need for these to be in sync with processor.h. */ -#define X86_VENDOR_INTEL 0 -#define X86_VENDOR_AMD 2 -#define X86_VENDOR_UNKNOWN 0xff - #define MMAP_SZ 16384 bool emul_test_make_stack_executable(void); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 08/11] x86emul: use CR definitions in x86-defns.h
And remove the duplicates. No functional change. Signed-off-by: Wei LiuReviewed-by: Jan Beulich --- xen/arch/x86/x86_emulate/x86_emulate.c | 45 -- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index e649205848..77b9de4a1e 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -415,21 +415,6 @@ typedef union { # define ASM_FLAG_OUT(yes, no) no #endif -/* Control register flags. */ -#define CR0_PE(1<<0) -#define CR0_MP(1<<1) -#define CR0_EM(1<<2) -#define CR0_TS(1<<3) - -#define CR4_VME(1<<0) -#define CR4_PVI(1<<1) -#define CR4_TSD(1<<2) -#define CR4_OSFXSR (1<<9) -#define CR4_OSXMMEXCPT (1<<10) -#define CR4_UMIP (1<<11) -#define CR4_FSGSBASE (1<<16) -#define CR4_OSXSAVE(1<<18) - /* Floating point status word definitions. */ #define FSW_ES(1U << 7) @@ -810,7 +795,7 @@ static int _get_fpu( if ( rc != X86EMUL_OKAY ) return rc; generate_exception_if(!(cr4 & ((type == X86EMUL_FPU_xmm) - ? CR4_OSFXSR : CR4_OSXSAVE)), + ? X86_CR4_OSFXSR : X86_CR4_OSXSAVE)), EXC_UD); } @@ -820,16 +805,16 @@ static int _get_fpu( if ( type >= X86EMUL_FPU_ymm ) { /* Should be unreachable if VEX decoding is working correctly. */ -ASSERT((cr0 & CR0_PE) && !(ctxt->regs->_eflags & X86_EFLAGS_VM)); +ASSERT((cr0 & X86_CR0_PE) && !(ctxt->regs->_eflags & X86_EFLAGS_VM)); } -if ( cr0 & CR0_EM ) +if ( cr0 & X86_CR0_EM ) { generate_exception_if(type == X86EMUL_FPU_fpu, EXC_NM); generate_exception_if(type == X86EMUL_FPU_mmx, EXC_UD); generate_exception_if(type == X86EMUL_FPU_xmm, EXC_UD); } -generate_exception_if((cr0 & CR0_TS) && - (type != X86EMUL_FPU_wait || (cr0 & CR0_MP)), +generate_exception_if((cr0 & X86_CR0_TS) && + (type != X86EMUL_FPU_wait || (cr0 & X86_CR0_MP)), EXC_NM); } @@ -852,7 +837,7 @@ do { \ _put_fpu(); \ if ( (_fic)->exn_raised == EXC_XM && ops->read_cr &&\ ops->read_cr(4, , ctxt) == X86EMUL_OKAY && \ - !(cr4 & CR4_OSXMMEXCPT) ) \ + !(cr4 & X86_CR4_OSXMMEXCPT) ) \ (_fic)->exn_raised = EXC_UD;\ generate_exception_if((_fic)->exn_raised >= 0, \ (_fic)->exn_raised); \ @@ -1184,7 +1169,7 @@ _mode_iopl( rc = ops->read_cr(4, , ctxt);\ if ( rc != X86EMUL_OKAY ) goto done; \ }\ -!!(cr4 & (_regs._eflags & X86_EFLAGS_VM ? CR4_VME : CR4_PVI)); \ +!!(cr4 & (_regs._eflags & X86_EFLAGS_VM ? X86_CR4_VME : X86_CR4_PVI)); \ }) static int ioport_access_check( @@ -1258,7 +1243,7 @@ in_realmode( return 0; rc = ops->read_cr(0, , ctxt); -return (!rc && !(cr0 & CR0_PE)); +return (!rc && !(cr0 & X86_CR0_PE)); } static bool @@ -1726,7 +1711,7 @@ static bool umip_active(struct x86_emulate_ctxt *ctxt, /* Intentionally not using mode_ring0() here to avoid its fail_if(). */ return get_cpl(ctxt, ops) > 0 && ops->read_cr && ops->read_cr(4, , ctxt) == X86EMUL_OKAY && - (cr4 & CR4_UMIP); + (cr4 & X86_CR4_UMIP); } /* Inject a software interrupt/exception, emulating if needed. */ @@ -3345,7 +3330,7 @@ x86_emulate( if ( rc != X86EMUL_OKAY ) goto done; } -generate_exception_if(!(cr4 & CR4_VME), EXC_GP, 0); +generate_exception_if(!(cr4 & X86_CR4_VME), EXC_GP, 0); src.val = (_regs.flags & ~X86_EFLAGS_IF) | X86_EFLAGS_IOPL; if ( _regs._eflags & X86_EFLAGS_VIF ) src.val |= X86_EFLAGS_IF; @@ -3368,7 +3353,7 @@ x86_emulate( if ( rc != X86EMUL_OKAY ) goto done; } -generate_exception_if(!(cr4 & CR4_VME) && +generate_exception_if(!(cr4 & X86_CR4_VME) && MASK_EXTR(_regs._eflags, X86_EFLAGS_IOPL) != 3, EXC_GP, 0); } @@ -3385,7 +3370,7 @@ x86_emulate( if ( op_bytes == 2 ) { dst.val = (uint16_t)dst.val | (_regs._eflags & 0xu); -
[Xen-devel] [PATCH v3 05/11] x86emul: use eflags definitions in x86-defns.h
Basically this patch does 's/EFLG_/X86_EFLAGS_/g' and with indentation fixed up. And remove the duplicates in x86_emualte.c. This in turn requires userspace test harness to include x86-defns.h. Also remove a few duplicates in userspace harness program. No functional change. Signed-off-by: Wei LiuReviewed-by: Jan Beulich --- v3: 1. Adapt build system to previous patch 2. Add spaces around "|" --- tools/fuzz/x86_instruction_emulator/Makefile | 4 +- tools/tests/x86_emulator/Makefile| 4 +- tools/tests/x86_emulator/test_x86_emulator.c | 9 - tools/tests/x86_emulator/x86_emulate.h | 1 + xen/arch/x86/x86_emulate/x86_emulate.c | 321 +-- 5 files changed, 160 insertions(+), 179 deletions(-) diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile index d5c5bba0b0..240bc58b06 100644 --- a/tools/fuzz/x86_instruction_emulator/Makefile +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -11,7 +11,7 @@ endif x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . -asm/x86-vendors.h: +asm/x86-vendors.h asm/x86-defns.h: [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm x86_emulate.c x86_emulate.h: %: @@ -19,7 +19,7 @@ x86_emulate.c x86_emulate.h: %: CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I. -x86_emulate.h: asm/x86-vendors.h +x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile index bd1c4d664e..6978288294 100644 --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -43,10 +43,10 @@ install: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . -asm/x86-vendors.h: +asm/x86-vendors.h asm/x86-defns.h: [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm -x86_emulate.h: asm/x86-vendors.h +x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h HOSTCFLAGS += $(CFLAGS_xeninclude) -I. diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c index 924fd36f18..27200e0283 100644 --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -23,15 +23,6 @@ static const struct { #endif }; -/* EFLAGS bit definitions. */ -#define EFLG_OF (1<<11) -#define EFLG_DF (1<<10) -#define EFLG_SF (1<<7) -#define EFLG_ZF (1<<6) -#define EFLG_AF (1<<4) -#define EFLG_PF (1<<2) -#define EFLG_CF (1<<0) - static unsigned int bytes_read; static int read( diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h index db287004e6..e064deaeea 100644 --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -6,6 +6,7 @@ #include #include +#include #include #define BUG() abort() diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 21dd98cebc..6e8221d9d8 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -447,26 +447,6 @@ typedef union { #define CR4_FSGSBASE (1<<16) #define CR4_OSXSAVE(1<<18) -/* EFLAGS bit definitions. */ -#define EFLG_ID (1<<21) -#define EFLG_VIP (1<<20) -#define EFLG_VIF (1<<19) -#define EFLG_AC (1<<18) -#define EFLG_VM (1<<17) -#define EFLG_RF (1<<16) -#define EFLG_NT (1<<14) -#define EFLG_IOPL (3<<12) -#define EFLG_OF (1<<11) -#define EFLG_DF (1<<10) -#define EFLG_IF (1<<9) -#define EFLG_TF (1<<8) -#define EFLG_SF (1<<7) -#define EFLG_ZF (1<<6) -#define EFLG_AF (1<<4) -#define EFLG_PF (1<<2) -#define EFLG_MBS (1<<1) -#define EFLG_CF (1<<0) - /* Floating point status word definitions. */ #define FSW_ES(1U << 7) @@ -521,14 +501,16 @@ typedef union { * These EFLAGS bits are restored from saved value during emulation, and * any changes are written back to the saved value after emulation. */ -#define EFLAGS_MASK (EFLG_OF|EFLG_SF|EFLG_ZF|EFLG_AF|EFLG_PF|EFLG_CF) +#define EFLAGS_MASK (X86_EFLAGS_OF | X86_EFLAGS_SF | X86_EFLAGS_ZF | \ + X86_EFLAGS_AF | X86_EFLAGS_PF | X86_EFLAGS_CF) /* * These EFLAGS bits are modifiable (by POPF and IRET), possibly subject * to further CPL and IOPL constraints. */ -#define EFLAGS_MODIFIABLE (EFLG_ID|EFLG_AC|EFLG_RF|EFLG_NT|EFLG_IOPL| \ - EFLG_DF|EFLG_IF|EFLG_TF|EFLAGS_MASK) +#define EFLAGS_MODIFIABLE (X86_EFLAGS_ID | X86_EFLAGS_AC | X86_EFLAGS_RF | \ + X86_EFLAGS_NT | X86_EFLAGS_IOPL | X86_EFLAGS_DF | \ + X86_EFLAGS_IF | X86_EFLAGS_TF | EFLAGS_MASK) /* Before executing instruction: restore necessary bits in EFLAGS. */
[Xen-devel] [PATCH v3 06/11] x86emul: use msr definitions in msr-index.h
Change the names used in code according to numeric values. Remove the now unused macros in x86_emualte.c and fix indentation. This in turns requires including msr-index.h and removing duplicates in userspace x86_emulate.c in userspace harness program. No functional change. Signed-off-by: Wei LiuReviewed-by: Jan Beulich --- v3: 1. Adapt build system to previous patch --- tools/fuzz/x86_instruction_emulator/Makefile | 4 +-- tools/tests/x86_emulator/Makefile| 4 +-- tools/tests/x86_emulator/x86_emulate.c | 3 -- tools/tests/x86_emulator/x86_emulate.h | 1 + xen/arch/x86/x86_emulate/x86_emulate.c | 43 ++-- 5 files changed, 20 insertions(+), 35 deletions(-) diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile index 240bc58b06..1dfacf7da4 100644 --- a/tools/fuzz/x86_instruction_emulator/Makefile +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -11,7 +11,7 @@ endif x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . -asm/x86-vendors.h asm/x86-defns.h: +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm x86_emulate.c x86_emulate.h: %: @@ -19,7 +19,7 @@ x86_emulate.c x86_emulate.h: %: CFLAGS += $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -I. -x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h +x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile index 6978288294..1fbcd8bd71 100644 --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -43,10 +43,10 @@ install: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . -asm/x86-vendors.h asm/x86-defns.h: +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm -x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h +x86_emulate.h: asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h HOSTCFLAGS += $(CFLAGS_xeninclude) -I. diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c index 615326259b..cda0fd8ee1 100644 --- a/tools/tests/x86_emulator/x86_emulate.c +++ b/tools/tests/x86_emulator/x86_emulate.c @@ -2,9 +2,6 @@ #include -#define EFER_SCE (1 << 0) -#define EFER_LMA (1 << 10) - #define cpu_has_amd_erratum(nr) 0 #define mark_regs_dirty(r) ((void)(r)) #define cpu_has_mpx false diff --git a/tools/tests/x86_emulator/x86_emulate.h b/tools/tests/x86_emulator/x86_emulate.h index e064deaeea..6d6f5125ca 100644 --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -6,6 +6,7 @@ #include #include +#include #include #include diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c index 6e8221d9d8..e649205848 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -415,23 +415,6 @@ typedef union { # define ASM_FLAG_OUT(yes, no) no #endif -/* MSRs. */ -#define MSR_TSC 0x0010 -#define MSR_SYSENTER_CS 0x0174 -#define MSR_SYSENTER_ESP 0x0175 -#define MSR_SYSENTER_EIP 0x0176 -#define MSR_DEBUGCTL 0x01d9 -#define DEBUGCTL_BTF (1 << 1) -#define MSR_BNDCFGS 0x0d90 -#define BNDCFG_ENABLE(1 << 0) -#define BNDCFG_PRESERVE (1 << 1) -#define MSR_EFER 0xc080 -#define MSR_STAR 0xc081 -#define MSR_LSTAR0xc082 -#define MSR_CSTAR0xc083 -#define MSR_FMASK0xc084 -#define MSR_TSC_AUX 0xc103 - /* Control register flags. */ #define CR0_PE(1<<0) #define CR0_MP(1<<1) @@ -1731,8 +1714,8 @@ static bool is_branch_step(struct x86_emulate_ctxt *ctxt, uint64_t debugctl; return ops->read_msr && - ops->read_msr(MSR_DEBUGCTL, , ctxt) == X86EMUL_OKAY && - (debugctl & DEBUGCTL_BTF); + ops->read_msr(MSR_IA32_DEBUGCTLMSR, , ctxt) == X86EMUL_OKAY && + (debugctl & IA32_DEBUGCTLMSR_BTF); } static bool umip_active(struct x86_emulate_ctxt *ctxt, @@ -1894,9 +1877,9 @@ static void adjust_bnd(struct x86_emulate_ctxt *ctxt, if ( !mode_ring0() ) bndcfg = read_bndcfgu(); else if ( !ops->read_msr || - ops->read_msr(MSR_BNDCFGS, , ctxt) != X86EMUL_OKAY ) + ops->read_msr(MSR_IA32_BNDCFGS, , ctxt) != X86EMUL_OKAY ) return; -if ( (bndcfg & BNDCFG_ENABLE) && !(bndcfg & BNDCFG_PRESERVE) ) +if ( (bndcfg & IA32_BNDCFGS_ENABLE) && !(bndcfg & IA32_BNDCFGS_PRESERVE) ) { /* * Using BNDMK or any other MPX instruction here
[Xen-devel] [PATCH v3 02/11] x86: extract macros to x86-defns.h
... so that they can be used by userspace x86 instruction emulator test program and fuzzer as well. No functional change. Signed-off-by: Wei Liu--- Cc: Jan Beulich Cc: Andrew Cooper v3: Don't move TRAP_* --- xen/include/asm-x86/processor.h | 65 ++- xen/include/asm-x86/x86-defns.h | 68 + 2 files changed, 70 insertions(+), 63 deletions(-) create mode 100644 xen/include/asm-x86/x86-defns.h diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h index 4da9c193e0..0b9191befa 100644 --- a/xen/include/asm-x86/processor.h +++ b/xen/include/asm-x86/processor.h @@ -16,6 +16,8 @@ #include #endif +#include "x86-defns.h" + /* * CPU vendor IDs */ @@ -25,69 +27,6 @@ #define X86_VENDOR_NUM 3 #define X86_VENDOR_UNKNOWN 0xff -/* - * EFLAGS bits - */ -#define X86_EFLAGS_CF 0x0001 /* Carry Flag */ -#define X86_EFLAGS_MBS 0x0002 /* Resvd bit */ -#define X86_EFLAGS_PF 0x0004 /* Parity Flag */ -#define X86_EFLAGS_AF 0x0010 /* Auxillary carry Flag */ -#define X86_EFLAGS_ZF 0x0040 /* Zero Flag */ -#define X86_EFLAGS_SF 0x0080 /* Sign Flag */ -#define X86_EFLAGS_TF 0x0100 /* Trap Flag */ -#define X86_EFLAGS_IF 0x0200 /* Interrupt Flag */ -#define X86_EFLAGS_DF 0x0400 /* Direction Flag */ -#define X86_EFLAGS_OF 0x0800 /* Overflow Flag */ -#define X86_EFLAGS_IOPL0x3000 /* IOPL mask */ -#define X86_EFLAGS_NT 0x4000 /* Nested Task */ -#define X86_EFLAGS_RF 0x0001 /* Resume Flag */ -#define X86_EFLAGS_VM 0x0002 /* Virtual Mode */ -#define X86_EFLAGS_AC 0x0004 /* Alignment Check */ -#define X86_EFLAGS_VIF 0x0008 /* Virtual Interrupt Flag */ -#define X86_EFLAGS_VIP 0x0010 /* Virtual Interrupt Pending */ -#define X86_EFLAGS_ID 0x0020 /* CPUID detection flag */ - -#define X86_EFLAGS_ARITH_MASK \ -(X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | \ - X86_EFLAGS_ZF | X86_EFLAGS_SF | X86_EFLAGS_OF) - -/* - * Intel CPU flags in CR0 - */ -#define X86_CR0_PE 0x0001 /* Enable Protected Mode(RW) */ -#define X86_CR0_MP 0x0002 /* Monitor Coprocessor (RW) */ -#define X86_CR0_EM 0x0004 /* Require FPU Emulation(RO) */ -#define X86_CR0_TS 0x0008 /* Task Switched(RW) */ -#define X86_CR0_ET 0x0010 /* Extension type (RO) */ -#define X86_CR0_NE 0x0020 /* Numeric Error Reporting (RW) */ -#define X86_CR0_WP 0x0001 /* Supervisor Write Protect (RW) */ -#define X86_CR0_AM 0x0004 /* Alignment Checking (RW) */ -#define X86_CR0_NW 0x2000 /* Not Write-Through(RW) */ -#define X86_CR0_CD 0x4000 /* Cache Disable(RW) */ -#define X86_CR0_PG 0x8000 /* Paging (RW) */ - -/* - * Intel CPU features in CR4 - */ -#define X86_CR4_VME0x0001 /* enable vm86 extensions */ -#define X86_CR4_PVI0x0002 /* virtual interrupts flag enable */ -#define X86_CR4_TSD0x0004 /* disable time stamp at ipl 3 */ -#define X86_CR4_DE 0x0008 /* enable debugging extensions */ -#define X86_CR4_PSE0x0010 /* enable page size extensions */ -#define X86_CR4_PAE0x0020 /* enable physical address extensions */ -#define X86_CR4_MCE0x0040 /* Machine check enable */ -#define X86_CR4_PGE0x0080 /* enable global pages */ -#define X86_CR4_PCE0x0100 /* enable performance counters at ipl 3 */ -#define X86_CR4_OSFXSR 0x0200 /* enable fast FPU save and restore */ -#define X86_CR4_OSXMMEXCPT 0x0400 /* enable unmasked SSE exceptions */ -#define X86_CR4_VMXE 0x2000 /* enable VMX */ -#define X86_CR4_SMXE 0x4000 /* enable SMX */ -#define X86_CR4_FSGSBASE 0x0001 /* enable {rd,wr}{fs,gs}base */ -#define X86_CR4_PCIDE 0x0002 /* enable PCID */ -#define X86_CR4_OSXSAVE0x0004 /* enable XSAVE/XRSTOR */ -#define X86_CR4_SMEP 0x0010 /* enable SMEP */ -#define X86_CR4_SMAP 0x0020 /* enable SMAP */ -#define X86_CR4_PKE0x0040 /* enable PKE */ /* * Trap/fault mnemonics. diff --git a/xen/include/asm-x86/x86-defns.h b/xen/include/asm-x86/x86-defns.h new file mode 100644 index 00..48ea5ea03b --- /dev/null +++ b/xen/include/asm-x86/x86-defns.h @@ -0,0 +1,68 @@ +#ifndef __XEN_X86_DEFNS_H__ +#define __XEN_X86_DEFNS_H__ + +/* + * EFLAGS bits + */ +#define X86_EFLAGS_CF 0x0001 /* Carry Flag */ +#define X86_EFLAGS_MBS 0x0002 /* Resvd bit */ +#define X86_EFLAGS_PF 0x0004 /* Parity Flag */ +#define X86_EFLAGS_AF 0x0010 /* Auxillary carry Flag */ +#define X86_EFLAGS_ZF 0x0040 /* Zero Flag */ +#define X86_EFLAGS_SF 0x0080 /* Sign Flag */
[Xen-devel] [PATCH v3 07/11] x86: add UMIP CR4 bit
It will be used later to remove duplicates in x86emul. Signed-off-by: Wei LiuReviewed-by: Jan Beulich --- xen/include/asm-x86/x86-defns.h | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/include/asm-x86/x86-defns.h b/xen/include/asm-x86/x86-defns.h index 48ea5ea03b..70453e8dfb 100644 --- a/xen/include/asm-x86/x86-defns.h +++ b/xen/include/asm-x86/x86-defns.h @@ -56,6 +56,7 @@ #define X86_CR4_PCE0x0100 /* enable performance counters at ipl 3 */ #define X86_CR4_OSFXSR 0x0200 /* enable fast FPU save and restore */ #define X86_CR4_OSXMMEXCPT 0x0400 /* enable unmasked SSE exceptions */ +#define X86_CR4_UMIP 0x0800 /* enable UMIP */ #define X86_CR4_VMXE 0x2000 /* enable VMX */ #define X86_CR4_SMXE 0x4000 /* enable SMX */ #define X86_CR4_FSGSBASE 0x0001 /* enable {rd,wr}{fs,gs}base */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 03/11] x86: extract vendor numeric id to x86-vendors.h
They will be shared between xen and userspace programs. This is not strictly necessary, but it helps reduce overall code size. No functional change. Signed-off-by: Wei LiuAcked-by: Jan Beulich --- xen/include/asm-x86/processor.h | 10 +- xen/include/asm-x86/x86-vendors.h | 13 + 2 files changed, 14 insertions(+), 9 deletions(-) create mode 100644 xen/include/asm-x86/x86-vendors.h diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h index 0b9191befa..ebe4e37aad 100644 --- a/xen/include/asm-x86/processor.h +++ b/xen/include/asm-x86/processor.h @@ -17,15 +17,7 @@ #endif #include "x86-defns.h" - -/* - * CPU vendor IDs - */ -#define X86_VENDOR_INTEL 0 -#define X86_VENDOR_AMD 1 -#define X86_VENDOR_CENTAUR 2 -#define X86_VENDOR_NUM 3 -#define X86_VENDOR_UNKNOWN 0xff +#include "x86-vendors.h" /* diff --git a/xen/include/asm-x86/x86-vendors.h b/xen/include/asm-x86/x86-vendors.h new file mode 100644 index 00..cae5507bd0 --- /dev/null +++ b/xen/include/asm-x86/x86-vendors.h @@ -0,0 +1,13 @@ +#ifndef __XEN_X86_VENDORS_H__ +#define __XEN_X86_VENDORS_H__ + +/* + * CPU vendor IDs + */ +#define X86_VENDOR_INTEL 0 +#define X86_VENDOR_AMD 1 +#define X86_VENDOR_CENTAUR 2 +#define X86_VENDOR_NUM 3 +#define X86_VENDOR_UNKNOWN 0xff + +#endif /* __XEN_X86_VENDORS_H__ */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 09/11] fuzz/x86emul: update fuzzer
Provide the fuzzer with more ops, and more sophisticated input structure. Based on a patch originally written by Andrew and George. Signed-off-by: Andrew CooperSigned-off-by: George Dunlap Signed-off-by: Wei Liu --- Cc: Ian Jackson Cc: Jan Beulich Cc: Andrew Cooper Cc: George Dunlap v3: 1. Address comments from Jan v2: 1. Address comments from Jan 2. Provide architecturally correct behaviour for rep stub --- .../x86-insn-emulator-fuzzer.c | 661 +++-- 1 file changed, 599 insertions(+), 62 deletions(-) diff --git a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c index 7d7f731677..4c404ceb81 100644 --- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -16,26 +17,78 @@ #include "x86_emulate.h" -static unsigned char data[4096]; +#define MSR_INDEX_MAX 16 + +#define SEG_NUM x86_seg_none + +struct input_struct { +unsigned long cr[5]; +uint64_t msr[MSR_INDEX_MAX]; +struct cpu_user_regs regs; +struct segment_register segments[SEG_NUM]; +unsigned long options; +unsigned char data[4096]; +} input; +#define DATA_OFFSET offsetof(struct input_struct, data) static unsigned int data_index; -static unsigned int data_max; +static unsigned int data_num; + +/* + * Randomly return success or failure when processing data. If + * `exception` is false, this function turns _EXCEPTION to _OKAY. + */ +int maybe_fail(const char *why, bool exception) +{ +int rc; + +if ( data_index >= data_num ) +rc = X86EMUL_EXCEPTION; +else +{ +/* Randomly returns value: + * 50% okay + * 25% unhandlable + * 25% exception + */ +if ( input.data[data_index] > 0xc0 ) +rc = X86EMUL_EXCEPTION; +else if ( input.data[data_index] > 0x80 ) +rc = X86EMUL_UNHANDLEABLE; +else +rc = X86EMUL_OKAY; +data_index++; +} + +if ( rc == X86EMUL_EXCEPTION && !exception ) +rc = X86EMUL_OKAY; + +printf("maybe_fail %s: %d\n", why, rc); + +return rc; +} static int data_read(const char *why, void *dst, unsigned int bytes) { unsigned int i; +int rc; -if ( data_index + bytes > data_max ) -return X86EMUL_EXCEPTION; +if ( data_index + bytes > data_num ) +rc = X86EMUL_EXCEPTION; +else +rc = maybe_fail(why, true); -memcpy(dst, data + data_index, bytes); -data_index += bytes; +if ( rc == X86EMUL_OKAY ) +{ +memcpy(dst, input.data + data_index, bytes); +data_index += bytes; -printf("%s: ", why); -for ( i = 0; i < bytes; i++ ) -printf(" %02x", *(unsigned char *)(dst + i)); -printf("\n"); +printf("%s: ", why); +for ( i = 0; i < bytes; i++ ) +printf(" %02x", *(unsigned char *)(dst + i)); +printf("\n"); +} -return X86EMUL_OKAY; +return rc; } static int fuzz_read( @@ -48,14 +101,112 @@ static int fuzz_read( return data_read("read", p_data, bytes); } -static int fuzz_fetch( +static int fuzz_read_io( +unsigned int port, +unsigned int bytes, +unsigned long *val, +struct x86_emulate_ctxt *ctxt) +{ +return data_read("read_io", val, bytes); +} + +static int fuzz_insn_fetch( unsigned int seg, unsigned long offset, void *p_data, unsigned int bytes, struct x86_emulate_ctxt *ctxt) { -return data_read("fetch", p_data, bytes); +return data_read("insn_fetch", p_data, bytes); +} + +static int _fuzz_rep_read(const char *why, unsigned long *reps) +{ +int rc; +unsigned long bytes_read = 0; + +rc = data_read(why, _read, sizeof(bytes_read)); + +if ( bytes_read <= *reps ) +*reps = bytes_read; + +switch ( rc ) +{ +case X86EMUL_UNHANDLEABLE: +/* No work is done in this case */ +*reps = 0; +break; +case X86EMUL_EXCEPTION: +case X86EMUL_RETRY: +/* Halve the amount in this case */ +*reps /= 2; +} + +return rc; +} + +static int _fuzz_rep_write(const char *why, unsigned long *reps) +{ +int rc = maybe_fail(why, true); + +switch ( rc ) +{ +case X86EMUL_UNHANDLEABLE: +/* No work is done in this case */ +*reps = 0; +break; +case X86EMUL_EXCEPTION: +case X86EMUL_RETRY: +/* Halve the amount in this case */ +*reps /= 2; +} + +return rc; +} + +static int fuzz_rep_ins( +uint16_t src_port, +enum x86_segment dst_seg, +unsigned long dst_offset, +unsigned int
[Xen-devel] [PATCH v3 01/11] x86emul/test: add missing dependency for x86_emulate.o
f4497d6b74 added x86_emulate.h private header but didn't add dependency for it. Signed-off-by: Wei Liu--- Cc: Jan Beulich Cc: Andrew Cooper --- tools/tests/x86_emulator/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tests/x86_emulator/Makefile b/tools/tests/x86_emulator/Makefile index 0b5baff67c..883daf30a3 100644 --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -45,7 +45,7 @@ x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: HOSTCFLAGS += $(CFLAGS_xeninclude) -x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h +x86_emulate.o: x86_emulate.c x86_emulate.h x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h $(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $< test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/dmop: Fix compat_dm_op() ABI
On 01/02/17 10:46, Jan Beulich wrote: On 31.01.17 at 20:59,wrote: >> What is the pupose of COMPAT_HANDLE_PARAM()? It is a packed structure of one >> and a half pointers, so isn't safe at all for use in the hypercall function >> APIs (depsite its naming making it look deceptively like it is the correct >> thing to use). > Btw, where are you taking this "one and a half pointers" from? > It's half a pointer (a compat one) plus a zero sized array. Hmm. I had missed the ZLA, but debugging proves that the raw value of bufs.c was garbage even when passing a NULL handle from userspace. As a result, the copy_from_compat_offset() was hitting -EFAULT for every hypercall. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 1/2] swiotlb-xen: implement xen_swiotlb_dma_mmap callback
Konrad, Stefano, >> What does 'local pages' mean? > > A page that belongs to this domain, rather than a foreign page that has > been mapped. I guess it should be clarified, but not sure in the commit message or comment to the code? Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/HVM: make hvm_find_io_handler() static
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: 01 February 2017 11:24 > To: xen-devel> Cc: Andrew Cooper ; Paul Durrant > > Subject: [PATCH] x86/HVM: make hvm_find_io_handler() static > > This reduces the chance of misuse - calling it must in particular > always be accompanied by calling the corresponding ->complete() hook. > Constify its parameter at once. Indeed. No good reason this is not already static AFACT. > > Signed-off-by: Jan Beulich Reviewed-by: Paul Durrant > > --- a/xen/arch/x86/hvm/intercept.c > +++ b/xen/arch/x86/hvm/intercept.c > @@ -210,7 +210,7 @@ int hvm_process_io_intercept(const struc > return rc; > } > > -const struct hvm_io_handler *hvm_find_io_handler(ioreq_t *p) > +static const struct hvm_io_handler *hvm_find_io_handler(const ioreq_t > *p) > { > struct domain *curr_d = current->domain; > unsigned int i; > --- a/xen/include/asm-x86/hvm/io.h > +++ b/xen/include/asm-x86/hvm/io.h > @@ -97,8 +97,6 @@ struct hvm_io_ops { > int hvm_process_io_intercept(const struct hvm_io_handler *handler, > ioreq_t *p); > > -const struct hvm_io_handler *hvm_find_io_handler(ioreq_t *p); > - > int hvm_io_intercept(ioreq_t *p); > > struct hvm_io_handler *hvm_next_io_handler(struct domain *d); > > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [xen-unstable-coverity test] 105200: all pass - PUSHED
flight 105200 xen-unstable-coverity real [real] http://logs.test-lab.xenproject.org/osstest/logs/105200/ Perfect :-) All tests in this flight passed as required version targeted for testing: xen 5a44ff111d2e1ba9f375936294117ff4e263c4ac baseline version: xen 40705830907aae52dc679362b7ec214b9ff25681 Last test of basis 104901 2017-01-29 09:18:41 Z3 days Testing same since 105200 2017-02-01 09:57:42 Z0 days1 attempts People who touched revisions under test: Andrew CooperEdgar E. Iglesias George Dunlap Julien Grall Konrad Rzeszutek Wilk Paul Durrant Razvan Cojocaru Stefano Stabellini Tamas K Lengyel Wei Liu jobs: coverity-amd64 pass 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-coverity + revision=5a44ff111d2e1ba9f375936294117ff4e263c4ac + . ./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-coverity 5a44ff111d2e1ba9f375936294117ff4e263c4ac + branch=xen-unstable-coverity + revision=5a44ff111d2e1ba9f375936294117ff4e263c4ac + . ./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-coverity + qemuubranch=qemu-upstream-unstable-coverity + qemuubranch=qemu-upstream-unstable + '[' xxen = xlinux ']' + linuxbranch= + '[' xqemu-upstream-unstable = x ']' + select_prevxenbranch ++ ./cri-getprevxenbranch xen-unstable-coverity + prevxenbranch=xen-4.8-testing + '[' x5a44ff111d2e1ba9f375936294117ff4e263c4ac = 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/xtf.git ++ : osst...@xenbits.xen.org:/home/xen/git/xtf.git ++ : git://xenbits.xen.org/xtf.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/osstest/rumprun.git ++ : git ++ : git://xenbits.xen.org/osstest/rumprun.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/rumprun.git ++ : git://git.seabios.org/seabios.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/seabios.git ++ : git://xenbits.xen.org/osstest/seabios.git ++ : https://github.com/tianocore/edk2.git ++ : osst...@xenbits.xen.org:/home/xen/git/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/ovmf.git ++ : git://xenbits.xen.org/osstest/linux-firmware.git ++ : osst...@xenbits.xen.org:/home/osstest/ext/linux-firmware.git ++ : git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git ++ : osst...@xenbits.xen.org:/home/xen/git/linux-pvops.git ++ : git://xenbits.xen.org/linux-pvops.git ++ :
[Xen-devel] [PATCH] x86/HVM: make hvm_find_io_handler() static
This reduces the chance of misuse - calling it must in particular always be accompanied by calling the corresponding ->complete() hook. Constify its parameter at once. Signed-off-by: Jan Beulich--- a/xen/arch/x86/hvm/intercept.c +++ b/xen/arch/x86/hvm/intercept.c @@ -210,7 +210,7 @@ int hvm_process_io_intercept(const struc return rc; } -const struct hvm_io_handler *hvm_find_io_handler(ioreq_t *p) +static const struct hvm_io_handler *hvm_find_io_handler(const ioreq_t *p) { struct domain *curr_d = current->domain; unsigned int i; --- a/xen/include/asm-x86/hvm/io.h +++ b/xen/include/asm-x86/hvm/io.h @@ -97,8 +97,6 @@ struct hvm_io_ops { int hvm_process_io_intercept(const struct hvm_io_handler *handler, ioreq_t *p); -const struct hvm_io_handler *hvm_find_io_handler(ioreq_t *p); - int hvm_io_intercept(ioreq_t *p); struct hvm_io_handler *hvm_next_io_handler(struct domain *d); x86/HVM: make hvm_find_io_handler() static This reduces the chance of misuse - calling it must in particular always be accompanied by calling the corresponding ->complete() hook. Constify its parameter at once. Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/intercept.c +++ b/xen/arch/x86/hvm/intercept.c @@ -210,7 +210,7 @@ int hvm_process_io_intercept(const struc return rc; } -const struct hvm_io_handler *hvm_find_io_handler(ioreq_t *p) +static const struct hvm_io_handler *hvm_find_io_handler(const ioreq_t *p) { struct domain *curr_d = current->domain; unsigned int i; --- a/xen/include/asm-x86/hvm/io.h +++ b/xen/include/asm-x86/hvm/io.h @@ -97,8 +97,6 @@ struct hvm_io_ops { int hvm_process_io_intercept(const struct hvm_io_handler *handler, ioreq_t *p); -const struct hvm_io_handler *hvm_find_io_handler(ioreq_t *p); - int hvm_io_intercept(ioreq_t *p); struct hvm_io_handler *hvm_next_io_handler(struct domain *d); ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 11/11] x86emul: test coverage for SSE/SSE2 insns
... and their AVX equivalents. Note that a few instructions aren't covered (yet), but those all fall into common pattern groups, so I would hope that for now we can do with what is there. MMX insns aren't being covered at all, as they're not easy to deal with: The compiler refuses to emit such for other than uses of built-in functions. The current way of testing AVX insns is meant to be temporary only: Once we fully support that feature, the present tests should rather be replaced than full ones simply added. Signed-off-by: Jan Beulich--- v2: New. --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -11,11 +11,36 @@ all: $(TARGET) run: $(TARGET) ./$(TARGET) -TESTCASES := blowfish +TESTCASES := blowfish simd blowfish-cflags := "" blowfish-cflags-x86_32 := "-mno-accumulate-outgoing-args -Dstatic=" +sse-vecs := 16 +sse-ints := +sse-flts := 4 +sse2-vecs := $(sse-vecs) +sse2-ints := 1 2 4 8 +sse2-flts := 4 8 + +# When converting SSE to AVX, have the compiler avoid XMM0 to widen +# coverage og the VEX. checks in the emulator. +sse2avx := -ffixed-xmm0 -Wa,-msse2avx + +simd-cflags := $(foreach flavor,sse sse2, \ + $(foreach vec,$($(flavor)-vecs), \ + $(foreach int,$($(flavor)-ints), \ + "-D$(flavor)_$(vec)i$(int) -m$(flavor) -O2 -DVEC_SIZE=$(vec) -DINT_SIZE=$(int)" \ + "-D$(flavor)_$(vec)u$(int) -m$(flavor) -O2 -DVEC_SIZE=$(vec) -DUINT_SIZE=$(int)" \ + "-D$(flavor)_avx_$(vec)i$(int) -m$(flavor) $(sse2avx) -O2 -DVEC_SIZE=$(vec) -DINT_SIZE=$(int)" \ + "-D$(flavor)_avx_$(vec)u$(int) -m$(flavor) $(sse2avx) -O2 -DVEC_SIZE=$(vec) -DUINT_SIZE=$(int)") \ + $(foreach flt,$($(flavor)-flts), \ + "-D$(flavor)_$(vec)f$(flt) -m$(flavor) -O2 -DVEC_SIZE=$(vec) -DFLOAT_SIZE=$(flt)" \ + "-D$(flavor)_avx_$(vec)f$(flt) -m$(flavor) $(sse2avx) -O2 -DVEC_SIZE=$(vec) -DFLOAT_SIZE=$(flt)")) \ + $(foreach flt,$($(flavor)-flts), \ + "-D$(flavor)_f$(flt) -m$(flavor) -mfpmath=sse -O2 -DFLOAT_SIZE=$(flt)" \ + "-D$(flavor)_avx_f$(flt) -m$(flavor) -mfpmath=sse $(sse2avx) -O2 -DFLOAT_SIZE=$(flt)")) + $(addsuffix .h,$(TESTCASES)): %.h: %.c testcase.mk Makefile rm -f $@.new $*.bin $(foreach arch,$(filter-out $(XEN_COMPILE_ARCH),x86_32) $(XEN_COMPILE_ARCH), \ --- /dev/null +++ b/tools/tests/x86_emulator/simd.c @@ -0,0 +1,450 @@ +#include + +asm ( +"\t.text\n" +"\t.globl _start\n" +"_start:\n" +#if defined(__i386__) && VEC_SIZE == 16 +"\tpush %ebp\n" +"\tmov %esp,%ebp\n" +"\tand $~0xf,%esp\n" +"\tcall simd_test\n" +"\tleave\n" +"\tret" +#else +"\tjmp simd_test" +#endif +); + +typedef +#if defined(INT_SIZE) +# define ELEM_SIZE INT_SIZE +signed int +# if INT_SIZE == 1 +# define MODE QI +# elif INT_SIZE == 2 +# define MODE HI +# elif INT_SIZE == 4 +# define MODE SI +# elif INT_SIZE == 8 +# define MODE DI +# endif +#elif defined(UINT_SIZE) +# define ELEM_SIZE UINT_SIZE +unsigned int +# if UINT_SIZE == 1 +# define MODE QI +# elif UINT_SIZE == 2 +# define MODE HI +# elif UINT_SIZE == 4 +# define MODE SI +# elif UINT_SIZE == 8 +# define MODE DI +# endif +#elif defined(FLOAT_SIZE) +float +# define ELEM_SIZE FLOAT_SIZE +# if FLOAT_SIZE == 4 +# define MODE SF +# elif FLOAT_SIZE == 8 +# define MODE DF +# endif +#endif +#ifndef VEC_SIZE +# define VEC_SIZE ELEM_SIZE +#endif +__attribute__((mode(MODE), vector_size(VEC_SIZE))) vec_t; + +#define ELEM_COUNT (VEC_SIZE / ELEM_SIZE) + +typedef unsigned int __attribute((mode(QI), vector_size(VEC_SIZE))) byte_vec_t; + +/* Various builtins want plain char / int / long long vector types ... */ +typedef char __attribute__((vector_size(VEC_SIZE))) vqi_t; +typedef short __attribute__((vector_size(VEC_SIZE))) vhi_t; +typedef int __attribute__((vector_size(VEC_SIZE))) vsi_t; +#if VEC_SIZE >= 8 +typedef long long __attribute__((vector_size(VEC_SIZE))) vdi_t; +#endif + +#if VEC_SIZE == 8 && defined(__SSE__) +# define to_bool(cmp) (__builtin_ia32_pmovmskb(cmp) == 0xff) +#elif VEC_SIZE == 16 +# if defined(__SSE__) && ELEM_SIZE == 4 +# define to_bool(cmp) (__builtin_ia32_movmskps(cmp) == 0xf) +# elif defined(__SSE2__) +# if ELEM_SIZE == 8 +# define to_bool(cmp) (__builtin_ia32_movmskpd(cmp) == 3) +# else +# define to_bool(cmp) (__builtin_ia32_pmovmskb128(cmp) == 0x) +# endif +# endif +#endif + +#ifndef to_bool +static inline bool _to_bool(byte_vec_t bv) +{ +unsigned int i; + +for ( i = 0; i < VEC_SIZE; ++i ) +if ( bv[i] != 0xff ) +return false; + +return true; +} +# define to_bool(cmp) _to_bool((byte_vec_t)(cmp)) +#endif + +#if VEC_SIZE == FLOAT_SIZE +# define to_int(x) ((vec_t){ (int)(x)[0] }) +#elif VEC_SIZE == 16 && defined(__SSE2__) +# if FLOAT_SIZE == 4 +# define to_int(x)
[Xen-devel] [PATCH] xen: Kconfig: Add DEBUG_XEN config option
From: Andrii AnisovAdd a debug option to enable xen drivers debug code. Signed-off-by: Andrii Anisov --- drivers/xen/Kconfig | 8 drivers/xen/Makefile | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index f15bb3b7..733c433 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -298,4 +298,12 @@ config XEN_SYMS config XEN_HAVE_VPMU bool +config DEBUG_XEN + bool "XEN Drivers debug" + depends on DEBUG_KERNEL + help + Say Y here if you want to enable XEN drivers debug code. + + If you are unsure about this, say N here. + endmenu diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 8feab810..c886e9d 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -42,3 +42,5 @@ xen-evtchn-y := evtchn.o xen-gntdev-y := gntdev.o xen-gntalloc-y := gntalloc.o xen-privcmd-y := privcmd.o + +ccflags-$(CONFIG_DEBUG_XEN) := -DDEBUG \ No newline at end of file -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 10/11] x86emul/test: split generic and testcase specific parts
Both the build logic and the invocation have their blowfish specific aspects abstracted out here. Additionally - run native execution (if suitable) first (as that one failing suggests a problem with the to be tested code itself, in which case having the emulator have a go over it is kind of pointless) - move the 64-bit tests up in blobs[] so 64-bit native execution will also precede 32-bit emulation (on 64-bit systems only of course) - instead of -msoft-float (we'd rather not have the compiler generate such code), pass -fno-asynchronous-unwind-tables and -g0 (reducing binary size of the helper images as well as [slightly] compilation time) - skip tests with zero length blobs (these can result from failed compilation, but not failing the build in this case seems desirable: it may allow partial testing - e.g. with older compilers - and permits manually removing certain tests from the generated headers without having to touch actual source code) - constrain rIP to the actual blob range rather than looking for the specific (fake) return address put on the stack - also print the opcode when x86_emulate() fails - print at least three progress dots (for relatively short tests) Signed-off-by: Jan Beulich--- v2: New. --- a/tools/tests/x86_emulator/Makefile +++ b/tools/tests/x86_emulator/Makefile @@ -11,18 +11,21 @@ all: $(TARGET) run: $(TARGET) ./$(TARGET) -cflags-x86_32 := "-mno-accumulate-outgoing-args -Dstatic=" +TESTCASES := blowfish -blowfish.h: blowfish.c blowfish.mk Makefile - rm -f $@.new blowfish.bin +blowfish-cflags := "" +blowfish-cflags-x86_32 := "-mno-accumulate-outgoing-args -Dstatic=" + +$(addsuffix .h,$(TESTCASES)): %.h: %.c testcase.mk Makefile + rm -f $@.new $*.bin $(foreach arch,$(filter-out $(XEN_COMPILE_ARCH),x86_32) $(XEN_COMPILE_ARCH), \ - for cflags in "" $(cflags-$(arch)); do \ - $(MAKE) -f blowfish.mk XEN_TARGET_ARCH=$(arch) BLOWFISH_CFLAGS="$$cflags" all; \ + for cflags in $($*-cflags) $($*-cflags-$(arch)); do \ + $(MAKE) -f testcase.mk TESTCASE=$* XEN_TARGET_ARCH=$(arch) $*-cflags="$$cflags" all; \ flavor=$$(echo $${cflags} | sed -e 's, .*,,' -e 'y,-=,__,') ; \ - (echo "static unsigned int blowfish_$(arch)$${flavor}[] = {"; \ -od -v -t x blowfish.bin | sed -e 's/^[0-9]* /0x/' -e 's/ /, 0x/g' -e 's/$$/,/'; \ + (echo "static const unsigned int $*_$(arch)$${flavor}[] = {"; \ +od -v -t x $*.bin | sed -e 's/^[0-9]* /0x/' -e 's/ /, 0x/g' -e 's/$$/,/'; \ echo "};") >>$@.new; \ - rm -f blowfish.bin; \ + rm -f $*.bin; \ done; \ ) mv $@.new $@ @@ -32,7 +35,7 @@ $(TARGET): x86_emulate.o test_x86_emulat .PHONY: clean clean: - rm -rf $(TARGET) *.o *~ core blowfish.h blowfish.bin x86_emulate + rm -rf $(TARGET) *.o *~ core $(addsuffix .h,$(TESTCASES)) *.bin x86_emulate .PHONY: distclean distclean: clean @@ -48,5 +51,5 @@ HOSTCFLAGS += $(CFLAGS_xeninclude) x86_emulate.o: x86_emulate.c x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h $(HOSTCC) $(HOSTCFLAGS) -D__XEN_TOOLS__ -c -g -o $@ $< -test_x86_emulator.o: test_x86_emulator.c blowfish.h x86_emulate/x86_emulate.h +test_x86_emulator.o: test_x86_emulator.c $(addsuffix .h,$(TESTCASES)) x86_emulate/x86_emulate.h $(HOSTCC) $(HOSTCFLAGS) -c -g -o $@ $< --- a/tools/tests/x86_emulator/blowfish.mk +++ /dev/null @@ -1,17 +0,0 @@ - -XEN_ROOT = $(CURDIR)/../../.. -CFLAGS = -include $(XEN_ROOT)/tools/Rules.mk - -$(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) - -CFLAGS += -fno-builtin -msoft-float $(BLOWFISH_CFLAGS) - -.PHONY: all -all: blowfish.bin - -blowfish.bin: blowfish.c - $(CC) $(CFLAGS) -c blowfish.c - $(LD) $(LDFLAGS_DIRECT) -N -Ttext 0x10 -o blowfish.tmp blowfish.o - $(OBJCOPY) -O binary blowfish.tmp blowfish.bin - rm -f blowfish.tmp --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -8,19 +8,37 @@ #define verbose false /* Switch to true for far more logging. */ +static void blowfish_set_regs(struct cpu_user_regs *regs) +{ +regs->eax = 2; +regs->edx = 1; +} + +static bool blowfish_check_regs(const struct cpu_user_regs *regs) +{ +return regs->eax == 2 && regs->edx == 1; +} + static const struct { const void *code; size_t size; unsigned int bitness; const char*name; +void (*set_regs)(struct cpu_user_regs *); +bool (*check_regs)(const struct cpu_user_regs *); } blobs[] = { -{ blowfish_x86_32, sizeof(blowfish_x86_32), 32, "blowfish" }, -{ blowfish_x86_32_mno_accumulate_outgoing_args, - sizeof(blowfish_x86_32_mno_accumulate_outgoing_args), - 32, "blowfish (push)" }, +#define BLOWFISH(bits, desc, tag) \ +{ .code = blowfish_x86_##bits##tag, \ + .size =
[Xen-devel] [PATCH v2 09/11] x86emul: support {,V}MOVNTDQA
... as the only post-SSE2 move insn. Signed-off-by: Jan Beulich--- v2: Re-base. --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -2389,6 +2389,74 @@ int main(int argc, char **argv) else printf("skipped\n"); +printf("%-40s", "Testing movntdqa 16(%edx),%xmm4..."); +if ( stack_exec && cpu_has_sse4_1 ) +{ +decl_insn(movntdqa); + +asm volatile ( "pcmpgtb %%xmm4, %%xmm4\n" + put_insn(movntdqa, "movntdqa 16(%0), %%xmm4") + :: "d" (NULL) ); + +set_insn(movntdqa); +memset(res, 0x55, 64); +memset(res + 4, 0xff, 16); +regs.edx = (unsigned long)res; +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(movntdqa) ) +goto fail; +asm ( "pcmpeqb %%xmm2, %%xmm2\n\t" + "pcmpeqb %%xmm4, %%xmm2\n\t" + "pmovmskb %%xmm2, %0" : "=r" (rc) ); +if ( rc != 0x ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + +printf("%-40s", "Testing vmovntdqa (%ecx),%ymm4..."); +if ( stack_exec && cpu_has_avx2 ) +{ +decl_insn(vmovntdqa); + +#if 0 /* Don't use AVX2 instructions for now */ +asm volatile ( "vpxor %%ymm4, %%ymm4, %%ymm4\n" + put_insn(vmovntdqa, "vmovntdqa (%0), %%ymm4") + :: "c" (NULL) ); +#else +asm volatile ( "vpxor %xmm4, %xmm4, %xmm4\n" + put_insn(vmovntdqa, +".byte 0xc4, 0xe2, 0x7d, 0x2a, 0x21") ); +#endif + +set_insn(vmovntdqa); +memset(res, 0x55, 96); +memset(res + 8, 0xff, 32); +regs.ecx = (unsigned long)(res + 8); +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(vmovntdqa) ) +goto fail; +#if 0 /* Don't use AVX2 instructions for now */ +asm ( "vpcmpeqb %%ymm2, %%ymm2, %%ymm2\n\t" + "vpcmpeqb %%ymm4, %%ymm2, %%ymm0\n\t" + "vpmovmskb %%ymm0, %0" : "=r" (rc) ); +#else +asm ( "vextractf128 $1, %%ymm4, %%xmm3\n\t" + "vpcmpeqb %%xmm2, %%xmm2, %%xmm2\n\t" + "vpcmpeqb %%xmm4, %%xmm2, %%xmm0\n\t" + "vpcmpeqb %%xmm3, %%xmm2, %%xmm1\n\t" + "vpmovmskb %%xmm0, %0\n\t" + "vpmovmskb %%xmm1, %1" : "=r" (rc), "=r" (i) ); +rc |= i << 16; +#endif +if ( ~rc ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + printf("%-40s", "Testing stmxcsr (%edx)..."); if ( cpu_has_sse ) { --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -95,6 +95,12 @@ static inline uint64_t xgetbv(uint32_t x (res.c & (1U << 0)) != 0; \ }) +#define cpu_has_sse4_1 ({ \ +struct cpuid_leaf res; \ +emul_test_cpuid(1, 0, , NULL); \ +(res.c & (1U << 19)) != 0; \ +}) + #define cpu_has_popcnt ({ \ struct cpuid_leaf res; \ emul_test_cpuid(1, 0, , NULL); \ --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -1433,6 +1433,7 @@ static bool vcpu_has( #define vcpu_has_sse2()vcpu_has( 1, EDX, 26, ctxt, ops) #define vcpu_has_sse3()vcpu_has( 1, ECX, 0, ctxt, ops) #define vcpu_has_cx16()vcpu_has( 1, ECX, 13, ctxt, ops) +#define vcpu_has_sse4_1() vcpu_has( 1, ECX, 19, ctxt, ops) #define vcpu_has_sse4_2() vcpu_has( 1, ECX, 20, ctxt, ops) #define vcpu_has_movbe() vcpu_has( 1, ECX, 22, ctxt, ops) #define vcpu_has_popcnt() vcpu_has( 1, ECX, 23, ctxt, ops) @@ -5944,6 +5945,7 @@ x86_emulate( case X86EMUL_OPC_VEX_66(0x0f, 0x7f): /* vmovdqa {x,y}mm,{x,y}mm/m128 */ case X86EMUL_OPC_F3(0x0f, 0x7f): /* movdqu xmm,xmm/m128 */ case X86EMUL_OPC_VEX_F3(0x0f, 0x7f): /* vmovdqu {x,y}mm,{x,y}mm/mem */ +movdqa: if ( vex.opcx != vex_none ) { host_and_vcpu_must_have(avx); @@ -6868,6 +6870,23 @@ x86_emulate( sfence = true; break; +case X86EMUL_OPC_66(0x0f38, 0x2a): /* movntdqa m128,xmm */ +case X86EMUL_OPC_VEX_66(0x0f38, 0x2a): /* vmovntdqa mem,{x,y}mm */ +generate_exception_if(ea.type != OP_MEM, EXC_UD); +/* Ignore the non-temporal hint for now, using movdqa instead. */ +asm volatile ( "mfence" ::: "memory" ); +b = 0x6f; +if ( vex.opcx == vex_none ) +vcpu_must_have(sse4_1); +else +{ +vex.opcx = vex_0f; +if ( vex.l ) +vcpu_must_have(avx2); +} +state->simd_size = simd_packed_int; +goto movdqa; + case X86EMUL_OPC(0x0f38, 0xf0): /* movbe m,r */ case X86EMUL_OPC(0x0f38, 0xf1): /* movbe r,m */ vcpu_must_have(movbe); x86emul: support {,V}MOVNTDQA ... as the
[Xen-devel] [PATCH v2 08/11] x86emul: support {,V}{LD,ST}MXCSR
Signed-off-by: Jan Beulich--- a/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c +++ b/tools/fuzz/x86_instruction_emulator/x86-insn-emulator-fuzzer.c @@ -119,7 +119,7 @@ int LLVMFuzzerTestOneInput(const uint8_t unsigned int x; const uint8_t *p = data_p; -stack_exec = emul_test_make_stack_executable(); +stack_exec = emul_test_init(); if ( !stack_exec ) { printf("Warning: Stack could not be made executable (%d).\n", errno); --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -210,7 +210,7 @@ int main(int argc, char **argv) } instr = (char *)res + 0x100; -stack_exec = emul_test_make_stack_executable(); +stack_exec = emul_test_init(); if ( !stack_exec ) printf("Warning: Stack could not be made executable (%d).\n", errno); @@ -2386,6 +2386,87 @@ int main(int argc, char **argv) goto fail; printf("okay\n"); } +else +printf("skipped\n"); + +printf("%-40s", "Testing stmxcsr (%edx)..."); +if ( cpu_has_sse ) +{ +decl_insn(stmxcsr); + +asm volatile ( put_insn(stmxcsr, "stmxcsr (%0)") :: "d" (NULL) ); + +res[0] = 0x12345678; +res[1] = 0x87654321; +asm ( "stmxcsr %0" : "=m" (res[2]) ); +set_insn(stmxcsr); +regs.edx = (unsigned long)res; +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(stmxcsr) || + res[0] != res[2] || res[1] != 0x87654321 ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + +printf("%-40s", "Testing ldmxcsr 4(%ecx)..."); +if ( cpu_has_sse ) +{ +decl_insn(ldmxcsr); + +asm volatile ( put_insn(ldmxcsr, "ldmxcsr 4(%0)") :: "c" (NULL) ); + +set_insn(ldmxcsr); +res[1] = mxcsr_mask; +regs.ecx = (unsigned long)res; +rc = x86_emulate(, ); +asm ( "stmxcsr %0; ldmxcsr %1" : "=m" (res[0]) : "m" (res[2]) ); +if ( rc != X86EMUL_OKAY || !check_eip(ldmxcsr) || + res[0] != mxcsr_mask ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + +printf("%-40s", "Testing vstmxcsr (%ecx)..."); +if ( cpu_has_avx ) +{ +decl_insn(vstmxcsr); + +asm volatile ( put_insn(vstmxcsr, "vstmxcsr (%0)") :: "c" (NULL) ); + +res[0] = 0x12345678; +res[1] = 0x87654321; +set_insn(vstmxcsr); +regs.ecx = (unsigned long)res; +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(vstmxcsr) || + res[0] != res[2] || res[1] != 0x87654321 ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + +printf("%-40s", "Testing vldmxcsr 4(%edx)..."); +if ( cpu_has_avx ) +{ +decl_insn(vldmxcsr); + +asm volatile ( put_insn(vldmxcsr, "vldmxcsr 4(%0)") :: "d" (NULL) ); + +set_insn(vldmxcsr); +res[1] = mxcsr_mask; +regs.edx = (unsigned long)res; +rc = x86_emulate(, ); +asm ( "stmxcsr %0; ldmxcsr %1" : "=m" (res[0]) : "m" (res[2]) ); +if ( rc != X86EMUL_OKAY || !check_eip(vldmxcsr) || + res[0] != mxcsr_mask ) +goto fail; +printf("okay\n"); +} else printf("skipped\n"); --- a/tools/tests/x86_emulator/x86_emulate.c +++ b/tools/tests/x86_emulator/x86_emulate.c @@ -25,10 +25,29 @@ #define get_stub(stb) ((void *)((stb).addr = (uintptr_t)(stb).buf)) #define put_stub(stb) -bool emul_test_make_stack_executable(void) +uint32_t mxcsr_mask = 0xffbf; + +bool emul_test_init(void) { unsigned long sp; +if ( cpu_has_fxsr ) +{ +static union __attribute__((__aligned__(16))) { +char x[464]; +struct { +uint32_t other[6]; +uint32_t mxcsr; +uint32_t mxcsr_mask; +/* ... */ +}; +} fxs; + +asm ( "fxsave %0" : "=m" (fxs) ); +if ( fxs.mxcsr_mask ) +mxcsr_mask = fxs.mxcsr_mask; +} + /* * Mark the entire stack executable so that the stub executions * don't fault --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -43,8 +43,10 @@ #define X86_VENDOR_AMD 2 #define X86_VENDOR_UNKNOWN 0xff +extern uint32_t mxcsr_mask; + #define MMAP_SZ 16384 -bool emul_test_make_stack_executable(void); +bool emul_test_init(void); #include "x86_emulate/x86_emulate.h" @@ -69,6 +71,12 @@ static inline uint64_t xgetbv(uint32_t x (res.d & (1U << 23)) != 0; \ }) +#define cpu_has_fxsr ({ \ +struct cpuid_leaf res; \ +emul_test_cpuid(1, 0, , NULL); \ +(res.d & (1U << 24)) != 0; \ +}) + #define cpu_has_sse ({ \ struct cpuid_leaf res; \ emul_test_cpuid(1, 0, , NULL); \ ---
[Xen-devel] [PATCH v2 07/11] x86emul: support MMX/SSE/SSE2 insns with only register operands
This involves fixing a decode bug: VEX encoded insns aren't necessarily followed by a ModR/M byte. Signed-off-by: Jan Beulich--- v2: Correct {,v}pextrw operand descriptor. --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -269,10 +269,10 @@ static const struct { [0x6e] = { DstImplicit|SrcMem|ModRM|Mov }, [0x6f] = { DstImplicit|SrcMem|ModRM|Mov, simd_packed_int }, [0x70] = { SrcImmByte|ModRM|TwoOp, simd_other }, -[0x71 ... 0x73] = { SrcImmByte|ModRM }, +[0x71 ... 0x73] = { DstImplicit|SrcImmByte|ModRM }, [0x74 ... 0x76] = { DstImplicit|SrcMem|ModRM, simd_packed_int }, [0x77] = { DstImplicit|SrcNone }, -[0x78 ... 0x79] = { ModRM }, +[0x78 ... 0x79] = { ImplicitOps|ModRM }, [0x7c ... 0x7d] = { DstImplicit|SrcMem|ModRM, simd_other }, [0x7e] = { DstMem|SrcImplicit|ModRM|Mov }, [0x7f] = { DstMem|SrcImplicit|ModRM|Mov, simd_packed_int }, @@ -310,7 +310,7 @@ static const struct { [0xc2] = { DstImplicit|SrcImmByte|ModRM, simd_any_fp }, [0xc3] = { DstMem|SrcReg|ModRM|Mov }, [0xc4] = { DstReg|SrcImmByte|ModRM, simd_packed_int }, -[0xc5] = { SrcImmByte|ModRM }, +[0xc5] = { DstReg|SrcImmByte|ModRM|Mov }, [0xc6] = { DstImplicit|SrcImmByte|ModRM, simd_packed_fp }, [0xc7] = { ImplicitOps|ModRM }, [0xc8 ... 0xcf] = { ImplicitOps }, @@ -2515,12 +2515,21 @@ x86_decode( opcode |= b | MASK_INSR(vex.pfx, X86EMUL_OPC_PFX_MASK); +if ( !(d & ModRM) ) +{ +modrm_reg = modrm_rm = modrm_mod = modrm = 0; +break; +} + modrm = insn_fetch_type(uint8_t); modrm_mod = (modrm & 0xc0) >> 6; break; } +} +if ( d & ModRM ) +{ modrm_reg = ((rex_prefix & 4) << 1) | ((modrm & 0x38) >> 3); modrm_rm = modrm & 0x07; @@ -5670,6 +5679,18 @@ x86_emulate( CASE_SIMD_PACKED_FP(_VEX, 0x0f, 0x50): /* vmovmskp{s,d} {x,y}mm,reg */ CASE_SIMD_PACKED_INT(0x0f, 0xd7): /* pmovmskb {,x}mm,reg */ case X86EMUL_OPC_VEX_66(0x0f, 0xd7): /* vpmovmskb {x,y}mm,reg */ +opc = init_prefixes(stub); +opc[0] = b; +/* Convert GPR destination to %rAX. */ +rex_prefix &= ~REX_R; +vex.r = 1; +if ( !mode_64bit() ) +vex.w = 0; +opc[1] = modrm & 0xc7; +fic.insn_bytes = PFX_BYTES + 2; +simd_0f_to_gpr: +opc[fic.insn_bytes - PFX_BYTES] = 0xc3; + generate_exception_if(ea.type != OP_REG, EXC_UD); if ( vex.opcx == vex_none ) @@ -5697,17 +5718,6 @@ x86_emulate( get_fpu(X86EMUL_FPU_ymm, ); } -opc = init_prefixes(stub); -opc[0] = b; -/* Convert GPR destination to %rAX. */ -rex_prefix &= ~REX_R; -vex.r = 1; -if ( !mode_64bit() ) -vex.w = 0; -opc[1] = modrm & 0xc7; -fic.insn_bytes = PFX_BYTES + 2; -opc[2] = 0xc3; - copy_REX_VEX(opc, rex_prefix, vex); invoke_stub("", "", "=a" (dst.val) : [dummy] "i" (0)); @@ -5984,6 +5994,138 @@ x86_emulate( fic.insn_bytes = PFX_BYTES + 3; break; +CASE_SIMD_PACKED_INT(0x0f, 0x71):/* Grp12 */ +case X86EMUL_OPC_VEX_66(0x0f, 0x71): +CASE_SIMD_PACKED_INT(0x0f, 0x72):/* Grp13 */ +case X86EMUL_OPC_VEX_66(0x0f, 0x72): +switch ( modrm_reg & 7 ) +{ +case 2: /* psrl{w,d} $imm8,{,x}mm */ +/* vpsrl{w,d} $imm8,{x,y}mm,{x,y}mm */ +case 4: /* psra{w,d} $imm8,{,x}mm */ +/* vpsra{w,d} $imm8,{x,y}mm,{x,y}mm */ +case 6: /* psll{w,d} $imm8,{,x}mm */ +/* vpsll{w,d} $imm8,{x,y}mm,{x,y}mm */ +break; +default: +goto cannot_emulate; +} +simd_0f_shift_imm: +generate_exception_if(ea.type != OP_REG, EXC_UD); + +if ( vex.opcx != vex_none ) +{ +if ( vex.l ) +host_and_vcpu_must_have(avx2); +else +host_and_vcpu_must_have(avx); +get_fpu(X86EMUL_FPU_ymm, ); +} +else if ( vex.pfx ) +{ +vcpu_must_have(sse2); +get_fpu(X86EMUL_FPU_xmm, ); +} +else +{ +host_and_vcpu_must_have(mmx); +get_fpu(X86EMUL_FPU_mmx, ); +} + +opc = init_prefixes(stub); +opc[0] = b; +opc[1] = modrm; +opc[2] = imm1; +fic.insn_bytes = PFX_BYTES + 3; +simd_0f_reg_only: +opc[fic.insn_bytes - PFX_BYTES] = 0xc3; + +copy_REX_VEX(opc, rex_prefix, vex); +invoke_stub("", "", [dummy_out] "=g" (cr4) : [dummy_in] "i" (0) ); + +put_stub(stub); +put_fpu(); +break; + +case X86EMUL_OPC(0x0f, 0x73):/* Grp14 */ +switch ( modrm_reg & 7 ) +{ +case
[Xen-devel] [PATCH v2 04/11] x86emul: support MMX/SSE/SSE2 moves
Previously supported insns are being converted to the new model, and several new ones are being added. To keep the stub handling reasonably simple, integrate SET_SSE_PREFIX() into copy_REX_VEX(), at once switching the stubs to use an empty REX prefix instead of a double DS: one (no byte registers are being accessed, so an empty REX prefix has no effect), except (of course) for the 32-bit test harness build. Signed-off-by: Jan Beulich--- v2: Don't clear TwoOp for vmov{l,h}p{s,d} to memory. Move re-setting of TwoOp into VEX-specific code paths where possible. Special case {,v}maskmov{q,dqu} in stub invocation. Move {,v}movq code block to proper position. Add zero-mask {,v}maskmov{q,dqu} tests. --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -1557,6 +1557,29 @@ int main(int argc, char **argv) else printf("skipped\n"); +printf("%-40s", "Testing movq 32(%ecx),%xmm1..."); +if ( stack_exec && cpu_has_sse2 ) +{ +decl_insn(movq_from_mem2); + +asm volatile ( "pcmpeqb %%xmm1, %%xmm1\n" + put_insn(movq_from_mem2, "movq 32(%0), %%xmm1") + :: "c" (NULL) ); + +set_insn(movq_from_mem2); +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(movq_from_mem2) ) +goto fail; +asm ( "pcmpgtb %%xmm0, %%xmm0\n\t" + "pcmpeqb %%xmm1, %%xmm0\n\t" + "pmovmskb %%xmm0, %0" : "=r" (rc) ); +if ( rc != 0x ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + printf("%-40s", "Testing vmovq %xmm1,32(%edx)..."); if ( stack_exec && cpu_has_avx ) { @@ -1581,6 +1604,29 @@ int main(int argc, char **argv) else printf("skipped\n"); +printf("%-40s", "Testing vmovq 32(%edx),%xmm0..."); +if ( stack_exec && cpu_has_avx ) +{ +decl_insn(vmovq_from_mem); + +asm volatile ( "pcmpeqb %%xmm0, %%xmm0\n" + put_insn(vmovq_from_mem, "vmovq 32(%0), %%xmm0") + :: "d" (NULL) ); + +set_insn(vmovq_from_mem); +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(vmovq_from_mem) ) +goto fail; +asm ( "pcmpgtb %%xmm1, %%xmm1\n\t" + "pcmpeqb %%xmm0, %%xmm1\n\t" + "pmovmskb %%xmm1, %0" : "=r" (rc) ); +if ( rc != 0x ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + printf("%-40s", "Testing movdqu %xmm2,(%ecx)..."); if ( stack_exec && cpu_has_sse2 ) { @@ -1812,6 +1858,33 @@ int main(int argc, char **argv) else printf("skipped\n"); +printf("%-40s", "Testing movd 32(%ecx),%mm4..."); +if ( stack_exec && cpu_has_mmx ) +{ +decl_insn(movd_from_mem); + +asm volatile ( "pcmpgtb %%mm4, %%mm4\n" + put_insn(movd_from_mem, "movd 32(%0), %%mm4") + :: "c" (NULL) ); + +set_insn(movd_from_mem); +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(movd_from_mem) ) +goto fail; +asm ( "pxor %%mm2,%%mm2\n\t" + "pcmpeqb %%mm4, %%mm2\n\t" + "pmovmskb %%mm2, %0" : "=r" (rc) ); +if ( rc != 0xf0 ) +goto fail; +asm ( "pcmpeqb %%mm4, %%mm3\n\t" + "pmovmskb %%mm3, %0" : "=r" (rc) ); +if ( rc != 0x0f ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + printf("%-40s", "Testing movd %xmm2,32(%edx)..."); if ( stack_exec && cpu_has_sse2 ) { @@ -1836,6 +1909,34 @@ int main(int argc, char **argv) else printf("skipped\n"); +printf("%-40s", "Testing movd 32(%edx),%xmm3..."); +if ( stack_exec && cpu_has_sse2 ) +{ +decl_insn(movd_from_mem2); + +asm volatile ( "pcmpeqb %%xmm3, %%xmm3\n" + put_insn(movd_from_mem2, "movd 32(%0), %%xmm3") + :: "d" (NULL) ); + +set_insn(movd_from_mem2); +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(movd_from_mem2) ) +goto fail; +asm ( "pxor %%xmm1,%%xmm1\n\t" + "pcmpeqb %%xmm3, %%xmm1\n\t" + "pmovmskb %%xmm1, %0" : "=r" (rc) ); +if ( rc != 0xfff0 ) +goto fail; +asm ( "pcmpeqb %%xmm2, %%xmm2\n\t" + "pcmpeqb %%xmm3, %%xmm2\n\t" + "pmovmskb %%xmm2, %0" : "=r" (rc) ); +if ( rc != 0x000f ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + printf("%-40s", "Testing vmovd %xmm1,32(%ecx)..."); if ( stack_exec && cpu_has_avx ) { @@ -1860,6 +1961,34 @@ int main(int argc, char **argv) else printf("skipped\n"); +printf("%-40s", "Testing
[Xen-devel] [PATCH v2 03/11] x86emul: support most memory accessing MMX/SSE/SSE2 insns
This aims at covering most MMX/SSEn/AVX instructions in the 0x0f-escape space with memory operands. Not covered here are irregular moves, converts, and {,U}COMIS{S,D} (modifying EFLAGS). Note that the distinction between simd_*_fp isn't strictly needed, but I've kept them as separate entries since in an earlier version I needed them to be separate, and we may well find it useful down the road to have that distinction. Also take the opportunity and adjust the vmovdqu test case the new LDDQU one here has been cloned from: To zero a ymm register we don't need to go through hoops, as 128-bit AVX insns zero the upper portion of the destination register, and in the disabled AVX2 code there was a wrong YMM register used. Signed-off-by: Jan Beulich--- v2: Correct SSE2 p{max,min}{ub,sw} case labels. Correct MMX ps{ll,r{a,l}} and MMX punpckh{bw,wd,dq} operand sizes. Correct zapping of TwoOp in x86_decode_twobyte() (and vmovs{s,d} handling as a result). Also decode pshuf{h,l}w. Correct v{rcp,rsqrt}ss and vsqrts{s,d} comments (they allow memory operands). --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -1656,12 +1656,7 @@ int main(int argc, char **argv) { decl_insn(vmovdqu_from_mem); -#if 0 /* Don't use AVX2 instructions for now */ -asm volatile ( "vpcmpgtb %%ymm4, %%ymm4, %%ymm4\n" -#else -asm volatile ( "vpcmpgtb %%xmm4, %%xmm4, %%xmm4\n\t" - "vinsertf128 $1, %%xmm4, %%ymm4, %%ymm4\n" -#endif +asm volatile ( "vpxor %%xmm4, %%xmm4, %%xmm4\n" put_insn(vmovdqu_from_mem, "vmovdqu (%0), %%ymm4") :: "d" (NULL) ); @@ -1675,7 +1670,7 @@ int main(int argc, char **argv) #if 0 /* Don't use AVX2 instructions for now */ asm ( "vpcmpeqb %%ymm2, %%ymm2, %%ymm2\n\t" "vpcmpeqb %%ymm4, %%ymm2, %%ymm0\n\t" - "vpmovmskb %%ymm1, %0" : "=r" (rc) ); + "vpmovmskb %%ymm0, %0" : "=r" (rc) ); #else asm ( "vextractf128 $1, %%ymm4, %%xmm3\n\t" "vpcmpeqb %%xmm2, %%xmm2, %%xmm2\n\t" @@ -2083,6 +2078,67 @@ int main(int argc, char **argv) printf("skipped\n"); #endif +printf("%-40s", "Testing lddqu 4(%edx),%xmm4..."); +if ( stack_exec && cpu_has_sse3 ) +{ +decl_insn(lddqu); + +asm volatile ( "pcmpgtb %%xmm4, %%xmm4\n" + put_insn(lddqu, "lddqu 4(%0), %%xmm4") + :: "d" (NULL) ); + +set_insn(lddqu); +memset(res, 0x55, 64); +memset(res + 1, 0xff, 16); +regs.edx = (unsigned long)res; +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(lddqu) ) +goto fail; +asm ( "pcmpeqb %%xmm2, %%xmm2\n\t" + "pcmpeqb %%xmm4, %%xmm2\n\t" + "pmovmskb %%xmm2, %0" : "=r" (rc) ); +if ( rc != 0x ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + +printf("%-40s", "Testing vlddqu (%ecx),%ymm4..."); +if ( stack_exec && cpu_has_avx ) +{ +decl_insn(vlddqu); + +asm volatile ( "vpxor %%xmm4, %%xmm4, %%xmm4\n" + put_insn(vlddqu, "vlddqu (%0), %%ymm4") + :: "c" (NULL) ); + +set_insn(vlddqu); +memset(res + 1, 0xff, 32); +regs.ecx = (unsigned long)(res + 1); +rc = x86_emulate(, ); +if ( rc != X86EMUL_OKAY || !check_eip(vlddqu) ) +goto fail; +#if 0 /* Don't use AVX2 instructions for now */ +asm ( "vpcmpeqb %%ymm2, %%ymm2, %%ymm2\n\t" + "vpcmpeqb %%ymm4, %%ymm2, %%ymm0\n\t" + "vpmovmskb %%ymm0, %0" : "=r" (rc) ); +#else +asm ( "vextractf128 $1, %%ymm4, %%xmm3\n\t" + "vpcmpeqb %%xmm2, %%xmm2, %%xmm2\n\t" + "vpcmpeqb %%xmm4, %%xmm2, %%xmm0\n\t" + "vpcmpeqb %%xmm3, %%xmm2, %%xmm1\n\t" + "vpmovmskb %%xmm0, %0\n\t" + "vpmovmskb %%xmm1, %1" : "=r" (rc), "=r" (i) ); +rc |= i << 16; +#endif +if ( ~rc ) +goto fail; +printf("okay\n"); +} +else +printf("skipped\n"); + #undef decl_insn #undef put_insn #undef set_insn --- a/tools/tests/x86_emulator/x86_emulate.h +++ b/tools/tests/x86_emulator/x86_emulate.h @@ -81,6 +81,12 @@ static inline uint64_t xgetbv(uint32_t x (res.d & (1U << 26)) != 0; \ }) +#define cpu_has_sse3 ({ \ +struct cpuid_leaf res; \ +emul_test_cpuid(1, 0, , NULL); \ +(res.c & (1U << 0)) != 0; \ +}) + #define cpu_has_popcnt ({ \ struct cpuid_leaf res; \ emul_test_cpuid(1, 0, , NULL); \ --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -45,6 +45,8 @@ #define ModRM (1<<6) /* Destination is only written; never read. */ #define Mov (1<<7) +/* VEX/EVEX (SIMD only): 2nd source operand unused
[Xen-devel] [PATCH v2 01/11] x86emul: catch exceptions occurring in stubs
Before adding more use of stubs cloned from decoded guest insns, guard ourselves against mistakes there: Should an exception (with the noteworthy exception of #PF) occur inside the stub, forward it to the guest. Since the exception fixup table entry can't encode the address of the faulting insn itself, attach it to the return address instead. This at once provides a convenient place to hand the exception information back: The return address is being overwritten by it before branching to the recovery code. Take the opportunity and (finally!) add symbol resolution to the respective log messages (the new one is intentionally not being coded that way, as it covers stub addresses only, which don't have symbols associated). Also take the opportunity and make search_one_extable() static again. Suggested-by: Andrew CooperSigned-off-by: Jan Beulich --- There's one possible caveat here: A stub invocation immediately followed by another instruction having fault revovery attached to it would not work properly, as the table lookup can only ever find one of the two entries. Such CALL instructions would then need to be followed by a NOP for disambiguation (even if only a slim chance exists for the compiler to emit things that way). TBD: Instead of adding a 2nd search_exception_table() invocation to do_trap(), we may want to consider moving the existing one down: Xen code (except when executing stubs) shouldn't be raising #MF or #XM, and hence fixups attached to instructions shouldn't care about getting invoked for those. With that, doing the HVM special case for them before running search_exception_table() would be fine. Note that the two SIMD related stub invocations in the insn emulator intentionally don't get adjusted here, as subsequent patches will replace them anyway. --- a/xen/arch/x86/extable.c +++ b/xen/arch/x86/extable.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -62,7 +63,7 @@ void __init sort_exception_tables(void) sort_exception_table(__start___pre_ex_table, __stop___pre_ex_table); } -unsigned long +static unsigned long search_one_extable(const struct exception_table_entry *first, const struct exception_table_entry *last, unsigned long value) @@ -85,15 +86,88 @@ search_one_extable(const struct exceptio } unsigned long -search_exception_table(unsigned long addr) +search_exception_table(const struct cpu_user_regs *regs, bool check_stub) { -const struct virtual_region *region = find_text_region(addr); +const struct virtual_region *region = find_text_region(regs->rip); +unsigned long stub = this_cpu(stubs.addr); if ( region && region->ex ) -return search_one_extable(region->ex, region->ex_end - 1, addr); +return search_one_extable(region->ex, region->ex_end - 1, regs->rip); + +if ( check_stub && + regs->rip >= stub + STUB_BUF_SIZE / 2 && + regs->rip < stub + STUB_BUF_SIZE && + regs->rsp > (unsigned long)_stub && + regs->rsp < (unsigned long)get_cpu_info() ) +{ +unsigned long retptr = *(unsigned long *)regs->rsp; + +region = find_text_region(retptr); +retptr = region && region->ex + ? search_one_extable(region->ex, region->ex_end - 1, retptr) + : 0; +if ( retptr ) +{ +/* + * Put trap number and error code on the stack (in place of the + * original return address) for recovery code to pick up. + */ +*(unsigned long *)regs->rsp = regs->error_code | +((uint64_t)(uint8_t)regs->entry_vector << 32); +return retptr; +} +} + +return 0; +} + +#ifndef NDEBUG +static int __init stub_selftest(void) +{ +static const struct { +uint8_t opc[4]; +uint64_t rax; +union stub_exception_token res; +} tests[] __initconst = { +{ .opc = { 0x0f, 0xb9, 0xc3, 0xc3 }, /* ud1 */ + .res.fields.trapnr = TRAP_invalid_op }, +{ .opc = { 0x90, 0x02, 0x00, 0xc3 }, /* nop; add (%rax),%al */ + .rax = 0x0123456789abcdef, + .res.fields.trapnr = TRAP_gp_fault }, +{ .opc = { 0x02, 0x04, 0x04, 0xc3 }, /* add (%rsp,%rax),%al */ + .rax = 0xfedcba9876543210, + .res.fields.trapnr = TRAP_stack_error }, +}; +unsigned long addr = this_cpu(stubs.addr) + STUB_BUF_SIZE / 2; +unsigned int i; + +for ( i = 0; i < ARRAY_SIZE(tests); ++i ) +{ +uint8_t *ptr = map_domain_page(_mfn(this_cpu(stubs.mfn))) + + (addr & ~PAGE_MASK); +unsigned long res = ~0; + +memset(ptr, 0xcc, STUB_BUF_SIZE / 2); +memcpy(ptr, tests[i].opc, ARRAY_SIZE(tests[i].opc)); +unmap_domain_page(ptr); + +asm volatile ( "call *%[stb]\n" + ".Lret%=:\n\t" +
[Xen-devel] [PATCH v2 02/11] x86emul: flatten twobyte_table[]
... in the hope of making it more readable, and in preparation of adding a second field to the structure. Signed-off-by: Jan Beulich--- v2: Split off from subsequent patch, to (hopefully) aid review. --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -180,104 +180,82 @@ static const opcode_desc_t opcode_table[ ImplicitOps, ImplicitOps, ByteOp|DstMem|SrcNone|ModRM, DstMem|SrcNone|ModRM }; -static const opcode_desc_t twobyte_table[256] = { -/* 0x00 - 0x07 */ -ModRM, ImplicitOps|ModRM, DstReg|SrcMem16|ModRM, DstReg|SrcMem16|ModRM, -0, ImplicitOps, ImplicitOps, ImplicitOps, -/* 0x08 - 0x0F */ -ImplicitOps, ImplicitOps, 0, ImplicitOps, -0, ImplicitOps|ModRM, ImplicitOps, ModRM|SrcImmByte, -/* 0x10 - 0x17 */ -ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, -ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, -/* 0x18 - 0x1F */ -ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, -ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, -/* 0x20 - 0x27 */ -DstMem|SrcImplicit|ModRM, DstMem|SrcImplicit|ModRM, -DstImplicit|SrcMem|ModRM, DstImplicit|SrcMem|ModRM, -0, 0, 0, 0, -/* 0x28 - 0x2F */ -ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, -ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, -/* 0x30 - 0x37 */ -ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, -ImplicitOps, ImplicitOps, 0, ImplicitOps, -/* 0x38 - 0x3F */ -DstReg|SrcMem|ModRM, 0, DstReg|SrcImmByte|ModRM, 0, 0, 0, 0, 0, -/* 0x40 - 0x47 */ -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -/* 0x48 - 0x4F */ -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -/* 0x50 - 0x5F */ -ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, -ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, -/* 0x60 - 0x6F */ -ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, -ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ModRM, ImplicitOps|ModRM, -/* 0x70 - 0x7F */ -SrcImmByte|ModRM, SrcImmByte|ModRM, SrcImmByte|ModRM, SrcImmByte|ModRM, -ModRM, ModRM, ModRM, ImplicitOps, -ModRM, ModRM, 0, 0, ModRM, ModRM, ImplicitOps|ModRM, ImplicitOps|ModRM, -/* 0x80 - 0x87 */ -DstImplicit|SrcImm, DstImplicit|SrcImm, -DstImplicit|SrcImm, DstImplicit|SrcImm, -DstImplicit|SrcImm, DstImplicit|SrcImm, -DstImplicit|SrcImm, DstImplicit|SrcImm, -/* 0x88 - 0x8F */ -DstImplicit|SrcImm, DstImplicit|SrcImm, -DstImplicit|SrcImm, DstImplicit|SrcImm, -DstImplicit|SrcImm, DstImplicit|SrcImm, -DstImplicit|SrcImm, DstImplicit|SrcImm, -/* 0x90 - 0x97 */ -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -/* 0x98 - 0x9F */ -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -ByteOp|DstMem|SrcNone|ModRM|Mov, ByteOp|DstMem|SrcNone|ModRM|Mov, -/* 0xA0 - 0xA7 */ -ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps, DstBitBase|SrcReg|ModRM, -DstMem|SrcImmByte|ModRM, DstMem|SrcReg|ModRM, ModRM, ModRM, -/* 0xA8 - 0xAF */ -ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps, DstBitBase|SrcReg|ModRM, -DstMem|SrcImmByte|ModRM, DstMem|SrcReg|ModRM, -ImplicitOps|ModRM, DstReg|SrcMem|ModRM, -/* 0xB0 - 0xB7 */ -ByteOp|DstMem|SrcReg|ModRM, DstMem|SrcReg|ModRM, -DstReg|SrcMem|ModRM|Mov, DstBitBase|SrcReg|ModRM, -DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem|ModRM|Mov, -ByteOp|DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem16|ModRM|Mov, -/* 0xB8 - 0xBF */ -DstReg|SrcMem|ModRM, ModRM, -DstBitBase|SrcImmByte|ModRM, DstBitBase|SrcReg|ModRM, -DstReg|SrcMem|ModRM, DstReg|SrcMem|ModRM, -ByteOp|DstReg|SrcMem|ModRM|Mov, DstReg|SrcMem16|ModRM|Mov, -/* 0xC0 - 0xC7 */ -ByteOp|DstMem|SrcReg|ModRM, DstMem|SrcReg|ModRM, -SrcImmByte|ModRM, DstMem|SrcReg|ModRM|Mov, -SrcImmByte|ModRM, SrcImmByte|ModRM, SrcImmByte|ModRM, ImplicitOps|ModRM, -/* 0xC8 - 0xCF */ -ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, -ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps, -/* 0xD0 - 0xDF */ -ModRM, ModRM, ModRM, ModRM, ModRM, ModRM,
Re: [Xen-devel] [PATCH] x86emul: correct behavior for single iteration REP INS/OUTS
On Wed, Feb 01, 2017 at 03:59:16AM -0700, Jan Beulich wrote: > The initial operation done on these paths may raise an exception (for > ->read_io() that's possible only on the PV path, when the I/O port > access check has been deferred). We have to suppress put_rep_prefix() > updating rCX in that case. From an abstract perspective this also > applies to RETRY being returned. > > Reported-by: Wei Liu> Signed-off-by: Jan Beulich Reviewed-by: Wei Liu Tested-by: Wei Liu > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -3116,7 +3116,7 @@ x86_emulate( > if ( nr_reps == 1 && ops->read_io && ops->write ) > { > rc = ops->read_io(port, dst.bytes, , ctxt); > -if ( rc == X86EMUL_OKAY ) > +if ( rc != X86EMUL_UNHANDLEABLE ) > nr_reps = 0; > } > if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_ins ) > @@ -3157,7 +3157,7 @@ x86_emulate( > { > rc = read_ulong(ea.mem.seg, ea.mem.off, , dst.bytes, > ctxt, ops); > -if ( rc == X86EMUL_OKAY ) > +if ( rc != X86EMUL_UNHANDLEABLE ) > nr_reps = 0; > } > if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_outs ) > > > > x86emul: correct behavior for single iteration REP INS/OUTS > > The initial operation done on these paths may raise an exception (for > ->read_io() that's possible only on the PV path, when the I/O port > access check has been deferred). We have to suppress put_rep_prefix() > updating rCX in that case. From an abstract perspective this also > applies to RETRY being returned. > > Reported-by: Wei Liu > Signed-off-by: Jan Beulich > > --- a/xen/arch/x86/x86_emulate/x86_emulate.c > +++ b/xen/arch/x86/x86_emulate/x86_emulate.c > @@ -3116,7 +3116,7 @@ x86_emulate( > if ( nr_reps == 1 && ops->read_io && ops->write ) > { > rc = ops->read_io(port, dst.bytes, , ctxt); > -if ( rc == X86EMUL_OKAY ) > +if ( rc != X86EMUL_UNHANDLEABLE ) > nr_reps = 0; > } > if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_ins ) > @@ -3157,7 +3157,7 @@ x86_emulate( > { > rc = read_ulong(ea.mem.seg, ea.mem.off, , dst.bytes, > ctxt, ops); > -if ( rc == X86EMUL_OKAY ) > +if ( rc != X86EMUL_UNHANDLEABLE ) > nr_reps = 0; > } > if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_outs ) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 00/11] x86emul: MMX/SSE/SSE2 support
This includes support for AVX counterparts of them as well as a few later SSE additions (basically covering the entire 0f-prefixed opcode space, but not the 0f38 and 0f3a ones, nor 3dnow). 1: catch exceptions occurring in stubs 2: flatten twobyte_table[] 3: support most memory accessing MMX/SSE/SSE2 insns 4: support MMX/SSE/SSE2 moves 5: support MMX/SSE/SSE2 converts 6: support {,V}{,U}COMIS{S,D} 7: support MMX/SSE/SSE2 insns with only register operands 8: support {,V}{LD,ST}MXCSR 9: support {,V}MOVNTDQA 10: test: split generic and testcase specific parts 11: test coverage for SSE/SSE2 insns Signed-off-by: Jan Beulich--- v2: New patches 2 (split off from what now is 3), 10, and 11. Various bugs fixed which the added test code has helped find (see individual patches for details). ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] Xen and user requests.
Any idea? On Mon, 1/30/17, Jason Longwrote: Subject: Xen and user requests. To: "Xen-users" , "Xen-devel" Date: Monday, January 30, 2017, 4:01 AM Hello.Xen Project has any part for give feedback or feature requests from Xen users? Thank you. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86emul: correct behavior for single iteration REP INS/OUTS
The initial operation done on these paths may raise an exception (for ->read_io() that's possible only on the PV path, when the I/O port access check has been deferred). We have to suppress put_rep_prefix() updating rCX in that case. From an abstract perspective this also applies to RETRY being returned. Reported-by: Wei LiuSigned-off-by: Jan Beulich --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -3116,7 +3116,7 @@ x86_emulate( if ( nr_reps == 1 && ops->read_io && ops->write ) { rc = ops->read_io(port, dst.bytes, , ctxt); -if ( rc == X86EMUL_OKAY ) +if ( rc != X86EMUL_UNHANDLEABLE ) nr_reps = 0; } if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_ins ) @@ -3157,7 +3157,7 @@ x86_emulate( { rc = read_ulong(ea.mem.seg, ea.mem.off, , dst.bytes, ctxt, ops); -if ( rc == X86EMUL_OKAY ) +if ( rc != X86EMUL_UNHANDLEABLE ) nr_reps = 0; } if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_outs ) x86emul: correct behavior for single iteration REP INS/OUTS The initial operation done on these paths may raise an exception (for ->read_io() that's possible only on the PV path, when the I/O port access check has been deferred). We have to suppress put_rep_prefix() updating rCX in that case. From an abstract perspective this also applies to RETRY being returned. Reported-by: Wei Liu Signed-off-by: Jan Beulich --- a/xen/arch/x86/x86_emulate/x86_emulate.c +++ b/xen/arch/x86/x86_emulate/x86_emulate.c @@ -3116,7 +3116,7 @@ x86_emulate( if ( nr_reps == 1 && ops->read_io && ops->write ) { rc = ops->read_io(port, dst.bytes, , ctxt); -if ( rc == X86EMUL_OKAY ) +if ( rc != X86EMUL_UNHANDLEABLE ) nr_reps = 0; } if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_ins ) @@ -3157,7 +3157,7 @@ x86_emulate( { rc = read_ulong(ea.mem.seg, ea.mem.off, , dst.bytes, ctxt, ops); -if ( rc == X86EMUL_OKAY ) +if ( rc != X86EMUL_UNHANDLEABLE ) nr_reps = 0; } if ( (nr_reps > 1 || rc == X86EMUL_UNHANDLEABLE) && ops->rep_outs ) ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 2/2] swiotlb-xen: implement xen_swiotlb_get_sgtable callback
On 01/02/2017 10:57, Julien Grall wrote: Hi, On 01/02/2017 10:42, Andrii Anisov wrote: Ah! Ops :-) DEBUG_KERNEL? You said DEBUG_DRIVER last time. Both DEBUG_KERNEL and DEBUG_DRIVER are not used in code. DEBUG_KERNEL is used through Kconfigs to make available debug options to other different stuff, which consequently define DEBUG through appropriate makefiles. DEBUG_DRIVER enables DEBUG macro in base, opp and power makefiles. Technically this check should be done on any debug build for safety. So I would add new Kconfig DEBUG_KENREL that is selected by DEBUG_KERNEL. Sent the e-mail too quickly, sorry. I meant adding DEBUG_XEN selected by DEBUG_KERNEL. Cheers -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/VMX: introduce vmx_find_guest_msr()
>>> On 01.02.17 at 11:43,wrote: > On Wed, 2017-02-01 at 03:19 -0700, Jan Beulich wrote: >> > >> > > >> > > > >> > > > On 01.02.17 at 10:38, wrote: >> > On Tue, 2017-01-31 at 05:43 -0700, Jan Beulich wrote: >> > > >> > > > >> > > > >> > > > > >> > > > > >> > > > > > >> > > > > > >> > > > > > On 31.01.17 at 13:06, wrote: >> > > > On 31/01/17 11:54, Jan Beulich wrote: >> > > > > >> > > > > >> > > > > > >> > > > > > >> > > > > > > >> > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > On 31.01.17 at 12:49, wrote: >> > > > > > On 31/01/17 11:29, Jan Beulich wrote: >> > > > > > > >> > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > On 25.01.17 at 18:26, wrote: >> > > > > > > > > > >> > > > > > > > @@ -1369,6 +1410,9 @@ int vmx_add_msr(u32 msr, int type) >> > > > > > > > msr_area_elem->data = 0; >> > > > > > > > __vmwrite(VM_EXIT_MSR_STORE_COUNT, *msr_count); >> > > > > > > > __vmwrite(VM_ENTRY_MSR_LOAD_COUNT, *msr_count); >> > > > > > > > + >> > > > > > > > +sort(*msr_area, *msr_count, sizeof(struct >> > > > > > > > vmx_msr_entry), >> > > > > > > > + vmx_msr_entry_cmp, vmx_msr_entry_swap); >> > > > > > > ... how about avoiding the sort() here altogether, by simply >> > > > > > > going through the list linearly (which, being O(n), is still >> > > > > > > faster >> > > > > > > than sort())? The more that there is a linear scan already >> > > > > > > anyway. At which point it may then be beneficial to also keep >> > > > > > > the host MSR array sorted. >> > > > > > The entire point of sorting this list is to trade an O(n) search >> > > > > > for >> > > > > > O(log(n)) in every vmentry when fixing up the LBR MSR values. >> > > > > > >> > > > > > There should be no O(n) searches across the list after this patch. >> > > > > And that's indeed not the case. But the sort() is O(n * log(n)). >> > > > I don't understand what point you are trying to make. >> > > > >> > > > Adding MSRs to the list (turns out we have no remove yet) is a rare >> > > > occurrence, and in practice, this LBR addition is the only one which >> > > > happens at runtime rather than domain creation. >> > > > >> > > > However, you cannot have an efficient fixup on vmenter if the list >> > > > isn't >> > > > sorted, and it is not possible to sort a list in less than O(n * >> > > > log(n)) >> > > > in the general case. >> > > True, but we're adding incrementally, i.e. the list is already sorted, >> > > and it is already being walked linearly a few lines up from where the >> > > sort() invocation is being added. Hence the addition can as well be >> > > done without sort(), and then in O(n). >> > 1. Guest's MSR list is not sorted currently, which can be seen from >> > lbr_info: >> > >> > MSR_IA32_LASTINTFROMIP 0x01dd >> > MSR_IA32_LASTINTTOIP0x01de >> > MSR_C2_LASTBRANCH_TOS 0x01c9 >> > MSR_P4_LASTBRANCH_0_FROM_LIP0x0680 >> I don't understand: Your patch arranges for the list to be sorted, >> doesn't it? All I'm questioning is the approach of how the sorting >> is being done - what I'm trying to say is that I think you can do >> without any sort() invocation, leveraging the fact that the list >> you want to add to is already sorted (inductively, starting from a >> zero length list, by always inserting at the right spot, the list will >> always be sorted). > > You are right that the most effective way would be to use insertion sort. > However, this will require to write some > new code for it. Since I'm not > convinced that performance is really critical here, the heap sort code > is simply > reused. Which is - I think - more new code than simply leveraging the existing linear scan over the array, recording the insertion point, perhaps (since it's sorted already) bailing once an entry with a higher numbered MSR is found, and memmov()ing any higher numbered entries. Performance isn't the main aspect here, but I don't think we should write slow code (even if that code is being used rarely) when - with about the same amount of effort/code - we can have a faster variant. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel