Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
Hi, On 05/04/18 04:17, Alex Williamson wrote: > On Thu, 5 Apr 2018 10:53:38 +1000 > Alexey Kardashevskiy wrote: > >> On 5/4/18 9:09 am, Alex Williamson wrote: >>> On Wed, 4 Apr 2018 22:30:50 +0200 >>> Eric Auger wrote: >>> The 567b5b309abe ("vfio/pci: Relax DMA map errors for MMIO regions") added an error message if a passed memory section address or size is not aligned to the page size and thus cannot be DMA mapped. This patch fixes the trace by printing the region name and the memory region section offset within the address space (instead of offset_within_region). We also turn the error_report into a trace event. Indeed, In some cases, the traces can be confusing to non expert end-users and let think the use case does not work (whereas it works as before). This is the case where a BAR is successively mapped at different GPAs and its sections are not compatible with dma map. The listener is called several times and traces are issued for each intermediate mapping. The end-user cannot easily match those GPAs against the final GPA output by lscpi. So let's keep those information to informed users. In mid term, the plan is to advise the user about BAR relocation relevance. Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions" Signed-off-by: Alexey Kardashevskiy Signed-off-by: Eric Auger --- v1 -> v2: - use a trace point - add some details in the commit message >>> >>> I think this is v2 with v1 being >>> Message-Id:<20180322081837.21460-1-...@ozlabs.ru> but this is >>> substantially different from Alexey's posting so I'd like to verify the >>> Sign-off. Alexey? >> >> My understanding it is not "sign-off" any more (I am fine either way, it >> just does not seem fully correct), but >> >> Reviewed-by: Alexey Kardashevskiy > > Yep, I think Eric was intending to give you credit since this patch > builds on your patch. I'll include the R-b. Yes this was my intent, sorry for the confusion. I was eager to get this for 2.12, hence this follow-up. Thanks Eric > > There's still work to do, I'd like to see a single, concise message as > we're parsing the device, but there's no benefit in alarming users when > nothing has changed and it's too late into the release cycle to do more > than just silence this for now. Patches welcome towards a better > solution, to be considered for 2.13. Thanks, > > Alex > >>> >>> I agree that this seems to be the right thing to do for 2.12, nothing >>> has changed for these mappings and the error reports are difficult even >>> for experts to decipher and explain to users. Thanks, >>> >>> Alex >>> --- hw/vfio/common.c | 11 +-- hw/vfio/trace-events | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 5e84716..07ffa0b 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -548,12 +548,11 @@ static void vfio_listener_region_add(MemoryListener *listener, hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1; if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) { -error_report("Region 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx - " is not aligned to 0x%"HWADDR_PRIx - " and cannot be mapped for DMA", - section->offset_within_region, - int128_getlo(section->size), - pgmask + 1); +trace_vfio_listener_region_add_no_dma_map( +memory_region_name(section->mr), +section->offset_within_address_space, +int128_getlo(section->size), +pgmask + 1); return; } } diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index 79f63a2..20109cb 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -90,6 +90,7 @@ vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "i vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add 0x%"PRIx64" - 0x%"PRIx64 vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add [iommu] 0x%"PRIx64" - 0x%"PRIx64 vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" +vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING region_del 0x%"PRIx64" - 0x%"PRIx64 vfio_listen
[Qemu-devel] Virito devices bridge vs Macvtap performance
Hi, I am doing kvm guest network performance analysis to compare relevant alternatives for virtio devices on Arrach64. I have generated bridge tap and macvtap performance numbers on guest 10G interface connected back to another 10G on other host. The results for guest with "macvtap" vs "bridge" are marginally differed and the difference is +/- 3%. As per my understanding on macvtap, expected better performance values with macvtap. for macvtap device:- -netdev tap,vhost=on,vhostforce=on,id=macvtap_netdev,fd=3 3<>/dev/tap$(cat /sys/class/net/macvtap1/ifindex) \ -device virtionet-pci,netdev=macvtap_netdev,id=net0,mac=$(cat /sys/class/net/macvtap1/address) for Bridge tap device:- -netdev type=tap,vhost=on,vhostforce=on,id=tap0,ifname=tap0,script=no,downscript=no -device virtio-net-pci,netdev=tap0,mac=12:03:04:05:06:07 ---< End of commands> Anyone please tell me if I missed anything. Other information. - Kernel sources : 4.14.28 qemu-aarch64 : 2.11.91 Did the same experiments with ThunderX vnic and Intel 10G nic and results are as mentioned above. Tried macvtap in bridge and passthru modes. Tell me if you need further investigations or more data . thanks, Prakash B
[Qemu-devel] [PATCH for-2.12] hw/arm/fsl-imx: Fix introspection problem with fsl-imx6 and fsl-imx7
QEMU currently exits unexpectedly when trying to introspect the fsl-imx6 and fsl-imx7 devices on systems with many SMP CPUs: $ echo "{'execute':'qmp_capabilities'}"\ "{'execute':'device-list-properties',"\ " 'arguments':{'typename':'fsl,imx6'}}" \ | arm-softmmu/qemu-system-arm -M virt,accel=qtest -qmp stdio -smp 8 {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} {"return": {}} fsl,imx6: Only 4 CPUs are supported (8 requested) And: $ echo "{'execute':'qmp_capabilities'}"\ "{'execute':'device-list-properties',"\ " 'arguments':{'typename':'fsl,imx7'}}" \ | arm-softmmu/qemu-system-arm -M raspi2,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} {"return": {}} fsl,imx7: Only 2 CPUs are supported (4 requested) This happens because these devices are doing an exit() from their instance_init function - which should never be done since instance_init can be called at any time for device introspection! Fix it by moving the deadly check into the realize() function instead. Signed-off-by: Thomas Huth --- hw/arm/fsl-imx6.c | 14 +++--- hw/arm/fsl-imx7.c | 13 +++-- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c index b6ac72d..9dfbc9a 100644 --- a/hw/arm/fsl-imx6.c +++ b/hw/arm/fsl-imx6.c @@ -37,13 +37,7 @@ static void fsl_imx6_init(Object *obj) char name[NAME_SIZE]; int i; -if (smp_cpus > FSL_IMX6_NUM_CPUS) { -error_report("%s: Only %d CPUs are supported (%d requested)", - TYPE_FSL_IMX6, FSL_IMX6_NUM_CPUS, smp_cpus); -exit(1); -} - -for (i = 0; i < smp_cpus; i++) { +for (i = 0; i < MIN(smp_cpus, FSL_IMX6_NUM_CPUS); i++) { object_initialize(&s->cpu[i], sizeof(s->cpu[i]), "cortex-a9-" TYPE_ARM_CPU); snprintf(name, NAME_SIZE, "cpu%d", i); @@ -119,6 +113,12 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp) uint16_t i; Error *err = NULL; +if (smp_cpus > FSL_IMX6_NUM_CPUS) { +error_setg(errp, "%s: Only %d CPUs are supported (%d requested)", + TYPE_FSL_IMX6, FSL_IMX6_NUM_CPUS, smp_cpus); +return; +} + for (i = 0; i < smp_cpus; i++) { /* On uniprocessor, the CBAR is set to 0 */ diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c index 26ef36c..390b431 100644 --- a/hw/arm/fsl-imx7.c +++ b/hw/arm/fsl-imx7.c @@ -35,13 +35,8 @@ static void fsl_imx7_init(Object *obj) char name[NAME_SIZE]; int i; -if (smp_cpus > FSL_IMX7_NUM_CPUS) { -error_report("%s: Only %d CPUs are supported (%d requested)", - TYPE_FSL_IMX7, FSL_IMX7_NUM_CPUS, smp_cpus); -exit(1); -} -for (i = 0; i < smp_cpus; i++) { +for (i = 0; i < MIN(smp_cpus, FSL_IMX7_NUM_CPUS); i++) { object_initialize(&s->cpu[i], sizeof(s->cpu[i]), ARM_CPU_TYPE_NAME("cortex-a7")); snprintf(name, NAME_SIZE, "cpu%d", i); @@ -197,6 +192,12 @@ static void fsl_imx7_realize(DeviceState *dev, Error **errp) qemu_irq irq; char name[NAME_SIZE]; +if (smp_cpus > FSL_IMX7_NUM_CPUS) { +error_setg(errp, "%s: Only %d CPUs are supported (%d requested)", + TYPE_FSL_IMX7, FSL_IMX7_NUM_CPUS, smp_cpus); +return; +} + for (i = 0; i < smp_cpus; i++) { o = OBJECT(&s->cpu[i]); -- 1.8.3.1
Re: [Qemu-devel] [qemu-web PATCH] download: Add instructions for MacPorts
On 04/04/2018 19:41, Stefan Weil wrote: > Am 04.04.2018 um 18:11 schrieb Paolo Bonzini: >> On 04/04/2018 17:55, Stefan Weil wrote: >>> By the way: https://qemu.weilnetz.de provides https (maybe I should >>> enforce it), it includes sha512, and I also sign the binaries with my >>> key. You still have to trust me, Debian and Cygwin (which provides lots >>> of libraries used for the build). >> >> Cool! I had noticed sha512, but it is not very useful without https >> (except to verify bitflips). Good news that you support https, we >> should change the website to use https links instead. >> >> Regarding signing, there is no GPG signature. That's okay, but we >> should document how to verify the installer signature from either Linux >> or Windows. >> >> Thanks, >> >> Paolo > > > The executables (installer, installed exe files) are signed using > osslsigncode (https://packages.debian.org/sid/otherosfs/osslsigncode) > and my personal CACert key for code signing. > > The signatures can be checked on Windows (e.g. during the installation > process or from Windows Explorer with file properties) or on Linux (see > example below). That's Windows standard. The only problem is that > Windows does not automatically accept CACert keys (and that I have no > better key for code signing). Very good, thanks. I'll add that information to the wiki. Paolo > Stefan > > > $ osslsigncode verify /var/www/html/w32/qemu-w32-setup-20180321.exe > Current PE checksum : 04D7CD55 > Calculated PE checksum: 04D7CD55 > > Message digest algorithm : SHA1 > Current message digest: B2B13EB4765B4708D999BE3E4893915BBCAB0F8E > Calculated message digest : B2B13EB4765B4708D999BE3E4893915BBCAB0F8E > > Signature verification: ok > > Number of signers: 1 > Signer #0: > Subject: /CN=Stefan Weil/emailAddress=s...@weilnetz.de > Issuer : /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing > Authority/emailAddress=supp...@cacert.org > Serial : 0D6AA6 > > Number of certificates: 2 > Cert #0: > Subject: /CN=Stefan Weil/emailAddress=s...@weilnetz.de > Issuer : /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing > Authority/emailAddress=supp...@cacert.org > Serial : 0D6AA6 > -- > Cert #1: > Subject: /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing > Authority/emailAddress=supp...@cacert.org > Issuer : /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing > Authority/emailAddress=supp...@cacert.org > Serial : 0 > > Succeeded >
[Qemu-devel] [PATCH v2] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init
An instance_init function must not fail - and might be called multiple times, e.g. during device introspection with the 'device-list-properties' QMP command. Since the integratorcm device ignores this rule, QEMU currently aborts in this case (though it really should not): echo "{'execute':'qmp_capabilities'}"\ "{'execute':'device-list-properties',"\ "'arguments':{'typename':'integrator_core'}}" \ | arm-softmmu/qemu-system-arm -M integratorcp,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} {"return": {}} RAMBlock "integrator.flash" already registered, abort! Aborted (core dumped) Move the problematic code to the realize() function instead to fix this problem. Reviewed-by: Philippe Mathieu-Daud?? Signed-off-by: Thomas Huth --- v2: Use "OBJECT(d)" instead of "(Object *)d" hw/arm/integratorcp.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index e8303b8..58b40ef 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -266,7 +266,6 @@ static const MemoryRegionOps integratorcm_ops = { static void integratorcm_init(Object *obj) { IntegratorCMState *s = INTEGRATOR_CM(obj); -SysBusDevice *dev = SYS_BUS_DEVICE(obj); s->cm_osc = 0x0148; /* ??? What should the high bits of this value be? */ @@ -276,20 +275,28 @@ static void integratorcm_init(Object *obj) s->cm_init = 0x0112; s->cm_refcnt_offset = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24, 1000); -memory_region_init_ram(&s->flash, obj, "integrator.flash", 0x10, - &error_fatal); -memory_region_init_io(&s->iomem, obj, &integratorcm_ops, s, - "integratorcm", 0x0080); -sysbus_init_mmio(dev, &s->iomem); - -integratorcm_do_remap(s); /* ??? Save/restore. */ } static void integratorcm_realize(DeviceState *d, Error **errp) { IntegratorCMState *s = INTEGRATOR_CM(d); +SysBusDevice *dev = SYS_BUS_DEVICE(d); +Error *local_err = NULL; + +memory_region_init_ram(&s->flash, OBJECT(d), "integrator.flash", 0x10, + &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} + +memory_region_init_io(&s->iomem, OBJECT(d), &integratorcm_ops, s, + "integratorcm", 0x0080); +sysbus_init_mmio(dev, &s->iomem); + +integratorcm_do_remap(s); if (s->memsz >= 256) { integrator_spd[31] = 64; -- 1.8.3.1
Re: [Qemu-devel] [PULL 16/20] target/arm: Use vector infrastructure for aa64 compares
On 05.04.2018 02:54, Richard Henderson wrote: > On 04/05/2018 10:07 AM, Richard Henderson wrote: >> On 04/05/2018 02:49 AM, Emilio G. Cota wrote: >>> 1. grab this binary: >>> http://cs.columbia.edu/~cota/qemu/nbench-aarch64 >>> 2. run it on a PowerPC host with: >>> $ aarch64-linux-user/qemu-aarch64 nbench-aarch64 -V >>> >>> Note: the "-V" (or "-v") flag is important! Without it, there's no segfault. >> >> How quickly? I did not see one up until it exited for lack of NNET.DAT. >> >> I will note that I am using gcc 7.2 on gcc112 >> (/opt/cfarm/gcc-latest/bin/gcc). >> >> Are you using gcc 4.8.5, and is commit 74912f6dad in your tree? >> That commit might have made a difference... > > Bah. I confirm that it doesn't, and that the test still fails. > > Since qemu does work on the same host when built with gcc 7.2, I can only > blame > this failure on a compiler bug wrt gcc 4.8 on ppc64. > > I have not tracked down exactly what's going wrong, and probably won't unless > someone feels that it's worthwhile. This only begs the question of whether we > should blacklist this compiler entirely. Certainly it's old enough that it's > probably not worth fixing. GCC 4.8 is still used in enterprise distros like RHEL7 and AFAIK also in SLES 12 ... so blacklisting this compiler does not sound like a good idea to me. Thomas
Re: [Qemu-devel] [PULL 16/20] target/arm: Use vector infrastructure for aa64 compares
On Thu, Apr 05, 2018 at 10:54:46 +1000, Richard Henderson wrote: > On 04/05/2018 10:07 AM, Richard Henderson wrote: > > On 04/05/2018 02:49 AM, Emilio G. Cota wrote: > >> 1. grab this binary: > >> http://cs.columbia.edu/~cota/qemu/nbench-aarch64 > >> 2. run it on a PowerPC host with: > >> $ aarch64-linux-user/qemu-aarch64 nbench-aarch64 -V > >> > >> Note: the "-V" (or "-v") flag is important! Without it, there's no > >> segfault. > > > > How quickly? I did not see one up until it exited for lack of NNET.DAT. > > > > I will note that I am using gcc 7.2 on gcc112 > > (/opt/cfarm/gcc-latest/bin/gcc). > > > > Are you using gcc 4.8.5, and is commit 74912f6dad in your tree? > > That commit might have made a difference... > > Bah. I confirm that it doesn't, and that the test still fails. > > Since qemu does work on the same host when built with gcc 7.2, I can only > blame > this failure on a compiler bug wrt gcc 4.8 on ppc64. Thanks! I confirm it as well that it works with the more recent gcc. > I have not tracked down exactly what's going wrong, and probably won't unless > someone feels that it's worthwhile. This only begs the question of whether we > should blacklist this compiler entirely. Certainly it's old enough that it's > probably not worth fixing. Agreed. Thanks, Emilio
Re: [Qemu-devel] [PATCH v3 4/4] target/ppc: generalize check on radix when in HV mode
On Thu, Mar 15, 2018 at 01:34:02PM +, Cédric Le Goater wrote: > On a POWER9 processor, the first doubleword of the partition table > entry (as pointed to by the PTCR) indicates whether the host uses HPT > or Radix Tree translation for that partition. Use that bit to check > for radix mode on pseries and powernv QEMU machines. > > Signed-off-by: Cédric Le Goater Reviewed-by: David Gibson > --- > > Changes since v2: > > - reworked ppc64_v3_radix() to distinguish pseries machine from >powernv machines > - kept ppc64_radix_guest() > > Changes since v1: > > - fixed commit log > - introduced ppc64_v3_get_patbe0() > - renamed ppc64_radix() in ppc64_v3_radix() > > target/ppc/mmu-book3s-v3.c | 23 +-- > target/ppc/mmu-book3s-v3.h | 4 +++- > target/ppc/mmu_helper.c| 4 ++-- > 3 files changed, 26 insertions(+), 5 deletions(-) > > diff --git a/target/ppc/mmu-book3s-v3.c b/target/ppc/mmu-book3s-v3.c > index b60df4408f3b..89edbb6abc5c 100644 > --- a/target/ppc/mmu-book3s-v3.c > +++ b/target/ppc/mmu-book3s-v3.c > @@ -19,16 +19,35 @@ > > #include "qemu/osdep.h" > #include "cpu.h" > +#include "qemu/error-report.h" > #include "mmu-hash64.h" > #include "mmu-book3s-v3.h" > #include "mmu-radix64.h" > > +bool ppc64_v3_radix(PowerPCCPU *cpu) > +{ > +CPUPPCState *env = &cpu->env; > + > +/* sPAPR machine */ > +if (cpu->vhyp) { > +return ppc64_radix_guest(cpu); > +} > + > +/* PowerNV machine - only HV mode is supported */ > +if (msr_hv) { > +return ppc64_v3_get_patbe0(cpu) & PATBE0_HR; > +} else { > +error_report("PowerNV guest support Unimplemented"); > +exit(1); > +} > +} > + > int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx, >int mmu_idx) > { > -if (ppc64_radix_guest(cpu)) { /* Guest uses radix */ > +if (ppc64_v3_radix(cpu)) { > return ppc_radix64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx); > -} else { /* Guest uses hash */ > +} else { > return ppc_hash64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx); > } > } > diff --git a/target/ppc/mmu-book3s-v3.h b/target/ppc/mmu-book3s-v3.h > index a7ab580c3140..9721791d2dd3 100644 > --- a/target/ppc/mmu-book3s-v3.h > +++ b/target/ppc/mmu-book3s-v3.h > @@ -29,7 +29,8 @@ > #define PTCR_PATS 0x001FULL /* Partition Table > Size */ > > /* Partition Table Entry Fields */ > -#define PATBE1_GR 0x8000 > +#define PATBE0_HR PPC_BIT(0)/* 1:Host Radix 0:HPT > */ > +#define PATBE1_GR PPC_BIT(0)/* 1:Guest Radix 0:HPT > */ > > /* Process Table Entry */ > struct prtb_entry { > @@ -50,6 +51,7 @@ static inline bool ppc64_radix_guest(PowerPCCPU *cpu) > > return !!(vhc->get_patbe(cpu->vhyp) & PATBE1_GR); > } > +bool ppc64_v3_radix(PowerPCCPU *cpu); > > int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx, >int mmu_idx); > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c > index 03009eee723a..fba203cdef18 100644 > --- a/target/ppc/mmu_helper.c > +++ b/target/ppc/mmu_helper.c > @@ -1285,7 +1285,7 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf, > CPUPPCState *env) > dump_slb(f, cpu_fprintf, ppc_env_get_cpu(env)); > break; > case POWERPC_MMU_VER_3_00: > -if (ppc64_radix_guest(ppc_env_get_cpu(env))) { > +if (ppc64_v3_radix(ppc_env_get_cpu(env))) { > /* TODO - Unsupported */ > } else { > dump_slb(f, cpu_fprintf, ppc_env_get_cpu(env)); > @@ -1431,7 +1431,7 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr > addr) > case POWERPC_MMU_VER_2_07: > return ppc_hash64_get_phys_page_debug(cpu, addr); > case POWERPC_MMU_VER_3_00: > -if (ppc64_radix_guest(ppc_env_get_cpu(env))) { > +if (ppc64_v3_radix(ppc_env_get_cpu(env))) { > return ppc_radix64_get_phys_page_debug(cpu, addr); > } else { > return ppc_hash64_get_phys_page_debug(cpu, addr); -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.12] memfd: fix vhost-user-test on non-memfd capable host
On Wed, Mar 28, 2018 at 02:18:04PM +0200, Marc-André Lureau wrote: > On RHEL7, memfd is not supported, and vhost-user-test fails: > TEST: tests/vhost-user-test... (pid=10248) > /x86_64/vhost-user/migrate: > qemu-system-x86_64: -object memory-backend-memfd,id=mem,size=2M,: failed to > create memfd > FAIL > > There is a qemu_memfd_check() to prevent running memfd path, but it > also checks for fallback implementation. Let's specialize > qemu_memfd_check() to check memfd only, while qemu_memfd_alloc_check() > checks for the qemu_memfd_alloc() API. > > Reported-by: Miroslav Rezanina > Tested-by: Miroslav Rezanina > Signed-off-by: Marc-André Lureau > --- Patch is solving failing build on RHEL 7. Needed in 2.12. Tested-by: Miroslav Rezanina > include/qemu/memfd.h | 1 + > hw/virtio/vhost.c| 2 +- > util/memfd.c | 30 +- > 3 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h > index de10198ed6..49e79634da 100644 > --- a/include/qemu/memfd.h > +++ b/include/qemu/memfd.h > @@ -18,6 +18,7 @@ > > int qemu_memfd_create(const char *name, size_t size, bool hugetlb, >uint64_t hugetlbsize, unsigned int seals, Error > **errp); > +bool qemu_memfd_alloc_check(void); > void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals, > int *fd, Error **errp); > void qemu_memfd_free(void *ptr, size_t size, int fd); > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 250f886acb..27c1ec5fe8 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -1223,7 +1223,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, > if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) { > error_setg(&hdev->migration_blocker, > "Migration disabled: vhost lacks VHOST_F_LOG_ALL > feature."); > -} else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_check()) { > +} else if (vhost_dev_log_is_shared(hdev) && > !qemu_memfd_alloc_check()) { > error_setg(&hdev->migration_blocker, > "Migration disabled: failed to allocate shared > memory"); > } > diff --git a/util/memfd.c b/util/memfd.c > index 07d579ea7d..277f7211af 100644 > --- a/util/memfd.c > +++ b/util/memfd.c > @@ -173,7 +173,13 @@ enum { > MEMFD_TODO > }; > > -bool qemu_memfd_check(void) > +/** > + * qemu_memfd_alloc_check(): > + * > + * Check if qemu_memfd_alloc() can allocate, including using a > + * fallback implementation when host doesn't support memfd. > + */ > +bool qemu_memfd_alloc_check(void) > { > static int memfd_check = MEMFD_TODO; > > @@ -188,3 +194,25 @@ bool qemu_memfd_check(void) > > return memfd_check == MEMFD_OK; > } > + > +/** > + * qemu_memfd_check(): > + * > + * Check if host supports memfd. > + */ > +bool qemu_memfd_check(void) > +{ > +static int memfd_check = MEMFD_TODO; > + > +if (memfd_check == MEMFD_TODO) { > +int mfd = memfd_create("test", 0); > +if (mfd >= 0) { > +memfd_check = MEMFD_OK; > +close(mfd); > +} else { > +memfd_check = MEMFD_KO; > +} > +} > + > +return memfd_check == MEMFD_OK; > +} > -- > 2.17.0.rc1.1.g4c4f2b46a3 > >
Re: [Qemu-devel] [PATCH v2] linux-user: fix preadv/pwritev offsets
On 04/05/2018 11:41 AM, Max Filippov wrote: > +static void target_low_high_to_host_low_high(abi_ulong tlow, > + abi_ulong thigh, > + unsigned long *hlow, > + unsigned long *hhigh) > +{ > +#if TARGET_LONG_BITS == HOST_LONG_BITS > +*hlow = tlow; > +*hhigh = thigh; > +#elif TARGET_LONG_BITS < HOST_LONG_BITS > +*hlow = tlow | (unsigned long)thigh << TARGET_LONG_BITS; > +*hhigh = 0; > +#else > +*hlow = (unsigned long)tlow; > +*hhigh = (unsigned long)(tlow >> HOST_LONG_BITS); > +#endif It might still be worth a check for HOST_LONG_BITS >= 2 * TARGET_LONG_BITS and #error otherwise. Or explicit checks vs 32 & 64 if you like. r~
Re: [Qemu-devel] [PATCH] linux-user: fix preadv/pwritev offsets
On 04/05/2018 11:33 AM, Max Filippov wrote: > +static void target_low_high_to_host_low_high(abi_ulong tlow, > + abi_ulong thigh, > + unsigned long *hlow, > + unsigned long *hhigh) > +{ > +#if TARGET_LONG_BITS == HOST_LONG_BITS > +*hlow = tlow; > +*hhigh = thigh; > +#elif TARGET_LONG_BITS < HOST_LONG_BITS > +*hlow = tlow | (unsigned long)thigh << TARGET_LONG_BITS; > +*hhigh = 0; > +#else > +*hlow = (unsigned long)tlow; > +*hhigh = (unsigned long)(thigh >> HOST_LONG_BITS); > +#endif This third clause does not look right. It might have been thigh >> HOST_LONG_BITS, which is good for the specific cases (32, 64) that we will always have. But you'd certainly want to not pretend otherwise by only checking relative sizes. r~
Re: [Qemu-devel] [RFC PATCH v2 4/4] memory: Add memory_region_set_priority()
On 5/4/18 11:22 am, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé > --- > Sadly I'm missing something, this does not work. What does not work precisely? memory_region_set_priority() is not called or visit_type_int32() does not return priority, etc? > > memory.c | 18 +- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/memory.c b/memory.c > index eaa5fa7f23..ae45ea7779 100644 > --- a/memory.c > +++ b/memory.c > @@ -1225,6 +1225,22 @@ static void memory_region_get_priority(Object *obj, > Visitor *v, > visit_type_int32(v, name, &value, errp); > } > > +static void memory_region_set_priority(Object *obj, Visitor *v, > + const char *name, void *opaque, > + Error **errp) > +{ > +MemoryRegion *mr = MEMORY_REGION(obj); > +int32_t priority; > +Error *local_err = NULL; > + > +visit_type_int32(v, name, &priority, &local_err); > +if (local_err) { > +error_propagate(errp, local_err); > +return; > +} > +mr->priority = priority; > +} > + > static void memory_region_get_size(Object *obj, Visitor *v, const char *name, > void *opaque, Error **errp) > { > @@ -1260,7 +1276,7 @@ static void memory_region_initfn(Object *obj) > NULL, NULL, &error_abort); > object_property_add(OBJECT(mr), "priority", "int32", > memory_region_get_priority, > -NULL, /* memory_region_set_priority */ > +memory_region_set_priority, > NULL, NULL, &error_abort); > object_property_add(OBJECT(mr), "size", "uint64", > memory_region_get_size, > -- Alexey
Re: [Qemu-devel] [PATCHv2 for-2.13 0/2] Helpers to obtain host page sizes for guest RAM
On Wed, Apr 04, 2018 at 07:35:17PM -0700, no-re...@patchew.org wrote: > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Type: series > Message-id: 20180405022002.17809-1-da...@gibson.dropbear.id.au > Subject: [Qemu-devel] [PATCHv2 for-2.13 0/2] Helpers to obtain host page > sizes for guest RAM > > === TEST SCRIPT BEGIN === > #!/bin/bash > > BASE=base > n=1 > total=$(git log --oneline $BASE.. | wc -l) > failed=0 > > git config --local diff.renamelimit 0 > git config --local diff.renames True > git config --local diff.algorithm histogram > > commits="$(git log --format=%H --reverse $BASE..)" > for c in $commits; do > echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." > if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; > then > failed=1 > echo > fi > n=$((n+1)) > done > > exit $failed > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > From https://github.com/patchew-project/qemu > * [new tag] > patchew/20180405022002.17809-1-da...@gibson.dropbear.id.au -> > patchew/20180405022002.17809-1-da...@gibson.dropbear.id.au > Switched to a new branch 'test' > 3a2019a8ad Add host_memory_backend_pagesize() helper > d70b1b2b92 Make qemu_mempath_getpagesize() accept NULL > > === OUTPUT BEGIN === > Checking PATCH 1/2: Make qemu_mempath_getpagesize() accept NULL... > ERROR: braces {} are necessary for all arms of this statement > #96: FILE: util/mmap-alloc.c:56: > +} while (ret != 0 && errno == EINTR); > [...] AFAICT this is a false positive where checkpatch is mistaking a do {} while for a plain while loop. > total: 1 errors, 0 warnings, 82 lines checked > > Your patch has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > Checking PATCH 2/2: Add host_memory_backend_pagesize() helper... > === OUTPUT END === > > Test command exited with code: 1 > > > --- > Email generated automatically by Patchew [http://patchew.org/]. > Please send your feedback to patchew-de...@redhat.com -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCHv2 for-2.13 0/2] Helpers to obtain host page sizes for guest RAM
Hi, This series failed docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. Type: series Message-id: 20180405022002.17809-1-da...@gibson.dropbear.id.au Subject: [Qemu-devel] [PATCHv2 for-2.13 0/2] Helpers to obtain host page sizes for guest RAM === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' 3a2019a8ad Add host_memory_backend_pagesize() helper d70b1b2b92 Make qemu_mempath_getpagesize() accept NULL === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-4be3mrc1/src/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' BUILD fedora make[1]: Entering directory '/var/tmp/patchew-tester-tmp-4be3mrc1/src' GEN /var/tmp/patchew-tester-tmp-4be3mrc1/src/docker-src.2018-04-04-22.35.38.5043/qemu.tar Cloning into '/var/tmp/patchew-tester-tmp-4be3mrc1/src/docker-src.2018-04-04-22.35.38.5043/qemu.tar.vroot'... done. Your branch is up-to-date with 'origin/test'. Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-4be3mrc1/src/docker-src.2018-04-04-22.35.38.5043/qemu.tar.vroot/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-4be3mrc1/src/docker-src.2018-04-04-22.35.38.5043/qemu.tar.vroot/ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' COPYRUNNER RUN test-mingw in qemu:fedora Packages installed: PyYAML-3.12-5.fc27.x86_64 SDL-devel-1.2.15-29.fc27.x86_64 bc-1.07.1-3.fc27.x86_64 bison-3.0.4-8.fc27.x86_64 bzip2-1.0.6-24.fc27.x86_64 ccache-3.3.6-1.fc27.x86_64 clang-5.0.1-3.fc27.x86_64 findutils-4.6.0-16.fc27.x86_64 flex-2.6.1-5.fc27.x86_64 gcc-7.3.1-5.fc27.x86_64 gcc-c++-7.3.1-5.fc27.x86_64 gettext-0.19.8.1-12.fc27.x86_64 git-2.14.3-3.fc27.x86_64 glib2-devel-2.54.3-2.fc27.x86_64 hostname-3.18-4.fc27.x86_64 libaio-devel-0.3.110-9.fc27.x86_64 libasan-7.3.1-5.fc27.x86_64 libfdt-devel-1.4.6-1.fc27.x86_64 libubsan-7.3.1-5.fc27.x86_64 llvm-5.0.1-3.fc27.x86_64 make-4.2.1-4.fc27.x86_64 mingw32-SDL-1.2.15-9.fc27.noarch mingw32-bzip2-1.0.6-9.fc27.noarch mingw32-curl-7.54.1-2.fc27.noarch mingw32-glib2-2.54.1-1.fc27.noarch mingw32-gmp-6.1.2-2.fc27.noarch mingw32-gnutls-3.5.13-2.fc27.noarch mingw32-gtk2-2.24.31-4.fc27.noarch mingw32-gtk3-3.22.16-1.fc27.noarch mingw32-libjpeg-turbo-1.5.1-3.fc27.noarch mingw32-libpng-1.6.29-2.fc27.noarch mingw32-libssh2-1.8.0-3.fc27.noarch mingw32-libtasn1-4.13-1.fc27.noarch mingw32-nettle-3.3-3.fc27.noarch mingw32-pixman-0.34.0-3.fc27.noarch mingw32-pkg-config-0.28-9.fc27.x86_64 mingw64-SDL-1.2.15-9.fc27.noarch mingw64-bzip2-1.0.6-9.fc27.noarch mingw64-curl-7.54.1-2.fc27.noarch mingw64-glib2-2.54.1-1.fc27.noarch mingw64-gmp-6.1.2-2.fc27.noarch mingw64-gnutls-3.5.13-2.fc27.noarch mingw64-gtk2-2.24.31-4.fc27.noarch mingw64-gtk3-3.22.16-1.fc27.noarch mingw64-libjpeg-turbo-1.5.1-3.fc27.noarch mingw64-libpng-1.6.29-2.fc27.noarch mingw64-libssh2-1.8.0-3.fc27.noarch mingw64-libtasn1-4.13-1.fc27.noarch mingw64-nettle-3.3-3.fc27.noarch mingw64-pixman-0.34.0-3.fc27.noarch mingw64-pkg-config-0.28-9.fc27.x86_64 nettle-devel-3.4-1.fc27.x86_64 perl-5.26.1-403.fc27.x86_64 pixman-devel-0.34.0-4.fc27.x86_64 python3-3.6.2-13.fc27.x86_64 sparse-0.5.1-2.fc27.x86_64 tar-1.29-7.fc27.x86_64 which-2.21-4.fc27.x86_64 zlib-devel-1.2.11-4.fc27.x86_64 Environment variables: TARGET_LIST= PACKAGES=ccache gettext git tar PyYAML sparse flex bison python3 bzip2 hostname glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel gcc gcc-c++ llvm clang make perl which bc findutils libaio-devel nettle-devel libasan libubsan mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 mingw32-bzip2 mingw64-pixman mingw64-glib2 mingw64-gmp mingw64-SDL mingw64-pkg-config mingw64-gtk2 mingw64-gtk3 mingw64-gnutls mingw64-nettle mingw64-libtasn1 mingw64-libjpeg-turbo mingw64-libpng mingw64-curl mingw64-libssh2 mingw64-bzip2 J=8 V= HOSTNAME=8fd406fb9298 DEBUG= SHOW_ENV=1 PWD=/ HOME=/root CCACHE_DIR=/var/tmp/ccache DISTTAG=f27container QEMU_CONFIGURE_OPTS=--python=/usr/bin/python3 FGC=f27 TEST_DIR=/tmp/qemu-test SHLVL=1 FEATURES=mingw clang pyyaml asan dtc PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin MAKE
Re: [Qemu-devel] [PATCHv2 for-2.13 0/2] Helpers to obtain host page sizes for guest RAM
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20180405022002.17809-1-da...@gibson.dropbear.id.au Subject: [Qemu-devel] [PATCHv2 for-2.13 0/2] Helpers to obtain host page sizes for guest RAM === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20180405022002.17809-1-da...@gibson.dropbear.id.au -> patchew/20180405022002.17809-1-da...@gibson.dropbear.id.au Switched to a new branch 'test' 3a2019a8ad Add host_memory_backend_pagesize() helper d70b1b2b92 Make qemu_mempath_getpagesize() accept NULL === OUTPUT BEGIN === Checking PATCH 1/2: Make qemu_mempath_getpagesize() accept NULL... ERROR: braces {} are necessary for all arms of this statement #96: FILE: util/mmap-alloc.c:56: +} while (ret != 0 && errno == EINTR); [...] total: 1 errors, 0 warnings, 82 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 2/2: Add host_memory_backend_pagesize() helper... === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [PATCH qemu] spapr: Initialize reserved areas list in FDT in H_CAS handler
On Thu, Apr 05, 2018 at 12:07:38PM +1000, Alexey Kardashevskiy wrote: > At the moment the device tree produced by the H_CAS handler has no > reserved map initialized at all which is not correct as at least one > empty record is required to be present as a marker of the end. > This does not cause problems now as the only consumer is SLOF which > does not look at the reserved map area. > > However when DTC's "Improve libfdt's memory safety" changeset hits > the QEMU upstream, there will be errors reported and crashes observed. > > This fixes the problem by adding an empty entry to the reserved map, > just like create_device_tree() does already. > > Signed-off-by: Alexey Kardashevskiy Applied to ppc-for-2.12, thanks. > --- > hw/ppc/spapr.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 2c0be8c..a81570e 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -865,6 +865,7 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr, > /* Create skeleton */ > fdt_skel = g_malloc0(size); > _FDT((fdt_create(fdt_skel, size))); > +_FDT((fdt_finish_reservemap(fdt_skel))); > _FDT((fdt_begin_node(fdt_skel, ""))); > _FDT((fdt_end_node(fdt_skel))); > _FDT((fdt_finish(fdt_skel))); -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[Qemu-devel] [PATCHv2 for-2.13 2/2] Add host_memory_backend_pagesize() helper
There are a couple places (one generic, one target specific) where we need to get the host page size associated with a particular memory backend. I have some upcoming code which will add another place which wants this. So, for convenience, add a helper function to calculate this. host_memory_backend_pagesize() returns the host pagesize for a given HostMemoryBackend object. Signed-off-by: David Gibson --- backends/hostmem.c | 11 +++ exec.c | 5 ++--- include/sysemu/hostmem.h | 2 ++ target/ppc/kvm.c | 6 +- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/backends/hostmem.c b/backends/hostmem.c index f61093654e..ff2bb0489c 100644 --- a/backends/hostmem.c +++ b/backends/hostmem.c @@ -18,6 +18,7 @@ #include "qapi/visitor.h" #include "qemu/config-file.h" #include "qom/object_interfaces.h" +#include "qemu/mmap-alloc.h" #ifdef CONFIG_NUMA #include @@ -262,6 +263,16 @@ bool host_memory_backend_is_mapped(HostMemoryBackend *backend) return backend->is_mapped; } +size_t host_memory_backend_pagesize(HostMemoryBackend *memdev) +{ +Object *obj = OBJECT(memdev); +char *path = object_property_get_str(obj, "mem-path", NULL); +size_t pagesize = qemu_mempath_getpagesize(path); + +g_free(path); +return pagesize; +} + static void host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) { diff --git a/exec.c b/exec.c index b38b004563..c7fcefa851 100644 --- a/exec.c +++ b/exec.c @@ -1491,9 +1491,8 @@ static int find_max_supported_pagesize(Object *obj, void *opaque) long *hpsize_min = opaque; if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) { -char *mem_path = object_property_get_str(obj, "mem-path", NULL); -long hpsize = qemu_mempath_getpagesize(mem_path); -g_free(mem_path); +long hpsize = host_memory_backend_pagesize(MEMORY_BACKEND(obj)); + if (hpsize < *hpsize_min) { *hpsize_min = hpsize; } diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h index 47bc9846ac..bc36899bb8 100644 --- a/include/sysemu/hostmem.h +++ b/include/sysemu/hostmem.h @@ -68,4 +68,6 @@ MemoryRegion *host_memory_backend_get_memory(HostMemoryBackend *backend, void host_memory_backend_set_mapped(HostMemoryBackend *backend, bool mapped); bool host_memory_backend_is_mapped(HostMemoryBackend *backend); +size_t host_memory_backend_pagesize(HostMemoryBackend *memdev); + #endif diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index f393eae127..1ab9cf3a8a 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -493,11 +493,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) { Object *mem_obj = object_resolve_path(obj_path, NULL); -char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); -long pagesize; - -pagesize = qemu_mempath_getpagesize(mempath); -g_free(mempath); +long pagesize = host_memory_backend_pagesize(MEMORY_BACKEND(mem_obj)); return pagesize >= max_cpu_page_size; } -- 2.14.3
[Qemu-devel] [PATCH for-2.13 13/13] target/ppc: Fold slb_nr into PPCHash64Options
The env->slb_nr field gives the size of the SLB (Segment Lookaside Buffer). This is another static-after-initialization parameter of the specific version of the 64-bit hash MMU in the CPU. So, this patch folds the field into PPCHash64Options with the other hash MMU options. This is a bit more complicated that the things previously put in there, because slb_nr was foolishly included in the migration stream. So we need some of the usual dance to handle backwards compatible migration. Signed-off-by: David Gibson --- hw/ppc/pnv.c| 2 +- hw/ppc/spapr.c | 11 --- target/ppc/cpu.h| 3 ++- target/ppc/kvm.c| 2 +- target/ppc/machine.c| 23 --- target/ppc/mmu-hash64.c | 15 +-- target/ppc/mmu-hash64.h | 1 + target/ppc/translate_init.c | 15 --- 8 files changed, 42 insertions(+), 30 deletions(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 5905be3f71..53f672afa8 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -180,7 +180,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt) _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq))); _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq))); -_FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr))); +_FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size))); _FDT((fdt_setprop_string(fdt, offset, "status", "okay"))); _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0))); diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 60bc8417b6..6021631722 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -547,8 +547,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, _FDT((fdt_setprop_cell(fdt, offset, "timebase-frequency", tbfreq))); _FDT((fdt_setprop_cell(fdt, offset, "clock-frequency", cpufreq))); -_FDT((fdt_setprop_cell(fdt, offset, "slb-size", env->slb_nr))); -_FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", env->slb_nr))); +_FDT((fdt_setprop_cell(fdt, offset, "slb-size", cpu->hash64_opts->slb_size))); +_FDT((fdt_setprop_cell(fdt, offset, "ibm,slb-size", cpu->hash64_opts->slb_size))); _FDT((fdt_setprop_string(fdt, offset, "status", "okay"))); _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0))); @@ -4000,7 +4000,12 @@ DEFINE_SPAPR_MACHINE(2_13, "2.13", true); * pseries-2.12 */ #define SPAPR_COMPAT_2_12 \ -HW_COMPAT_2_12 +HW_COMPAT_2_12 \ +{ \ +.driver = TYPE_POWERPC_CPU,\ +.property = "pre-2.13-migration", \ +.value= "on", \ +}, static void spapr_machine_2_12_instance_options(MachineState *machine) { diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index c0c44fb91d..8c9e03f54d 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1025,7 +1025,6 @@ struct CPUPPCState { #if defined(TARGET_PPC64) /* PowerPC 64 SLB area */ ppc_slb_t slb[MAX_SLB_ENTRIES]; -int32_t slb_nr; /* tcg TLB needs flush (deferred slb inval instruction typically) */ #endif /* segment registers */ @@ -1216,6 +1215,8 @@ struct PowerPCCPU { uint64_t mig_insns_flags2; uint32_t mig_nb_BATs; bool pre_2_10_migration; +bool pre_2_13_migration; +int32_t mig_slb_nr; }; static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index b329cd8173..1bd38c6a90 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -484,7 +484,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) break; } } -env->slb_nr = smmu_info.slb_size; +cpu->hash64_opts->slb_size = smmu_info.slb_size; if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) { cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG; } diff --git a/target/ppc/machine.c b/target/ppc/machine.c index 0634cdb295..3d6434a006 100644 --- a/target/ppc/machine.c +++ b/target/ppc/machine.c @@ -18,6 +18,9 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id) unsigned int i, j; target_ulong sdr1; uint32_t fpscr; +#if defined(TARGET_PPC64) +int32_t slb_nr; +#endif target_ulong xer; for (i = 0; i < 32; i++) @@ -49,7 +52,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id) qemu_get_sbe32s(f, &env->access_type); #if defined(TARGET_PPC64) qemu_get_betls(f, &env->spr[SPR_ASR]); -qemu_get_sbe32s(f, &env->slb_nr); +qemu_get_sbe32s(f, &slb_nr); #endif qemu_get_betls(f, &sdr1); for (i = 0; i < 32; i++) @@ -146,6 +149,15 @@ static bool cpu_pre_2_8_migration(void *opaque, int version_id) return cpu->pre_2_8_migration; } +#if defined(T
Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
On Thu, 5 Apr 2018 10:53:38 +1000 Alexey Kardashevskiy wrote: > On 5/4/18 9:09 am, Alex Williamson wrote: > > On Wed, 4 Apr 2018 22:30:50 +0200 > > Eric Auger wrote: > > > >> The 567b5b309abe ("vfio/pci: Relax DMA map errors for MMIO regions") > >> added an error message if a passed memory section address or size > >> is not aligned to the page size and thus cannot be DMA mapped. > >> > >> This patch fixes the trace by printing the region name and the > >> memory region section offset within the address space (instead of > >> offset_within_region). > >> > >> We also turn the error_report into a trace event. Indeed, In some > >> cases, the traces can be confusing to non expert end-users and > >> let think the use case does not work (whereas it > >> works as before). > >> > >> This is the case where a BAR is successively mapped at different > >> GPAs and its sections are not compatible with dma map. The listener > >> is called several times and traces are issued for each intermediate > >> mapping. The end-user cannot easily match those GPAs against the > >> final GPA output by lscpi. So let's keep those information to > >> informed users. In mid term, the plan is to advise the user about > >> BAR relocation relevance. > >> > >> Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions" > >> Signed-off-by: Alexey Kardashevskiy > >> Signed-off-by: Eric Auger > >> > >> --- > >> > >> v1 -> v2: > >> - use a trace point > >> - add some details in the commit message > > > > I think this is v2 with v1 being > > Message-Id:<20180322081837.21460-1-...@ozlabs.ru> but this is > > substantially different from Alexey's posting so I'd like to verify the > > Sign-off. Alexey? > > My understanding it is not "sign-off" any more (I am fine either way, it > just does not seem fully correct), but > > Reviewed-by: Alexey Kardashevskiy Yep, I think Eric was intending to give you credit since this patch builds on your patch. I'll include the R-b. There's still work to do, I'd like to see a single, concise message as we're parsing the device, but there's no benefit in alarming users when nothing has changed and it's too late into the release cycle to do more than just silence this for now. Patches welcome towards a better solution, to be considered for 2.13. Thanks, Alex > > > > I agree that this seems to be the right thing to do for 2.12, nothing > > has changed for these mappings and the error reports are difficult even > > for experts to decipher and explain to users. Thanks, > > > > Alex > > > >> --- > >> hw/vfio/common.c | 11 +-- > >> hw/vfio/trace-events | 1 + > >> 2 files changed, 6 insertions(+), 6 deletions(-) > >> > >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c > >> index 5e84716..07ffa0b 100644 > >> --- a/hw/vfio/common.c > >> +++ b/hw/vfio/common.c > >> @@ -548,12 +548,11 @@ static void vfio_listener_region_add(MemoryListener > >> *listener, > >> hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1; > >> > >> if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) { > >> -error_report("Region 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx > >> - " is not aligned to 0x%"HWADDR_PRIx > >> - " and cannot be mapped for DMA", > >> - section->offset_within_region, > >> - int128_getlo(section->size), > >> - pgmask + 1); > >> +trace_vfio_listener_region_add_no_dma_map( > >> +memory_region_name(section->mr), > >> +section->offset_within_address_space, > >> +int128_getlo(section->size), > >> +pgmask + 1); > >> return; > >> } > >> } > >> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events > >> index 79f63a2..20109cb 100644 > >> --- a/hw/vfio/trace-events > >> +++ b/hw/vfio/trace-events > >> @@ -90,6 +90,7 @@ vfio_iommu_map_notify(const char *op, uint64_t > >> iova_start, uint64_t iova_end) "i > >> vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING > >> region_add 0x%"PRIx64" - 0x%"PRIx64 > >> vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add > >> [iommu] 0x%"PRIx64" - 0x%"PRIx64 > >> vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void > >> *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" > >> +vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, > >> uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" > >> size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for > >> DMA" > >> vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING > >> region_del 0x%"PRIx64" - 0x%"PRIx64 > >> vfio_listener_region_del(uint64_t start, uint64_t end) "region_del > >> 0x%"PRIx64" - 0x%"PRIx64 > >> vfio_disconnect_container(int fd) "close container->fd=%d" > > > >
[Qemu-devel] [PATCH for-2.13 09/13] target/ppc: Move 1T segment and AMR options to PPCHash64Options
Currently env->mmu_model is a bit of an unholy mess of an enum of distinct MMU types, with various flag bits as well. This makes which bits of the field should be compared pretty confusing. Make a start on cleaning that up by moving two of the flags bits - POWERPC_MMU_1TSEG and POWERPC_MMU_AMR - which are specific to the 64-bit hash MMU into a new flags field in PPCHash64Options structure. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/ppc/pnv.c| 3 ++- hw/ppc/spapr.c | 2 +- target/ppc/cpu-qom.h| 11 +++ target/ppc/kvm.c| 4 ++-- target/ppc/mmu-hash64.c | 6 -- target/ppc/mmu-hash64.h | 8 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 5a79b24828..5905be3f71 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -36,6 +36,7 @@ #include "monitor/monitor.h" #include "hw/intc/intc.h" #include "hw/ipmi/ipmi.h" +#include "target/ppc/mmu-hash64.h" #include "hw/ppc/xics.h" #include "hw/ppc/pnv_xscom.h" @@ -187,7 +188,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt) _FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0))); } -if (env->mmu_model & POWERPC_MMU_1TSEG) { +if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) { _FDT((fdt_setprop(fdt, offset, "ibm,processor-segment-sizes", segs, sizeof(segs; } diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 14c31f82fa..f86cb09080 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -557,7 +557,7 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, _FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0))); } -if (env->mmu_model & POWERPC_MMU_1TSEG) { +if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) { _FDT((fdt_setprop(fdt, offset, "ibm,processor-segment-sizes", segs, sizeof(segs; } diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index 3e5ef7375f..2bd58b2a84 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -68,22 +68,17 @@ enum powerpc_mmu_t { /* PowerPC 601 MMU model (specific BATs format)*/ POWERPC_MMU_601= 0x000A, #define POWERPC_MMU_64 0x0001 -#define POWERPC_MMU_1TSEG0x0002 -#define POWERPC_MMU_AMR 0x0004 #define POWERPC_MMU_V3 0x0010 /* ISA V3.00 MMU Support */ /* 64 bits PowerPC MMU */ POWERPC_MMU_64B= POWERPC_MMU_64 | 0x0001, /* Architecture 2.03 and later (has LPCR) */ POWERPC_MMU_2_03 = POWERPC_MMU_64 | 0x0002, /* Architecture 2.06 variant */ -POWERPC_MMU_2_06 = POWERPC_MMU_64 | POWERPC_MMU_1TSEG - | POWERPC_MMU_AMR | 0x0003, +POWERPC_MMU_2_06 = POWERPC_MMU_64 | 0x0003, /* Architecture 2.07 variant */ -POWERPC_MMU_2_07 = POWERPC_MMU_64 | POWERPC_MMU_1TSEG - | POWERPC_MMU_AMR | 0x0004, +POWERPC_MMU_2_07 = POWERPC_MMU_64 | 0x0004, /* Architecture 3.00 variant */ -POWERPC_MMU_3_00 = POWERPC_MMU_64 | POWERPC_MMU_1TSEG - | POWERPC_MMU_AMR | POWERPC_MMU_V3 +POWERPC_MMU_3_00 = POWERPC_MMU_64 | POWERPC_MMU_V3 | 0x0005, }; #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0x)) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index bc6d0a8314..22487cef06 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -302,7 +302,7 @@ static void kvm_get_fallback_smmu_info(PowerPCCPU *cpu, /* HV KVM has backing store size restrictions */ info->flags = KVM_PPC_PAGE_SIZES_REAL; -if (env->mmu_model & POWERPC_MMU_1TSEG) { +if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) { info->flags |= KVM_PPC_1T_SEGMENTS; } @@ -482,7 +482,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) } env->slb_nr = smmu_info.slb_size; if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) { -env->mmu_model &= ~POWERPC_MMU_1TSEG; +cpu->hash64_opts->flags &= ~PPC_HASH64_1TSEG; } } diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index 2809c31170..c9ee55e1ea 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -160,7 +160,7 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot, if (vsid & (SLB_VSID_B & ~SLB_VSID_B_1T)) { return -1; /* Bad segment size */ } -if ((vsid & SLB_VSID_B) && !(env->mmu_model & POWERPC_MMU_1TSEG)) { +if ((vsid & SLB_VSID_B) && !(ppc_hash64_has(cpu, PPC_HASH64_1TSEG))) { return -1; /* 1T segment on MMU that doesn't support it */ } @@ -369,7 +369,7 @@ static int ppc_hash64_amr_prot(PowerPCCPU *cpu, ppc_hash_pte64_t pte) int prot = PAGE_
[Qemu-devel] [PATCHv2 for-2.13 0/2] Helpers to obtain host page sizes for guest RAM
This series makes some small changes to make it easier to obtain the host page size backing given portions of guest RAM. We use this in a couple of places currently, and I have one or two more to add in some upcoming code. Changes since v1: * Reworked design based on Eduardo's feedback * Split into two patches David Gibson (2): Make qemu_mempath_getpagesize() accept NULL Add host_memory_backend_pagesize() helper backends/hostmem.c | 11 +++ exec.c | 20 +--- include/sysemu/hostmem.h | 2 ++ target/ppc/kvm.c | 10 +- util/mmap-alloc.c| 26 ++ 5 files changed, 33 insertions(+), 36 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH for-2.13 12/13] target/ppc: Get rid of POWERPC_MMU_VER() macros
These macros were introduced to deal with the fact that the mmu_model field has bit flags mixed in with what's otherwise an enum of various mmu types. We've now eliminated all those flags except for one, and that one - POWERPC_MMU_64 - is already included/compared in the MMU_VER macros. So, we can get rid of those macros and just directly compare mmu_model values in the places it was used. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- target/ppc/cpu-qom.h| 6 -- target/ppc/kvm.c| 8 target/ppc/mmu-hash64.c | 12 ++-- target/ppc/mmu_helper.c | 24 target/ppc/translate.c | 12 ++-- 5 files changed, 28 insertions(+), 34 deletions(-) diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index ef96d42cf2..433a71e484 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -79,12 +79,6 @@ enum powerpc_mmu_t { /* Architecture 3.00 variant */ POWERPC_MMU_3_00 = POWERPC_MMU_64 | 0x0005, }; -#define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0x)) -#define POWERPC_MMU_VER_64B POWERPC_MMU_VER(POWERPC_MMU_64B) -#define POWERPC_MMU_VER_2_03 POWERPC_MMU_VER(POWERPC_MMU_2_03) -#define POWERPC_MMU_VER_2_06 POWERPC_MMU_VER(POWERPC_MMU_2_06) -#define POWERPC_MMU_VER_2_07 POWERPC_MMU_VER(POWERPC_MMU_2_07) -#define POWERPC_MMU_VER_3_00 POWERPC_MMU_VER(POWERPC_MMU_3_00) /*/ /* Exception model */ diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index fff2c601e0..b329cd8173 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -306,8 +306,8 @@ static void kvm_get_fallback_smmu_info(PowerPCCPU *cpu, info->flags |= KVM_PPC_1T_SEGMENTS; } -if (POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_06 || - POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_07) { +if (env->mmu_model == POWERPC_MMU_2_06 || +env->mmu_model == POWERPC_MMU_2_07) { info->slb_size = 32; } else { info->slb_size = 64; @@ -321,8 +321,8 @@ static void kvm_get_fallback_smmu_info(PowerPCCPU *cpu, i++; /* 64K on MMU 2.06 and later */ -if (POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_06 || -POWERPC_MMU_VER(env->mmu_model) == POWERPC_MMU_VER_2_07) { +if (env->mmu_model == POWERPC_MMU_2_06 || +env->mmu_model == POWERPC_MMU_2_07) { info->sps[i].page_shift = 16; info->sps[i].slb_enc = 0x110; info->sps[i].enc[0].page_shift = 16; diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index df26a03c15..a5570c8774 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -1033,8 +1033,8 @@ void helper_store_lpcr(CPUPPCState *env, target_ulong val) uint64_t lpcr = 0; /* Filter out bits */ -switch (POWERPC_MMU_VER(env->mmu_model)) { -case POWERPC_MMU_VER_64B: /* 970 */ +switch (env->mmu_model) { +case POWERPC_MMU_64B: /* 970 */ if (val & 0x40) { lpcr |= LPCR_LPES0; } @@ -1060,26 +1060,26 @@ void helper_store_lpcr(CPUPPCState *env, target_ulong val) * to dig HRMOR out of HID5 */ break; -case POWERPC_MMU_VER_2_03: /* P5p */ +case POWERPC_MMU_2_03: /* P5p */ lpcr = val & (LPCR_RMLS | LPCR_ILE | LPCR_LPES0 | LPCR_LPES1 | LPCR_RMI | LPCR_HDICE); break; -case POWERPC_MMU_VER_2_06: /* P7 */ +case POWERPC_MMU_2_06: /* P7 */ lpcr = val & (LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_DPFD | LPCR_VRMASD | LPCR_RMLS | LPCR_ILE | LPCR_P7_PECE0 | LPCR_P7_PECE1 | LPCR_P7_PECE2 | LPCR_MER | LPCR_TC | LPCR_LPES0 | LPCR_LPES1 | LPCR_HDICE); break; -case POWERPC_MMU_VER_2_07: /* P8 */ +case POWERPC_MMU_2_07: /* P8 */ lpcr = val & (LPCR_VPM0 | LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | LPCR_VRMASD | LPCR_RMLS | LPCR_ILE | LPCR_AIL | LPCR_ONL | LPCR_P8_PECE0 | LPCR_P8_PECE1 | LPCR_P8_PECE2 | LPCR_P8_PECE3 | LPCR_P8_PECE4 | LPCR_MER | LPCR_TC | LPCR_LPES0 | LPCR_HDICE); break; -case POWERPC_MMU_VER_3_00: /* P9 */ +case POWERPC_MMU_3_00: /* P9 */ lpcr = val & (LPCR_VPM1 | LPCR_ISL | LPCR_KBV | LPCR_DPFD | (LPCR_PECE_U_MASK & LPCR_HVEE) | LPCR_ILE | LPCR_AIL | LPCR_UPRT | LPCR_EVIRT | LPCR_ONL | diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c index 5568d1642b..8075b7149a 100644 --- a/target/ppc/mmu_helper.c +++ b/target/ppc/mmu_helper.c @@ -1266,7 +1266,7 @@ static void mmu6xx_dump_mmu(FILE *f, fpri
[Qemu-devel] [PATCH for-2.13 05/13] target/ppc: Remove fallback 64k pagesize information
CPU definitions for cpus with the 64-bit hash MMU can include a table of available pagesizes. If this isn't supplied ppc_cpu_instance_init() will fill it in a fallback table based on the POWERPC_MMU_64K bit in mmu_model. However, it turns out all the cpus which support 64K pages already include an explicit table of page sizes, so there's no point to the fallback table including 64k pages. That removes the only place which tests POWERPC_MMU_64K, so we can remove it. Which in turn allows some logic to be removed from kvm_fixup_page_sizes(). Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- target/ppc/cpu-qom.h| 4 target/ppc/kvm.c| 7 --- target/ppc/translate_init.c | 20 ++-- 3 files changed, 2 insertions(+), 29 deletions(-) diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index deaa46a14b..9bbb05cf62 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -70,7 +70,6 @@ enum powerpc_mmu_t { #define POWERPC_MMU_64 0x0001 #define POWERPC_MMU_1TSEG0x0002 #define POWERPC_MMU_AMR 0x0004 -#define POWERPC_MMU_64K 0x0008 #define POWERPC_MMU_V3 0x0010 /* ISA V3.00 MMU Support */ /* 64 bits PowerPC MMU */ POWERPC_MMU_64B= POWERPC_MMU_64 | 0x0001, @@ -78,15 +77,12 @@ enum powerpc_mmu_t { POWERPC_MMU_2_03 = POWERPC_MMU_64 | 0x0002, /* Architecture 2.06 variant */ POWERPC_MMU_2_06 = POWERPC_MMU_64 | POWERPC_MMU_1TSEG - | POWERPC_MMU_64K | POWERPC_MMU_AMR | 0x0003, /* Architecture 2.07 variant */ POWERPC_MMU_2_07 = POWERPC_MMU_64 | POWERPC_MMU_1TSEG - | POWERPC_MMU_64K | POWERPC_MMU_AMR | 0x0004, /* Architecture 3.00 variant */ POWERPC_MMU_3_00 = POWERPC_MMU_64 | POWERPC_MMU_1TSEG - | POWERPC_MMU_64K | POWERPC_MMU_AMR | POWERPC_MMU_V3 | 0x0005, }; diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 79a436a384..6160356a4a 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -425,7 +425,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) static bool has_smmu_info; CPUPPCState *env = &cpu->env; int iq, ik, jq, jk; -bool has_64k_pages = false; /* We only handle page sizes for 64-bit server guests for now */ if (!(env->mmu_model & POWERPC_MMU_64)) { @@ -471,9 +470,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) ksps->enc[jk].page_shift)) { continue; } -if (ksps->enc[jk].page_shift == 16) { -has_64k_pages = true; -} qsps->enc[jq].page_shift = ksps->enc[jk].page_shift; qsps->enc[jq].pte_enc = ksps->enc[jk].pte_enc; if (++jq >= PPC_PAGE_SIZES_MAX_SZ) { @@ -488,9 +484,6 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) if (!(smmu_info.flags & KVM_PPC_1T_SEGMENTS)) { env->mmu_model &= ~POWERPC_MMU_1TSEG; } -if (!has_64k_pages) { -env->mmu_model &= ~POWERPC_MMU_64K; -} } bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c index 29bd6f3654..99be6fcd68 100644 --- a/target/ppc/translate_init.c +++ b/target/ppc/translate_init.c @@ -10469,7 +10469,7 @@ static void ppc_cpu_instance_init(Object *obj) env->sps = *pcc->sps; } else if (env->mmu_model & POWERPC_MMU_64) { /* Use default sets of page sizes. We don't support MPSS */ -static const struct ppc_segment_page_sizes defsps_4k = { +static const struct ppc_segment_page_sizes defsps = { .sps = { { .page_shift = 12, /* 4K */ .slb_enc = 0, @@ -10481,23 +10481,7 @@ static void ppc_cpu_instance_init(Object *obj) }, }, }; -static const struct ppc_segment_page_sizes defsps_64k = { -.sps = { -{ .page_shift = 12, /* 4K */ - .slb_enc = 0, - .enc = { { .page_shift = 12, .pte_enc = 0 } } -}, -{ .page_shift = 16, /* 64K */ - .slb_enc = 0x110, - .enc = { { .page_shift = 16, .pte_enc = 1 } } -}, -{ .page_shift = 24, /* 16M */ - .slb_enc = 0x100, - .enc = { { .page_shift = 24, .pte_enc = 0 } } -}, -}, -}; -env->sps = (env->mmu_model & POWERPC_MMU_64K) ? defsps_64k : defsps_4k; +env->sps = defsps; } #endif /* defined(TARGET_PPC64) */ } -
[Qemu-devel] [PATCH for-2.13 04/13] target/ppc: Avoid taking "env" parameter to mmu-hash64 functions
In most cases we prefer to pass a PowerPCCPU rather than the (embedded) CPUPPCState. For ppc_hash64_update_{rmls,vrma}() change to take "cpu" instead of "env". For ppc_hash64_set_{dsi,isi}() remove the redundant "env" parameter. In theory this makes more work for the functions, but since "cs", "cpu" and "env" are related by at most constant offsets, the compiler should be able to optimize out the difference at effectively zero cost. helper_*() functions are left alone - since they're more closely tied to the TCG generated code, passing "env" is still the standard there. While we're there, fix an incorrect indentation. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- target/ppc/mmu-hash64.c | 35 +++ target/ppc/mmu-hash64.h | 4 ++-- target/ppc/translate_init.c | 4 ++-- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index c9b72b7429..a87fa7c83f 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -633,9 +633,9 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu, return 0; } -static void ppc_hash64_set_isi(CPUState *cs, CPUPPCState *env, - uint64_t error_code) +static void ppc_hash64_set_isi(CPUState *cs, uint64_t error_code) { +CPUPPCState *env = &POWERPC_CPU(cs)->env; bool vpm; if (msr_ir) { @@ -659,9 +659,9 @@ static void ppc_hash64_set_isi(CPUState *cs, CPUPPCState *env, env->error_code = error_code; } -static void ppc_hash64_set_dsi(CPUState *cs, CPUPPCState *env, uint64_t dar, - uint64_t dsisr) +static void ppc_hash64_set_dsi(CPUState *cs, uint64_t dar, uint64_t dsisr) { +CPUPPCState *env = &POWERPC_CPU(cs)->env; bool vpm; if (msr_dr) { @@ -741,13 +741,13 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, } else { /* The access failed, generate the approriate interrupt */ if (rwx == 2) { -ppc_hash64_set_isi(cs, env, SRR1_PROTFAULT); +ppc_hash64_set_isi(cs, SRR1_PROTFAULT); } else { int dsisr = DSISR_PROTFAULT; if (rwx == 1) { dsisr |= DSISR_ISSTORE; } -ppc_hash64_set_dsi(cs, env, eaddr, dsisr); +ppc_hash64_set_dsi(cs, eaddr, dsisr); } return 1; } @@ -783,7 +783,7 @@ skip_slb_search: /* 3. Check for segment level no-execute violation */ if ((rwx == 2) && (slb->vsid & SLB_VSID_N)) { -ppc_hash64_set_isi(cs, env, SRR1_NOEXEC_GUARD); +ppc_hash64_set_isi(cs, SRR1_NOEXEC_GUARD); return 1; } @@ -791,13 +791,13 @@ skip_slb_search: ptex = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte, &apshift); if (ptex == -1) { if (rwx == 2) { -ppc_hash64_set_isi(cs, env, SRR1_NOPTE); +ppc_hash64_set_isi(cs, SRR1_NOPTE); } else { int dsisr = DSISR_NOPTE; if (rwx == 1) { dsisr |= DSISR_ISSTORE; } -ppc_hash64_set_dsi(cs, env, eaddr, dsisr); +ppc_hash64_set_dsi(cs, eaddr, dsisr); } return 1; } @@ -824,7 +824,7 @@ skip_slb_search: if (PAGE_EXEC & ~amr_prot) { srr1 |= SRR1_IAMR; /* Access violates virt pg class key prot */ } -ppc_hash64_set_isi(cs, env, srr1); +ppc_hash64_set_isi(cs, srr1); } else { int dsisr = 0; if (need_prot[rwx] & ~pp_prot) { @@ -836,7 +836,7 @@ skip_slb_search: if (need_prot[rwx] & ~amr_prot) { dsisr |= DSISR_AMR; } -ppc_hash64_set_dsi(cs, env, eaddr, dsisr); +ppc_hash64_set_dsi(cs, eaddr, dsisr); } return 1; } @@ -942,8 +942,9 @@ void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex, cpu->env.tlb_need_flush = TLB_NEED_GLOBAL_FLUSH | TLB_NEED_LOCAL_FLUSH; } -void ppc_hash64_update_rmls(CPUPPCState *env) +void ppc_hash64_update_rmls(PowerPCCPU *cpu) { +CPUPPCState *env = &cpu->env; uint64_t lpcr = env->spr[SPR_LPCR]; /* @@ -976,8 +977,9 @@ void ppc_hash64_update_rmls(CPUPPCState *env) } } -void ppc_hash64_update_vrma(CPUPPCState *env) +void ppc_hash64_update_vrma(PowerPCCPU *cpu) { +CPUPPCState *env = &cpu->env; const struct ppc_one_seg_page_size *sps = NULL; target_ulong esid, vsid, lpcr; ppc_slb_t *slb = &env->vrma_slb; @@ -1002,7 +1004,7 @@ void ppc_hash64_update_vrma(CPUPPCState *env) vsid |= (vrmasd << 4) & (SLB_VSID_L | SLB_VSID_LP); esid = SLB_ESID_V; - for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) { +for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) { const stru
[Qemu-devel] [PATCH for-2.13 07/13] target/ppc: Split page size information into a separate allocation
env->sps contains page size encoding information as an embedded structure. Since this information is specific to 64-bit hash MMUs, split it out into a separately allocated structure, to reduce the basic env size for other cpus. Along the way we make a few other cleanups: * Rename to PPCHash64Options which is more in line with qemu name conventions, and reflects that we're going to merge some more hash64 mmu specific details in there in future. Also rename its substructures to match qemu conventions. * Move structure definitions to the mmu-hash64.[ch] files. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/fdt.c| 4 +-- target/ppc/cpu-qom.h| 4 +-- target/ppc/cpu.h| 26 +++ target/ppc/kvm.c| 4 +-- target/ppc/mmu-hash64.c | 61 + target/ppc/mmu-hash64.h | 22 target/ppc/translate_init.c | 36 +++--- 7 files changed, 80 insertions(+), 77 deletions(-) diff --git a/hw/ppc/fdt.c b/hw/ppc/fdt.c index 2721603ffa..0828ad7254 100644 --- a/hw/ppc/fdt.c +++ b/hw/ppc/fdt.c @@ -9,6 +9,7 @@ #include "qemu/osdep.h" #include "target/ppc/cpu.h" +#include "target/ppc/mmu-hash64.h" #include "hw/ppc/fdt.h" @@ -16,13 +17,12 @@ size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop, size_t maxsize) { -CPUPPCState *env = &cpu->env; size_t maxcells = maxsize / sizeof(uint32_t); int i, j, count; uint32_t *p = prop; for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) { -struct ppc_one_seg_page_size *sps = &env->sps.sps[i]; +PPCHash64SegmentPageSizes *sps = &cpu->hash64_opts->sps[i]; if (!sps->page_shift) { break; diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index 9bbb05cf62..3e5ef7375f 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -160,7 +160,7 @@ enum powerpc_input_t { PPC_FLAGS_INPUT_RCPU, }; -struct ppc_segment_page_sizes; +typedef struct PPCHash64Options PPCHash64Options; /** * PowerPCCPUClass: @@ -194,7 +194,7 @@ typedef struct PowerPCCPUClass { uint32_t flags; int bfd_mach; uint32_t l1_dcache_size, l1_icache_size; -const struct ppc_segment_page_sizes *sps; +const PPCHash64Options *hash64_opts; struct ppc_radix_page_info *radix_page_info; void (*init_proc)(CPUPPCState *env); int (*check_pow)(CPUPPCState *env); diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index c621a6bd5e..1c5c33ca11 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -327,11 +327,13 @@ union ppc_tlb_t { #define TLB_MAS3 #endif +typedef struct PPCHash64SegmentPageSizes PPCHash64SegmentPageSizes; + typedef struct ppc_slb_t ppc_slb_t; struct ppc_slb_t { uint64_t esid; uint64_t vsid; -const struct ppc_one_seg_page_size *sps; +const PPCHash64SegmentPageSizes *sps; }; #define MAX_SLB_ENTRIES 64 @@ -948,28 +950,8 @@ enum { #define DBELL_PROCIDTAG_MASK PPC_BITMASK(44, 63) -/*/ -/* Segment page size information, used by recent hash MMUs - * The format of this structure mirrors kvm_ppc_smmu_info - */ - #define PPC_PAGE_SIZES_MAX_SZ 8 -struct ppc_one_page_size { -uint32_t page_shift; /* Page shift (or 0) */ -uint32_t pte_enc; /* Encoding in the HPTE (>>12) */ -}; - -struct ppc_one_seg_page_size { -uint32_t page_shift; /* Base page shift of segment (or 0) */ -uint32_t slb_enc; /* SLB encoding for BookS */ -struct ppc_one_page_size enc[PPC_PAGE_SIZES_MAX_SZ]; -}; - -struct ppc_segment_page_sizes { -struct ppc_one_seg_page_size sps[PPC_PAGE_SIZES_MAX_SZ]; -}; - struct ppc_radix_page_info { uint32_t count; uint32_t entries[PPC_PAGE_SIZES_MAX_SZ]; @@ -1106,7 +1088,6 @@ struct CPUPPCState { uint64_t insns_flags; uint64_t insns_flags2; #if defined(TARGET_PPC64) -struct ppc_segment_page_sizes sps; ppc_slb_t vrma_slb; target_ulong rmls; bool ci_large_pages; @@ -1227,6 +1208,7 @@ struct PowerPCCPU { PPCVirtualHypervisor *vhyp; Object *intc; int32_t node_id; /* NUMA node this CPU belongs to */ +PPCHash64Options *hash64_opts; /* Fields related to migration compatibility hacks */ bool pre_2_8_migration; diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 6160356a4a..bc6d0a8314 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -442,7 +442,7 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) } /* Convert to QEMU form */ -memset(&env->sps, 0, sizeof(env->sps)); +memset(cpu->hash64_opts, 0, sizeof(*cpu->hash64_opts)); /* If we have HV KVM, we need to forbid CI large pages if our * host page size is smaller than 64K. @@ -456,7 +456,7 @@ static void kvm_fixup_page_s
[Qemu-devel] [PATCH for-2.13 01/13] target/ppc: Standardize instance_init and realize function names
Because of the various hooks called some variant on "init" - and the rather greater number that used to exist, I'm always wondering when a function called simply "*_init" or "*_initfn" will be called. To make it easier on myself, and maybe others, rename the instance_init hooks for ppc cpus to *_instance_init(). While we're at it rename the realize time hooks to *_realize() (from *_realizefn()) which seems to be the more common current convention. Signed-off-by: David Gibson Reviewed-by: Greg Kurz --- target/ppc/translate_init.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c index 391b94b97d..56b80a204a 100644 --- a/target/ppc/translate_init.c +++ b/target/ppc/translate_init.c @@ -9726,7 +9726,7 @@ static inline bool ppc_cpu_is_valid(PowerPCCPUClass *pcc) #endif } -static void ppc_cpu_realizefn(DeviceState *dev, Error **errp) +static void ppc_cpu_realize(DeviceState *dev, Error **errp) { CPUState *cs = CPU(dev); PowerPCCPU *cpu = POWERPC_CPU(dev); @@ -9952,7 +9952,7 @@ unrealize: cpu_exec_unrealizefn(cs); } -static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp) +static void ppc_cpu_unrealize(DeviceState *dev, Error **errp) { PowerPCCPU *cpu = POWERPC_CPU(dev); PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); @@ -10438,7 +10438,7 @@ static bool ppc_cpu_is_big_endian(CPUState *cs) } #endif -static void ppc_cpu_initfn(Object *obj) +static void ppc_cpu_instance_init(Object *obj) { CPUState *cs = CPU(obj); PowerPCCPU *cpu = POWERPC_CPU(obj); @@ -10561,9 +10561,9 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data) CPUClass *cc = CPU_CLASS(oc); DeviceClass *dc = DEVICE_CLASS(oc); -device_class_set_parent_realize(dc, ppc_cpu_realizefn, +device_class_set_parent_realize(dc, ppc_cpu_realize, &pcc->parent_realize); -device_class_set_parent_unrealize(dc, ppc_cpu_unrealizefn, +device_class_set_parent_unrealize(dc, ppc_cpu_unrealize, &pcc->parent_unrealize); pcc->pvr_match = ppc_pvr_match_default; pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always; @@ -10623,7 +10623,7 @@ static const TypeInfo ppc_cpu_type_info = { .name = TYPE_POWERPC_CPU, .parent = TYPE_CPU, .instance_size = sizeof(PowerPCCPU), -.instance_init = ppc_cpu_initfn, +.instance_init = ppc_cpu_instance_init, .abstract = true, .class_size = sizeof(PowerPCCPUClass), .class_init = ppc_cpu_class_init, -- 2.14.3
[Qemu-devel] [PATCHv2 for-2.13 1/2] Make qemu_mempath_getpagesize() accept NULL
qemu_mempath_getpagesize() gets the effective (host side) page size for a block of memory backed by an mmap()ed file on the host. It requires the mem_path parameter to be non-NULL. This ends up meaning all the callers need a different case for handling anonymous memory (for memory-backend-ram or default memory with -mem-path is not specified). We can make all those callers a little simpler by qemu_mempath_getpagesize() accept NULL, and treat that as the anonymous memory case. Signed-off-by: David Gibson --- exec.c| 21 ++--- target/ppc/kvm.c | 8 ++-- util/mmap-alloc.c | 26 ++ 3 files changed, 22 insertions(+), 33 deletions(-) diff --git a/exec.c b/exec.c index 02b1efebb7..b38b004563 100644 --- a/exec.c +++ b/exec.c @@ -1488,19 +1488,14 @@ void ram_block_dump(Monitor *mon) */ static int find_max_supported_pagesize(Object *obj, void *opaque) { -char *mem_path; long *hpsize_min = opaque; if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) { -mem_path = object_property_get_str(obj, "mem-path", NULL); -if (mem_path) { -long hpsize = qemu_mempath_getpagesize(mem_path); -g_free(mem_path); -if (hpsize < *hpsize_min) { -*hpsize_min = hpsize; -} -} else { -*hpsize_min = getpagesize(); +char *mem_path = object_property_get_str(obj, "mem-path", NULL); +long hpsize = qemu_mempath_getpagesize(mem_path); +g_free(mem_path); +if (hpsize < *hpsize_min) { +*hpsize_min = hpsize; } } @@ -1513,11 +1508,7 @@ long qemu_getrampagesize(void) long mainrampagesize; Object *memdev_root; -if (mem_path) { -mainrampagesize = qemu_mempath_getpagesize(mem_path); -} else { -mainrampagesize = getpagesize(); -} +mainrampagesize = qemu_mempath_getpagesize(mem_path); /* it's possible we have memory-backend objects with * hugepage-backed RAM. these may get mapped into system diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 1bd38c6a90..f393eae127 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -496,12 +496,8 @@ bool kvmppc_is_mem_backend_page_size_ok(const char *obj_path) char *mempath = object_property_get_str(mem_obj, "mem-path", NULL); long pagesize; -if (mempath) { -pagesize = qemu_mempath_getpagesize(mempath); -g_free(mempath); -} else { -pagesize = getpagesize(); -} +pagesize = qemu_mempath_getpagesize(mempath); +g_free(mempath); return pagesize >= max_cpu_page_size; } diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c index 2fd8cbcc6f..fd329eccd8 100644 --- a/util/mmap-alloc.c +++ b/util/mmap-alloc.c @@ -50,19 +50,21 @@ size_t qemu_mempath_getpagesize(const char *mem_path) struct statfs fs; int ret; -do { -ret = statfs(mem_path, &fs); -} while (ret != 0 && errno == EINTR); - -if (ret != 0) { -fprintf(stderr, "Couldn't statfs() memory path: %s\n", -strerror(errno)); -exit(1); -} +if (mem_path) { +do { +ret = statfs(mem_path, &fs); +} while (ret != 0 && errno == EINTR); -if (fs.f_type == HUGETLBFS_MAGIC) { -/* It's hugepage, return the huge page size */ -return fs.f_bsize; +if (ret != 0) { +fprintf(stderr, "Couldn't statfs() memory path: %s\n", +strerror(errno)); +exit(1); +} + +if (fs.f_type == HUGETLBFS_MAGIC) { +/* It's hugepage, return the huge page size */ +return fs.f_bsize; +} } #ifdef __sparc__ /* SPARC Linux needs greater alignment than the pagesize */ -- 2.14.3
[Qemu-devel] [PATCH for-2.13 00/13] target/ppc: Assorted cpu cleanups (esp. hash64 MMU)
Here's a set of cleanups for the ppc cpu code. Most are related specifically to the 64-bit hash MMU, but there are some others as well. In particular it establishes a new structure PPCHash64Options which contains details of the hash64 mmu which can vary from one cpu to another. This attempts to gather such options in one place, instead of spreading them around various bits of env->mmu_model as well as other fields. Most of these arose while I was looking to improve the way we handle available page sizes for the pseries machine type, although they're mostly not closely tied to that. Changes since RFC: * Added an extra patch folding slb_nr into the new scheme * Assorted minor fixes based on feedback David Gibson (13): target/ppc: Standardize instance_init and realize function names target/ppc: Simplify cpu valid check in ppc_cpu_realize target/ppc: Pass cpu instead of env to ppc_create_page_sizes_prop() target/ppc: Avoid taking "env" parameter to mmu-hash64 functions target/ppc: Remove fallback 64k pagesize information target/ppc: Move page size setup to helper function target/ppc: Split page size information into a separate allocation target/ppc: Make hash64_opts field mandatory for 64-bit hash MMUs target/ppc: Move 1T segment and AMR options to PPCHash64Options target/ppc: Fold ci_large_pages flag into PPCHash64Options target/ppc: Remove unnecessary POWERPC_MMU_V3 flag from mmu_model target/ppc: Get rid of POWERPC_MMU_VER() macros target/ppc: Fold slb_nr into PPCHash64Options hw/ppc/fdt.c| 7 +- hw/ppc/pnv.c| 9 +-- hw/ppc/spapr.c | 20 +++--- include/hw/ppc/fdt.h| 2 +- target/ppc/cpu-qom.h| 27 ++-- target/ppc/cpu.h| 30 ++--- target/ppc/kvm.c| 31 - target/ppc/machine.c| 23 ++- target/ppc/mmu-hash64.c | 152 +++- target/ppc/mmu-hash64.h | 48 +- target/ppc/mmu_helper.c | 24 +++ target/ppc/translate.c | 12 ++-- target/ppc/translate_init.c | 126 +++- 13 files changed, 263 insertions(+), 248 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH for-2.13 08/13] target/ppc: Make hash64_opts field mandatory for 64-bit hash MMUs
Currently some cpus set the hash64_opts field in the class structure, with specific details of their variant of the 64-bit hash mmu. For the remaining cpus with that mmu, ppc_hash64_realize() fills in defaults. But there are only a couple of cpus that use those fallbacks, so just have them to set the has64_opts field instead, simplifying the logic. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- target/ppc/mmu-hash64.c | 36 ++-- target/ppc/mmu-hash64.h | 1 + target/ppc/translate_init.c | 2 ++ 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index 6758afd9de..2809c31170 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -1100,25 +1100,12 @@ void ppc_hash64_init(PowerPCCPU *cpu) CPUPPCState *env = &cpu->env; PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); -if (pcc->hash64_opts) { -cpu->hash64_opts = g_memdup(pcc->hash64_opts, -sizeof(*cpu->hash64_opts)); -} else if (env->mmu_model & POWERPC_MMU_64) { -/* Use default sets of page sizes. We don't support MPSS */ -static const PPCHash64Options defopts = { -.sps = { -{ .page_shift = 12, /* 4K */ - .slb_enc = 0, - .enc = { { .page_shift = 12, .pte_enc = 0 } } -}, -{ .page_shift = 24, /* 16M */ - .slb_enc = 0x100, - .enc = { { .page_shift = 24, .pte_enc = 0 } } -}, -}, -}; -cpu->hash64_opts = g_memdup(&defopts, sizeof(*cpu->hash64_opts)); +if (!pcc->hash64_opts) { +assert(!(env->mmu_model & POWERPC_MMU_64)); +return; } + +cpu->hash64_opts = g_memdup(pcc->hash64_opts, sizeof(*cpu->hash64_opts)); } void ppc_hash64_finalize(PowerPCCPU *cpu) @@ -1126,6 +1113,19 @@ void ppc_hash64_finalize(PowerPCCPU *cpu) g_free(cpu->hash64_opts); } +const PPCHash64Options ppc_hash64_opts_basic = { +.sps = { +{ .page_shift = 12, /* 4K */ + .slb_enc = 0, + .enc = { { .page_shift = 12, .pte_enc = 0 } } +}, +{ .page_shift = 24, /* 16M */ + .slb_enc = 0x100, + .enc = { { .page_shift = 24, .pte_enc = 0 } } +}, +}, +}; + const PPCHash64Options ppc_hash64_opts_POWER7 = { .sps = { { diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h index 957bd68e33..341c1524c2 100644 --- a/target/ppc/mmu-hash64.h +++ b/target/ppc/mmu-hash64.h @@ -156,6 +156,7 @@ struct PPCHash64Options { PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ]; }; +extern const PPCHash64Options ppc_hash64_opts_basic; extern const PPCHash64Options ppc_hash64_opts_POWER7; #endif /* CONFIG_USER_ONLY */ diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c index 040d6fbac3..ae005b2a54 100644 --- a/target/ppc/translate_init.c +++ b/target/ppc/translate_init.c @@ -8242,6 +8242,7 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data) pcc->mmu_model = POWERPC_MMU_64B; #if defined(CONFIG_SOFTMMU) pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault; +pcc->hash64_opts = &ppc_hash64_opts_basic; #endif pcc->excp_model = POWERPC_EXCP_970; pcc->bus_model = PPC_FLAGS_INPUT_970; @@ -8319,6 +8320,7 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data) pcc->mmu_model = POWERPC_MMU_2_03; #if defined(CONFIG_SOFTMMU) pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault; +pcc->hash64_opts = &ppc_hash64_opts_basic; #endif pcc->excp_model = POWERPC_EXCP_970; pcc->bus_model = PPC_FLAGS_INPUT_970; -- 2.14.3
[Qemu-devel] [PATCH for-2.13 03/13] target/ppc: Pass cpu instead of env to ppc_create_page_sizes_prop()
As a rule we prefer to pass PowerPCCPU instead of CPUPPCState, and this change will make some things simpler later on. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Cédric Le Goater --- hw/ppc/fdt.c | 5 +++-- hw/ppc/pnv.c | 4 ++-- hw/ppc/spapr.c | 4 ++-- include/hw/ppc/fdt.h | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/hw/ppc/fdt.c b/hw/ppc/fdt.c index 2ffc5866e4..2721603ffa 100644 --- a/hw/ppc/fdt.c +++ b/hw/ppc/fdt.c @@ -13,9 +13,10 @@ #include "hw/ppc/fdt.h" #if defined(TARGET_PPC64) -size_t ppc_create_page_sizes_prop(CPUPPCState *env, uint32_t *prop, - size_t maxsize) +size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop, + size_t maxsize) { +CPUPPCState *env = &cpu->env; size_t maxcells = maxsize / sizeof(uint32_t); int i, j, count; uint32_t *p = prop; diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index 98ee3c607a..5a79b24828 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -209,8 +209,8 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt) _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1))); } -page_sizes_prop_size = ppc_create_page_sizes_prop(env, page_sizes_prop, - sizeof(page_sizes_prop)); +page_sizes_prop_size = ppc_create_page_sizes_prop(cpu, page_sizes_prop, + sizeof(page_sizes_prop)); if (page_sizes_prop_size) { _FDT((fdt_setprop(fdt, offset, "ibm,segment-page-sizes", page_sizes_prop, page_sizes_prop_size))); diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e764f999c5..14c31f82fa 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -581,8 +581,8 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset, _FDT((fdt_setprop_cell(fdt, offset, "ibm,dfp", 1))); } -page_sizes_prop_size = ppc_create_page_sizes_prop(env, page_sizes_prop, - sizeof(page_sizes_prop)); +page_sizes_prop_size = ppc_create_page_sizes_prop(cpu, page_sizes_prop, + sizeof(page_sizes_prop)); if (page_sizes_prop_size) { _FDT((fdt_setprop(fdt, offset, "ibm,segment-page-sizes", page_sizes_prop, page_sizes_prop_size))); diff --git a/include/hw/ppc/fdt.h b/include/hw/ppc/fdt.h index bd5b0a8c3d..a8cd85069f 100644 --- a/include/hw/ppc/fdt.h +++ b/include/hw/ppc/fdt.h @@ -23,7 +23,7 @@ } \ } while (0) -size_t ppc_create_page_sizes_prop(CPUPPCState *env, uint32_t *prop, +size_t ppc_create_page_sizes_prop(PowerPCCPU *cpu, uint32_t *prop, size_t maxsize); #endif /* PPC_FDT_H */ -- 2.14.3
[Qemu-devel] [PATCH for-2.13 10/13] target/ppc: Fold ci_large_pages flag into PPCHash64Options
The ci_large_pages boolean in CPUPPCState is only relevant to 64-bit hash MMU machines, indicating whether it's possible to map large (> 4kiB) pages as cache-inhibitied (i.e. for IO, rather than memory). Fold it as another flag into the PPCHash64Options structure. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- hw/ppc/spapr.c | 3 +-- target/ppc/cpu.h| 1 - target/ppc/kvm.c| 6 +- target/ppc/mmu-hash64.c | 2 +- target/ppc/mmu-hash64.h | 1 + target/ppc/translate_init.c | 3 --- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index f86cb09080..60bc8417b6 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -263,7 +263,6 @@ static void spapr_populate_pa_features(sPAPRMachineState *spapr, void *fdt, int offset, bool legacy_guest) { -CPUPPCState *env = &cpu->env; uint8_t pa_features_206[] = { 6, 0, 0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 }; uint8_t pa_features_207[] = { 24, 0, @@ -315,7 +314,7 @@ static void spapr_populate_pa_features(sPAPRMachineState *spapr, return; } -if (env->ci_large_pages) { +if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) { /* * Note: we keep CI large pages off by default because a 64K capable * guest provisioned with large pages might otherwise try to map a qemu diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 1c5c33ca11..c0c44fb91d 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1090,7 +1090,6 @@ struct CPUPPCState { #if defined(TARGET_PPC64) ppc_slb_t vrma_slb; target_ulong rmls; -bool ci_large_pages; #endif #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 22487cef06..fff2c601e0 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -448,7 +448,11 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu) * host page size is smaller than 64K. */ if (smmu_info.flags & KVM_PPC_PAGE_SIZES_REAL) { -env->ci_large_pages = getpagesize() >= 0x1; +if (getpagesize() >= 0x1) { +cpu->hash64_opts->flags |= PPC_HASH64_CI_LARGEPAGE; +} else { +cpu->hash64_opts->flags &= ~PPC_HASH64_CI_LARGEPAGE; +} } /* diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index c9ee55e1ea..f341714550 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -1128,7 +1128,7 @@ const PPCHash64Options ppc_hash64_opts_basic = { }; const PPCHash64Options ppc_hash64_opts_POWER7 = { -.flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR, +.flags = PPC_HASH64_1TSEG | PPC_HASH64_AMR | PPC_HASH64_CI_LARGEPAGE, .sps = { { .page_shift = 12, /* 4K */ diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h index b2b5d25238..f1babb0afc 100644 --- a/target/ppc/mmu-hash64.h +++ b/target/ppc/mmu-hash64.h @@ -155,6 +155,7 @@ struct PPCHash64SegmentPageSizes { struct PPCHash64Options { #define PPC_HASH64_1TSEG0x1 #define PPC_HASH64_AMR 0x2 +#define PPC_HASH64_CI_LARGEPAGE 0x4 unsigned flags; PPCHash64SegmentPageSizes sps[PPC_PAGE_SIZES_MAX_SZ]; }; diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c index ae005b2a54..a925cf5cd3 100644 --- a/target/ppc/translate_init.c +++ b/target/ppc/translate_init.c @@ -8392,7 +8392,6 @@ static void init_proc_POWER7(CPUPPCState *env) #if !defined(CONFIG_USER_ONLY) env->slb_nr = 32; #endif -env->ci_large_pages = true; env->dcache_line_size = 128; env->icache_line_size = 128; @@ -8547,7 +8546,6 @@ static void init_proc_POWER8(CPUPPCState *env) #if !defined(CONFIG_USER_ONLY) env->slb_nr = 32; #endif -env->ci_large_pages = true; env->dcache_line_size = 128; env->icache_line_size = 128; @@ -8748,7 +8746,6 @@ static void init_proc_POWER9(CPUPPCState *env) #if !defined(CONFIG_USER_ONLY) env->slb_nr = 32; #endif -env->ci_large_pages = true; env->dcache_line_size = 128; env->icache_line_size = 128; -- 2.14.3
[Qemu-devel] [PATCH for-2.13 11/13] target/ppc: Remove unnecessary POWERPC_MMU_V3 flag from mmu_model
The only place we test this flag is in conjunction with ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- target/ppc/cpu-qom.h| 4 +--- target/ppc/mmu-hash64.c | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index 2bd58b2a84..ef96d42cf2 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -68,7 +68,6 @@ enum powerpc_mmu_t { /* PowerPC 601 MMU model (specific BATs format)*/ POWERPC_MMU_601= 0x000A, #define POWERPC_MMU_64 0x0001 -#define POWERPC_MMU_V3 0x0010 /* ISA V3.00 MMU Support */ /* 64 bits PowerPC MMU */ POWERPC_MMU_64B= POWERPC_MMU_64 | 0x0001, /* Architecture 2.03 and later (has LPCR) */ @@ -78,8 +77,7 @@ enum powerpc_mmu_t { /* Architecture 2.07 variant */ POWERPC_MMU_2_07 = POWERPC_MMU_64 | 0x0004, /* Architecture 3.00 variant */ -POWERPC_MMU_3_00 = POWERPC_MMU_64 | POWERPC_MMU_V3 - | 0x0005, +POWERPC_MMU_3_00 = POWERPC_MMU_64 | 0x0005, }; #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0x)) #define POWERPC_MMU_VER_64B POWERPC_MMU_VER(POWERPC_MMU_64B) diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index f341714550..df26a03c15 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -761,7 +761,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, slb = slb_lookup(cpu, eaddr); if (!slb) { /* No entry found, check if in-memory segment tables are in use */ -if ((env->mmu_model & POWERPC_MMU_V3) && ppc64_use_proc_tbl(cpu)) { +if (ppc64_use_proc_tbl(cpu)) { /* TODO - Unsupported */ error_report("Segment Table Support Unimplemented"); exit(1); -- 2.14.3
[Qemu-devel] [PATCH for-2.13 02/13] target/ppc: Simplify cpu valid check in ppc_cpu_realize
The #if isn't necessary, because there's a suitable one inside ppc_cpu_is_valid(). We've already filtered for suitable cpu models in the functions that search and register them. So by the time we get to realize having an invalid one indicates a code error, not a user error, so an assert() is more appropriate than error_setg(). Signed-off-by: David Gibson Reviewed-by: Thomas Huth Reviewed-by: Greg Kurz --- target/ppc/translate_init.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c index 56b80a204a..2ae718242a 100644 --- a/target/ppc/translate_init.c +++ b/target/ppc/translate_init.c @@ -9749,14 +9749,7 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp) } } -#if defined(TARGET_PPCEMB) -if (!ppc_cpu_is_valid(pcc)) { -error_setg(errp, "CPU does not possess a BookE or 4xx MMU. " - "Please use qemu-system-ppc or qemu-system-ppc64 instead " - "or choose another CPU model."); -goto unrealize; -} -#endif +assert(ppc_cpu_is_valid(pcc)); create_ppc_opcodes(cpu, &local_err); if (local_err != NULL) { -- 2.14.3
[Qemu-devel] [PATCH for-2.13 06/13] target/ppc: Move page size setup to helper function
Initialization of the env->sps structure at the end of instance_init is specific to the 64-bit hash MMU, so move the code into a helper function in mmu-hash64.c. We also create a corresponding function to be called at finalize time - it's empty for now, but we'll need it shortly. Signed-off-by: David Gibson Reviewed-by: Cédric Le Goater Reviewed-by: Greg Kurz --- target/ppc/mmu-hash64.c | 29 + target/ppc/mmu-hash64.h | 11 +++ target/ppc/translate_init.c | 29 + 3 files changed, 49 insertions(+), 20 deletions(-) diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index a87fa7c83f..4cb7d1cf07 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -1095,3 +1095,32 @@ void helper_store_lpcr(CPUPPCState *env, target_ulong val) ppc_hash64_update_rmls(cpu); ppc_hash64_update_vrma(cpu); } + +void ppc_hash64_init(PowerPCCPU *cpu) +{ +CPUPPCState *env = &cpu->env; +PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); + +if (pcc->sps) { +env->sps = *pcc->sps; +} else if (env->mmu_model & POWERPC_MMU_64) { +/* Use default sets of page sizes. We don't support MPSS */ +static const struct ppc_segment_page_sizes defsps = { +.sps = { +{ .page_shift = 12, /* 4K */ + .slb_enc = 0, + .enc = { { .page_shift = 12, .pte_enc = 0 } } +}, +{ .page_shift = 24, /* 16M */ + .slb_enc = 0x100, + .enc = { { .page_shift = 24, .pte_enc = 0 } } +}, +}, +}; +env->sps = defsps; +} +} + +void ppc_hash64_finalize(PowerPCCPU *cpu) +{ +} diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h index 95a8c330d6..074ded4c27 100644 --- a/target/ppc/mmu-hash64.h +++ b/target/ppc/mmu-hash64.h @@ -19,6 +19,8 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu, uint64_t pte0, uint64_t pte1); void ppc_hash64_update_vrma(PowerPCCPU *cpu); void ppc_hash64_update_rmls(PowerPCCPU *cpu); +void ppc_hash64_init(PowerPCCPU *cpu); +void ppc_hash64_finalize(PowerPCCPU *cpu); #endif /* @@ -136,4 +138,13 @@ static inline uint64_t ppc_hash64_hpte1(PowerPCCPU *cpu, #endif /* CONFIG_USER_ONLY */ +#if defined(CONFIG_USER_ONLY) || !defined(TARGET_PPC64) +static inline void ppc_hash64_init(PowerPCCPU *cpu) +{ +} +static inline void ppc_hash64_finalize(PowerPCCPU *cpu) +{ +} +#endif + #endif /* MMU_HASH64_H */ diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c index 99be6fcd68..aa63a5dcb3 100644 --- a/target/ppc/translate_init.c +++ b/target/ppc/translate_init.c @@ -10464,26 +10464,14 @@ static void ppc_cpu_instance_init(Object *obj) env->has_hv_mode = !!(env->msr_mask & MSR_HVB); #endif -#if defined(TARGET_PPC64) -if (pcc->sps) { -env->sps = *pcc->sps; -} else if (env->mmu_model & POWERPC_MMU_64) { -/* Use default sets of page sizes. We don't support MPSS */ -static const struct ppc_segment_page_sizes defsps = { -.sps = { -{ .page_shift = 12, /* 4K */ - .slb_enc = 0, - .enc = { { .page_shift = 12, .pte_enc = 0 } } -}, -{ .page_shift = 24, /* 16M */ - .slb_enc = 0x100, - .enc = { { .page_shift = 24, .pte_enc = 0 } } -}, -}, -}; -env->sps = defsps; -} -#endif /* defined(TARGET_PPC64) */ +ppc_hash64_init(cpu); +} + +static void ppc_cpu_instance_finalize(Object *obj) +{ +PowerPCCPU *cpu = POWERPC_CPU(obj); + +ppc_hash64_finalize(cpu); } static bool ppc_pvr_match_default(PowerPCCPUClass *pcc, uint32_t pvr) @@ -10601,6 +10589,7 @@ static const TypeInfo ppc_cpu_type_info = { .parent = TYPE_CPU, .instance_size = sizeof(PowerPCCPU), .instance_init = ppc_cpu_instance_init, +.instance_finalize = ppc_cpu_instance_finalize, .abstract = true, .class_size = sizeof(PowerPCCPUClass), .class_init = ppc_cpu_class_init, -- 2.14.3
[Qemu-devel] [PATCH qemu] spapr: Initialize reserved areas list in FDT in H_CAS handler
At the moment the device tree produced by the H_CAS handler has no reserved map initialized at all which is not correct as at least one empty record is required to be present as a marker of the end. This does not cause problems now as the only consumer is SLOF which does not look at the reserved map area. However when DTC's "Improve libfdt's memory safety" changeset hits the QEMU upstream, there will be errors reported and crashes observed. This fixes the problem by adding an empty entry to the reserved map, just like create_device_tree() does already. Signed-off-by: Alexey Kardashevskiy --- hw/ppc/spapr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2c0be8c..a81570e 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -865,6 +865,7 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr, /* Create skeleton */ fdt_skel = g_malloc0(size); _FDT((fdt_create(fdt_skel, size))); +_FDT((fdt_finish_reservemap(fdt_skel))); _FDT((fdt_begin_node(fdt_skel, ""))); _FDT((fdt_end_node(fdt_skel))); _FDT((fdt_finish(fdt_skel))); -- 2.11.0
[Qemu-devel] [PATCH v2] linux-user: fix preadv/pwritev offsets
preadv/pwritev accept low and high parts of file offset in two separate parameters. When host bitness doesn't match guest bitness these parts must be appropriately recombined. Introduce target_low_high_to_host_low_high that does this recombination and use it in preadv/pwritev syscalls. This fixes glibc testsuite test misc/tst-preadvwritev64. Signed-off-by: Max Filippov --- Changes v1->v2: - fix host high computation in TARGET_LONG_BITS > HOST_LONG_BITS case linux-user/syscall.c | 27 +-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 5ef517613577..7e014066260a 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3386,6 +3386,23 @@ static abi_long do_getsockopt(int sockfd, int level, int optname, return ret; } +static void target_low_high_to_host_low_high(abi_ulong tlow, + abi_ulong thigh, + unsigned long *hlow, + unsigned long *hhigh) +{ +#if TARGET_LONG_BITS == HOST_LONG_BITS +*hlow = tlow; +*hhigh = thigh; +#elif TARGET_LONG_BITS < HOST_LONG_BITS +*hlow = tlow | (unsigned long)thigh << TARGET_LONG_BITS; +*hhigh = 0; +#else +*hlow = (unsigned long)tlow; +*hhigh = (unsigned long)(tlow >> HOST_LONG_BITS); +#endif +} + static struct iovec *lock_iovec(int type, abi_ulong target_addr, abi_ulong count, int copy) { @@ -10449,7 +10466,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0); if (vec != NULL) { -ret = get_errno(safe_preadv(arg1, vec, arg3, arg4, arg5)); +unsigned long low, high; + +target_low_high_to_host_low_high(arg4, arg5, &low, &high); +ret = get_errno(safe_preadv(arg1, vec, arg3, low, high)); unlock_iovec(vec, arg2, arg3, 1); } else { ret = -host_to_target_errno(errno); @@ -10462,7 +10482,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1); if (vec != NULL) { -ret = get_errno(safe_pwritev(arg1, vec, arg3, arg4, arg5)); +unsigned long low, high; + +target_low_high_to_host_low_high(arg4, arg5, &low, &high); +ret = get_errno(safe_pwritev(arg1, vec, arg3, low, high)); unlock_iovec(vec, arg2, arg3, 0); } else { ret = -host_to_target_errno(errno); -- 2.11.0
Re: [Qemu-devel] Loadable block drivers?
On 4 April 2018 at 23:41, Stefan Hajnoczi wrote: > On Tue, Apr 03, 2018 at 11:30:33AM +0800, Fam Zheng wrote: > > On Tue, 04/03 13:17, Lindsay Mathieson wrote: > > > On 3 April 2018 at 13:11, Fam Zheng wrote: > > > > > > > On Tue, 04/03 12:59, Lindsay Mathieson wrote: > > > > > Hi all, was looking at developing a block driver for qemu - have > examined > > > > > the drivers at: > > > > > > > > > > https://github.com/qemu/qemu/tree/master/block > > > > > > > > > > And it seems straightforward enough. > > > > > > > > > > One thing that is unclear - all the drivers appear to be compiled > > > > directly > > > > > into qemu. Is there no way to load them dynamically as .so modules? > > > > > > > > './configure --enable-modules' will enable building block drivers as > .so > > > > objects, and they are loaded dynamically. These are in-tree .so > modules; > > > > out-of-tree modules like in Linux kernel are intentionally forbidden. > > > > > > > > Fam > > > > > > > > > > > > > > > > Rats, I take it that means I can't develop a testing block module and > load > > > it with an pre-existing qemu install. > > > > No, that's not possible. > > Depending on what you are trying to do, you could use the blkdebug, > null-co, NBD, or iSCSI drivers to perform your testing. > > blkdebug does fault injection (e.g. you can test what happens when > certain I/O requests fail). > > null-co is a nop block driver useful for some types of performance > testing and it also supports introducing an artificial delays. > > NBD and iSCSI can be used to forward I/O requests to an external server > where you can implement any behavior you want. > > We can discuss it more if you can explain what you're trying to do. > > Stefan > Thanks Stefan, looking to develop a lizardfs block driver. A process that only involved building a module rather than the entire qemu tree would mike life easier, especially if I could test it on a live system (proxmox cluster). A custom qemu install is not an option for that. -- Lindsay
[Qemu-devel] [PATCH] linux-user: fix preadv/pwritev offsets
preadv/pwritev accept low and high parts of file offset in two separate parameters. When host bitness doesn't match guest bitness these parts must be appropriately recombined. Introduce target_low_high_to_host_low_high that does this recombination and use it in preadv/pwritev syscalls. This fixes glibc testsuite test misc/tst-preadvwritev64. Signed-off-by: Max Filippov --- linux-user/syscall.c | 27 +-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 5ef517613577..dfa015b9354d 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3386,6 +3386,23 @@ static abi_long do_getsockopt(int sockfd, int level, int optname, return ret; } +static void target_low_high_to_host_low_high(abi_ulong tlow, + abi_ulong thigh, + unsigned long *hlow, + unsigned long *hhigh) +{ +#if TARGET_LONG_BITS == HOST_LONG_BITS +*hlow = tlow; +*hhigh = thigh; +#elif TARGET_LONG_BITS < HOST_LONG_BITS +*hlow = tlow | (unsigned long)thigh << TARGET_LONG_BITS; +*hhigh = 0; +#else +*hlow = (unsigned long)tlow; +*hhigh = (unsigned long)(thigh >> HOST_LONG_BITS); +#endif +} + static struct iovec *lock_iovec(int type, abi_ulong target_addr, abi_ulong count, int copy) { @@ -10449,7 +10466,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0); if (vec != NULL) { -ret = get_errno(safe_preadv(arg1, vec, arg3, arg4, arg5)); +unsigned long low, high; + +target_low_high_to_host_low_high(arg4, arg5, &low, &high); +ret = get_errno(safe_preadv(arg1, vec, arg3, low, high)); unlock_iovec(vec, arg2, arg3, 1); } else { ret = -host_to_target_errno(errno); @@ -10462,7 +10482,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { struct iovec *vec = lock_iovec(VERIFY_READ, arg2, arg3, 1); if (vec != NULL) { -ret = get_errno(safe_pwritev(arg1, vec, arg3, arg4, arg5)); +unsigned long low, high; + +target_low_high_to_host_low_high(arg4, arg5, &low, &high); +ret = get_errno(safe_pwritev(arg1, vec, arg3, low, high)); unlock_iovec(vec, arg2, arg3, 0); } else { ret = -host_to_target_errno(errno); -- 2.11.0
Re: [Qemu-devel] [RFC PATCH v2 0/4] memory: fix access_with_adjusted_size() and misc
Hi, This series failed docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. Type: series Message-id: 20180405012241.25714-1-f4...@amsat.org Subject: [Qemu-devel] [RFC PATCH v2 0/4] memory: fix access_with_adjusted_size() and misc === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=8 time make docker-test-mingw@fedora === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' b58dcfc7a0 memory: Add memory_region_set_priority() 0104ce5428 memory: Fix memory_region 'priority' property sign 2b480c4738 memory: Fix access_with_adjusted_size() when size < access_size_min c5b3292e86 memory: Avoid to create tiny RAM regions, handled as subpages === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-hs1n67tk/src/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' BUILD fedora make[1]: Entering directory '/var/tmp/patchew-tester-tmp-hs1n67tk/src' GEN /var/tmp/patchew-tester-tmp-hs1n67tk/src/docker-src.2018-04-04-21.28.07.31227/qemu.tar Cloning into '/var/tmp/patchew-tester-tmp-hs1n67tk/src/docker-src.2018-04-04-21.28.07.31227/qemu.tar.vroot'... done. Checking out files: 46% (2791/6066) Checking out files: 47% (2852/6066) Checking out files: 48% (2912/6066) Checking out files: 49% (2973/6066) Checking out files: 50% (3033/6066) Checking out files: 51% (3094/6066) Checking out files: 52% (3155/6066) Checking out files: 53% (3215/6066) Checking out files: 54% (3276/6066) Checking out files: 55% (3337/6066) Checking out files: 56% (3397/6066) Checking out files: 57% (3458/6066) Checking out files: 58% (3519/6066) Checking out files: 59% (3579/6066) Checking out files: 60% (3640/6066) Checking out files: 61% (3701/6066) Checking out files: 62% (3761/6066) Checking out files: 63% (3822/6066) Checking out files: 64% (3883/6066) Checking out files: 65% (3943/6066) Checking out files: 66% (4004/6066) Checking out files: 67% (4065/6066) Checking out files: 68% (4125/6066) Checking out files: 69% (4186/6066) Checking out files: 70% (4247/6066) Checking out files: 71% (4307/6066) Checking out files: 72% (4368/6066) Checking out files: 73% (4429/6066) Checking out files: 74% (4489/6066) Checking out files: 75% (4550/6066) Checking out files: 76% (4611/6066) Checking out files: 77% (4671/6066) Checking out files: 78% (4732/6066) Checking out files: 79% (4793/6066) Checking out files: 80% (4853/6066) Checking out files: 81% (4914/6066) Checking out files: 82% (4975/6066) Checking out files: 83% (5035/6066) Checking out files: 84% (5096/6066) Checking out files: 85% (5157/6066) Checking out files: 86% (5217/6066) Checking out files: 87% (5278/6066) Checking out files: 88% (5339/6066) Checking out files: 89% (5399/6066) Checking out files: 90% (5460/6066) Checking out files: 91% (5521/6066) Checking out files: 92% (5581/6066) Checking out files: 93% (5642/6066) Checking out files: 94% (5703/6066) Checking out files: 95% (5763/6066) Checking out files: 96% (5824/6066) Checking out files: 97% (5885/6066) Checking out files: 98% (5945/6066) Checking out files: 99% (6006/6066) Checking out files: 100% (6066/6066) Checking out files: 100% (6066/6066), done. Your branch is up-to-date with 'origin/test'. Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into '/var/tmp/patchew-tester-tmp-hs1n67tk/src/docker-src.2018-04-04-21.28.07.31227/qemu.tar.vroot/dtc'... Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42' Submodule 'ui/keycodemapdb' (git://git.qemu.org/keycodemapdb.git) registered for path 'ui/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-hs1n67tk/src/docker-src.2018-04-04-21.28.07.31227/qemu.tar.vroot/ui/keycodemapdb'... Submodule path 'ui/keycodemapdb': checked out '6b3d716e2b6472eb7189d3220552280ef3d832ce' COPYRUNNER RUN test-mingw in qemu:fedora Packages installed: PyYAML-3.12-5.fc27.x86_64 SDL-devel-1.2.15-29.fc27.x86_64 bc-1.07.1-3.fc27.x86_64 bison-3.0.4-8.fc27.x86_64 bzip2-1.0.6-24.fc27.x86_64 ccache-3.3.6-1.fc27.x86_64 clang-5.0.1-3.fc27.x86_64 findutils-4.6.0-16.fc27.x86_64 flex-2.6.1-5.fc27.x86_64 gcc-7.3.1-5.fc27.x86_64 gcc-c++-7.3.1-5.fc27.x86_64 gettext-0.19.8.1-12.fc27.x86_64 git-2.14.3-3.fc27.x86_64 glib2-devel-2.54.3-2.fc27.x86_64 hostname-3.18-4.fc27.x86_64 libaio-devel-0.3.110-9.fc27.x86_64 libasan-7.3.1-5.fc27.x86_64 libfdt-devel-1.4.6-1.fc27.x86_64 libubsan-7.3.1-5.fc27.x86_64 llvm-5.0.1-3.fc27.x86_64 make-4.2.1-4.fc27.x86_64 mingw32-SDL-1
[Qemu-devel] [RFC PATCH v2 4/4] memory: Add memory_region_set_priority()
Signed-off-by: Philippe Mathieu-Daudé --- Sadly I'm missing something, this does not work. memory.c | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/memory.c b/memory.c index eaa5fa7f23..ae45ea7779 100644 --- a/memory.c +++ b/memory.c @@ -1225,6 +1225,22 @@ static void memory_region_get_priority(Object *obj, Visitor *v, visit_type_int32(v, name, &value, errp); } +static void memory_region_set_priority(Object *obj, Visitor *v, + const char *name, void *opaque, + Error **errp) +{ +MemoryRegion *mr = MEMORY_REGION(obj); +int32_t priority; +Error *local_err = NULL; + +visit_type_int32(v, name, &priority, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} +mr->priority = priority; +} + static void memory_region_get_size(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { @@ -1260,7 +1276,7 @@ static void memory_region_initfn(Object *obj) NULL, NULL, &error_abort); object_property_add(OBJECT(mr), "priority", "int32", memory_region_get_priority, -NULL, /* memory_region_set_priority */ +memory_region_set_priority, NULL, NULL, &error_abort); object_property_add(OBJECT(mr), "size", "uint64", memory_region_get_size, -- 2.16.3
[Qemu-devel] [PATCH v2 3/4] memory: Fix memory_region 'priority' property sign
Priorities can be negative, fix this limitation. Signed-off-by: Philippe Mathieu-Daudé --- memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/memory.c b/memory.c index e77f9e4036..eaa5fa7f23 100644 --- a/memory.c +++ b/memory.c @@ -1258,7 +1258,7 @@ static void memory_region_initfn(Object *obj) memory_region_get_addr, NULL, /* memory_region_set_addr */ NULL, NULL, &error_abort); -object_property_add(OBJECT(mr), "priority", "uint32", +object_property_add(OBJECT(mr), "priority", "int32", memory_region_get_priority, NULL, /* memory_region_set_priority */ NULL, NULL, &error_abort); -- 2.16.3
[Qemu-devel] [PATCH v2 1/4] memory: Avoid to create tiny RAM regions, handled as subpages
If an user creates a RAM region smaller than TARGET_PAGE_SIZE, this region will be handled as a subpage. While the subpage behavior can be noticed by an experienced QEMU developper, it might takes hours to a novice to figure it out. To save time to novices, do not allow subpage creation via the memory_region_init_ram_*() functions. Signed-off-by: Philippe Mathieu-Daudé --- memory.c | 9 + 1 file changed, 9 insertions(+) diff --git a/memory.c b/memory.c index e70b64b8b9..51d27b7b26 100644 --- a/memory.c +++ b/memory.c @@ -1519,6 +1519,15 @@ void memory_region_init_ram_shared_nomigrate(MemoryRegion *mr, bool share, Error **errp) { +if (size < TARGET_PAGE_SIZE) { +/* Region less than PAGE_SIZE are handled as subpages, which are + * surely not what the caller expects. + * Limit the minimum ram region size to avoid annoying debugging. + */ +error_setg(errp, "Invalid RAM size: %ld (minimum required: %d)", + size, TARGET_PAGE_SIZE); +return; +} memory_region_init(mr, owner, name, size); mr->ram = true; mr->terminates = true; -- 2.16.3
[Qemu-devel] [NOTFORMERGE PATCH v2 2/4] memory: Fix access_with_adjusted_size() when size < access_size_min
ASan reported: shift exponent 4294967280 is too large for 64-bit type 'long unsigned int' ... runtime error: shift exponent -16 is negative This can occurs when MemoryRegionOps .valid.min_access_size < .impl.min_access_size, for example if a guest uses a 16-bit operand to access a 32-bit register. The current code is limited to right shifting. Use a signed shift, to allow shifting left when the value is negative. Reported-by: AddressSanitizer Signed-off-by: Philippe Mathieu-Daudé --- memory.c | 74 +--- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/memory.c b/memory.c index 51d27b7b26..e77f9e4036 100644 --- a/memory.c +++ b/memory.c @@ -404,7 +404,7 @@ static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr, hwaddr addr, uint64_t *value, unsigned size, - unsigned shift, + signed shift, uint64_t mask, MemTxAttrs attrs) { @@ -422,7 +422,11 @@ static MemTxResult memory_region_oldmmio_read_accessor(MemoryRegion *mr, hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr); trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size); } -*value |= (tmp & mask) << shift; +if (likely(shift >= 0)) { +*value |= (tmp & mask) << shift; +} else { +*value |= (tmp >> -shift) & mask; +} return MEMTX_OK; } @@ -430,7 +434,7 @@ static MemTxResult memory_region_read_accessor(MemoryRegion *mr, hwaddr addr, uint64_t *value, unsigned size, -unsigned shift, +signed shift, uint64_t mask, MemTxAttrs attrs) { @@ -448,7 +452,11 @@ static MemTxResult memory_region_read_accessor(MemoryRegion *mr, hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr); trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size); } -*value |= (tmp & mask) << shift; +if (likely(shift >= 0)) { +*value |= (tmp & mask) << shift; +} else { +*value |= (tmp >> -shift) & mask; +} return MEMTX_OK; } @@ -456,7 +464,7 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr, hwaddr addr, uint64_t *value, unsigned size, - unsigned shift, + signed shift, uint64_t mask, MemTxAttrs attrs) { @@ -475,7 +483,11 @@ static MemTxResult memory_region_read_with_attrs_accessor(MemoryRegion *mr, hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr); trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, size); } -*value |= (tmp & mask) << shift; +if (likely(shift >= 0)) { +*value |= (tmp & mask) << shift; +} else { +*value |= (tmp >> -shift) & mask; +} return r; } @@ -483,13 +495,17 @@ static MemTxResult memory_region_oldmmio_write_accessor(MemoryRegion *mr, hwaddr addr, uint64_t *value, unsigned size, -unsigned shift, +signed shift, uint64_t mask, MemTxAttrs attrs) { uint64_t tmp; -tmp = (*value >> shift) & mask; +if (likely(shift >= 0)) { +tmp = (*value >> shift) & mask; +} else { +tmp = (*value & mask) << -shift; +} if (mr->subpage) { trace_memory_region_subpage_write(get_cpu_index(), mr, addr, tmp, size); } else if (mr == &io_mem_notdirty) { @@ -509,13 +525,17 @@ static MemTxResult memory_region_write_accessor(MemoryRegion *mr, hwaddr addr, ui
[Qemu-devel] [RFC PATCH v2 0/4] memory: fix access_with_adjusted_size() and misc
Hi, these are few memory related patches. Patch #2 v1 was: http://lists.gnu.org/archive/html/qemu-devel/2017-08/msg02255.html I'll send the related qtests in v3. Patch #4 is a failed attempt to change a region priority at runtime. Phil. Philippe Mathieu-Daudé (4): memory: Avoid to create tiny RAM regions, handled as subpages memory: Fix access_with_adjusted_size() when size < access_size_min memory: Fix memory_region 'priority' property sign memory: Add memory_region_set_priority() memory.c | 103 --- 1 file changed, 79 insertions(+), 24 deletions(-) -- 2.16.3
Re: [Qemu-devel] [PULL 16/20] target/arm: Use vector infrastructure for aa64 compares
On 04/05/2018 10:07 AM, Richard Henderson wrote: > On 04/05/2018 02:49 AM, Emilio G. Cota wrote: >> 1. grab this binary: >> http://cs.columbia.edu/~cota/qemu/nbench-aarch64 >> 2. run it on a PowerPC host with: >> $ aarch64-linux-user/qemu-aarch64 nbench-aarch64 -V >> >> Note: the "-V" (or "-v") flag is important! Without it, there's no segfault. > > How quickly? I did not see one up until it exited for lack of NNET.DAT. > > I will note that I am using gcc 7.2 on gcc112 (/opt/cfarm/gcc-latest/bin/gcc). > > Are you using gcc 4.8.5, and is commit 74912f6dad in your tree? > That commit might have made a difference... Bah. I confirm that it doesn't, and that the test still fails. Since qemu does work on the same host when built with gcc 7.2, I can only blame this failure on a compiler bug wrt gcc 4.8 on ppc64. I have not tracked down exactly what's going wrong, and probably won't unless someone feels that it's worthwhile. This only begs the question of whether we should blacklist this compiler entirely. Certainly it's old enough that it's probably not worth fixing. r~
Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
On 5/4/18 9:09 am, Alex Williamson wrote: > On Wed, 4 Apr 2018 22:30:50 +0200 > Eric Auger wrote: > >> The 567b5b309abe ("vfio/pci: Relax DMA map errors for MMIO regions") >> added an error message if a passed memory section address or size >> is not aligned to the page size and thus cannot be DMA mapped. >> >> This patch fixes the trace by printing the region name and the >> memory region section offset within the address space (instead of >> offset_within_region). >> >> We also turn the error_report into a trace event. Indeed, In some >> cases, the traces can be confusing to non expert end-users and >> let think the use case does not work (whereas it >> works as before). >> >> This is the case where a BAR is successively mapped at different >> GPAs and its sections are not compatible with dma map. The listener >> is called several times and traces are issued for each intermediate >> mapping. The end-user cannot easily match those GPAs against the >> final GPA output by lscpi. So let's keep those information to >> informed users. In mid term, the plan is to advise the user about >> BAR relocation relevance. >> >> Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions" >> Signed-off-by: Alexey Kardashevskiy >> Signed-off-by: Eric Auger >> >> --- >> >> v1 -> v2: >> - use a trace point >> - add some details in the commit message > > I think this is v2 with v1 being > Message-Id:<20180322081837.21460-1-...@ozlabs.ru> but this is > substantially different from Alexey's posting so I'd like to verify the > Sign-off. Alexey? My understanding it is not "sign-off" any more (I am fine either way, it just does not seem fully correct), but Reviewed-by: Alexey Kardashevskiy > > I agree that this seems to be the right thing to do for 2.12, nothing > has changed for these mappings and the error reports are difficult even > for experts to decipher and explain to users. Thanks, > > Alex > >> --- >> hw/vfio/common.c | 11 +-- >> hw/vfio/trace-events | 1 + >> 2 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index 5e84716..07ffa0b 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -548,12 +548,11 @@ static void vfio_listener_region_add(MemoryListener >> *listener, >> hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1; >> >> if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) { >> -error_report("Region 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx >> - " is not aligned to 0x%"HWADDR_PRIx >> - " and cannot be mapped for DMA", >> - section->offset_within_region, >> - int128_getlo(section->size), >> - pgmask + 1); >> +trace_vfio_listener_region_add_no_dma_map( >> +memory_region_name(section->mr), >> +section->offset_within_address_space, >> +int128_getlo(section->size), >> +pgmask + 1); >> return; >> } >> } >> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events >> index 79f63a2..20109cb 100644 >> --- a/hw/vfio/trace-events >> +++ b/hw/vfio/trace-events >> @@ -90,6 +90,7 @@ vfio_iommu_map_notify(const char *op, uint64_t iova_start, >> uint64_t iova_end) "i >> vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING >> region_add 0x%"PRIx64" - 0x%"PRIx64 >> vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add >> [iommu] 0x%"PRIx64" - 0x%"PRIx64 >> vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void >> *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" >> +vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, >> uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" >> size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" >> vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING >> region_del 0x%"PRIx64" - 0x%"PRIx64 >> vfio_listener_region_del(uint64_t start, uint64_t end) "region_del >> 0x%"PRIx64" - 0x%"PRIx64 >> vfio_disconnect_container(int fd) "close container->fd=%d" > -- Alexey
Re: [Qemu-devel] [PULL 16/20] target/arm: Use vector infrastructure for aa64 compares
On 04/05/2018 02:49 AM, Emilio G. Cota wrote: > 1. grab this binary: > http://cs.columbia.edu/~cota/qemu/nbench-aarch64 > 2. run it on a PowerPC host with: > $ aarch64-linux-user/qemu-aarch64 nbench-aarch64 -V > > Note: the "-V" (or "-v") flag is important! Without it, there's no segfault. How quickly? I did not see one up until it exited for lack of NNET.DAT. I will note that I am using gcc 7.2 on gcc112 (/opt/cfarm/gcc-latest/bin/gcc). Are you using gcc 4.8.5, and is commit 74912f6dad in your tree? That commit might have made a difference... r~
[Qemu-devel] [PATCH for-2.12] hw/block/pflash_cfi: fix off-by-one error
ASAN reported: hw/block/pflash_cfi02.c:245:33: runtime error: index 82 out of bounds for type 'uint8_t [82]' Since the 'cfi_len' member is not used, remove it to keep the code safer. Reported-by: AddressSanitizer Signed-off-by: Philippe Mathieu-Daudé --- hw/block/pflash_cfi01.c | 10 -- hw/block/pflash_cfi02.c | 9 - 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 1113ab1ccf..2e8284001d 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -90,7 +90,6 @@ struct pflash_t { uint16_t ident1; uint16_t ident2; uint16_t ident3; -uint8_t cfi_len; uint8_t cfi_table[0x52]; uint64_t counter; unsigned int writeblock_size; @@ -153,7 +152,7 @@ static uint32_t pflash_cfi_query(pflash_t *pfl, hwaddr offset) boff = offset >> (ctz32(pfl->bank_width) + ctz32(pfl->max_device_width) - ctz32(pfl->device_width)); -if (boff > pfl->cfi_len) { +if (boff >= sizeof(pfl->cfi_table)) { return 0; } /* Now we will construct the CFI response generated by a single @@ -385,10 +384,10 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset, boff = boff >> 2; } -if (boff > pfl->cfi_len) { -ret = 0; -} else { +if (boff < sizeof(pfl->cfi_table)) { ret = pfl->cfi_table[boff]; +} else { +ret = 0; } } else { /* If we have a read larger than the bank_width, combine multiple @@ -791,7 +790,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp) pfl->cmd = 0; pfl->status = 0; /* Hardcoded CFI table */ -pfl->cfi_len = 0x52; /* Standard "QRY" string */ pfl->cfi_table[0x10] = 'Q'; pfl->cfi_table[0x11] = 'R'; diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index c81ddd3a99..75d1ae1026 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -83,7 +83,6 @@ struct pflash_t { uint16_t ident3; uint16_t unlock_addr0; uint16_t unlock_addr1; -uint8_t cfi_len; uint8_t cfi_table[0x52]; QEMUTimer *timer; /* The device replicates the flash memory across its memory space. Emulate @@ -235,10 +234,11 @@ static uint32_t pflash_read (pflash_t *pfl, hwaddr offset, break; case 0x98: /* CFI query mode */ -if (boff > pfl->cfi_len) -ret = 0; -else +if (boff < sizeof(pfl->cfi_table)) { ret = pfl->cfi_table[boff]; +} else { +ret = 0; +} break; } @@ -663,7 +663,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp) pfl->cmd = 0; pfl->status = 0; /* Hardcoded CFI table (mostly from SG29 Spansion flash) */ -pfl->cfi_len = 0x52; /* Standard "QRY" string */ pfl->cfi_table[0x10] = 'Q'; pfl->cfi_table[0x11] = 'R'; -- 2.16.3
Re: [Qemu-devel] [PATCH v3 00/15] fp-test + hardfloat
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 1522883475-27858-1-git-send-email-c...@braap.org Subject: [Qemu-devel] [PATCH v3 00/15] fp-test + hardfloat === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu t [tag update] patchew/1522869306-17292-1-git-send-email-th...@redhat.com -> patchew/1522869306-17292-1-git-send-email-th...@redhat.com t [tag update] patchew/1522873850-28733-1-git-send-email-eric.au...@redhat.com -> patchew/1522873850-28733-1-git-send-email-eric.au...@redhat.com * [new tag] patchew/1522883475-27858-1-git-send-email-c...@braap.org -> patchew/1522883475-27858-1-git-send-email-c...@braap.org Switched to a new branch 'test' 0160699351 hardfloat: support float32/64 comparison 6d049c226a hardfloat: support float32/64 square root f3fe3550b9 hardfloat: support float32/64 fused multiply-add 46eb7c82ae hardfloat: support float32/64 division ef7ad86525 hardfloat: support float32/64 multiplication 49d8c51e8d hardfloat: support float32/64 addition and subtraction 4c75ec0397 fpu: introduce hardfloat a01d10e08c softfloat: add float{32, 64}_is_zero_or_normal 89e7dbd313 softfloat: rename canonicalize to sf_canonicalize 9ac9b3dbc8 tests/fp: add fp-bench, a collection of simple floating point microbenchmarks d7ced9b1de target/tricore: use float32_is_denormal 17d8cfd693 softfloat: add float{32, 64}_is_{de, }normal 606b2bd962 fp-test: add muladd variants 0a1379a091 softfloat: fix {min, max}nummag for same-abs-value inputs 104af15d75 tests: add fp-test, a floating point test suite === OUTPUT BEGIN === Checking PATCH 1/15: tests: add fp-test, a floating point test suite... ERROR: Macros with complex values should be enclosed in parenthesis #380: FILE: tests/fp/fp-test.c:220: +#define PR_EXCEPTIONS(x)\ +((x) & STANDARD_EXCEPTIONS ? "" : "none"), \ +(((x) & float_flag_inexact) ? "x" : ""), \ +(((x) & float_flag_underflow) ? "u" : ""), \ +(((x) & float_flag_overflow) ? "o" : ""), \ +(((x) & float_flag_divbyzero) ? "z" : ""), \ +(((x) & float_flag_invalid) ? "i" : "") ERROR: consider using qemu_strtoul in preference to strtoul #841: FILE: tests/fp/fp-test.c:681: +significand = strtoul(&p[3], &pos, 16); ERROR: consider using qemu_strtol in preference to strtol #846: FILE: tests/fp/fp-test.c:686: +exponent = strtol(pos, &pos, 10) + 127; ERROR: consider using qemu_strtoul in preference to strtoul #871: FILE: tests/fp/fp-test.c:711: +significand = strtoul(&p[3], &pos, 16); ERROR: consider using qemu_strtol in preference to strtol #876: FILE: tests/fp/fp-test.c:716: +exponent = strtol(pos, &pos, 10) + 1023; ERROR: consider using qemu_strtof in preference to strtof #895: FILE: tests/fp/fp-test.c:735: +float f = strtof(p, &pos); total: 6 errors, 0 warnings, 1219 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 2/15: softfloat: fix {min, max}nummag for same-abs-value inputs... Checking PATCH 3/15: fp-test: add muladd variants... Checking PATCH 4/15: softfloat: add float{32, 64}_is_{de, }normal... Checking PATCH 5/15: target/tricore: use float32_is_denormal... Checking PATCH 6/15: tests/fp: add fp-bench, a collection of simple floating point microbenchmarks... ERROR: braces {} are necessary for all arms of this statement #183: FILE: tests/fp/fp-bench.c:133: +} while (!float32_is_normal(r)); [...] ERROR: braces {} are necessary for all arms of this statement #187: FILE: tests/fp/fp-bench.c:137: +} while (!float64_is_normal(r)); [...] total: 2 errors, 0 warnings, 547 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 7/15: softfloat: rename canonicalize to sf_canonicalize... Checking PATCH 8/15: softfloat: add float{32, 64}_is_zero_or_normal... Checking PATCH 9/15: fpu: introduce hardfloat... ERROR: spaces required around that '*' (ctx:WxV) #96: FILE: fpu/softfloat.c:154: +static inline void name(soft_t *a, float_status *s) \
Re: [Qemu-devel] [PATCH v3 13/15] hardfloat: support float32/64 fused multiply-add
On Wed, Apr 04, 2018 at 19:11:13 -0400, Emilio G. Cota wrote: (snip) > +if (likely((soft_t ## _is_normal(a) || soft_t ## _is_zero(a)) && \ > + (soft_t ## _is_normal(b) || soft_t ## _is_zero(b)) && \ > + (soft_t ## _is_normal(c) || soft_t ## _is_zero(c)) && \ This is outdated wrt to the v3 tree on github. Changed there to: -if (likely((soft_t ## _is_normal(a) || soft_t ## _is_zero(a)) && \ - (soft_t ## _is_normal(b) || soft_t ## _is_zero(b)) && \ - (soft_t ## _is_normal(c) || soft_t ## _is_zero(c)) && \ +if (likely(soft_t ## _is_zero_or_normal(a) && \ + soft_t ## _is_zero_or_normal(b) && \ + soft_t ## _is_zero_or_normal(c) && \ E.
Re: [Qemu-devel] [PATCHv1 00/14] Translation loop conversion for sh4/sparc/mips/s390x/openrisc targets
On Thu, Mar 01, 2018 at 17:53:44 -0500, Emilio G. Cota wrote: > [ What is this all about? See this message: > http://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04785.html ] (snip) > You can fetch this series from: > https://github.com/cota/qemu/tree/trloop-conv-v1 *ping* on this series. A few patches are missing reviews, mostly mips/openrisc bits. Thanks, Emilio
[Qemu-devel] [PATCH v3 09/15] fpu: introduce hardfloat
The appended paves the way for leveraging the host FPU for a subset of guest FP operations. For most guest workloads (e.g. FP flags aren't ever cleared, inexact occurs often and rounding is set to the default [to nearest]) this will yield sizable performance speedups. The approach followed here avoids checking the FP exception flags register. See the added comment for details. This assumes that QEMU is running on an IEEE754-compliant FPU and that the rounding is set to the default (to nearest). The implementation-dependent specifics of the FPU should not matter; things like tininess detection and snan representation are still dealt with in soft-fp. However, this approach will break on most hosts if we compile QEMU with flags such as -ffast-math. We control the flags so this should be easy to enforce though. This patch just adds common code. Some operations will be migrated to hardfloat in subsequent patches to ease bisection. Note: some architectures (at least PPC, there might be others) clear the status flags passed to softfloat before most FP operations. This precludes the use of hardfloat, so to avoid introducing a performance regression for those targets, we add a flag to disable hardfloat. In the long run though it would be good to fix the targets so that at least the inexact flag passed to softfloat is indeed sticky. Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 342 1 file changed, 342 insertions(+) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index c3b9d07..956b938 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -82,6 +82,8 @@ this code that are retained. /* softfloat (and in particular the code in softfloat-specialize.h) is * target-dependent and needs the TARGET_* macros. */ +#include + #include "qemu/osdep.h" #include "qemu/bitops.h" #include "fpu/softfloat.h" @@ -105,6 +107,346 @@ this code that are retained. **/ #include "softfloat-specialize.h" +/* + * Hardfloat + * + * Fast emulation of guest FP instructions is challenging for two reasons. + * First, FP instruction semantics are similar but not identical, particularly + * when handling NaNs. Second, emulating at reasonable speed the guest FP + * exception flags is not trivial: reading the host's flags register with a + * feclearexcept & fetestexcept pair is slow [slightly slower than soft-fp], + * and trapping on every FP exception is not fast nor pleasant to work with. + * + * We address these challenges by leverage the host FPU for a subset of the + * operations. To do this we follow the main idea presented in this paper: + * + * Guo, Yu-Chuan, et al. "Translating the ARM Neon and VFP instructions in a + * binary translator." Software: Practice and Experience 46.12 (2016):1591-1615. + * + * The idea is thus to leverage the host FPU to (1) compute FP operations + * and (2) identify whether FP exceptions occurred while avoiding + * expensive exception flag register accesses. + * + * An important optimization shown in the paper is that given that exception + * flags are rarely cleared by the guest, we can avoid recomputing some flags. + * This is particularly useful for the inexact flag, which is very frequently + * raised in floating-point workloads. + * + * We optimize the code further by deferring to soft-fp whenever FP exception + * detection might get hairy. Two examples: (1) when at least one operand is + * denormal/inf/NaN; (2) when operands are not guaranteed to lead to a 0 result + * and the result is < the minimum normal. + */ +#define GEN_TYPE_CONV(name, to_t, from_t) \ +static inline to_t name(from_t a) \ +{ \ +to_t r = *(to_t *)&a; \ +return r; \ +} + +GEN_TYPE_CONV(float32_to_float, float, float32) +GEN_TYPE_CONV(float64_to_double, double, float64) +GEN_TYPE_CONV(float_to_float32, float32, float) +GEN_TYPE_CONV(double_to_float64, float64, double) +#undef GEN_TYPE_CONV + +#define GEN_INPUT_FLUSH__NOCHECK(name, soft_t) \ +static inline void name(soft_t *a, float_status *s) \ +{ \ +if (unlikely(soft_t ## _is_denormal(*a))) { \ +*a = soft_t ## _set_sign(soft_t ## _zero, \ + soft_t ## _is_neg(*a));\ +s->float_exception_flags |= float_flag_input_denormal; \ +} \ +} + +GEN_INPUT_FLUSH__NOCHECK(float32_input_flush__nocheck, float32) +GEN_INPUT_FLUSH__NOCHECK(float64_input_flush__nocheck, float64) +#undef GEN_INPUT_FLUSH__NOCHECK + +#define GEN_INPUT_FLUSH1(name, soft_t) \ +static inline void name(soft_t *a, float
[Qemu-devel] [PATCH v3 08/15] softfloat: add float{32, 64}_is_zero_or_normal
These will gain some users very soon. Signed-off-by: Emilio G. Cota --- include/fpu/softfloat.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h index a8512fb..66985e1 100644 --- a/include/fpu/softfloat.h +++ b/include/fpu/softfloat.h @@ -422,6 +422,11 @@ static inline bool float32_is_denormal(float32 a) return float32_is_zero_or_denormal(a) && !float32_is_zero(a); } +static inline bool float32_is_zero_or_normal(float32 a) +{ +return float32_is_normal(a) || float32_is_zero(a); +} + static inline float32 float32_set_sign(float32 a, int sign) { return make_float32((float32_val(a) & 0x7fff) | (sign << 31)); @@ -561,6 +566,11 @@ static inline bool float64_is_denormal(float64 a) return float64_is_zero_or_denormal(a) && !float64_is_zero(a); } +static inline bool float64_is_zero_or_normal(float64 a) +{ +return float64_is_normal(a) || float64_is_zero(a); +} + static inline float64 float64_set_sign(float64 a, int sign) { return make_float64((float64_val(a) & 0x7fffULL) -- 2.7.4
Re: [Qemu-devel] [PATCH v3 14/15] hardfloat: support float32/64 square root
On Wed, Apr 04, 2018 at 19:11:14 -0400, Emilio G. Cota wrote: (snip) > +if (likely((soft_t ## _is_normal(a) || soft_t ## _is_zero(a)) && \ Updated on the github tree to: -if (likely((soft_t ## _is_normal(a) || soft_t ## _is_zero(a)) && \ +if (likely(soft_t ## _is_zero_or_normal(a) && \ All the other patches in this series are identical to the v3 on github. Apologies for the last-minute mismatch on my end. Emilio
[Qemu-devel] [PATCH v3 12/15] hardfloat: support float32/64 division
Performance results for fp-bench: 1. Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz - before: div-single: 34.84 MFlops div-double: 34.04 MFlops - after: div-single: 275.23 MFlops div-double: 216.38 MFlops 2. ARM Aarch64 A57 @ 2.4GHz - before: div-single: 9.33 MFlops div-double: 9.30 MFlops - after: div-single: 51.55 MFlops div-double: 15.09 MFlops 3. IBM POWER8E @ 2.1 GHz - before: div-single: 25.65 MFlops div-double: 24.91 MFlops - after: div-single: 96.83 MFlops div-double: 31.01 MFlops Here setting 2FP64_USE_FP to 1 pays off for x86_64: [1] 215.97 vs [0] 62.15 MFlops Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 88 +++-- 1 file changed, 86 insertions(+), 2 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 2c68b9d..4323dc2 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1666,7 +1666,8 @@ float16 float16_div(float16 a, float16 b, float_status *status) return float16_round_pack_canonical(pr, status); } -float32 float32_div(float32 a, float32 b, float_status *status) +static float32 QEMU_SOFTFLOAT_ATTR +soft_float32_div(float32 a, float32 b, float_status *status) { FloatParts pa = float32_unpack_canonical(a, status); FloatParts pb = float32_unpack_canonical(b, status); @@ -1675,7 +1676,8 @@ float32 float32_div(float32 a, float32 b, float_status *status) return float32_round_pack_canonical(pr, status); } -float64 float64_div(float64 a, float64 b, float_status *status) +static float64 QEMU_SOFTFLOAT_ATTR +soft_float64_div(float64 a, float64 b, float_status *status) { FloatParts pa = float64_unpack_canonical(a, status); FloatParts pb = float64_unpack_canonical(b, status); @@ -1684,6 +1686,88 @@ float64 float64_div(float64 a, float64 b, float_status *status) return float64_round_pack_canonical(pr, status); } +static float float_div(float a, float b) +{ +return a / b; +} + +static double double_div(double a, double b) +{ +return a / b; +} + +static bool f32_div_pre(float32 a, float32 b, const struct float_status *s) +{ +return likely(float32_is_zero_or_normal(a) && + float32_is_normal(b) && + can_use_fpu(s)); +} + +static bool f64_div_pre(float64 a, float64 b, const struct float_status *s) +{ +return likely(float64_is_zero_or_normal(a) && + float64_is_normal(b) && + can_use_fpu(s)); +} + +static bool float_div_pre(float a, float b, const struct float_status *s) +{ +return likely((fpclassify(a) == FP_NORMAL || fpclassify(a) == FP_ZERO) && + fpclassify(b) == FP_NORMAL && + can_use_fpu(s)); +} + +static bool double_div_pre(double a, double b, const struct float_status *s) +{ +return likely((fpclassify(a) == FP_NORMAL || fpclassify(a) == FP_ZERO) && + fpclassify(b) == FP_NORMAL && + can_use_fpu(s)); +} + +static bool f32_div_post(float32 a, float32 b, const struct float_status *s) +{ +return !float32_is_zero(a); +} + +static bool f64_div_post(float64 a, float64 b, const struct float_status *s) +{ +return !float64_is_zero(a); +} + +static bool float_div_post(float a, float b, const struct float_status *s) +{ +return fpclassify(a) != FP_ZERO; +} + +static bool double_div_post(double a, double b, const struct float_status *s) +{ +return fpclassify(a) != FP_ZERO; +} + +float32 __attribute__((flatten)) +float32_div(float32 a, float32 b, float_status *s) +{ +if (QEMU_HARDFLOAT_2F32_USE_FP) { +return float_gen2(a, b, s, float_div, soft_float32_div, float_div_pre, + float_div_post, NULL, NULL); +} else { +return f32_gen2(a, b, s, float_div, soft_float32_div, f32_div_pre, +f32_div_post, NULL, NULL); +} +} + +float64 __attribute__((flatten)) +float64_div(float64 a, float64 b, float_status *s) +{ +if (QEMU_HARDFLOAT_2F64_USE_FP) { +return double_gen2(a, b, s, double_div, soft_float64_div, + double_div_pre, double_div_post, NULL, NULL); +} else { +return f64_gen2(a, b, s, double_div, soft_float64_div, f64_div_pre, +f64_div_post, NULL, NULL); +} +} + /* * Rounds the floating-point value `a' to an integer, and returns the * result as a floating-point value. The operation is performed -- 2.7.4
[Qemu-devel] [PATCH v3 06/15] tests/fp: add fp-bench, a collection of simple floating point microbenchmarks
This will allow us to measure the performance impact of FP emulation optimizations. Note that we can measure both directly the impact on the softfloat functions (with "-t soft"), or the impact on an emulated workload (call with "-t host" and run under qemu user-mode). Signed-off-by: Emilio G. Cota --- tests/fp/fp-bench.c | 528 tests/fp/.gitignore | 1 + tests/fp/Makefile | 4 +- 3 files changed, 532 insertions(+), 1 deletion(-) create mode 100644 tests/fp/fp-bench.c diff --git a/tests/fp/fp-bench.c b/tests/fp/fp-bench.c new file mode 100644 index 000..a012b78 --- /dev/null +++ b/tests/fp/fp-bench.c @@ -0,0 +1,528 @@ +/* + * fp-bench.c - A collection of simple floating point microbenchmarks. + * + * Copyright (C) 2018, Emilio G. Cota + * + * License: GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#ifndef HW_POISON_H +#error Must define HW_POISON_H to work around TARGET_* poisoning +#endif + +#include "qemu/osdep.h" +#include "qemu/timer.h" + +#include "fpu/softfloat.h" + +#include + +/* amortize the computation of random inputs */ +#define OPS_PER_ITER 5 + +#define MAX_OPERANDS 3 + +#define SEED_A 0xdeadfacedeadface +#define SEED_B 0xbadc0feebadc0fee +#define SEED_C 0xbeefdeadbeefdead + +enum op { +OP_ADD, +OP_SUB, +OP_MUL, +OP_DIV, +OP_FMA, +OP_SQRT, +OP_CMP, +OP_MAX_NR, +}; + +static const char * const op_names[] = { +[OP_ADD] = "add", +[OP_SUB] = "sub", +[OP_MUL] = "mul", +[OP_DIV] = "div", +[OP_FMA] = "fma", +[OP_SQRT] = "sqrt", +[OP_CMP] = "cmp", +[OP_MAX_NR] = NULL, +}; + +enum precision { +PREC_SINGLE, +PREC_DOUBLE, +PREC_FLOAT32, +PREC_FLOAT64, +PREC_MAX_NR, +}; + +enum tester { +TESTER_SOFT, +TESTER_HOST, +TESTER_MAX_NR, +}; + +static const char * const tester_names[] = { +[TESTER_SOFT] = "soft", +[TESTER_HOST] = "host", +[TESTER_MAX_NR] = NULL, +}; + +union fp { +float f; +double d; +float32 f32; +float64 f64; +uint64_t u64; +}; + +struct op_state; + +typedef float (*float_func_t)(const struct op_state *s); +typedef double (*double_func_t)(const struct op_state *s); + +union fp_func { +float_func_t float_func; +double_func_t double_func; +}; + +typedef void (*bench_func_t)(void); + +struct op_desc { +const char * const name; +}; + +#define DEFAULT_DURATION_SECS 1 + +static uint64_t random_ops[MAX_OPERANDS] = { +SEED_A, SEED_B, SEED_C, +}; +static float_status soft_status; +static enum precision precision; +static enum op operation; +static enum tester tester; +static uint64_t n_completed_ops; +static unsigned int duration = DEFAULT_DURATION_SECS; +static int64_t ns_elapsed; +/* disable optimizations with volatile */ +static volatile union fp res; + +/* + * From: https://en.wikipedia.org/wiki/Xorshift + * This is faster than rand_r(), and gives us a wider range (RAND_MAX is only + * guaranteed to be >= INT_MAX). + */ +static uint64_t xorshift64star(uint64_t x) +{ +x ^= x >> 12; /* a */ +x ^= x << 25; /* b */ +x ^= x >> 27; /* c */ +return x * UINT64_C(2685821657736338717); +} + +static void update_random_ops(int n_ops, enum precision prec) +{ +int i; + +for (i = 0; i < n_ops; i++) { +uint64_t r = random_ops[i]; + +if (prec == PREC_SINGLE || PREC_FLOAT32) { +do { +r = xorshift64star(r); +} while (!float32_is_normal(r)); +} else if (prec == PREC_DOUBLE || PREC_FLOAT64) { +do { +r = xorshift64star(r); +} while (!float64_is_normal(r)); +} else { +g_assert_not_reached(); +} +random_ops[i] = r; +} +} + +static void fill_random(union fp *ops, int n_ops, enum precision prec, +bool no_neg) +{ +int i; + +for (i = 0; i < n_ops; i++) { +switch (prec) { +case PREC_SINGLE: +case PREC_FLOAT32: +ops[i].f32 = make_float32(random_ops[i]); +if (no_neg && float32_is_neg(ops[i].f32)) { +ops[i].f32 = float32_chs(ops[i].f32); +} +/* raise the exponent to limit the frequency of denormal results */ +ops[i].f32 |= 0x4000; +break; +case PREC_DOUBLE: +case PREC_FLOAT64: +ops[i].f64 = make_float64(random_ops[i]); +if (no_neg && float64_is_neg(ops[i].f64)) { +ops[i].f64 = float64_chs(ops[i].f64); +} +/* raise the exponent to limit the frequency of denormal results */ +ops[i].f64 |= LIT64(0x4000); +break; +default: +g_assert_not_reached(); +} +} +} + +/* + * The main benchmark function. Instead of (ab)using macros, we rely + * on the compiler to unfold this at compile-time. + */ +static void bench(enum precision
[Qemu-devel] [PATCH v3 11/15] hardfloat: support float32/64 multiplication
Performance results for fp-bench: 1. Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz - before: mul-single: 126.91 MFlops mul-double: 118.28 MFlops - after: mul-single: 258.02 MFlops mul-double: 197.96 MFlops 2. ARM Aarch64 A57 @ 2.4GHz - before: mul-single: 37.42 MFlops mul-double: 38.77 MFlops - after: mul-single: 73.41 MFlops mul-double: 76.93 MFlops 3. IBM POWER8E @ 2.1 GHz - before: mul-single: 58.40 MFlops mul-double: 59.33 MFlops - after: mul-single: 60.25 MFlops mul-double: 94.79 MFlops Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 66 + 1 file changed, 62 insertions(+), 4 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index ca0b8ab..2c68b9d 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1281,8 +1281,8 @@ float16 __attribute__((flatten)) float16_mul(float16 a, float16 b, return float16_round_pack_canonical(pr, status); } -float32 __attribute__((flatten)) float32_mul(float32 a, float32 b, - float_status *status) +static float32 QEMU_SOFTFLOAT_ATTR +soft_float32_mul(float32 a, float32 b, float_status *status) { FloatParts pa = float32_unpack_canonical(a, status); FloatParts pb = float32_unpack_canonical(b, status); @@ -1291,8 +1291,8 @@ float32 __attribute__((flatten)) float32_mul(float32 a, float32 b, return float32_round_pack_canonical(pr, status); } -float64 __attribute__((flatten)) float64_mul(float64 a, float64 b, - float_status *status) +static float64 QEMU_SOFTFLOAT_ATTR +soft_float64_mul(float64 a, float64 b, float_status *status) { FloatParts pa = float64_unpack_canonical(a, status); FloatParts pb = float64_unpack_canonical(b, status); @@ -1301,6 +1301,64 @@ float64 __attribute__((flatten)) float64_mul(float64 a, float64 b, return float64_round_pack_canonical(pr, status); } +static float float_mul(float a, float b) +{ +return a * b; +} + +static double double_mul(double a, double b) +{ +return a * b; +} + +static bool f32_mul_fast(float32 a, float32 b, const struct float_status *s) +{ +return float32_is_zero(a) || float32_is_zero(b); +} + +static bool f64_mul_fast(float64 a, float64 b, const struct float_status *s) +{ +return float64_is_zero(a) || float64_is_zero(b); +} + +static float32 f32_mul_fast_op(float32 a, float32 b, float_status *s) +{ +bool signbit = float32_is_neg(a) ^ float32_is_neg(b); + +return float32_set_sign(float32_zero, signbit); +} + +static float64 f64_mul_fast_op(float64 a, float64 b, float_status *s) +{ +bool signbit = float64_is_neg(a) ^ float64_is_neg(b); + +return float64_set_sign(float64_zero, signbit); +} + +float32 __attribute__((flatten)) +float32_mul(float32 a, float32 b, float_status *s) +{ +if (QEMU_HARDFLOAT_2F32_USE_FP) { +return float_gen2(a, b, s, float_mul, soft_float32_mul, float_is_zon2, + NULL, f32_mul_fast, f32_mul_fast_op); +} else { +return f32_gen2(a, b, s, float_mul, soft_float32_mul, f32_is_zon2, NULL, +f32_mul_fast, f32_mul_fast_op); +} +} + +float64 __attribute__((flatten)) +float64_mul(float64 a, float64 b, float_status *s) +{ +if (QEMU_HARDFLOAT_2F64_USE_FP) { +return double_gen2(a, b, s, double_mul, soft_float64_mul, + double_is_zon2, NULL, f64_mul_fast, f64_mul_fast_op); +} else { +return f64_gen2(a, b, s, double_mul, soft_float64_mul, f64_is_zon2, +NULL, f64_mul_fast, f64_mul_fast_op); +} +} + /* * Returns the result of multiplying the floating-point values `a' and * `b' then adding 'c', with no intermediate rounding step after the -- 2.7.4
[Qemu-devel] [PATCH v3 15/15] hardfloat: support float32/64 comparison
Performance results for fp-bench: 1. Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz - before: cmp-single: 113.01 MFlops cmp-double: 115.54 MFlops - after: cmp-single: 527.83 MFlops cmp-double: 457.21 MFlops 2. ARM Aarch64 A57 @ 2.4GHz - before: cmp-single: 39.32 MFlops cmp-double: 39.80 MFlops - after: cmp-single: 162.74 MFlops cmp-double: 167.08 MFlops 3. IBM POWER8E @ 2.1 GHz - before: cmp-single: 60.81 MFlops cmp-double: 62.76 MFlops - after: cmp-single: 235.39 MFlops cmp-double: 283.44 MFlops Here using float{32,64}_is_any_nan is faster than using isnan for all machines. On x86_64 the perf difference is just a few percentage points, but on aarch64 we go from 117/119 to 164/169 MFlops for single/double precision, respectively. Aggregate performance improvement for the last few patches: [ all charts in png: https://imgur.com/a/4yV8p ] 1. Host: Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz qemu-aarch64 NBench score; higher is better Host: Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz 16 +-+---+-+===---+---===---+---+-+ 14 +-+..@@@&&.=...@@@&&.=...+-+ 12 +-+..@.@.&.=...@.@.&.=.+befor=== +-+ 10 +-+..@.@.&.=...@.@.&.=.+ad@@&& = +-+ 8 +-+...$$$%.@.&.=...@.@.&.=.+ @@u& = +-+ 6 +-+@@@&&=+***##.$%.@.&.=***##$$%+@.&.=..###$$%%@i& = +-+ 4 +-+...###$%%.@.&=.*.*.#.$%.@.&.=*.*.#.$%.@.&.=+**.#+$ +@m& = +-+ 2 +-+.***.#$.%.@.&=.*.*.#.$%.@.&.=*.*.#.$%.@.&.=.**.#+$+sqr& = +-+ 0 +-+-***##$%%@@&&=-***##$$%@@&&==***##$$%@@&&==-**##$$%+cmp==-+-+ FOURIERNEURAL NELU DECOMPOSITION gmean qemu-aarch64 SPEC06fp (test set) speedup over QEMU 4c2c1015905 Host: Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz error bars: 95% confidence interval 4.5 +-+---+-++-+-+-&---+-++-+-+-++-+-+-+-++-+---+-+ 4 +-+..+@@+...+-+ 3.5 +-+..%%@&.@@..%%@&+++dsub +-+ 2.5 +-+&&+...%%@&...+%%@..+%%&+..@@&+.%%@&+%%&+.+%@&++%%@& +-+ 2 +-+..+%%&..+%@&+.%%@&...+++..%%@...%%&.+$$@&..%%@&..%%@&...+%%&+.%%@&+..+%%@&.+%%&++$$@&++d%@& %%@&+-+ 1.5 +-+**#$%&**#$@&**#%@&**$%@**#$%@**#$%&**#$@&**$%@&*#$%@**#$%@**#$%&**#%@&**$%@&*#$%@**#$%&**#$@&*+f%@&**$%@&+-+ 0.5 +-+**#$%&**#$@&**#%@&**$%@**#$%@**#$%&**#$@&**$%@&*#$%@**#$%@**#$%&**#%@&**$%@&*#$%@**#$%&**#$@&+sqr@&**$%@&+-+ 0 +-+**#$%&**#$@&**#%@&**$%@**#$%@**#$%&**#$@&**$%@&*#$%@**#$%@**#$%&**#%@&**$%@&*#$%@**#$%&**#$@&*+cmp&**$%@&+-+ 410.bw416.gam433.434.z435.436.cac437.lesli444.447.de450.so453454.ca459.GemsF465.tont470.lb4482.sphinxgeomean 2. Host: ARM Aarch64 A57 @ 2.4GHz qemu-aarch64 NBench score; higher is better Host: Applied Micro X-Gene, Aarch64 A57 @ 2.4 GHz 5 +-+---+-+-+-+---+-+ 4.5 +-+@@@&==...+-+ 3 4 +-+..@@@&==@.@&.=.+before +-+ 3 +-+..@.@&.=@.@&.=.+ad@@@&== +-+ 2.5 +-+.##$$%%.@&.=@.@&.=.+ @m@& = +-+ 2 +-+@@@&==.***#.$.%.@&.=.***#$$%%.@&.=.***#$$%%d@& = +-+ 1.5 +-+.***#$$%%.@&.=.*.*#.$.%.@&.=.*.*#.$.%.@&.=.*.*#+$ +f@& = +-+ 0.5 +-+.*.*#.$.%.@&.=.*.*#.$.%.@&.=.*.*#.$.%.@&.=.*.*#+$+sqr& = +-+ 0 +-+-***#$$%%@@&==-***#$$%%@@&==-***#$$%%@@&==-***#$$%+cmp==-+-+ FOURIERNEURAL NLU DECOMPOSITION gmean Note that by not inlining the soft-fp primitives we end up with a smaller softfloat.o--in particular, see the difference for the softfloat.o built for fp-bench: - before this series: textdata bss dec hex filename 103235 0 0 103235 19343 softfloat.o - after: textdata bss dec hex filename 93369 0 0 93369 16cb9 softfloat.o Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 74 ++--- 1 file changed, 60 insertions(+), 14 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 5434d29..459dd87 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -2581,28 +2581,74 @@ static int compare_floats(FloatParts a, FloatParts b, bool is_quiet, } } -#define COMPARE(sz) \ -int float ## sz ## _compare(float ## sz a, float ## sz b, \ -
[Qemu-devel] [PATCH v3 05/15] target/tricore: use float32_is_denormal
Cc: Bastian Koppelmann Signed-off-by: Emilio G. Cota --- target/tricore/fpu_helper.c | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/target/tricore/fpu_helper.c b/target/tricore/fpu_helper.c index df16290..31df462 100644 --- a/target/tricore/fpu_helper.c +++ b/target/tricore/fpu_helper.c @@ -44,11 +44,6 @@ static inline uint8_t f_get_excp_flags(CPUTriCoreState *env) | float_flag_inexact); } -static inline bool f_is_denormal(float32 arg) -{ -return float32_is_zero_or_denormal(arg) && !float32_is_zero(arg); -} - static inline float32 f_maddsub_nan_result(float32 arg1, float32 arg2, float32 arg3, float32 result, uint32_t muladd_negate_c) @@ -260,8 +255,8 @@ uint32_t helper_fcmp(CPUTriCoreState *env, uint32_t r1, uint32_t r2) set_flush_inputs_to_zero(0, &env->fp_status); result = 1 << (float32_compare_quiet(arg1, arg2, &env->fp_status) + 1); -result |= f_is_denormal(arg1) << 4; -result |= f_is_denormal(arg2) << 5; +result |= float32_is_denormal(arg1) << 4; +result |= float32_is_denormal(arg2) << 5; flags = f_get_excp_flags(env); if (flags) { -- 2.7.4
[Qemu-devel] [PATCH v3 07/15] softfloat: rename canonicalize to sf_canonicalize
glibc >= 2.25 defines canonicalize in commit eaf5ad0 (Add canonicalize, canonicalizef, canonicalizel., 2016-10-26). Given that we'll be including soon, prepare for this by prefixing our canonicalize() with sf_ to avoid clashing with the libc's canonicalize(). Reported-by: Bastian Koppelmann Cc: Bastian Koppelmann Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 6803279..c3b9d07 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -323,8 +323,8 @@ static inline float64 float64_pack_raw(FloatParts p) } /* Canonicalize EXP and FRAC, setting CLS. */ -static FloatParts canonicalize(FloatParts part, const FloatFmt *parm, - float_status *status) +static FloatParts sf_canonicalize(FloatParts part, const FloatFmt *parm, + float_status *status) { if (part.exp == parm->exp_max) { if (part.frac == 0) { @@ -494,7 +494,7 @@ static FloatParts round_canonical(FloatParts p, float_status *s, static FloatParts float16_unpack_canonical(float16 f, float_status *s) { -return canonicalize(float16_unpack_raw(f), &float16_params, s); +return sf_canonicalize(float16_unpack_raw(f), &float16_params, s); } static float16 float16_round_pack_canonical(FloatParts p, float_status *s) @@ -512,7 +512,7 @@ static float16 float16_round_pack_canonical(FloatParts p, float_status *s) static FloatParts float32_unpack_canonical(float32 f, float_status *s) { -return canonicalize(float32_unpack_raw(f), &float32_params, s); +return sf_canonicalize(float32_unpack_raw(f), &float32_params, s); } static float32 float32_round_pack_canonical(FloatParts p, float_status *s) @@ -530,7 +530,7 @@ static float32 float32_round_pack_canonical(FloatParts p, float_status *s) static FloatParts float64_unpack_canonical(float64 f, float_status *s) { -return canonicalize(float64_unpack_raw(f), &float64_params, s); +return sf_canonicalize(float64_unpack_raw(f), &float64_params, s); } static float64 float64_round_pack_canonical(FloatParts p, float_status *s) -- 2.7.4
[Qemu-devel] [PATCH v3 14/15] hardfloat: support float32/64 square root
Performance results for fp-bench: 1. Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz - before: sqrt-single: 43.27 MFlops sqrt-double: 24.81 MFlops - after: sqrt-single: 297.94 MFlops sqrt-double: 210.46 MFlops 2. ARM Aarch64 A57 @ 2.4GHz - before: sqrt-single: 12.41 MFlops sqrt-double: 6.22 MFlops - after: sqrt-single: 55.58 MFlops sqrt-double: 40.63 MFlops 3. IBM POWER8E @ 2.1 GHz - before: sqrt-single: 17.01 MFlops sqrt-double: 9.61 MFlops - after: sqrt-single: 104.17 MFlops sqrt-double: 133.32 MFlops Here none of the machines got faster from enabling USE_FP. For instance, on x86_64 sqrt is 23% slower for single precision, with it enabled, and 17% slower for double precision. Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 73 +++-- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index ce14c87..5434d29 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -2717,20 +2717,89 @@ float16 __attribute__((flatten)) float16_sqrt(float16 a, float_status *status) return float16_round_pack_canonical(pr, status); } -float32 __attribute__((flatten)) float32_sqrt(float32 a, float_status *status) +static float32 QEMU_SOFTFLOAT_ATTR +soft_float32_sqrt(float32 a, float_status *status) { FloatParts pa = float32_unpack_canonical(a, status); FloatParts pr = sqrt_float(pa, status, &float32_params); return float32_round_pack_canonical(pr, status); } -float64 __attribute__((flatten)) float64_sqrt(float64 a, float_status *status) +static float64 QEMU_SOFTFLOAT_ATTR +soft_float64_sqrt(float64 a, float_status *status) { FloatParts pa = float64_unpack_canonical(a, status); FloatParts pr = sqrt_float(pa, status, &float64_params); return float64_round_pack_canonical(pr, status); } +#define GEN_SQRT_SF(name, soft_t, host_t, host_sqrt_func) \ +static soft_t name(soft_t a, float_status *s) \ +{ \ +if (QEMU_NO_HARDFLOAT) {\ +goto soft; \ +} \ +soft_t ## _input_flush1(&a, s); \ +if (likely((soft_t ## _is_normal(a) || soft_t ## _is_zero(a)) && \ + !soft_t ## _is_neg(a) && \ + can_use_fpu(s))) { \ +host_t ha = soft_t ## _to_ ## host_t(a);\ +host_t hr = host_sqrt_func(ha); \ +\ +return host_t ## _to_ ## soft_t(hr);\ +} \ +soft: \ +return soft_ ## soft_t ## _sqrt(a, s); \ +} + +#define GEN_SQRT_FP(name, soft_t, host_t, host_sqrt_func) \ +static soft_t name(soft_t a, float_status *s) \ +{ \ +host_t ha; \ +\ +if (QEMU_NO_HARDFLOAT) {\ +goto soft; \ +} \ +soft_t ## _input_flush1(&a, s); \ +ha = soft_t ## _to_ ## host_t(a); \ +if (likely((fpclassify(ha) == FP_NORMAL || \ +fpclassify(ha) == FP_ZERO) && \ + !signbit(ha) && \ + can_use_fpu(s))) { \ +host_t hr = host_sqrt_func(ha); \ +\ +return host_t ## _to_ ## soft_t(hr);\ +} \ +soft: \ +return soft_ ## soft_t ## _sqrt(a, s); \ +} + +GEN_SQRT_SF(f32_sqrt, float32, float, sqrtf) +GEN_SQRT_SF(f64_sqrt, float64, double, sqrt) +#undef GEN_SQRT_SF + +GEN_SQRT_FP(float_sqrt, float32, float, sqrtf) +GEN_SQRT_FP(double_sqrt, float64, double, sqrt) +#undef GEN_SQRT_FP + +float32 __attribute__((flatten)) float32_sqrt(float32 a, float_status *s) +{ +if (QEMU_HARDFLOAT_1F32_USE_FP) { +return floa
[Qemu-devel] [PATCH v3 04/15] softfloat: add float{32, 64}_is_{de, }normal
This paves the way for upcoming work. Cc: Bastian Koppelmann Reviewed-by: Alex Bennée Signed-off-by: Emilio G. Cota --- include/fpu/softfloat.h | 20 1 file changed, 20 insertions(+) diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h index 36626a5..a8512fb 100644 --- a/include/fpu/softfloat.h +++ b/include/fpu/softfloat.h @@ -412,6 +412,16 @@ static inline int float32_is_zero_or_denormal(float32 a) return (float32_val(a) & 0x7f80) == 0; } +static inline bool float32_is_normal(float32 a) +{ +return ((float32_val(a) + 0x0080) & 0x7fff) >= 0x0100; +} + +static inline bool float32_is_denormal(float32 a) +{ +return float32_is_zero_or_denormal(a) && !float32_is_zero(a); +} + static inline float32 float32_set_sign(float32 a, int sign) { return make_float32((float32_val(a) & 0x7fff) | (sign << 31)); @@ -541,6 +551,16 @@ static inline int float64_is_zero_or_denormal(float64 a) return (float64_val(a) & 0x7ff0LL) == 0; } +static inline bool float64_is_normal(float64 a) +{ +return ((float64_val(a) + (1ULL << 52)) & -1ULL >> 1) >= 1ULL << 53; +} + +static inline bool float64_is_denormal(float64 a) +{ +return float64_is_zero_or_denormal(a) && !float64_is_zero(a); +} + static inline float64 float64_set_sign(float64 a, int sign) { return make_float64((float64_val(a) & 0x7fffULL) -- 2.7.4
[Qemu-devel] [PATCH v3 03/15] fp-test: add muladd variants
These are a few muladd-related operations that the original IBM syntax does not specify; model files for these are in muladd.fptest. Signed-off-by: Emilio G. Cota --- tests/fp/fp-test.c | 24 tests/fp/muladd.fptest | 51 ++ 2 files changed, 75 insertions(+) create mode 100644 tests/fp/muladd.fptest diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c index 27637c4..2200d40 100644 --- a/tests/fp/fp-test.c +++ b/tests/fp/fp-test.c @@ -53,6 +53,9 @@ enum op { OP_SUB, OP_MUL, OP_MULADD, +OP_MULADD_NEG_ADDEND, +OP_MULADD_NEG_PRODUCT, +OP_MULADD_NEG_RESULT, OP_DIV, OP_SQRT, OP_MINNUM, @@ -69,6 +72,9 @@ static const struct op_desc ops[] = { [OP_SUB] = { "-", 2 }, [OP_MUL] = { "*", 2 }, [OP_MULADD] ={ "*+", 3 }, +[OP_MULADD_NEG_ADDEND] = { "*+nc", 3 }, +[OP_MULADD_NEG_PRODUCT] = { "*+np", 3 }, +[OP_MULADD_NEG_RESULT] = { "*+nr", 3 }, [OP_DIV] = { "/", 2 }, [OP_SQRT] = { "V", 1 }, [OP_MINNUM] ={ " Q i +b32*+nc =0 -1.7FP127 -Inf +Inf -> Q i +b32*+nc =0 -1.6C9AE7P113 -Inf +Inf -> Q i +b32*+nc =0 -1.00P-126 -Inf +Inf -> Q i +b32*+nc =0 -0.7FP-126 -Inf +Inf -> Q i +b32*+nc =0 -0.1B977AP-126 -Inf +Inf -> Q i +b32*+nc =0 -0.01P-126 -Inf +Inf -> Q i +b32*+nc =0 -1.00P0 -Inf +Inf -> Q i +b32*+nc =0 -Zero -Inf +Inf -> Q i +b32*+nc =0 +Zero -Inf +Inf -> Q i +b32*+nc =0 -Zero -1.00P-126 +1.7FP127 -> -1.7FP127 +b32*+nc =0 +Zero -1.00P-126 +1.7FP127 -> -1.7FP127 +b32*+nc =0 -1.00P-126 -1.7FP127 -1.4B9156P109 -> +1.4B9156P109 x +b32*+nc =0 -0.7FP-126 -1.7FP127 -1.51BA59P-113 -> +1.7DP1 x +b32*+nc =0 -0.3D6B57P-126 -1.7FP127 -1.265398P-67 -> +1.75AD5BP0 x +b32*+nc =0 -0.01P-126 -1.7FP127 -1.677330P-113 -> +1.7FP-22 x + +# np == negate product +b32*+np =0 +Inf -Inf -Inf -> Q i +b32*+np =0 +1.7FP127 -Inf -Inf -> Q i +b32*+np =0 +1.6C9AE7P113 -Inf -Inf -> Q i +b32*+np =0 +1.00P-126 -Inf -Inf -> Q i +b32*+np =0 +0.7FP-126 -Inf -Inf -> Q i +b32*+np =0 +0.1B977AP-126 -Inf -Inf -> Q i +b32*+np =0 +0.01P-126 -Inf -Inf -> Q i +b32*+np =0 +1.00P0 -Inf -Inf -> Q i +b32*+np =0 +Zero -Inf -Inf -> Q i +b32*+np =0 +Zero -Inf -Inf -> Q i +b32*+np =0 -Zero -1.00P-126 -1.7FP127 -> -1.7FP127 +b32*+np =0 +Zero -1.00P-126 -1.7FP127 -> -1.7FP127 +b32*+np =0 -1.3A6A89P-18 +1.24E7AEP9 -0.7FP-126 -> +1.7029E9P-9 x + +# nr == negate result +b32*+nr =0 -Inf -Inf -Inf -> Q i +b32*+nr =0 -1.7FP127 -Inf -Inf -> Q i +b32*+nr =0 -1.6C9AE7P113 -Inf -Inf -> Q i +b32*+nr =0 -1.00P-126 -Inf -Inf -> Q i +b32*+nr =0 -0.7FP-126 -Inf -Inf -> Q i +b32*+nr =0 -0.1B977AP-126 -Inf -Inf -> Q i +b32*+nr =0 -0.01P-126 -Inf -Inf -> Q i +b32*+nr =0 -1.00P0 -Inf -Inf -> Q i +b32*+nr =0 -Zero -Inf -Inf -> Q i +b32*+nr =0 -Zero -Inf -Inf -> Q i +b32*+nr =0 +Zero -1.00P-126 -1.7FP127 -> +1.7FP127 +b32*+nr =0 -Zero -1.00P-126 -1.7FP127 -> +1.7FP127 +b32*+nr =0 -1.00P-126 -1.7FP127 -1.4B9156P109 -> +1.4B9156P109 x +b32*+nr =0 -0.7FP-126 -1.7FP127 -1.51BA59P-113 -> -1.7DP1 x +b32*+nr =0 -0.3D6B57P-126 -1.7FP127 -1.265398P-67 -> -1.75AD5BP0 x +b32*+nr =0 -0.01P-126 -1.7FP127 -1.677330P-113 -> -1.7FP-22 x +b32*+nr =0 +1.72E53AP-33 -1.7FP127 -1.5AA684P-2 -> +1.72E539P95 x -- 2.7.4
[Qemu-devel] [PATCH v3 10/15] hardfloat: support float32/64 addition and subtraction
Performance results (single and double precision) for fp-bench: 1. Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz - before: add-single: 135.07 MFlops add-double: 131.60 MFlops sub-single: 130.04 MFlops sub-double: 133.01 MFlops - after: add-single: 443.04 MFlops add-double: 301.95 MFlops sub-single: 411.36 MFlops sub-double: 293.15 MFlops 2. ARM Aarch64 A57 @ 2.4GHz - before: add-single: 44.79 MFlops add-double: 49.20 MFlops sub-single: 44.55 MFlops sub-double: 49.06 MFlops - after: add-single: 93.28 MFlops add-double: 88.27 MFlops sub-single: 91.47 MFlops sub-double: 88.27 MFlops 3. IBM POWER8E @ 2.1 GHz - before: add-single: 72.59 MFlops add-double: 72.27 MFlops sub-single: 75.33 MFlops sub-double: 70.54 MFlops - after: add-single: 112.95 MFlops add-double: 201.11 MFlops sub-single: 116.80 MFlops sub-double: 188.72 MFlops Note that the IBM and ARM machines benefit from having HARDFLOAT_2F{32,64}_USE_FP set to 0. Otherwise their performance can suffer significantly: - IBM Power8: add-single: [1] 54.94 vs [0] 116.37 MFlops add-double: [1] 58.92 vs [0] 201.44 MFlops - Aarch64 A57: add-single: [1] 80.72 vs [0] 93.24 MFlops add-double: [1] 82.10 vs [0] 88.18 MFlops On the Intel machine, having 2F64 set to 1 pays off, but it doesn't for 2F32: - Intel i7-6700K: add-single: [1] 285.79 vs [0] 426.70 MFlops add-double: [1] 302.15 vs [0] 278.82 MFlops Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 106 +++- 1 file changed, 98 insertions(+), 8 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 956b938..ca0b8ab 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1080,8 +1080,8 @@ float16 __attribute__((flatten)) float16_add(float16 a, float16 b, return float16_round_pack_canonical(pr, status); } -float32 __attribute__((flatten)) float32_add(float32 a, float32 b, - float_status *status) +static float32 QEMU_SOFTFLOAT_ATTR +soft_float32_add(float32 a, float32 b, float_status *status) { FloatParts pa = float32_unpack_canonical(a, status); FloatParts pb = float32_unpack_canonical(b, status); @@ -1090,8 +1090,8 @@ float32 __attribute__((flatten)) float32_add(float32 a, float32 b, return float32_round_pack_canonical(pr, status); } -float64 __attribute__((flatten)) float64_add(float64 a, float64 b, - float_status *status) +static float64 QEMU_SOFTFLOAT_ATTR +soft_float64_add(float64 a, float64 b, float_status *status) { FloatParts pa = float64_unpack_canonical(a, status); FloatParts pb = float64_unpack_canonical(b, status); @@ -1110,8 +1110,8 @@ float16 __attribute__((flatten)) float16_sub(float16 a, float16 b, return float16_round_pack_canonical(pr, status); } -float32 __attribute__((flatten)) float32_sub(float32 a, float32 b, - float_status *status) +static float32 QEMU_SOFTFLOAT_ATTR +soft_float32_sub(float32 a, float32 b, float_status *status) { FloatParts pa = float32_unpack_canonical(a, status); FloatParts pb = float32_unpack_canonical(b, status); @@ -1120,8 +1120,8 @@ float32 __attribute__((flatten)) float32_sub(float32 a, float32 b, return float32_round_pack_canonical(pr, status); } -float64 __attribute__((flatten)) float64_sub(float64 a, float64 b, - float_status *status) +static float64 QEMU_SOFTFLOAT_ATTR +soft_float64_sub(float64 a, float64 b, float_status *status) { FloatParts pa = float64_unpack_canonical(a, status); FloatParts pb = float64_unpack_canonical(b, status); @@ -1130,6 +1130,96 @@ float64 __attribute__((flatten)) float64_sub(float64 a, float64 b, return float64_round_pack_canonical(pr, status); } +static float float_add(float a, float b) +{ +return a + b; +} + +static float float_sub(float a, float b) +{ +return a - b; +} + +static double double_add(double a, double b) +{ +return a + b; +} + +static double double_sub(double a, double b) +{ +return a - b; +} + +static bool f32_addsub_post(float32 a, float32 b, const struct float_status *s) +{ +return !(float32_is_zero(a) && float32_is_zero(b)); +} + +static bool +float_addsub_post(float a, float b, const struct float_status *s) +{ +return !(fpclassify(a) == FP_ZERO && fpclassify(b) == FP_ZERO); +} + +static bool f64_addsub_post(float64 a, float64 b, const struct float_status *s) +{ +return !(float64_is_zero(a) && float64_is_zero(b)); +} + +static bool +double_addsub_post(double a, double b, const struct float_status *s) +{ +return !(fpclassify(a) == FP_ZERO && fpclassify(b) == FP_ZERO); +} + +static float32 float32_addsub(float32 a, float32 b, float_status *s, + float_op2_func_t hard, f32_op2_func_t soft) +{ +if (QEMU_HARDFLOAT_2F32_USE_FP) { +return float_gen2(a, b, s, hard, soft, float_is_zon2, float_addsub_post, +
[Qemu-devel] [PATCH v3 01/15] tests: add fp-test, a floating point test suite
This will allow us to run correctness tests against our FP implementation. The test can be run in two modes (called "testers"): host and soft. With the former we check the results and FP flags on the host machine against the model. With the latter we check QEMU's fpu primitives against the model. Note that in soft mode we are not instantiating any particular CPU (hence the HW_POISON_H hack to avoid macro poisoning); for that we need to run the test in host mode under QEMU. The input files are taken from IBM's FPGen test suite: https://www.research.ibm.com/haifa/projects/verification/fpgen/ I see no license file in there so I am just downloading them with wget. We might want to keep a copy on a qemu server though, in case IBM takes those files down in the future. The "IBM" syntax of those files (for now the only syntax supported in fp-test) is documented here: https://www.research.ibm.com/haifa/projects/verification/fpgen/papers/ieee-test-suite-v2.pdf Note that the syntax document has some inaccuracies; the appended parsing code works around some of those. The exception flag (-e) is important: many of the optimizations included in the following commits assume that the inexact flag is set, so "-e x" is necessary in order to test those code paths. The whitelist flag (-w) points to a file with test cases to be ignored. I have put some whitelist files online, but we should have them on a QEMU-related server. Thus, a typical of fp-test is as follows: $ cd qemu/build/tests/fp-test $ make -j && \ ./fp-test -t soft ibm/*.fptest \ -w whitelist.txt \ -e x If we want to test after-rounding tininess detection, then we need to pass "-a -w whitelist-tininess-after.txt" in addition to the above. (NB. we can pass "-w" as many times as we want.) The patch immediately after this one fixes a mismatch against the model in softfloat, but after that is applied the above should finish with a 0 return code, and print something like: All tests OK. Tests passed: 76572. Not handled: 51237, whitelisted: 2662 The tests pass on "host" mode on x86_64 and aarch64 machines, although note that for the x86_64 you need to pass -w whitelist-tininess-after.txt. Running on host mode under QEMU reports flag mismatches (e.g. for x86_64-linux-user), but that isn't too surprising given how little love the i386 frontend gets. Host mode under aarch64-linux-user passes OK. Flush-to-zero and flush-inputs-to-zero modes can be tested with the -z and -Z flags. Note however that the IBM input files are only IEEE-compliant, so for now I've tested these modes by diff'ing the reported errors against the model files. We should look into generating files for these non-standard modes to make testing these modes less painful. Signed-off-by: Emilio G. Cota --- configure |2 + tests/fp/fp-test.c | 1159 tests/Makefile.include |3 + tests/fp/.gitignore|3 + tests/fp/Makefile | 34 ++ 5 files changed, 1201 insertions(+) create mode 100644 tests/fp/fp-test.c create mode 100644 tests/fp/.gitignore create mode 100644 tests/fp/Makefile diff --git a/configure b/configure index f156805..07dc5da 100755 --- a/configure +++ b/configure @@ -7106,12 +7106,14 @@ fi # build tree in object directory in case the source is not in the current directory DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema tests/tcg/xtensa tests/qemu-iotests tests/vm" +DIRS="$DIRS tests/fp" DIRS="$DIRS docs docs/interop fsdev scsi" DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw" DIRS="$DIRS roms/seabios roms/vgabios" FILES="Makefile tests/tcg/Makefile qdict-test-data.txt" FILES="$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit" FILES="$FILES tests/tcg/lm32/Makefile tests/tcg/xtensa/Makefile po/Makefile" +FILES="$FILES tests/fp/Makefile" FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps" FILES="$FILES pc-bios/spapr-rtas/Makefile" FILES="$FILES pc-bios/s390-ccw/Makefile" diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c new file mode 100644 index 000..27637c4 --- /dev/null +++ b/tests/fp/fp-test.c @@ -0,0 +1,1159 @@ +/* + * fp-test.c - Floating point test suite. + * + * Copyright (C) 2018, Emilio G. Cota + * + * License: GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#ifndef HW_POISON_H +#error Must define HW_POISON_H to work around TARGET_* poisoning +#endif + +#include "qemu/osdep.h" +#include "fpu/softfloat.h" + +#include +#include + +enum error { +ERROR_NONE, +ERROR_NOT_HANDLED, +ERROR_WHITELISTED, +ERROR_COMMENT, +ERROR_INPUT, +ERROR_RESULT, +ERROR_EXCEPTIONS, +ERROR_MAX, +}; + +enum input_fmt { +INPUT_FMT_IBM, +}; + +struct input { +const char * const name; +enum error (*test_line)(const char *line); +}; + +enum precision { +PREC_FLOAT, +PREC_DOUBLE, +PREC_QUAD, +PREC_FLOAT_
[Qemu-devel] [PATCH v3 13/15] hardfloat: support float32/64 fused multiply-add
Performance results for fp-bench: 1. Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz - before: fma-single: 74.73 MFlops fma-double: 74.54 MFlops - after: fma-single: 203.37 MFlops fma-double: 169.37 MFlops 2. ARM Aarch64 A57 @ 2.4GHz - before: fma-single: 23.24 MFlops fma-double: 23.70 MFlops - after: fma-single: 66.14 MFlops fma-double: 63.10 MFlops 3. IBM POWER8E @ 2.1 GHz - before: fma-single: 37.26 MFlops fma-double: 37.29 MFlops - after: fma-single: 48.90 MFlops fma-double: 59.51 MFlops Here having 3FP64 set to 1 pays off for x86_64: [1] 170.15 vs [0] 153.12 MFlops Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 169 ++-- 1 file changed, 165 insertions(+), 4 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 4323dc2..ce14c87 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1574,8 +1574,9 @@ float16 __attribute__((flatten)) float16_muladd(float16 a, float16 b, float16 c, return float16_round_pack_canonical(pr, status); } -float32 __attribute__((flatten)) float32_muladd(float32 a, float32 b, float32 c, -int flags, float_status *status) +static float32 QEMU_SOFTFLOAT_ATTR +soft_float32_muladd(float32 a, float32 b, float32 c, int flags, +float_status *status) { FloatParts pa = float32_unpack_canonical(a, status); FloatParts pb = float32_unpack_canonical(b, status); @@ -1585,8 +1586,9 @@ float32 __attribute__((flatten)) float32_muladd(float32 a, float32 b, float32 c, return float32_round_pack_canonical(pr, status); } -float64 __attribute__((flatten)) float64_muladd(float64 a, float64 b, float64 c, -int flags, float_status *status) +static float64 QEMU_SOFTFLOAT_ATTR +soft_float64_muladd(float64 a, float64 b, float64 c, int flags, +float_status *status) { FloatParts pa = float64_unpack_canonical(a, status); FloatParts pb = float64_unpack_canonical(b, status); @@ -1597,6 +1599,165 @@ float64 __attribute__((flatten)) float64_muladd(float64 a, float64 b, float64 c, } /* + * FMA generator for softfloat-based condition checks. + * + * When (a || b) == 0, there's no need to check for under/over flow, + * since we know the addend is (normal || 0) and the product is 0. + */ +#define GEN_FMA_SF(name, soft_t, host_t, host_fma_f, host_abs_f, min_normal) \ +static soft_t \ +name(soft_t a, soft_t b, soft_t c, int flags, float_status *s) \ +{ \ +if (QEMU_NO_HARDFLOAT) {\ +goto soft; \ +} \ +soft_t ## _input_flush3(&a, &b, &c, s); \ +if (likely((soft_t ## _is_normal(a) || soft_t ## _is_zero(a)) && \ + (soft_t ## _is_normal(b) || soft_t ## _is_zero(b)) && \ + (soft_t ## _is_normal(c) || soft_t ## _is_zero(c)) && \ + !(flags & float_muladd_halve_result) && \ + can_use_fpu(s))) { \ +if (soft_t ## _is_zero(a) || soft_t ## _is_zero(b)) { \ +soft_t p, r;\ +host_t hp, hc, hr; \ +bool prod_sign; \ +\ +prod_sign = soft_t ## _is_neg(a) ^ soft_t ## _is_neg(b); \ +prod_sign ^= !!(flags & float_muladd_negate_product); \ +p = soft_t ## _set_sign(soft_t ## _zero, prod_sign);\ +\ +if (flags & float_muladd_negate_c) {\ +c = soft_t ## _chs(c); \ +} \ +\ +hp = soft_t ## _to_ ## host_t(p); \ +hc = soft_t ## _to_ ## host_t(c); \ +hr = hp + hc; \ +r = host_t ## _to_ ## soft_t(hr); \ +return flags & float_muladd_negate_result ? \ +soft_t ## _chs(r) : r; \ +} else {\ +host_t ha, hb, hc, hr; \ +soft_t r; \
[Qemu-devel] [PATCH v3 02/15] softfloat: fix {min, max}nummag for same-abs-value inputs
Before 8936006 ("fpu/softfloat: re-factor minmax", 2018-02-21), we used to return +Zero for maxnummag(-Zero,+Zero); after that commit, we return -Zero. Fix it by making {min,max}nummag consistent with {min,max}num, deferring to the latter when the absolute value of the operands is the same. With this fix we now pass fp-test. Reviewed-by: Alex Bennée Signed-off-by: Emilio G. Cota --- fpu/softfloat.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 6e16284..6803279 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1704,7 +1704,6 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin, return pick_nan(a, b, s); } else { int a_exp, b_exp; -bool a_sign, b_sign; switch (a.cls) { case float_class_normal: @@ -1735,20 +1734,22 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin, break; } -a_sign = a.sign; -b_sign = b.sign; -if (ismag) { -a_sign = b_sign = 0; +if (ismag && (a_exp != b_exp || a.frac != b.frac)) { +bool a_less = a_exp < b_exp; +if (a_exp == b_exp) { +a_less = a.frac < b.frac; +} +return a_less ^ ismin ? b : a; } -if (a_sign == b_sign) { +if (a.sign == b.sign) { bool a_less = a_exp < b_exp; if (a_exp == b_exp) { a_less = a.frac < b.frac; } -return a_sign ^ a_less ^ ismin ? b : a; +return a.sign ^ a_less ^ ismin ? b : a; } else { -return a_sign ^ ismin ? b : a; +return a.sign ^ ismin ? b : a; } } } -- 2.7.4
[Qemu-devel] [PATCH v3 00/15] fp-test + hardfloat
v2: https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06805.html Changes since v2: - Add R-b tags - Add a patch to rename our canonicalize to sf_canonicalize, to avoid clashing with glibc's. - Add a patch to define float{32,64}_is_zero_or_normal - Simplify the float{32,64}_input_flushX macros -- now the macros are more verbose but the full function names are greppable. - Move tests/fp-test to tests/fp, since now both fp-bench and fp-test are under tests/fp. + Use tests/fp/fp-test.h for helpers common to both fp-bench and fp-test. - Complete rewrite of fp-bench: + We can now directly call the softfloat functions, thereby making the benchmark more sensitive to changes to those functions. + We can still use the native ops with "-t host". + The rewrite also has less macro trickery; we rely instead on constant propagation by the compiler. + Alex: dropped your R-b since this changed a lot. I think you'll like this version better though! - Define a generic function to generate the hardfloat implementation for ops with 2 inputs; add, sub, mul and div depend on it. Instead of using macros, rely on the constant propagation done by the compiler. [Alex: I dropped your R-b for the addsub patch because it changed a lot] + I kept macros for other ops, because I think the subsequent code duplication savings are worth the pain. - Add #define's to select whether to use fpclassify etc. or float32_is_zero etc. + Benchmark perf differences on x86_64, aarch64 and IBM Power8 hosts. + For 32-bit we don't use fpclassify etc. for any architectures, so I was tempted to get rid of this option to save some code. It's possible however that on some hosts I have not tested this option might pay off, so I decided to keep it there. - Add a #define to select whether to use isinf() or floatX_is_infinity(). Turns out this makes a big difference for power64. - Remove float32_to_float64 support in hardfloat, since nbench or SPEC actually showed a small yet measurable slowdown with it, despite fp-bench showing a significant speedup for this operation. - Do not flatten soft-fp functions; these are now slow paths. This shrinks the size of the softfloat object below its original size (see last patch's log). - Add a #define to disable hardfloat for some targets. I noticed that some targets (at least I noticed PPC, there might be others) do clear the FP flags before calling softfloat. This precludes hardfloat since it relies on inexact not being set. In the long run we should fix these targets though. Note: fp-bench can run _very_ slowly (~0.5 IPC) for -o fma on some x86_64 hosts. I have not pinned down what's going on, but from the few hosts I have access to, it seems that machines that have been patched for Spectre/Meltdown are susceptible to this slowdown. Fortunately though: 1) when fma is run in QEMU (and not under a microbenchmark such as fp-bench), fma performance is still very good (much better than with soft-fp). 2) Compiling with -march=native gets rid of the problem. I've reproduced this with both gcc 5.4.0 and gcc 7.1.0. The *very* same fp-bench binary that performs very well for FMA on two machines (one AMD, one Intel, neither patched against Meltdown/Spectre) performs below soft-fp on another three machines (all Intel, all patched). Note: there are some checkpatch errors, but they are false positives. Perf numbers for fp-bench are in each commit log; numbers for several benchmarks are in the last patch's commit log. You can fetch this series from: https://github.com/cota/qemu/tree/hardfloat-v3 Thanks, Emilio --- configure |2 + fpu/softfloat.c | 945 ++-- include/fpu/softfloat.h | 30 + target/tricore/fpu_helper.c |9 +- tests/Makefile.include |3 + tests/fp/.gitignore |4 + tests/fp/Makefile | 36 ++ tests/fp/fp-bench.c | 528 ++ tests/fp/fp-test.c | 1183 tests/fp/muladd.fptest | 51 ++ 10 files changed, 2737 insertions(+), 54 deletions(-) create mode 100644 tests/fp/.gitignore create mode 100644 tests/fp/Makefile create mode 100644 tests/fp/fp-bench.c create mode 100644 tests/fp/fp-test.c create mode 100644 tests/fp/muladd.fptest
Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
On Wed, 4 Apr 2018 22:30:50 +0200 Eric Auger wrote: > The 567b5b309abe ("vfio/pci: Relax DMA map errors for MMIO regions") > added an error message if a passed memory section address or size > is not aligned to the page size and thus cannot be DMA mapped. > > This patch fixes the trace by printing the region name and the > memory region section offset within the address space (instead of > offset_within_region). > > We also turn the error_report into a trace event. Indeed, In some > cases, the traces can be confusing to non expert end-users and > let think the use case does not work (whereas it > works as before). > > This is the case where a BAR is successively mapped at different > GPAs and its sections are not compatible with dma map. The listener > is called several times and traces are issued for each intermediate > mapping. The end-user cannot easily match those GPAs against the > final GPA output by lscpi. So let's keep those information to > informed users. In mid term, the plan is to advise the user about > BAR relocation relevance. > > Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions" > Signed-off-by: Alexey Kardashevskiy > Signed-off-by: Eric Auger > > --- > > v1 -> v2: > - use a trace point > - add some details in the commit message I think this is v2 with v1 being Message-Id:<20180322081837.21460-1-...@ozlabs.ru> but this is substantially different from Alexey's posting so I'd like to verify the Sign-off. Alexey? I agree that this seems to be the right thing to do for 2.12, nothing has changed for these mappings and the error reports are difficult even for experts to decipher and explain to users. Thanks, Alex > --- > hw/vfio/common.c | 11 +-- > hw/vfio/trace-events | 1 + > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 5e84716..07ffa0b 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -548,12 +548,11 @@ static void vfio_listener_region_add(MemoryListener > *listener, > hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1; > > if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) { > -error_report("Region 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx > - " is not aligned to 0x%"HWADDR_PRIx > - " and cannot be mapped for DMA", > - section->offset_within_region, > - int128_getlo(section->size), > - pgmask + 1); > +trace_vfio_listener_region_add_no_dma_map( > +memory_region_name(section->mr), > +section->offset_within_address_space, > +int128_getlo(section->size), > +pgmask + 1); > return; > } > } > diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events > index 79f63a2..20109cb 100644 > --- a/hw/vfio/trace-events > +++ b/hw/vfio/trace-events > @@ -90,6 +90,7 @@ vfio_iommu_map_notify(const char *op, uint64_t iova_start, > uint64_t iova_end) "i > vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING > region_add 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add > [iommu] 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void > *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" > +vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, > uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" > size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" > vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING > region_del 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_del(uint64_t start, uint64_t end) "region_del > 0x%"PRIx64" - 0x%"PRIx64 > vfio_disconnect_container(int fd) "close container->fd=%d"
Re: [Qemu-devel] [Qemu-arm] [PATCH for-2.12] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init
Hi Thomas, On 04/04/2018 04:15 PM, Thomas Huth wrote: > An instance_init function must not fail - and might be called multiple times, > e.g. during device introspection with the 'device-list-properties' QMP > command. Since the integratorcm device ignores this rule, QEMU currently > aborts in this case (though it really should not): > > echo "{'execute':'qmp_capabilities'}"\ > "{'execute':'device-list-properties',"\ > "'arguments':{'typename':'integrator_core'}}" \ > | arm-softmmu/qemu-system-arm -M integratorcp,accel=qtest -qmp stdio > {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, > "package": "build-all"}, "capabilities": []}} > {"return": {}} > RAMBlock "integrator.flash" already registered, abort! > Aborted (core dumped) > > Move the problematic code to the realize() function instead to fix this > problem. > > Signed-off-by: Thomas Huth > --- > hw/arm/integratorcp.c | 23 +++ > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c > index e8303b8..9e2df34 100644 > --- a/hw/arm/integratorcp.c > +++ b/hw/arm/integratorcp.c > @@ -266,7 +266,6 @@ static const MemoryRegionOps integratorcm_ops = { > static void integratorcm_init(Object *obj) > { > IntegratorCMState *s = INTEGRATOR_CM(obj); > -SysBusDevice *dev = SYS_BUS_DEVICE(obj); > > s->cm_osc = 0x0148; > /* ??? What should the high bits of this value be? */ > @@ -276,20 +275,28 @@ static void integratorcm_init(Object *obj) > s->cm_init = 0x0112; > s->cm_refcnt_offset = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24, > 1000); > -memory_region_init_ram(&s->flash, obj, "integrator.flash", 0x10, > - &error_fatal); > > -memory_region_init_io(&s->iomem, obj, &integratorcm_ops, s, > - "integratorcm", 0x0080); > -sysbus_init_mmio(dev, &s->iomem); > - > -integratorcm_do_remap(s); > /* ??? Save/restore. */ > } > > static void integratorcm_realize(DeviceState *d, Error **errp) > { > IntegratorCMState *s = INTEGRATOR_CM(d); > +SysBusDevice *dev = SYS_BUS_DEVICE(d); > +Error *local_err = NULL; > + > +memory_region_init_ram(&s->flash, (Object *)d, "integrator.flash", > 0x10, To keep consistency: memory_region_init_ram(&s->flash, OBJECT(d), "integrator.flash", ... > + &local_err); > +if (local_err) { > +error_propagate(errp, local_err); > +return; > +} > + > +memory_region_init_io(&s->iomem, (Object *)d, &integratorcm_ops, s, memory_region_init_io(&s->iomem, OBJECT(d), &integratorcm_ops, s, > + "integratorcm", 0x0080); > +sysbus_init_mmio(dev, &s->iomem); > + > +integratorcm_do_remap(s); > > if (s->memsz >= 256) { > integrator_spd[31] = 64; > With the macro instead of the cast: Reviewed-by: Philippe Mathieu-Daudé Regards, Phil.
[Qemu-devel] [ANNOUNCE] QEMU 2.12.0-rc2 is now available
Hello, On behalf of the QEMU Team, I'd like to announce the availability of the third release candidate for the QEMU 2.12 release. This release is meant for testing purposes and should not be used in a production environment. http://download.qemu-project.org/qemu-2.12.0-rc2.tar.xz http://download.qemu-project.org/qemu-2.12.0-rc2.tar.xz.sig You can help improve the quality of the QEMU 2.12 release by testing this release and reporting bugs on Launchpad: https://bugs.launchpad.net/qemu/ The release plan, as well a documented known issues for release candidates, are available at: http://wiki.qemu.org/Planning/2.12 Please add entries to the ChangeLog for the 2.12 release below: http://wiki.qemu.org/ChangeLog/2.12 Changes since rc1: 0e87fdc966: Update version for v2.12.0-rc2 release (Peter Maydell) bfb15b4bec: block/rbd: remove processed options from qdict (Jeff Cody) 74912f6dad: tcg: fix 16-byte vector operations detection (Laurent Vivier) abd3622cc0: iotests: Test abnormally large size in compressed cluster descriptor (Alberto Garcia) 96914159b7: qemu-iotests: Use ppc64 qemu_arch on ppc64le host (Lukáš Doktor) 733d1dce0f: iotests: Test preallocated truncate of 2G image (Max Reitz) 82b45e0a0b: block/file-posix: Fix fully preallocated truncate (Max Reitz) eb42e7193e: iotests: fix 208 for luks format (Vladimir Sementsov-Ogievskiy) 627f607e3d: iotests: Update 186 after commit ac64273c66ab136c44043259162 (Alberto Garcia) 242c172132: iotests: Update 051 and 186 after commit 1454509726719e0933c (Alberto Garcia) 9dae635afa: gluster: Fix blockdev-add with server.N.type=unix (Kevin Wolf) 604343ced7: blockjob: use qapi enum helpers (Marc-André Lureau) a865cebb82: blockjob: leak fix, remove from txn when failing early (Marc-André Lureau) a03083a017: block: handle invalid lseek returns gracefully (Jeff Cody) 3e4d88eabf: gluster: Fix blockdev-add with server.N.type=unix (Kevin Wolf) 3ea7f4a226: linux-user: fix TARGET___O_TMPFILE for sparc (Laurent Vivier) 5de154e82f: linux-user: define TARGET_ARCH_HAS_KA_RESTORER (Laurent Vivier) 95a29a4e3e: linux-user: fix alpha signal emulation (Laurent Vivier) d3b6e3bb6d: pc-bios/s390-ccw: update image (Cornelia Huck) 23bf419c1c: pc-bios/s390-ccw: Increase virtio timeout to 30 seconds (Thomas Huth) d9b06db813: hw/s390x: fix memory leak in s390_init_ipl_dev() (Greg Kurz) 5d7bc72a43: sev/i386: fix memory leak in sev_guest_init() (Greg Kurz) 72a841d2a4: exec: fix memory leak in find_max_supported_pagesize() (Greg Kurz) 2b53af2523: nbd: trace meta context negotiation (Eric Blake) 260e34dbb7: nbd/client: Correctly handle bad server REP_META_CONTEXT (Eric Blake) 00d96a4612: nbd: Fix 32-bit compilation on BLOCK_STATUS (Eric Blake) 64a563dd8d: target/xtensa: linux-user: fix fadvise64 call (Max Filippov) 12e3340c23: linux-user: implement clock_settime (Max Filippov) b9f9908e2d: linux-user: fix error propagation in clock_gettime (Max Filippov) a3da8be512: target/xtensa: linux-user: fix sysv IPC structures (Max Filippov) a23ea40982: linux-user: fix mq_getsetattr implementation (Max Filippov) 73a988d957: linux-user: call cpu_copy under clone_lock (Max Filippov) 4a6bf7adb9: target/xtensa: linux-user: rewind pc for restarted syscall (Max Filippov) 20ef667060: target/xtensa: fix flush_window_regs (Max Filippov) a0350d: qemu-doc: Rework the network options chapter to make "-net" less prominent (Thomas Huth) 4d0d1c077e: tests: Tests more flags of the CRB interface (Stefan Berger) 384cf1fc64: tpm: CRB: Enforce locality is requested before processing buffer (Stefan Berger) 025bc93619: tpm: CRB: Reset Granted flag when relinquishing locality (Stefan Berger) 3a3c873502: tpm: CRB: set the Idle flag by default (Stefan Berger) b02403363f: RISC-V: Workaround for critical mstatus.FS bug (Michael Clark) 0746a92612: migration: Don't activate block devices if using -S (Dr. David Alan Gilbert) fc6008f37a: migration: fix pfd leak (Marc-André Lureau) 33b4f859f1: RISC-V: Fix incorrect disassembly for addiw (Michael Clark) eab1586257: RISC-V: Convert cpu definition to future model (Michael Clark) f2f1dde751: tcg: Mark muluh_i64 and mulsh_i64 as 64-bit ops (Richard Henderson)
Re: [Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
On 04/04/2018 05:30 PM, Eric Auger wrote: > The 567b5b309abe ("vfio/pci: Relax DMA map errors for MMIO regions") > added an error message if a passed memory section address or size > is not aligned to the page size and thus cannot be DMA mapped. > > This patch fixes the trace by printing the region name and the > memory region section offset within the address space (instead of > offset_within_region). > > We also turn the error_report into a trace event. Indeed, In some > cases, the traces can be confusing to non expert end-users and > let think the use case does not work (whereas it > works as before). > > This is the case where a BAR is successively mapped at different > GPAs and its sections are not compatible with dma map. The listener > is called several times and traces are issued for each intermediate > mapping. The end-user cannot easily match those GPAs against the > final GPA output by lscpi. So let's keep those information to > informed users. In mid term, the plan is to advise the user about > BAR relocation relevance. > > Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions" > Signed-off-by: Alexey Kardashevskiy > Signed-off-by: Eric Auger Reviewed-by: Philippe Mathieu-Daudé > --- > > v1 -> v2: > - use a trace point > - add some details in the commit message > --- > hw/vfio/common.c | 11 +-- > hw/vfio/trace-events | 1 + > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 5e84716..07ffa0b 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -548,12 +548,11 @@ static void vfio_listener_region_add(MemoryListener > *listener, > hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1; > > if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) { > -error_report("Region 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx > - " is not aligned to 0x%"HWADDR_PRIx > - " and cannot be mapped for DMA", > - section->offset_within_region, > - int128_getlo(section->size), > - pgmask + 1); > +trace_vfio_listener_region_add_no_dma_map( > +memory_region_name(section->mr), > +section->offset_within_address_space, > +int128_getlo(section->size), > +pgmask + 1); > return; > } > } > diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events > index 79f63a2..20109cb 100644 > --- a/hw/vfio/trace-events > +++ b/hw/vfio/trace-events > @@ -90,6 +90,7 @@ vfio_iommu_map_notify(const char *op, uint64_t iova_start, > uint64_t iova_end) "i > vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING > region_add 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add > [iommu] 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void > *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" > +vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, > uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" > size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" > vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING > region_del 0x%"PRIx64" - 0x%"PRIx64 > vfio_listener_region_del(uint64_t start, uint64_t end) "region_del > 0x%"PRIx64" - 0x%"PRIx64 > vfio_disconnect_container(int fd) "close container->fd=%d" >
Re: [Qemu-devel] [PULL 16/20] target/arm: Use vector infrastructure for aa64 compares
On Wed, Apr 04, 2018 at 19:24:52 +0100, Alex Bennée wrote: > > Emilio G. Cota writes: > > > On Wed, Feb 07, 2018 at 14:55:36 -0800, Richard Henderson wrote: > >> Reviewed-by: Alex Bennée > >> Reviewed-by: Peter Maydell > >> Signed-off-by: Richard Henderson > > > > Just bisected a segfault for aarch64 nbench on a PowerPC host > > to this commit (79d61de6bdc398). I've also tested on x86_64 and > > aarch64 hosts, and these do not seem to be affected. > > I wonder about other hosts without TCG_vec backend support (powerpc is > one)? I suspect it must be the fall-back for the vector expanders is > wrong. I just tried on two additional machines: - Reproduced on a different PowerPC host with different endianness (gcc110, which is power7 and therefore big endian) - Could not reproduce on a mips64 host (gcc23) So it might be a Power-only thing. Thanks, Emilio
[Qemu-devel] [PATCH v2 for 2.12] vfio: Use a trace point when a RAM section cannot be DMA mapped
The 567b5b309abe ("vfio/pci: Relax DMA map errors for MMIO regions") added an error message if a passed memory section address or size is not aligned to the page size and thus cannot be DMA mapped. This patch fixes the trace by printing the region name and the memory region section offset within the address space (instead of offset_within_region). We also turn the error_report into a trace event. Indeed, In some cases, the traces can be confusing to non expert end-users and let think the use case does not work (whereas it works as before). This is the case where a BAR is successively mapped at different GPAs and its sections are not compatible with dma map. The listener is called several times and traces are issued for each intermediate mapping. The end-user cannot easily match those GPAs against the final GPA output by lscpi. So let's keep those information to informed users. In mid term, the plan is to advise the user about BAR relocation relevance. Fixes: 567b5b309abe "vfio/pci: Relax DMA map errors for MMIO regions" Signed-off-by: Alexey Kardashevskiy Signed-off-by: Eric Auger --- v1 -> v2: - use a trace point - add some details in the commit message --- hw/vfio/common.c | 11 +-- hw/vfio/trace-events | 1 + 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 5e84716..07ffa0b 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -548,12 +548,11 @@ static void vfio_listener_region_add(MemoryListener *listener, hwaddr pgmask = (1ULL << ctz64(hostwin->iova_pgsizes)) - 1; if ((iova & pgmask) || (int128_get64(llsize) & pgmask)) { -error_report("Region 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx - " is not aligned to 0x%"HWADDR_PRIx - " and cannot be mapped for DMA", - section->offset_within_region, - int128_getlo(section->size), - pgmask + 1); +trace_vfio_listener_region_add_no_dma_map( +memory_region_name(section->mr), +section->offset_within_address_space, +int128_getlo(section->size), +pgmask + 1); return; } } diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index 79f63a2..20109cb 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -90,6 +90,7 @@ vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "i vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add 0x%"PRIx64" - 0x%"PRIx64 vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add [iommu] 0x%"PRIx64" - 0x%"PRIx64 vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]" +vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA" vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING region_del 0x%"PRIx64" - 0x%"PRIx64 vfio_listener_region_del(uint64_t start, uint64_t end) "region_del 0x%"PRIx64" - 0x%"PRIx64 vfio_disconnect_container(int fd) "close container->fd=%d" -- 2.5.5
Re: [Qemu-devel] [PATCH v3 6/7] s390x/kvm: handle AP instruction interception
On 04/04/2018 09:43 AM, Pierre Morel wrote: On 04/04/2018 15:33, Tony Krowiak wrote: On 04/04/2018 07:09 AM, Pierre Morel wrote: On 02/04/2018 18:36, Tony Krowiak wrote: On 03/26/2018 05:03 AM, Pierre Morel wrote: On 26/03/2018 10:32, David Hildenbrand wrote: On 16.03.2018 00:24, Tony Krowiak wrote: If the CPU model indicates that AP facility is installed on the guest (i.e., -cpu ,ap=on), then the expectation is that the AP bus running in the guest will initialize; however, if the AP instructions are not being interpreted by the firmware, then they will be intercepted and routed back to QEMU for handling. If a handler is not defined to process the intercepted instruciton, t ...snip... +int ap_device_handle_nqap(S390CPU *cpu) +{ +CPUS390XState *env = &cpu->env; + +if (s390_has_feat(S390_FEAT_AP)) { +env->regs[1] = 0x1; + +return 0; +} + +return -EOPNOTSUPP; +} + +int ap_device_handle_dqap(S390CPU *cpu) +{ +CPUS390XState *env = &cpu->env; + +if (s390_has_feat(S390_FEAT_AP)) { +env->regs[1] = 0x1; + +return 0; +} + +return -EOPNOTSUPP; +} + +int ap_device_handle_pqap(S390CPU *cpu) +{ +CPUS390XState *env = &cpu->env; +int fc = 4 & (env->regs[0] >> 24); + +/* + * The Query Configuration Information (QCI) function (fc == 4) does not + * set a response code in reg 1, so check for that along with the + * AP feature. + */ +if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { +env->regs[1] = 0x1; + +return 0; +} This would imply an operation exception in case fc==4, which sounds very wrong. It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be tested to know what to answer. If the feature is there, QCI must be answered correctly. This is an interesting proposition which raises several issues that will need to be addressed. The only time the PQAP(QCI) instruction is intercepted is when: * A vfio-ap device is NOT defined for the guest because the vfio_ap device driver will set ECA.28 and the PQAP(QCI) instruction will be interpreted * STFLE.12 is set for the guest You say that the QCI must be answered correctly, but what is the correct response? If we return the matrix - i.e., APM, ADM and AQM - configured via the mediated matrix device's sysfs attributes files, then if any APQNs are defined in the matrix, we will have to handle *ALL* AP instructions by executing them on behalf of the guest. I suppose we could return an empty matrix in which case the AP bus will come up without any devices on the guest, but what is the expectation of an admin who deliberately configures the mediated matrix device? Should we forego handling interception of AP instructions and consider a guest configuration that turns on S390_FEAT_AP but does not define a vfio-ap device to be erroneous and terminate starting of the guest? Anybody have any thoughts? there are also some error situations to handle in all three functions. To what error situations are you referring? program exceptions, like access, privilege or specification exceptions. Are you suggesting that these handlers need to verify that the instruction specification is correct? For example, the NQAP instruction - NQAP r1,r2 - expects an even-odd pair of general registers in the r1 and r2 and must designate an even register; otherwise a specification exception is recognized. Are you saying that the handler for NQAP must verify this requirement and inject a specification exception if it is not met? If not, then can you provide a specific example of what you are talking about? Yes I mean this. But one other thing. Returning code 0x01 seems wrong for me, this is right if no AP card are in the system but I do not thing that it is what we mean. I disagree for the guest, this is exactly what we mean. In my opinion, assigning AP devices to the mediated matrix device is like assigning AP devices to an LPAR. If no vfio-ap device is defined for the guest, then it is effectively the same as starting a linux host in an LPAR without any AP devices assigned to it. I think we should return code 0x03 stating that the AP card is not in configured state The difference is that we can get out of a not-configured state with a SCLP command but we can not recover from an un-existing APQN. Remember, this state will occur only when the AP feature of the CPU model is turned on for the guest and no vfio-ap device is defined, so there is no configuration to recover for the guest because no AP devices will be configured for the guest. Hard to review without access to documentation. (hard to understand why such stuff is to be kept confidential for decades) + +return -EOPNOTSUPP; +} + static Property vfio_ap_properties[] = { DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), DEFINE_PROP_END_OF_LIST(), diff --git a/include/hw/s390x/ap-device.h b/include/h
Re: [Qemu-devel] [PATCH 1/1] block-backend: simplify blk_get_aio_context
Ping On 03/27/2018 10:08 AM, Daniel Henrique Barboza wrote: blk_get_aio_context verifies if BlockDriverState bs is not NULL, return bdrv_get_aio_context(bs) if true or qemu_get_aio_context() otherwise. However, bdrv_get_aio_context from block.c already does this verification itself, also returning qemu_get_aio_context() if bs is NULL: AioContext *bdrv_get_aio_context(BlockDriverState *bs) { return bs ? bs->aio_context : qemu_get_aio_context(); } This patch simplifies blk_get_aio_context to simply call bdrv_get_aio_context instead of replicating the same logic. Signed-off-by: Daniel Henrique Barboza --- block/block-backend.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 681b240b12..89f47b00ea 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1865,13 +1865,7 @@ void blk_op_unblock_all(BlockBackend *blk, Error *reason) AioContext *blk_get_aio_context(BlockBackend *blk) { -BlockDriverState *bs = blk_bs(blk); - -if (bs) { -return bdrv_get_aio_context(bs); -} else { -return qemu_get_aio_context(); -} +return bdrv_get_aio_context(blk_bs(blk)); } static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
Re: [Qemu-devel] [PULL for-2.12 0/1] Block patches
On 4 April 2018 at 17:10, Jeff Cody wrote: > The following changes since commit fd69ad866b62ca8ed4337ffee83b6d82a4e99282: > > Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging > (2018-04-04 14:00:07 +0100) > > are available in the git repository at: > > git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request > > for you to fetch changes up to bfb15b4becf2c9cf484bc3bf893a37c8f3c15cb3: > > block/rbd: remove processed options from qdict (2018-04-04 12:05:13 -0400) > > > Regression fix for 2.12 > > > Jeff Cody (1): > block/rbd: remove processed options from qdict > > block/rbd.c | 7 +++ > 1 file changed, 7 insertions(+) Applied, thanks. -- PMM
[Qemu-devel] [PATCH for-2.12] hw/arm/integratorcp: Don't do things that could be fatal in the instance_init
An instance_init function must not fail - and might be called multiple times, e.g. during device introspection with the 'device-list-properties' QMP command. Since the integratorcm device ignores this rule, QEMU currently aborts in this case (though it really should not): echo "{'execute':'qmp_capabilities'}"\ "{'execute':'device-list-properties',"\ "'arguments':{'typename':'integrator_core'}}" \ | arm-softmmu/qemu-system-arm -M integratorcp,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} {"return": {}} RAMBlock "integrator.flash" already registered, abort! Aborted (core dumped) Move the problematic code to the realize() function instead to fix this problem. Signed-off-by: Thomas Huth --- hw/arm/integratorcp.c | 23 +++ 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index e8303b8..9e2df34 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -266,7 +266,6 @@ static const MemoryRegionOps integratorcm_ops = { static void integratorcm_init(Object *obj) { IntegratorCMState *s = INTEGRATOR_CM(obj); -SysBusDevice *dev = SYS_BUS_DEVICE(obj); s->cm_osc = 0x0148; /* ??? What should the high bits of this value be? */ @@ -276,20 +275,28 @@ static void integratorcm_init(Object *obj) s->cm_init = 0x0112; s->cm_refcnt_offset = muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), 24, 1000); -memory_region_init_ram(&s->flash, obj, "integrator.flash", 0x10, - &error_fatal); -memory_region_init_io(&s->iomem, obj, &integratorcm_ops, s, - "integratorcm", 0x0080); -sysbus_init_mmio(dev, &s->iomem); - -integratorcm_do_remap(s); /* ??? Save/restore. */ } static void integratorcm_realize(DeviceState *d, Error **errp) { IntegratorCMState *s = INTEGRATOR_CM(d); +SysBusDevice *dev = SYS_BUS_DEVICE(d); +Error *local_err = NULL; + +memory_region_init_ram(&s->flash, (Object *)d, "integrator.flash", 0x10, + &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} + +memory_region_init_io(&s->iomem, (Object *)d, &integratorcm_ops, s, + "integratorcm", 0x0080); +sysbus_init_mmio(dev, &s->iomem); + +integratorcm_do_remap(s); if (s->memsz >= 256) { integrator_spd[31] = 64; -- 1.8.3.1
[Qemu-devel] TCG MicroBlaze 64-bit extended addressing
Hello, I'm looking at adding support for MicroBlaze extended addressing allowing 32bit cores to reach a 64-bit address space. The ABI for MicroBlaze remains 32-bits. It's basically a PAE-like MMU extension + a new set of extended address Load/Store instructions for the non-MMU mode. I'm primarily looking to implement the non-MMU mode now but it's not entirely clear to me how to do this. It would be nice to avoid multiple builds of microblaze and keep the address size run-time selectable. One way I think of is to set: #define TARGET_VIRT_ADDR_SPACE_BITS 64 #define TARGET_VIRT_ADDR_SPACE_BITS 64 And use a helper with cpu_ld_ hacks to do 64-bit loads. If I set TARGET_LONG_BITS=64 I can avoid the helper hack but IIUC, now all load stores need to take TCGv_i64's which complicates things a little in the translator since we now need to "emulate" the 32bit mode (common-case) with 64-bit addresses (wraps and such). Also, I'm guessing I'd need to set TARGET_ABI32 to avoid breaking linux-user. Does someone have any good suggestions or opinion on what the right way is? Am I missing something? Cheers, Edgar
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.12] gluster: Fix blockdev-add with server.N.type=unix
On 04/04/2018 01:37 PM, Kevin Wolf wrote: > Am 04.04.2018 um 20:16 hat Eric Blake geschrieben: >> On 04/04/2018 10:54 AM, Kevin Wolf wrote: >> > Should we also add a deprecation warning for 'socket' and update the > deprecation documentation, so we can start the clock ticking on getting > rid of maintaining the back-compat glue forever? > We only strictly need it in introspection at the time when we remove the > old option. But maybe it would be nicer to wait with the deprecation > error_report() until libvirt has a chance to avoid the warning. Makes sense - so no deprecation warning for 2.12, and the timeline for performing a deprecation is shifted by a couple of releases after the first time we introduce proper introspection support. Which makes this patch fine as-is. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.12] gluster: Fix blockdev-add with server.N.type=unix
Am 04.04.2018 um 20:16 hat Eric Blake geschrieben: > On 04/04/2018 10:54 AM, Kevin Wolf wrote: > > >>> Should we also add a deprecation warning for 'socket' and update the > >>> deprecation documentation, so we can start the clock ticking on getting > >>> rid of maintaining the back-compat glue forever? > >> > >> Well, that won't be as easy. Since there is at least one qemu release > >> which declared this in the QAPI schema but did not support using it it's > >> hard for libvirt to detect that this was fixed, and thus we can't infer > >> a capability which would be used to switch to the new-syntax only. > > > > Markus wanted to add some way to declare capabilities for a command in > > the schema that could be queried with schema introspection and would > > cover such cases. I think we even discussed this with you at KVM Forum? > > > > Eric, do you happen to know if there were any patches on the list > > related to this? > > I don't recall seeing any patches from Markus before his leave, and > doubt anyone else has tackled anything along these lines. It's too late > for 2.12, at any rate, so even if we had such a feature implemented in > time for 2.13, it won't help introspecting the 2.12 release. We only strictly need it in introspection at the time when we remove the old option. But maybe it would be nicer to wait with the deprecation error_report() until libvirt has a chance to avoid the warning. Kevin signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH RFC v3 for-2.12?] scripts/checkpatch.pl: Bug fix
[adding a few more cc's] On 03/25/2018 09:06 PM, Su Hang wrote: > Commit 2b9aef6fcd96ba7ed8c1ee723e391901852d344c introduced a regression: > checkpatch.pl started complaining about the following valid pattern: > do { > /* something */ > } while (condition); > > Fix the script to once again permit this pattern. We can probably drop the RFC from the title (RFC means you are unsure if the patch is in its final form), and probably want this patch included in 2.12 as we are still getting emails that hit the false positive: https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg00403.html > > Signed-off-by: Su Hang > --- > v1: fix bug. > v2: correct inappropriate patch description. > v3: put version description under Signed-off-by line. > > scripts/checkpatch.pl | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) Perl is not my strongest point, so take my review with a grain of salt. However, since I already have a couple of other random patches that may still be appropriate for -rc3, I can pick this up in a pull request if I get at least one more review (and if no one else picks it up first, such as the qemu-trivial process or Paolo's misc tree) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 57daae05ea18..d52207a3cc9d 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2356,6 +2356,18 @@ sub process { > # check for missing bracing around if etc > if ($line =~ /(^.*)\b(?:if|while|for)\b/ && > $line !~ /\#\s*if/) { > + my $allowed = 0; > + > + # Check the pre-context. > + if ($line =~ /(\}.*?)$/) { Why are you using minimal match coupled with an anchored expression? Isn't '($line =~ /(\}.*)$/)' going to match the same subexpression (namely, any line containing } but not as the last character)? Otherwise, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 16/20] target/arm: Use vector infrastructure for aa64 compares
Emilio G. Cota writes: > On Wed, Feb 07, 2018 at 14:55:36 -0800, Richard Henderson wrote: >> Reviewed-by: Alex Bennée >> Reviewed-by: Peter Maydell >> Signed-off-by: Richard Henderson > > Just bisected a segfault for aarch64 nbench on a PowerPC host > to this commit (79d61de6bdc398). I've also tested on x86_64 and > aarch64 hosts, and these do not seem to be affected. I wonder about other hosts without TCG_vec backend support (powerpc is one)? I suspect it must be the fall-back for the vector expanders is wrong. > > To reproduce: > 1. grab this binary: > http://cs.columbia.edu/~cota/qemu/nbench-aarch64 > 2. run it on a PowerPC host with: > $ aarch64-linux-user/qemu-aarch64 nbench-aarch64 -V > > Note: the "-V" (or "-v") flag is important! Without it, there's no segfault. > > I can reproduce this consistently on a Power8 host -- I'm using gcc112 > from the gcc compile farm. > > Thanks, > > Emilio -- Alex Bennée
Re: [Qemu-devel] [Qemu-block] [PATCH for-2.12] gluster: Fix blockdev-add with server.N.type=unix
On 04/04/2018 10:54 AM, Kevin Wolf wrote: >>> Should we also add a deprecation warning for 'socket' and update the >>> deprecation documentation, so we can start the clock ticking on getting >>> rid of maintaining the back-compat glue forever? >> >> Well, that won't be as easy. Since there is at least one qemu release >> which declared this in the QAPI schema but did not support using it it's >> hard for libvirt to detect that this was fixed, and thus we can't infer >> a capability which would be used to switch to the new-syntax only. > > Markus wanted to add some way to declare capabilities for a command in > the schema that could be queried with schema introspection and would > cover such cases. I think we even discussed this with you at KVM Forum? > > Eric, do you happen to know if there were any patches on the list > related to this? I don't recall seeing any patches from Markus before his leave, and doubt anyone else has tackled anything along these lines. It's too late for 2.12, at any rate, so even if we had such a feature implemented in time for 2.13, it won't help introspecting the 2.12 release. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [Bug 1025244] Re: qcow2 image increasing disk size above the virtual limit
Looking through old bug tickets... is there anything left to do here? Or should we rather close this ticket nowadays? ** Changed in: qemu Status: New => Incomplete ** Changed in: qemu-kvm (Ubuntu) Status: Triaged => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1025244 Title: qcow2 image increasing disk size above the virtual limit Status in QEMU: Incomplete Status in qemu-kvm package in Ubuntu: Incomplete Bug description: Using qemu/kvm, qcow2 images, ext4 file systems on both guest and host Host and Guest: Ubuntu server 12.04 64bit To create an image I did this: qemu-img create -f qcow2 -o preallocation=metadata ubuntu-pdc-vda.img 10737418240 (not sure about the exact bytes, but around this) ls -l ubuntu-pdc-vda.img fallocate -l theSizeInBytesFromAbove ubuntu-pdc-vda.img The problem is that the image is growing progressively and has obviously no limit, although I gave it one. The root filesystem's image is the same case: qemu-img info ubuntu-pdc-vda.img image: ubuntu-pdc-vda.img file format: qcow2 virtual size: 10G (10737418240 bytes) disk size: 14G cluster_size: 65536 and for confirmation: du -sh ubuntu-pdc-vda.img 15G ubuntu-pdc-vda.img I made a test and saw that when I delete something from the guest, the real size of the image is not decreasing (I read it is normal). OK, but when I write something again, it doesn't use the freed space, but instead grows the image. So for example: 1. The initial physical size of the image is 1GB. 2. I copy 1GB of data in the guest. It's physical size becomes 2GB. 3. I delete this data (1GB). The physical size of the image remains 2GB. 4. I copy another 1GB of data to the guest. 5. The physical size of the image becomes 3GB. 6. And so on with no limit. It doesn't care if the virtual size is less. Is this normal - the real/physical size of the image to be larger than the virtual limit??? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1025244/+subscriptions
[Qemu-devel] [Bug 1426092] Re: virtio-balloon-device locks up Guest
Looking through old bug tickets... can you still reproduce this issue with the latest version of QEMU? ... anyway, this rather sounds like a bug with the guest kernel to me, so in case this problem persists, you likely should report this in the kernel bug tracker instead. ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1426092 Title: virtio-balloon-device locks up Guest Status in QEMU: Incomplete Bug description: Setting arbitrary balloon target values locks up the guest in some cases crashes it, if the target memory is < used +~5% free. Found while testing aggressive memory over-commit, scenarios. You get messages like: [ 155.827448] [] (show_stack) from [] (dump_stack+0x6c/0x88) [ 155.837076] [] (dump_stack) from [] (warn_alloc_failed+0xe0/0x120) [ 155.847075] [] (warn_alloc_failed) from [] (__alloc_pages_nodemask+0x600/0x91c) [ 155.859039] [] (__alloc_pages_nodemask) from [] (balloon_page_enqueue+0x20/0xbc) [ 155.870556] [] (balloon_page_enqueue) from [] (balloon+0x140/0x2cc) [ 155.881377] [] (balloon) from [] (kthread+0xd8/0xf4) page dumped bacause: nonzero _count BUG: BAad page state in process Xorg pfn:396e5 Test Environment: x86_64 Ubuntu 13.10, Guest Linux Kernel 3.19, qemu 2.2.0 with following patches applied - balloon OOM enhancement commit 5a10b7dbf904bfe01bb9fcc6298f7df09eed77d5 Author: Raushaniya Maksudova Boot guest with '-balloon virtio', -qmp -hmp access 1. sudo info balloon | socat - tcp4-connect:127.0.0.1: - or over qmp interface {"execute":"qom-get", "arguments": { "path": "/machine/peripheral-anon/device[1]","property": "guest-stats" }} {"return": {"stats": {"stat-swap-out": 0, "stat-free-memory": 513454080, "stat-minor-faults": 1261, "stat-major-faults": 0, "stat-total-memory": 526073856, "stat-swap-in": 0}, "last-update": 11109}} 2. Take memory down check free memory using (1) 3. Issue "sudo echo balloon XXX | socat - tcp4-connect:127.0.0.1:" - XXX is value in threshold mentioned above and you get Guest lockup ARM - samething except '-device virtio-balloon-device' used Libvirt - virsh setmem causes same issue. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1426092/+subscriptions
[Qemu-devel] [Bug 1441443] Re: Is there a way to create a 10G network interface for VMs in KVM2.0?
Looking through old bug tickets ... have you already tried to use vhost- net? That should be one of the fastest ways of networking with QEMU as far as I know... ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1441443 Title: Is there a way to create a 10G network interface for VMs in KVM2.0? Status in QEMU: Incomplete Bug description: We have installed & configured the KVM 2.0 (qemu-kvm 2.0.0+dfsg- 2ubuntu1.10) on Ubuntu 14.04. The physical server is connected to 10G network, KVM is configured in Bridged mode But the issue is, when we create Network interface on VMs, we have only 1G device as an options for vmhosts. Is this the limit of the KVM or is there a way to create a 10G network interface for VMs? Available device models E1000 Ne2k_pci Pcnet Rtl8139 virtio Please find the network configuration details Source device : Host device vnet1 (Bridge ‘br0’) Device model : virtio Network configuration in the host /etc/network/interfaces auto br0 iface br0 inet static address 10.221.x.10 netmask 255.255.255.0 network 10.221.x.0 broadcast 10.221.x.255 gateway 10.221.x.1 # dns-* options are implemented by the resolvconf package, if installed dns-nameservers X.X.X.X dns-search XXX.NET bridge_ports em1 bridge_fd 0 bridge_stp off bridge_maxwait 0 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1441443/+subscriptions
Re: [Qemu-devel] [qemu-web PATCH] download: Add instructions for MacPorts
Am 04.04.2018 um 18:11 schrieb Paolo Bonzini: > On 04/04/2018 17:55, Stefan Weil wrote: >> By the way: https://qemu.weilnetz.de provides https (maybe I should >> enforce it), it includes sha512, and I also sign the binaries with my >> key. You still have to trust me, Debian and Cygwin (which provides lots >> of libraries used for the build). > > Cool! I had noticed sha512, but it is not very useful without https > (except to verify bitflips). Good news that you support https, we > should change the website to use https links instead. > > Regarding signing, there is no GPG signature. That's okay, but we > should document how to verify the installer signature from either Linux > or Windows. > > Thanks, > > Paolo The executables (installer, installed exe files) are signed using osslsigncode (https://packages.debian.org/sid/otherosfs/osslsigncode) and my personal CACert key for code signing. The signatures can be checked on Windows (e.g. during the installation process or from Windows Explorer with file properties) or on Linux (see example below). That's Windows standard. The only problem is that Windows does not automatically accept CACert keys (and that I have no better key for code signing). Stefan $ osslsigncode verify /var/www/html/w32/qemu-w32-setup-20180321.exe Current PE checksum : 04D7CD55 Calculated PE checksum: 04D7CD55 Message digest algorithm : SHA1 Current message digest: B2B13EB4765B4708D999BE3E4893915BBCAB0F8E Calculated message digest : B2B13EB4765B4708D999BE3E4893915BBCAB0F8E Signature verification: ok Number of signers: 1 Signer #0: Subject: /CN=Stefan Weil/emailAddress=s...@weilnetz.de Issuer : /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing Authority/emailAddress=supp...@cacert.org Serial : 0D6AA6 Number of certificates: 2 Cert #0: Subject: /CN=Stefan Weil/emailAddress=s...@weilnetz.de Issuer : /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing Authority/emailAddress=supp...@cacert.org Serial : 0D6AA6 -- Cert #1: Subject: /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing Authority/emailAddress=supp...@cacert.org Issuer : /O=Root CA/OU=http://www.cacert.org/CN=CA Cert Signing Authority/emailAddress=supp...@cacert.org Serial : 0 Succeeded
Re: [Qemu-devel] [qemu-web PATCH] download: Add instructions for MacPorts
On 04/04/2018 18:19, Programmingkid wrote: >>> I guess there is just too much distrust to provide a QEMU binary for >>> download. >> It's not distrust, it's responsibility. >> >> Paolo > So from what I learned, in order to provide a binary of QEMU, these things > must be done: > - Some kind of checksum be provided for the binary (md5, SHA512, ...) > - A zip file that has the exact code used to build the binary be provided > - The complete environment use to build the binary be documented > -- Operating system name and version > -- name and version of various tools used to build the binary (GCC, make, ...) > -- name and version of libraries that are linked to QEMU (libc, pixman, ...) > - The exact command-line options used to build the binary be provided > - The email address and identity of the person who made the binary be provided > > If anything is missing please feel free to share. In practice a GPG signature, with a signature well-connected to other people in the QEMU community, would already be a very good start. If the exact code is not a release tarball, that would also be required. The command line options used for the build can be documented in the wiki. Paolo
[Qemu-devel] [PATCH for-2.12] hw/arm/allwinner-a10: Do not use nd_table in instance_init function
The instance_init function of a device can be called at any time, even if the device is not going to be used (i.e. not going to be realized). So a instance_init function must not do things that could cause QEMU to exit, like calling qemu_check_nic_model(&nd_table[0], ...) for example. But this is what the instance_init function of the allwinner-a10 device is currently doing - and this causes QEMU to quit unexpectedly when you run the 'device-list-properties' QMP command for example: $ echo "{'execute':'qmp_capabilities'}"\ "{'execute':'device-list-properties',"\ " 'arguments':{'typename':'allwinner-a10'}}" \ | arm-softmmu/qemu-system-arm -M mps2-an505,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} {"return": {}} Unsupported NIC model: lan9118 ... and QEMU quits after printing the last line (which should not happen just because of running 'device-list-properties' here). And with the cubieboard, this even causes QEMU to abort(): $ echo "{'execute':'qmp_capabilities'}"\ "{'execute':'device-list-properties',"\ " 'arguments':{'typename':'allwinner-a10'}}" \ | arm-softmmu/qemu-system-arm -M cubieboard,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} {"return": {}} Unexpected error in error_set_from_qdev_prop_error() at hw/core/qdev-properties.c:1095: Property 'allwinner-emac.netdev' can't take value 'hub0port0', it's in use Aborted (core dumped) To fix the problem we've got to move the offending code to the realize function instead. Signed-off-by: Thomas Huth --- I know, an even cleaner fix would likely be to remove serial_hds and nd_table from the device completely and wire it up from the board code instead - but that's a major rework compared to this simple fix here, which we likely should avoid at this point in time of the hard freeze period. hw/arm/allwinner-a10.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c index 43a3f01..5dbbacb 100644 --- a/hw/arm/allwinner-a10.c +++ b/hw/arm/allwinner-a10.c @@ -38,11 +38,6 @@ static void aw_a10_init(Object *obj) object_initialize(&s->emac, sizeof(s->emac), TYPE_AW_EMAC); qdev_set_parent_bus(DEVICE(&s->emac), sysbus_get_default()); -/* FIXME use qdev NIC properties instead of nd_table[] */ -if (nd_table[0].used) { -qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC); -qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]); -} object_initialize(&s->sata, sizeof(s->sata), TYPE_ALLWINNER_AHCI); qdev_set_parent_bus(DEVICE(&s->sata), sysbus_get_default()); @@ -91,6 +86,11 @@ static void aw_a10_realize(DeviceState *dev, Error **errp) sysbus_connect_irq(sysbusdev, 4, s->irq[67]); sysbus_connect_irq(sysbusdev, 5, s->irq[68]); +/* FIXME use qdev NIC properties instead of nd_table[] */ +if (nd_table[0].used) { +qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC); +qdev_set_nic_properties(DEVICE(&s->emac), &nd_table[0]); +} object_property_set_bool(OBJECT(&s->emac), true, "realized", &err); if (err != NULL) { error_propagate(errp, err); @@ -118,7 +118,7 @@ static void aw_a10_class_init(ObjectClass *oc, void *data) DeviceClass *dc = DEVICE_CLASS(oc); dc->realize = aw_a10_realize; -/* Reason: Uses serial_hds in realize and nd_table in instance_init */ +/* Reason: Uses serial_hds and nd_table in realize function */ dc->user_creatable = false; } -- 1.8.3.1
Re: [Qemu-devel] [PULL 16/20] target/arm: Use vector infrastructure for aa64 compares
On Wed, Feb 07, 2018 at 14:55:36 -0800, Richard Henderson wrote: > Reviewed-by: Alex Bennée > Reviewed-by: Peter Maydell > Signed-off-by: Richard Henderson Just bisected a segfault for aarch64 nbench on a PowerPC host to this commit (79d61de6bdc398). I've also tested on x86_64 and aarch64 hosts, and these do not seem to be affected. To reproduce: 1. grab this binary: http://cs.columbia.edu/~cota/qemu/nbench-aarch64 2. run it on a PowerPC host with: $ aarch64-linux-user/qemu-aarch64 nbench-aarch64 -V Note: the "-V" (or "-v") flag is important! Without it, there's no segfault. I can reproduce this consistently on a Power8 host -- I'm using gcc112 from the gcc compile farm. Thanks, Emilio
Re: [Qemu-devel] [PATCH] tcg: fix 16-byte vector operations detection
On 3 April 2018 at 16:45, Laurent Vivier wrote: > On 29/03/2018 18:54, Laurent Vivier wrote: >> Hi, >> >> I think it would be good to have this fix (or something similar) in -rc2. > > So no one agrees with that? Applied to master, thanks. -- PMM
[Qemu-devel] [Bug 1749393] Re: sbrk() not working under qemu-user with a PIE-compiled binary?
qemu patch proposed at http://lists.nongnu.org/archive/html/qemu- devel/2018-03/msg04700.html -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1749393 Title: sbrk() not working under qemu-user with a PIE-compiled binary? Status in QEMU: New Status in qemu package in Ubuntu: Confirmed Bug description: In Debian unstable, we recently switched bash to be a PIE-compiled binary (for hardening). Unfortunately this resulted in bash being broken when run under qemu-user (for all target architectures, host being amd64 for me). $ sudo chroot /srv/chroots/sid-i386/ qemu-i386-static /bin/bash bash: xmalloc: .././shell.c:1709: cannot allocate 10 bytes (0 bytes allocated) bash has its own malloc implementation based on sbrk(): https://git.savannah.gnu.org/cgit/bash.git/tree/lib/malloc/malloc.c When we disable this internal implementation and rely on glibc's malloc, then everything is fine. But it might be that glibc has a fallback when sbrk() is not working properly and it might hide the underlying problem in qemu-user. This issue has also been reported to the bash upstream author and he suggested that the issue might be in qemu-user so I'm opening a ticket here. Here's the discussion with the bash upstream author: https://lists.gnu.org/archive/html/bug-bash/2018-02/threads.html#00080 You can find the problematic bash binary in that .deb file: http://snapshot.debian.org/archive/debian/20180206T154716Z/pool/main/b/bash/bash_4.4.18-1_i386.deb The version of qemu I have been using is 2.11 (Debian package qemu- user-static version 1:2.11+dfsg-1) but I have had reports that the problem is reproducible with older versions (back to 2.8 at least). Here are the related Debian bug reports: https://bugs.debian.org/889869 https://bugs.debian.org/865599 It's worth noting that bash used to have this problem (when compiled as a PIE binary) even when run directly but then something got fixed in the kernel and now the problem only appears when run under qemu-user: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1518483 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1749393/+subscriptions
Re: [Qemu-devel] [qemu-web PATCH] download: Add instructions for MacPorts
> On Apr 4, 2018, at 12:08 PM, Paolo Bonzini wrote: > > On 04/04/2018 18:05, Programmingkid wrote: >> >>> On Apr 4, 2018, at 11:55 AM, Stefan Weil wrote: >>> >>> Am 04.04.2018 um 16:58 schrieb Daniel P. Berrangé: On Wed, Apr 04, 2018 at 04:45:48PM +0200, Paolo Bonzini wrote: > On 04/04/2018 16:38, Daniel P. Berrangé wrote: >> The source/quality of those binaries is completely opaque. We've no idea >> who >> built them, nor what build options were used, nor what/where the >> corresponding >> source is (required for GPL compliance), nor any checksum / signature to >> validate the binary isn't compromised since build, etc, etc. >> >> Pointing users to those binaries makes it appear QEMU project is blessing >> them, and so any issues with them directly reflect on QEMU's reputation. >> >> If we're going to link to binaries telling users to download them, we >> need >> to be hosting them on qemu.org and have a clearly documented formal >> process >> around building & distributing them. >> >> Since both Homebrew & Macports are providing formal bulds though, it >> looks >> simpler to just entirely delegate the problem to them, as we do for Linux >> where we delegate to distro vendors to build & distribute binaries. > > Note that, to some extent, the same issues do apply to Win32 binaries > (in particular, they are distributed under http and there are no > signatures). However, the situation is better in that they are hosted > on an identifiable person's website, and of course Windows doesn't have > something akin to Homebrew and Macports so there is no alternative to > volunteers building and hosting the binaries. It would be desirable & practical to address that for Win32, by building the Win32 binaries at time of cutting the release, using the Mingw toolchain via one of our formal Docker environments. Would need buy-in of our release manager to accept the extra work for making releases though... Regards, Daniel >>> >>> That would be one possible way. A more automated way could use CI builds >>> (for example on GitHub) to generate executables for Windows. >>> >>> By the way: https://qemu.weilnetz.de provides https (maybe I should >>> enforce it), it includes sha512, and I also sign the binaries with my >>> key. You still have to trust me, Debian and Cygwin (which provides lots >>> of libraries used for the build). >>> >>> Regards, >>> Stefan >> >> I guess there is just too much distrust to provide a QEMU binary for >> download. > > It's not distrust, it's responsibility. > > Paolo So from what I learned, in order to provide a binary of QEMU, these things must be done: - Some kind of checksum be provided for the binary (md5, SHA512, ...) - A zip file that has the exact code used to build the binary be provided - The complete environment use to build the binary be documented -- Operating system name and version -- name and version of various tools used to build the binary (GCC, make, ...) -- name and version of libraries that are linked to QEMU (libc, pixman, ...) - The exact command-line options used to build the binary be provided - The email address and identity of the person who made the binary be provided If anything is missing please feel free to share.
Re: [Qemu-devel] [PATCH for-2.12 v2] qemu-iotests: update 185 output
On 2018-04-04 17:01, Stefan Hajnoczi wrote: > Commit 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268 ("vl: introduce > vm_shutdown()") added a bdrv_drain_all() call. As a side-effect of the > drain operation the block job iterates one more time than before. The > 185 output no longer matches and the test is failing now. > > It may be possible to avoid the superfluous block job iteration, but > that type of patch is not suitable late in the QEMU 2.12 release cycle. > > This patch simply updates the 185 output file. The new behavior is > correct, just not optimal, so make the test pass again. > > Fixes: 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268 ("vl: introduce > vm_shutdown()") > Cc: Kevin Wolf > Cc: QingFeng Hao > Signed-off-by: Stefan Hajnoczi > --- > tests/qemu-iotests/185 | 10 ++ > tests/qemu-iotests/185.out | 12 +++- > 2 files changed, 13 insertions(+), 9 deletions(-) On tmpfs, this isn't enough to let the test pass. There, the active commit job finishes before the quit is sent, resulting in this diff: --- tests/qemu-iotests/185.out 2018-04-04 18:10:02.015935435 +0200 +++ tests/qemu-iotests/185.out.bad 2018-04-04 18:10:21.045473817 +0200 @@ -26,9 +26,9 @@ {"return": {}} {"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "commit"}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "commit"}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "commit"}} === Start mirror job and exit qemu === This seems to be independent of whether there is actually data on TEST_IMG (the commit source), so something doesn't seem quite right with the block job throttling here...? Max
[Qemu-devel] [PULL for-2.12 1/1] block/rbd: remove processed options from qdict
Commit 4bfb274 added some QAPIfication of option parsing in qemu_rbd_open(). We need to remove all the options we processed, otherwise in bdrv_open_inherit() we will think the remaining options are invalid. (This needs to go in 2.12 to avoid a regression that prevents rbd from being opened.) Suggested-by: Kevin Wolf Signed-off-by: Jeff Cody Reviewed-by: Kevin Wolf --- block/rbd.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/block/rbd.c b/block/rbd.c index 5b64849dc6..c9359d0ad8 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -623,6 +623,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, BlockdevOptionsRbd *opts = NULL; Visitor *v; QObject *crumpled = NULL; +const QDictEntry *e; Error *local_err = NULL; const char *filename; char *keypairs, *secretid; @@ -671,6 +672,12 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, goto out; } +/* Remove the processed options from the QDict (the visitor processes + * _all_ options in the QDict) */ +while ((e = qdict_first(options))) { +qdict_del(options, e->key); +} + r = qemu_rbd_connect(&s->cluster, &s->io_ctx, opts, !(flags & BDRV_O_NOCACHE), keypairs, secretid, errp); if (r < 0) { -- 2.13.6
Re: [Qemu-devel] [qemu-web PATCH] download: Add instructions for MacPorts
On 04/04/2018 17:55, Stefan Weil wrote: > Am 04.04.2018 um 16:58 schrieb Daniel P. Berrangé: >> On Wed, Apr 04, 2018 at 04:45:48PM +0200, Paolo Bonzini wrote: >>> On 04/04/2018 16:38, Daniel P. Berrangé wrote: The source/quality of those binaries is completely opaque. We've no idea who built them, nor what build options were used, nor what/where the corresponding source is (required for GPL compliance), nor any checksum / signature to validate the binary isn't compromised since build, etc, etc. Pointing users to those binaries makes it appear QEMU project is blessing them, and so any issues with them directly reflect on QEMU's reputation. If we're going to link to binaries telling users to download them, we need to be hosting them on qemu.org and have a clearly documented formal process around building & distributing them. Since both Homebrew & Macports are providing formal bulds though, it looks simpler to just entirely delegate the problem to them, as we do for Linux where we delegate to distro vendors to build & distribute binaries. >>> >>> Note that, to some extent, the same issues do apply to Win32 binaries >>> (in particular, they are distributed under http and there are no >>> signatures). However, the situation is better in that they are hosted >>> on an identifiable person's website, and of course Windows doesn't have >>> something akin to Homebrew and Macports so there is no alternative to >>> volunteers building and hosting the binaries. >> >> It would be desirable & practical to address that for Win32, by building >> the Win32 binaries at time of cutting the release, using the Mingw toolchain >> via one of our formal Docker environments. Would need buy-in of our release >> manager to accept the extra work for making releases though... >> >> Regards, >> Daniel > > That would be one possible way. A more automated way could use CI builds > (for example on GitHub) to generate executables for Windows. > > By the way: https://qemu.weilnetz.de provides https (maybe I should > enforce it), it includes sha512, and I also sign the binaries with my > key. You still have to trust me, Debian and Cygwin (which provides lots > of libraries used for the build). Cool! I had noticed sha512, but it is not very useful without https (except to verify bitflips). Good news that you support https, we should change the website to use https links instead. Regarding signing, there is no GPG signature. That's okay, but we should document how to verify the installer signature from either Linux or Windows. Thanks, Paolo
Re: [Qemu-devel] [qemu-web PATCH] download: Add instructions for MacPorts
On 04/04/2018 18:05, Programmingkid wrote: > >> On Apr 4, 2018, at 11:55 AM, Stefan Weil wrote: >> >> Am 04.04.2018 um 16:58 schrieb Daniel P. Berrangé: >>> On Wed, Apr 04, 2018 at 04:45:48PM +0200, Paolo Bonzini wrote: On 04/04/2018 16:38, Daniel P. Berrangé wrote: > The source/quality of those binaries is completely opaque. We've no idea > who > built them, nor what build options were used, nor what/where the > corresponding > source is (required for GPL compliance), nor any checksum / signature to > validate the binary isn't compromised since build, etc, etc. > > Pointing users to those binaries makes it appear QEMU project is blessing > them, and so any issues with them directly reflect on QEMU's reputation. > > If we're going to link to binaries telling users to download them, we need > to be hosting them on qemu.org and have a clearly documented formal > process > around building & distributing them. > > Since both Homebrew & Macports are providing formal bulds though, it looks > simpler to just entirely delegate the problem to them, as we do for Linux > where we delegate to distro vendors to build & distribute binaries. Note that, to some extent, the same issues do apply to Win32 binaries (in particular, they are distributed under http and there are no signatures). However, the situation is better in that they are hosted on an identifiable person's website, and of course Windows doesn't have something akin to Homebrew and Macports so there is no alternative to volunteers building and hosting the binaries. >>> >>> It would be desirable & practical to address that for Win32, by building >>> the Win32 binaries at time of cutting the release, using the Mingw toolchain >>> via one of our formal Docker environments. Would need buy-in of our release >>> manager to accept the extra work for making releases though... >>> >>> Regards, >>> Daniel >> >> That would be one possible way. A more automated way could use CI builds >> (for example on GitHub) to generate executables for Windows. >> >> By the way: https://qemu.weilnetz.de provides https (maybe I should >> enforce it), it includes sha512, and I also sign the binaries with my >> key. You still have to trust me, Debian and Cygwin (which provides lots >> of libraries used for the build). >> >> Regards, >> Stefan > > I guess there is just too much distrust to provide a QEMU binary for download. It's not distrust, it's responsibility. Paolo
[Qemu-devel] [PULL for-2.12 0/1] Block patches
The following changes since commit fd69ad866b62ca8ed4337ffee83b6d82a4e99282: Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2018-04-04 14:00:07 +0100) are available in the git repository at: git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request for you to fetch changes up to bfb15b4becf2c9cf484bc3bf893a37c8f3c15cb3: block/rbd: remove processed options from qdict (2018-04-04 12:05:13 -0400) Regression fix for 2.12 Jeff Cody (1): block/rbd: remove processed options from qdict block/rbd.c | 7 +++ 1 file changed, 7 insertions(+) -- 2.13.6
Re: [Qemu-devel] [qemu-web PATCH] download: Add instructions for MacPorts
> On Apr 4, 2018, at 11:55 AM, Stefan Weil wrote: > > Am 04.04.2018 um 16:58 schrieb Daniel P. Berrangé: >> On Wed, Apr 04, 2018 at 04:45:48PM +0200, Paolo Bonzini wrote: >>> On 04/04/2018 16:38, Daniel P. Berrangé wrote: The source/quality of those binaries is completely opaque. We've no idea who built them, nor what build options were used, nor what/where the corresponding source is (required for GPL compliance), nor any checksum / signature to validate the binary isn't compromised since build, etc, etc. Pointing users to those binaries makes it appear QEMU project is blessing them, and so any issues with them directly reflect on QEMU's reputation. If we're going to link to binaries telling users to download them, we need to be hosting them on qemu.org and have a clearly documented formal process around building & distributing them. Since both Homebrew & Macports are providing formal bulds though, it looks simpler to just entirely delegate the problem to them, as we do for Linux where we delegate to distro vendors to build & distribute binaries. >>> >>> Note that, to some extent, the same issues do apply to Win32 binaries >>> (in particular, they are distributed under http and there are no >>> signatures). However, the situation is better in that they are hosted >>> on an identifiable person's website, and of course Windows doesn't have >>> something akin to Homebrew and Macports so there is no alternative to >>> volunteers building and hosting the binaries. >> >> It would be desirable & practical to address that for Win32, by building >> the Win32 binaries at time of cutting the release, using the Mingw toolchain >> via one of our formal Docker environments. Would need buy-in of our release >> manager to accept the extra work for making releases though... >> >> Regards, >> Daniel > > That would be one possible way. A more automated way could use CI builds > (for example on GitHub) to generate executables for Windows. > > By the way: https://qemu.weilnetz.de provides https (maybe I should > enforce it), it includes sha512, and I also sign the binaries with my > key. You still have to trust me, Debian and Cygwin (which provides lots > of libraries used for the build). > > Regards, > Stefan I guess there is just too much distrust to provide a QEMU binary for download.