Re: [Qemu-devel] [qemu-s390x] [PATCH v7 04/12] s390-ccw: update libc
On 16.02.2018 23:07, Collin L. Walling wrote: [...] > +/** > + * uitoa: > + * @num: an integer (base 10) to be converted. > + * @str: a pointer to a string to store the conversion. > + * @len: the length of the passed string. > + * > + * Given an integer @num, convert it to a string. The string @str must be > + * allocated beforehand. The resulting string will be null terminated and > + * returned. This function only handles numbers between 0 and UINT64_MAX > + * inclusive. > + * > + * Returns: the string @str of the converted integer @num > + */ > +char *uitoa(uint64_t num, char *str, size_t len) > +{ > +size_t num_idx = 0; > +uint64_t tmp = num; > + > +IPL_assert(str != NULL, "uitoa: no space allocated to store string"); > + > +/* Get index to ones place */ > +while ((tmp /= 10) != 0) { > +num_idx++; > +} > + > +/* Check if we have enough space for num and null */ > +IPL_assert(len > num_idx, "uitoa: array too small for conversion"); Well, in v5 of this patch you've had "len >= num_idx + 1" where we agreed that it was wrong. Now you have "len > num_idx" which is pretty much the same. WTF? I still think you need "len > num_idx + 1" here to properly take the trailing NUL-byte into account properly. Please fix it! Thomas
[Qemu-devel] [Bug 1708077] Re: PPC interrupt exception!
What kind of exeception are you seeing here exactly? Can you still reproduce it with the latest version of QEMU? ** Information type changed from Private Security to Public ** 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/1708077 Title: PPC interrupt exception! Status in QEMU: Incomplete Bug description: There is a exception on interrupt system when run the system with debug app on qemu-system-ppc.exe。I have try in version SHA-1: 2421f381dc38a8a6d12477c08c2f74a35a0698f8 no problem,but the next version SHA-1: 28f997a82cb509bf4775d4006b368e1bde8b7bdd have this exception。 And I found during this period in the repair of multi- threaded mutex,so I guess whether the PPC has some mutex needed are not taken into account。My english is poor,so there may be many grammatical errors。I hope you can understand the problem I described。 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1708077/+subscriptions
Re: [Qemu-devel] [PATCH v7 01/12] s390-ccw: refactor boot map table code
On 16.02.2018 23:07, Collin L. Walling wrote: > Some ECKD bootmap code was using structs designed for SCSI. > Even though this works, it confuses readability. Add a new > BootMapTable struct to assist with readability in bootmap > entry code. Also: > > - replace ScsiMbr in ECKD code with appropriate structs > - fix read_block messages to reflect BootMapTable > - fixup ipl_scsi to use BootMapTable (referred to as Program Table) > - defined value for maximum table entries > > Signed-off-by: Collin L. Walling> --- > pc-bios/s390-ccw/bootmap.c | 60 > +- > pc-bios/s390-ccw/bootmap.h | 11 - > 2 files changed, 37 insertions(+), 34 deletions(-) > > diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c > index 67a6123..286de40 100644 > --- a/pc-bios/s390-ccw/bootmap.c > +++ b/pc-bios/s390-ccw/bootmap.c > @@ -182,24 +182,24 @@ static block_number_t load_eckd_segments(block_number_t > blk, uint64_t *address) > return block_nr; > } > > -static void run_eckd_boot_script(block_number_t mbr_block_nr) > +static void run_eckd_boot_script(block_number_t bmt_block_nr) > { > int i; > unsigned int loadparm = get_loadparm_index(); > block_number_t block_nr; > uint64_t address; > -ScsiMbr *bte = (void *)sec; /* Eckd bootmap table entry */ > +BootMapTable *bmt = (void *)sec; > BootMapScript *bms = (void *)sec; > > debug_print_int("loadparm", loadparm); > -IPL_assert(loadparm < 31, "loadparm value greater than" > +IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than" > " maximum number of boot entries allowed"); > > memset(sec, FREE_SPACE_FILLER, sizeof(sec)); > -read_block(mbr_block_nr, sec, "Cannot read MBR"); > +read_block(bmt_block_nr, sec, "Cannot read Boot Map Table"); > > -block_nr = eckd_block_num((void *)&(bte->blockptr[loadparm])); > -IPL_assert(block_nr != -1, "No Boot Map"); > +block_nr = eckd_block_num((void *)>entry[loadparm]); I think you could drop the "(void *)" cast here now. > +IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry"); > > memset(sec, FREE_SPACE_FILLER, sizeof(sec)); > read_block(block_nr, sec, "Cannot read Boot Map Script"); > @@ -223,7 +223,7 @@ static void ipl_eckd_cdl(void) > XEckdMbr *mbr; > Ipl2 *ipl2 = (void *)sec; > IplVolumeLabel *vlbl = (void *)sec; > -block_number_t block_nr; > +block_number_t bmt_block_nr; > > /* we have just read the block #0 and recognized it as "IPL1" */ > sclp_print("CDL\n"); > @@ -238,8 +238,8 @@ static void ipl_eckd_cdl(void) > IPL_assert(mbr->dev_type == DEV_TYPE_ECKD, > "Non-ECKD device type in zIPL section of IPL2 record."); > > -/* save pointer to Boot Script */ > -block_nr = eckd_block_num((void *)&(mbr->blockptr)); > +/* save pointer to Boot Map Table */ > +bmt_block_nr = eckd_block_num((void *)>blockptr); dito, (void *) should not be needed here. But apart from those two nits, the patch looks fine to me now, so: Reviewed-by: Thomas Huth
[Qemu-devel] [PATCH] hw/acpi-build: build SRAT memory affinity structures for NVDIMM
ACPI 6.2A Table 5-129 "SPA Range Structure" requires the proximity domain of a NVDIMM SPA range must match with corresponding entry in SRAT table. The address ranges of vNVDIMM in QEMU are allocated from the hot-pluggable address space, which is entirely covered by one SRAT memory affinity structure. However, users can set the vNVDIMM proximity domain in NFIT SPA range structure by the 'node' property of '-device nvdimm' to a value different than the one in the above SRAT memory affinity structure. In order to solve such proximity domain mismatch, this patch build one SRAT memory affinity structure for each NVDIMM device with the proximity domain used in NFIT. The remaining hot-pluggable address space is covered by one or multiple SRAT memory affinity structures with the proximity domain of the last node as before. Signed-off-by: Haozhong Zhang--- hw/acpi/nvdimm.c| 15 +-- hw/i386/acpi-build.c| 47 +++ include/hw/mem/nvdimm.h | 11 +++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 59d6e4254c..dff0818e77 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -33,12 +33,23 @@ #include "hw/nvram/fw_cfg.h" #include "hw/mem/nvdimm.h" +static gint nvdimm_addr_sort(gconstpointer a, gconstpointer b) +{ +uint64_t addr0 = object_property_get_uint(OBJECT(NVDIMM(a)), + PC_DIMM_ADDR_PROP, NULL); +uint64_t addr1 = object_property_get_uint(OBJECT(NVDIMM(b)), + PC_DIMM_ADDR_PROP, NULL); + +return addr0 < addr1 ? -1 : + addr0 > addr1 ? 1 : 0; +} + static int nvdimm_device_list(Object *obj, void *opaque) { GSList **list = opaque; if (object_dynamic_cast(obj, TYPE_NVDIMM)) { -*list = g_slist_append(*list, DEVICE(obj)); +*list = g_slist_insert_sorted(*list, DEVICE(obj), nvdimm_addr_sort); } object_child_foreach(obj, nvdimm_device_list, opaque); @@ -52,7 +63,7 @@ static int nvdimm_device_list(Object *obj, void *opaque) * Note: it is the caller's responsibility to free the list to avoid * memory leak. */ -static GSList *nvdimm_get_device_list(void) +GSList *nvdimm_get_device_list(void) { GSList *list = NULL; diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index deb440f286..637ac3a8f0 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2323,6 +2323,46 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog) #define HOLE_640K_START (640 * 1024) #define HOLE_640K_END (1024 * 1024) +static void build_srat_hotpluggable_memory(GArray *table_data, uint64_t base, + uint64_t len, int default_node) +{ +GSList *nvdimms = nvdimm_get_device_list(); +GSList *ent = nvdimms; +NVDIMMDevice *dev; +uint64_t end = base + len, addr, size; +int node; +AcpiSratMemoryAffinity *numamem; + +while (base < end) { +numamem = acpi_data_push(table_data, sizeof *numamem); + +if (!ent) { +build_srat_memory(numamem, base, end - base, default_node, + MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED); +break; +} + +dev = NVDIMM(ent->data); +addr = object_property_get_uint(OBJECT(dev), PC_DIMM_ADDR_PROP, NULL); +size = object_property_get_uint(OBJECT(dev), PC_DIMM_SIZE_PROP, NULL); +node = object_property_get_uint(OBJECT(dev), PC_DIMM_NODE_PROP, NULL); + +if (base < addr) { +build_srat_memory(numamem, base, addr - base, default_node, + MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED); +numamem = acpi_data_push(table_data, sizeof *numamem); +} +build_srat_memory(numamem, addr, size, node, + MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED | + MEM_AFFINITY_NON_VOLATILE); + +base = addr + size; +ent = ent->next; +} + +g_slist_free(nvdimms); +} + static void build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) { @@ -2434,10 +2474,9 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine) * providing _PXM method if necessary. */ if (hotplugabble_address_space_size) { -numamem = acpi_data_push(table_data, sizeof *numamem); -build_srat_memory(numamem, pcms->hotplug_memory.base, - hotplugabble_address_space_size, pcms->numa_nodes - 1, - MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED); +build_srat_hotpluggable_memory(table_data, pcms->hotplug_memory.base, + hotplugabble_address_space_size, + pcms->numa_nodes - 1); } build_header(linker,
[Qemu-devel] [Bug 1587970] Re: QEMU Crashes when attaching USB 3.00 devices to xhci bus
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1587970 Title: QEMU Crashes when attaching USB 3.00 devices to xhci bus Status in QEMU: Expired Bug description: Using qemu 2.6 with a windows7 32-bit VM, if I plug a USB 3.0 memory stick in to a USB 3.0 port, then pass it through to the VM via the monitor (device_add usb-host,bus=xhci.0,hostbus=xx,hostaddr=xx,id=stick1) then qemu asserts and dies - I have seen 2 different asserts one is from the xchi module - Assertion `intr->er_full, and one is in core.c (line 400 I IIRCC) with "Assertion dev->state == 3 failed" Tried to work around by only passing in an ehci controller to the VM, but then if I attach a usb 3.0 memory stick to that it doesn't work in windows. I have made sure the xhci drivers in the windows VM are up to date, latest version of SeaBios etc, but at the moment, I have had to disable xhci in my system bios and just use ehci for everything. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1587970/+subscriptions
[Qemu-devel] [Bug 1490853] Re: qemu windows guest hangs on 100% cpu usage
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1490853 Title: qemu windows guest hangs on 100% cpu usage Status in QEMU: Expired Bug description: hi: I have two VM , one is winXP Prefessional SP3 32bit, another one is WindowsServer2008 Enterprise SP2 64bit. When I hot reboot winXP in guest OS, it'll hangs on progress bar, and all the vcpu thread in qemu is 100% usage. There are no message in kernel log when it happened, I try to rebuild kvm and add some debug info , I found the cpu exit reason is EXIT_REASON_PAUSE_INSTRUCTION. It seems like all the vcpu always in spinlock waiting. I'm not sure whether it's qemu's bug or kvm's. Any help would be appreciated. How reproducible: WinXP: seems always. WinServer2008: rare. Steps to Reproduce: winXP: hot reboot the xp guest os, hot reboot is necessary. WinServer2008: not sure, I didn't do anything, it just happened. The different between WinXP and WInServer2008: 1. When WinXP hangs, the boot progress bar is rolling, I think that vnc is work fine. 2. When WinServer2008 hangs, the vnc show the last screen and the screen won't change anything include system time. 3. When the VM hangs , if I execute "virsh suspend vm-name" and "virsh resume vm-name", the WinServer2008 will change to normal , and work fine not hangs anymore. But WinXP not change anything, still hangs. qemu version: QEMU emulator version 1.5.0, Copyright (c) 2003-2008 Fabrice Bellard host info: Intel(R) Xeon(R) CPU E5-2620 0 @ 2.00GHz Ubuntu 12.04 LTS \n \l Linux cvknode2026 3.13.6 #1 SMP Fri Dec 12 09:17:35 CST 2014 x86_64 x86_64 x86_64 GNU/Linux qemu command line (guest OS XP): root 7124 1178 7.6 7750360 3761644 ? Sl 14:02 435:23 /usr/bin/kvm -name x -S -machine pc-i440fx-1.5,accel=kvm,usb=off,system=windows -cpu qemu64,hv_relaxed,hv_spinlocks=0x2000 -m 6144 -smp 12,maxcpus=72,sockets=12,cores=6,threads=1 -uuid d3832129-f77d-4b21-bbf7-fd337f53e572 -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/x.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime,clock=vm,driftfix=slew -no-hpet -no-shutdown -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device usb-ehci,id=ehci,bus=pci.0,addr=0x4 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x5 -drive file=/vms/images/sn1-of-ff.qcow2,if=none,id=drive-ide0-0-0,format=qcow2,cache=directsync -device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -drive if=none,id=drive-ide0-1-1,readonly=on,format=raw -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1,bootindex=2 -netdev tap,fd=24,id=hostnet0 -device rtl8139,netdev=hostnet0,id=net0,mac=0c:da:41:1d:f8:40,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -chardev socket,id=charchannel0,path=/var/lib/libvirt/qemu/x.agent,server,nowait -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0 -device usb-tablet,id=input0,bus=usb.0 -vnc 0.0.0.0:0 -device VGA,id=video0,bus=pci.0,addr=0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 all qemu thread (guest OS XP): root@cvknode2026:/proc/7124/task# top -d 1 -H -p 7124 top - 14:37:05 up 7 days, 4:07, 1 user, load average: 10.71, 10.90, 10.19 Tasks: 14 total, 12 running, 2 sleeping, 0 stopped, 0 zombie Cpu(s): 38.8%us, 11.2%sy, 0.0%ni, 50.0%id, 0.0%wa, 0.0%hi, 0.0%si, 0.0%st Mem: 49159888k total, 35665128k used, 13494760k free, 436312k buffers Swap: 8803324k total,0k used, 8803324k free, 28595100k cached PID USER PR NI VIRT RES SHR S %CPU %MEMTIME+ P SWAP WCHAN COMMAND 7130 root 20 0 7568m 3.6g 6628 R 101 7.7 33:43.48 3 3.8g - kvm 7132 root 20 0 7568m 3.6g 6628 R 101 7.7 33:43.13 1 3.8g - kvm 7133 root 20 0 7568m 3.6g 6628 R 101 7.7 33:42.70 6 3.8g - kvm 7135 root 20 0 7568m 3.6g 6628 R 101 7.7 33:42.33 11 3.8g - kvm 7137 root 20 0 7568m 3.6g 6628 R 101 7.7 33:42.59 17 3.8g - kvm 7126 root 20 0 7568m 3.6g 6628 R 100 7.7 34:06.76 4 3.8g - kvm 7127 root 20 0 7568m 3.6g 6628 R 100 7.7 33:44.14 8 3.8g - kvm 7128 root 20 0 7568m 3.6g 6628 R 100 7.7 33:43.64 13 3.8g - kvm 7129 root 20 0 7568m 3.6g 6628 R 100 7.7 33:43.64 7 3.8g - kvm 7131 root 20 0 7568m 3.6g 6628 R 100 7.7 33:44.24 10 3.8g - kvm 7134 root 20 0 7568m 3.6g 6628 R 100 7.7 33:42.47 12 3.8g - kvm 7136 root 20 0 7568m 3.6g 6628 R 100
[Qemu-devel] [Bug 1731588] Re: qemu-system-arm black screen and keyboard not detected
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1731588 Title: qemu-system-arm black screen and keyboard not detected Status in QEMU: Expired Bug description: Hi guys, I try to emulate FreeRTOS with this guide : http://wiki.csie.ncku.edu.tw/embedded/Lab32 But, the keys on my keyboard are not taken into account. - Command line : qemu_stm32/arm-softmmu/qemu-system-arm -M stm32-p103 -monitor stdio -kernel build/main.bin -semihosting If I try to add usb flag with : qemu_stm32/arm-softmmu/qemu-system-arm -M stm32-p103 -monitor stdio -kernel build/main.bin -usb -device usb-host,hostbus=1,hostaddr=1 -show-curso I obtain this message "qemu-system-arm: -device usb-host,hostbus=1,hostaddr=1: 'usb-host' is not a valid device model name" My second option is try with the latest version of qemu with this command line : "qemu-system-arm -M lm3s6965evb -monitor stdio -kernel build/main.bin -semihosting" but I obtain a black screen. Could someone guide me on this problem ? To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1731588/+subscriptions
[Qemu-devel] [Bug 1721187] Re: install Centos7 or fedora27 on qemu on windows8.1
[Expired for QEMU because there has been no activity for 60 days.] ** Changed in: qemu Status: Incomplete => Expired -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1721187 Title: install Centos7 or fedora27 on qemu on windows8.1 Status in QEMU: Expired Bug description: Hello, I have tried to install CentOs or Fedora27 on my Windows8 using QEMU. I work on notepad with 4GB. Unfortunatly, my touchpad nor my usb-mouse are not recognise on the graphical installation of CentOs and Fedora installation. So, I cannot install them. Here are the commands I use for installation : qemu-img create -f qcow2 fedora27b2_hd.qcow2 80G qemu-system-x86_64 -k fr -hda fedora27b2_hd.qcow2 -cdrom Fedora- Workstation-Live-x86_64-27_Beta-1.5.iso -m 512 -boot d I have tried to add the option : -device usb-mouse but, I got the error message that no 'usb-bus' found for the usb-mouse device. What is wrong ? QEMU or my installation command ? Thank, BRgds, Laurent To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1721187/+subscriptions
[Qemu-devel] [PATCH v2] linux-user: make getcpu optional
Not all arches implement this, and the kernel doesn't require them to. Add ifdef logic to disable it when not available. Signed-off-by: Mike Frysinger--- linux-user/syscall.c | 4 1 file changed, 4 insertions(+) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 799c8e2800ea..a9904fac791f 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -290,8 +290,10 @@ _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned int, len, #define __NR_sys_sched_setaffinity __NR_sched_setaffinity _syscall3(int, sys_sched_setaffinity, pid_t, pid, unsigned int, len, unsigned long *, user_mask_ptr); +#ifdef TARGET_NR_getcpu #define __NR_sys_getcpu __NR_getcpu _syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *, tcache); +#endif _syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd, void *, arg); _syscall2(int, capget, struct __user_cap_header_struct *, header, @@ -10595,6 +10597,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask)); } break; +#ifdef TARGET_NR_getcpu case TARGET_NR_getcpu: { unsigned cpu, node; @@ -10612,6 +10615,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, } } break; +#endif case TARGET_NR_sched_setparam: { struct sched_param *target_schp; -- 2.16.1
[Qemu-devel] [PATCH] linux-user: make getcpu optional
Not all arches implement this, and the kernel doesn't require them to. Add ifdef logic to disable it when not available. Signed-off-by: Mike Frysinger--- linux-user/syscall.c| 4 target/bfin/op_helper.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 799c8e2800ea..a9904fac791f 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -290,8 +290,10 @@ _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned int, len, #define __NR_sys_sched_setaffinity __NR_sched_setaffinity _syscall3(int, sys_sched_setaffinity, pid_t, pid, unsigned int, len, unsigned long *, user_mask_ptr); +#ifdef TARGET_NR_getcpu #define __NR_sys_getcpu __NR_getcpu _syscall3(int, sys_getcpu, unsigned *, cpu, unsigned *, node, void *, tcache); +#endif _syscall4(int, reboot, int, magic1, int, magic2, unsigned int, cmd, void *, arg); _syscall2(int, capget, struct __user_cap_header_struct *, header, @@ -10595,6 +10597,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, ret = get_errno(sys_sched_setaffinity(arg1, mask_size, mask)); } break; +#ifdef TARGET_NR_getcpu case TARGET_NR_getcpu: { unsigned cpu, node; @@ -10612,6 +10615,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, } } break; +#endif case TARGET_NR_sched_setparam: { struct sched_param *target_schp; diff --git a/target/bfin/op_helper.c b/target/bfin/op_helper.c index 5b80aea23bc6..abb3e9301814 100644 --- a/target/bfin/op_helper.c +++ b/target/bfin/op_helper.c @@ -18,8 +18,8 @@ NULL, it means that the function was called in C code (i.e. not from generated code or from helper.c) */ /* XXX: fix it to restore all registers */ -void tlb_fill(CPUState *cs, target_ulong addr, MMUAccessType access_type, - int mmu_idx, uintptr_t retaddr) +void tlb_fill(CPUState *cs, target_ulong addr, int size, + MMUAccessType access_type, int mmu_idx, uintptr_t retaddr) { int ret; -- 2.16.1
Re: [Qemu-devel] [PATCH v9 16/29] sev/i386: add command to encrypt guest memory region
On 2/16/18 9:47 AM, Dr. David Alan Gilbert wrote: > * Brijesh Singh (brijesh.si...@amd.com) wrote: >> The KVM_SEV_LAUNCH_UPDATE_DATA command is used to encrypt a guest memory >> region using the VM Encryption Key created using LAUNCH_START. >> >> Cc: Paolo Bonzini>> Cc: Richard Henderson >> Cc: Eduardo Habkost >> Signed-off-by: Brijesh Singh >> --- >> accel/kvm/kvm-all.c | 2 ++ >> include/sysemu/sev.h | 1 + >> stubs/sev.c | 5 + >> target/i386/sev.c| 49 >> >> target/i386/trace-events | 1 + >> 5 files changed, 58 insertions(+) >> >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >> index 4468c8fe002c..4974c00c46fb 100644 >> --- a/accel/kvm/kvm-all.c >> +++ b/accel/kvm/kvm-all.c >> @@ -1679,6 +1679,8 @@ static int kvm_init(MachineState *ms) >> if (!kvm_state->memcrypt_handle) { >> goto err; >> } >> + >> +kvm_state->memcrypt_encrypt_data = sev_encrypt_data; >> } >> >> ret = kvm_arch_init(ms, s); >> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h >> index 5c8c549b68ec..c16102b05ec4 100644 >> --- a/include/sysemu/sev.h >> +++ b/include/sysemu/sev.h >> @@ -69,5 +69,6 @@ struct SEVState { >> typedef struct SEVState SEVState; >> >> void *sev_guest_init(const char *id); >> +int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len); >> >> #endif >> diff --git a/stubs/sev.c b/stubs/sev.c >> index 24c7b0c3e04d..74182bb545e2 100644 >> --- a/stubs/sev.c >> +++ b/stubs/sev.c >> @@ -15,6 +15,11 @@ >> #include "qemu-common.h" >> #include "sysemu/sev.h" >> >> +int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len) >> +{ >> +return 1; >> +} >> + >> SevState sev_get_current_state(void) >> { >> return SEV_STATE_UNINIT; >> diff --git a/target/i386/sev.c b/target/i386/sev.c >> index 6f767084fd57..04a64b5bc61d 100644 >> --- a/target/i386/sev.c >> +++ b/target/i386/sev.c >> @@ -90,6 +90,12 @@ fw_error_to_str(int code) >> return sev_fw_errlist[code]; >> } >> >> +static bool >> +sev_check_state(SevState state) >> +{ >> +return current_sev_guest_state == state ? true : false; >> +} >> + >> static void >> sev_set_guest_state(SevState new_state) >> { >> @@ -466,6 +472,36 @@ sev_launch_start(SEVState *s) >> return 0; >> } >> >> +static int >> +sev_launch_update_data(uint8_t *addr, uint64_t len) >> +{ >> +int ret, fw_error; >> +struct kvm_sev_launch_update_data *update; >> + >> +if (addr == NULL || len <= 0) { >> +return 1; >> +} >> + >> +update = g_malloc0(sizeof(*update)); >> +if (!update) { >> +return 1; >> +} >> > Keep checking for the g_malloc0 use - it will never return NULL; > if you want it to be safe from running out of memory use g_try_malloc0 > otherwise you can just remove the !update check. > Also it's better to use the g_new0 macro (or g_try_new0) - it's neater > and avoids the whole sizeof thing. > (You have that in a bunch of the patches) I didn't realized that g_malloc0() will never return NULL. I checked just glib doc, if v10 is needed then I can remove them all or can submit a follow-up patch. thanks > Dave > >> +update->uaddr = (__u64)addr; >> +update->len = len; >> +trace_kvm_sev_launch_update_data(addr, len); >> +ret = sev_ioctl(KVM_SEV_LAUNCH_UPDATE_DATA, update, _error); >> +if (ret) { >> +error_report("%s: LAUNCH_UPDATE ret=%d fw_error=%d '%s'", >> +__func__, ret, fw_error, fw_error_to_str(fw_error)); >> +goto err; >> +} >> + >> +err: >> +g_free(update); >> +return ret; >> +} >> + >> void * >> sev_guest_init(const char *id) >> { >> @@ -540,6 +576,19 @@ err: >> return NULL; >> } >> >> +int >> +sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len) >> +{ >> +assert(handle); >> + >> +/* if SEV is in update state then encrypt the data else do nothing */ >> +if (sev_check_state(SEV_STATE_LUPDATE)) { >> +return sev_launch_update_data(ptr, len); >> +} >> + >> +return 0; >> +} >> + >> static void >> sev_register_types(void) >> { >> diff --git a/target/i386/trace-events b/target/i386/trace-events >> index 9402251e9991..c0cd8e93217f 100644 >> --- a/target/i386/trace-events >> +++ b/target/i386/trace-events >> @@ -12,3 +12,4 @@ kvm_memcrypt_register_region(void *addr, size_t len) "addr >> %p len 0x%lu" >> kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%lu" >> kvm_sev_change_state(const char *old, const char *new) "%s -> %s" >> kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x >> session %p pdh %p" >> +kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" >> PRIu64 >> -- >> 2.14.3 >> > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v21 1/5] xbitmap: Introduce xbitmap
On Tue, Jan 9, 2018 at 1:10 PM, Wei Wangwrote: > From: Matthew Wilcox > > The eXtensible Bitmap is a sparse bitmap representation which is > efficient for set bits which tend to cluster. It supports up to > 'unsigned long' worth of bits. > lib/xbitmap.c| 444 > +++ Please, split tests to a separate module. -- With Best Regards, Andy Shevchenko
Re: [Qemu-devel] [PATCH v21 1/5] xbitmap: Introduce xbitmap
On Fri, Feb 16, 2018 at 8:30 PM, Matthew Wilcoxwrote: > On Fri, Feb 16, 2018 at 07:44:50PM +0200, Andy Shevchenko wrote: >> On Tue, Jan 9, 2018 at 1:10 PM, Wei Wang wrote: >> > From: Matthew Wilcox >> > >> > The eXtensible Bitmap is a sparse bitmap representation which is >> > efficient for set bits which tend to cluster. It supports up to >> > 'unsigned long' worth of bits. >> >> > lib/xbitmap.c| 444 >> > +++ >> >> Please, split tests to a separate module. > > Hah, I just did this two days ago! I didn't publish it yet, but I also made > it compile both in userspace and as a kernel module. > > It's the top two commits here: > > http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray-2018-02-12 > Thanks! > Note this is a complete rewrite compared to the version presented here; it > sits on top of the XArray and no longer has a preload interface. It has a > superset of the IDA functionality. Noted. Now, the question about test case. Why do you heavily use BUG_ON? Isn't resulting statistics enough? See how other lib/test_* modules do. -- With Best Regards, Andy Shevchenko
[Qemu-devel] [PATCH 3/3] target/s390x: convert to TranslatorOps
Signed-off-by: Emilio G. Cota--- target/s390x/translate.c | 170 --- 1 file changed, 86 insertions(+), 84 deletions(-) diff --git a/target/s390x/translate.c b/target/s390x/translate.c index dd504a1..2b27a69 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -55,10 +55,12 @@ struct DisasContext { DisasContextBase base; const DisasInsn *insn; DisasFields *fields; +uint64_t next_page_start; uint64_t ex_value; uint64_t pc, next_pc; uint32_t ilen; enum cc_op cc_op; +bool do_debug; }; /* Information carried about a condition to be evaluated. */ @@ -6108,101 +6110,92 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) return ret; } -void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) +static int s390x_tr_init_disas_context(DisasContextBase *dcbase, + CPUState *cs, int max_insns) { -CPUS390XState *env = cs->env_ptr; -DisasContext dc; -uint64_t next_page_start; -int num_insns, max_insns; -DisasJumpType status; -bool do_debug; +DisasContext *s = container_of(dcbase, DisasContext, base); -dc.base.pc_first = tb->pc; /* 31-bit mode */ -if (!(tb->flags & FLAG_MASK_64)) { -dc.base.pc_first &= 0x7fff; +if (!(s->base.tb->flags & FLAG_MASK_64)) { +s->base.pc_first &= 0x7fff; +s->base.pc_next = s->base.pc_first; } -dc.base.pc_next = dc.base.pc_first; -dc.base.tb = tb; -dc.base.singlestep_enabled = cs->singlestep_enabled; -dc.pc = dc.base.pc_first; -dc.cc_op = CC_OP_DYNAMIC; -dc.ex_value = dc.base.tb->cs_base; -do_debug = cs->singlestep_enabled; +s->pc = s->base.pc_first; +s->cc_op = CC_OP_DYNAMIC; +s->ex_value = s->base.tb->cs_base; +s->do_debug = s->base.singlestep_enabled; +s->next_page_start = (s->base.pc_first & TARGET_PAGE_MASK) + +TARGET_PAGE_SIZE; -next_page_start = (dc.base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; +return max_insns; +} -num_insns = 0; -max_insns = tb_cflags(tb) & CF_COUNT_MASK; -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; -} -if (max_insns > TCG_MAX_INSNS) { -max_insns = TCG_MAX_INSNS; -} +static void s390x_tr_tb_start(DisasContextBase *db, CPUState *cs) +{ +} -gen_tb_start(tb); +static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) +{ +DisasContext *s = container_of(dcbase, DisasContext, base); -do { -tcg_gen_insn_start(dc.pc, dc.cc_op); -num_insns++; +tcg_gen_insn_start(s->pc, s->cc_op); +} -if (unlikely(cpu_breakpoint_test(cs, dc.base.pc_next, BP_ANY))) { -status = DISAS_PC_STALE; -do_debug = true; -/* The address covered by the breakpoint must be included in - [tb->pc, tb->pc + tb->size) in order to for it to be - properly cleared -- thus we increment the PC here so that - the logic setting tb->size below does the right thing. */ -dc.pc += 2; -dc.base.pc_next += 2; -break; -} +static bool s390x_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs, + const CPUBreakpoint *bp) +{ +DisasContext *s = container_of(dcbase, DisasContext, base); -if (num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { -gen_io_start(); -} +s->base.is_jmp = DISAS_PC_STALE; +s->do_debug = true; +/* The address covered by the breakpoint must be included in + [tb->pc, tb->pc + tb->size) in order to for it to be + properly cleared -- thus we increment the PC here so that + the logic setting tb->size below does the right thing. */ +s->pc += 2; +s->base.pc_next += 2; +return true; +} -status = translate_one(env, ); -dc.base.pc_next = dc.pc; - -/* If we reach a page boundary, are single stepping, - or exhaust instruction count, stop generation. */ -if (status == DISAS_NEXT -&& (dc.pc >= next_page_start -|| tcg_op_buf_full() -|| num_insns >= max_insns -|| singlestep -|| dc.base.singlestep_enabled -|| dc.ex_value)) { -status = DISAS_TOO_MANY; -} -} while (status == DISAS_NEXT); +static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) +{ +CPUS390XState *env = cs->env_ptr; +DisasContext *s = container_of(dcbase, DisasContext, base); + +s->base.is_jmp = translate_one(env, s); +s->base.pc_next = s->pc; -if (tb_cflags(tb) & CF_LAST_IO) { -gen_io_end(); +if (s->base.is_jmp == DISAS_NEXT +&& (s->pc >= s->next_page_start +|| s->ex_value)) { +s->base.is_jmp =
[Qemu-devel] [PATCH 1/3] target/s390x: convert to DisasJumpType
The only non-trivial modification is the use of DISAS_TOO_MANY in the same way is used by the generic translation loop. Signed-off-by: Emilio G. Cota--- target/s390x/translate.c | 1267 +++--- 1 file changed, 632 insertions(+), 635 deletions(-) diff --git a/target/s390x/translate.c b/target/s390x/translate.c index b470d69..5346791 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -42,6 +42,7 @@ #include "exec/helper-gen.h" #include "trace-tcg.h" +#include "exec/translator.h" #include "exec/log.h" @@ -73,9 +74,6 @@ typedef struct { } u; } DisasCompare; -/* is_jmp field values */ -#define DISAS_EXCP DISAS_TARGET_0 - #ifdef DEBUG_INLINE_BRANCHES static uint64_t inline_branch_hit[CC_OP_MAX]; static uint64_t inline_branch_miss[CC_OP_MAX]; @@ -1087,26 +1085,24 @@ typedef struct { #define SPEC_r2_f12816 /* Return values from translate_one, indicating the state of the TB. */ -typedef enum { -/* Continue the TB. */ -NO_EXIT, -/* We have emitted one or more goto_tb. No fixup required. */ -EXIT_GOTO_TB, -/* We are not using a goto_tb (for whatever reason), but have updated - the PC (for whatever reason), so there's no need to do it again on - exiting the TB. */ -EXIT_PC_UPDATED, -/* We have updated the PC and CC values. */ -EXIT_PC_CC_UPDATED, -/* We are exiting the TB, but have neither emitted a goto_tb, nor - updated the PC for the next instruction to be executed. */ -EXIT_PC_STALE, -/* We are exiting the TB to the main loop. */ -EXIT_PC_STALE_NOCHAIN, -/* We are ending the TB with a noreturn function call, e.g. longjmp. - No following code will be executed. */ -EXIT_NORETURN, -} ExitStatus; + +/* We are not using a goto_tb (for whatever reason), but have updated + the PC (for whatever reason), so there's no need to do it again on + exiting the TB. */ +#define DISAS_PC_UPDATEDDISAS_TARGET_0 + +/* We have emitted one or more goto_tb. No fixup required. */ +#define DISAS_GOTO_TB DISAS_TARGET_1 + +/* We have updated the PC and CC values. */ +#define DISAS_PC_CC_UPDATED DISAS_TARGET_2 + +/* We are exiting the TB, but have neither emitted a goto_tb, nor + updated the PC for the next instruction to be executed. */ +#define DISAS_PC_STALE DISAS_TARGET_3 + +/* We are exiting the TB to the main loop. */ +#define DISAS_PC_STALE_NOCHAIN DISAS_TARGET_4 struct DisasInsn { unsigned opc:16; @@ -1121,7 +1117,7 @@ struct DisasInsn { void (*help_prep)(DisasContext *, DisasFields *, DisasOps *); void (*help_wout)(DisasContext *, DisasFields *, DisasOps *); void (*help_cout)(DisasContext *, DisasOps *); -ExitStatus (*help_op)(DisasContext *, DisasOps *); +DisasJumpType (*help_op)(DisasContext *, DisasOps *); uint64_t data; }; @@ -1143,11 +1139,11 @@ static void help_l2_shift(DisasContext *s, DisasFields *f, } } -static ExitStatus help_goto_direct(DisasContext *s, uint64_t dest) +static DisasJumpType help_goto_direct(DisasContext *s, uint64_t dest) { if (dest == s->next_pc) { per_branch(s, true); -return NO_EXIT; +return DISAS_NEXT; } if (use_goto_tb(s, dest)) { update_cc_op(s); @@ -1155,31 +1151,31 @@ static ExitStatus help_goto_direct(DisasContext *s, uint64_t dest) tcg_gen_goto_tb(0); tcg_gen_movi_i64(psw_addr, dest); tcg_gen_exit_tb((uintptr_t)s->tb); -return EXIT_GOTO_TB; +return DISAS_GOTO_TB; } else { tcg_gen_movi_i64(psw_addr, dest); per_branch(s, false); -return EXIT_PC_UPDATED; +return DISAS_PC_UPDATED; } } -static ExitStatus help_branch(DisasContext *s, DisasCompare *c, - bool is_imm, int imm, TCGv_i64 cdest) +static DisasJumpType help_branch(DisasContext *s, DisasCompare *c, + bool is_imm, int imm, TCGv_i64 cdest) { -ExitStatus ret; +DisasJumpType ret; uint64_t dest = s->pc + 2 * imm; TCGLabel *lab; /* Take care of the special cases first. */ if (c->cond == TCG_COND_NEVER) { -ret = NO_EXIT; +ret = DISAS_NEXT; goto egress; } if (is_imm) { if (dest == s->next_pc) { /* Branch to next. */ per_branch(s, true); -ret = NO_EXIT; +ret = DISAS_NEXT; goto egress; } if (c->cond == TCG_COND_ALWAYS) { @@ -1189,13 +1185,13 @@ static ExitStatus help_branch(DisasContext *s, DisasCompare *c, } else { if (!cdest) { /* E.g. bcr %r0 -> no branch. */ -ret = NO_EXIT; +ret = DISAS_NEXT; goto egress; } if (c->cond == TCG_COND_ALWAYS) { tcg_gen_mov_i64(psw_addr, cdest); per_branch(s, false); -
[Qemu-devel] [PATCH 2/3] target/s390x: convert to DisasContextBase
Notes: - Kept ctx.pc and ctx.next_pc; it would be very confusing to have ctx.base.pc_next and ctx.next_pc ! Instead, just updated ctx.base.pc_next where relevant. - Did not convert num_insns and is_jmp, since the corresponding code will go away in the next patch. - Avoided a checkpatch error in use_exit_tb. Signed-off-by: Emilio G. Cota--- target/s390x/translate.c | 85 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 5346791..dd504a1 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -52,14 +52,13 @@ typedef struct DisasInsn DisasInsn; typedef struct DisasFields DisasFields; struct DisasContext { -struct TranslationBlock *tb; +DisasContextBase base; const DisasInsn *insn; DisasFields *fields; uint64_t ex_value; uint64_t pc, next_pc; uint32_t ilen; enum cc_op cc_op; -bool singlestep_enabled; }; /* Information carried about a condition to be evaluated. */ @@ -81,8 +80,8 @@ static uint64_t inline_branch_miss[CC_OP_MAX]; static uint64_t pc_to_link_info(DisasContext *s, uint64_t pc) { -if (!(s->tb->flags & FLAG_MASK_64)) { -if (s->tb->flags & FLAG_MASK_32) { +if (!(s->base.tb->flags & FLAG_MASK_64)) { +if (s->base.tb->flags & FLAG_MASK_32) { return pc | 0x8000; } } @@ -196,7 +195,7 @@ static void per_branch(DisasContext *s, bool to_next) #ifndef CONFIG_USER_ONLY tcg_gen_movi_i64(gbea, s->pc); -if (s->tb->flags & FLAG_MASK_PER) { +if (s->base.tb->flags & FLAG_MASK_PER) { TCGv_i64 next_pc = to_next ? tcg_const_i64(s->next_pc) : psw_addr; gen_helper_per_branch(cpu_env, gbea, next_pc); if (to_next) { @@ -210,7 +209,7 @@ static void per_branch_cond(DisasContext *s, TCGCond cond, TCGv_i64 arg1, TCGv_i64 arg2) { #ifndef CONFIG_USER_ONLY -if (s->tb->flags & FLAG_MASK_PER) { +if (s->base.tb->flags & FLAG_MASK_PER) { TCGLabel *lab = gen_new_label(); tcg_gen_brcond_i64(tcg_invert_cond(cond), arg1, arg2, lab); @@ -250,7 +249,7 @@ static inline uint64_t ld_code4(CPUS390XState *env, uint64_t pc) static int get_mem_index(DisasContext *s) { -switch (s->tb->flags & FLAG_MASK_ASC) { +switch (s->base.tb->flags & FLAG_MASK_ASC) { case PSW_ASC_PRIMARY >> FLAG_MASK_PSW_SHIFT: return 0; case PSW_ASC_SECONDARY >> FLAG_MASK_PSW_SHIFT: @@ -315,7 +314,7 @@ static inline void gen_trap(DisasContext *s) #ifndef CONFIG_USER_ONLY static void check_privileged(DisasContext *s) { -if (s->tb->flags & FLAG_MASK_PSTATE) { +if (s->base.tb->flags & FLAG_MASK_PSTATE) { gen_program_exception(s, PGM_PRIVILEGED); } } @@ -324,7 +323,7 @@ static void check_privileged(DisasContext *s) static TCGv_i64 get_address(DisasContext *s, int x2, int b2, int d2) { TCGv_i64 tmp = tcg_temp_new_i64(); -bool need_31 = !(s->tb->flags & FLAG_MASK_64); +bool need_31 = !(s->base.tb->flags & FLAG_MASK_64); /* Note that d2 is limited to 20 bits, signed. If we crop negative displacements early we create larger immedate addends. */ @@ -537,9 +536,9 @@ static void gen_op_calc_cc(DisasContext *s) static bool use_exit_tb(DisasContext *s) { -return (s->singlestep_enabled || -(tb_cflags(s->tb) & CF_LAST_IO) || -(s->tb->flags & FLAG_MASK_PER)); +return s->base.singlestep_enabled || +(tb_cflags(s->base.tb) & CF_LAST_IO) || +(s->base.tb->flags & FLAG_MASK_PER); } static bool use_goto_tb(DisasContext *s, uint64_t dest) @@ -548,7 +547,7 @@ static bool use_goto_tb(DisasContext *s, uint64_t dest) return false; } #ifndef CONFIG_USER_ONLY -return (dest & TARGET_PAGE_MASK) == (s->tb->pc & TARGET_PAGE_MASK) || +return (dest & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) || (dest & TARGET_PAGE_MASK) == (s->pc & TARGET_PAGE_MASK); #else return true; @@ -1150,7 +1149,7 @@ static DisasJumpType help_goto_direct(DisasContext *s, uint64_t dest) per_breaking_event(s); tcg_gen_goto_tb(0); tcg_gen_movi_i64(psw_addr, dest); -tcg_gen_exit_tb((uintptr_t)s->tb); +tcg_gen_exit_tb((uintptr_t)s->base.tb); return DISAS_GOTO_TB; } else { tcg_gen_movi_i64(psw_addr, dest); @@ -1211,14 +1210,14 @@ static DisasJumpType help_branch(DisasContext *s, DisasCompare *c, /* Branch not taken. */ tcg_gen_goto_tb(0); tcg_gen_movi_i64(psw_addr, s->next_pc); -tcg_gen_exit_tb((uintptr_t)s->tb + 0); +tcg_gen_exit_tb((uintptr_t)s->base.tb + 0); /* Branch taken. */ gen_set_label(lab); per_breaking_event(s); tcg_gen_goto_tb(1); tcg_gen_movi_i64(psw_addr,
[Qemu-devel] [PATCH 0/3] target/s390x: translation loop conversion
Tested with the "Moon Buggy" image: http://www.qemu-advent-calendar.org/2014/#day-22 Thanks, Emilio
[Qemu-devel] [Bug 589315] Re: qemu: Improve error reporting when migration can't connect
** Changed in: qemu Status: In Progress => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/589315 Title: qemu: Improve error reporting when migration can't connect Status in QEMU: Fix Released Bug description: Tested with upstream qemu as of Jun 3 2010 If the source qemu instance can't connect to the migration destination (say there is no listening QEMU instance, or port is blocked by a firewall), all we get is info migrate -> Migration status: failed. This is all we have to report back to libvirt users if their firewall is misconfigured, which is crappy. Ideally, if we can't connect, migration would fail immediately with a relevant message and strerror(). More info from 'info migrate' would be nice too, no idea how this will play with QMP though. As a slightly related issue, try entering migrate tcp:127.0.0.0:6000 We get a 'migration failed' error, and then the monitor hangs! To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/589315/+subscriptions
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/3] Sam460ex emulation
On 16.02.2018 11:55, BALATON Zoltan wrote: > On Fri, 16 Feb 2018, Thomas Huth wrote: >> On 15.02.2018 22:27, BALATON Zoltan wrote: >>> Remaining patches for Sam460ex emulation. The original cover letter >>> with more details is here: >>> >>> http://lists.nongnu.org/archive/html/qemu-ppc/2017-08/msg00112.html >>> >>> We'll need to also add binaries for firmware (customised u-boot >>> version) and dtb but I'm not sure how to submit those. >> >> For the dtb, I think you could simply provide a patch that adds the dts >> file to the pc-bios directory and another one that adds the dtb. Just >> like it is already done with pc-bios/bamboo.dts / pc-bios/bamboo.dtb. > > OK thanks, I'll do that. Does it have to be two separate patches? I don't think so, I just thought that would be cleaner ... but one patch should be fine, too, I guess. David? >> For u-boot, can you use the same upstream level as e500 ? I.e. check >> whether "git submodule status roms/u-boot" is fine for you? If that's >> ok, just do a "git submodule update roms/u-boot" and build uboot from >> that directory - you then can submit a binary patch with that file for >> pc-bios, too. >> >> In case you need another u-boot version, I think you've got to update >> the submodule to the newer upstream version first, and then also rebuild >> the e500 binary... Cumbersome, but that's necessary since we've got to >> ship the u-boot sources in the QEMU release tarballs, too, to be >> compliant with the GPL. > > Unfortunately we can't use the same u-boot as e500 because this board > uses a forked and patched version which is not in upstream u-boot and > upstream u-boot has even dropped support for this CPU in latest version > so we actually need an older version (with patches) and not a newer one. That's very unfortunate ... any chance that you could try to get that CPU activated in upstream u-boot again and get the patches included there? > Therefore, it needs to be a binary built from a separate source so I > think a new submodule will need to be added for this. How to do that? > Where to host this git repo? Should I put it on github and refer to that > as an external repo or should it be hosted in qemu repo somehow? No clue ... adding Stefan and Jeff to CC:, maybe they can recommend something here. Thomas
[Qemu-devel] [PATCH v7 08/12] s390-ccw: read stage2 boot loader data to find menu
Read the stage2 boot loader data block-by-block. We scan the current block for the string "zIPL" to detect the start of the boot menu banner. We then load the adjacent blocks (previous block and next block) to account for the possibility of menu data spanning multiple blocks. Signed-off-by: Collin L. WallingReviewed-by: Thomas Huth --- pc-bios/s390-ccw/bootmap.c | 94 +++--- pc-bios/s390-ccw/bootmap.h | 23 +++- pc-bios/s390-ccw/menu.c| 5 +++ pc-bios/s390-ccw/menu.h| 1 + 4 files changed, 116 insertions(+), 7 deletions(-) diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 092fb35..4c6ccf3 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -13,6 +13,7 @@ #include "bootmap.h" #include "virtio.h" #include "bswap.h" +#include "menu.h" #ifdef DEBUG /* #define DEBUG_FALLBACK */ @@ -83,6 +84,10 @@ static void jump_to_IPL_code(uint64_t address) static unsigned char _bprs[8*1024]; /* guessed "max" ECKD sector size */ static const int max_bprs_entries = sizeof(_bprs) / sizeof(ExtEckdBlockPtr); +static uint8_t _s2[MAX_SECTOR_SIZE * 3] __attribute__((__aligned__(PAGE_SIZE))); +static void *s2_prev_blk = _s2; +static void *s2_cur_blk = _s2 + MAX_SECTOR_SIZE; +static void *s2_next_blk = _s2 + MAX_SECTOR_SIZE * 2; static inline void verify_boot_info(BootInfo *bip) { @@ -182,7 +187,76 @@ static block_number_t load_eckd_segments(block_number_t blk, uint64_t *address) return block_nr; } -static void run_eckd_boot_script(block_number_t bmt_block_nr) +static bool find_zipl_boot_menu_banner(int *offset) +{ +int i; + +/* Menu banner starts with "zIPL" */ +for (i = 0; i < virtio_get_block_size() - 4; i++) { +if (magic_match(s2_cur_blk + i, ZIPL_MAGIC_EBCDIC)) { +*offset = i; +return true; +} +} + +return false; +} + +static int eckd_get_boot_menu_index(block_number_t s1b_block_nr) +{ +block_number_t cur_block_nr; +block_number_t prev_block_nr = 0; +block_number_t next_block_nr = 0; +EckdStage1b *s1b = (void *)sec; +int offset; +int i; + +/* Get Stage1b data */ +memset(sec, FREE_SPACE_FILLER, sizeof(sec)); +read_block(s1b_block_nr, s1b, "Cannot read stage1b boot loader"); + +memset(_s2, FREE_SPACE_FILLER, sizeof(_s2)); + +/* Get Stage2 data */ +for (i = 0; i < STAGE2_BLK_CNT_MAX; i++) { +cur_block_nr = eckd_block_num(>seek[i].chs); + +if (!cur_block_nr) { +break; +} + +read_block(cur_block_nr, s2_cur_blk, "Cannot read stage2 boot loader"); + +if (find_zipl_boot_menu_banner()) { +/* Load the adjacent blocks to account for the + * possibility of menu data spanning multiple blocks. + */ +if (prev_block_nr) { +read_block(prev_block_nr, s2_prev_blk, + "Cannot read stage2 boot loader"); +} + +if (i + 1 < STAGE2_BLK_CNT_MAX) { +next_block_nr = eckd_block_num(>seek[i + 1].chs); +} + +if (next_block_nr) { +read_block(next_block_nr, s2_next_blk, + "Cannot read stage2 boot loader"); +} + +return menu_get_zipl_boot_index(s2_cur_blk, offset); +} + +prev_block_nr = cur_block_nr; +} + +sclp_print("No zipl boot menu data found. Booting default entry."); +return 0; +} + +static void run_eckd_boot_script(block_number_t bmt_block_nr, + block_number_t s1b_block_nr) { int i; unsigned int loadparm = get_loadparm_index(); @@ -191,6 +265,10 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr) BootMapTable *bmt = (void *)sec; BootMapScript *bms = (void *)sec; +if (menu_check_flags(BOOT_MENU_FLAG_CMD_OPTS | BOOT_MENU_FLAG_ZIPL_OPTS)) { +loadparm = eckd_get_boot_menu_index(s1b_block_nr); +} + debug_print_int("loadparm", loadparm); IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than" " maximum number of boot entries allowed"); @@ -223,7 +301,7 @@ static void ipl_eckd_cdl(void) XEckdMbr *mbr; EckdCdlIpl2 *ipl2 = (void *)sec; IplVolumeLabel *vlbl = (void *)sec; -block_number_t bmt_block_nr; +block_number_t bmt_block_nr, s1b_block_nr; /* we have just read the block #0 and recognized it as "IPL1" */ sclp_print("CDL\n"); @@ -241,6 +319,9 @@ static void ipl_eckd_cdl(void) /* save pointer to Boot Map Table */ bmt_block_nr = eckd_block_num(>blockptr.xeckd.bptr.chs); +/* save pointer to Stage1b Data */ +s1b_block_nr = eckd_block_num(>stage1.seek[0].chs); + memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(2, vlbl, "Cannot read Volume Label at block 2");
[Qemu-devel] [PATCH v7 09/12] s390-ccw: print zipl boot menu
When the boot menu options are present and the guest's disk has been configured by the zipl tool, then the user will be presented with an interactive boot menu with labeled entries. An example of what the menu might look like: zIPL v1.37.1-build-20170714 interactive boot menu. 0. default (linux-4.13.0) 1. linux-4.13.0 2. performance 3. kvm Signed-off-by: Collin L. WallingReviewed-by: Thomas Huth --- pc-bios/s390-ccw/menu.c | 49 - 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c index 5790e0c..9631ac0 100644 --- a/pc-bios/s390-ccw/menu.c +++ b/pc-bios/s390-ccw/menu.c @@ -10,13 +10,60 @@ */ #include "menu.h" +#include "s390-ccw.h" + +/* Offsets from zipl fields to zipl banner start */ +#define ZIPL_TIMEOUT_OFFSET 138 +#define ZIPL_FLAG_OFFSET140 static uint8_t flags; static uint64_t timeout; +static int get_boot_index(int entries) +{ +return 0; /* Implemented next patch */ +} + +static void zipl_println(const char *data, size_t len) +{ +char buf[len + 2]; + +ebcdic_to_ascii(data, buf, len); +buf[len] = '\n'; +buf[len + 1] = '\0'; + +sclp_print(buf); +} + int menu_get_zipl_boot_index(const void *stage2, int offset) { -return 0; /* implemented next patch */ +const char *data = stage2 + offset; +uint16_t zipl_flag = *(uint16_t *)(data - ZIPL_FLAG_OFFSET); +uint16_t zipl_timeout = *(uint16_t *)(data - ZIPL_TIMEOUT_OFFSET); +size_t len; +int ct; + +if (flags & BOOT_MENU_FLAG_ZIPL_OPTS) { +if (!zipl_flag) { +return 0; /* Boot default */ +} +timeout = zipl_timeout * 1000; +} + +/* Print and count all menu items, including the banner */ +for (ct = 0; *data; ct++) { +len = strlen(data); +zipl_println(data, len); +data += len + 1; + +if (ct < 2) { +sclp_print("\n"); +} +} + +sclp_print("\n"); + +return get_boot_index(ct - 1); } void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout) -- 2.7.4
[Qemu-devel] [PATCH v7 10/12] s390-ccw: read user input for boot index via the SCLP console
Implements an sclp_read function to capture input from the console and a wrapper function that handles parsing certain characters and adding input to a buffer. The input is checked for any erroneous values and is handled appropriately. A prompt will persist until input is entered or the timeout expires (if one was set). Example: Please choose (default will boot in 10 seconds): Correct input will boot the respective boot index. If the user's input is empty, 0, or if the timeout expires, then the default zipl entry will be chosen. If the input is within the range of available boot entries, then the selection will be booted. Any erroneous input will cancel the timeout and re-prompt the user. Signed-off-by: Collin L. WallingReviewed-by: Thomas Huth --- pc-bios/s390-ccw/menu.c | 146 +++- pc-bios/s390-ccw/s390-ccw.h | 2 + pc-bios/s390-ccw/sclp.c | 19 ++ pc-bios/s390-ccw/virtio.c | 2 +- 4 files changed, 167 insertions(+), 2 deletions(-) diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c index 9631ac0..9601043 100644 --- a/pc-bios/s390-ccw/menu.c +++ b/pc-bios/s390-ccw/menu.c @@ -12,16 +12,160 @@ #include "menu.h" #include "s390-ccw.h" +#define KEYCODE_NO_INP '\0' +#define KEYCODE_ESCAPE '\033' +#define KEYCODE_BACKSP '\177' +#define KEYCODE_ENTER '\r' + /* Offsets from zipl fields to zipl banner start */ #define ZIPL_TIMEOUT_OFFSET 138 #define ZIPL_FLAG_OFFSET140 +#define TOD_CLOCK_MILLISECOND 0x3e8000 + static uint8_t flags; static uint64_t timeout; +static inline void enable_clock_int(void) +{ +uint64_t tmp = 0; + +asm volatile( +"stctg 0,0,%0\n" +"oi 6+%0, 0x8\n" +"lctlg 0,0,%0" +: : "Q" (tmp) : "memory" +); +} + +static inline void disable_clock_int(void) +{ +uint64_t tmp = 0; + +asm volatile( +"stctg 0,0,%0\n" +"ni 6+%0, 0xf7\n" +"lctlg 0,0,%0" +: : "Q" (tmp) : "memory" +); +} + +static inline void set_clock_comparator(uint64_t time) +{ +asm volatile("sckc %0" : : "Q" (time)); +} + +static inline bool check_clock_int(void) +{ +uint16_t *code = (uint16_t *)0x86; /* low-core external interrupt code */ + +consume_sclp_int(); + +return *code == 0x1004; +} + +static int read_prompt(char *buf, size_t len) +{ +char inp[2] = {}; +uint8_t idx = 0; +uint64_t time; + +if (timeout) { +time = get_clock() + timeout * TOD_CLOCK_MILLISECOND; +set_clock_comparator(time); +enable_clock_int(); +timeout = 0; +} + +while (!check_clock_int()) { + +sclp_read(inp, 1); /* Process only one character at a time */ + +switch (inp[0]) { +case KEYCODE_NO_INP: +case KEYCODE_ESCAPE: +continue; +case KEYCODE_BACKSP: +if (idx > 0) { +buf[--idx] = 0; +sclp_print("\b \b"); +} +continue; +case KEYCODE_ENTER: +disable_clock_int(); +return idx; +default: +/* Echo input and add to buffer */ +if (idx < len) { +buf[idx++] = inp[0]; +sclp_print(inp); +} +} +} + +disable_clock_int(); +*buf = 0; + +return 0; +} + +static int get_index(void) +{ +char buf[10]; +int len; +int i; + +memset(buf, 0, sizeof(buf)); + +len = read_prompt(buf, sizeof(buf)); + +/* If no input, boot default */ +if (len == 0) { +return 0; +} + +/* Check for erroneous input */ +for (i = 0; i < len; i++) { +if (!isdigit(buf[i])) { +return -1; +} +} + +return atoui(buf); +} + +static void boot_menu_prompt(bool retry) +{ +char tmp[6]; + +if (retry) { +sclp_print("\nError: undefined configuration" + "\nPlease choose:\n"); +} else if (timeout > 0) { +sclp_print("Please choose (default will boot in "); +sclp_print(uitoa(timeout / 1000, tmp, sizeof(tmp))); +sclp_print(" seconds):\n"); +} else { +sclp_print("Please choose:\n"); +} +} + static int get_boot_index(int entries) { -return 0; /* Implemented next patch */ +int boot_index; +bool retry = false; +char tmp[5]; + +do { +boot_menu_prompt(retry); +boot_index = get_index(); +retry = true; +} while (boot_index < 0 || boot_index >= entries); + +sclp_print("\nBooting entry #"); +sclp_print(uitoa(boot_index, tmp, sizeof(tmp))); + +return boot_index; } static void zipl_println(const char *data, size_t len) diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index 25d4d21..a7e6253 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -71,6 +71,7 @@ unsigned int get_loadparm_index(void);
[Qemu-devel] [PATCH v7 11/12] s390-ccw: set cp_receive mask only when needed and consume pending service irqs
It is possible while waiting for multiple types of external interrupts that we might have pending irqs remaining between irq consumption and irq-type disabling. Those interrupts could potentially propagate to the guest after IPL completes and cause unwanted behavior. As it is today, the SCLP will only recognize write events that are enabled by the control program's send and receive masks. To limit the window for, and prevent further irqs from, ASCII console events (specifically keystrokes), we should only enable the control program's receive mask when we need it. As an additional measure, once we've disabled/cleared the control program's receive mask, we will print an empty string in order to consume any pending service interrupt. Signed-off-by: Collin L. Walling--- pc-bios/s390-ccw/menu.c | 5 + pc-bios/s390-ccw/s390-ccw.h | 1 + pc-bios/s390-ccw/sclp.c | 10 -- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c index 9601043..14410a8 100644 --- a/pc-bios/s390-ccw/menu.c +++ b/pc-bios/s390-ccw/menu.c @@ -11,6 +11,7 @@ #include "menu.h" #include "s390-ccw.h" +#include "sclp.h" #define KEYCODE_NO_INP '\0' #define KEYCODE_ESCAPE '\033' @@ -117,8 +118,12 @@ static int get_index(void) memset(buf, 0, sizeof(buf)); +sclp_set_write_mask(SCLP_EVENT_MASK_MSG_ASCII, SCLP_EVENT_MASK_MSG_ASCII); len = read_prompt(buf, sizeof(buf)); +sclp_set_write_mask(0, SCLP_EVENT_MASK_MSG_ASCII); +sclp_print(""); /* Clear any pending service int */ + /* If no input, boot default */ if (len == 0) { return 0; diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h index a7e6253..ebdcf86 100644 --- a/pc-bios/s390-ccw/s390-ccw.h +++ b/pc-bios/s390-ccw/s390-ccw.h @@ -69,6 +69,7 @@ unsigned int get_loadparm_index(void); /* sclp.c */ void sclp_print(const char *string); +void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask); void sclp_setup(void); void sclp_get_loadparm_ascii(char *loadparm); int sclp_read(char *str, size_t count); diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c index a2f25eb..3836cb4 100644 --- a/pc-bios/s390-ccw/sclp.c +++ b/pc-bios/s390-ccw/sclp.c @@ -46,23 +46,21 @@ static int sclp_service_call(unsigned int command, void *sccb) return 0; } -static void sclp_set_write_mask(void) +void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask) { WriteEventMask *sccb = (void *)_sccb; sccb->h.length = sizeof(WriteEventMask); sccb->mask_length = sizeof(unsigned int); -sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII; -sccb->cp_receive_mask = SCLP_EVENT_MASK_MSG_ASCII; -sccb->send_mask = SCLP_EVENT_MASK_MSG_ASCII; -sccb->cp_send_mask = SCLP_EVENT_MASK_MSG_ASCII; +sccb->cp_receive_mask = receive_mask; +sccb->cp_send_mask = send_mask; sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb); } void sclp_setup(void) { -sclp_set_write_mask(); +sclp_set_write_mask(0, SCLP_EVENT_MASK_MSG_ASCII); } long write(int fd, const void *str, size_t len) -- 2.7.4
[Qemu-devel] [PATCH v7 04/12] s390-ccw: update libc
Moved: memcmp from bootmap.h to libc.h (renamed from _memcmp) strlen from sclp.c to libc.h (renamed from _strlen) Added C standard functions: isdigit Added non C-standard function: uitoa atoui Signed-off-by: Collin L. WallingAcked-by: Christian Borntraeger Reviewed-by: Janosch Frank --- pc-bios/s390-ccw/Makefile | 2 +- pc-bios/s390-ccw/bootmap.c | 4 +-- pc-bios/s390-ccw/bootmap.h | 16 + pc-bios/s390-ccw/libc.c| 89 ++ pc-bios/s390-ccw/libc.h| 37 +-- pc-bios/s390-ccw/main.c| 17 + pc-bios/s390-ccw/sclp.c| 10 +- 7 files changed, 130 insertions(+), 45 deletions(-) create mode 100644 pc-bios/s390-ccw/libc.c diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile index 6d0c2ee..9f7904f 100644 --- a/pc-bios/s390-ccw/Makefile +++ b/pc-bios/s390-ccw/Makefile @@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw) .PHONY : all clean build-all -OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o +OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS)) QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index a94638d..092fb35 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -506,7 +506,7 @@ static bool is_iso_bc_entry_compatible(IsoBcSection *s) "Failed to read image sector 0"); /* Checking bytes 8 - 32 for S390 Linux magic */ -return !_memcmp(magic_sec + 8, linux_s390_magic, 24); +return !memcmp(magic_sec + 8, linux_s390_magic, 24); } /* Location of the current sector of the directory */ @@ -635,7 +635,7 @@ static uint32_t find_iso_bc(void) if (vd->type == VOL_DESC_TYPE_BOOT) { IsoVdElTorito *et = >vd.boot; -if (!_memcmp(>el_torito[0], el_torito_magic, 32)) { +if (!memcmp(>el_torito[0], el_torito_magic, 32)) { return bswap32(et->bc_offset); } } diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h index 4bd95cd..4cf7e1e 100644 --- a/pc-bios/s390-ccw/bootmap.h +++ b/pc-bios/s390-ccw/bootmap.h @@ -328,20 +328,6 @@ static inline bool magic_match(const void *data, const void *magic) return *((uint32_t *)data) == *((uint32_t *)magic); } -static inline int _memcmp(const void *s1, const void *s2, size_t n) -{ -int i; -const uint8_t *p1 = s1, *p2 = s2; - -for (i = 0; i < n; i++) { -if (p1[i] != p2[i]) { -return p1[i] > p2[i] ? 1 : -1; -} -} - -return 0; -} - static inline uint32_t iso_733_to_u32(uint64_t x) { return (uint32_t)x; @@ -434,7 +420,7 @@ const uint8_t vol_desc_magic[] = "CD001"; static inline bool is_iso_vd_valid(IsoVolDesc *vd) { -return !_memcmp(>ident[0], vol_desc_magic, 5) && +return !memcmp(>ident[0], vol_desc_magic, 5) && vd->version == 0x1 && vd->type <= VOL_DESC_TYPE_PARTITION; } diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c new file mode 100644 index 000..a144388 --- /dev/null +++ b/pc-bios/s390-ccw/libc.c @@ -0,0 +1,89 @@ +/* + * libc-style definitions and functions + * + * Copyright 2018 IBM Corp. + * Author(s): Collin L. Walling + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include "libc.h" +#include "s390-ccw.h" + +/** + * atoui: + * @str: the string to be converted. + * + * Given a string @str, convert it to an integer. Leading spaces are + * ignored. Any other non-numerical value will terminate the conversion + * and return 0. This function only handles numbers between 0 and + * UINT64_MAX inclusive. + * + * Returns: an integer converted from the string @str, or the number 0 + * if an error occurred. + */ +uint64_t atoui(const char *str) +{ +int val = 0; + +if (!str || !str[0]) { +return 0; +} + +while (*str == ' ') { +str++; +} + +while (*str) { +if (!isdigit(*str)) { +break; +} +val = val * 10 + *str - '0'; +str++; +} + +return val; +} + +/** + * uitoa: + * @num: an integer (base 10) to be converted. + * @str: a pointer to a string to store the conversion. + * @len: the length of the passed string. + * + * Given an integer @num, convert it to a string. The string @str must be + * allocated beforehand. The resulting string will be null terminated and + * returned. This function only handles numbers
[Qemu-devel] [PATCH v7 07/12] s390-ccw: set up interactive boot menu parameters
Reads boot menu flag and timeout values from the iplb and sets the respective fields for the menu. Signed-off-by: Collin L. WallingReviewed-by: Thomas Huth --- pc-bios/s390-ccw/Makefile | 2 +- pc-bios/s390-ccw/main.c | 24 pc-bios/s390-ccw/menu.c | 26 ++ pc-bios/s390-ccw/menu.h | 23 +++ 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 pc-bios/s390-ccw/menu.c create mode 100644 pc-bios/s390-ccw/menu.h diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile index 9f7904f..1712c2d 100644 --- a/pc-bios/s390-ccw/Makefile +++ b/pc-bios/s390-ccw/Makefile @@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw) .PHONY : all clean build-all -OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o +OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o virtio-blkdev.o libc.o menu.o QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS)) QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index e41b264..c643a6b 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -11,6 +11,7 @@ #include "libc.h" #include "s390-ccw.h" #include "virtio.h" +#include "menu.h" char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE))); static SubChannelId blk_schid = { .one = 1 }; @@ -18,6 +19,9 @@ IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); static char loadparm[8] = { 0, 0, 0, 0, 0, 0, 0, 0 }; QemuIplParameters qipl; +#define LOADPARM_PROMPT "PROMPT " +#define LOADPARM_EMPTY "" + /* * Priniciples of Operations (SA22-7832-09) chapter 17 requires that * a subsystem-identification is at 184-187 and bytes 188-191 are zero @@ -74,6 +78,25 @@ static bool find_dev(Schib *schib, int dev_no) return false; } +static void menu_setup(void) +{ +if (memcmp(loadparm, LOADPARM_PROMPT, 8) == 0) { +menu_set_parms(BOOT_MENU_FLAG_CMD_OPTS, 0); +return; +} + +/* If loadparm was set to any other value, then do not enable menu */ +if (memcmp(loadparm, LOADPARM_EMPTY, 8) != 0) { +return; +} + +switch (iplb.pbt) { +case S390_IPL_TYPE_CCW: +menu_set_parms(qipl.boot_menu_flags, qipl.boot_menu_timeout); +return; +} +} + static void virtio_setup(void) { Schib schib; @@ -117,6 +140,7 @@ static void virtio_setup(void) default: panic("List-directed IPL not supported yet!\n"); } +menu_setup(); } else { for (ssid = 0; ssid < 0x3; ssid++) { blk_schid.ssid = ssid; diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c new file mode 100644 index 000..3056cfc --- /dev/null +++ b/pc-bios/s390-ccw/menu.c @@ -0,0 +1,26 @@ +/* + * QEMU S390 Interactive Boot Menu + * + * Copyright 2018 IBM Corp. + * Author: Collin L. Walling + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at + * your option) any later version. See the COPYING file in the top-level + * directory. + */ + +#include "menu.h" + +static uint8_t flags; +static uint64_t timeout; + +void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout) +{ +flags = boot_menu_flag; +timeout = boot_menu_timeout; +} + +int menu_check_flags(uint8_t check_flags) +{ +return flags & check_flags; +} diff --git a/pc-bios/s390-ccw/menu.h b/pc-bios/s390-ccw/menu.h new file mode 100644 index 000..7df4114 --- /dev/null +++ b/pc-bios/s390-ccw/menu.h @@ -0,0 +1,23 @@ +/* + * QEMU S390 Interactive Boot Menu + * + * Copyright 2018 IBM Corp. + * Author: Collin L. Walling + * + * This work is licensed under the terms of the GNU GPL, version 2 or (at + * your option) any later version. See the COPYING file in the top-level + * directory. + */ + +#ifndef MENU_H +#define MENU_H + +#include "libc.h" + +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 + +void menu_set_parms(uint8_t boot_menu_flags, uint32_t boot_menu_timeout); +bool menu_check_flags(uint8_t check_flags); + +#endif /* MENU_H */ -- 2.7.4
[Qemu-devel] [PATCH v7 12/12] s390-ccw: interactive boot menu for scsi
Interactive boot menu for scsi. This follows a similar procedure as the interactive menu for eckd dasd. An example follows: s390x Enumerated Boot Menu. 3 entries detected. Select from index 0 to 2. Signed-off-by: Collin L. WallingReviewed-by: Thomas Huth --- pc-bios/s390-ccw/bootmap.c | 4 pc-bios/s390-ccw/main.c| 1 + pc-bios/s390-ccw/menu.c| 14 ++ pc-bios/s390-ccw/menu.h| 1 + 4 files changed, 20 insertions(+) diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 4c6ccf3..05d3308 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -568,6 +568,10 @@ static void ipl_scsi(void) debug_print_int("program table entries", program_table_entries); IPL_assert(program_table_entries != 0, "Empty Program Table"); +if (menu_check_flags(BOOT_MENU_FLAG_CMD_OPTS)) { +loadparm = menu_get_enum_boot_index(program_table_entries); +} + debug_print_int("loadparm", loadparm); IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than" " maximum number of boot entries allowed"); diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index c643a6b..2226871 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -92,6 +92,7 @@ static void menu_setup(void) switch (iplb.pbt) { case S390_IPL_TYPE_CCW: +case S390_IPL_TYPE_QEMU_SCSI: menu_set_parms(qipl.boot_menu_flags, qipl.boot_menu_timeout); return; } diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c index 14410a8..d104327 100644 --- a/pc-bios/s390-ccw/menu.c +++ b/pc-bios/s390-ccw/menu.c @@ -215,6 +215,20 @@ int menu_get_zipl_boot_index(const void *stage2, int offset) return get_boot_index(ct - 1); } +int menu_get_enum_boot_index(int entries) +{ +char tmp[4]; + +sclp_print("s390x Enumerated Boot Menu.\n\n"); + +sclp_print(uitoa(entries, tmp, sizeof(tmp))); +sclp_print(" entries detected. Select from boot index 0 to "); +sclp_print(uitoa(entries - 1, tmp, sizeof(tmp))); +sclp_print(".\n\n"); + +return get_boot_index(entries); +} + void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout) { flags = boot_menu_flag; diff --git a/pc-bios/s390-ccw/menu.h b/pc-bios/s390-ccw/menu.h index c603de3..790516c 100644 --- a/pc-bios/s390-ccw/menu.h +++ b/pc-bios/s390-ccw/menu.h @@ -18,6 +18,7 @@ #define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 int menu_get_zipl_boot_index(const void *stage2, int offset); +int menu_get_enum_boot_index(int entries); void menu_set_parms(uint8_t boot_menu_flags, uint32_t boot_menu_timeout); bool menu_check_flags(uint8_t check_flags); -- 2.7.4
[Qemu-devel] [PATCH v7 05/12] s390-ccw: move auxiliary IPL data to separate location
The s390-ccw firmware needs some information in support of the boot process which is not available on the native machine. Examples are the netboot firmware load address and now the boot menu parameters. While storing that data in unused fields of the IPL parameter block works, that approach could create problems if the parameter block definition should change in the future. Because then a guest could overwrite these fields using the set IPLB diagnose. In fact the data in question is of more global nature and not really tied to an IPL device, so separating it is rather logical. This commit introduces a new structure to hold firmware relevant IPL parameters set by QEMU. The data is stored at location 204 (dec) and can contain up to 7 32-bit words. This area is available to programming in the z/Architecture Principles of Operation and can thus safely be used by the firmware until the IPL has completed. Signed-off-by: Viktor MihajlovskiSigned-off-by: Collin L. Walling --- hw/s390x/ipl.c | 19 ++- hw/s390x/ipl.h | 19 +-- pc-bios/s390-ccw/iplb.h | 20 ++-- pc-bios/s390-ccw/main.c | 6 +- 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 0d06fc1..31565ce 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -399,6 +399,21 @@ void s390_reipl_request(void) qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); } +static void s390_ipl_prepare_qipl(S390CPU *cpu) +{ +S390IPLState *ipl = get_ipl_device(); +uint8_t *addr; +uint64_t len = 4096; + +addr = cpu_physical_memory_map(cpu->env.psa, , 1); +if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) { +error_report("Cannot set QEMU IPL parameters"); +return; +} +memcpy(addr + QIPL_ADDRESS, >iplb.qipl, sizeof(QemuIplParameters)); +cpu_physical_memory_unmap(addr, len, 1, len); +} + void s390_ipl_prepare_cpu(S390CPU *cpu) { S390IPLState *ipl = get_ipl_device(); @@ -418,8 +433,10 @@ void s390_ipl_prepare_cpu(S390CPU *cpu) error_report_err(err); vm_stop(RUN_STATE_INTERNAL_ERROR); } -ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr); +ipl->iplb.qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr); } +s390_ipl_prepare_qipl(cpu); + } static void s390_ipl_reset(DeviceState *dev) diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index 8a705e0..74469b1 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -16,8 +16,7 @@ #include "cpu.h" struct IplBlockCcw { -uint64_t netboot_start_addr; -uint8_t reserved0[77]; +uint8_t reserved0[85]; uint8_t ssid; uint16_t devno; uint8_t vm_flags; @@ -59,6 +58,21 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; #define DIAG308_FLAGS_LP_VALID 0x80 +#define QIPL_ADDRESS 0xcc + +/* + * The QEMU IPL Parameters will be stored 32-bit word aligned. + * Placement of data fields in this area must account for + * their alignment needs. + * The entire structure must not be larger than 28 bytes. + */ +struct QemuIplParameters { +uint8_t reserved1[4]; +uint64_t netboot_start_addr; +uint8_t reserved2[16]; +} QEMU_PACKED; +typedef struct QemuIplParameters QemuIplParameters; + union IplParameterBlock { struct { uint32_t len; @@ -74,6 +88,7 @@ union IplParameterBlock { IplBlockFcp fcp; IplBlockQemuScsi scsi; }; +QemuIplParameters qipl; } QEMU_PACKED; struct { uint8_t reserved1[110]; diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h index 890aed9..a23237e 100644 --- a/pc-bios/s390-ccw/iplb.h +++ b/pc-bios/s390-ccw/iplb.h @@ -13,8 +13,7 @@ #define IPLB_H struct IplBlockCcw { -uint64_t netboot_start_addr; -uint8_t reserved0[77]; +uint8_t reserved0[85]; uint8_t ssid; uint16_t devno; uint8_t vm_flags; @@ -73,6 +72,23 @@ typedef struct IplParameterBlock IplParameterBlock; extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); +#define QIPL_ADDRESS 0xcc + +/* + * The QEMU IPL Parameters will be stored 32-bit word aligned. + * Placement of data fields in this area must account for + * their alignment needs. + * The entire structure must not be larger than 28 bytes. + */ +struct QemuIplParameters { +uint8_t reserved1[4]; +uint64_t netboot_start_addr; +uint8_t reserved2[16]; +} __attribute__ ((packed)); +typedef struct QemuIplParameters QemuIplParameters; + +extern QemuIplParameters qipl; + #define S390_IPL_TYPE_FCP 0x00 #define S390_IPL_TYPE_CCW 0x02 #define S390_IPL_TYPE_QEMU_SCSI 0xff diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c index e857ce4..e41b264 100644 --- a/pc-bios/s390-ccw/main.c +++ b/pc-bios/s390-ccw/main.c @@ -16,6 +16,7 @@ char stack[PAGE_SIZE * 8] __attribute__((__aligned__(PAGE_SIZE)));
[Qemu-devel] [PATCH v7 01/12] s390-ccw: refactor boot map table code
Some ECKD bootmap code was using structs designed for SCSI. Even though this works, it confuses readability. Add a new BootMapTable struct to assist with readability in bootmap entry code. Also: - replace ScsiMbr in ECKD code with appropriate structs - fix read_block messages to reflect BootMapTable - fixup ipl_scsi to use BootMapTable (referred to as Program Table) - defined value for maximum table entries Signed-off-by: Collin L. Walling--- pc-bios/s390-ccw/bootmap.c | 60 +- pc-bios/s390-ccw/bootmap.h | 11 - 2 files changed, 37 insertions(+), 34 deletions(-) diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 67a6123..286de40 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -182,24 +182,24 @@ static block_number_t load_eckd_segments(block_number_t blk, uint64_t *address) return block_nr; } -static void run_eckd_boot_script(block_number_t mbr_block_nr) +static void run_eckd_boot_script(block_number_t bmt_block_nr) { int i; unsigned int loadparm = get_loadparm_index(); block_number_t block_nr; uint64_t address; -ScsiMbr *bte = (void *)sec; /* Eckd bootmap table entry */ +BootMapTable *bmt = (void *)sec; BootMapScript *bms = (void *)sec; debug_print_int("loadparm", loadparm); -IPL_assert(loadparm < 31, "loadparm value greater than" +IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than" " maximum number of boot entries allowed"); memset(sec, FREE_SPACE_FILLER, sizeof(sec)); -read_block(mbr_block_nr, sec, "Cannot read MBR"); +read_block(bmt_block_nr, sec, "Cannot read Boot Map Table"); -block_nr = eckd_block_num((void *)&(bte->blockptr[loadparm])); -IPL_assert(block_nr != -1, "No Boot Map"); +block_nr = eckd_block_num((void *)>entry[loadparm]); +IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry"); memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(block_nr, sec, "Cannot read Boot Map Script"); @@ -223,7 +223,7 @@ static void ipl_eckd_cdl(void) XEckdMbr *mbr; Ipl2 *ipl2 = (void *)sec; IplVolumeLabel *vlbl = (void *)sec; -block_number_t block_nr; +block_number_t bmt_block_nr; /* we have just read the block #0 and recognized it as "IPL1" */ sclp_print("CDL\n"); @@ -238,8 +238,8 @@ static void ipl_eckd_cdl(void) IPL_assert(mbr->dev_type == DEV_TYPE_ECKD, "Non-ECKD device type in zIPL section of IPL2 record."); -/* save pointer to Boot Script */ -block_nr = eckd_block_num((void *)&(mbr->blockptr)); +/* save pointer to Boot Map Table */ +bmt_block_nr = eckd_block_num((void *)>blockptr); memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(2, vlbl, "Cannot read Volume Label at block 2"); @@ -249,7 +249,7 @@ static void ipl_eckd_cdl(void) "Invalid magic of volser block"); print_volser(vlbl->f.volser); -run_eckd_boot_script(block_nr); +run_eckd_boot_script(bmt_block_nr); /* no return */ } @@ -280,7 +280,7 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode) static void ipl_eckd_ldl(ECKD_IPL_mode_t mode) { -block_number_t block_nr; +block_number_t bmt_block_nr; BootInfo *bip = (void *)(sec + 0x70); /* BootInfo is MBR for LDL */ if (mode != ECKD_LDL_UNLABELED) { @@ -299,8 +299,10 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode) } verify_boot_info(bip); -block_nr = eckd_block_num((void *)&(bip->bp.ipl.bm_ptr.eckd.bptr)); -run_eckd_boot_script(block_nr); +/* save pointer to Boot Map Table */ +bmt_block_nr = eckd_block_num((void *)>bp.ipl.bm_ptr.eckd.bptr); + +run_eckd_boot_script(bmt_block_nr); /* no return */ } @@ -325,7 +327,7 @@ static void print_eckd_msg(void) static void ipl_eckd(void) { -ScsiMbr *mbr = (void *)sec; +XEckdMbr *mbr = (void *)sec; LDL_VTOC *vlbl = (void *)sec; print_eckd_msg(); @@ -449,10 +451,8 @@ static void zipl_run(ScsiBlockPtr *pte) static void ipl_scsi(void) { ScsiMbr *mbr = (void *)sec; -uint8_t *ns, *ns_end; int program_table_entries = 0; -const int pte_len = sizeof(ScsiBlockPtr); -ScsiBlockPtr *prog_table_entry = NULL; +BootMapTable *prog_table = (void *)sec; unsigned int loadparm = get_loadparm_index(); /* Grab the MBR */ @@ -467,34 +467,28 @@ static void ipl_scsi(void) debug_print_int("MBR Version", mbr->version_id); IPL_check(mbr->version_id == 1, "Unknown MBR layout version, assuming version 1"); -debug_print_int("program table", mbr->blockptr[0].blockno); -IPL_assert(mbr->blockptr[0].blockno, "No Program Table"); +debug_print_int("program table", mbr->pt.blockno); +IPL_assert(mbr->pt.blockno, "No Program Table"); /* Parse the program table */ -read_block(mbr->blockptr[0].blockno, sec, -
[Qemu-devel] [PATCH v7 06/12] s390-ccw: parse and set boot menu options
Set boot menu options for an s390 guest and store them in the iplb. These options are set via the QEMU command line option: -boot menu=on|off[,splash-time=X] or via the libvirt domain xml: Where X represents some positive integer representing milliseconds. Any value set for loadparm will override all boot menu options. If loadparm=PROMPT, then the menu will be enabled without a timeout. The absence of any boot options on the command line will flag to later use the zipl boot loader values. Signed-off-by: Collin L. WallingReviewed-by: Janosch Frank Reviewed-by: Thomas Huth --- hw/s390x/ipl.c | 48 hw/s390x/ipl.h | 9 +++-- pc-bios/s390-ccw/iplb.h | 6 -- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c index 31565ce..c8109f5 100644 --- a/hw/s390x/ipl.c +++ b/hw/s390x/ipl.c @@ -23,6 +23,9 @@ #include "hw/s390x/ebcdic.h" #include "ipl.h" #include "qemu/error-report.h" +#include "qemu/config-file.h" +#include "qemu/cutils.h" +#include "qemu/option.h" #define KERN_IMAGE_START0x01UL #define KERN_PARM_AREA 0x010480UL @@ -219,6 +222,50 @@ static Property s390_ipl_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static void s390_ipl_set_boot_menu(IplParameterBlock *iplb) +{ +QemuOptsList *plist = qemu_find_opts("boot-opts"); +QemuOpts *opts = QTAILQ_FIRST(>head); +uint8_t *flags; +uint32_t *timeout; +const char *tmp; +unsigned long splash_time = 0; + +switch (iplb->pbt) { +case S390_IPL_TYPE_CCW: +case S390_IPL_TYPE_QEMU_SCSI: +flags = >qipl.boot_menu_flags; +timeout = >qipl.boot_menu_timeout; +break; +default: +error_report("boot menu is not supported for this device type."); +return; +} + +/* In the absence of -boot menu, use zipl parameters */ +if (!qemu_opt_get(opts, "menu")) { +*flags = BOOT_MENU_FLAG_ZIPL_OPTS; +} else if (boot_menu) { +*flags = BOOT_MENU_FLAG_CMD_OPTS; + +tmp = qemu_opt_get(opts, "splash-time"); + +if (tmp && qemu_strtoul(tmp, NULL, 10, _time)) { +error_report("splash-time is invalid, forcing it to 0."); +splash_time = 0; +return; +} + +if (splash_time > 0x) { +error_report("splash-time is too large, forcing it to max value."); +splash_time = 0x; +return; +} + +*timeout = cpu_to_be32(splash_time); +} +} + static bool s390_gen_initial_iplb(S390IPLState *ipl) { DeviceState *dev_st; @@ -435,6 +482,7 @@ void s390_ipl_prepare_cpu(S390CPU *cpu) } ipl->iplb.qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr); } +s390_ipl_set_boot_menu(>iplb); s390_ipl_prepare_qipl(cpu); } diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index 74469b1..f632c59 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -60,6 +60,9 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; #define QIPL_ADDRESS 0xcc +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 + /* * The QEMU IPL Parameters will be stored 32-bit word aligned. * Placement of data fields in this area must account for @@ -67,9 +70,11 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; * The entire structure must not be larger than 28 bytes. */ struct QemuIplParameters { -uint8_t reserved1[4]; +uint8_t boot_menu_flags; +uint8_t reserved1[3]; +uint32_t boot_menu_timeout; uint64_t netboot_start_addr; -uint8_t reserved2[16]; +uint8_t reserved2[12]; } QEMU_PACKED; typedef struct QemuIplParameters QemuIplParameters; diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h index a23237e..0e39aa0 100644 --- a/pc-bios/s390-ccw/iplb.h +++ b/pc-bios/s390-ccw/iplb.h @@ -81,9 +81,11 @@ extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE))); * The entire structure must not be larger than 28 bytes. */ struct QemuIplParameters { -uint8_t reserved1[4]; +uint8_t boot_menu_flags; +uint8_t reserved1[3]; +uint32_t boot_menu_timeout; uint64_t netboot_start_addr; -uint8_t reserved2[16]; +uint8_t reserved2[12]; } __attribute__ ((packed)); typedef struct QemuIplParameters QemuIplParameters; -- 2.7.4
[Qemu-devel] [PATCH v7 03/12] s390-ccw: refactor IPL structs
ECKD DASDs have different IPL structures for CDL and LDL formats. The current Ipl1 and Ipl2 structs follow the CDL format, so we prepend "EckdCdl" to them. Boot info for LDL has been moved to a new struct: EckdLdlIpl1. Signed-off-by: Collin L. WallingAcked-by: Janosch Frank Reviewed-by: Thomas Huth --- pc-bios/s390-ccw/bootmap.c | 12 ++-- pc-bios/s390-ccw/bootmap.h | 37 + 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 9534f56..a94638d 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -221,7 +221,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr) static void ipl_eckd_cdl(void) { XEckdMbr *mbr; -Ipl2 *ipl2 = (void *)sec; +EckdCdlIpl2 *ipl2 = (void *)sec; IplVolumeLabel *vlbl = (void *)sec; block_number_t bmt_block_nr; @@ -231,7 +231,7 @@ static void ipl_eckd_cdl(void) memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(1, ipl2, "Cannot read IPL2 record at block 1"); -mbr = >u.x.mbr; +mbr = >mbr; IPL_assert(magic_match(mbr, ZIPL_MAGIC), "No zIPL section in IPL2 record."); IPL_assert(block_size_ok(mbr->blockptr.xeckd.bptr.size), "Bad block size in zIPL section of IPL2 record."); @@ -281,7 +281,7 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode) static void ipl_eckd_ldl(ECKD_IPL_mode_t mode) { block_number_t bmt_block_nr; -BootInfo *bip = (void *)(sec + 0x70); /* BootInfo is MBR for LDL */ +EckdLdlIpl1 *ipl1 = (void *)sec; if (mode != ECKD_LDL_UNLABELED) { print_eckd_ldl_msg(mode); @@ -292,15 +292,15 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode) memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(0, sec, "Cannot read block 0 to grab boot info."); if (mode == ECKD_LDL_UNLABELED) { -if (!magic_match(bip->magic, ZIPL_MAGIC)) { +if (!magic_match(ipl1->bip.magic, ZIPL_MAGIC)) { return; /* not applicable layout */ } sclp_print("unlabeled LDL.\n"); } -verify_boot_info(bip); +verify_boot_info(>bip); /* save pointer to Boot Map Table */ -bmt_block_nr = eckd_block_num(>bp.ipl.bm_ptr.eckd.bptr.chs); +bmt_block_nr = eckd_block_num(>bip.bp.ipl.bm_ptr.eckd.bptr.chs); run_eckd_boot_script(bmt_block_nr); /* no return */ diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h index b361084..4bd95cd 100644 --- a/pc-bios/s390-ccw/bootmap.h +++ b/pc-bios/s390-ccw/bootmap.h @@ -239,22 +239,27 @@ typedef struct BootInfo { /* @ 0x70, record #0 */ } bp; } __attribute__ ((packed)) BootInfo; /* see also XEckdMbr */ -typedef struct Ipl1 { -unsigned char key[4]; /* == "IPL1" */ -unsigned char data[24]; -} __attribute__((packed)) Ipl1; - -typedef struct Ipl2 { -unsigned char key[4]; /* == "IPL2" */ -union { -unsigned char data[144]; -struct { -unsigned char reserved1[92-4]; -XEckdMbr mbr; -unsigned char reserved2[144-(92-4)-sizeof(XEckdMbr)]; -} x; -} u; -} __attribute__((packed)) Ipl2; +/* + * Structs for IPL + */ +#define STAGE2_BLK_CNT_MAX 24 /* Stage 1b can load up to 24 blocks */ + +typedef struct EckdCdlIpl1 { +uint8_t key[4]; /* == "IPL1" */ +uint8_t data[24]; +} __attribute__((packed)) EckdCdlIpl1; + +typedef struct EckdCdlIpl2 { +uint8_t key[4]; /* == "IPL2" */ +uint8_t reserved0[88]; +XEckdMbr mbr; +uint8_t reserved[24]; +} __attribute__((packed)) EckdCdlIpl2; + +typedef struct EckdLdlIpl1 { +uint8_t reserved[112]; +BootInfo bip; /* BootInfo is MBR for LDL */ +} __attribute__((packed)) EckdLdlIpl1; typedef struct IplVolumeLabel { unsigned char key[4]; /* == "VOL1" */ -- 2.7.4
[Qemu-devel] [PATCH v7 02/12] s390-ccw: refactor eckd_block_num to use CHS
Add new cylinder/head/sector struct. Use it to calculate eckd block numbers instead of a BootMapPointer (which used eckd chs anyway). Signed-off-by: Collin L. WallingReviewed-by: Thomas Huth --- pc-bios/s390-ccw/bootmap.c | 28 ++-- pc-bios/s390-ccw/bootmap.h | 8 ++-- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c index 286de40..9534f56 100644 --- a/pc-bios/s390-ccw/bootmap.c +++ b/pc-bios/s390-ccw/bootmap.c @@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip) "Bad block size in zIPL section of the 1st record."); } -static block_number_t eckd_block_num(BootMapPointer *p) +static block_number_t eckd_block_num(EckdCHS *chs) { const uint64_t sectors = virtio_get_sectors(); const uint64_t heads = virtio_get_heads(); -const uint64_t cylinder = p->eckd.cylinder -+ ((p->eckd.head & 0xfff0) << 12); -const uint64_t head = p->eckd.head & 0x000f; +const uint64_t cylinder = chs->cylinder ++ ((chs->head & 0xfff0) << 12); +const uint64_t head = chs->head & 0x000f; const block_number_t block = sectors * heads * cylinder + sectors * head - + p->eckd.sector + + chs->sector - 1; /* block nr starts with zero */ return block; } static bool eckd_valid_address(BootMapPointer *p) { -const uint64_t head = p->eckd.head & 0x000f; +const uint64_t head = p->eckd.chs.head & 0x000f; if (head >= virtio_get_heads() -|| p->eckd.sector > virtio_get_sectors() -|| p->eckd.sector <= 0) { +|| p->eckd.chs.sector > virtio_get_sectors() +|| p->eckd.chs.sector <= 0) { return false; } if (!virtio_guessed_disk_nature() && -eckd_block_num(p) >= virtio_get_blocks()) { +eckd_block_num(>eckd.chs) >= virtio_get_blocks()) { return false; } @@ -140,7 +140,7 @@ static block_number_t load_eckd_segments(block_number_t blk, uint64_t *address) do { more_data = false; for (j = 0;; j++) { -block_nr = eckd_block_num((void *)&(bprs[j].xeckd)); +block_nr = eckd_block_num([j].xeckd.bptr.chs); if (is_null_block_number(block_nr)) { /* end of chunk */ break; } @@ -198,7 +198,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr) memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(bmt_block_nr, sec, "Cannot read Boot Map Table"); -block_nr = eckd_block_num((void *)>entry[loadparm]); +block_nr = eckd_block_num(>entry[loadparm].xeckd.bptr.chs); IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry"); memset(sec, FREE_SPACE_FILLER, sizeof(sec)); @@ -206,7 +206,7 @@ static void run_eckd_boot_script(block_number_t bmt_block_nr) for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD; i++) { address = bms->entry[i].address.load_address; -block_nr = eckd_block_num(&(bms->entry[i].blkptr)); +block_nr = eckd_block_num(>entry[i].blkptr.xeckd.bptr.chs); do { block_nr = load_eckd_segments(block_nr, ); @@ -239,7 +239,7 @@ static void ipl_eckd_cdl(void) "Non-ECKD device type in zIPL section of IPL2 record."); /* save pointer to Boot Map Table */ -bmt_block_nr = eckd_block_num((void *)>blockptr); +bmt_block_nr = eckd_block_num(>blockptr.xeckd.bptr.chs); memset(sec, FREE_SPACE_FILLER, sizeof(sec)); read_block(2, vlbl, "Cannot read Volume Label at block 2"); @@ -300,7 +300,7 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode) verify_boot_info(bip); /* save pointer to Boot Map Table */ -bmt_block_nr = eckd_block_num((void *)>bp.ipl.bm_ptr.eckd.bptr); +bmt_block_nr = eckd_block_num(>bp.ipl.bm_ptr.eckd.bptr.chs); run_eckd_boot_script(bmt_block_nr); /* no return */ diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h index 486c0f3..b361084 100644 --- a/pc-bios/s390-ccw/bootmap.h +++ b/pc-bios/s390-ccw/bootmap.h @@ -32,10 +32,14 @@ typedef struct FbaBlockPtr { uint16_t blockct; } __attribute__ ((packed)) FbaBlockPtr; -typedef struct EckdBlockPtr { -uint16_t cylinder; /* cylinder/head/sector is an address of the block */ +typedef struct EckdCHS { +uint16_t cylinder; uint16_t head; uint8_t sector; +} __attribute__ ((packed)) EckdCHS; + +typedef struct EckdBlockPtr { +EckdCHS chs; /* cylinder/head/sector is an address of the block */ uint16_t size; uint8_t count; /* (size_in_blocks-1); * it's 0 for TablePtr, ScriptPtr, and SectionPtr */ -- 2.7.4
[Qemu-devel] [PATCH v7 00/12] Interactive Boot Menu for DASD and SCSI Guests on s390x
Sorry for posting this version so close to the previous posting. The requested changes are getting smaller and smaller and I'm hoping to get these patches satisfactory soon. --- [v7] --- - fixed alignment mistake in QemuIplParameters - also added comment explaining this - fixed rebasing error with _strlen and _memcmp - dropped reserved2 field in ScsiMbr - r-b's and signoffs aded where suggested --- [v6] --- - cleaned up libc.c - expanded timeout field in QemuIPLB from 2 bytes to 4 bytes - we can now store the timeout value from command line as ms - sclp_set_write_mask now accepts two parameters: - receive_mask - send_mask - the write mask for receive is only set when we need it (reading user inp) and cleared when we no longer need it (no longer reading user inp) - pending irq code cleaned up - bootmap code for scsi fixed up -- it has the same limitations as DASD, so I added some similar checks in ipl_scsi --- [Summary] --- These patches implement a boot menu for ECKD DASD and SCSI guests on s390x. The menu will only appear if the disk has been configured for IPL with the zIPL tool and with the following QEMU command line options: -boot menu=on[,splash-time=X] and/or -machine loadparm='prompt' The following must be specified for the device to be IPL'd from: -device ...,bootindex=1 or via the following libvirt domain xml: or ... Where X is some positive integer representing time in milliseconds. must be specified for the device to be IPL'd from A loadparm other than 'prompt' will disable the menu and just boot the specified entry. If no boot options are specified, we will attempt to use the values provided by zipl (ECKD DASD only). Collin L. Walling (12): s390-ccw: refactor boot map table code s390-ccw: refactor eckd_block_num to use CHS s390-ccw: refactor IPL structs s390-ccw: update libc s390-ccw: move auxiliary IPL data to separate location s390-ccw: parse and set boot menu options s390-ccw: set up interactive boot menu parameters s390-ccw: read stage2 boot loader data to find menu s390-ccw: print zipl boot menu s390-ccw: read user input for boot index via the SCLP console s390-ccw: set cp_receive mask only when needed and consume pending service irqs s390-ccw: interactive boot menu for scsi hw/s390x/ipl.c | 67 +++- hw/s390x/ipl.h | 24 - pc-bios/s390-ccw/Makefile | 2 +- pc-bios/s390-ccw/bootmap.c | 184 +++-- pc-bios/s390-ccw/bootmap.h | 91 ++--- pc-bios/s390-ccw/iplb.h | 22 +++- pc-bios/s390-ccw/libc.c | 89 pc-bios/s390-ccw/libc.h | 37 ++- pc-bios/s390-ccw/main.c | 48 + pc-bios/s390-ccw/menu.c | 241 pc-bios/s390-ccw/menu.h | 25 + pc-bios/s390-ccw/s390-ccw.h | 3 + pc-bios/s390-ccw/sclp.c | 39 --- pc-bios/s390-ccw/virtio.c | 2 +- 14 files changed, 748 insertions(+), 126 deletions(-) create mode 100644 pc-bios/s390-ccw/libc.c create mode 100644 pc-bios/s390-ccw/menu.c create mode 100644 pc-bios/s390-ccw/menu.h -- 2.7.4
Re: [Qemu-devel] [PATCH v21 1/5] xbitmap: Introduce xbitmap
On Fri, Feb 16, 2018 at 11:45:51PM +0200, Andy Shevchenko wrote: > Now, the question about test case. Why do you heavily use BUG_ON? > Isn't resulting statistics enough? No. If any of those tests fail, we want to stop dead. They'll lead to horrendous bugs throughout the kernel if they're wrong. I think more of the in-kernel test suite should stop dead instead of printing a warning. Would you want to boot a machine which has a known bug in the page cache, for example?
[Qemu-devel] [PATCH v3 4/5] aarch64-linux-user: Add support for EXTRA signal frame records
The EXTRA record allows for additional space to be allocated beyon what is currently reserved. Add code to emit and read this record type. Nothing uses extra space yet. Signed-off-by: Richard Henderson--- linux-user/signal.c | 55 + 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index f9eef3d753..ca0ba28c98 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -1443,6 +1443,15 @@ struct target_fpsimd_context { uint64_t vregs[32 * 2]; /* really uint128_t vregs[32] */ }; +#define TARGET_EXTRA_MAGIC 0x45585401 + +struct target_extra_context { +struct target_aarch64_ctx head; +uint64_t datap; /* 16-byte aligned pointer to extra space cast to __u64 */ +uint32_t size; /* size in bytes of the extra space */ +uint32_t reserved[3]; +}; + struct target_rt_sigframe { struct target_siginfo info; struct target_ucontext uc; @@ -1502,6 +1511,15 @@ static void target_setup_fpsimd_record(struct target_fpsimd_context *fpsimd, } } +static void target_setup_extra_record(struct target_extra_context *extra, + uint64_t datap, uint32_t extra_size) +{ +__put_user(TARGET_EXTRA_MAGIC, >head.magic); +__put_user(sizeof(struct target_extra_context), >head.size); +__put_user(datap, >datap); +__put_user(extra_size, >size); +} + static void target_setup_end_record(struct target_aarch64_ctx *end) { __put_user(0, >magic); @@ -1554,14 +1572,16 @@ static void target_restore_fpsimd_record(CPUARMState *env, static int target_restore_sigframe(CPUARMState *env, struct target_rt_sigframe *sf) { -struct target_aarch64_ctx *ctx; +struct target_aarch64_ctx *ctx, *extra = NULL; struct target_fpsimd_context *fpsimd = NULL; +uint64_t extra_datap = 0; +bool used_extra = false; target_restore_general_frame(env, sf); ctx = (struct target_aarch64_ctx *)sf->uc.tuc_mcontext.__reserved; while (ctx) { -uint32_t magic, size; +uint32_t magic, size, extra_size; __get_user(magic, >magic); __get_user(size, >size); @@ -1570,7 +1590,12 @@ static int target_restore_sigframe(CPUARMState *env, if (size != 0) { return 1; } -ctx = NULL; +if (used_extra) { +ctx = NULL; +} else { +ctx = extra; +used_extra = true; +} continue; case TARGET_FPSIMD_MAGIC: @@ -1580,6 +1605,17 @@ static int target_restore_sigframe(CPUARMState *env, fpsimd = (struct target_fpsimd_context *)ctx; break; +case TARGET_EXTRA_MAGIC: +if (extra || size != sizeof(struct target_extra_context)) { +return 1; +} +__get_user(extra_datap, + &((struct target_extra_context *)ctx)->datap); +__get_user(extra_size, + &((struct target_extra_context *)ctx)->size); +extra = lock_user(VERIFY_READ, extra_datap, extra_size, 0); +break; + default: /* Unknown record -- we certainly didn't generate it. * Did we in fact get out of sync? @@ -1595,6 +1631,9 @@ static int target_restore_sigframe(CPUARMState *env, } target_restore_fpsimd_record(env, fpsimd); +if (extra) { +unlock_user(extra, extra_datap, 0); +} return 0; } @@ -1621,7 +1660,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka, CPUARMState *env) { int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved); -int fpsimd_ofs, end1_ofs, fr_ofs; +int fpsimd_ofs, end1_ofs, fr_ofs, end2_ofs = 0; +int extra_ofs = 0, extra_base = 0, extra_size = 0; struct target_rt_sigframe *frame; struct target_rt_frame_record *fr; abi_ulong frame_addr, return_addr; @@ -1641,7 +1681,14 @@ static void target_setup_frame(int usig, struct target_sigaction *ka, target_setup_general_frame(frame, env, set); target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env); +if (extra_ofs) { +target_setup_extra_record((void *)frame + extra_ofs, + frame_addr + extra_base, extra_size); +} target_setup_end_record((void *)frame + end1_ofs); +if (end2_ofs) { +target_setup_end_record((void *)frame + end2_ofs); +} /* Set up the stack frame for unwinding. */ fr = (void *)frame + fr_ofs; -- 2.14.3
[Qemu-devel] [PATCH v3 2/5] aarch64-linux-user: Split out helpers for guest signal handling
Split out helpers from target_setup_frame and target_restore_sigframe for dealing with general registers, fpsimd registers, and the end record. When we add support for sve registers, the relative positions of these will change. Signed-off-by: Richard Henderson--- linux-user/signal.c | 120 ++-- 1 file changed, 69 insertions(+), 51 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index 9a380b9e31..25c9743aed 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -1462,16 +1462,17 @@ struct target_rt_sigframe { uint32_t tramp[2]; }; -static int target_setup_sigframe(struct target_rt_sigframe *sf, - CPUARMState *env, target_sigset_t *set) +static void target_setup_general_frame(struct target_rt_sigframe *sf, + CPUARMState *env, target_sigset_t *set) { int i; -struct target_aux_context *aux = -(struct target_aux_context *)sf->uc.tuc_mcontext.__reserved; -/* set up the stack frame for unwinding */ -__put_user(env->xregs[29], >fp); -__put_user(env->xregs[30], >lr); +__put_user(0, >uc.tuc_flags); +__put_user(0, >uc.tuc_link); + +__put_user(target_sigaltstack_used.ss_sp, >uc.tuc_stack.ss_sp); +__put_user(sas_ss_flags(env->xregs[31]), >uc.tuc_stack.ss_flags); +__put_user(target_sigaltstack_used.ss_size, >uc.tuc_stack.ss_size); for (i = 0; i < 31; i++) { __put_user(env->xregs[i], >uc.tuc_mcontext.regs[i]); @@ -1485,39 +1486,42 @@ static int target_setup_sigframe(struct target_rt_sigframe *sf, for (i = 0; i < TARGET_NSIG_WORDS; i++) { __put_user(set->sig[i], >uc.tuc_sigmask.sig[i]); } +} + +static void target_setup_fpsimd_record(struct target_fpsimd_context *fpsimd, + CPUARMState *env) +{ +int i; + +__put_user(TARGET_FPSIMD_MAGIC, >head.magic); +__put_user(sizeof(struct target_fpsimd_context), >head.size); +__put_user(vfp_get_fpsr(env), >fpsr); +__put_user(vfp_get_fpcr(env), >fpcr); for (i = 0; i < 32; i++) { uint64_t *q = aa64_vfp_qreg(env, i); #ifdef TARGET_WORDS_BIGENDIAN -__put_user(q[0], >fpsimd.vregs[i * 2 + 1]); -__put_user(q[1], >fpsimd.vregs[i * 2]); +__put_user(q[0], >vregs[i * 2 + 1]); +__put_user(q[1], >vregs[i * 2]); #else -__put_user(q[0], >fpsimd.vregs[i * 2]); -__put_user(q[1], >fpsimd.vregs[i * 2 + 1]); +__put_user(q[0], >vregs[i * 2]); +__put_user(q[1], >vregs[i * 2 + 1]); #endif } -__put_user(vfp_get_fpsr(env), >fpsimd.fpsr); -__put_user(vfp_get_fpcr(env), >fpsimd.fpcr); -__put_user(TARGET_FPSIMD_MAGIC, >fpsimd.head.magic); -__put_user(sizeof(struct target_fpsimd_context), ->fpsimd.head.size); - -/* set the "end" magic */ -__put_user(0, >end.magic); -__put_user(0, >end.size); - -return 0; } -static int target_restore_sigframe(CPUARMState *env, - struct target_rt_sigframe *sf) +static void target_setup_end_record(struct target_aarch64_ctx *end) +{ +__put_user(0, >magic); +__put_user(0, >size); +} + +static void target_restore_general_frame(CPUARMState *env, + struct target_rt_sigframe *sf) { sigset_t set; -int i; -struct target_aux_context *aux = -(struct target_aux_context *)sf->uc.tuc_mcontext.__reserved; -uint32_t magic, size, fpsr, fpcr; uint64_t pstate; +int i; target_to_host_sigset(, >uc.tuc_sigmask); set_sigmask(); @@ -1530,30 +1534,48 @@ static int target_restore_sigframe(CPUARMState *env, __get_user(env->pc, >uc.tuc_mcontext.pc); __get_user(pstate, >uc.tuc_mcontext.pstate); pstate_write(env, pstate); +} -__get_user(magic, >fpsimd.head.magic); -__get_user(size, >fpsimd.head.size); +static void target_restore_fpsimd_record(CPUARMState *env, + struct target_fpsimd_context *fpsimd) +{ +uint32_t fpsr, fpcr; +int i; -if (magic != TARGET_FPSIMD_MAGIC -|| size != sizeof(struct target_fpsimd_context)) { -return 1; -} +__get_user(fpsr, >fpsr); +vfp_set_fpsr(env, fpsr); +__get_user(fpcr, >fpcr); +vfp_set_fpcr(env, fpcr); for (i = 0; i < 32; i++) { uint64_t *q = aa64_vfp_qreg(env, i); #ifdef TARGET_WORDS_BIGENDIAN -__get_user(q[0], >fpsimd.vregs[i * 2 + 1]); -__get_user(q[1], >fpsimd.vregs[i * 2]); +__get_user(q[0], >vregs[i * 2 + 1]); +__get_user(q[1], >vregs[i * 2]); #else -__get_user(q[0], >fpsimd.vregs[i * 2]); -__get_user(q[1], >fpsimd.vregs[i * 2 + 1]); +__get_user(q[0], >vregs[i * 2]); +__get_user(q[1], >vregs[i * 2 + 1]); #endif } -__get_user(fpsr, >fpsimd.fpsr); -vfp_set_fpsr(env, fpsr); -
[Qemu-devel] [PATCH v3 5/5] aarch64-linux-user: Add support for SVE signal frame records
Depending on the currently selected size of the SVE vector registers, we can either store the data within the "standard" allocation, or we may beedn to allocate additional space with an EXTRA record. Signed-off-by: Richard Henderson--- linux-user/signal.c | 141 ++-- 1 file changed, 138 insertions(+), 3 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index ca0ba28c98..4c9fef4bb2 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -1452,6 +1452,30 @@ struct target_extra_context { uint32_t reserved[3]; }; +#define TARGET_SVE_MAGIC0x53564501 + +struct target_sve_context { +struct target_aarch64_ctx head; +uint16_t vl; +uint16_t reserved[3]; +}; + +#define TARGET_SVE_VQ_BYTES 16 + +#define TARGET_SVE_SIG_ZREG_SIZE(VQ) ((VQ) * TARGET_SVE_VQ_BYTES) +#define TARGET_SVE_SIG_PREG_SIZE(VQ) ((VQ) * (TARGET_SVE_VQ_BYTES / 8)) + +#define TARGET_SVE_SIG_REGS_OFFSET \ +QEMU_ALIGN_UP(sizeof(struct target_sve_context), TARGET_SVE_VQ_BYTES) +#define TARGET_SVE_SIG_ZREG_OFFSET(VQ, N) \ +(TARGET_SVE_SIG_REGS_OFFSET + TARGET_SVE_SIG_ZREG_SIZE(VQ) * (N)) +#define TARGET_SVE_SIG_PREG_OFFSET(VQ, N) \ +(TARGET_SVE_SIG_ZREG_OFFSET(VQ, 32) + TARGET_SVE_SIG_PREG_SIZE(VQ) * (N)) +#define TARGET_SVE_SIG_FFR_OFFSET(VQ) \ +(TARGET_SVE_SIG_PREG_OFFSET(VQ, 16)) +#define TARGET_SVE_SIG_CONTEXT_SIZE(VQ) \ +(TARGET_SVE_SIG_PREG_OFFSET(VQ, 17)) + struct target_rt_sigframe { struct target_siginfo info; struct target_ucontext uc; @@ -1526,6 +1550,34 @@ static void target_setup_end_record(struct target_aarch64_ctx *end) __put_user(0, >size); } +static void target_setup_sve_record(struct target_sve_context *sve, +CPUARMState *env, int vq, int size) +{ +int i, j; + +__put_user(TARGET_SVE_MAGIC, >head.magic); +__put_user(size, >head.size); +__put_user(vq * TARGET_SVE_VQ_BYTES, >vl); + +/* Note that SVE regs are stored as a byte stream, with each byte element + * at a subsequent address. This corresponds to a little-endian store + * of our 64-bit hunks. + */ +for (i = 0; i < 32; ++i) { +uint64_t *z = (void *)sve + TARGET_SVE_SIG_ZREG_OFFSET(vq, i); +for (j = 0; j < vq * 2; ++j) { +__put_user_e(env->vfp.zregs[i].d[j], z + j, le); +} +} +for (i = 0; i <= 16; ++i) { +uint16_t *p = (void *)sve + TARGET_SVE_SIG_PREG_OFFSET(vq, i); +for (j = 0; j < vq; ++j) { +uint64_t r = env->vfp.pregs[i].p[j >> 2]; +__put_user_e(r >> ((j & 3) * 16), p + j, le); +} +} +} + static void target_restore_general_frame(CPUARMState *env, struct target_rt_sigframe *sf) { @@ -1569,13 +1621,44 @@ static void target_restore_fpsimd_record(CPUARMState *env, } } +static void target_restore_sve_record(CPUARMState *env, + struct target_sve_context *sve, int vq) +{ +int i, j; + +/* Note that SVE regs are stored as a byte stream, with each byte element + * at a subsequent address. This corresponds to a little-endian store + * of our 64-bit hunks. + */ +for (i = 0; i < 32; ++i) { +uint64_t *z = (void *)sve + TARGET_SVE_SIG_ZREG_OFFSET(vq, i); +for (j = 0; j < vq * 2; ++j) { +__get_user_e(env->vfp.zregs[i].d[j], z + j, le); +} +} +for (i = 0; i <= 16; ++i) { +uint16_t *p = (void *)sve + TARGET_SVE_SIG_PREG_OFFSET(vq, i); +for (j = 0; j < vq; ++j) { +uint16_t r; +__get_user_e(r, p + j, le); +if (j & 3) { +env->vfp.pregs[i].p[j >> 2] |= (uint64_t)r << ((j & 3) * 16); +} else { +env->vfp.pregs[i].p[j >> 2] = r; +} +} +} +} + static int target_restore_sigframe(CPUARMState *env, struct target_rt_sigframe *sf) { struct target_aarch64_ctx *ctx, *extra = NULL; struct target_fpsimd_context *fpsimd = NULL; +struct target_sve_context *sve = NULL; uint64_t extra_datap = 0; bool used_extra = false; +int vq = 0, sve_size = 0; target_restore_general_frame(env, sf); @@ -1605,6 +1688,17 @@ static int target_restore_sigframe(CPUARMState *env, fpsimd = (struct target_fpsimd_context *)ctx; break; +case TARGET_SVE_MAGIC: +if (arm_feature(env, ARM_FEATURE_SVE)) { +vq = (env->vfp.zcr_el[1] & 0xf) + 1; +sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16); +if (!sve && size == sve_size) { +sve = (struct target_sve_context *)ctx; +break; +} +} +return 1; + case TARGET_EXTRA_MAGIC: if (extra || size != sizeof(struct
[Qemu-devel] [PATCH v3 0/5] target/arm: linux-user changes for sve
Changes since v2: * 5 patches merged, * The PR_SVE_SET/GET_VL patch is more specifically user-only. * Split the signal frame patch into 4 parts. r~ Richard Henderson (5): linux-user: Implement aarch64 PR_SVE_SET/GET_VL aarch64-linux-user: Split out helpers for guest signal handling aarch64-linux-user: Remove struct target_aux_context aarch64-linux-user: Add support for EXTRA signal frame records aarch64-linux-user: Add support for SVE signal frame records linux-user/aarch64/target_syscall.h | 3 + target/arm/cpu.h| 1 + linux-user/signal.c | 367 +--- linux-user/syscall.c| 27 +++ target/arm/cpu64.c | 41 5 files changed, 372 insertions(+), 67 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH v3 3/5] aarch64-linux-user: Remove struct target_aux_context
This changes the qemu signal frame layout to be more like the kernel's, in that the various records are dynamically allocated rather than fixed in place by a structure. For now, all of the allocation is out of uc.tuc_mcontext.__reserved, so the allocation is actually trivial. That will change with SVE support. Signed-off-by: Richard Henderson--- linux-user/signal.c | 89 - 1 file changed, 61 insertions(+), 28 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index 25c9743aed..f9eef3d753 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -1443,20 +1443,12 @@ struct target_fpsimd_context { uint64_t vregs[32 * 2]; /* really uint128_t vregs[32] */ }; -/* - * Auxiliary context saved in the sigcontext.__reserved array. Not exported to - * user space as it will change with the addition of new context. User space - * should check the magic/size information. - */ -struct target_aux_context { -struct target_fpsimd_context fpsimd; -/* additional context to be added before "end" */ -struct target_aarch64_ctx end; -}; - struct target_rt_sigframe { struct target_siginfo info; struct target_ucontext uc; +}; + +struct target_rt_frame_record { uint64_t fp; uint64_t lr; uint32_t tramp[2]; @@ -1562,20 +1554,47 @@ static void target_restore_fpsimd_record(CPUARMState *env, static int target_restore_sigframe(CPUARMState *env, struct target_rt_sigframe *sf) { -struct target_aux_context *aux -= (struct target_aux_context *)sf->uc.tuc_mcontext.__reserved; -uint32_t magic, size; +struct target_aarch64_ctx *ctx; +struct target_fpsimd_context *fpsimd = NULL; target_restore_general_frame(env, sf); -__get_user(magic, >fpsimd.head.magic); -__get_user(size, >fpsimd.head.size); -if (magic == TARGET_FPSIMD_MAGIC -&& size == sizeof(struct target_fpsimd_context)) { -target_restore_fpsimd_record(env, >fpsimd); -} else { +ctx = (struct target_aarch64_ctx *)sf->uc.tuc_mcontext.__reserved; +while (ctx) { +uint32_t magic, size; + +__get_user(magic, >magic); +__get_user(size, >size); +switch (magic) { +case 0: +if (size != 0) { +return 1; +} +ctx = NULL; +continue; + +case TARGET_FPSIMD_MAGIC: +if (fpsimd || size != sizeof(struct target_fpsimd_context)) { +return 1; +} +fpsimd = (struct target_fpsimd_context *)ctx; +break; + +default: +/* Unknown record -- we certainly didn't generate it. + * Did we in fact get out of sync? + */ +return 1; +} +ctx = (void *)ctx + size; +} + +/* Require FPSIMD always. */ +if (!fpsimd) { return 1; } +target_restore_fpsimd_record(env, fpsimd); + return 0; } @@ -1601,20 +1620,33 @@ static void target_setup_frame(int usig, struct target_sigaction *ka, target_siginfo_t *info, target_sigset_t *set, CPUARMState *env) { +int size = offsetof(struct target_rt_sigframe, uc.tuc_mcontext.__reserved); +int fpsimd_ofs, end1_ofs, fr_ofs; struct target_rt_sigframe *frame; -struct target_aux_context *aux; +struct target_rt_frame_record *fr; abi_ulong frame_addr, return_addr; +fpsimd_ofs = size; +size += sizeof(struct target_fpsimd_context); +end1_ofs = size; +size += sizeof(struct target_aarch64_ctx); +fr_ofs = size; +size += sizeof(struct target_rt_frame_record); + frame_addr = get_sigframe(ka, env); trace_user_setup_frame(env, frame_addr); if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) { goto give_sigsegv; } -aux = (struct target_aux_context *)frame->uc.tuc_mcontext.__reserved; target_setup_general_frame(frame, env, set); -target_setup_fpsimd_record(>fpsimd, env); -target_setup_end_record(>end); +target_setup_fpsimd_record((void *)frame + fpsimd_ofs, env); +target_setup_end_record((void *)frame + end1_ofs); + +/* Set up the stack frame for unwinding. */ +fr = (void *)frame + fr_ofs; +__put_user(env->xregs[29], >fp); +__put_user(env->xregs[30], >lr); if (ka->sa_flags & TARGET_SA_RESTORER) { return_addr = ka->sa_restorer; @@ -1624,13 +1656,14 @@ static void target_setup_frame(int usig, struct target_sigaction *ka, * Since these are instructions they need to be put as little-endian * regardless of target default or current CPU endianness. */ -__put_user_e(0xd2801168, >tramp[0], le); -__put_user_e(0xd401, >tramp[1], le); -return_addr = frame_addr + offsetof(struct target_rt_sigframe, tramp); +
[Qemu-devel] [PATCH v3 1/5] linux-user: Implement aarch64 PR_SVE_SET/GET_VL
As an implementation choice, widening VL has zeroed the previously inaccessible portion of the sve registers. Signed-off-by: Richard Henderson--- linux-user/aarch64/target_syscall.h | 3 +++ target/arm/cpu.h| 1 + linux-user/syscall.c| 27 target/arm/cpu64.c | 41 + 4 files changed, 72 insertions(+) diff --git a/linux-user/aarch64/target_syscall.h b/linux-user/aarch64/target_syscall.h index 604ab99b14..205265e619 100644 --- a/linux-user/aarch64/target_syscall.h +++ b/linux-user/aarch64/target_syscall.h @@ -19,4 +19,7 @@ struct target_pt_regs { #define TARGET_MLOCKALL_MCL_CURRENT 1 #define TARGET_MLOCKALL_MCL_FUTURE 2 +#define TARGET_PR_SVE_SET_VL 50 +#define TARGET_PR_SVE_GET_VL 51 + #endif /* AARCH64_TARGET_SYSCALL_H */ diff --git a/target/arm/cpu.h b/target/arm/cpu.h index de62df091c..f5fbb9b450 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -846,6 +846,7 @@ int arm_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs, #ifdef TARGET_AARCH64 int aarch64_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg); int aarch64_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg); +void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq); #endif target_ulong do_arm_semihosting(CPUARMState *env); diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 82b35a6bdf..cf00ce10f1 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -10659,6 +10659,33 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, break; } #endif +#ifdef TARGET_AARCH64 +case TARGET_PR_SVE_SET_VL: +/* We cannot support either PR_SVE_SET_VL_ONEXEC + or PR_SVE_VL_INHERIT. Therefore, anything above + ARM_MAX_VQ results in EINVAL. */ +ret = -TARGET_EINVAL; +if (arm_feature(cpu_env, ARM_FEATURE_SVE) +&& arg2 >= 0 && arg2 <= ARM_MAX_VQ * 16 && !(arg2 & 15)) { +CPUARMState *env = cpu_env; +int old_vq = (env->vfp.zcr_el[1] & 0xf) + 1; +int vq = MAX(arg2 / 16, 1); + +if (vq < old_vq) { +aarch64_sve_narrow_vq(env, vq); +} +env->vfp.zcr_el[1] = vq - 1; +ret = vq * 16; +} +break; +case TARGET_PR_SVE_GET_VL: +ret = -TARGET_EINVAL; +if (arm_feature(cpu_env, ARM_FEATURE_SVE)) { +CPUARMState *env = cpu_env; +ret = ((env->vfp.zcr_el[1] & 0xf) + 1) * 16; +} +break; +#endif /* AARCH64 */ case PR_GET_SECCOMP: case PR_SET_SECCOMP: /* Disable seccomp to prevent the target disabling syscalls we diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 1c330adc28..a0a81014b2 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -363,3 +363,44 @@ static void aarch64_cpu_register_types(void) } type_init(aarch64_cpu_register_types) + +/* The manual says that when SVE is enabled and VQ is widened the + * implementation is allowed to zero the previously inaccessible + * portion of the registers. The corollary to that is that when + * SVE is enabled and VQ is narrowed we are also allowed to zero + * the now inaccessible portion of the registers. + * + * The intent of this is that no predicate bit beyond VQ is ever set. + * Which means that some operations on predicate registers themselves + * may operate on full uint64_t or even unrolled across the maximum + * uint64_t[4]. Performing 4 bits of host arithmetic unconditionally + * may well be cheaper than conditionals to restrict the operation + * to the relevant portion of a uint16_t[16]. + * + * TODO: Need to call this for changes to the real system registers + * and EL state changes. + */ +void aarch64_sve_narrow_vq(CPUARMState *env, unsigned vq) +{ +int i, j; +uint64_t pmask; + +assert(vq >= 1 && vq <= ARM_MAX_VQ); + +/* Zap the high bits of the zregs. */ +for (i = 0; i < 32; i++) { +memset(>vfp.zregs[i].d[2 * vq], 0, 16 * (ARM_MAX_VQ - vq)); +} + +/* Zap the high bits of the pregs and ffr. */ +pmask = 0; +if (vq & 3) { +pmask = ~(-1ULL << (16 * (vq & 3))); +} +for (j = vq / 4; j < ARM_MAX_VQ / 4; j++) { +for (i = 0; i < 17; ++i) { +env->vfp.pregs[i].p[j] &= pmask; +} +pmask = 0; +} +} -- 2.14.3
Re: [Qemu-devel] [PATCH] intel_iommu: allow updating FEADDR and FEUADDR with one 64bit write
On Wed, Jan 24, 2018 at 03:18:48PM +0100, Marek Marczykowski-Górecki wrote: > Allow updating those two adjacent 32bit fields with one 64bit write. > This fixes qemu crash when booting Xen inside. > > See discussion on Xen side of the thing here: > http://xen.markmail.org/message/6mrmemrnmhxvaxba Bump. > Signed-off-by: Marek Marczykowski-Górecki> --- > hw/i386/intel_iommu.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 2e841cde27..d214dce277 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -2129,8 +2129,12 @@ static void vtd_mem_write(void *opaque, hwaddr addr, > > /* Fault Event Address Register, 32-bit */ > case DMAR_FEADDR_REG: > -assert(size == 4); > -vtd_set_long(s, addr, val); > +assert(size == 4 || size == 8); > +if (size == 4) { > +vtd_set_long(s, addr, val); > +} else { > +vtd_set_quad(s, addr, val); > +} > break; > > /* Fault Event Upper Address Register, 32-bit */ -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v3 7/7] hw/sd: move sdcard legacy API to "hw/sd/sdcard_legacy.h"
On Thu, Feb 15, 2018 at 6:29 PM, Philippe Mathieu-Daudéwrote: > omap_mmc.c is the last user left. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > include/hw/sd/sd.h| 16 -- > include/hw/sd/sdcard_legacy.h | 50 > +++ > hw/sd/omap_mmc.c | 2 +- > hw/sd/sd.c| 1 + > 4 files changed, 52 insertions(+), 17 deletions(-) > create mode 100644 include/hw/sd/sdcard_legacy.h > > diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h > index 4491db98de..58528b4d2c 100644 > --- a/include/hw/sd/sd.h > +++ b/include/hw/sd/sd.h > @@ -131,22 +131,6 @@ typedef struct { > void (*set_readonly)(DeviceState *dev, bool readonly); > } SDBusClass; > > -/* Legacy functions to be used only by non-qdevified callers */ > -SDState *sd_init(BlockBackend *bs, bool is_spi); > -int sd_do_command(SDState *sd, SDRequest *req, > - uint8_t *response); > -void sd_write_data(SDState *sd, uint8_t value); > -uint8_t sd_read_data(SDState *sd); > -void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert); > -/* sd_enable should not be used -- it is only used on the nseries boards, > - * where it is part of a broken implementation of the MMC card slot switch > - * (there should be two card slots which are multiplexed to a single MMC > - * controller, but instead we model it with one card and controller and > - * disable the card when the second slot is selected, so it looks like the > - * second slot is always empty). > - */ > -void sd_enable(SDState *sd, bool enable); > - > /* Functions to be used by qdevified callers (working via > * an SDBus rather than directly with SDState) > */ > diff --git a/include/hw/sd/sdcard_legacy.h b/include/hw/sd/sdcard_legacy.h > new file mode 100644 > index 00..8681f8089b > --- /dev/null > +++ b/include/hw/sd/sdcard_legacy.h > @@ -0,0 +1,50 @@ > +/* > + * SD Memory Card emulation (deprecated legacy API) > + * > + * Copyright (c) 2006 Andrzej Zaborowski > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * > + * 1. Redistributions of source code must retain the above copyright > + *notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + *notice, this list of conditions and the following disclaimer in > + *the documentation and/or other materials provided with the > + *distribution. > + * > + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, > + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR > + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, > + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, > + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR > + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY > + * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > +#ifndef HW_SDCARD_LEGACY_H > +#define HW_SDCARD_LEGACY_H > + > +#include "hw/sd/sd.h" > + > +/* Legacy functions to be used only by non-qdevified callers */ > +SDState *sd_init(BlockBackend *blk, bool is_spi); > +int sd_do_command(SDState *card, SDRequest *request, uint8_t *response); > +void sd_write_data(SDState *card, uint8_t value); > +uint8_t sd_read_data(SDState *card); > +void sd_set_cb(SDState *card, qemu_irq readonly, qemu_irq insert); > + > +/* sd_enable should not be used -- it is only used on the nseries boards, > + * where it is part of a broken implementation of the MMC card slot switch > + * (there should be two card slots which are multiplexed to a single MMC > + * controller, but instead we model it with one card and controller and > + * disable the card when the second slot is selected, so it looks like the > + * second slot is always empty). > + */ > +void sd_enable(SDState *card, bool enable); > + > +#endif /* HW_SDCARD_LEGACY_H */ > diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c > index 5b47cadf11..be14ac4f40 100644 > --- a/hw/sd/omap_mmc.c > +++ b/hw/sd/omap_mmc.c > @@ -19,7 +19,7 @@ > #include "qemu/osdep.h" > #include "hw/hw.h" > #include "hw/arm/omap.h" > -#include "hw/sd/sd.h" > +#include "hw/sd/sdcard_legacy.h" > > struct omap_mmc_s { > qemu_irq irq; > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 72d32f7845..ae2d915977 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -34,6 +34,7 @@ > #include "hw/hw.h" > #include
Re: [Qemu-devel] [PATCH v3 6/7] hw/sd: make sd_data_ready() static
On Thu, Feb 15, 2018 at 6:29 PM, Philippe Mathieu-Daudéwrote: > It belongs to the legacy API (the last user has been converted). > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > include/hw/sd/sd.h | 1 - > hw/sd/sd.c | 2 +- > 2 files changed, 1 insertion(+), 2 deletions(-) > > diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h > index bf1eb0713c..4491db98de 100644 > --- a/include/hw/sd/sd.h > +++ b/include/hw/sd/sd.h > @@ -138,7 +138,6 @@ int sd_do_command(SDState *sd, SDRequest *req, > void sd_write_data(SDState *sd, uint8_t value); > uint8_t sd_read_data(SDState *sd); > void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert); > -bool sd_data_ready(SDState *sd); > /* sd_enable should not be used -- it is only used on the nseries boards, > * where it is part of a broken implementation of the MMC card slot switch > * (there should be two card slots which are multiplexed to a single MMC > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 9ac9b63ff8..72d32f7845 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -1885,7 +1885,7 @@ uint8_t sd_read_data(SDState *sd) > return ret; > } > > -bool sd_data_ready(SDState *sd) > +static bool sd_data_ready(SDState *sd) > { > return sd->state == sd_sendingdata_state; > } > -- > 2.16.1 > >
Re: [Qemu-devel] [PATCH 9/9] iotests: new test 206 for NBD BLOCK_STATUS
On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy--- tests/qemu-iotests/206 | 34 ++ tests/qemu-iotests/206.out | 2 ++ tests/qemu-iotests/group | 1 + 3 files changed, 37 insertions(+) create mode 100644 tests/qemu-iotests/206 create mode 100644 tests/qemu-iotests/206.out It's a race! Kevin already has test 206 and 207 claimed in his pending series: https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02363.html Whoever loses gets to rebase and renumber (or the maintainer on their behalf) ;) diff --git a/tests/qemu-iotests/206 b/tests/qemu-iotests/206 new file mode 100644 Oops, you forgot to mark this executable. And oops, I forgot to flag that on test 205. Other inconsistent tests: 096, 124, 129, 132, 136, 139, 148, 152, 163. Separate cleanup patch coming up soon. index 00..259e991ec6 --- /dev/null +++ b/tests/qemu-iotests/206 @@ -0,0 +1,34 @@ +#!/usr/bin/env python +# +# Tests for NBD BLOCK_STATUS extension +# +import iotests +from iotests import qemu_img_create, qemu_io, qemu_img_verbose, qemu_nbd, \ +file_path + +iotests.verify_image_format(supported_fmts=['qcow2']) Don't we also need to blacklist v2 files (and only operate on v3 or newer), since the behavior of zero clusters is much worse on v2 images? + +disk, nbd_sock = file_path('disk', 'nbd-sock') +nbd_uri = 'nbd+unix:///exp?socket=' + nbd_sock + +qemu_img_create('-f', iotests.imgfmt, disk, '1M') +qemu_io('-f', iotests.imgfmt, '-c', 'write 0 512K', disk) Hopefully large enough for all the major filesystems with sparse support to show half-data, half-hole, and not run into any weird issues where granularity differences between filesystems change the outcome. + +qemu_nbd('-k', nbd_sock, '-x', 'exp', '-f', iotests.imgfmt, disk) +qemu_img_verbose('map', '-f', 'raw', '--output=json', nbd_uri) diff --git a/tests/qemu-iotests/206.out b/tests/qemu-iotests/206.out new file mode 100644 index 00..0d29724e84 --- /dev/null +++ b/tests/qemu-iotests/206.out @@ -0,0 +1,2 @@ +[{ "start": 0, "length": 524288, "depth": 0, "zero": false, "data": true}, +{ "start": 524288, "length": 524288, "depth": 0, "zero": true, "data": false}] But definite evidence of getting block status over NBD! Progress! And kudos for producing an .out file that I can actually read and reproduce without the aid of python (in contrast to the more "unit-test"-y ones like 205, where we had a big long side-discussion about that. No need to repeat it here). diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index a2dfe79d86..2c3925566a 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -202,3 +202,4 @@ 203 rw auto 204 rw auto quick 205 rw auto quick +206 rw auto quick -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v4 02/11] sdcard: replace DPRINTF() by trace events
On Thu, Feb 15, 2018 at 2:05 PM, Philippe Mathieu-Daudéwrote: > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/sd/sd.c | 32 ++-- > hw/sd/trace-events | 6 ++ > 2 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index ce1f2fdf76..72e9b47e34 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -40,6 +40,7 @@ > #include "qemu/error-report.h" > #include "qemu/timer.h" > #include "qemu/log.h" > +#include "trace.h" > > //#define DEBUG_SD 1 > > @@ -132,6 +133,26 @@ struct SDState { > bool cmd_line; > }; > > +static const char *sd_state_name(enum SDCardStates state) > +{ > +static const char *state_name[] = { > +[sd_idle_state] = "idle", > +[sd_ready_state]= "ready", > +[sd_identification_state] = "identification", > +[sd_standby_state] = "standby", > +[sd_transfer_state] = "transfer", > +[sd_sendingdata_state] = "sendingdata", > +[sd_receivingdata_state]= "receivingdata", > +[sd_programming_state] = "programming", > +[sd_disconnect_state] = "disconnect", > +}; > +if (state == sd_inactive_state) { > +return "inactive"; > +} > +assert(state <= ARRAY_SIZE(state_name)); > +return state_name[state]; > +} > + > static uint8_t sd_get_dat_lines(SDState *sd) > { > return sd->enable ? sd->dat_lines : 0; > @@ -776,6 +797,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, > uint32_t rca = 0x; > uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : > req.arg; > > +trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state)); > + > /* Not interpreting this as an app command */ > sd->card_status &= ~APP_CMD; > > @@ -790,7 +813,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, > sd->multi_blk_cnt = 0; > } > > -DPRINTF("CMD%d 0x%08x state %d\n", req.cmd, req.arg, sd->state); > switch (req.cmd) { > /* Basic commands (Class 0 and Class 1) */ > case 0:/* CMD0: GO_IDLE_STATE */ > @@ -1310,8 +1332,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, > return sd_r1; > > case 56: /* CMD56: GEN_CMD */ > -fprintf(stderr, "SD: GEN_CMD 0x%08x\n", req.arg); > - > switch (sd->state) { > case sd_transfer_state: > sd->data_offset = 0; > @@ -1345,7 +1365,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, > static sd_rsp_type_t sd_app_command(SDState *sd, > SDRequest req) > { > -DPRINTF("ACMD%d 0x%08x\n", req.cmd, req.arg); > +trace_sdcard_app_command(req.cmd, req.arg); > sd->card_status |= APP_CMD; > switch (req.cmd) { > case 6:/* ACMD6: SET_BUS_WIDTH */ > @@ -1606,8 +1626,7 @@ send_response: > > static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) > { > -DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n", > -(unsigned long long) addr, len); > +trace_sdcard_read_block(addr, len); > if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { > fprintf(stderr, "sd_blk_read: read error on host side\n"); > } > @@ -1615,6 +1634,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, > uint32_t len) > > static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) > { > +trace_sdcard_write_block(addr, len); > if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) { > fprintf(stderr, "sd_blk_write: write error on host side\n"); > } > diff --git a/hw/sd/trace-events b/hw/sd/trace-events > index 0f8536db32..75dac5a2cd 100644 > --- a/hw/sd/trace-events > +++ b/hw/sd/trace-events > @@ -23,6 +23,12 @@ sdhci_read_dataport(uint16_t data_count) "all %u bytes of > data have been read fr > sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes > of data" > sdhci_capareg(const char *desc, uint16_t val) "%s: %u" > > +# hw/sd/sd.c > +sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d > arg 0x%08x (state %s)" > +sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x" > +sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x" > +sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size > 0x%x" > + > # hw/sd/milkymist-memcard.c > milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x > value 0x%08x" > milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x > value 0x%08x" > -- > 2.16.1 > >
Re: [Qemu-devel] [PATCH 5/6] ahci-test: fix opts leak of skip tests
On 02/15/2018 04:25 PM, Marc-André Lureau wrote: > Fixes the following ASAN report: > > Direct leak of 128 byte(s) in 8 object(s) allocated from: > #0 0x7fefce311850 in malloc (/lib64/libasan.so.4+0xde850) > #1 0x7fefcdd5ef0c in g_malloc ../glib/gmem.c:94 > #2 0x559b976faff0 in create_ahci_io_test > /home/elmarco/src/qemu/tests/ahci-test.c:1810 > > Signed-off-by: Marc-André Lureau> --- > tests/ahci-test.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/ahci-test.c b/tests/ahci-test.c > index 7aa5af428c..1bd3cc7ca8 100644 > --- a/tests/ahci-test.c > +++ b/tests/ahci-test.c > @@ -1822,6 +1822,7 @@ static void create_ahci_io_test(enum IOMode type, enum > AddrMode addr, > if ((addr == ADDR_MODE_LBA48) && (offset == OFFSET_HIGH) && > (mb_to_sectors(test_image_size_mb) <= 0xFFF)) { > g_test_message("%s: skipped; test image too small", name); > +g_free(opts); > g_free(name); > return; > } > Whupps. Thanks. Reviewed-by: John Snow And, feel free to stage this in whomever's branch, it won't conflict with anything: Acked-by: John Snow
Re: [Qemu-devel] [PATCH 8/9] iotests: add file_path helper
On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote: Simple way to have auto generated filenames with auto clenup. Like s/clenup/cleanup/ FilePath but without using 'with' statement and without additional indentation of the whole test. Signed-off-by: Vladimir Sementsov-Ogievskiy--- tests/qemu-iotests/iotests.py | 32 1 file changed, 32 insertions(+) Jeff, where do we stand on your iotests cleanups? Is this something you want to use? diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index c1302a2f9b..f2d05ca3fd 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -27,6 +27,7 @@ import struct import json import signal import logging +import atexit sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'scripts')) import qtest @@ -250,6 +251,37 @@ class FilePath(object): return False +def file_path_remover(): +for path in reversed(file_path_remover.paths): +try: +os.remove(path) +except OSError: +pass + + +def file_path(*names): +''' Another way to get auto-generated filename that cleans itself up. + +Use it as simple as: + +img_a, img_b = file_path('a.img', 'b.img') +sock = file_path('socket') +''' + +if not hasattr(file_path_remover, 'paths'): +file_path_remover.paths = [] +atexit.register(file_path_remover) + +paths = [] +for name in names: +filename = '{0}-{1}'.format(os.getpid(), name) +path = os.path.join(test_dir, filename) +file_path_remover.paths.append(path) +paths.append(path) + +return paths[0] if len(paths) == 1 else paths + + class VM(qtest.QEMUQtestMachine): '''A QEMU VM''' -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 7/9] iotests.py: tiny refactor: move system imports up
On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy--- tests/qemu-iotests/iotests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) What breaks if they aren't moved? But stylistically, this looks reasonable; and can be merged independently of NBD stuff if someone else wants. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH 6/9] nbd: BLOCK_STATUS for standard get_block_status function: client part
On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote: Minimal realization: only one extent in server answer is supported. Flag NBD_CMD_FLAG_REQ_ONE is used to force this behavior. Tests 140, 147 and 205 are fixed due to now server failed on searching export in context of NBD_OPT_SET_META_CONTEXT option negotiation. Signed-off-by: Vladimir Sementsov-Ogievskiy--- +++ b/block/nbd-client.c @@ -228,6 +228,45 @@ static int nbd_parse_offset_hole_payload(NBDStructuredReplyChunk *chunk, return 0; } +/* nbd_parse_blockstatus_payload + * support only one extent in reply and only for + * base:allocation context Reasonable, since the server should not be sending us contexts we did not ask for during negotiation. + */ +static int nbd_parse_blockstatus_payload(NBDClientSession *client, + NBDStructuredReplyChunk *chunk, + uint8_t *payload, uint64_t orig_length, + NBDExtent *extent, Error **errp) +{ +uint32_t context_id; + +if (chunk->length != sizeof(context_id) + sizeof(extent)) { Okay as long as we use REQ_ONE to ensure exactly one extent per chunk. +error_setg(errp, "Protocol error: invalid payload for " + "NBD_REPLY_TYPE_BLOCK_STATUS"); +return -EINVAL; +} + +context_id = payload_advance32(); +if (client->info.meta_base_allocation_id != context_id) { +error_setg(errp, "Protocol error: unexpected context id: %d for " + "NBD_REPLY_TYPE_BLOCK_STATUS, when negotiated context " + "id is %d", context_id, + client->info.meta_base_allocation_id); +return -EINVAL; +} + +memcpy(extent, payload, sizeof(*extent)); +be32_to_cpus(>length); +be32_to_cpus(>flags); Instead of doing a memcpy() and then in-place bit-swizzling, you could do the swapping as part of assignment, for one less function call (and make the code a bit easier to extend, if we later drop our REQ_ONE limitation on only having one extent, because you'll advance payload as needed): extent->length = payload_advance32(); extent->flags = payload_advance32(); We should probably validate that the length field is a multiple of min_block (if a server tells us that all operations must be 512-byte aligned, then reports an extent that is smaller than 512 bytes, we have no way to ask for the status of the second half of the sector). Probably also something that needs to be explicitly stated in the NBD spec. [1] + +if (extent->length > orig_length) { +error_setg(errp, "Protocol error: server sent chunk exceeding requested" + " region"); +return -EINVAL; That matches the current spec wording, but I'm not sure I agree with it - what's wrong with a server providing a final extent that extends beyond the request, if the information was already available for free (the classic example: if the server never replies with HOLE or ZERO, then the entire file has the same status, so all requests could trivially be replied to by taking the starting offset to the end of the file as the returned length, rather than just clamping at the requested length). +} + +return 0; +} + /* nbd_parse_error_payload * on success @errp contains message describing nbd error reply */ @@ -642,6 +681,61 @@ static int nbd_co_receive_cmdread_reply(NBDClientSession *s, uint64_t handle, return iter.ret; } +static int nbd_co_receive_blockstatus_reply(NBDClientSession *s, +uint64_t handle, uint64_t length, +NBDExtent *extent, Error **errp) +{ +NBDReplyChunkIter iter; +NBDReply reply; +void *payload = NULL; +Error *local_err = NULL; +bool received = false; + +NBD_FOREACH_REPLY_CHUNK(s, iter, handle, s->info.structured_reply, +NULL, , ) +{ +int ret; +NBDStructuredReplyChunk *chunk = + +assert(nbd_reply_is_structured()); + +switch (chunk->type) { +case NBD_REPLY_TYPE_BLOCK_STATUS: Depending on the outcome of the discussion on the NBD list, here's where you could make a client easily listen to both your initial server (that sent 5) and a server compliant to the current spec wording (where this constant is 3); although it remains to be seen if that's the only difference between your initial implementation and the NBD spec wording that gets promoted to stable. +if (received) { +s->quit = true; +error_setg(_err, "Several BLOCK_STATUS chunks in reply"); +nbd_iter_error(, true, -EINVAL, _err); +} We may change this in the future to ask for more than one context at once; but for now, this matches that we negotiated for
[Qemu-devel] [PATCH v7] ui/cocoa.m: Add ability for user to specify mouse ungrab key
Currently the ungrab keys for the Cocoa and GTK interface are Control-Alt-g. This combination may not be very fun for the user to have to enter, so we now enable the user to specify their own key(s) as the ungrab key(s). The list of keys that can be used is found in the file qapi/ui.json under QKeyCode. The max number of keys that can be used is three. Syntax: -display cocoa,hotkey-grab= Example usage: -display cocoa,hotkey-grab=home -display cocoa,hotkey-grab=shift-ctrl -display cocoa,hotkey-grab=ctrl-x -display cocoa,hotkey-grab=pgup-pgdn -display cocoa,hotkey-grab=kp_5-kp_6 -display cocoa,hotkey-grab=kp_4-kp_5-kp_6 -display cocoa,hotkey-grab=ctrl-alt Signed-off-by: John Arbuckle--- v7 changes: - Prevent ungrab keys from being seen by guest. v6 changes: - changed ungrab command-line option to -display cocoa,hotkey-grab - Removed NSMutableSet code - Implemented C version of Set datatype v5 changes: - Removed ungrab detection code from keydown event in handleEvent. - Removed console_ungrab_sequence_length(). - Removed ability to always use the default ctrl-alt-g ungrab key sequence. - Added ability to actually send keys to the guest that might overlap ungrab keys. Example for -ungrab ctrl-alt: down(ctrl) down(alt) up(ctrl) up(alt) ..ungrab activates.. down(ctrl) down(alt) down(f1) up(ctrl) up(alt) up(f1) ..no ungrab activates.. v4 changes: - Removed initialization code for key_value_array. - Added void keyword to console_ungrab_key_sequence(), and console_ungrab_key_string() functions. v3 changes: - Added the ability for any "sendkey supported" key to be used. - Added ability for one to three key sequences to be used. v2 changes: - Removed the "int i" code from the for loops. include/ui/console.h | 22 +++ qemu-options.hx | 1 + ui/cocoa.m | 98 + ui/console.c | 169 +++ vl.c | 17 ++ 5 files changed, 295 insertions(+), 12 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 12fef80923..5664b4c087 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -508,4 +508,26 @@ static inline void early_gtk_display_init(int opengl) /* egl-headless.c */ void egl_headless_init(void); +/* console.c */ +/* max number of keys that can be used as the ungrab keys */ +#define MAX_UNGRAB_KEYS 3 +void set_ungrab_seq(const char *new_seq); +int *console_ungrab_key_sequence(void); +const char *console_ungrab_key_string(void); +void use_default_ungrab_keys(void); +void init_ungrab_keys(void); + +/* Set datatype related code */ +typedef struct Set_struct { +int size; /* The size of the array */ +int *array; /* The array used to store the set's values */ +} Set; + +Set *new_set(int max_size); +void add_number(Set *the_set, int the_number); +void remove_number(Set *the_set, int the_number); +bool contains_number(Set *the_set, int the_number); +void clear_set(Set *the_set); +bool are_sets_equal(Set *set1, Set *set2); + #endif diff --git a/qemu-options.hx b/qemu-options.hx index 5050a49a5e..4a613e4e9c 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1243,6 +1243,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display, "[,window_close=on|off][,gl=on|off]\n" "-display gtk[,grab_on_hover=on|off][,gl=on|off]|\n" "-display vnc=[,]\n" +"-display cocoa[hotkey-grab= ]\n" "-display curses\n" "-display none" "select display type\n" diff --git a/ui/cocoa.m b/ui/cocoa.m index 51db47cd71..a4b1f9b9f4 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -106,6 +106,8 @@ bool stretch_video; NSTextField *pauseLabel; NSArray * supportedImageFileTypes; +Set *key_set, *ungrab_set; +int ungrab_sequence_length; // Mac to QKeyCode conversion const int mac_to_qkeycode_map[] = { @@ -261,6 +263,15 @@ static void handleAnyDeviceErrors(Error * err) } } +/* Sends any keys that were delayed because they were part of the ungrab set */ +static void send_key_if_delayed(Set *the_set, int keycode) +{ +if (contains_number(the_set, keycode)) { +qemu_input_event_send_key_qcode(dcl->con, keycode, true); +qemu_input_event_send_key_qcode(dcl->con, keycode, false); +} +} + /* -- QemuCocoaView @@ -489,8 +500,6 @@ - (void) switchSurface:(DisplaySurface *)surface [[fullScreenWindow contentView] setFrame:[[NSScreen mainScreen] frame]]; [normalWindow setFrame:NSMakeRect([normalWindow frame].origin.x, [normalWindow frame].origin.y - h + oldh, w, h + [normalWindow frame].size.height - oldh) display:NO animate:NO]; } else { -if (qemu_name) -[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s", qemu_name]]; [normalWindow setFrame:NSMakeRect([normalWindow
Re: [Qemu-devel] Block Migration and CPU throttling
* Peter Lieven (p...@kamp.de) wrote: > > > Am 07.02.2018 um 19:29 schrieb Dr. David Alan Gilbert: > > > > * Peter Lieven (p...@kamp.de) wrote: > >> Am 12.12.2017 um 18:05 schrieb Dr. David Alan Gilbert: > >>> * Peter Lieven (p...@kamp.de) wrote: > Am 21.09.2017 um 14:36 schrieb Dr. David Alan Gilbert: > > * Peter Lieven (p...@kamp.de) wrote: > >> Am 19.09.2017 um 16:41 schrieb Dr. David Alan Gilbert: > >>> * Peter Lieven (p...@kamp.de) wrote: > Am 19.09.2017 um 16:38 schrieb Dr. David Alan Gilbert: > > * Peter Lieven (p...@kamp.de) wrote: > >> Hi, > >> > >> I just noticed that CPU throttling and Block Migration don't work > >> together very well. > >> During block migration the throttling heuristic detects that we > >> obviously make no progress > >> in ram transfer. But the reason is the running block migration and > >> not a too high dirty pages rate. > >> > >> The result is that any VM is throttled by 99% during block > >> migration. > > Hmm that's unfortunate; do you have a bandwidth set lower than your > > actual network connection? I'm just wondering if it's actually going > > between the block and RAM iterative sections or getting stuck in ne. > It happens also if source and dest are on the same machine and speed > is set to 100G. > >>> But does it happen if they're not and the speed is set low? > >> Yes, it does. I noticed it in our test environment between different > >> nodes with a 10G > >> link in between. But its totally clear why it happens. During block > >> migration we transfer > >> all dirty memory pages in each round (if there is moderate memory > >> load), but all dirty > >> pages are obviously more than 50% of the transferred ram in that round. > >> It is more exactly 100%. But the current logic triggers on this > >> condition. > >> > >> I think I will go forward and send a patch which disables auto > >> converge during > >> block migration bulk stage. > > Yes, that's fair; it probably would also make sense to throttle the RAM > > migration during the block migration bulk stage, since the chances are > > it's not going to get far. (I think in the nbd setup, the main > > migration process isn't started until the end of bulk). > Catching up with the idea of delaying ram migration until block bulk has > completed. > What do you think is the easiest way to achieve this? > >>> > >>> > >>> I think the answer depends whether we think this is a 'special' or we > >>> need a new general purpose mechanism. > >>> > >>> If it was really general then we'd probably want to split the iterative > >>> stage in two somehow, and only do RAM in the second half. > >>> > >>> But I'm not sure it's worth it; I suspect the easiest way is: > >>> > >>>a) Add a counter in migration/ram.c or in the RAM state somewhere > >>>b) Make ram_save_inhibit increment the counter > >>>c) Check the counter at the head of ram_save_iterate and just exit > >>> if it's none 0 > >>>d) Call ram_save_inhibit from block_save_setup > >>>e) Then release it when you've finished the bulk stage > >>> > >>> Make sure you still count the RAM in the pending totals, otherwise > >>> migration might think it's finished a bit early. > >> > >> Is there any culprit I don't see or is it as easy as this? > > > > Hmm, looks promising doesn't it; might need an include or two tidied > > up, but looks worth a try. Just be careful that there are no cases > > where block migration can't transfer data in that state, otherwise we'll > > keep coming back to here and spewing empty sections. > > I already tested it and it actually works. OK. > What would you expect to be cleaned up before it would be a proper patch? It's simple enough so not much; maybe add a trace for when it does the exit just to make it easier to watch; I hadn't realised ram.c already included migration/block.h for the !bulk case. > Are there any implications with RDMA Hmm; don't think so; it's mainly concerned with how the RAM is transferred; and the ram_control_* hooks are called after your test, and on the load side only once it's read the flags and got a block. > and/or post copy migration? Again I don't think so; I think block migration always shows the outstanding amount of block storage, and so it won't flip into postcopy mode until the bulk of block migration is done. Also the 'ram_save_complete' does it's own scan of blocks and doesn't share the iterate code, so it won't be affected by your change. > Is block migration possible at all with those? Yes I think so in both cases; with RDMA I'm pretty sure it is, and I think people have had it running with postcopy as well. The postcopy case isn't great (because the actual block storage isn't
Re: [Qemu-devel] [PATCH v3] migration: change blocktime type to uint32_t
* Alexey Perevalov (a.pereva...@samsung.com) wrote: > Initially int64_t was used, but on PowerPC architecture, > clang doesn't have atomic_*_8 function, so it produces > link time error. > > QEMU is working with time as with 64bit value, but by > fact 32 bit is enough with CLOCK_REALTIME. In this case > blocktime will keep only 1200 hours time interval. Hi Alexey, > Signed-off-by: Alexey Perevalov> Acked-by: Eric Blake > --- > hmp.c| 4 ++-- > migration/postcopy-ram.c | 48 > +++- > migration/trace-events | 4 ++-- > qapi/migration.json | 4 ++-- > 4 files changed, 33 insertions(+), 27 deletions(-) > > diff --git a/hmp.c b/hmp.c > index c6bab53..3c376b3 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -265,7 +265,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) > } > > if (info->has_postcopy_blocktime) { > -monitor_printf(mon, "postcopy blocktime: %" PRId64 "\n", > +monitor_printf(mon, "postcopy blocktime: %u\n", > info->postcopy_blocktime); > } > > @@ -273,7 +273,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) > Visitor *v; > char *str; > v = string_output_visitor_new(false, ); > -visit_type_int64List(v, NULL, >postcopy_vcpu_blocktime, NULL); > +visit_type_uint32List(v, NULL, >postcopy_vcpu_blocktime, NULL); > visit_complete(v, ); > monitor_printf(mon, "postcopy vcpu blocktime: %s\n", str); > g_free(str); > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c > index 7814da5..6694fd3 100644 > --- a/migration/postcopy-ram.c > +++ b/migration/postcopy-ram.c > @@ -63,16 +63,17 @@ struct PostcopyDiscardState { > > typedef struct PostcopyBlocktimeContext { > /* time when page fault initiated per vCPU */ > -int64_t *page_fault_vcpu_time; > +uint32_t *page_fault_vcpu_time; > /* page address per vCPU */ > uintptr_t *vcpu_addr; > -int64_t total_blocktime; > +uint32_t total_blocktime; > /* blocktime per vCPU */ > -int64_t *vcpu_blocktime; > +uint32_t *vcpu_blocktime; > /* point in time when last page fault was initiated */ > -int64_t last_begin; > +uint32_t last_begin; > /* number of vCPU are suspended */ > int smp_cpus_down; > +uint64_t start_time; > > /* > * Handler for exit event, necessary for > @@ -99,22 +100,23 @@ static void migration_exit_cb(Notifier *n, void *data) > static struct PostcopyBlocktimeContext *blocktime_context_new(void) > { > PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1); > -ctx->page_fault_vcpu_time = g_new0(int64_t, smp_cpus); > +ctx->page_fault_vcpu_time = g_new0(uint32_t, smp_cpus); > ctx->vcpu_addr = g_new0(uintptr_t, smp_cpus); > -ctx->vcpu_blocktime = g_new0(int64_t, smp_cpus); > +ctx->vcpu_blocktime = g_new0(uint32_t, smp_cpus); > > ctx->exit_notifier.notify = migration_exit_cb; > +ctx->start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); > qemu_add_exit_notifier(>exit_notifier); > return ctx; > } > > -static int64List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx) > +static uint32List *get_vcpu_blocktime_list(PostcopyBlocktimeContext *ctx) > { > -int64List *list = NULL, *entry = NULL; > +uint32List *list = NULL, *entry = NULL; > int i; > > for (i = smp_cpus - 1; i >= 0; i--) { > -entry = g_new0(int64List, 1); > +entry = g_new0(uint32List, 1); > entry->value = ctx->vcpu_blocktime[i]; > entry->next = list; > list = entry; > @@ -145,7 +147,7 @@ void > fill_destination_postcopy_migration_info(MigrationInfo *info) > info->postcopy_vcpu_blocktime = get_vcpu_blocktime_list(bc); > } > > -static uint64_t get_postcopy_total_blocktime(void) > +static uint32_t get_postcopy_total_blocktime(void) > { > MigrationIncomingState *mis = migration_incoming_get_current(); > PostcopyBlocktimeContext *bc = mis->blocktime_ctx; > @@ -633,7 +635,8 @@ static void mark_postcopy_blocktime_begin(uintptr_t addr, > uint32_t ptid, > int cpu, already_received; > MigrationIncomingState *mis = migration_incoming_get_current(); > PostcopyBlocktimeContext *dc = mis->blocktime_ctx; > -int64_t now_ms; > +int64_t start_time_offset; > +uint32_t low_time_offset; > > if (!dc || ptid == 0) { > return; > @@ -643,14 +646,15 @@ static void mark_postcopy_blocktime_begin(uintptr_t > addr, uint32_t ptid, > return; > } > > -now_ms = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); > +start_time_offset = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) - > dc->start_time; > +low_time_offset = start_time_offset & UINT32_MAX; OK, I suggest you add a sanity check that start_time_offset >= 1 - to cover the cases where it was unusually quick to get to this
Re: [Qemu-devel] [PATCH v2] slirp: Add domainname option to slirp's DHCP server
Hi Philippe, Am Freitag, den 16.02.2018, 16:23 -0300 schrieb Philippe Mathieu-Daudé: > Hi Benjamin, > > On 02/16/2018 02:55 PM, Benjamin Drung wrote: > > This patch will allow the user to include the domainname option in > > replies from the built-in DHCP server. > > > > Signed-off-by: Benjamin Drung> > --- > > net/slirp.c | 7 --- > > qapi/net.json| 4 > > qemu-options.hx | 7 +-- > > slirp/bootp.c| 8 > > slirp/libslirp.h | 2 +- > > slirp/slirp.c| 4 +++- > > slirp/slirp.h| 1 + > > 7 files changed, 26 insertions(+), 7 deletions(-) > > > > diff --git a/net/slirp.c b/net/slirp.c > > index 8991816bbf..9511ff3bb7 100644 > > --- a/net/slirp.c > > +++ b/net/slirp.c > > @@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, > > const char *model, > >const char *bootfile, const char > > *vdhcp_start, > >const char *vnameserver, const char > > *vnameserver6, > >const char *smb_export, const char > > *vsmbserver, > > - const char **dnssearch, Error **errp) > > + const char **dnssearch, const char > > *vdomainname, > > + Error **errp) > > { > > /* default settings according to historic slirp */ > > struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* > > 10.0.2.0 */ > > @@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer, > > const char *model, > > s->slirp = slirp_init(restricted, ipv4, net, mask, host, > >ipv6, ip6_prefix, vprefix6_len, > > ip6_host, > >vhostname, tftp_export, bootfile, dhcp, > > - dns, ip6_dns, dnssearch, s); > > + dns, ip6_dns, dnssearch, vdomainname, > > s); > > QTAILQ_INSERT_TAIL(_stacks, s, entry); > > > > for (config = slirp_configs; config; config = config->next) { > > @@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const > > char *name, > > user->ipv6_host, user->hostname, user- > > >tftp, > > user->bootfile, user->dhcpstart, > > user->dns, user->ipv6_dns, user->smb, > > - user->smbserver, dnssearch, errp); > > + user->smbserver, dnssearch, user- > > >domainname, errp); > > > > while (slirp_configs) { > > config = slirp_configs; > > diff --git a/qapi/net.json b/qapi/net.json > > index 1238ba5de1..9dfd34cafa 100644 > > --- a/qapi/net.json > > +++ b/qapi/net.json > > @@ -160,6 +160,9 @@ > > # @dnssearch: list of DNS suffixes to search, passed as DHCP > > option > > # to the guest > > # > > +# @domainname: guest-visible domain name of the virtual nameserver > > +# (since 2.12) > > +# > > # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since > > # 2.6). The network prefix is given in the usual > > # hexadecimal IPv6 address notation. > > @@ -197,6 +200,7 @@ > > '*dhcpstart': 'str', > > '*dns': 'str', > > '*dnssearch': ['String'], > > +'*domainname': 'str', > > '*ipv6-prefix': 'str', > > '*ipv6-prefixlen': 'int', > > '*ipv6-host':'str', > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 5050a49a5e..06983db7be 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, > > "-netdev > > user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n" > > " [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6- > > host=addr]\n" > > " [,restrict=on|off][,hostname=host][,dhcpstart=addr]\ > > n" > > -" [,dns=addr][,ipv6- > > dns=addr][,dnssearch=domain][,tftp=dir]\n" > > -" [,bootfile=f][,hostfwd=rule][,guestfwd=rule]" > > +" [,dns=addr][,ipv6- > > dns=addr][,dnssearch=domain][,domainname=domain]\n" > > +" [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=ru > > le]" > > #ifndef _WIN32 > > "[,smb=dir[,smbserver > > =addr]]\n" > > #endif > > @@ -2116,6 +2116,9 @@ Example: > > qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org > > [...] > > @end example > > > > +@item domainname=@var{domain} > > +Specifies the client domain name reported by the built-in DHCP > > server. > > + > > @item tftp=@var{dir} > > When using the user mode network stack, activate a built-in TFTP > > server. The files in @var{dir} will be exposed as the root of a > > TFTP server. > > diff --git a/slirp/bootp.c b/slirp/bootp.c > > index 5dd1a415b5..91896f7400 100644 > > --- a/slirp/bootp.c > > +++ b/slirp/bootp.c > > @@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const > > struct bootp_t *bp) > > q += val; > >
Re: [Qemu-devel] [PATCH v2] slirp: Add domainname option to slirp's DHCP server
Hi Benjamin, On 02/16/2018 02:55 PM, Benjamin Drung wrote: > This patch will allow the user to include the domainname option in > replies from the built-in DHCP server. > > Signed-off-by: Benjamin Drung> --- > net/slirp.c | 7 --- > qapi/net.json| 4 > qemu-options.hx | 7 +-- > slirp/bootp.c| 8 > slirp/libslirp.h | 2 +- > slirp/slirp.c| 4 +++- > slirp/slirp.h| 1 + > 7 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/net/slirp.c b/net/slirp.c > index 8991816bbf..9511ff3bb7 100644 > --- a/net/slirp.c > +++ b/net/slirp.c > @@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const > char *model, >const char *bootfile, const char *vdhcp_start, >const char *vnameserver, const char *vnameserver6, >const char *smb_export, const char *vsmbserver, > - const char **dnssearch, Error **errp) > + const char **dnssearch, const char *vdomainname, > + Error **errp) > { > /* default settings according to historic slirp */ > struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */ > @@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer, const > char *model, > s->slirp = slirp_init(restricted, ipv4, net, mask, host, >ipv6, ip6_prefix, vprefix6_len, ip6_host, >vhostname, tftp_export, bootfile, dhcp, > - dns, ip6_dns, dnssearch, s); > + dns, ip6_dns, dnssearch, vdomainname, s); > QTAILQ_INSERT_TAIL(_stacks, s, entry); > > for (config = slirp_configs; config; config = config->next) { > @@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const char *name, > user->ipv6_host, user->hostname, user->tftp, > user->bootfile, user->dhcpstart, > user->dns, user->ipv6_dns, user->smb, > - user->smbserver, dnssearch, errp); > + user->smbserver, dnssearch, user->domainname, errp); > > while (slirp_configs) { > config = slirp_configs; > diff --git a/qapi/net.json b/qapi/net.json > index 1238ba5de1..9dfd34cafa 100644 > --- a/qapi/net.json > +++ b/qapi/net.json > @@ -160,6 +160,9 @@ > # @dnssearch: list of DNS suffixes to search, passed as DHCP option > # to the guest > # > +# @domainname: guest-visible domain name of the virtual nameserver > +# (since 2.12) > +# > # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since > # 2.6). The network prefix is given in the usual > # hexadecimal IPv6 address notation. > @@ -197,6 +200,7 @@ > '*dhcpstart': 'str', > '*dns': 'str', > '*dnssearch': ['String'], > +'*domainname': 'str', > '*ipv6-prefix': 'str', > '*ipv6-prefixlen': 'int', > '*ipv6-host':'str', > diff --git a/qemu-options.hx b/qemu-options.hx > index 5050a49a5e..06983db7be 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, > "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n" > " [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n" > " [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n" > -" [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n" > -" [,bootfile=f][,hostfwd=rule][,guestfwd=rule]" > +" > [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n" > +" [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]" > #ifndef _WIN32 > "[,smb=dir[,smbserver=addr]]\n" > #endif > @@ -2116,6 +2116,9 @@ Example: > qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...] > @end example > > +@item domainname=@var{domain} > +Specifies the client domain name reported by the built-in DHCP server. > + > @item tftp=@var{dir} > When using the user mode network stack, activate a built-in TFTP > server. The files in @var{dir} will be exposed as the root of a TFTP server. > diff --git a/slirp/bootp.c b/slirp/bootp.c > index 5dd1a415b5..91896f7400 100644 > --- a/slirp/bootp.c > +++ b/slirp/bootp.c > @@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct > bootp_t *bp) > q += val; > } > > +if (*slirp->vdomainname) { This is safe but harder to read/review... > +val = strlen(slirp->vdomainname); > +*q++ = RFC1533_DOMAINNAME; > +*q++ = val; > +memcpy(q, slirp->vdomainname, val); > +q += val; > +} > + > if (slirp->vdnssearch) { > size_t spaceleft =
[Qemu-devel] [PATCH] spapr: fix missing CPU core nodes in DT when running with TCG
Commit 5d0fb1508e2d "spapr: consolidate the VCPU id numbering logic in a single place" introduced a helper to detect thread0 of a virtual core based on its VCPU id. This is used to create CPU core nodes in the DT, but it is broken in TCG. $ qemu-system-ppc64 -nographic -accel tcg -machine dumpdtb=dtb.bin \ -smp cores=16,maxcpus=16,threads=1 $ dtc -f -O dts dtb.bin | grep POWER8 PowerPC,POWER8@0 { PowerPC,POWER8@8 { instead of the expected 16 cores that we get with KVM: $ dtc -f -O dts dtb.bin | grep POWER8 PowerPC,POWER8@0 { PowerPC,POWER8@8 { PowerPC,POWER8@10 { PowerPC,POWER8@18 { PowerPC,POWER8@20 { PowerPC,POWER8@28 { PowerPC,POWER8@30 { PowerPC,POWER8@38 { PowerPC,POWER8@40 { PowerPC,POWER8@48 { PowerPC,POWER8@50 { PowerPC,POWER8@58 { PowerPC,POWER8@60 { PowerPC,POWER8@68 { PowerPC,POWER8@70 { PowerPC,POWER8@78 { This happens because spapr_get_vcpu_id() maps VCPU ids to cs->cpu_index in TCG mode. This confuses the code in spapr_is_thread0_in_vcore(), since it assumes thread0 VCPU ids to have a spapr->vsmt spacing. spapr_get_vcpu_id(cpu) % spapr->vsmt == 0 Actually, there's no real reason to expose cs->cpu_index instead of the VCPU id, since we also generate it with TCG. Also we already set it explicitly in spapr_set_vcpu_id(), so there's no real reason either to call kvm_arch_vcpu_id() with KVM. This patch unifies spapr_get_vcpu_id() to always return the computed VCPU id both in TCG and KVM. This is one step forward towards KVM<->TCG migration. Fixes: 5d0fb1508e2d Reported-by: Cédric Le GoaterSigned-off-by: Greg Kurz --- hw/ppc/spapr.c |8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 83c9d66dd56f..d6fd0e666e74 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3810,13 +3810,7 @@ static void spapr_pic_print_info(InterruptStatsProvider *obj, int spapr_get_vcpu_id(PowerPCCPU *cpu) { -CPUState *cs = CPU(cpu); - -if (kvm_enabled()) { -return kvm_arch_vcpu_id(cs); -} else { -return cs->cpu_index; -} +return cpu->vcpu_id; } void spapr_set_vcpu_id(PowerPCCPU *cpu, int cpu_index, Error **errp)
Re: [Qemu-devel] [PATCH] cuda.h: Fix multiple typedef
On 16 February 2018 at 17:31, Dr. David Alan Gilbert (git)wrote: > From: "Dr. David Alan Gilbert" > > RHEL6's compilers don't like the repeated typedef. > > Signed-off-by: Dr. David Alan Gilbert > --- > include/hw/misc/macio/cuda.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h > index 6afbdd13ee..494b709579 100644 > --- a/include/hw/misc/macio/cuda.h > +++ b/include/hw/misc/macio/cuda.h > @@ -93,12 +93,12 @@ typedef struct CUDAState { > } CUDAState; > > /* MOS6522 CUDA */ > -typedef struct MOS6522CUDAState { > +struct MOS6522CUDAState { > /*< private >*/ > MOS6522State parent_obj; > > CUDAState *cuda; > -} MOS6522CUDAState; > +}; > > #define TYPE_MOS6522_CUDA "mos6522-cuda" > #define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \ > -- Thanks; applied to master as a buildfix. -- PMM
Re: [Qemu-devel] [PATCH v21 1/5] xbitmap: Introduce xbitmap
On Fri, Feb 16, 2018 at 07:44:50PM +0200, Andy Shevchenko wrote: > On Tue, Jan 9, 2018 at 1:10 PM, Wei Wangwrote: > > From: Matthew Wilcox > > > > The eXtensible Bitmap is a sparse bitmap representation which is > > efficient for set bits which tend to cluster. It supports up to > > 'unsigned long' worth of bits. > > > lib/xbitmap.c| 444 > > +++ > > Please, split tests to a separate module. Hah, I just did this two days ago! I didn't publish it yet, but I also made it compile both in userspace and as a kernel module. It's the top two commits here: http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray-2018-02-12 Note this is a complete rewrite compared to the version presented here; it sits on top of the XArray and no longer has a preload interface. It has a superset of the IDA functionality.
[Qemu-devel] [PATCH] kvm: add stubs for kvm_vcpu_id_is_valid() and kvm_arch_vcpu_id()
These two functions are essentially called by code that is only compiled when CONFIG_KVM=y, with the notable exception of the two users in the sPAPR code: $ git grep -E -l 'kvm_arch_vcpu_id|kvm_vcpu_id_is_valid' accel/kvm/kvm-all.c hw/intc/openpic_kvm.c hw/intc/xics_kvm.c hw/ppc/spapr.c include/sysemu/kvm.h target/arm/kvm.c target/i386/kvm.c target/mips/kvm.c target/ppc/kvm.c target/s390x/kvm.c In hw/ppc/spapr.c: if (kvm_enabled()) { return kvm_arch_vcpu_id(cs); } else { return cs->cpu_index; } and if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) { ... } This code happens to compile without CONFIG_KVM=y simply because kvm_enabled() expands to (0) and the compiler optimizes the dead code away. Unless this was done on purpose to indicate no stubs are required, and we'd rather break the build if calling these from KVM agnostic code, it seems better practice to have stubs. It doesn't really make sense to call these when kvm_enabled() is false, since they were introduced to allow the architecture to control VCPU ids passed to the KVM_CREATE_VCPU ioctl. This being said, I see no reason for not giving the stub a friendly default behaviour: - VCPU id is cpu_index - a VCPU id is always valid Signed-off-by: Greg Kurz--- accel/stubs/kvm-stub.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c index c964af3e1c97..d3e7d39ff9a9 100644 --- a/accel/stubs/kvm-stub.c +++ b/accel/stubs/kvm-stub.c @@ -160,4 +160,14 @@ bool kvm_arm_supports_user_irq(void) { return false; } + +bool kvm_vcpu_id_is_valid(int vcpu_id) +{ +return true; +} + +unsigned long kvm_arch_vcpu_id(CPUState *cpu) +{ +return cpu->cpu_index; +} #endif
[Qemu-devel] [PATCH v2] slirp: Add domainname option to slirp's DHCP server
This patch will allow the user to include the domainname option in replies from the built-in DHCP server. Signed-off-by: Benjamin Drung--- net/slirp.c | 7 --- qapi/net.json| 4 qemu-options.hx | 7 +-- slirp/bootp.c| 8 slirp/libslirp.h | 2 +- slirp/slirp.c| 4 +++- slirp/slirp.h| 1 + 7 files changed, 26 insertions(+), 7 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index 8991816bbf..9511ff3bb7 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -157,7 +157,8 @@ static int net_slirp_init(NetClientState *peer, const char *model, const char *bootfile, const char *vdhcp_start, const char *vnameserver, const char *vnameserver6, const char *smb_export, const char *vsmbserver, - const char **dnssearch, Error **errp) + const char **dnssearch, const char *vdomainname, + Error **errp) { /* default settings according to historic slirp */ struct in_addr net = { .s_addr = htonl(0x0a000200) }; /* 10.0.2.0 */ @@ -371,7 +372,7 @@ static int net_slirp_init(NetClientState *peer, const char *model, s->slirp = slirp_init(restricted, ipv4, net, mask, host, ipv6, ip6_prefix, vprefix6_len, ip6_host, vhostname, tftp_export, bootfile, dhcp, - dns, ip6_dns, dnssearch, s); + dns, ip6_dns, dnssearch, vdomainname, s); QTAILQ_INSERT_TAIL(_stacks, s, entry); for (config = slirp_configs; config; config = config->next) { @@ -958,7 +959,7 @@ int net_init_slirp(const Netdev *netdev, const char *name, user->ipv6_host, user->hostname, user->tftp, user->bootfile, user->dhcpstart, user->dns, user->ipv6_dns, user->smb, - user->smbserver, dnssearch, errp); + user->smbserver, dnssearch, user->domainname, errp); while (slirp_configs) { config = slirp_configs; diff --git a/qapi/net.json b/qapi/net.json index 1238ba5de1..9dfd34cafa 100644 --- a/qapi/net.json +++ b/qapi/net.json @@ -160,6 +160,9 @@ # @dnssearch: list of DNS suffixes to search, passed as DHCP option # to the guest # +# @domainname: guest-visible domain name of the virtual nameserver +# (since 2.12) +# # @ipv6-prefix: IPv6 network prefix (default is fec0::) (since # 2.6). The network prefix is given in the usual # hexadecimal IPv6 address notation. @@ -197,6 +200,7 @@ '*dhcpstart': 'str', '*dns': 'str', '*dnssearch': ['String'], +'*domainname': 'str', '*ipv6-prefix': 'str', '*ipv6-prefixlen': 'int', '*ipv6-host':'str', diff --git a/qemu-options.hx b/qemu-options.hx index 5050a49a5e..06983db7be 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1906,8 +1906,8 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev, "-netdev user,id=str[,ipv4[=on|off]][,net=addr[/mask]][,host=addr]\n" " [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n" " [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n" -" [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,tftp=dir]\n" -" [,bootfile=f][,hostfwd=rule][,guestfwd=rule]" +" [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n" +" [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]" #ifndef _WIN32 "[,smb=dir[,smbserver=addr]]\n" #endif @@ -2116,6 +2116,9 @@ Example: qemu -net user,dnssearch=mgmt.example.org,dnssearch=example.org [...] @end example +@item domainname=@var{domain} +Specifies the client domain name reported by the built-in DHCP server. + @item tftp=@var{dir} When using the user mode network stack, activate a built-in TFTP server. The files in @var{dir} will be exposed as the root of a TFTP server. diff --git a/slirp/bootp.c b/slirp/bootp.c index 5dd1a415b5..91896f7400 100644 --- a/slirp/bootp.c +++ b/slirp/bootp.c @@ -298,6 +298,14 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp) q += val; } +if (*slirp->vdomainname) { +val = strlen(slirp->vdomainname); +*q++ = RFC1533_DOMAINNAME; +*q++ = val; +memcpy(q, slirp->vdomainname, val); +q += val; +} + if (slirp->vdnssearch) { size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend); val = slirp->vdnssearch_len; diff --git a/slirp/libslirp.h b/slirp/libslirp.h index 540b3e5903..740408a96e 100644 --- a/slirp/libslirp.h +++ b/slirp/libslirp.h @@ -16,7 +16,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork, const char
Re: [Qemu-devel] [PATCH v4 1/2] qmp: adding 'wakeup-suspend-support' in query-target
On 02/15/2018 07:14 PM, Michael Roth wrote: Quoting Daniel Henrique Barboza (2018-01-05 07:03:13) When issuing the qmp/hmp 'system_wakeup' command, what happens in a nutshell is: - qmp_system_wakeup_request set runstate to RUNNING, sets a wakeup_reason and notify the event - in the main_loop, all vcpus are paused, a system reset is issued, all subscribers of wakeup_notifiers receives a notification, vcpus are then resumed and the wake up QAPI event is fired Note that this procedure alone doesn't ensure that the guest will awake from SUSPENDED state - the subscribers of the wake up event must take action to resume the guest, otherwise the guest will simply reboot. At this moment there are only two subscribers of the wake up event: one in hw/acpi/core.c and another one in hw/i386/xen/xen-hvm.c. This means that system_wakeup does not work as intended with other architectures. However, only the presence of 'system_wakeup' is required for QGA to support 'guest-suspend-ram' and 'guest-suspend-hybrid' at this moment. This means that the user/management will expect to suspend the guest using one of those suspend commands and then resume execution using system_wakeup, regardless of the support offered in system_wakeup in the first place. This patch adds a new flag called 'wakeup-suspend-support' in TargetInfo that allows the caller to query if the guest supports wake up from suspend via system_wakeup. It goes over the subscribers of the wake up event and, if it's empty, it assumes that the guest does not support wake up from suspend (and thus, pm-suspend itself). This is the expected output of query-target when running a x86 guest: {"execute" : "query-target"} {"return": {"arch": "x86_64", "wakeup-suspend-support": true}} This is the output when running a pseries guest: {"execute" : "query-target"} {"return": {"arch": "ppc64", "wakeup-suspend-support": false}} Given that the TargetInfo structure is read-only, adding a new flag to it is backwards compatible. There is no need to deprecate the old TargetInfo format. With this extra tool, management can avoid situations where a guest that does not have proper suspend/wake capabilities ends up in inconsistent state (e.g. https://github.com/open-power-host-os/qemu/issues/31). Signed-off-by: Daniel Henrique Barboza--- arch_init.c | 1 + include/sysemu/sysemu.h | 1 + qapi-schema.json| 4 +++- vl.c| 29 + 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/arch_init.c b/arch_init.c index a0b8ed6167..9c5c519d9d 100644 --- a/arch_init.c +++ b/arch_init.c @@ -109,6 +109,7 @@ TargetInfo *qmp_query_target(Error **errp) TargetInfo *info = g_malloc0(sizeof(*info)); info->arch = g_strdup(TARGET_NAME); +info->wakeup_suspend_support = !qemu_wakeup_notifier_is_empty(); return info; } diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 31612caf10..b15374e0b8 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -69,6 +69,7 @@ typedef enum WakeupReason { void qemu_system_reset_request(ShutdownCause reason); void qemu_system_suspend_request(void); void qemu_register_suspend_notifier(Notifier *notifier); +bool qemu_wakeup_notifier_is_empty(void); void qemu_system_wakeup_request(WakeupReason reason); void qemu_system_wakeup_enable(WakeupReason reason, bool enabled); void qemu_register_wakeup_notifier(Notifier *notifier); diff --git a/qapi-schema.json b/qapi-schema.json index 5c06745c79..2f1c08fde7 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2388,11 +2388,13 @@ # Information describing the QEMU target. # # @arch: the target architecture (eg "x86_64", "i386", etc) +# @wakeup-suspend-support: true if the target supports wake up from +# suspend (since 2.12) # # Since: 1.2.0 ## { 'struct': 'TargetInfo', - 'data': { 'arch': 'str' } } + 'data': { 'arch': 'str', 'wakeup-suspend-support': 'bool' } } ## # @query-target: diff --git a/vl.c b/vl.c index d3a5c5d021..56d6a1ce5b 100644 --- a/vl.c +++ b/vl.c @@ -1839,11 +1839,40 @@ void qemu_system_wakeup_enable(WakeupReason reason, bool enabled) } } +/* The existence of a wake-up notifier is being checked in the function + * qemu_wakeup_notifier_is_empty and it's used in the logic of the + * wakeup-suspend-support flag of QMP 'query-target' command. The idea + * of this flag is to indicate whether the guest supports wake-up from + * suspend (via system_wakeup QMP/HMP call for example), warning the user + * that the guest can't handle both wake-up from suspend and the suspend + * itself via QGA guest-suspend-ram and guest-suspend-hybrid (if it + * can't wake up, it can't be suspended safely). + * + * The motivation of this design is that this notification will only + * fire if a wake up attempt is made by qemu_system_wakeup_request and + * the current runstate is RUN_STATE_SUSPENDED.
[Qemu-devel] [PULL 1/2] monitor: Remove legacy "-mon default=on" parameter
From: Thomas HuthThe "default" parameter of the "-mon" option is useless since QEMU v2.4.0, and marked as deprecated since QEMU v2.8.0. That should have been long enough to let people update their scripts, so time to remove it now. Signed-off-by: Thomas Huth Message-Id: <1513700253-10045-1-git-send-email-th...@redhat.com> Reviewed-by: Dr. David Alan Gilbert Signed-off-by: Dr. David Alan Gilbert --- monitor.c | 3 --- qemu-doc.texi | 9 - vl.c | 4 3 files changed, 16 deletions(-) diff --git a/monitor.c b/monitor.c index f4992505b1..9a0d480c53 100644 --- a/monitor.c +++ b/monitor.c @@ -4142,9 +4142,6 @@ QemuOptsList qemu_mon_opts = { },{ .name = "chardev", .type = QEMU_OPT_STRING, -},{ -.name = "default", /* deprecated */ -.type = QEMU_OPT_BOOL, },{ .name = "pretty", .type = QEMU_OPT_BOOL, diff --git a/qemu-doc.texi b/qemu-doc.texi index 137f5814a8..56388b5b33 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -2628,15 +2628,6 @@ setting ``-machine kernel_irqchip=off''. The ``-no-kvm'' argument is now a synonym for setting ``-machine accel=tcg''. -@subsection -mon default=on (since 2.4.0) - -The ``default'' option to the ``-mon'' argument is -now ignored. When multiple monitors were enabled, it -indicated which monitor would receive log messages -from the various subsystems. This feature is no longer -required as messages are now only sent to the monitor -in response to explicitly monitor commands. - @subsection -vnc tls (since 2.5.0) The ``-vnc tls'' argument is now a synonym for setting diff --git a/vl.c b/vl.c index 7a5554bc41..81724f5f17 100644 --- a/vl.c +++ b/vl.c @@ -2423,10 +2423,6 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp) if (qemu_opt_get_bool(opts, "pretty", 0)) flags |= MONITOR_USE_PRETTY; -if (qemu_opt_get_bool(opts, "default", 0)) { -error_report("option 'default' does nothing and is deprecated"); -} - chardev = qemu_opt_get(opts, "chardev"); chr = qemu_chr_find(chardev); if (chr == NULL) { -- 2.14.3
[Qemu-devel] [PULL 0/2] hmp queue
From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> The following changes since commit 5e8d6a12d643a38b82a0a713a77d1192117dbdca: Merge remote-tracking branch 'remotes/kraxel/tags/ui-20180216-pull-request' into staging (2018-02-16 15:55:45 +) are available in the Git repository at: git://github.com/dagrh/qemu.git tags/pull-hmp-20180216 for you to fetch changes up to bf67f1c0b16c0de43b8a10cb53808dd62b0cdc04: monitor.c: Fix infinite loop in monitor's auto-complete (2018-02-16 17:36:16 +) HMP pull 2018-02-16 Dr. David Alan Gilbert (1): monitor.c: Fix infinite loop in monitor's auto-complete Thomas Huth (1): monitor: Remove legacy "-mon default=on" parameter monitor.c | 9 - qemu-doc.texi | 9 - vl.c | 4 3 files changed, 4 insertions(+), 18 deletions(-)
[Qemu-devel] [PULL 2/2] monitor.c: Fix infinite loop in monitor's auto-complete
From: "Dr. David Alan Gilbert"The QEMU monitor enters an infinite loop when trying to auto-complete commands that accept only optional parameters. The commands currently affected by this issue are 'info registers' and 'info mtree'. Reported-by: Dimitris Karagkasidis Fixes: 48fe86f6400574165979e0db6f5937ad487b6888 Signed-off-by: Dr. David Alan Gilbert Message-Id: <20180213125143.23488-1-dgilb...@redhat.com> Reviewed-by: Stefan Hajnoczi Signed-off-by: Dr. David Alan Gilbert --- monitor.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/monitor.c b/monitor.c index 9a0d480c53..373bb8d1c3 100644 --- a/monitor.c +++ b/monitor.c @@ -3696,7 +3696,7 @@ static void monitor_find_completion_by_table(Monitor *mon, { const char *cmdname; int i; -const char *ptype, *str, *name; +const char *ptype, *old_ptype, *str, *name; const mon_cmd_t *cmd; BlockBackend *blk = NULL; @@ -3741,7 +3741,9 @@ static void monitor_find_completion_by_table(Monitor *mon, } } str = args[nb_args - 1]; -while (*ptype == '-' && ptype[1] != '\0') { +old_ptype = NULL; +while (*ptype == '-' && old_ptype != ptype) { +old_ptype = ptype; ptype = next_arg_type(ptype); } switch(*ptype) { -- 2.14.3
Re: [Qemu-devel] [PATCH] cuda.h: Fix multiple typedef
On 02/16/2018 02:31 PM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert"> > RHEL6's compilers don't like the repeated typedef. > > Signed-off-by: Dr. David Alan Gilbert Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé > --- > include/hw/misc/macio/cuda.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h > index 6afbdd13ee..494b709579 100644 > --- a/include/hw/misc/macio/cuda.h > +++ b/include/hw/misc/macio/cuda.h > @@ -93,12 +93,12 @@ typedef struct CUDAState { > } CUDAState; > > /* MOS6522 CUDA */ > -typedef struct MOS6522CUDAState { > +struct MOS6522CUDAState { > /*< private >*/ > MOS6522State parent_obj; > > CUDAState *cuda; > -} MOS6522CUDAState; > +}; > > #define TYPE_MOS6522_CUDA "mos6522-cuda" > #define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \ >
Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer
On 02/16/2018 02:41 PM, Kamil Rytarowski wrote: > On 16.02.2018 18:30, Philippe Mathieu-Daudé wrote: >> But before announcing the host OS being supported again, I'd rather see >> reproducible build/tests logs, in a (public - if possible) continuous >> integration system. Else it is hard to notice when it get broken. >> > > This is already done for FreeBSD, NetBSD and OpenBSD. We have the ability to run those, but afaik no CI are using them. $ make vm-test vm-test: Test QEMU in preconfigured virtual machines vm-build-ubuntu.i386- Build QEMU in ubuntu i386 VM vm-build-freebsd- Build QEMU in FreeBSD VM vm-build-netbsd - Build QEMU in NetBSD VM vm-build-openbsd- Build QEMU in OpenBSD VM > > CC: Fam who can confirm this. signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer
On Fri, Feb 16, 2018 at 05:37:32PM +, Peter Maydell wrote: > On 16 February 2018 at 17:30, Philippe Mathieu-Daudéwrote: > > But before announcing the host OS being supported again, I'd rather see > > reproducible build/tests logs, in a (public - if possible) continuous > > integration system. Else it is hard to notice when it get broken. > > I do include OpenBSD in my set of build/tests for merges. There's also "make vm-build-openbsd" for devs who want to test easily in an OpenBSD VM, or investigate brokeness. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 1/2] hw/sysbus.h: New sysbus_init_child() helper function
On 02/16/2018 01:28 PM, Igor Mammedov wrote: > On Fri, 16 Feb 2018 13:45:15 + > Peter Maydellwrote: ... >> static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent); >> static char *sysbus_get_fw_dev_path(DeviceState *dev); >> @@ -372,6 +373,19 @@ BusState *sysbus_get_default(void) >> return main_system_bus; >> } >> >> +void sysbus_init_child(Object *parent, const char *childname, >> + void *child, size_t childsize, >> + const char *childtype) >> +{ > > >> +object_initialize(child, childsize, childtype); >> +/* error_abort is fine here because this can only fail for >> + * programming-error reasons: child already parented, or >> + * parent already has a child with the given name. >> + */ >> +object_property_add_child(parent, childname, OBJECT(child), >> _abort); > It would be useful not only for sysbus devices. > maybe we should extend object_initialize instead, > something like this: >void object_initialize(void *data, size_t size, const char *typename, > Object *parent, const char *name) > and set parent in it. > git counts about 152 uses, so it would be tree wide change > but still not too much. we can keep object_initialize() when no parent, and add object_initialize_child(, const char *childname, Object *parent) 'parent' last because all previous args are child-related. > > >> +qdev_set_parent_bus(DEVICE(child), sysbus_get_default()); > and then assuming we don't create sysbus devices, nor should be able to, > which are not attached to sysbus_get_default() this one can go sysbus' > instance_init() good idea. > > then there won't be need for sysbus specific helper, > inheritance will do the rest of the job. > >> +} >> +
Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer
On 16.02.2018 18:30, Philippe Mathieu-Daudé wrote: > But before announcing the host OS being supported again, I'd rather see > reproducible build/tests logs, in a (public - if possible) continuous > integration system. Else it is hard to notice when it get broken. > This is already done for FreeBSD, NetBSD and OpenBSD. CC: Fam who can confirm this. signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 5/9] nbd/client: fix error messages in nbd_handle_reply_err
On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote: 1. NBD_REP_ERR_INVALID is not only about length, so, make message more general 2. hex format is not very good: it's hard to read something like "option a (set meta context)", so switch to dec. It would be okay as option 0xa; I also want to be sure we match the output in server traces with the output in client traces; for example, I found: nbd/server.c-error_setg(errp, "Unsupported option 0x%" PRIx32 " (%s)", nbd/server.c: option, nbd_opt_lookup(option)); So we want consistency through all the call sites, before this patch is ready to go, although I agree we need it. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer
On 16 February 2018 at 17:30, Philippe Mathieu-Daudéwrote: > But before announcing the host OS being supported again, I'd rather see > reproducible build/tests logs, in a (public - if possible) continuous > integration system. Else it is hard to notice when it get broken. I do include OpenBSD in my set of build/tests for merges. thanks -- PMM
Re: [Qemu-devel] [PATCH 4/9] block/nbd-client: save first fatal error in nbd_iter_error
On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote: It is ok, that fatal error hides previous not fatal, but hiding first fatal error is a bad feature. Signed-off-by: Vladimir Sementsov-Ogievskiy--- block/nbd-client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Can this trigger in 2.11 clients with structured reads to the point where we'd want to add qemu-stable in CC? Or did you only hit it once block_status was added to the mix? Note that comparing 2.11 server with 2.11 client doesn't help - because 2.11 as server only sends a single chunk per structured read (as the only structured reply); it wasn't until commit 418638d3 that we have a qemu server that can send multiple chunks, and it looks like you need multiple chunks before multiple errors becomes a possibility. At any rate, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH] cuda.h: Fix multiple typedef
From: "Dr. David Alan Gilbert"RHEL6's compilers don't like the repeated typedef. Signed-off-by: Dr. David Alan Gilbert --- include/hw/misc/macio/cuda.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/hw/misc/macio/cuda.h b/include/hw/misc/macio/cuda.h index 6afbdd13ee..494b709579 100644 --- a/include/hw/misc/macio/cuda.h +++ b/include/hw/misc/macio/cuda.h @@ -93,12 +93,12 @@ typedef struct CUDAState { } CUDAState; /* MOS6522 CUDA */ -typedef struct MOS6522CUDAState { +struct MOS6522CUDAState { /*< private >*/ MOS6522State parent_obj; CUDAState *cuda; -} MOS6522CUDAState; +}; #define TYPE_MOS6522_CUDA "mos6522-cuda" #define MOS6522_CUDA(obj) OBJECT_CHECK(MOS6522CUDAState, (obj), \ -- 2.14.3
[Qemu-devel] [RFC 3/5] vhost_net: send virtio device status update to the backend
This patch adds ans uses a new function to send virtio device status updates to the backend. Suggested-by: Stefan HajnocziSigned-off-by: Maxime Coquelin --- hw/net/vhost_net.c | 10 ++ hw/net/virtio-net.c | 7 ++- include/net/vhost_net.h | 2 ++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index e037db63a3..75e2165163 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -450,6 +450,11 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu) return vhost_ops->vhost_net_set_mtu(>dev, mtu); } +int vhost_net_set_virtio_status(struct vhost_net *net, uint8_t status) +{ +return vhost_dev_set_virtio_status(>dev, status); +} + #else uint64_t vhost_net_get_max_queues(VHostNetState *net) { @@ -521,4 +526,9 @@ int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu) { return 0; } + +int vhost_net_set_virtio_status(struct vhost_net *net, uint8_t status) +{ +return 0; +} #endif diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 38674b08aa..06c431ea28 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -136,7 +136,7 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) if ((virtio_net_started(n, status) && !nc->peer->link_down) == !!n->vhost_started) { -return; +goto out; } if (!n->vhost_started) { int r, i; @@ -175,11 +175,16 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status) error_report("unable to start vhost net: %d: " "falling back on userspace virtio", -r); n->vhost_started = 0; + +return; } } else { vhost_net_stop(vdev, n->nic->ncs, queues); n->vhost_started = 0; } + +out: +vhost_net_set_virtio_status(get_vhost_net(nc->peer), status); } static int virtio_net_set_vnet_endian_one(VirtIODevice *vdev, diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h index afc1499eb9..bf083e2d2e 100644 --- a/include/net/vhost_net.h +++ b/include/net/vhost_net.h @@ -37,4 +37,6 @@ uint64_t vhost_net_get_acked_features(VHostNetState *net); int vhost_net_set_mtu(struct vhost_net *net, uint16_t mtu); +int vhost_net_set_virtio_status(struct vhost_net *net, uint8_t status); + #endif -- 2.14.3
Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer
Hi Brad, On 02/16/2018 01:46 PM, Brad Smith wrote: > Add myself as an OpenBSD maintainer and add OpenBSD as maintained. > > Signed-off-by: Brad Smith> > > diff --git a/MAINTAINERS b/MAINTAINERS > index 57358a08e2..86329e274f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -386,6 +386,12 @@ M: Kamil Rytarowski > S: Maintained > K: ^Subject:.*(?i)NetBSD > > +OPENBSD > +L: qemu-devel@nongnu.org > +M: Brad Smith > +S: Maintained > +K: ^Subject:.*(?i)OpenBSD > + No problem here, > W32, W64 > L: qemu-devel@nongnu.org > M: Stefan Weil > diff --git a/configure b/configure > index 913e14839d..850c46ebac 100755 > --- a/configure > +++ b/configure > @@ -760,6 +760,7 @@ OpenBSD) >audio_drv_list="sdl" >audio_possible_drivers="sdl" >HOST_VARIANT_DIR="openbsd" > + supported_os="yes" But before announcing the host OS being supported again, I'd rather see reproducible build/tests logs, in a (public - if possible) continuous integration system. Else it is hard to notice when it get broken. > ;; > Darwin) >bsd="yes" > Regards, Phil.
[Qemu-devel] [RFC 2/5] vhost-user: Introduce new request to send virtio device status
This patch implements the .vhost_set_virtio_status() backend callback for user backend by intooducing a new vhost-user VHOST_USER_SET_VIRTIO_STATUS request. Suggested-by: Stefan HajnocziSigned-off-by: Maxime Coquelin --- docs/interop/vhost-user.txt | 14 ++ hw/virtio/vhost-user.c | 35 +++ 2 files changed, 49 insertions(+) diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt index 9fcf48d611..daa452bd36 100644 --- a/docs/interop/vhost-user.txt +++ b/docs/interop/vhost-user.txt @@ -368,6 +368,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_MTU4 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN 6 +#define VHOST_USER_PROTOCOL_F_VIRTIO_STATUS 7 Master message types @@ -663,6 +664,19 @@ Master message types field, and slaves MUST NOT accept SET_CONFIG for read-only configuration space fields unless the live migration bit is set. +* VHOST_USER_SET_VIRTIO_STATUS + + Id: 26 + Equivalent ioctl: N/A + Master payload: u64 + Slave payload: N/A + + Sent by the vhost-user master to notify of virtio device status change. + The payload is a u64 representing the virtio device status as defined in + the virtio specification. + The request should be sent only when VHOST_USER_PROTOCOL_F_VIRTIO_STATUS + protocol feature has been negotiated. + Slave message types --- diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 6eb97980ad..519646799b 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -39,6 +39,7 @@ enum VhostUserProtocolFeature { VHOST_USER_PROTOCOL_F_NET_MTU = 4, VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5, VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6, +VHOST_USER_PROTOCOL_F_VIRTIO_STATUS = 7, VHOST_USER_PROTOCOL_F_MAX }; @@ -72,6 +73,7 @@ typedef enum VhostUserRequest { VHOST_USER_SET_VRING_ENDIAN = 23, VHOST_USER_GET_CONFIG = 24, VHOST_USER_SET_CONFIG = 25, +VHOST_USER_SET_VIRTIO_STATUS = 26, VHOST_USER_MAX } VhostUserRequest; @@ -1054,6 +1056,38 @@ static int vhost_user_set_config(struct vhost_dev *dev, const uint8_t *data, return 0; } +static int vhost_user_set_virtio_status(struct vhost_dev *dev, uint8_t status) +{ +bool reply_supported = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_REPLY_ACK); + +VhostUserMsg msg = { +.hdr.request = VHOST_USER_SET_VIRTIO_STATUS, +.hdr.flags = VHOST_USER_VERSION, +.hdr.size = sizeof(msg.payload.u64), +.payload.u64 = status, +}; + +if (!virtio_has_feature(dev->protocol_features, +VHOST_USER_PROTOCOL_F_VIRTIO_STATUS)) { +return 0; +} + +if (reply_supported) { +msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK; +} + +if (vhost_user_write(dev, , NULL, 0) < 0) { +return -1; +} + +if (reply_supported) { +return process_message_reply(dev, ); +} + +return 0; +} + const VhostOps user_ops = { .backend_type = VHOST_BACKEND_TYPE_USER, .vhost_backend_init = vhost_user_init, @@ -1082,4 +1116,5 @@ const VhostOps user_ops = { .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg, .vhost_get_config = vhost_user_get_config, .vhost_set_config = vhost_user_set_config, +.vhost_set_virtio_status = vhost_user_set_virtio_status, }; -- 2.14.3
[Qemu-devel] [RFC 5/5] vhost-user-scsi: send virtio status to the backend
Suggested-by: Stefan HajnocziSigned-off-by: Maxime Coquelin --- hw/scsi/vhost-user-scsi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 9389ed48e0..da36a3b7f5 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -58,6 +58,8 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status) } else { vhost_scsi_common_stop(vsc); } + +vhost_dev_set_virtio_status(>dev, status); } static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) -- 2.14.3
[Qemu-devel] [RFC 4/5] vhost-user-blk: send virtio status to the backend
Suggested-by: Stefan HajnocziSigned-off-by: Maxime Coquelin --- hw/block/vhost-user-blk.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index b53b4c9c57..a88b6f13a4 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -190,6 +190,7 @@ static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status) vhost_user_blk_stop(vdev); } +vhost_dev_set_virtio_status(>dev, status); } static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev, -- 2.14.3
[Qemu-devel] [RFC 0/5] vhost-user: Forward virtio device status updates
This series introduces a new vhost-user request to notify the backend with virtio device status updates. This is done to address the case when the guest driver only intializes a subset of the virtqueues. For example, it happens with Windows virtio-net driver, when the virtio-net device has more queue pairs than vCPUs. With Virtio 1.0 devices, the driver sets DRIVER_OK after having intialized all queues, so the backend can use this information to start the vhost port. With legacy devices, this is not guaranteed as mentionned in the spec, so the backend should not rely on DRIVER_OK. A solution has yet to be found for legacy devices. Maxime Coquelin (5): vhost: send virtio device status update to the backend vhost-user: Introduce new request to send virtio device status vhost_net: send virtio device status update to the backend vhost-user-blk: send virtio status to the backend vhost-user-scsi: send virtio status to the backend docs/interop/vhost-user.txt | 14 ++ hw/block/vhost-user-blk.c | 1 + hw/net/vhost_net.c| 10 ++ hw/net/virtio-net.c | 7 ++- hw/scsi/vhost-user-scsi.c | 2 ++ hw/virtio/vhost-user.c| 35 +++ hw/virtio/vhost.c | 11 +++ include/hw/virtio/vhost-backend.h | 3 +++ include/hw/virtio/vhost.h | 3 +++ include/net/vhost_net.h | 2 ++ 10 files changed, 87 insertions(+), 1 deletion(-) -- 2.14.3
[Qemu-devel] [RFC 1/5] vhost: send virtio device status update to the backend
This patch adds a function to notify the vhost backend with virtio device status updates. Suggested-by: Stefan HajnocziSigned-off-by: Maxime Coquelin --- hw/virtio/vhost.c | 11 +++ include/hw/virtio/vhost-backend.h | 3 +++ include/hw/virtio/vhost.h | 3 +++ 3 files changed, 17 insertions(+) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 338e4395b7..95981e5a32 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1546,6 +1546,17 @@ void vhost_dev_set_config_notifier(struct vhost_dev *hdev, hdev->config_ops = ops; } +int vhost_dev_set_virtio_status(struct vhost_dev *hdev, uint8_t status) +{ +const VhostOps *vhost_ops = hdev->vhost_ops; + +if (vhost_ops && vhost_ops->vhost_set_virtio_status) { +return vhost_ops->vhost_set_virtio_status(hdev, status); +} + +return 0; +} + /* Host notifiers must be enabled at this point. */ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev) { diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h index 592254f40d..45f6b8f6c5 100644 --- a/include/hw/virtio/vhost-backend.h +++ b/include/hw/virtio/vhost-backend.h @@ -94,6 +94,8 @@ typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *data, uint32_t flags); typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config, uint32_t config_len); +typedef int (*vhost_set_virtio_status_op)(struct vhost_dev *dev, + uint8_t status); typedef struct VhostOps { VhostBackendType backend_type; @@ -130,6 +132,7 @@ typedef struct VhostOps { vhost_send_device_iotlb_msg_op vhost_send_device_iotlb_msg; vhost_get_config_op vhost_get_config; vhost_set_config_op vhost_set_config; +vhost_set_virtio_status_op vhost_set_virtio_status; } VhostOps; extern const VhostOps user_ops; diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index 1dc2d73d76..fe055b2bd5 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -121,4 +121,7 @@ int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data, */ void vhost_dev_set_config_notifier(struct vhost_dev *dev, const VhostDevConfigOps *ops); + +int vhost_dev_set_virtio_status(struct vhost_dev *hdev, uint8_t status); + #endif -- 2.14.3
Re: [Qemu-devel] [PATCH v3 2/5] block: extract AIO_WAIT_WHILE() from BlockDriverState
On 02/16/2018 10:50 AM, Stefan Hajnoczi wrote: BlockDriverState has the BDRV_POLL_WHILE() macro to wait on event loop activity while a condition evaluates to true. This is used to implement synchronous operations where it acts as a condvar between the IOThread running the operation and the main loop waiting for the operation. It can also be called from the thread that owns the AioContext and in that case it's just a nested event loop. BlockBackend needs this behavior but doesn't always have a BlockDriverState it can use. This patch extracts BDRV_POLL_WHILE() into the AioWait abstraction, which can be used with AioContext and isn't tied to BlockDriverState anymore. This feature could be built directly into AioContext but then all users would kick the event loop even if they signal different conditions. Imagine an AioContext with many BlockDriverStates, each time a request completes any waiter would wake up and re-check their condition. It's nicer to keep a separate AioWait object for each condition instead. Please see "block/aio-wait.h" for details on the API. The name AIO_WAIT_WHILE() avoids the confusion between AIO_POLL_WHILE() and AioContext polling. Signed-off-by: Stefan Hajnoczi--- Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v3 1/5] aio: rename aio_context_in_iothread() to in_aio_context_home_thread()
On 02/16/2018 10:50 AM, Stefan Hajnoczi wrote: The name aio_context_in_iothread() is misleading because it also return s/return/returns/ true when called on the main AioContext from the main loop thread, which is not an IOThread. This patch renames it to in_aio_context_home_thread() and expands the doc comment to make the semantics clearer. Signed-off-by: Stefan Hajnoczi--- include/block/aio.h | 7 +-- include/block/block.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) /** + * in_aio_context_home_thread: * @ctx: the aio context * - * Return whether we are running in the I/O thread that manages @ctx. + * Return whether we are running in the thread that normally runs @ctx. Note + * that acquiring/releasing ctx does not affect the outcome, each AioContext + * still only has a one home thread that is responsible for running it. s/a one/one/ Typo fixes are trivial, so Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v9 14/29] hmp: add 'info sev' command
* Brijesh Singh (brijesh.si...@amd.com) wrote: > The command can be used to show the SEV information when memory > encryption is enabled on AMD platform. > > Cc: Eric Blake> Cc: "Daniel P. Berrangé" > Cc: "Dr. David Alan Gilbert" > Cc: Markus Armbruster > Signed-off-by: Brijesh Singh That's ok, you might like to add something to decode the policy into a human readable form. You might also want to ifdef it for x86 (like info lapic). Reviewed-by: Dr. David Alan Gilbert > --- > hmp-commands-info.hx | 14 ++ > hmp.c| 19 +++ > hmp.h| 1 + > 3 files changed, 34 insertions(+) > > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx > index ad590a4ffb2b..236811c465d2 100644 > --- a/hmp-commands-info.hx > +++ b/hmp-commands-info.hx > @@ -865,6 +865,20 @@ STEXI > @findex info memory_size_summary > Display the amount of initially allocated and present hotpluggable (if > enabled) memory in bytes. > +ETEXI > + > +{ > +.name = "sev", > +.args_type = "", > +.params = "", > +.help = "show SEV information", > +.cmd= hmp_info_sev, > +}, > + > +STEXI > +@item info sev > +@findex info sev > +Show SEV information. > ETEXI > > STEXI > diff --git a/hmp.c b/hmp.c > index 7870d6a3004e..f51a107a9be3 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -2924,3 +2924,22 @@ void hmp_info_memory_size_summary(Monitor *mon, const > QDict *qdict) > } > hmp_handle_error(mon, ); > } > + > +void hmp_info_sev(Monitor *mon, const QDict *qdict) > +{ > +SevInfo *info; > + > +info = qmp_query_sev(NULL); > +monitor_printf(mon, "sev support: "); > +monitor_printf(mon, "%s\n", info->enabled ? "enabled" : "disabled"); > + > +if (info->enabled) { > +monitor_printf(mon, "state: %s\n", SevState_str(info->state)); > +monitor_printf(mon, "policy: 0x%x\n", info->policy); > +monitor_printf(mon, "build id: %u\n", info->build_id); > +monitor_printf(mon, "api version: %u.%u\n", > + info->api_major, info->api_minor); > +} > + > +qapi_free_SevInfo(info); > +} > diff --git a/hmp.h b/hmp.h > index 1143db44a760..4ca1a77b2c1f 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -146,5 +146,6 @@ void hmp_info_ramblock(Monitor *mon, const QDict *qdict); > void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict); > void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict); > void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict); > +void hmp_info_sev(Monitor *mon, const QDict *qdict); > > #endif > -- > 2.14.3 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer
On 16.02.2018 17:46, Brad Smith wrote: > Add myself as an OpenBSD maintainer and add OpenBSD as maintained. > > Signed-off-by: Brad Smith> Reviewed-by: Kamil Rytarowski Would you like to co-maintain with myself a merge-branch for BSD? Right now I send patches through other developers and it does not always work smoothly. > > diff --git a/MAINTAINERS b/MAINTAINERS > index 57358a08e2..86329e274f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -386,6 +386,12 @@ M: Kamil Rytarowski > S: Maintained > K: ^Subject:.*(?i)NetBSD > > +OPENBSD > +L: qemu-devel@nongnu.org > +M: Brad Smith > +S: Maintained > +K: ^Subject:.*(?i)OpenBSD > + > W32, W64 > L: qemu-devel@nongnu.org > M: Stefan Weil > diff --git a/configure b/configure > index 913e14839d..850c46ebac 100755 > --- a/configure > +++ b/configure > @@ -760,6 +760,7 @@ OpenBSD) >audio_drv_list="sdl" >audio_possible_drivers="sdl" >HOST_VARIANT_DIR="openbsd" > + supported_os="yes" > ;; > Darwin) >bsd="yes" > signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 3/9] nbd: BLOCK_STATUS for standard get_block_status function: server part
On 02/16/2018 08:43 AM, Vladimir Sementsov-Ogievskiy wrote: 16.02.2018 16:21, Eric Blake wrote: On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote: Minimal realization: only one extent in server answer is supported. Signed-off-by: Vladimir Sementsov-Ogievskiy--- Again, function comments are useful. + if (query[0] == '\0' || strcmp(query, "allocation") == 0) { + /* Note: empty query should select all contexts within base + * namespace. */ + meta->base_allocation = true; From the client perspective, this handling of the empty leaf-name works well for NBD_OPT_LIST_META_CONTEXT (I want to see what leaf nodes the server supports), but not so well for NBD_OPT_SET_META_CONTEXT (asking the server to send ALL base allocations, even when I don't necessarily know how to interpret anything other than base:allocation, is a waste). So this function needs a bool parameter that says whether it is being invoked from _LIST (empty string okay, to advertise ALL base leaf names back to client, which for now is just base:allocation) or from _SET (empty string is ignored as invalid; client has to specifically ask for base:allocation by name). "empty string is ignored as invalid", hm, do we have this in spec? I think SET and LIST must select exactly same sets of contexts. No, it is specifically NOT intended that SET and LIST have to produce the same set of contexts; although I do see your point that as currently written, it does not appear to require SET to ignore "base:" or to treat it as an error. At any rate, the spec gives the example of: In either case, however, for any given namespace the server MAY, instead of exhaustively listing every matching context available to select (or every context available to select where no query is given), send sufficient context records back to allow a client with knowledge of the namespace to select any context. This may be helpful where a client can construct algorithmic queries. For instance, a client might reply simply with the namespace with no leaf-name (e.g. 'X-FooBar:') or with a range of values (e.g. 'X-ModifiedDate:20160310-20161214'). The semantics of such a reply are a matter for the definition of the namespace. However each namespace returned MUST begin with the relevant namespace, followed by a colon, and then other UTF-8 characters, with the entire string following the restrictions for strings set out earlier in this document. with the intent being that for some namespaces, it may be easy to perform an algorithmic query via _LIST to see what ranges are supported, but that you cannot select ALL elements in the range simultaneously. The empty query for _LIST exists to enumerate what is supported, but does not have to equate to an empty query for _SET selecting everything possible. I could even see it being possible to have some round-trips, depending on the namespace (of course, any namespace other than "base:" will be tightly coordinated between both client and server, so they understand each other - the point was that the NBD spec didn't want to constrain what a client and server could do as long as they stay within the generic framework): C> LIST "" S> REPLY "base:allocation" id 0 S> REPLY "X-FooBar:" id 0 S> ACK C> LIST "X-FooBar:" S> REPLY "X-FooBar:A_Required", id 0 S> REPLY "X-FooBar:A_Min=100", id 0 S> REPLY "X-FooBar:A_Max=200", id 0 S> REPLY "X-FooBar:B_Default=300", id 0 S> REPLY "X-FooBar:B_Min=300", id 0 S> REPLY "X-FooBar:B_Max=400", id 0 S> ACK C> SET "X-FooBar:A=150" "base:allocation" S> REPLY "X-FooBar:A=150,B=300", id 1 S> REPLY "base:allocation", id 2 S> ACK where the global query of all available contexts merely lists that X-FooBar: is understood, but that a specific query is needed for more details (to avoid the client having to parse those specifics if it doesn't care about X-FooBar:), and the specific query sets up the algorithmic description (parameter A is required, between 100 and 200; parameter B is optional, between 300 and 400, defaulting to 300), and the final SET gives the actual request (A given a value, B left to its default; but the reply names the entire context rather than repeating the shorter request). So the spec is written to permit something like that for third-party namespaces, while also trying to be very specific about the "base:" context as that is the one that needs the most interoperability. It is strange behavior of client to set "base:", but it is its decision. And I don't thing that it is invalid. For LIST, querying "base:" makes total sense (for the sake of example, we may add "base:frob" down the road that does something new. Being able to LIST whether "base:" turns into just "base:allocation" or into "base:allocation"+"base:frob" may be useful to a client that understands both formats and wants to probe if the server is new; and even for a client right now, the client can gracefully
Re: [Qemu-devel] [qemu-s390x] [PATCH v6 06/12] s390-ccw: parse and set boot menu options
On 16.02.2018 17:44, Collin L. Walling wrote: > On 02/16/2018 11:36 AM, Viktor Mihajlovski wrote: >> On 16.02.2018 17:20, Thomas Huth wrote: >> [...] diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index cab8a97..7c3cab8 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; #define QIPL_ADDRESS 0xcc +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 + struct QemuIplParameters { - uint8_t reserved1[4]; + uint8_t boot_menu_flags; + uint8_t reserved1; + uint32_t boot_menu_timeout; uint64_t netboot_start_addr; >>> The netboot_start_addr field is now never aligned anymore, neither on >>> the host side, nor in guest memory. Not a big problem since the struct >>> is declared with "QEMU_PACKED", but still ... it's always nicer to try >>> to align fields to their natural boundaries. So maybe move >>> boot_menu_flags and reserved1 after netboot_start_addr ? >>> >> Good catch ... we probably should document the alignment needs and state >> that the ipl parameters starts on a word boundary (and that the block >> may not be larger than 28 bytes) in a comment block. > > How does this sound? > > /* word aligned and cannot exceed 28 bytes */ > Maybe: /* * The Qemu IPL Parameters will be stored 32-bit word aligned. * Placement of data fields in this area must account for * their alignment needs. * The entire structure must not be larger than 28 bytes. */ > - uint8_t reserved2[16]; + uint8_t reserved2[14]; } QEMU_PACKED; typedef struct QemuIplParameters QemuIplParameters; >>> Thomas >>> >> > > -- Regards, Viktor Mihajlovski
[Qemu-devel] [PATCH v3 4/5] block: test blk_aio_flush() with blk->root == NULL
From: Kevin WolfThis patch adds test cases for the scenario where blk_aio_flush() is called on a BlockBackend with no root. Calling drain afterwards should complete the requests with -ENOMEDIUM. Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- tests/Makefile.include | 2 ++ tests/test-block-backend.c | 82 ++ 2 files changed, 84 insertions(+) create mode 100644 tests/test-block-backend.c diff --git a/tests/Makefile.include b/tests/Makefile.include index a1bcbffe12..c15cb2dca4 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -83,6 +83,7 @@ gcov-files-test-hbitmap-y = blockjob.c check-unit-y += tests/test-bdrv-drain$(EXESUF) check-unit-y += tests/test-blockjob$(EXESUF) check-unit-y += tests/test-blockjob-txn$(EXESUF) +check-unit-y += tests/test-block-backend$(EXESUF) check-unit-y += tests/test-x86-cpuid$(EXESUF) # all code tested by test-x86-cpuid is inside topology.h gcov-files-test-x86-cpuid-y = @@ -612,6 +613,7 @@ tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y) tests/test-bdrv-drain$(EXESUF): tests/test-bdrv-drain.o $(test-block-obj-y) $(test-util-obj-y) tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-util-obj-y) tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y) +tests/test-block-backend$(EXESUF): tests/test-block-backend.o $(test-block-obj-y) $(test-util-obj-y) tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y) tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y) tests/test-hbitmap$(EXESUF): tests/test-hbitmap.o $(test-util-obj-y) $(test-crypto-obj-y) diff --git a/tests/test-block-backend.c b/tests/test-block-backend.c new file mode 100644 index 00..fd59f02bd0 --- /dev/null +++ b/tests/test-block-backend.c @@ -0,0 +1,82 @@ +/* + * BlockBackend tests + * + * Copyright (c) 2017 Kevin Wolf + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "block/block.h" +#include "sysemu/block-backend.h" +#include "qapi/error.h" + +static void test_drain_aio_error_flush_cb(void *opaque, int ret) +{ +bool *completed = opaque; + +g_assert(ret == -ENOMEDIUM); +*completed = true; +} + +static void test_drain_aio_error(void) +{ +BlockBackend *blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL); +BlockAIOCB *acb; +bool completed = false; + +acb = blk_aio_flush(blk, test_drain_aio_error_flush_cb, ); +g_assert(acb != NULL); +g_assert(completed == false); + +blk_drain(blk); +g_assert(completed == true); + +blk_unref(blk); +} + +static void test_drain_all_aio_error(void) +{ +BlockBackend *blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL); +BlockAIOCB *acb; +bool completed = false; + +acb = blk_aio_flush(blk, test_drain_aio_error_flush_cb, ); +g_assert(acb != NULL); +g_assert(completed == false); + +blk_drain_all(); +g_assert(completed == true); + +blk_unref(blk); +} + +int main(int argc, char **argv) +{ +bdrv_init(); +qemu_init_main_loop(_abort); + +g_test_init(, , NULL); + +g_test_add_func("/block-backend/drain_aio_error", test_drain_aio_error); +g_test_add_func("/block-backend/drain_all_aio_error", +test_drain_all_aio_error); + +return g_test_run(); +} -- 2.14.3
[Qemu-devel] [PATCH v3 2/5] block: extract AIO_WAIT_WHILE() from BlockDriverState
BlockDriverState has the BDRV_POLL_WHILE() macro to wait on event loop activity while a condition evaluates to true. This is used to implement synchronous operations where it acts as a condvar between the IOThread running the operation and the main loop waiting for the operation. It can also be called from the thread that owns the AioContext and in that case it's just a nested event loop. BlockBackend needs this behavior but doesn't always have a BlockDriverState it can use. This patch extracts BDRV_POLL_WHILE() into the AioWait abstraction, which can be used with AioContext and isn't tied to BlockDriverState anymore. This feature could be built directly into AioContext but then all users would kick the event loop even if they signal different conditions. Imagine an AioContext with many BlockDriverStates, each time a request completes any waiter would wake up and re-check their condition. It's nicer to keep a separate AioWait object for each condition instead. Please see "block/aio-wait.h" for details on the API. The name AIO_WAIT_WHILE() avoids the confusion between AIO_POLL_WHILE() and AioContext polling. Signed-off-by: Stefan Hajnoczi--- util/Makefile.objs| 2 +- include/block/aio-wait.h | 116 ++ include/block/block.h | 40 +++- include/block/block_int.h | 7 ++- block.c | 5 ++ block/io.c| 10 +--- util/aio-wait.c | 40 7 files changed, 174 insertions(+), 46 deletions(-) create mode 100644 include/block/aio-wait.h create mode 100644 util/aio-wait.c diff --git a/util/Makefile.objs b/util/Makefile.objs index 3fb611631f..ae90b9963d 100644 --- a/util/Makefile.objs +++ b/util/Makefile.objs @@ -1,7 +1,7 @@ util-obj-y = osdep.o cutils.o unicode.o qemu-timer-common.o util-obj-y += bufferiszero.o util-obj-y += lockcnt.o -util-obj-y += aiocb.o async.o thread-pool.o qemu-timer.o +util-obj-y += aiocb.o async.o aio-wait.o thread-pool.o qemu-timer.o util-obj-y += main-loop.o iohandler.o util-obj-$(CONFIG_POSIX) += aio-posix.o util-obj-$(CONFIG_POSIX) += compatfd.o diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h new file mode 100644 index 00..a48c744fa8 --- /dev/null +++ b/include/block/aio-wait.h @@ -0,0 +1,116 @@ +/* + * AioContext wait support + * + * Copyright (C) 2018 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef QEMU_AIO_WAIT_H +#define QEMU_AIO_WAIT_H + +#include "block/aio.h" + +/** + * AioWait: + * + * An object that facilitates synchronous waiting on a condition. The main + * loop can wait on an operation running in an IOThread as follows: + * + * AioWait *wait = ...; + * AioContext *ctx = ...; + * MyWork work = { .done = false }; + * schedule_my_work_in_iothread(ctx, ); + * AIO_WAIT_WHILE(wait, ctx, !work.done); + * + * The IOThread must call aio_wait_kick() to notify the main loop when + * work.done changes: + * + * static void do_work(...) + * { + * ... + * work.done = true; + * aio_wait_kick(wait); + * } + */ +typedef struct { +/* Is the main loop waiting for a kick? Accessed with atomic ops. */ +bool need_kick; +} AioWait; + +/** + * AIO_WAIT_WHILE: + * @wait: the aio wait object + * @ctx: the aio context + * @cond: wait while this conditional expression is true + * + * Wait while a condition is true. Use this to implement synchronous + * operations that require event loop activity. + * + * The caller must be sure that something calls aio_wait_kick() when the value + * of @cond might have changed. + * + * The caller's thread must be the IOThread that owns @ctx or the main loop + * thread (with @ctx acquired exactly once). This function cannot be used to + * wait on conditions between two IOThreads since that could lead to deadlock, + * go via the main loop instead. + */ +#define
[Qemu-devel] [PATCH v3 1/5] aio: rename aio_context_in_iothread() to in_aio_context_home_thread()
The name aio_context_in_iothread() is misleading because it also return true when called on the main AioContext from the main loop thread, which is not an IOThread. This patch renames it to in_aio_context_home_thread() and expands the doc comment to make the semantics clearer. Signed-off-by: Stefan Hajnoczi--- include/block/aio.h | 7 +-- include/block/block.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/include/block/aio.h b/include/block/aio.h index e9aeeaec94..fc14a2acda 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -534,11 +534,14 @@ void aio_co_enter(AioContext *ctx, struct Coroutine *co); AioContext *qemu_get_current_aio_context(void); /** + * in_aio_context_home_thread: * @ctx: the aio context * - * Return whether we are running in the I/O thread that manages @ctx. + * Return whether we are running in the thread that normally runs @ctx. Note + * that acquiring/releasing ctx does not affect the outcome, each AioContext + * still only has a one home thread that is responsible for running it. */ -static inline bool aio_context_in_iothread(AioContext *ctx) +static inline bool in_aio_context_home_thread(AioContext *ctx) { return ctx == qemu_get_current_aio_context(); } diff --git a/include/block/block.h b/include/block/block.h index 19b3ab9cb5..d559419722 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -372,7 +372,7 @@ void bdrv_drain_all(void); bool busy_ = true; \ BlockDriverState *bs_ = (bs); \ AioContext *ctx_ = bdrv_get_aio_context(bs_); \ -if (aio_context_in_iothread(ctx_)) { \ +if (in_aio_context_home_thread(ctx_)) {\ while ((cond) || busy_) { \ busy_ = aio_poll(ctx_, (cond));\ waited_ |= !!(cond) | busy_; \ -- 2.14.3
[Qemu-devel] [PATCH v3 3/5] block: add BlockBackend->in_flight counter
BlockBackend currently relies on BlockDriverState->in_flight to track requests for blk_drain(). There is a corner case where BlockDriverState->in_flight cannot be used though: blk->root can be NULL when there is no medium. This results in a segfault when the NULL pointer is dereferenced. Introduce a BlockBackend->in_flight counter for aio requests so it works even when blk->root == NULL. Based on a patch by Kevin Wolf. Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi --- block.c | 2 +- block/block-backend.c | 60 +-- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/block.c b/block.c index 9e4da81213..a83037c2a5 100644 --- a/block.c +++ b/block.c @@ -4713,7 +4713,7 @@ out: AioContext *bdrv_get_aio_context(BlockDriverState *bs) { -return bs->aio_context; +return bs ? bs->aio_context : qemu_get_aio_context(); } AioWait *bdrv_get_aio_wait(BlockDriverState *bs) diff --git a/block/block-backend.c b/block/block-backend.c index 0266ac990b..a775a3dd2f 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -73,6 +73,14 @@ struct BlockBackend { int quiesce_counter; VMChangeStateEntry *vmsh; bool force_allow_inactivate; + +/* Number of in-flight aio requests. BlockDriverState also counts + * in-flight requests but aio requests can exist even when blk->root is + * NULL, so we cannot rely on its counter for that case. + * Accessed with atomic ops. + */ +unsigned int in_flight; +AioWait wait; }; typedef struct BlockBackendAIOCB { @@ -1225,11 +1233,22 @@ int blk_make_zero(BlockBackend *blk, BdrvRequestFlags flags) return bdrv_make_zero(blk->root, flags); } +static void blk_inc_in_flight(BlockBackend *blk) +{ +atomic_inc(>in_flight); +} + +static void blk_dec_in_flight(BlockBackend *blk) +{ +atomic_dec(>in_flight); +aio_wait_kick(>wait); +} + static void error_callback_bh(void *opaque) { struct BlockBackendAIOCB *acb = opaque; -bdrv_dec_in_flight(acb->common.bs); +blk_dec_in_flight(acb->blk); acb->common.cb(acb->common.opaque, acb->ret); qemu_aio_unref(acb); } @@ -1240,7 +1259,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk, { struct BlockBackendAIOCB *acb; -bdrv_inc_in_flight(blk_bs(blk)); +blk_inc_in_flight(blk); acb = blk_aio_get(_backend_aiocb_info, blk, cb, opaque); acb->blk = blk; acb->ret = ret; @@ -1263,7 +1282,7 @@ static const AIOCBInfo blk_aio_em_aiocb_info = { static void blk_aio_complete(BlkAioEmAIOCB *acb) { if (acb->has_returned) { -bdrv_dec_in_flight(acb->common.bs); +blk_dec_in_flight(acb->rwco.blk); acb->common.cb(acb->common.opaque, acb->rwco.ret); qemu_aio_unref(acb); } @@ -1284,7 +1303,7 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset, int bytes, BlkAioEmAIOCB *acb; Coroutine *co; -bdrv_inc_in_flight(blk_bs(blk)); +blk_inc_in_flight(blk); acb = blk_aio_get(_aio_em_aiocb_info, blk, cb, opaque); acb->rwco = (BlkRwCo) { .blk= blk, @@ -1521,14 +1540,41 @@ int blk_flush(BlockBackend *blk) void blk_drain(BlockBackend *blk) { -if (blk_bs(blk)) { -bdrv_drain(blk_bs(blk)); +BlockDriverState *bs = blk_bs(blk); + +if (bs) { +bdrv_drained_begin(bs); +} + +/* We may have -ENOMEDIUM completions in flight */ +AIO_WAIT_WHILE(>wait, +blk_get_aio_context(blk), +atomic_mb_read(>in_flight) > 0); + +if (bs) { +bdrv_drained_end(bs); } } void blk_drain_all(void) { -bdrv_drain_all(); +BlockBackend *blk = NULL; + +bdrv_drain_all_begin(); + +while ((blk = blk_all_next(blk)) != NULL) { +AioContext *ctx = blk_get_aio_context(blk); + +aio_context_acquire(ctx); + +/* We may have -ENOMEDIUM completions in flight */ +AIO_WAIT_WHILE(>wait, ctx, +atomic_mb_read(>in_flight) > 0); + +aio_context_release(ctx); +} + +bdrv_drain_all_end(); } void blk_set_on_error(BlockBackend *blk, BlockdevOnError on_read_error, -- 2.14.3
[Qemu-devel] [PATCH v3 5/5] Revert "IDE: Do not flush empty CDROM drives"
This reverts commit 4da97120d51a4383aa96d741a2b837f8c4bbcd0b. blk_aio_flush() now handles the blk->root == NULL case, so we no longer need this workaround. Cc: John SnowSigned-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake --- hw/ide/core.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 257b429381..139c843514 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1087,15 +1087,7 @@ static void ide_flush_cache(IDEState *s) s->status |= BUSY_STAT; ide_set_retry(s); block_acct_start(blk_get_stats(s->blk), >acct, 0, BLOCK_ACCT_FLUSH); - -if (blk_bs(s->blk)) { -s->pio_aiocb = blk_aio_flush(s->blk, ide_flush_cb, s); -} else { -/* XXX blk_aio_flush() crashes when blk_bs(blk) is NULL, remove this - * temporary workaround when blk_aio_*() functions handle NULL blk_bs. - */ -ide_flush_cb(s, 0); -} +s->pio_aiocb = blk_aio_flush(s->blk, ide_flush_cb, s); } static void ide_cfata_metadata_inquiry(IDEState *s) -- 2.14.3
[Qemu-devel] [PATCH v3 0/5] block: fix blk_aio_*() segfault when blk->root == NULL
v3: * Add Patch 1 to rename aio_context_in_iothread() to in_aio_context_home_thread() [Eric] v2: * Introduce AIO_WAIT_WHILE() since aio_poll(ctx, true) is not allowed [Paolo] Using bdrv_inc_in_flight(blk_bs(blk)) doesn't work since BlockBackend->root may be NULL. This patch series solves the issue by adding an BlockBackend->in_flight counter so requests can be tracked even when there is no BlockDriverState. This should fix the IDE and virtio-blk segfaults that have been encountered when there is no BlockDriverState. The patch is based on work by Kevin Wolf. Kevin Wolf (1): block: test blk_aio_flush() with blk->root == NULL Stefan Hajnoczi (4): aio: rename aio_context_in_iothread() to in_aio_context_home_thread() block: extract AIO_WAIT_WHILE() from BlockDriverState block: add BlockBackend->in_flight counter Revert "IDE: Do not flush empty CDROM drives" tests/Makefile.include | 2 + util/Makefile.objs | 2 +- include/block/aio-wait.h | 116 + include/block/aio.h| 7 ++- include/block/block.h | 40 +++- include/block/block_int.h | 7 ++- block.c| 7 ++- block/block-backend.c | 60 --- block/io.c | 10 +--- hw/ide/core.c | 10 +--- tests/test-block-backend.c | 82 util/aio-wait.c| 40 12 files changed, 318 insertions(+), 65 deletions(-) create mode 100644 include/block/aio-wait.h create mode 100644 tests/test-block-backend.c create mode 100644 util/aio-wait.c -- 2.14.3
[Qemu-devel] [PATCH] maintainers: Add myself as a OpenBSD maintainer
Add myself as an OpenBSD maintainer and add OpenBSD as maintained. Signed-off-by: Brad Smithdiff --git a/MAINTAINERS b/MAINTAINERS index 57358a08e2..86329e274f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -386,6 +386,12 @@ M: Kamil Rytarowski S: Maintained K: ^Subject:.*(?i)NetBSD +OPENBSD +L: qemu-devel@nongnu.org +M: Brad Smith +S: Maintained +K: ^Subject:.*(?i)OpenBSD + W32, W64 L: qemu-devel@nongnu.org M: Stefan Weil diff --git a/configure b/configure index 913e14839d..850c46ebac 100755 --- a/configure +++ b/configure @@ -760,6 +760,7 @@ OpenBSD) audio_drv_list="sdl" audio_possible_drivers="sdl" HOST_VARIANT_DIR="openbsd" + supported_os="yes" ;; Darwin) bsd="yes"
Re: [Qemu-devel] [PULL 0/9] Ui 20180216 patches
On 16 February 2018 at 11:54, Gerd Hoffmann <kra...@redhat.com> wrote: > The following changes since commit fb68096da3d35e64c88cd610c1fa42766c58e92a: > > Revert "tests: use memfd in vhost-user-test" (2018-02-13 09:51:52 +) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/ui-20180216-pull-request > > for you to fetch changes up to d50f09ff23f5509c05e3883440849b27af051f08: > > ui: extend VNC trottling tracing to SASL codepaths (2018-02-16 12:33:02 > +0100) > > > bugfixes for vnc and sdl2 > > Applied, thanks. -- PMM
Re: [Qemu-devel] [qemu-s390x] [PATCH v6 06/12] s390-ccw: parse and set boot menu options
On 02/16/2018 11:36 AM, Viktor Mihajlovski wrote: On 16.02.2018 17:20, Thomas Huth wrote: [...] diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index cab8a97..7c3cab8 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; #define QIPL_ADDRESS 0xcc +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 + struct QemuIplParameters { -uint8_t reserved1[4]; +uint8_t boot_menu_flags; +uint8_t reserved1; +uint32_t boot_menu_timeout; uint64_t netboot_start_addr; The netboot_start_addr field is now never aligned anymore, neither on the host side, nor in guest memory. Not a big problem since the struct is declared with "QEMU_PACKED", but still ... it's always nicer to try to align fields to their natural boundaries. So maybe move boot_menu_flags and reserved1 after netboot_start_addr ? Good catch ... we probably should document the alignment needs and state that the ipl parameters starts on a word boundary (and that the block may not be larger than 28 bytes) in a comment block. How does this sound? /* word aligned and cannot exceed 28 bytes */ -uint8_t reserved2[16]; +uint8_t reserved2[14]; } QEMU_PACKED; typedef struct QemuIplParameters QemuIplParameters; Thomas -- - Collin L Walling
Re: [Qemu-devel] [qemu-s390x] [PATCH v6 06/12] s390-ccw: parse and set boot menu options
On 16.02.2018 17:20, Thomas Huth wrote: [...] >> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h >> index cab8a97..7c3cab8 100644 >> --- a/hw/s390x/ipl.h >> +++ b/hw/s390x/ipl.h >> @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; >> >> #define QIPL_ADDRESS 0xcc >> >> +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 >> +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 >> + >> struct QemuIplParameters { >> -uint8_t reserved1[4]; >> +uint8_t boot_menu_flags; >> +uint8_t reserved1; >> +uint32_t boot_menu_timeout; >> uint64_t netboot_start_addr; > > The netboot_start_addr field is now never aligned anymore, neither on > the host side, nor in guest memory. Not a big problem since the struct > is declared with "QEMU_PACKED", but still ... it's always nicer to try > to align fields to their natural boundaries. So maybe move > boot_menu_flags and reserved1 after netboot_start_addr ? > Good catch ... we probably should document the alignment needs and state that the ipl parameters starts on a word boundary (and that the block may not be larger than 28 bytes) in a comment block. >> -uint8_t reserved2[16]; >> +uint8_t reserved2[14]; >> } QEMU_PACKED; >> typedef struct QemuIplParameters QemuIplParameters; > > Thomas > -- Regards, Viktor Mihajlovski
Re: [Qemu-devel] [PATCH 1/2] hw/sysbus.h: New sysbus_init_child() helper function
On Fri, 16 Feb 2018 13:45:15 + Peter Maydellwrote: > If you're using the increasingly-common QOM style of > having container devices create their child objects > in-place, you end up with a lot of boilerplate in the > container's init function: > > object_initialize() to init the child > object_property_add_child() to make the child a > child of the parent > qdev_set_parent_bus() to put the child on the > sysbus default bus > > If you forget the second of these then things sort of > work but trying to add a child to the child will segfault; > if you forget the third then the device won't get reset. > > Provide a helper function sysbus_init_child() which > does all these things for you, reducing the boilerplate > and making it harder to get wrong. > > Code that used to look like this: > object_initialize(>ic, sizeof(s->ic), TYPE_BCM2835_IC); > object_property_add_child(obj, "ic", OBJECT(>ic), NULL); > qdev_set_parent_bus(DEVICE(>ic), sysbus_get_default()); > can now look like this: > sysbus_init_child(obj, "ic", >ic, sizeof(s->ic), TYPE_BCM2835_IC); > > Signed-off-by: Peter Maydell > --- > include/hw/sysbus.h | 12 > hw/core/sysbus.c| 14 ++ > 2 files changed, 26 insertions(+) > > diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h > index e88bb6dae0..6095e24e86 100644 > --- a/include/hw/sysbus.h > +++ b/include/hw/sysbus.h > @@ -118,4 +118,16 @@ static inline DeviceState > *sysbus_try_create_simple(const char *name, > return sysbus_try_create_varargs(name, addr, irq, NULL); > } > > +/** > + * sysbus_init_child: in-place initialize and parent a SysBus device > + * @parent: object to parent the device to > + * @childname: name to use as the child property name > + * @child: child object > + * @childsize: size of the storage for the object > + * @childtype: type name of the child > + */ > +void sysbus_init_child(Object *parent, const char *childname, > + void *child, size_t childsize, > + const char *childtype); > + > #endif /* HW_SYSBUS_H */ > diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c > index 5d0887f499..acfa52dc68 100644 > --- a/hw/core/sysbus.c > +++ b/hw/core/sysbus.c > @@ -21,6 +21,7 @@ > #include "hw/sysbus.h" > #include "monitor/monitor.h" > #include "exec/address-spaces.h" > +#include "qapi/error.h" > > static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent); > static char *sysbus_get_fw_dev_path(DeviceState *dev); > @@ -372,6 +373,19 @@ BusState *sysbus_get_default(void) > return main_system_bus; > } > > +void sysbus_init_child(Object *parent, const char *childname, > + void *child, size_t childsize, > + const char *childtype) > +{ > +object_initialize(child, childsize, childtype); > +/* error_abort is fine here because this can only fail for > + * programming-error reasons: child already parented, or > + * parent already has a child with the given name. > + */ > +object_property_add_child(parent, childname, OBJECT(child), > _abort); It would be useful not only for sysbus devices. maybe we should extend object_initialize instead, something like this: void object_initialize(void *data, size_t size, const char *typename, Object *parent, const char *name) and set parent in it. git counts about 152 uses, so it would be tree wide change but still not too much. > +qdev_set_parent_bus(DEVICE(child), sysbus_get_default()); and then assuming we don't create sysbus devices, nor should be able to, which are not attached to sysbus_get_default() this one can go sysbus' instance_init() then there won't be need for sysbus specific helper, inheritance will do the rest of the job. > +} > + > static void sysbus_register_types(void) > { > type_register_static(_bus_info);
Re: [Qemu-devel] [qemu-s390x] [PATCH v6 06/12] s390-ccw: parse and set boot menu options
On 02/16/2018 11:20 AM, Thomas Huth wrote: On 15.02.2018 23:54, Collin L. Walling wrote: Set boot menu options for an s390 guest and store them in the iplb. These options are set via the QEMU command line option: -boot menu=on|off[,splash-time=X] or via the libvirt domain xml: Where X represents some positive integer representing milliseconds. Any value set for loadparm will override all boot menu options. If loadparm=PROMPT, then the menu will be enabled without a timeout. The absence of any boot options on the command line will flag to later use the zipl boot loader values. Signed-off-by: Collin L. WallingReviewed-by: Janosch Frank Reviewed-by: Thomas Huth --- hw/s390x/ipl.c | 48 hw/s390x/ipl.h | 9 +++-- pc-bios/s390-ccw/iplb.h | 6 -- 3 files changed, 59 insertions(+), 4 deletions(-) [] diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h index cab8a97..7c3cab8 100644 --- a/hw/s390x/ipl.h +++ b/hw/s390x/ipl.h @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; #define QIPL_ADDRESS 0xcc +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 + struct QemuIplParameters { -uint8_t reserved1[4]; +uint8_t boot_menu_flags; +uint8_t reserved1; +uint32_t boot_menu_timeout; uint64_t netboot_start_addr; The netboot_start_addr field is now never aligned anymore, neither on the host side, nor in guest memory. Not a big problem since the struct is declared with "QEMU_PACKED", but still ... it's always nicer to try to align fields to their natural boundaries. So maybe move boot_menu_flags and reserved1 after netboot_start_addr ? Makes sense. Will fixup. -uint8_t reserved2[16]; +uint8_t reserved2[14]; } QEMU_PACKED; typedef struct QemuIplParameters QemuIplParameters; Thomas -- - Collin L Walling
Re: [Qemu-devel] [PATCH] monitor.c: Fix infinite loop in monitor's auto-complete
* Stefan Hajnoczi (stefa...@gmail.com) wrote: > On Tue, Feb 13, 2018 at 12:51:43PM +, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert"> > > > Please include the details of how to trigger this bug. This helps > justify the patch as well as aiding anyone investigating/backporting the > same issue in the future. OK, I'll include Dimitris's: The QEMU monitor enters an infinite loop when trying to auto-complete commands that accept only optional parameters. The commands currently affected by this issue are 'info registers' and 'info mtree'. > Aside from that: > Reviewed-by: Stefan Hajnoczi Thanks! Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] monitor.c: Fix infinite loop in monitor's auto-complete
* Dr. David Alan Gilbert (git) (dgilb...@redhat.com) wrote: > From: "Dr. David Alan Gilbert"> > Reported-by: Dimitris Karagkasidis > Fixes: 48fe86f6400574165979e0db6f5937ad487b6888 > Signed-off-by: Dr. David Alan Gilbert Queued. > --- > monitor.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 0c0faec0a4..bec484440f 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -3696,7 +3696,7 @@ static void monitor_find_completion_by_table(Monitor > *mon, > { > const char *cmdname; > int i; > -const char *ptype, *str, *name; > +const char *ptype, *old_ptype, *str, *name; > const mon_cmd_t *cmd; > BlockBackend *blk = NULL; > > @@ -3741,7 +3741,9 @@ static void monitor_find_completion_by_table(Monitor > *mon, > } > } > str = args[nb_args - 1]; > -while (*ptype == '-' && ptype[1] != '\0') { > +old_ptype = NULL; > +while (*ptype == '-' && old_ptype != ptype) { > +old_ptype = ptype; > ptype = next_arg_type(ptype); > } > switch(*ptype) { > -- > 2.14.3 > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [qemu-s390x] [PATCH v6 06/12] s390-ccw: parse and set boot menu options
On 15.02.2018 23:54, Collin L. Walling wrote: > Set boot menu options for an s390 guest and store them in > the iplb. These options are set via the QEMU command line > option: > > -boot menu=on|off[,splash-time=X] > > or via the libvirt domain xml: > > > > > > Where X represents some positive integer representing > milliseconds. > > Any value set for loadparm will override all boot menu options. > If loadparm=PROMPT, then the menu will be enabled without a > timeout. > > The absence of any boot options on the command line will flag > to later use the zipl boot loader values. > > Signed-off-by: Collin L. Walling> Reviewed-by: Janosch Frank > Reviewed-by: Thomas Huth > --- > hw/s390x/ipl.c | 48 > hw/s390x/ipl.h | 9 +++-- > pc-bios/s390-ccw/iplb.h | 6 -- > 3 files changed, 59 insertions(+), 4 deletions(-) [] > diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h > index cab8a97..7c3cab8 100644 > --- a/hw/s390x/ipl.h > +++ b/hw/s390x/ipl.h > @@ -60,10 +60,15 @@ typedef struct IplBlockQemuScsi IplBlockQemuScsi; > > #define QIPL_ADDRESS 0xcc > > +#define BOOT_MENU_FLAG_CMD_OPTS 0x80 > +#define BOOT_MENU_FLAG_ZIPL_OPTS 0x40 > + > struct QemuIplParameters { > -uint8_t reserved1[4]; > +uint8_t boot_menu_flags; > +uint8_t reserved1; > +uint32_t boot_menu_timeout; > uint64_t netboot_start_addr; The netboot_start_addr field is now never aligned anymore, neither on the host side, nor in guest memory. Not a big problem since the struct is declared with "QEMU_PACKED", but still ... it's always nicer to try to align fields to their natural boundaries. So maybe move boot_menu_flags and reserved1 after netboot_start_addr ? > -uint8_t reserved2[16]; > +uint8_t reserved2[14]; > } QEMU_PACKED; > typedef struct QemuIplParameters QemuIplParameters; Thomas
Re: [Qemu-devel] [Nbd] [PATCH] Further tidy-up on block status
On 02/16/2018 07:53 AM, Vladimir Sementsov-Ogievskiy wrote: Good idea. But it would be tricky thing to maintain backward compatibility with published versions of virtuozzo product. And finally our implementation would be more complex because of this simplification. Hm. Finally, you suggested several changes (including already merged 56c77720 :( ). Suggestions are logical. But if they will be accepted, we (Virtuozzo) will have to invent tricky hard-to-maintain code, to distinguish by third factors our already published versions. So, I suggest to add some negotiation flag (BLOCK_STATUS_FIXED ?), to negotiate your changes (including 56c77720). In upstream we can skip supporting BLOCK_STATUS without that special flag, but it will help us to distinguish our previous versions reliably. So if I understand correctly, you already have an implementation of NBD servers in the wild that support a preliminary version of NBD_OPT_SET_META_CONTEXT, but which do not necessarily obey the semantics in the current extension branch (and which may not be the final semantics by the time we promote the extension branch into the NBD spec overall). Also, the Virtuozzo product hasn't been mentioned on the NBD list, and while you are preparing patches to the public qemu based on the Virtuozzo product, I'm not sure if there is a public repository for the existing Virtuozzo product out in the wild. Is your product using NBD_OPT_LIST_META_CONTEXT, or _just_ NBD_OPT_SET_META_CONTEXT? Now, let's assess the ramifications for changing the current NBD proposal, before finalizing it into mainline. Once it is in mainline, all other implementations have to obey the spec; but we want to leave ourselves the freedom to tweak the extension prior to that point according to things learned while implementing it. With that said, what combinations are you hoping to still support? And how long do you expect to have mismatched versions in the field, or is it something where it is relatively easy to upgrade both client and server within a short timeframe? Here's what things look like if we do nothing, and the NBD_OPT side of the handshake stays compatible (as I understand it, your complaint about commit 56c77720 is that it affects only the transmission side; leaving aside for the moment that I have opened the door for even more changes that may affect the handshake side). old client, old server (what is in the field now): The Virtuozzo client is able to request status from the Virtuozzo server, no other NBD client or server cares because they pre-date block status new client, old server: Any new client that implements block_status, as currently documented, but talks to an existing Virtuozzo server, will get garbage replies to NBD_CMD_BLOCK_STATUS (status 5 instead of expected status 3). Virtuozzo's client could be taught to special-case things and accept BOTH values for status replies (as long as nothing else changes between the old implementation and when NBD finally accepts the extension into mainline) old client, new server: The Virtuozzo client talking to a non-Virtuozzo server that understands block_status as currently documented will get garbage replies to NBD_CMD_BLOCK_STATUS (status 3 instead of expected status 5). The Virtuozzo server cannot be patched to work around this, because it has no way to detect a Virtuozzo client differently from any other client. new client, new server: Because everyone follows the same documentation, it should be interoperable. It's ALWAYS risky to put an experimental item into production use; some precautions such as using intentionally higher numbers than normal may make it easier to identify the experimental versions (that is, if the proposal is to implement item '10', the experimental code implements item '10010'; then, any changes to what actually gets finalized into the semantics for item '10' will not affect continued use of the '10010' semantics). But since you didn't do that, my immediate reaction is that adding a 'BLOCK_STATUS_FIXED' to the NBD protocol sounds crazy (fixed compared to what? An unpublished early implementation?). But what we CAN do is specify that NBD_OPT_ value 10 is reserved for a (withdrawn) experimental extension (similar to how NBD_OPT_ value 4 for PEEK_EXPORT is skipped), and make NBD_OPT_SET_META_CONTEXT be 11. Then you have: old client, old server: client sends NBD_OPT 10, Virtuozzo server replies, and both sides use whatever you implemented even if it differs from the final NBD spec new client, old server: any compliant client sends NBD_OPT 11, Virtuozzo server doesn't recognize it, and you lose the ability to query block status. Additionally, you can teach the Virtuozzo client to try 11 first, and if it fails, fall back to trying 10 - the client then has to understand both old and new flavors, but can now talk to any version of the Virtuozzo server. old client, new server: the