date:20190115

On Mon, 14 Jan 2019 13:44:12 -0500
"Jason J. Herne"  wrote:

> On 1/7/19 2:02 PM, Jason J. Herne wrote:
> >>> @@ -190,6 +247,9 @@ struct ciw {
> >>>   __u16 count;
> >>>   };
> >>> +#define CU_TYPE_VIRTIO  0x3832
> >>> +#define CU_TYPE_DASD0x3990  
> >>
> >> No other dasd types we want to support? :) (Not sure if others are out
> >> in the wild. Maybe FBA?)
> >>  
> > 
> > I have no idea. I assumed 3390 was the only thing we supported. Perhaps 
> > 3380? I'd need to 
> > find a test device, which I could probably do ... I'll look more into this. 
> >  
> 
> After a few discussions with folks in the lab we've decided that we don't see 
> a ton of 
> value in supporting anything other than 3990 at the moment. Anything else 
> would be older 
> (3380) and/or rare to see in the wild (and very difficult to test). As for 
> emulated setups 
> like z/VM, a user can just use 3390 instead of FBA. So I recommend we move 
> forward with 
> 3390/3990 support for now. We can always add in others types if/when we need 
> them.

Sounds reasonable.

What about calling the #define above CU_TYPE_DASD_3990 instead? Just to
make clear that there are other dasd types out there, but we only
support that particular one (at least at the moment).

[Qemu-devel] [Bug 1777252] Re: tests/Makefile.include trying to add linking library '-lutil' that break the build on Solaris

2019-01-15 Thread Thomas Huth

Does something like this work for you?

diff --git a/tests/Makefile.include b/tests/Makefile.include
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -777,7 +777,7 @@ tests/migration/initrd-stress.img: 
tests/migration/stress$(EXESUF)
rm $(INITRD_WORK_DIR)/init
rmdir $(INITRD_WORK_DIR)
 
-ifeq ($(CONFIG_POSIX),y)
+ifeq ($(CONFIG_POSIX)$(call lnot,$(CONFIG_SOLARIS)),yy)
 LIBS += -lutil
 endif

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1777252

Title:
  tests/Makefile.include trying to add linking library '-lutil' that
  break the build on Solaris

Status in QEMU:
  New

Bug description:
  Building script 'tests/Makefile.include' contains following code
  ```
  ifeq ($(CONFIG_POSIX),y)
  LIBS += -lutil
  endif
  ```

  library -lutil is not available on Solaris, so the building will failed, like
  ```
  ld: fatal: library -lutil: not found
  make: *** [SOMEWHERE/src/qemu-2.12.0/rules.mak:121: qemu-nbd] Error 1
  ```

  Commenting those code out fixed the error.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1777252/+subscriptions

Re: [Qemu-devel] [qemu-s390x] [PATCH v1] s390x/pci: Send correct event on hotplug.

On Mon, 14 Jan 2019 21:59:58 +0100
David Hildenbrand  wrote:

> On 14.01.19 21:00, Collin Walling wrote:
> > On 1/14/19 12:44 PM, Cornelia Huck wrote:  
> >> [restored cc:s]
> >>
> >> On Mon, 14 Jan 2019 11:06:19 +0100
> >> Pierre Morel  wrote:
> >>  
> >>> On 11/01/2019 10:38, Cornelia Huck wrote:  
>  On Fri, 11 Jan 2019 08:16:41 +0100
>  David Hildenbrand  wrote:
>  
> > On 10.01.19 22:03, David Hildenbrand wrote:
> >> Comit 2c28c490571f ("s390x/pci: let pci devices start in configured 
> >> mode")
> >> changed the initial state of zPCI devices from ZPCI_FS_STANDBY to
> >> ZPCI_FS_DISABLED (a.k.a. configured). However we still only send a
> >> HP_EVENT_RESERVED_TO_STANDBY event to the guest, indicating a wrong
> >> state.
> >>
> >> Let's send a HP_EVENT_TO_CONFIGURED event instead, to match the actual
> >> state the device is in.
> >>
> >> This fixes hotplugged devices having to be enabled explicitly in the
> >> guest e.g. via echo 1 > /sys/bus/pci/slots//power.
> >>
> >> Fixes: 2c28c490571f ("s390x/pci: let pci devices start in configured 
> >> mode")
> >> Report-by: Cornelia Huck 
> 
>  Cool, works for me as well.
> 
>  Tested-by: Cornelia Huck 
> 
>  Do we want to cc:stable? Probably not, as it's more annoying than
>  critical, and pci hotplug does not seem to be much used on s390x.
>  
> >
> > If this patch is the right thing to do, then
> >
> > 1. s/Report-by/Reported-by/
> > 2. Dropping the "." from the subject
> >
> > (yes, it was late)
> 
>  :) Can do while applying.
>  
> >
> > I wonder if we should do both events sequentially, but as I don't have
> > access to the architecture I have to rely on that this works :)
> 
>  Yep, let's wait for feedback from folks with architecture access.
>  
> >>>
> >>> Works fine on the architecture too.
> >>>
> >>> Seems the logical thing to do for me.
> >>>
> >>> Reviewed-by: Pierre Morel  
> >>
> >> Thanks for checking.
> >>
> >> I'd like to queue this, but I'd like an ack from Collin as well.
> >>  
> > 
> > Would you mind adding a comment somewhere that states something like
> > "we can safely bypass the standby state when PCI hotplugging for a guest" 
> > just to be clear that QEMU is a bit different from how we handle it on the 
> > LPAR
> > level?
> > 
> > That comment would more-or-less clarify why we set the ZPCI_FS_ 
> > directly
> > to disabled instead of to standby when hotplugging (which, AFAIU, is the 
> > order 
> > how things occur at the LPAR level)  
> 
> This patch relies on Christians patch, where the general concept was
> explained. As we changed the initial state, we have to send a
> corresponding hotplug event. But still we can add a comment to shine
> some light on the general concept.
> 
> @Conny, can you add after the first paragraph: (or let me know if you
> want a respin)
> 
> "On real HW, a PCI device always pops up in the STANDBY state. In QEMU,
> we decided to let it show up directly in the configured state (as
> configuring it is otherwise just an extra burden for the admin). We can
> safely bypass the STANDBY state when hotplugging PCI devices to a guest."

I'll just add that text.

> 
> > 
> > Otherwise,
> > 
> > Reviewed-by: Collin Walling   
> 
> Thanks!
> 

Thanks!

Re: [Qemu-devel] [PATCH v1] s390x/pci: Send correct event on hotplug.

On Thu, 10 Jan 2019 22:03:58 +0100
David Hildenbrand  wrote:

> Comit 2c28c490571f ("s390x/pci: let pci devices start in configured mode")
> changed the initial state of zPCI devices from ZPCI_FS_STANDBY to
> ZPCI_FS_DISABLED (a.k.a. configured). However we still only send a
> HP_EVENT_RESERVED_TO_STANDBY event to the guest, indicating a wrong
> state.
> 
> Let's send a HP_EVENT_TO_CONFIGURED event instead, to match the actual
> state the device is in.
> 
> This fixes hotplugged devices having to be enabled explicitly in the
> guest e.g. via echo 1 > /sys/bus/pci/slots//power.
> 
> Fixes: 2c28c490571f ("s390x/pci: let pci devices start in configured mode")
> Report-by: Cornelia Huck 
> Signed-off-by: David Hildenbrand 
> ---
>  hw/s390x/s390-pci-bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied.

[Qemu-devel] [Bug 1811782] [NEW] QEMU Windows fails to mount rootfs on an ISO where QEMU Linux works normally

2019-01-15 Thread Chris Ward

Public bug reported:

I have installed QEMU 3.1.0 for Microsoft Windows from
https://qemu.weilnetz.de/w64/ . When I give the command "qemu-system-x86_64.exe
-cdrom ..\QemuSaver\freeduc2.iso", qemu boots the ISO, but the resulting Linux
kernel panics when trying to mount the root file system. Running the equivalent
command under Linux (OpenSuSE Leap 15.0) results in success.
I will attach a screenshot of the command and the kernel panic message.
To reproduce the problem, download the zip file from Google Drive here
https://drive.google.com/file/d/1bAozvGeRF7PbkOnJKzrFHxhUh2kDLz6L/view?usp=sharing,
and unpack it under Microsoft Windows. You can either run the installer (which
will install QEMU 3.0.0 and an ISO image in C:\QemuSaver) or you can run the
command I gave from the directory where your QEMU is installed.

** Affects: qemu
Importance: Undecided
Status: New

** Tags: kernel-panic qemu rootfs windows

** Attachment added: "Screenshot of command to invoke QEMU and resulting error"

https://bugs.launchpad.net/bugs/1811782/+attachment/5229205/+files/qemu-fail-to-mount-root.png

--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1811782

Title:
QEMU Windows fails to mount rootfs on an ISO where QEMU Linux works
normally

Status in QEMU:
New

Bug description:
I have installed QEMU 3.1.0 for Microsoft Windows from
https://qemu.weilnetz.de/w64/ . When I give the command "qemu-system-x86_64.exe
-cdrom ..\QemuSaver\freeduc2.iso", qemu boots the ISO, but the resulting Linux
kernel panics when trying to mount the root file system. Running the equivalent
command under Linux (OpenSuSE Leap 15.0) results in success.
I will attach a screenshot of the command and the kernel panic message.
To reproduce the problem, download the zip file from Google Drive here
https://drive.google.com/file/d/1bAozvGeRF7PbkOnJKzrFHxhUh2kDLz6L/view?usp=sharing,
and unpack it under Microsoft Windows. You can either run the installer (which
will install QEMU 3.0.0 and an ISO image in C:\QemuSaver) or you can run the
command I gave from the directory where your QEMU is installed.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1811782/+subscriptions

Re: [Qemu-devel] [PATCH v3 04/19] nbd/server: Hoist length check to qemp_nbd_server_add

12.01.2019 20:57, Eric Blake wrote:
> We only had two callers to nbd_export_new; qemu-nbd.c always
> passed a valid offset/length pair (because it already checked
> the file length, to ensure that offset was in bounds), while
> blockdev-nbd always passed 0/-1.  Then nbd_export_new reduces
> the size to a multiple of BDRV_SECTOR_SIZE (can only happen
> when offset is not sector-aligned, since bdrv_getlength()
> currently rounds up), which can result in offset being greater
> than the enforced length, but that's not fatal (the server
> rejects client requests that exceed the advertised length).
> 
> However, I'm finding it easier to work with the code if we are
> consistent on having both callers pass in a valid length, and
> just assert that things are sane in nbd_export_new.
> 
> Signed-off-by: Eric Blake 
> 
> ---
> v3: new patch
> ---
>   blockdev-nbd.c | 10 +-
>   nbd/server.c   |  9 ++---
>   2 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index c76d5416b90..d73ac1b026a 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -146,6 +146,7 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>   BlockDriverState *bs = NULL;
>   BlockBackend *on_eject_blk;
>   NBDExport *exp;
> +int64_t len;
> 
>   if (!nbd_server) {
>   error_setg(errp, "NBD server not running");
> @@ -168,6 +169,13 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>   return;
>   }
> 
> +len = bdrv_getlength(bs);
> +if (len < 0) {
> +error_setg_errno(errp, -len,
> + "Failed to determine the NBD export's length");
> +return;
> +}
> +
>   if (!has_writable) {
>   writable = false;
>   }
> @@ -175,7 +183,7 @@ void qmp_nbd_server_add(const char *device, bool 
> has_name, const char *name,
>   writable = false;
>   }
> 
> -exp = nbd_export_new(bs, 0, -1, name, NULL, bitmap,
> +exp = nbd_export_new(bs, 0, len, name, NULL, bitmap,
>writable ? 0 : NBD_FLAG_READ_ONLY,
>NULL, false, on_eject_blk, errp);
>   if (!exp) {
> diff --git a/nbd/server.c b/nbd/server.c
> index e8c56607eff..c9937ccdc2a 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -1499,13 +1499,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
> dev_offset, off_t size,
>   exp->name = g_strdup(name);
>   exp->description = g_strdup(description);
>   exp->nbdflags = nbdflags;
> -exp->size = size < 0 ? blk_getlength(blk) : size;
> -if (exp->size < 0) {
> -error_setg_errno(errp, -exp->size,
> - "Failed to determine the NBD export's length");
> -goto fail;
> -}
> -exp->size -= exp->size % BDRV_SECTOR_SIZE;
> +assert(dev_offset <= size);

@size is not size of the image, but size of the export, so it may be less than 
dev_offset
(qemu-nbd.c do "fd_size -= dev_offset" before "nbd_export_new(bs, dev_offset, 
fd_size, "

> +exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
> 
>   if (bitmap) {
>   BdrvDirtyBitmap *bm = NULL;
> 


-- 
Best regards,
Vladimir

[Qemu-devel] [Bug 1811782] Re: QEMU Windows fails to mount rootfs on an ISO where QEMU Linux works normally

2019-01-15 Thread Chris Ward

This fails on Windows 7 and on Windows 10.
I have had success with different ISO files.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1811782

Title:
  QEMU Windows fails to mount rootfs on an ISO where QEMU Linux works
  normally

Status in QEMU:
  New

Bug description:
  I have installed QEMU 3.1.0 for Microsoft Windows from 
https://qemu.weilnetz.de/w64/ . When I give the command "qemu-system-x86_64.exe 
-cdrom ..\QemuSaver\freeduc2.iso", qemu boots the ISO, but the resulting Linux 
kernel panics when trying to mount the root file system. Running the equivalent 
command under Linux (OpenSuSE Leap 15.0) results in success.
  I will attach a screenshot of the command and the kernel panic message.
  To reproduce the problem, download the zip file from Google Drive here 
https://drive.google.com/file/d/1bAozvGeRF7PbkOnJKzrFHxhUh2kDLz6L/view?usp=sharing,
 and unpack it under Microsoft Windows. You can either run the installer (which 
will install QEMU 3.0.0 and an ISO image in C:\QemuSaver) or you can run the 
command I gave from the directory where your QEMU is installed.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1811782/+subscriptions

Re: [Qemu-devel] [RFC v2 0/4] QEMU changes to do PVH boot


Hi Stefano,

On 10/01/2019 15:12, Stefano Garzarella wrote:

On Wed, Jan 09, 2019 at 01:18:12PM -0800, Maran Wilson wrote:

On 1/9/2019 11:53 AM, Boris Ostrovsky wrote:

On 1/9/19 6:53 AM, Stefano Garzarella wrote:

Hi Liam,

On Tue, Jan 8, 2019 at 3:47 PM Liam Merwick  wrote:

QEMU sets the hvm_modlist_entry in load_linux() after the call to
load_elfboot() and then qboot loads it in boot_pvh_from_fw_cfg()

But the current PVH patches don't handle initrd (they have
start_info.nr_modules == 1).

Looking in the linux kernel (arch/x86/platform/pvh/enlighten.c) I saw:
  /* The first module is always ramdisk. */
  if (pvh_start_info.nr_modules) {
  struct hvm_modlist_entry *modaddr =
  __va(pvh_start_info.modlist_paddr);
  pvh_bootparams.hdr.ramdisk_image = modaddr->paddr;
  pvh_bootparams.hdr.ramdisk_size = modaddr->size;
  }

So, putting start_info.nr_modules = 1, means that the first
hvm_modlist_entry should have the ramdisk paddr and size. Is it
correct?


That's my understanding.

I think what's missing, is that we just need Qemu or qboot/seabios to
properly populate the pvh_start_info.modlist_paddr with the address (as
usable by the guest) of the hvm_modlist_entry which correctly defines the
details of the initrd that has already been loaded into memory that is
accessible by the guest.

-Maran



I tried and it works, I modified QEMU to load the initrd and to expose it
through fw_cfg, then qboot loads it and set correctly the hvm_modlist_entry.

You can find the patch of QEMU at the end of this email and the qboot
patch here: 
https://github.com/stefano-garzarella/qboot/commit/41e1fd765c8419e270fd79d9b3af5d53576e88a8

Do you think can be a good approach? If you want, you can add this patch
to your series.


Code looks good to me. I'll include it with v3 of my QEMU patches.

Regards,
Liam




Thanks,
Stefano


 From d5c0d51768f5a8fb214be6c2bb0cb7e86e9917b7 Mon Sep 17 00:00:00 2001
From: Stefano Garzarella 
Date: Thu, 10 Jan 2019 15:16:44 +0100
Subject: [PATCH] pvh: load initrd and expose it through fw_cfg

When initrd is specified, load and expose it to the guest firmware
through fw_cfg. The firmware will fill the hvm_start_info for the
kernel.

Signed-off-by: Stefano Garzarella 
Based-on: <1545422632-2-5-git-send-email-liam.merw...@oracle.com>
---
  hw/i386/pc.c | 38 +-
  1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 06bce6a101..f6721f51be 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -986,25 +986,45 @@ static void load_linux(PCMachineState *pcms,
   */
  if (load_elfboot(kernel_filename, kernel_size,
   header, pvh_start_addr, fw_cfg)) {
-struct hvm_modlist_entry ramdisk_mod = { 0 };
-
  fclose(f);
  
  fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,

  strlen(kernel_cmdline) + 1);
  fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
  
-assert(machine->device_memory != NULL);

-ramdisk_mod.paddr = machine->device_memory->base;
-ramdisk_mod.size =
-memory_region_size(&machine->device_memory->mr);
-
-fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, &ramdisk_mod,
- sizeof(ramdisk_mod));
  fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, sizeof(header));
  fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA,
   header, sizeof(header));
  
+/* load initrd */

+if (initrd_filename) {
+gsize initrd_size;
+gchar *initrd_data;
+GError *gerr = NULL;
+
+if (!g_file_get_contents(initrd_filename, &initrd_data,
+&initrd_size, &gerr)) {
+fprintf(stderr, "qemu: error reading initrd %s: %s\n",
+initrd_filename, gerr->message);
+exit(1);
+}
+
+initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 
1;
+if (initrd_size >= initrd_max) {
+fprintf(stderr, "qemu: initrd is too large, cannot 
support."
+"(max: %"PRIu32", need %"PRId64")\n",
+initrd_max, (uint64_t)initrd_size);
+exit(1);
+}
+
+initrd_addr = (initrd_max - initrd_size) & ~4095;
+
+fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
+fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
+fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data,
+ initrd_size);
+}
+
  return;
  }
  /* This looks like a multiboot kernel. If it is, let's stop

[Qemu-devel] [PATCH v2 0/4] pvh: add new PVH option rom

This patch series is based on "[RFC v2 0/4] QEMU changes to do PVH boot" and
provides a PVH option rom that can be used with SeaBIOS to boot uncompressed
kernel using the x86/HVM direct boot ABI.

Patches 1 and 2 are to prepare the PVH option rom, moving common functions in a
new header.  Patch 3 adds the new PVH option rom and patch 4 uses it when we
are booting an uncompressed kernel using the x86/HVM direct boot ABI.

Changes in v2:
- addressed comments by Stefan and Eric:
  - Patch 2: moved inludes on top of linuxboot_dma.c and add  in
optrom.h
  - Patch 4: added check of pvh.bin in xen_load_linux()
- modified commit message of patch 2 to explain better the patch

Stefano Garzarella (4):
  linuxboot_dma: remove duplicate definitions of FW_CFG
  linuxboot_dma: move common functions in a new header
  optionrom: add new PVH option rom
  hw/i386/pc: use PVH option rom

 hw/i386/pc.c  |   5 +
 pc-bios/optionrom/Makefile|   5 +-
 pc-bios/optionrom/linuxboot_dma.c | 112 +++--
 pc-bios/optionrom/optrom.h| 110 
 pc-bios/optionrom/optrom_fw_cfg.h |  92 ++
 pc-bios/optionrom/pvh.S   | 200 ++
 pc-bios/optionrom/pvh_main.c  | 117 +
 7 files changed, 544 insertions(+), 97 deletions(-)
 create mode 100644 pc-bios/optionrom/optrom.h
 create mode 100644 pc-bios/optionrom/optrom_fw_cfg.h
 create mode 100644 pc-bios/optionrom/pvh.S
 create mode 100644 pc-bios/optionrom/pvh_main.c

-- 
2.20.1

[Qemu-devel] [PATCH v2 3/4] optionrom: add new PVH option rom

The new pvh.bin option rom can be used with SeaBIOS to boot
uncompressed kernel using the x86/HVM direct boot ABI.

pvh.S contains the entry point of the option rom. It runs
in real mode, loads the e820 table querying the BIOS, and
then it switches to 32bit protect mode and jump to the
pvh_load_kernel() written in pvh_main.c.
pvh_load_kernel() loads the cmdline and kernel entry_point
using fw_cfg, then it looks for RSDP, fills the
hvm_start_info required by x86/HVM ABI, and finally jumps
to the kernel entry_point.

Signed-off-by: Stefano Garzarella 
Reviewed-by: Stefan Hajnoczi 
---
 pc-bios/optionrom/Makefile   |   5 +-
 pc-bios/optionrom/pvh.S  | 200 +++
 pc-bios/optionrom/pvh_main.c | 117 
 3 files changed, 321 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/optionrom/pvh.S
 create mode 100644 pc-bios/optionrom/pvh_main.c

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index a9a9e5e7eb..92c91d9949 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -37,7 +37,7 @@ Wa = -Wa,
 ASFLAGS += -32
 QEMU_CFLAGS += $(call cc-c-option, $(QEMU_CFLAGS), $(Wa)-32)
 
-build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin
+build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin kvmvapic.bin pvh.bin
 
 # suppress auto-removal of intermediate files
 .SECONDARY:
@@ -46,6 +46,9 @@ build-all: multiboot.bin linuxboot.bin linuxboot_dma.bin 
kvmvapic.bin
 %.o: %.S
$(call quiet-command,$(CPP) $(QEMU_INCLUDES) $(QEMU_DGFLAGS) -c -o - $< 
| $(AS) $(ASFLAGS) -o $@,"AS","$(TARGET_DIR)$@")
 
+%.img: %.o %_main.o
+   $(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_I386_EMULATION) -T 
$(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $?,"BUILD","$(TARGET_DIR)$@")
+
 %.img: %.o
$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_I386_EMULATION) -T 
$(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $<,"BUILD","$(TARGET_DIR)$@")
 
diff --git a/pc-bios/optionrom/pvh.S b/pc-bios/optionrom/pvh.S
new file mode 100644
index 00..e1d7f4a7a7
--- /dev/null
+++ b/pc-bios/optionrom/pvh.S
@@ -0,0 +1,200 @@
+/*
+ * PVH Option ROM
+ *
+ * This program 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ *
+ * Copyright Novell Inc, 2009
+ *   Authors: Alexander Graf 
+ *
+ * Copyright (c) 2019 Red Hat Inc.
+ *   Authors: Stefano Garzarella 
+ */
+
+#include "optionrom.h"
+
+#define BOOT_ROM_PRODUCT "PVH loader"
+
+#define GS_PROT_JUMP   0
+#define GS_GDT_DESC6
+
+#ifdef OPTION_ROM_START
+#undef OPTION_ROM_START
+#endif
+#ifdef OPTION_ROM_END
+#undef OPTION_ROM_END
+#endif
+
+/*
+ * Redefine OPTION_ROM_START and OPTION_ROM_END, because this rom is produced
+ * linking multiple objects.
+ * signrom.py will add padding.
+ */
+#define OPTION_ROM_START\
+.code16;   \
+.text; \
+   .global _start; \
+_start:;   \
+   .short  0xaa55; \
+   .byte   3; /* desired size in 512 units */
+
+#define OPTION_ROM_END \
+_end:
+
+BOOT_ROM_START
+
+run_pvhboot:
+
+   cli
+   cld
+
+   mov %cs, %eax
+   shl $0x4, %eax
+
+   /* set up a long jump descriptor that is PC relative */
+
+   /* move stack memory to %gs */
+   mov %ss, %ecx
+   shl $0x4, %ecx
+   mov %esp, %ebx
+   add %ebx, %ecx
+   sub $0x20, %ecx
+   sub $0x30, %esp
+   shr $0x4, %ecx
+   mov %cx, %gs
+
+   /* now push the indirect jump descriptor there */
+   mov (prot_jump), %ebx
+   add %eax, %ebx
+   movl%ebx, %gs:GS_PROT_JUMP
+   mov $8, %bx
+   movw%bx, %gs:GS_PROT_JUMP + 4
+
+   /* fix the gdt descriptor to be PC relative */
+   movw(gdt_desc), %bx
+   movw%bx, %gs:GS_GDT_DESC
+   movl(gdt_desc+2), %ebx
+   add %eax, %ebx
+   movl%ebx, %gs:GS_GDT_DESC + 2
+
+   /* initialize HVM memmap table using int 0x15(e820) */
+
+   /* ES

[Qemu-devel] [PATCH v1] virtio: add checks for the size of the indirect table

2019-01-15 Thread Dima Stepanov

The virtqueue_pop() and virtqueue_get_avail_bytes() routines can use the
INDIRECT table to get the data. It is possible to create a packet which
will lead to the assert message like:
  include/exec/memory.h:1995: void
  address_space_read_cached(MemoryRegionCache *, hwaddr, void *, int):
  Assertion `addr < cache->len && len <= cache->len - addr' failed.
  Aborted
To do it the first descriptor should have a link to the INDIRECT table
and set the size of it to 0. It doesn't look good that the guest should
be able to trigger the assert in qemu. Add additional check for the size
of the INDIRECT table, which should not be 0.

Signed-off-by: Dima Stepanov 
---
 hw/virtio/virtio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 22bd1ac..a1ff647 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -646,7 +646,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 vring_desc_read(vdev, &desc, desc_cache, i);
 
 if (desc.flags & VRING_DESC_F_INDIRECT) {
-if (desc.len % sizeof(VRingDesc)) {
+if (!desc.len || (desc.len % sizeof(VRingDesc))) {
 virtio_error(vdev, "Invalid size for indirect buffer table");
 goto err;
 }
@@ -902,7 +902,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 desc_cache = &caches->desc;
 vring_desc_read(vdev, &desc, desc_cache, i);
 if (desc.flags & VRING_DESC_F_INDIRECT) {
-if (desc.len % sizeof(VRingDesc)) {
+if (!desc.len || (desc.len % sizeof(VRingDesc))) {
 virtio_error(vdev, "Invalid size for indirect buffer table");
 goto done;
 }
-- 
2.7.4

[Qemu-devel] [PATCH v2 2/4] linuxboot_dma: move common functions in a new header

In order to allow other option roms to use these common
useful functions and definitions, this patch put them
in two new C header files called optrom.h and
optrom_fw_cfg.h. We also add useful out*() in*()
functions for different size, and new fw_cfg functions
to use when DMA feature is not available.

Signed-off-by: Stefano Garzarella 
---
 pc-bios/optionrom/linuxboot_dma.c | 100 +--
 pc-bios/optionrom/optrom.h| 110 ++
 pc-bios/optionrom/optrom_fw_cfg.h |  92 +
 3 files changed, 218 insertions(+), 84 deletions(-)
 create mode 100644 pc-bios/optionrom/optrom.h
 create mode 100644 pc-bios/optionrom/optrom_fw_cfg.h

diff --git a/pc-bios/optionrom/linuxboot_dma.c 
b/pc-bios/optionrom/linuxboot_dma.c
index f728dc839f..bf6ee79c38 100644
--- a/pc-bios/optionrom/linuxboot_dma.c
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -20,6 +20,10 @@
  * Richard W.M. Jones 
  */
 
+#include 
+#include "optrom.h"
+#include "optrom_fw_cfg.h"
+
 asm(
 ".text\n"
 ".global _start\n"
@@ -58,34 +62,12 @@ asm(
 "   jmp load_kernel\n"
 );
 
-#define BIOS_CFG_DMA_ADDR_HIGH 0x514
-#define BIOS_CFG_DMA_ADDR_LOW  0x518
-
-#define uint64_t unsigned long long
-#define uint32_t unsigned int
-#define uint16_t unsigned short
-
-#include "../../include/standard-headers/linux/qemu_fw_cfg.h"
-
-#define barrier() asm("" : : : "memory")
-
-static inline void outl(uint32_t value, uint16_t port)
-{
-asm("outl %0, %w1" : : "a"(value), "Nd"(port));
-}
-
 static inline void set_es(void *addr)
 {
 uint32_t seg = (uint32_t)addr >> 4;
 asm("movl %0, %%es" : : "r"(seg));
 }
 
-#ifdef __clang__
-#define ADDR32
-#else
-#define ADDR32 "addr32 "
-#endif
-
 static inline uint16_t readw_es(uint16_t offset)
 {
 uint16_t val;
@@ -108,56 +90,6 @@ static inline void writel_es(uint16_t offset, uint32_t val)
 asm(ADDR32 "movl %0, %%es:(%1)" : : "r"(val), "r"((uint32_t)offset));
 }
 
-static inline uint32_t bswap32(uint32_t x)
-{
-asm("bswapl %0" : "=r" (x) : "0" (x));
-return x;
-}
-
-static inline uint64_t bswap64(uint64_t x)
-{
-asm("bswapl %%eax; bswapl %%edx; xchg %%eax, %%edx" : "=A" (x) : "0" (x));
-return x;
-}
-
-static inline uint64_t cpu_to_be64(uint64_t x)
-{
-return bswap64(x);
-}
-
-static inline uint32_t cpu_to_be32(uint32_t x)
-{
-return bswap32(x);
-}
-
-static inline uint32_t be32_to_cpu(uint32_t x)
-{
-return bswap32(x);
-}
-
-/* clang is happy to inline this function, and bloats the
- * ROM.
- */
-static __attribute__((__noinline__))
-void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
-{
-struct fw_cfg_dma_access access;
-uint32_t control = (entry << 16) | FW_CFG_DMA_CTL_SELECT
-| FW_CFG_DMA_CTL_READ;
-
-access.address = cpu_to_be64((uint64_t)(uint32_t)buf);
-access.length = cpu_to_be32(len);
-access.control = cpu_to_be32(control);
-
-barrier();
-
-outl(cpu_to_be32((uint32_t)&access), BIOS_CFG_DMA_ADDR_LOW);
-
-while (be32_to_cpu(access.control) & ~FW_CFG_DMA_CTL_ERROR) {
-barrier();
-}
-}
-
 /* Return top of memory using BIOS function E801. */
 static uint32_t get_e801_addr(void)
 {
@@ -211,9 +143,9 @@ void load_kernel(void)
 uint32_t initrd_end_page, max_allowed_page;
 uint32_t segment_addr, stack_addr;
 
-bios_cfg_read_entry(&setup_addr, FW_CFG_SETUP_ADDR, 4);
-bios_cfg_read_entry(&setup_size, FW_CFG_SETUP_SIZE, 4);
-bios_cfg_read_entry(setup_addr, FW_CFG_SETUP_DATA, setup_size);
+bios_cfg_read_entry_dma(&setup_addr, FW_CFG_SETUP_ADDR, 4);
+bios_cfg_read_entry_dma(&setup_size, FW_CFG_SETUP_SIZE, 4);
+bios_cfg_read_entry_dma(setup_addr, FW_CFG_SETUP_DATA, setup_size);
 
 set_es(setup_addr);
 
@@ -223,8 +155,8 @@ void load_kernel(void)
 writel_es(0x22c, 0x37ff);
 }
 
-bios_cfg_read_entry(&initrd_addr, FW_CFG_INITRD_ADDR, 4);
-bios_cfg_read_entry(&initrd_size, FW_CFG_INITRD_SIZE, 4);
+bios_cfg_read_entry_dma(&initrd_addr, FW_CFG_INITRD_ADDR, 4);
+bios_cfg_read_entry_dma(&initrd_size, FW_CFG_INITRD_SIZE, 4);
 
 initrd_end_page = ((uint32_t)(initrd_addr + initrd_size) & -4096);
 max_allowed_page = (readl_es(0x22c) & -4096);
@@ -239,15 +171,15 @@ void load_kernel(void)
 
 }
 
-bios_cfg_read_entry(initrd_addr, FW_CFG_INITRD_DATA, initrd_size);
+bios_cfg_read_entry_dma(initrd_addr, FW_CFG_INITRD_DATA, initrd_size);
 
-bios_cfg_read_entry(&kernel_addr, FW_CFG_KERNEL_ADDR, 4);
-bios_cfg_read_entry(&kernel_size, FW_CFG_KERNEL_SIZE, 4);
-bios_cfg_read_entry(kernel_addr, FW_CFG_KERNEL_DATA, kernel_size);
+bios_cfg_read_entry_dma(&kernel_addr, FW_CFG_KERNEL_ADDR, 4);
+bios_cfg_read_entry_dma(&kernel_size, FW_CFG_KERNEL_SIZE, 4);
+bios_cfg_read_entry_dma(kernel_addr, FW_CFG_KERNEL_DATA, kernel_size);
 
-bios_cfg_read_entry(&cmdline_addr, FW_CFG_CMDLINE_ADDR, 4);
-bios_cfg_read_entry(&cmdline_size, FW_CFG_CMDLINE_SIZE, 4);
-bios_

[Qemu-devel] [PATCH v2 4/4] hw/i386/pc: use PVH option rom

Use pvh.bin option rom when we are booting an uncompressed
kernel using the x86/HVM direct boot ABI.

Signed-off-by: Stefano Garzarella 
Based-on: <1545422632-2-5-git-send-email-liam.merw...@oracle.com>
---
 hw/i386/pc.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 06bce6a101..7564ba51d2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1005,6 +1005,10 @@ static void load_linux(PCMachineState *pcms,
 fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA,
  header, sizeof(header));
 
+option_rom[nb_option_roms].bootindex = 0;
+option_rom[nb_option_roms].name = "pvh.bin";
+nb_option_roms++;
+
 return;
 }
 /* This looks like a multiboot kernel. If it is, let's stop
@@ -1456,6 +1460,7 @@ void xen_load_linux(PCMachineState *pcms)
 for (i = 0; i < nb_option_roms; i++) {
 assert(!strcmp(option_rom[i].name, "linuxboot.bin") ||
!strcmp(option_rom[i].name, "linuxboot_dma.bin") ||
+   !strcmp(option_rom[i].name, "pvh.bin") ||
!strcmp(option_rom[i].name, "multiboot.bin"));
 rom_add_option(option_rom[i].name, option_rom[i].bootindex);
 }
-- 
2.20.1

[Qemu-devel] [PATCH v2 1/4] linuxboot_dma: remove duplicate definitions of FW_CFG

FW_CFG_DMA_CTL_* bits and struct fw_cfg_dma_access are
defined in the qemu_fw_cfg.h header file already included
in linuxboot_dma.c, so we can remove the definition of
BIOS_CFG_DMA_CTL_* and struct FWCfgDmaAccess.

Signed-off-by: Stefano Garzarella 
Reviewed-by: Stefan Hajnoczi 
---
 pc-bios/optionrom/linuxboot_dma.c | 20 
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/pc-bios/optionrom/linuxboot_dma.c 
b/pc-bios/optionrom/linuxboot_dma.c
index d856d41b55..f728dc839f 100644
--- a/pc-bios/optionrom/linuxboot_dma.c
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -58,12 +58,6 @@ asm(
 "   jmp load_kernel\n"
 );
 
-/* QEMU_CFG_DMA_CONTROL bits */
-#define BIOS_CFG_DMA_CTL_ERROR   0x01
-#define BIOS_CFG_DMA_CTL_READ0x02
-#define BIOS_CFG_DMA_CTL_SKIP0x04
-#define BIOS_CFG_DMA_CTL_SELECT  0x08
-
 #define BIOS_CFG_DMA_ADDR_HIGH 0x514
 #define BIOS_CFG_DMA_ADDR_LOW  0x518
 
@@ -75,12 +69,6 @@ asm(
 
 #define barrier() asm("" : : : "memory")
 
-typedef struct FWCfgDmaAccess {
-uint32_t control;
-uint32_t length;
-uint64_t address;
-} __attribute__((packed)) FWCfgDmaAccess;
-
 static inline void outl(uint32_t value, uint16_t port)
 {
 asm("outl %0, %w1" : : "a"(value), "Nd"(port));
@@ -153,9 +141,9 @@ static inline uint32_t be32_to_cpu(uint32_t x)
 static __attribute__((__noinline__))
 void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
 {
-FWCfgDmaAccess access;
-uint32_t control = (entry << 16) | BIOS_CFG_DMA_CTL_SELECT
-| BIOS_CFG_DMA_CTL_READ;
+struct fw_cfg_dma_access access;
+uint32_t control = (entry << 16) | FW_CFG_DMA_CTL_SELECT
+| FW_CFG_DMA_CTL_READ;
 
 access.address = cpu_to_be64((uint64_t)(uint32_t)buf);
 access.length = cpu_to_be32(len);
@@ -165,7 +153,7 @@ void bios_cfg_read_entry(void *buf, uint16_t entry, 
uint32_t len)
 
 outl(cpu_to_be32((uint32_t)&access), BIOS_CFG_DMA_ADDR_LOW);
 
-while (be32_to_cpu(access.control) & ~BIOS_CFG_DMA_CTL_ERROR) {
+while (be32_to_cpu(access.control) & ~FW_CFG_DMA_CTL_ERROR) {
 barrier();
 }
 }
-- 
2.20.1

Re: [Qemu-devel] [PATCH] include/fpu/softfloat: Fix compilation with Clang on s390x

On Mon, 14 Jan 2019 at 22:48, Alex Bennée  wrote:
>
>
> Richard Henderson  writes:
> > But perhaps
> >
> > unsigned __int128 n = (unsigned __int128)n1 << 64 | n0;
> > *r = n % d;
> > return n / d;
> >
> > will allow the compiler to do what the assembly does for some 64-bit
> > hosts.
>
> I wonder how much cost is incurred by the jumping to the (libgcc?) div
> helper? Anyone got an s390x about so we can benchmark the two
> approaches?

The project has an s390x system available; however it's usually
running merge build tests so not so useful for benchmarking.
(I can set up accounts on it but that requires me to faff about
figuring out how to create new accounts :-))

thanks
-- PMM

[Qemu-devel] [Bug 1811720] Re: storage physical_block_size is restricted to uint16_t

2019-01-15 Thread Коренберг Марк

Yes, you are right. Thanks for the response.

** Changed in: qemu
   Status: New => Invalid

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1811720

Title:
  storage physical_block_size is restricted to uint16_t

Status in QEMU:
  Invalid

Bug description:
  It is desirable to set -global scsi-hd.physical_block_size=4194304 for
  RBD-backed storage.

  But unfortunatelly, this values is restricted with uint16_t, i.e.
  65536 maximum.

  For example, scsi-hd.discard_granularity=4194304 is not so restricted
  (and works as expected)

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1811720/+subscriptions

Re: [Qemu-devel] [PATCH v2 2/3] migration: fix memory leak when updating tls-creds and tls-hostname

2019-01-15 Thread Dr. David Alan Gilbert

* Peter Xu (pet...@redhat.com) wrote:
> On Fri, Jan 11, 2019 at 02:37:31PM +0800, guangrong.x...@gmail.com wrote:
> > From: Xiao Guangrong 
> > 
> > If we update parameter, tls-creds and tls-hostname, these string
> > values are duplicated to local variables in migrate_params_test_apply()
> > by using g_strdup(), however these new allocated memory are missed to
> > be freed
> > 
> > Actually, they are not used to check anything, we can directly skip
> > them
> > 
> > Signed-off-by: Xiao Guangrong 
> > ---
> >  migration/migration.c | 10 --
> >  1 file changed, 10 deletions(-)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index a82d594f29..fb39d7bec1 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -1145,16 +1145,6 @@ static void 
> > migrate_params_test_apply(MigrateSetParameters *params,
> >  dest->cpu_throttle_increment = params->cpu_throttle_increment;
> >  }
> >  
> > -if (params->has_tls_creds) {
> > -assert(params->tls_creds->type == QTYPE_QSTRING);
> > -dest->tls_creds = g_strdup(params->tls_creds->u.s);
> > -}
> > -
> > -if (params->has_tls_hostname) {
> > -assert(params->tls_hostname->type == QTYPE_QSTRING);
> > -dest->tls_hostname = g_strdup(params->tls_hostname->u.s);
> > -}
> > -
> 
> Hi, Guangrong,
> 
> The memleak seems to be correct here but before that I'm even a bit
> confused on why we need to copy the whole parameter list here instead
> of checking against a MigrateSetParameters* in migrate_params_check().
> Could anyone shed some light?  CC Markus too.

I think the problem is that
migrate_params_check checks a MigrationParameters

while the QMP command gives us a MigrateSetParameters; but we also use
migrate_params_check for the global check you added (8b0b29dc) which is
against migrationParameters; so that's why migrate_params_check takes
a MigrationParameters.

It's horrible we've got stuff duped so much.

However, I don't like this fix because if someone later was to add
a test for tls parameters to migrate_params_check, then they would be
confused why the hostname/creds weren't checked.
So while we have migrate_params_test_apply, it should cover all
parameters.

I think a cleaner check would be to write a MigrateParameters_free
that free'd any strings, and call that in qmp_migrate_set_parameters
on both exit paths.

Dave

> Thanks,
> 
> >  if (params->has_max_bandwidth) {
> >  dest->max_bandwidth = params->max_bandwidth;
> >  }
> > -- 
> > 2.14.5
> > 
> 
> Regards,
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v3 05/19] nbd/server: Favor [u]int64_t over off_t

12.01.2019 20:57, Eric Blake wrote:
> Although our compile-time environment is set up so that we always
> support long files with 64-bit off_t, we have no guarantee whether
> off_t is the same type as int64_t.  This requires casts when
> printing values, and prevents us from directly using qemu_strtoi64().
> Let's just flip to [u]int64_t (signed for length, because we have to
> detect failure of blk_getlength()

we have not, after previous patch

and because off_t was signed;
> unsigned for offset because it lets us simplify some math without
> having to worry about signed overflow).
> 
> Suggested-by: Vladimir Sementsov-Ogievskiy 
> Signed-off-by: Eric Blake 
> 
> ---
> v3: new patch
> ---
>   include/block/nbd.h |  4 ++--
>   nbd/server.c| 14 +++---
>   qemu-nbd.c  | 26 ++
>   3 files changed, 19 insertions(+), 25 deletions(-)
> 
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 1971b557896..0f252829376 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -294,8 +294,8 @@ int nbd_errno_to_system_errno(int err);
>   typedef struct NBDExport NBDExport;
>   typedef struct NBDClient NBDClient;
> 
> -NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
> -  const char *name, const char *description,
> +NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
> +  int64_t size, const char *name, const char *desc,

in previous patch you drop use of negative @size parameter, so it looks better
to use unsigned for size like for offset

> const char *bitmap, uint16_t nbdflags,
> void (*close)(NBDExport *), bool writethrough,
> BlockBackend *on_eject_blk, Error **errp);
> diff --git a/nbd/server.c b/nbd/server.c
> index c9937ccdc2a..15357d40fd7 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -77,8 +77,8 @@ struct NBDExport {
>   BlockBackend *blk;
>   char *name;
>   char *description;
> -off_t dev_offset;
> -off_t size;
> +uint64_t dev_offset;
> +int64_t size;
>   uint16_t nbdflags;
>   QTAILQ_HEAD(, NBDClient) clients;
>   QTAILQ_ENTRY(NBDExport) next;
> @@ -1455,8 +1455,8 @@ static void nbd_eject_notifier(Notifier *n, void *data)
>   nbd_export_close(exp);
>   }
> 
> -NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size,
> -  const char *name, const char *description,
> +NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
> +  int64_t size, const char *name, const char *desc,
> const char *bitmap, uint16_t nbdflags,
> void (*close)(NBDExport *), bool writethrough,
> BlockBackend *on_eject_blk, Error **errp)
> @@ -1497,7 +1497,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t 
> dev_offset, off_t size,
>   exp->blk = blk;
>   exp->dev_offset = dev_offset;
>   exp->name = g_strdup(name);
> -exp->description = g_strdup(description);
> +exp->description = g_strdup(desc);

unrelated but at least obvious, OK. However tiny note in commit message won't 
hurt.

>   exp->nbdflags = nbdflags;
>   assert(dev_offset <= size);
>   exp->size = QEMU_ALIGN_DOWN(size, BDRV_SECTOR_SIZE);
> @@ -2131,8 +2131,8 @@ static int nbd_co_receive_request(NBDRequestData *req, 
> NBDRequest *request,
>   if (request->from > client->exp->size ||
>   request->from + request->len > client->exp->size) {
>   error_setg(errp, "operation past EOF; From: %" PRIu64 ", Len: %" 
> PRIu32
> -   ", Size: %" PRIu64, request->from, request->len,
> -   (uint64_t)client->exp->size);
> +   ", Size: %" PRId64, request->from, request->len,
> +   client->exp->size);
>   return (request->type == NBD_CMD_WRITE ||
>   request->type == NBD_CMD_WRITE_ZEROES) ? -ENOSPC : -EINVAL;
>   }
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index ff4adb9b3eb..96c0829970c 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -176,7 +176,7 @@ static void read_partition(uint8_t *p, struct 
> partition_record *r)
>   }
> 
>   static int find_partition(BlockBackend *blk, int partition,
> -  off_t *offset, off_t *size)
> +  uint64_t *offset, int64_t *size)

function never return negative @size, so what is the reason to keep it signed?

Also, type conversion (uint64_t) should be dropped from the function code I 
think.

>   {
>   struct partition_record mbr[4];
>   uint8_t data[MBR_SIZE];
> @@ -500,14 +500,14 @@ int main(int argc, char **argv)
>   {
>   BlockBackend *blk;
>   BlockDriverState *bs;
> -off_t dev_offset = 0;
> +uint64_t dev_offset = 0;
>   uint16_t nbdflags = 0;
>   bool disconnect = false;

Re: [Qemu-devel] test-filter-mirror hangs

On Fri, 11 Jan 2019 at 16:15, Dr. David Alan Gilbert
 wrote:
>
> * Peter Maydell (peter.mayd...@linaro.org) wrote:
> > Recently I've noticed that test-filter-mirror has been hanging
> > intermittently, typically when run on some other TCG architecture.
> > In the instance I've just looked at, this was with s390x guest on
> > x86-64 host, though I've also seen it on other host archs and
> > perhaps with other guests.
>
> Watch out to see if you really do see it for other guests;
> it carefully avoids using virtio-net to avoid vhost; but on s390x it
> uses virtio-net-ccw - could that hit the vhost it was trying to avoid?

I've seen several hangs in the last few days, all on s390x guests.
It is intermittent though, so sometimes s390x works fine.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1] virtio: add checks for the size of the indirect table

2019-01-15 Thread Philippe Mathieu-Daudé

On 1/15/19 11:08 AM, Dima Stepanov wrote:
> The virtqueue_pop() and virtqueue_get_avail_bytes() routines can use the
> INDIRECT table to get the data. It is possible to create a packet which
> will lead to the assert message like:
>   include/exec/memory.h:1995: void
>   address_space_read_cached(MemoryRegionCache *, hwaddr, void *, int):
>   Assertion `addr < cache->len && len <= cache->len - addr' failed.
>   Aborted
> To do it the first descriptor should have a link to the INDIRECT table
> and set the size of it to 0. It doesn't look good that the guest should
> be able to trigger the assert in qemu. Add additional check for the size
> of the INDIRECT table, which should not be 0.
> 
> Signed-off-by: Dima Stepanov 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  hw/virtio/virtio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 22bd1ac..a1ff647 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -646,7 +646,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
> int *in_bytes,
>  vring_desc_read(vdev, &desc, desc_cache, i);
>  
>  if (desc.flags & VRING_DESC_F_INDIRECT) {
> -if (desc.len % sizeof(VRingDesc)) {
> +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
>  virtio_error(vdev, "Invalid size for indirect buffer table");
>  goto err;
>  }
> @@ -902,7 +902,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
>  desc_cache = &caches->desc;
>  vring_desc_read(vdev, &desc, desc_cache, i);
>  if (desc.flags & VRING_DESC_F_INDIRECT) {
> -if (desc.len % sizeof(VRingDesc)) {
> +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
>  virtio_error(vdev, "Invalid size for indirect buffer table");
>  goto done;
>  }
>

Re: [Qemu-devel] [PATCH v1] virtio: add checks for the size of the indirect table

On Tue, 15 Jan 2019 13:08:47 +0300
Dima Stepanov  wrote:

> The virtqueue_pop() and virtqueue_get_avail_bytes() routines can use the
> INDIRECT table to get the data. It is possible to create a packet which
> will lead to the assert message like:
>   include/exec/memory.h:1995: void
>   address_space_read_cached(MemoryRegionCache *, hwaddr, void *, int):
>   Assertion `addr < cache->len && len <= cache->len - addr' failed.
>   Aborted
> To do it the first descriptor should have a link to the INDIRECT table
> and set the size of it to 0. It doesn't look good that the guest should
> be able to trigger the assert in qemu. Add additional check for the size
> of the INDIRECT table, which should not be 0.

Ouch, being able to crash QEMU by a specially crafted descriptor is bad.

Looking at the virtio spec, we don't seem to explicitly disallow
indirect descriptors with a zero-length table. So, as an alternative to
marking the device broken, we could also skip over such a descriptor.
Not sure whether that makes sense, though.

> 
> Signed-off-by: Dima Stepanov 
> ---
>  hw/virtio/virtio.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 22bd1ac..a1ff647 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -646,7 +646,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
> int *in_bytes,
>  vring_desc_read(vdev, &desc, desc_cache, i);
>  
>  if (desc.flags & VRING_DESC_F_INDIRECT) {
> -if (desc.len % sizeof(VRingDesc)) {
> +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
>  virtio_error(vdev, "Invalid size for indirect buffer table");
>  goto err;
>  }
> @@ -902,7 +902,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
>  desc_cache = &caches->desc;
>  vring_desc_read(vdev, &desc, desc_cache, i);
>  if (desc.flags & VRING_DESC_F_INDIRECT) {
> -if (desc.len % sizeof(VRingDesc)) {
> +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
>  virtio_error(vdev, "Invalid size for indirect buffer table");
>  goto done;
>  }

Re: [Qemu-devel] [PATCH v1 4/8] RISC-V: Use riscv prefix consistently on cpu helpers

2019-01-15 Thread Philippe Mathieu-Daudé

Hi Alistair,

On 1/15/19 12:58 AM, Alistair Francis wrote:
> From: Michael Clark 
> 
> * Add riscv prefix to raise_exception function
> * Add riscv prefix to CSR read/write functions
> * Add riscv prefix to signal handler function
> * Add riscv prefix to get fflags function
> * Remove redundant declaration of riscv_cpu_init
>   and rename cpu_riscv_init to riscv_cpu_init
> * rename riscv_set_mode to riscv_cpu_set_mode
> 
> Cc: Sagar Karandikar 
> Cc: Bastian Koppelmann 
> Cc: Palmer Dabbelt 
> Cc: Alistair Francis 
> Signed-off-by: Michael Clark 
> Signed-off-by: Alistair Francis 
> ---
>  linux-user/riscv/signal.c |  4 ++--
>  target/riscv/cpu.h| 21 ++---
>  target/riscv/cpu_helper.c | 10 +-
>  target/riscv/csr.c|  8 
>  target/riscv/fpu_helper.c |  6 +++---
>  target/riscv/op_helper.c  | 28 ++--
>  6 files changed, 38 insertions(+), 39 deletions(-)
> 
> diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
> index f598d41891..83ecc6f799 100644
> --- a/linux-user/riscv/signal.c
> +++ b/linux-user/riscv/signal.c
> @@ -83,7 +83,7 @@ static void setup_sigcontext(struct target_sigcontext *sc, 
> CPURISCVState *env)
>  __put_user(env->fpr[i], &sc->fpr[i]);
>  }
>  
> -uint32_t fcsr = csr_read_helper(env, CSR_FCSR); /*riscv_get_fcsr(env);*/
> +uint32_t fcsr = riscv_csr_read(env, CSR_FCSR);
>  __put_user(fcsr, &sc->fcsr);
>  }
>  
> @@ -159,7 +159,7 @@ static void restore_sigcontext(CPURISCVState *env, struct 
> target_sigcontext *sc)
>  
>  uint32_t fcsr;
>  __get_user(fcsr, &sc->fcsr);
> -csr_write_helper(env, fcsr, CSR_FCSR);
> +riscv_csr_write(env, CSR_FCSR, fcsr);
>  }
>  
>  static void restore_ucontext(CPURISCVState *env, struct target_ucontext *uc)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 681341f5d5..a97435bd7b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -256,7 +256,7 @@ int riscv_cpu_handle_mmu_fault(CPUState *cpu, vaddr 
> address, int size,
>  char *riscv_isa_string(RISCVCPU *cpu);
>  void riscv_cpu_list(FILE *f, fprintf_function cpu_fprintf);
>  
> -#define cpu_signal_handler cpu_riscv_signal_handler
> +#define cpu_signal_handler riscv_cpu_signal_handler
>  #define cpu_list riscv_cpu_list
>  #define cpu_mmu_index riscv_cpu_mmu_index
>  
> @@ -264,16 +264,15 @@ void riscv_cpu_list(FILE *f, fprintf_function 
> cpu_fprintf);
>  uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t value);
>  #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
>  #endif
> -void riscv_set_mode(CPURISCVState *env, target_ulong newpriv);
> +void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
>  
>  void riscv_translate_init(void);
> -RISCVCPU *cpu_riscv_init(const char *cpu_model);
> -int cpu_riscv_signal_handler(int host_signum, void *pinfo, void *puc);
> -void QEMU_NORETURN do_raise_exception_err(CPURISCVState *env,
> -  uint32_t exception, uintptr_t pc);
> +int riscv_cpu_signal_handler(int host_signum, void *pinfo, void *puc);
> +void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env,
> + uint32_t exception, uintptr_t pc);
>  
> -target_ulong cpu_riscv_get_fflags(CPURISCVState *env);
> -void cpu_riscv_set_fflags(CPURISCVState *env, target_ulong);
> +target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
> +void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
>  
>  #define TB_FLAGS_MMU_MASK   3
>  #define TB_FLAGS_MSTATUS_FS MSTATUS_FS
> @@ -293,13 +292,13 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState 
> *env, target_ulong *pc,
>  int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
>  target_ulong new_value, target_ulong write_mask);
>  
> -static inline void csr_write_helper(CPURISCVState *env, target_ulong val,
> -int csrno)
> +static inline void riscv_csr_write(CPURISCVState *env, int csrno,
> +   target_ulong val)
>  {
>  riscv_csrrw(env, csrno, NULL, val, MAKE_64BIT_MASK(0, TARGET_LONG_BITS));
>  }
>  
> -static inline target_ulong csr_read_helper(CPURISCVState *env, int csrno)
> +static inline target_ulong riscv_csr_read(CPURISCVState *env, int csrno)

Don't you need to update target/riscv/gdbstub.c (in
riscv_cpu_gdb_read_register)?

>  {
>  target_ulong val = 0;
>  riscv_csrrw(env, csrno, &val, 0, 0);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f257050f12..f49e98ed59 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -93,7 +93,7 @@ uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, 
> uint32_t value)
>  return old;
>  }
>  
> -void riscv_set_mode(CPURISCVState *env, target_ulong newpriv)
> +void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
>  {
>  if (newpriv > PRV_M) {
>  g_as

[Qemu-devel] [PATCH 1/2] qcow2: include LUKS payload overhead in qemu-img measure

2019-01-15 Thread Stefan Hajnoczi

LUKS encryption reserves clusters for its own payload data.  The size of
this area must be included in the qemu-img measure calculation so that
we arrive at the correct minimum required image size.

(Ab)use the qcrypto_block_create() API to determine the payload
overhead.  We discard the payload data that qcrypto thinks will be
written to the image.

Signed-off-by: Stefan Hajnoczi 
---
 block/qcow2.c | 51 ++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 4897abae5e..7ab93a5d2f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4231,6 +4231,24 @@ static coroutine_fn int 
qcow2_co_flush_to_os(BlockDriverState *bs)
 return ret;
 }
 
+static ssize_t qcow2_measure_crypto_hdr_init_func(QCryptoBlock *block,
+size_t headerlen, void *opaque, Error **errp)
+{
+size_t *headerlenp = opaque;
+
+/* Stash away the payload size */
+*headerlenp = headerlen;
+return 0;
+}
+
+static ssize_t qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,
+size_t offset, const uint8_t *buf, size_t buflen,
+void *opaque, Error **errp)
+{
+/* Discard the bytes, we're not actually writing to an image */
+return buflen;
+}
+
 static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, BlockDriverState *in_bs,
Error **errp)
 {
@@ -4240,11 +4258,13 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 uint64_t virtual_size; /* disk size as seen by guest */
 uint64_t refcount_bits;
 uint64_t l2_tables;
+uint64_t luks_payload_size = 0;
 size_t cluster_size;
 int version;
 char *optstr;
 PreallocMode prealloc;
 bool has_backing_file;
+bool has_luks;
 
 /* Parse image creation options */
 cluster_size = qcow2_opt_get_cluster_size_del(opts, &local_err);
@@ -4274,6 +4294,35 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 has_backing_file = !!optstr;
 g_free(optstr);
 
+optstr = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT);
+has_luks = optstr && strcmp(optstr, "luks") == 0;
+g_free(optstr);
+
+if (has_luks) {
+QCryptoBlockCreateOptions cryptoopts = {
+.format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
+};
+QCryptoBlock *crypto;
+size_t headerlen;
+
+optstr = qemu_opt_get_del(opts, "encrypt.key-secret");
+cryptoopts.u.luks.has_key_secret = !!optstr;
+cryptoopts.u.luks.key_secret = optstr;
+
+crypto = qcrypto_block_create(&cryptoopts, "encrypt.",
+  qcow2_measure_crypto_hdr_init_func,
+  qcow2_measure_crypto_hdr_write_func,
+  &headerlen, &local_err);
+
+g_free(optstr);
+if (!crypto) {
+goto err;
+}
+qcrypto_block_free(crypto);
+
+luks_payload_size = ROUND_UP(headerlen, cluster_size);
+}
+
 virtual_size = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0);
 virtual_size = ROUND_UP(virtual_size, cluster_size);
 
@@ -4344,7 +4393,7 @@ static BlockMeasureInfo *qcow2_measure(QemuOpts *opts, 
BlockDriverState *in_bs,
 info = g_new(BlockMeasureInfo, 1);
 info->fully_allocated =
 qcow2_calc_prealloc_size(virtual_size, cluster_size,
- ctz32(refcount_bits));
+ ctz32(refcount_bits)) + luks_payload_size;
 
 /* Remove data clusters that are not required.  This overestimates the
  * required size because metadata needed for the fully allocated file is
-- 
2.20.1

[Qemu-devel] [PATCH 0/2] qcow2: include LUKS payload overhead in qemu-img measure

2019-01-15 Thread Stefan Hajnoczi

The LUKS payload has a significant size (>1 MB).  It must be included in the
qemu-img measure calculation so we arrive at the correct minimum image size.

Stefan Hajnoczi (2):
  qcow2: include LUKS payload overhead in qemu-img measure
  iotests: add LUKS payload overhead to 178 qemu-img measure test

 block/qcow2.c| 51 +++-
 tests/qemu-iotests/178   |  8 +
 tests/qemu-iotests/178.out.qcow2 | 24 +++
 3 files changed, 82 insertions(+), 1 deletion(-)

-- 
2.20.1

[Qemu-devel] [PATCH 2/2] iotests: add LUKS payload overhead to 178 qemu-img measure test

2019-01-15 Thread Stefan Hajnoczi

The previous patch includes the LUKS payload overhead into the qemu-img
measure calculation for qcow2.  Update qemu-iotests 178 to exercise this
new code path.

Signed-off-by: Stefan Hajnoczi 
---
 tests/qemu-iotests/178   |  8 
 tests/qemu-iotests/178.out.qcow2 | 24 
 2 files changed, 32 insertions(+)

diff --git a/tests/qemu-iotests/178 b/tests/qemu-iotests/178
index 3f4b4a4564..23eb017ea1 100755
--- a/tests/qemu-iotests/178
+++ b/tests/qemu-iotests/178
@@ -142,6 +142,14 @@ for ofmt in human json; do
 # The backing file doesn't need to exist :)
 $QEMU_IMG measure --output=$ofmt -o backing_file=x \
   -f "$fmt" -O "$IMGFMT" "$TEST_IMG"
+
+echo
+echo "== $fmt input image and LUKS encryption =="
+echo
+$QEMU_IMG measure --output=$ofmt \
+  --object secret,id=sec0,data=base \
+  -o encrypt.format=luks,encrypt.key-secret=sec0 \
+  -f "$fmt" -O "$IMGFMT" "$TEST_IMG"
 fi
 
 echo
diff --git a/tests/qemu-iotests/178.out.qcow2 b/tests/qemu-iotests/178.out.qcow2
index d42d4a4597..55a8dc926f 100644
--- a/tests/qemu-iotests/178.out.qcow2
+++ b/tests/qemu-iotests/178.out.qcow2
@@ -68,6 +68,11 @@ converted image file size in bytes: 458752
 required size: 1074135040
 fully allocated size: 1074135040
 
+== qcow2 input image and LUKS encryption ==
+
+required size: 2686976
+fully allocated size: 1076232192
+
 == qcow2 input image and preallocation (human) ==
 
 required size: 1074135040
@@ -114,6 +119,11 @@ converted image file size in bytes: 524288
 required size: 1074135040
 fully allocated size: 1074135040
 
+== raw input image and LUKS encryption ==
+
+required size: 2686976
+fully allocated size: 1076232192
+
 == raw input image and preallocation (human) ==
 
 required size: 1074135040
@@ -205,6 +215,13 @@ converted image file size in bytes: 458752
 "fully-allocated": 1074135040
 }
 
+== qcow2 input image and LUKS encryption ==
+
+{
+"required": 2686976,
+"fully-allocated": 1076232192
+}
+
 == qcow2 input image and preallocation (json) ==
 
 {
@@ -263,6 +280,13 @@ converted image file size in bytes: 524288
 "fully-allocated": 1074135040
 }
 
+== raw input image and LUKS encryption ==
+
+{
+"required": 2686976,
+"fully-allocated": 1076232192
+}
+
 == raw input image and preallocation (json) ==
 
 {
-- 
2.20.1

Re: [Qemu-devel] [PULL 00/21] misc testing fixes for Travis and docker

On Mon, 14 Jan 2019 at 15:01, Alex Bennée  wrote:
>
> The following changes since commit 7260438b7056469610ee166f7abe9ff8a26b8b16:
>
>   Merge remote-tracking branch 
> 'remotes/palmer/tags/riscv-for-master-3.2-part2' into staging (2019-01-14 
> 11:41:43 +)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-testing-next-140119-1
>
> for you to fetch changes up to a36270a4d1589b1ed309065fc8b3fe0ac8d6869d:
>
>   Revert "tests: Disable qht-bench parallel test when using gprof" 
> (2019-01-14 14:55:32 +)
>
> 
> A bunch of fixes for testing:
>
>   - Various Travis updates
>   - "stable" SID snapshot for docker
>   - avoid :latest docker tags
>   - g_usleep fix for some tests
>

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH 0/2] qcow2: include LUKS payload overhead in qemu-img measure

2019-01-15 Thread Philippe Mathieu-Daudé

On 1/15/19 12:10 PM, Stefan Hajnoczi wrote:
> The LUKS payload has a significant size (>1 MB).  It must be included in the
> qemu-img measure calculation so we arrive at the correct minimum image size.
> 
> Stefan Hajnoczi (2):
>   qcow2: include LUKS payload overhead in qemu-img measure
>   iotests: add LUKS payload overhead to 178 qemu-img measure test
> 
>  block/qcow2.c| 51 +++-
>  tests/qemu-iotests/178   |  8 +
>  tests/qemu-iotests/178.out.qcow2 | 24 +++
>  3 files changed, 82 insertions(+), 1 deletion(-)
> 

Reviewed-by: Philippe Mathieu-Daudé

Re: [Qemu-devel] [qemu-s390x] [PATCH v2] configure: Only build the s390-ccw bios if the compiler supports -march=z900

2019-01-15 Thread Christian Borntraeger




On 14.01.2019 13:52, Thomas Huth wrote:
> We want to build our s390-ccw bios with -march=z900 so that it also
> works with the oldest s390x CPU that we support with TCG. However,
> Clang on s390x does not support -march=z900 anymore, so we can not
> use this compiler to build the s390-ccw bios. Thus add a proper test
> to the configure script to see whether the compiler is usable.
> 
> Signed-off-by: Thomas Huth 

Acked-by: Christian Borntraeger 

> ---
>  v2: Use compile_prog as suggested by Peter
> 
>  configure | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 05b7e85..2b9ba7d 100755
> --- a/configure
> +++ b/configure
> @@ -5905,8 +5905,12 @@ if test "$cpu" = "ppc64" -a "$targetos" != "Darwin" ; 
> then
>roms="$roms spapr-rtas"
>  fi
>  
> +# Only build s390-ccw bios if we're on s390x and the compiler has -march=z900
>  if test "$cpu" = "s390x" ; then
> -  roms="$roms s390-ccw"
> +  write_c_skeleton
> +  if compile_prog "-march=z900" ""; then
> +roms="$roms s390-ccw"
> +  fi
>  fi
>  
>  # Probe for the need for relocating the user-only binary.
>

Re: [Qemu-devel] [PATCH 1/2] vfio-pci: Introduce vfio_register_event_notifier helper

On Fri, 11 Jan 2019 17:58:00 +0100
Eric Auger  wrote:

> The code used to attach the eventfd handler for the ERR and
> REQ irq indices can be factorized into a helper. In subsequent
> patches we will extend this helper to support other irq indices.

Looks like a nice refactoring to me.

> 
> We test the notification is allowed outside of the helper:

s/We test/We test whether/

> respectively check vdev->pci_aer and VFIO_FEATURE_ENABLE_REQ.
> Depending on the returned value we set vdev->pci_aer and
> vdev->req_enabled. An error handle is introduced for future usage
> although not strictly useful here.
> 
> Signed-off-by: Eric Auger 
> ---
>  hw/vfio/pci.c | 291 ++
>  1 file changed, 127 insertions(+), 164 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index c0cb1ec289..c589a4e666 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -105,6 +105,95 @@ static void vfio_intx_eoi(VFIODevice *vbasedev)
>  vfio_unmask_single_irqindex(vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
>  }
>  
> +/*
> + * vfio_register_event_notifier - setup/tear down eventfd
> + * notification and handling for IRQ indices that span over
> + * a single IRQ
> + *
> + * @vdev: VFIO device handle
> + * @index: IRQ index the eventfd/handler is associated to

s/to/with/ ?

> + * @target_state: true means notifier needs to be set up
> + * @handler to attach if @target_state is true
> + * @errp error handle
> + */
> +static int vfio_register_event_notifier(VFIOPCIDevice *vdev,
> +int index,
> +bool target_state,
> +void (*handler)(void *opaque),
> +Error **errp)

(...)

> @@ -3069,8 +2998,29 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>  goto out_teardown;
>  }
>  
> -vfio_register_err_notifier(vdev);
> -vfio_register_req_notifier(vdev);
> +if (vdev->pci_aer) {
> +/*
> + * Registers error notifier for devices supporting error recovery.
> + * If we encounter a failure in this function, we report an error

s/in this function/while registering it/ ?

> + * and continue after disabling error recovery support for the
> + * device.
> + */
> +vdev->pci_aer =
> +!vfio_register_event_notifier(vdev, VFIO_PCI_ERR_IRQ_INDEX, true,
> +  vfio_err_notifier_handler, &err);
> +if (err) {
> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> +}

I think you need to reset err to NULL if you want to reuse the variable.

Alternatively, you could keep the wrappers and define a local error
variable there.

> +}
> +
> +if (vdev->features & VFIO_FEATURE_ENABLE_REQ) {
> +vdev->req_enabled =
> +!vfio_register_event_notifier(vdev, VFIO_PCI_REQ_IRQ_INDEX, true,
> +  vfio_req_notifier_handler, &err);
> +if (err) {
> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> +}
> +}
>  vfio_setup_resetfn_quirk(vdev);
>  
>  return;
> @@ -3106,9 +3056,22 @@ static void vfio_instance_finalize(Object *obj)
>  static void vfio_exitfn(PCIDevice *pdev)
>  {
>  VFIOPCIDevice *vdev = PCI_VFIO(pdev);
> +Error *err = NULL;
>  
> -vfio_unregister_req_notifier(vdev);
> -vfio_unregister_err_notifier(vdev);
> +if (vdev->req_enabled) {
> +vfio_register_event_notifier(vdev, VFIO_PCI_REQ_IRQ_INDEX,
> + false, NULL, &err);
> +if (err) {
> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> +}

Likewise.

> +}
> +if (vdev->pci_aer) {
> +vfio_register_event_notifier(vdev, VFIO_PCI_ERR_IRQ_INDEX,
> + false, NULL, &err);
> +if (err) {
> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
> +}
> +}
>  pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
>  vfio_disable_interrupts(vdev);
>  if (vdev->intx.mmap_timer) {

Re: [Qemu-devel] [PATCH 2/2] vfio-pci: Use vfio_register_event_notifier in vfio_intx_enable_kvm

On Fri, 11 Jan 2019 17:58:01 +0100
Eric Auger  wrote:

> We can also use vfio_register_event_notifier() helper in
> vfio_intx_enable_kvm to set the signalling associated to
> VFIO_PCI_INTX_IRQ_INDEX.
> 
> Signed-off-by: Eric Auger 
> ---
>  hw/vfio/pci.c | 38 +++---
>  1 file changed, 7 insertions(+), 31 deletions(-)
> 

>  static void vfio_intx_disable(VFIOPCIDevice *vdev)

I'm wondering why the _disable path can't use the new helper. Ordering
issues?

[Qemu-devel] Profesjonalne Strony i Sklepy internetowe.

2019-01-15 Thread Strony, sklepy internetowe.



Dzień dobry,



tworzymy nowoczesne */Stron i Sklepów WWW.
/*


Jeżeli chcieliby Państwo otrzymać propozycję w tym zakresie dla Państwa firmy 
prosimy o odpowiedź "*TAK"*.







Serdecznie Pozdrawiamy

[Qemu-devel] [PATCH v3 0/4] QEMU changes to do PVH boot

For certain applications it is desirable to rapidly boot a KVM virtual
machine. In cases where legacy hardware and software support within the
guest is not needed, QEMU should be able to boot directly into the
uncompressed Linux kernel binary with minimal firmware involvement.

There already exists an ABI to allow this for Xen PVH guests and the ABI
is supported by Linux and FreeBSD:

   https://xenbits.xen.org/docs/unstable/misc/pvh.html

Details on the Linux changes (in 4.21): https://lkml.org/lkml/2018/12/14/1330
qboot pull request integrated: https://github.com/bonzini/qboot/pull/17 

This patch series provides QEMU support to read the ELF header of an
uncompressed kernel binary and get the 32-bit PVH kernel entry point
from an ELF Note.  In load_linux() a call is made to load_elfboot()
so see if the header matches that of an uncompressed kernel binary (ELF)
and if so, loads the binary and determines the kernel entry address
from an ELF Note in the binary.  Then qboot does futher initialisation
of the guest (e820, etc.) and jumps to the kernel entry address and
boots the guest.

changes v1 -> v2
- Based on feedback from Stefan Hajnoczi
- The reading of the PVH entry point is now done in a single pass during
  elf_load() which results in Patch2 in v1 being split into Patches 1&2 in v2
  and considerably reworked.
- Patch1 adds a new optional function pointer to parse the ELF note type
  (the type is passed in via the existing translate_opaque arg - the
  function already had 11 args so I didn't want to add more than one new arg).
- Patch2 adds a function to elf_ops.h to find an ELF note
  matching a specific type 
- Patch3 just has a line added to the commit message to state that the Xen
  repo is the canonical location
- Patch4 (that does the PVH boot) is mainly equivalent to Patch3 in v1
  just minor load_elfboot() changes and the addition of a
  read_pvh_start_addr() helper function for load_elf()

changes v2 -> v3
- Based on feedback from Stefan Hajnoczi
- Fix formatting issues where a few tabs snuck in v2
- Moved code to use ELF Note in load_elf() from Patch1 to Patch2
- In load_elf() set data to NULL after g_free() [now in Patch2 following move]
- Added Patch5 containing changes by Stefano Garzarella to support -initrd

Usіng the method/scripts documented by the NEMU team at

   https://github.com/intel/nemu/wiki/Measuring-Boot-Latency
   https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg00200.html

below are some timings measured (vmlinux and bzImage from the same build)
Time to get to kernel start is almost halved (95ṁs -> 48ms)

QEMU + qboot + vmlinux (PVH + 4.20-rc4)
 qemu_init_end: 41.550521
 fw_start: 41.667139 (+0.116618)
 fw_do_boot: 47.448495 (+5.781356)
 linux_startup_64: 47.720785 (+0.27229)
 linux_start_kernel: 48.399541 (+0.678756)
 linux_start_user: 296.952056 (+248.552515)

QEMU + qboot + bzImage:
 qemu_init_end: 29.209276
 fw_start: 29.317342 (+0.108066)
 linux_start_boot: 36.679362 (+7.36202)
 linux_startup_64: 94.531349 (+57.851987)
 linux_start_kernel: 94.900913 (+0.369564)
 linux_start_user: 401.060971 (+306.160058)

QEMU + bzImage:
 qemu_init_end: 30.424430
 linux_startup_64: 893.770334 (+863.345904)
 linux_start_kernel: 894.17049 (+0.400156)
 linux_start_user: 1208.679768 (+314.509278)


Liam Merwick (4):
  elf: Add optional function ptr to load_elf() to parse ELF notes
  elf-ops.h: Add get_elf_note_type()
  pvh: Add x86/HVM direct boot ABI header file
  pvh: Boot uncompressed kernel using direct boot ABI

 hw/alpha/dp264.c   |   4 +-
 hw/arm/armv7m.c|   3 +-
 hw/arm/boot.c  |   2 +-
 hw/core/generic-loader.c   |   2 +-
 hw/core/loader.c   |  24 ---
 hw/cris/boot.c |   3 +-
 hw/hppa/machine.c  |   6 +-
 hw/i386/multiboot.c|   2 +-
 hw/i386/pc.c   | 135 +
 hw/lm32/lm32_boards.c  |   6 +-
 hw/lm32/milkymist.c|   3 +-
 hw/m68k/an5206.c   |   2 +-
 hw/m68k/mcf5208.c  |   2 +-
 hw/microblaze/boot.c   |   7 +-
 hw/mips/mips_fulong2e.c|   5 +-
 hw/mips/mips_malta.c   |   5 +-
 hw/mips/mips_mipssim.c |   5 +-
 hw/mips/mips_r4k.c |   5 +-
 hw/moxie/moxiesim.c|   2 +-
 hw/nios2/boot.c|   7 +-
 hw/openrisc/openrisc_sim.c |   2 +-
 hw/pci-host/prep.c |   2 +-
 hw/ppc/e500.c  |   3 +-
 hw/ppc/mac_newworld.c  |   5 +-
 hw/ppc/mac_oldworld.c  |   5 +-
 hw/ppc/ppc440_bamboo.c |   2 +-
 hw/ppc/sam460ex.c  |   3 +-
 hw/ppc/spapr.c |   7 +-
 hw/ppc/virtex_ml507.c  |   2 +-
 hw/riscv/sifive_e.c|   2 +-
 hw/riscv/sifive_u.c|   2 +-
 hw/riscv/spike.c   |   2 +-
 hw/riscv/virt.c|   2 +-
 hw/s390x/ipl.c |   9 ++-
 hw/sparc/leon3.c   |   3 +-
 hw/sparc/sun4m

[Qemu-devel] [PATCH v3 2/5] elf-ops.h: Add get_elf_note_type()

Introduce a routine which, given a pointer to a range of ELF Notes,
searches through them looking for a note matching the type specified
and returns a pointer to the matching ELF note.

get_elf_note_type() is used by elf_load[32|64]() to find the
specified note type required by the 'elf_note_fn' parameter
added in the previous commit.

Signed-off-by: Liam Merwick 
---
 include/hw/elf_ops.h | 75 
 1 file changed, 75 insertions(+)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 3438d6f69e8d..690f9238c8cc 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -265,6 +265,51 @@ fail:
 return ret;
 }
 
+/*
+ * Given 'nhdr', a pointer to a range of ELF Notes, search through them
+ * for a note matching type 'elf_note_type' and return a pointer to
+ * the matching ELF note.
+ */
+static struct elf_note *glue(get_elf_note_type, SZ)(struct elf_note *nhdr,
+elf_word note_size,
+elf_word phdr_align,
+elf_word elf_note_type)
+{
+elf_word nhdr_size = sizeof(struct elf_note);
+elf_word elf_note_entry_offset = 0;
+elf_word note_type;
+elf_word nhdr_namesz;
+elf_word nhdr_descsz;
+
+if (nhdr == NULL) {
+return NULL;
+}
+
+note_type = nhdr->n_type;
+while (note_type != elf_note_type) {
+nhdr_namesz = nhdr->n_namesz;
+nhdr_descsz = nhdr->n_descsz;
+
+elf_note_entry_offset = nhdr_size +
+QEMU_ALIGN_UP(nhdr_namesz, phdr_align) +
+QEMU_ALIGN_UP(nhdr_descsz, phdr_align);
+
+/*
+ * If the offset calculated in this iteration exceeds the
+ * supplied size, we are done and no matching note was found.
+ */
+if (elf_note_entry_offset > note_size) {
+return NULL;
+}
+
+/* skip to the next ELF Note entry */
+nhdr = (void *)nhdr + elf_note_entry_offset;
+note_type = nhdr->n_type;
+}
+
+return nhdr;
+}
+
 static int glue(load_elf, SZ)(const char *name, int fd,
   uint64_t (*elf_note_fn)(void *, void *, bool),
   uint64_t (*translate_fn)(void *, uint64_t),
@@ -497,6 +542,36 @@ static int glue(load_elf, SZ)(const char *name, int fd,
 high = addr + mem_size;
 
 data = NULL;
+
+} else if (ph->p_type == PT_NOTE && elf_note_fn) {
+struct elf_note *nhdr = NULL;
+
+file_size = ph->p_filesz; /* Size of the range of ELF notes */
+data = g_malloc0(file_size);
+if (ph->p_filesz > 0) {
+if (lseek(fd, ph->p_offset, SEEK_SET) < 0) {
+goto fail;
+}
+if (read(fd, data, file_size) != file_size) {
+goto fail;
+}
+}
+
+/*
+ * Search the ELF notes to find one with a type matching the
+ * value passed in via 'translate_opaque'
+ */
+nhdr = (struct elf_note *)data;
+assert(translate_opaque != NULL);
+nhdr = glue(get_elf_note_type, SZ)(nhdr, file_size, ph->p_align,
+   *(uint64_t *)translate_opaque);
+if (nhdr != NULL) {
+bool is64 =
+sizeof(struct elf_note) == sizeof(struct elf64_note);
+elf_note_fn((void *)nhdr, (void *)&ph->p_align, is64);
+}
+g_free(data);
+data = NULL;
 }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 4/5] pvh: Boot uncompressed kernel using direct boot ABI

These changes (along with corresponding Linux kernel and qboot changes)
enable a guest to be booted using the x86/HVM direct boot ABI.

This commit adds a load_elfboot() routine to pass the size and
location of the kernel entry point to qboot (which will fill in
the start_info struct information needed to to boot the guest).
Having loaded the ELF binary, load_linux() will run qboot
which continues the boot.

The address for the kernel entry point is read from an ELF Note
in the uncompressed kernel binary by a helper routine passed
to load_elf().

Co-developed-by: George Kennedy 
Signed-off-by: George Kennedy 
Signed-off-by: Liam Merwick 
---
 hw/i386/pc.c  | 135 ++
 include/elf.h |  10 +
 2 files changed, 145 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 73d688f84239..6d549950a044 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -54,6 +54,7 @@
 #include "sysemu/qtest.h"
 #include "kvm_i386.h"
 #include "hw/xen/xen.h"
+#include "hw/xen/start_info.h"
 #include "ui/qemu-spice.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
@@ -110,6 +111,9 @@ static struct e820_entry *e820_table;
 static unsigned e820_entries;
 struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
 
+/* Physical Address of PVH entry point read from kernel ELF NOTE */
+static size_t pvh_start_addr;
+
 GlobalProperty pc_compat_3_1[] = {
 { "intel-iommu", "dma-drain", "off" },
 { "Opteron_G3" "-" TYPE_X86_CPU, "rdtscp", "off" },
@@ -1060,6 +1064,109 @@ struct setup_data {
 uint8_t data[0];
 } __attribute__((packed));
 
+
+/*
+ * The entry point into the kernel for PVH boot is different from
+ * the native entry point.  The PVH entry is defined by the x86/HVM
+ * direct boot ABI and is available in an ELFNOTE in the kernel binary.
+ *
+ * This function is passed to load_elf() when it is called from
+ * load_elfboot() which then additionally checks for an ELF Note of
+ * type XEN_ELFNOTE_PHYS32_ENTRY and passes it to this function to
+ * parse the PVH entry address from the ELF Note.
+ *
+ * Due to trickery in elf_opts.h, load_elf() is actually available as
+ * load_elf32() or load_elf64() and this routine needs to be able
+ * to deal with being called as 32 or 64 bit.
+ *
+ * The address of the PVH entry point is saved to the 'pvh_start_addr'
+ * global variable.  (although the entry point is 32-bit, the kernel
+ * binary can be either 32-bit or 64-bit).
+ */
+static uint64_t read_pvh_start_addr(void *arg1, void *arg2, bool is64)
+{
+size_t *elf_note_data_addr;
+
+/* Check if ELF Note header passed in is valid */
+if (arg1 == NULL) {
+return 0;
+}
+
+if (is64) {
+struct elf64_note *nhdr64 = (struct elf64_note *)arg1;
+uint64_t nhdr_size64 = sizeof(struct elf64_note);
+uint64_t phdr_align = *(uint64_t *)arg2;
+uint64_t nhdr_namesz = nhdr64->n_namesz;
+
+elf_note_data_addr =
+((void *)nhdr64) + nhdr_size64 +
+QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
+} else {
+struct elf32_note *nhdr32 = (struct elf32_note *)arg1;
+uint32_t nhdr_size32 = sizeof(struct elf32_note);
+uint32_t phdr_align = *(uint32_t *)arg2;
+uint32_t nhdr_namesz = nhdr32->n_namesz;
+
+elf_note_data_addr =
+((void *)nhdr32) + nhdr_size32 +
+QEMU_ALIGN_UP(nhdr_namesz, phdr_align);
+}
+
+pvh_start_addr = *elf_note_data_addr;
+
+return pvh_start_addr;
+}
+
+static bool load_elfboot(const char *kernel_filename,
+   int kernel_file_size,
+   uint8_t *header,
+   size_t pvh_xen_start_addr,
+   FWCfgState *fw_cfg)
+{
+uint32_t flags = 0;
+uint32_t mh_load_addr = 0;
+uint32_t elf_kernel_size = 0;
+uint64_t elf_entry;
+uint64_t elf_low, elf_high;
+int kernel_size;
+
+if (ldl_p(header) != 0x464c457f) {
+return false; /* no elfboot */
+}
+
+bool elf_is64 = header[EI_CLASS] == ELFCLASS64;
+flags = elf_is64 ?
+((Elf64_Ehdr *)header)->e_flags : ((Elf32_Ehdr *)header)->e_flags;
+
+if (flags & 0x00010004) { /* LOAD_ELF_HEADER_HAS_ADDR */
+error_report("elfboot unsupported flags = %x", flags);
+exit(1);
+}
+
+uint64_t elf_note_type = XEN_ELFNOTE_PHYS32_ENTRY;
+kernel_size = load_elf(kernel_filename, read_pvh_start_addr,
+   NULL, &elf_note_type, &elf_entry,
+   &elf_low, &elf_high, 0, I386_ELF_MACHINE,
+   0, 0);
+
+if (kernel_size < 0) {
+error_report("Error while loading elf kernel");
+exit(1);
+}
+mh_load_addr = elf_low;
+elf_kernel_size = elf_high - elf_low;
+
+if (pvh_start_addr == 0) {
+error_report("Error loading uncompressed kernel without PVH ELF Note");
+exit(1);
+}
+fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, pvh_start_addr);
+fw_cfg_add_i

[Qemu-devel] [PATCH v3 3/5] pvh: Add x86/HVM direct boot ABI header file

From: Liam Merwick 

The x86/HVM direct boot ABI permits Qemu to be able to boot directly
into the uncompressed Linux kernel binary with minimal firmware involvement.

https://xenbits.xen.org/docs/unstable/misc/pvh.html

This commit adds the header file that defines the start_info struct
that needs to be populated in order to use this ABI.

The canonical version of start_info.h is in the Xen codebase.
(like QEMU, the Linux kernel uses a copy as well).

Signed-off-by: Liam Merwick 
Reviewed-by: Konrad Rzeszutek Wilk 
---
 include/hw/xen/start_info.h | 146 
 1 file changed, 146 insertions(+)
 create mode 100644 include/hw/xen/start_info.h

diff --git a/include/hw/xen/start_info.h b/include/hw/xen/start_info.h
new file mode 100644
index ..348779eb10cd
--- /dev/null
+++ b/include/hw/xen/start_info.h
@@ -0,0 +1,146 @@
+/*
+ * 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.
+ *
+ * Copyright (c) 2016, Citrix Systems, Inc.
+ */
+
+#ifndef __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
+#define __XEN_PUBLIC_ARCH_X86_HVM_START_INFO_H__
+
+/*
+ * Start of day structure passed to PVH guests and to HVM guests in %ebx.
+ *
+ * NOTE: nothing will be loaded at physical address 0, so a 0 value in any
+ * of the address fields should be treated as not present.
+ *
+ *  0 ++
+ *| magic  | Contains the magic value XEN_HVM_START_MAGIC_VALUE
+ *|| ("xEn3" with the 0x80 bit of the "E" set).
+ *  4 ++
+ *| version| Version of this structure. Current version is 1. New
+ *|| versions are guaranteed to be backwards-compatible.
+ *  8 ++
+ *| flags  | SIF_xxx flags.
+ * 12 ++
+ *| nr_modules | Number of modules passed to the kernel.
+ * 16 ++
+ *| modlist_paddr  | Physical address of an array of modules
+ *|| (layout of the structure below).
+ * 24 ++
+ *| cmdline_paddr  | Physical address of the command line,
+ *|| a zero-terminated ASCII string.
+ * 32 ++
+ *| rsdp_paddr | Physical address of the RSDP ACPI data structure.
+ * 40 ++
+ *| memmap_paddr   | Physical address of the (optional) memory map. Only
+ *|| present in version 1 and newer of the structure.
+ * 48 ++
+ *| memmap_entries | Number of entries in the memory map table. Only
+ *|| present in version 1 and newer of the structure.
+ *|| Zero if there is no memory map being provided.
+ * 52 ++
+ *| reserved   | Version 1 and newer only.
+ * 56 ++
+ *
+ * The layout of each entry in the module structure is the following:
+ *
+ *  0 ++
+ *| paddr  | Physical address of the module.
+ *  8 ++
+ *| size   | Size of the module in bytes.
+ * 16 ++
+ *| cmdline_paddr  | Physical address of the command line,
+ *|| a zero-terminated ASCII string.
+ * 24 ++
+ *| reserved   |
+ * 32 ++
+ *
+ * The layout of each entry in the memory map table is as follows:
+ *
+ *  0 ++
+ *| addr   | Base address
+ *  8 ++
+ *| size   | Size of mapping in bytes
+ * 16 ++
+ *| type   | Type of mapping as defined between the hypervisor
+ *|| and guest it's starting. E820_TYPE_xxx, for example.
+ * 20 +|
+ *| reserved   |
+ * 24 ++
+ *
+ * The address and sizes are always a 64bit little endian unsigned integer.
+ *
+ * NB: Xen on x86 will always try to place all the data below the 4GiB
+ * boundary.
+ *
+ * Version numbers of the hvm_start_info structure have evolved lik

[Qemu-devel] [PATCH v3 1/5] elf: Add optional function ptr to load_elf() to parse ELF notes

This patch adds an optional function pointer, 'elf_note_fn', to
load_elf() which causes load_elf() to additionally parse any
ELF program headers of type PT_NOTE and check to see if the ELF
Note is of the type specified by the 'translate_opaque' arg.
If a matching ELF Note is found then the specfied function pointer
is called to process the ELF note.

Passing a NULL function pointer results in ELF Notes being skipped.

The first consumer of this functionality is the PVHboot support
which needs to read the XEN_ELFNOTE_PHYS32_ENTRY ELF Note while
loading the uncompressed kernel binary in order to discover the
boot entry address for the x86/HVM direct boot ABI.

Signed-off-by: Liam Merwick 
---
 hw/alpha/dp264.c   |  4 ++--
 hw/arm/armv7m.c|  3 ++-
 hw/arm/boot.c  |  2 +-
 hw/core/generic-loader.c   |  2 +-
 hw/core/loader.c   | 24 
 hw/cris/boot.c |  3 ++-
 hw/hppa/machine.c  |  6 +++---
 hw/i386/multiboot.c|  2 +-
 hw/lm32/lm32_boards.c  |  6 --
 hw/lm32/milkymist.c|  3 ++-
 hw/m68k/an5206.c   |  2 +-
 hw/m68k/mcf5208.c  |  2 +-
 hw/microblaze/boot.c   |  7 ---
 hw/mips/mips_fulong2e.c|  5 +++--
 hw/mips/mips_malta.c   |  5 +++--
 hw/mips/mips_mipssim.c |  5 +++--
 hw/mips/mips_r4k.c |  5 +++--
 hw/moxie/moxiesim.c|  2 +-
 hw/nios2/boot.c|  7 ---
 hw/openrisc/openrisc_sim.c |  2 +-
 hw/pci-host/prep.c |  2 +-
 hw/ppc/e500.c  |  3 ++-
 hw/ppc/mac_newworld.c  |  5 +++--
 hw/ppc/mac_oldworld.c  |  5 +++--
 hw/ppc/ppc440_bamboo.c |  2 +-
 hw/ppc/sam460ex.c  |  3 ++-
 hw/ppc/spapr.c |  7 ---
 hw/ppc/virtex_ml507.c  |  2 +-
 hw/riscv/sifive_e.c|  2 +-
 hw/riscv/sifive_u.c|  2 +-
 hw/riscv/spike.c   |  2 +-
 hw/riscv/virt.c|  2 +-
 hw/s390x/ipl.c |  9 ++---
 hw/sparc/leon3.c   |  3 ++-
 hw/sparc/sun4m.c   |  6 --
 hw/sparc64/sun4u.c |  4 ++--
 hw/tricore/tricore_testboard.c |  2 +-
 hw/xtensa/sim.c| 12 
 hw/xtensa/xtfpga.c |  2 +-
 include/hw/elf_ops.h   |  2 ++
 include/hw/loader.h|  9 -
 41 files changed, 113 insertions(+), 70 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index dd62f2a4050c..0347eb897c8a 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -114,7 +114,7 @@ static void clipper_init(MachineState *machine)
 error_report("no palcode provided");
 exit(1);
 }
-size = load_elf(palcode_filename, cpu_alpha_superpage_to_phys,
+size = load_elf(palcode_filename, NULL, cpu_alpha_superpage_to_phys,
 NULL, &palcode_entry, &palcode_low, &palcode_high,
 0, EM_ALPHA, 0, 0);
 if (size < 0) {
@@ -133,7 +133,7 @@ static void clipper_init(MachineState *machine)
 if (kernel_filename) {
 uint64_t param_offset;
 
-size = load_elf(kernel_filename, cpu_alpha_superpage_to_phys,
+size = load_elf(kernel_filename, NULL, cpu_alpha_superpage_to_phys,
 NULL, &kernel_entry, &kernel_low, &kernel_high,
 0, EM_ALPHA, 0, 0);
 if (size < 0) {
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index f4446528307f..ae68aadef965 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -293,7 +293,8 @@ void armv7m_load_kernel(ARMCPU *cpu, const char 
*kernel_filename, int mem_size)
 as = cpu_get_address_space(cs, asidx);
 
 if (kernel_filename) {
-image_size = load_elf_as(kernel_filename, NULL, NULL, &entry, &lowaddr,
+image_size = load_elf_as(kernel_filename, NULL, NULL, NULL,
+ &entry, &lowaddr,
  NULL, big_endian, EM_ARM, 1, 0, as);
 if (image_size < 0) {
 image_size = load_image_targphys_as(kernel_filename, 0,
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index c7a67af7a97c..9d8746f7613f 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -885,7 +885,7 @@ static int64_t arm_load_elf(struct arm_boot_info *info, 
uint64_t *pentry,
 }
 }
 
-ret = load_elf_as(info->kernel_filename, NULL, NULL,
+ret = load_elf_as(info->kernel_filename, NULL, NULL, NULL,
   pentry, lowaddr, highaddr, big_endian, elf_machine,
   1, data_swab, as);
 if (ret <= 0) {
diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index fbae05fb3b64..3695dd439cd0 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -136,7 +136,7 @@ static void generic_loader_realize(DeviceState *dev, Error 
**errp)
 AddressSpace *as = s->cpu ? s->cpu->as :  NULL;
 
 if (!s->force_raw) {
-

[Qemu-devel] [PATCH v3 5/5] pvh: load initrd and expose it through fw_cfg

From: Stefano Garzarella 

When initrd is specified, load and expose it to the guest firmware
through fw_cfg. The firmware will fill the hvm_start_info for the
kernel.

Signed-off-by: Stefano Garzarella 
Based-on: <1545422632-2-5-git-send-email-liam.merw...@oracle.com>
Signed-off-by: Liam Merwick 
---
 hw/i386/pc.c | 38 +-
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6d549950a044..9ed5063de8f8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1213,25 +1213,45 @@ static void load_linux(PCMachineState *pcms,
  */
 if (load_elfboot(kernel_filename, kernel_size,
  header, pvh_start_addr, fw_cfg)) {
-struct hvm_modlist_entry ramdisk_mod = { 0 };
-
 fclose(f);
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
 strlen(kernel_cmdline) + 1);
 fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, kernel_cmdline);
 
-assert(machine->device_memory != NULL);
-ramdisk_mod.paddr = machine->device_memory->base;
-ramdisk_mod.size =
-memory_region_size(&machine->device_memory->mr);
-
-fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, &ramdisk_mod,
- sizeof(ramdisk_mod));
 fw_cfg_add_i32(fw_cfg, FW_CFG_SETUP_SIZE, sizeof(header));
 fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA,
  header, sizeof(header));
 
+/* load initrd */
+if (initrd_filename) {
+gsize initrd_size;
+gchar *initrd_data;
+GError *gerr = NULL;
+
+if (!g_file_get_contents(initrd_filename, &initrd_data,
+&initrd_size, &gerr)) {
+fprintf(stderr, "qemu: error reading initrd %s: %s\n",
+initrd_filename, gerr->message);
+exit(1);
+}
+
+initrd_max = pcms->below_4g_mem_size - pcmc->acpi_data_size - 
1;
+if (initrd_size >= initrd_max) {
+fprintf(stderr, "qemu: initrd is too large, cannot 
support."
+"(max: %"PRIu32", need %"PRId64")\n",
+initrd_max, (uint64_t)initrd_size);
+exit(1);
+}
+
+initrd_addr = (initrd_max - initrd_size) & ~4095;
+
+fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
+fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
+fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data,
+ initrd_size);
+}
+
 return;
 }
 /* This looks like a multiboot kernel. If it is, let's stop
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h"

2019-01-15 Thread Markus Armbruster

Philippe Mathieu-Daudé  writes:

> From: Philippe Mathieu-Daudé 
>
> There are only three files requiring this typedef, let them
> include "hw/ssi/ssi.h" directly to simplify "qemu/typedefs.h".
>
> To clean "qemu/typedefs.h", move the forward declaration
> to "hw/ssi/ssi.h".
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/arm/strongarm.h  | 1 +
>  include/hw/arm/pxa.h| 1 +
>  include/hw/ssi/pl022.h  | 1 +
>  include/hw/ssi/ssi.h| 1 +
>  include/qemu/typedefs.h | 1 -
>  5 files changed, 4 insertions(+), 1 deletion(-)

When typedefs.h changes, we recompile the world, but it pretty much only
ever changes when new typedefs are added.  Thus, *keeping* a typedef
there is therefore pretty cheap.

Nevertheless, we shouldn't keep typedefs there without a real reason.
Being able to move one away without having to add any new #include
directives is a strong sign for "no real reason".  I like patches doing
that.

What I don't like is adding #include directives just so you can move
typedefs out of typedefs.h: it slows down the build.  Granted, the four
added by this patch are a drop in the bucket.  The point I'm trying to
make is typedefs.h's purpose: it's for avoiding #include directives.
Circular ones in particular, but others, too.

Re: [Qemu-devel] [PATCH v3 06/19] qemu-nbd: Avoid strtol open-coding

12.01.2019 20:57, Eric Blake wrote:
> Our copy-and-pasted open-coding of strtol handling forgot to
> handle overflow conditions.  Use qemu_strto*() instead.
> 
> In the case of --partition, since we insist on a user-supplied
> partition to be non-zero, we can use 0 rather than -1 for our
> initial value to distinguish when a partition is not being
> served, for slightly more optimal code.
> 
> The error messages for out-of-bounds values are less specific,
> but should not be a terrible loss in quality.
> 
> Signed-off-by: Eric Blake 
> Message-Id: <20181215135324.152629-8-ebl...@redhat.com>
> 
> ---
> v3: rebase to use int64_t rather than off_t [Vladimir]
> ---
>   qemu-nbd.c | 28 +---
>   1 file changed, 9 insertions(+), 19 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 96c0829970c..4670b659167 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -546,9 +546,8 @@ int main(int argc, char **argv)
>   };
>   int ch;
>   int opt_ind = 0;
> -char *end;
>   int flags = BDRV_O_RDWR;
> -int partition = -1;
> +int partition = 0;
>   int ret = 0;
>   bool seen_cache = false;
>   bool seen_discard = false;
> @@ -660,9 +659,8 @@ int main(int argc, char **argv)
>   port = optarg;
>   break;
>   case 'o':
> -dev_offset = strtoll (optarg, &end, 0);
> -if (*end) {
> -error_report("Invalid offset `%s'", optarg);
> +if (qemu_strtou64(optarg, NULL, 0, &dev_offset) < 0) {
> +error_report("Invalid offset '%s'", optarg);
>   exit(EXIT_FAILURE);
>   }
>   break;
> @@ -684,13 +682,9 @@ int main(int argc, char **argv)
>   flags &= ~BDRV_O_RDWR;
>   break;
>   case 'P':
> -partition = strtol(optarg, &end, 0);
> -if (*end) {
> -error_report("Invalid partition `%s'", optarg);
> -exit(EXIT_FAILURE);
> -}
> -if (partition < 1 || partition > 8) {
> -error_report("Invalid partition %d", partition);
> +if (qemu_strtoi(optarg, NULL, 0, &partition) < 0 ||

we can use unsigned conversion like for offset (and unsigned type for 
partition), but this doesn't really matter.

> +partition < 1 || partition > 8) {
> +error_report("Invalid partition '%s'", optarg);
>   exit(EXIT_FAILURE);
>   }
>   break;
> @@ -711,15 +705,11 @@ int main(int argc, char **argv)
>   device = optarg;
>   break;
>   case 'e':
> -shared = strtol(optarg, &end, 0);
> -if (*end) {
> +if (qemu_strtoi(optarg, NULL, 0, &shared) < 0 ||

and here

> +shared < 1) {
>   error_report("Invalid shared device number '%s'", optarg);
>   exit(EXIT_FAILURE);
>   }
> -if (shared < 1) {
> -error_report("Shared device number must be greater than 0");
> -exit(EXIT_FAILURE);
> -}
>   break;
>   case 'f':
>   fmt = optarg;
> @@ -1007,7 +997,7 @@ int main(int argc, char **argv)
>   }
>   fd_size -= dev_offset;
> 
> -if (partition != -1) {
> +if (partition) {
>   int64_t limit;
> 
>   if (dev_offset) {
> 

anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy 

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PULL 00/44] pci, pc, virtio: fixes, features

2019-01-15 Thread Michael S. Tsirkin

On Tue, Jan 15, 2019 at 01:38:02PM +0800, Peter Xu wrote:
> On Mon, Jan 14, 2019 at 08:35:11PM -0500, Michael S. Tsirkin wrote:
> > The following changes since commit 89bd861c2b470e3fb45596945509079c72af3ac2:
> > 
> >   Merge remote-tracking branch 
> > 'remotes/ehabkost/tags/x86-next-pull-request' into staging (2019-01-14 
> > 17:35:00 +)
> > 
> > are available in the Git repository at:
> > 
> >   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
> > 
> > for you to fetch changes up to b421506a3ac2f1b2a4f18d6f423a92dfa16e2645:
> > 
> >   acpi: update expected files (2019-01-14 19:31:05 -0500)
> > 
> > 
> > pci, pc, virtio: fixes, features
> > 
> > tpm physical presence interface
> > rsc support in virtio net
> > ivshmem is removed
> > misc cleanups and fixes all over the place
> 
> Hi, Michael,
> 
> Do you want to review/queue some VT-d patches that I posted recently?
> 
> [PATCH 0/5] intel_iommu: misc fixes for error exposed after 
> error_report_once()
> (https://patchwork.kernel.org/cover/10751913/, patch 5 dropped though)
> 
> They fix some bugs that were recently exposed.  Currently only the
> first two patches got acked-by from Jason.
> 
> They don't worth to block the pull but IMHO they fix real problems so
> just to make sure they won't fall through the cracks.
> 
> Thanks!
> 
> -- 
> Peter Xu

Assuming you have arrived at a consensus with Alex, pls
post a series that either doesn't affect vfio or
has his ack.

Thanks!

-- 
MST

Re: [Qemu-devel] [PATCH v4 for-4.0 2/7] vhost-user: Support transferring inflight buffer between qemu and backend

2019-01-15 Thread Michael S. Tsirkin

On Tue, Jan 15, 2019 at 02:46:42PM +0800, Yongji Xie wrote:
> On Tue, 15 Jan 2019 at 06:25, Michael S. Tsirkin  wrote:
> >
> > On Wed, Jan 09, 2019 at 07:27:23PM +0800, elohi...@gmail.com wrote:
> > > @@ -382,6 +397,30 @@ If VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD protocol 
> > > feature is negotiated,
> > >  slave can send file descriptors (at most 8 descriptors in each message)
> > >  to master via ancillary data using this fd communication channel.
> > >
> > > +Inflight I/O tracking
> > > +-
> > > +
> > > +To support slave reconnecting, slave need to track inflight I/O in a
> > > +shared memory. VHOST_USER_GET_INFLIGHT_FD and VHOST_USER_SET_INFLIGHT_FD
> > > +are used to transfer the memory between master and slave. And to 
> > > encourage
> > > +consistency, we provide a recommended format for this memory:
> >
> > I think we should make a stronger statement and actually
> > just say what the format is. Not recommend it weakly.
> >
> 
> Okey, will do it.
> 
> > > +
> > > +offsetwidthdescription
> > > +0x0  0x400region for queue0
> > > +0x4000x400region for queue1
> > > +0x8000x400region for queue2
> > > +...  ...  ...
> > > +
> > > +For each virtqueue, we have a 1024 bytes region.
> >
> >
> > Why is the size hardcoded? Why not a function of VQ size?
> >
> 
> Sorry, I didn't get your point. Should the region's size be fixed? Do
> you mean we need to document a function for the region's size?


Well you are saying 0x0 to 0x400 is for queue0.
How do you know that's enough? And why are 0x400
bytes necessary? After all max queue size can be very small.



> >
> > > The region's format is like:
> > > +
> > > +offset   widthdescription
> > > +0x0  0x1  descriptor 0 is in use or not
> > > +0x1  0x1  descriptor 1 is in use or not
> > > +0x2  0x1  descriptor 2 is in use or not
> > > +...  ...  ...
> > > +
> > > +For each descriptor, we use one byte to specify whether it's in use or 
> > > not.
> > > +
> > >  Protocol features
> > >  -
> > >
> >
> > I think that it's a good idea to have a version in this region.
> > Otherwise how are you going to handle compatibility when
> > this needs to be extended?
> >
> 
> I have put the version into the message's payload: VhostUserInflight. Is it 
> OK?
> 
> Thanks,
> Yongji

I'm not sure I like it.  So is qemu expected to maintain it? Reset it?
Also don't you want to be able to detect that qemu has reset the buffer?
If we have version 1 at a known offset that can serve both purposes.
Given it only has value within the buffer why not store it there?

-- 
MST

Re: [Qemu-devel] [PATCH for-4.0 v9 09/16] qemu_thread: supplement error handling for pci_edu_realize

2019-01-15 Thread Markus Armbruster

Fei Li  writes:

> 在 2019/1/14 下午8:36, Markus Armbruster 写道:
>> Fei Li  writes:
>>
>>> Just to make sure about how to do the cleanup. I notice that in 
>>> device_set_realized(),
>>> the current code does not call "dc->unrealize(dev, NULL);" when 
>>> dc->realize() fails.
> Sorry that I am still uncertain.. I guess the code below I pasted was
> misleading,
> actually I want to stress the *dc->unrealize() is not called when
> dc->realize() fails*
> and the incomplete below "goto fail" does not include the dc->unrealize(),
> but instead the dc->unrealize() is included in later
> child_realize_fail: & post_realize_fail:.
>
>
> Emm, IMHO, I think when dc->realize() fails, the dc->unrealize() is
> either should be
> called in the common function: device_set_realized() in a unified way,
> that is
>
>     if (local_err != NULL) {
> +  if (dc->unrealize) {
> +  dc->unrealize(dev, local_err);
> +  }
>     goto fail;
>     }
>
> or do the unrealize() locally for each device earlier when
> dc->realize() fails.
>
> But I checked several dc->realize() function, they did not call unrealize()
> when fails. Besides, it may mean verbose code if unrealize() locally.
> Thus I think the above way is the right way to do the cleanup when
> realize() fails.

The realize() method is specified to either succeed completely or fail
completely, i.e. fail and do nothing.  The "either succeed completely or
fail completely" aspect of the specification is sane and perfectly
ordinary.

How a concrete implementation accomplishes "fail completely" is up to
the implementation.

An implementation may choose to structure its FOO_realize() and
FOO_unrealize() in a way that lets FOO_realize() call FOO_unrealize() to
clean up on failure.

An implementation may also choose to clean up differently.

This freedom of choice is by design.

Changing the specification now would involve auditing and updating all
realize() and unrealize() methods.  Not going to happen without an
extremely compelling reason.

>>>
>>>      if (dc->realize) {
>>>      dc->realize(dev, &local_err);
>>>      }
>>>
>>>      if (local_err != NULL) {
>>>      goto fail;
>>>      }
>>>
>>> Is this on purpose? (Maybe due to some devices' realize() do their own 
>>> cleanup
>>> when fails? Sorry for the unsure, it is such a common function that I did 
>>> not
>>> check all. :( ) Or else, I prefer to do the cleanup in a unified manner, 
>>> e.g. call "dc->unrealize(dev, NULL);" which is the pci_qdev_unrealize() for 
>>> pci devices.
>> Yes, this is on purpose.
>>
>> When a realize() method fails, it must revert everything it has done so
>> far.  Results in sane "either succeed completely, or fail and do
>> nothing" semantics.
>
> Have a nice day, thanks
>
> Fei

Re: [Qemu-devel] [PATCH 03/15] hw/ssi: Remove SSIBus from "qemu/typedefs.h"

2019-01-15 Thread Thomas Huth

On 2019-01-15 13:28, Markus Armbruster wrote:
> Philippe Mathieu-Daudé  writes:
> 
>> From: Philippe Mathieu-Daudé 
>>
>> There are only three files requiring this typedef, let them
>> include "hw/ssi/ssi.h" directly to simplify "qemu/typedefs.h".
>>
>> To clean "qemu/typedefs.h", move the forward declaration
>> to "hw/ssi/ssi.h".
>>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  hw/arm/strongarm.h  | 1 +
>>  include/hw/arm/pxa.h| 1 +
>>  include/hw/ssi/pl022.h  | 1 +
>>  include/hw/ssi/ssi.h| 1 +
>>  include/qemu/typedefs.h | 1 -
>>  5 files changed, 4 insertions(+), 1 deletion(-)
> 
> When typedefs.h changes, we recompile the world, but it pretty much only
> ever changes when new typedefs are added.  Thus, *keeping* a typedef
> there is therefore pretty cheap.
> 
> Nevertheless, we shouldn't keep typedefs there without a real reason.
> Being able to move one away without having to add any new #include
> directives is a strong sign for "no real reason".  I like patches doing
> that.
> 
> What I don't like is adding #include directives just so you can move
> typedefs out of typedefs.h: it slows down the build.  Granted, the four
> added by this patch are a drop in the bucket.  The point I'm trying to
> make is typedefs.h's purpose: it's for avoiding #include directives.
> Circular ones in particular, but others, too.

Yes, agreed, removing things from typedefs.h just to add lots of
#includes in other files is not really the best idea. I also dropped
this patch in v2 of my current PULL request because of this reason.
Phil, I suggest to simply drop this patch. What we maybe could do is to
split up typedefs.h per subsystem, so that we additionally have a
hw/arm/typedefs.h, hw/ppc/typedefs.h etc. in the end, then the
target-specific typedefs would not clutter the common qemu/typedefs.h
file anymore.

 Thomas

Re: [Qemu-devel] [PATCH 1/2] vfio-pci: Introduce vfio_register_event_notifier helper

2019-01-15 Thread Auger Eric

Hi Cornelia,

On 1/15/19 1:03 PM, Cornelia Huck wrote:
> On Fri, 11 Jan 2019 17:58:00 +0100
> Eric Auger  wrote:
> 
>> The code used to attach the eventfd handler for the ERR and
>> REQ irq indices can be factorized into a helper. In subsequent
>> patches we will extend this helper to support other irq indices.
> 
> Looks like a nice refactoring to me.
thank you
> 
>>
>> We test the notification is allowed outside of the helper:
> 
> s/We test/We test whether/
> 
>> respectively check vdev->pci_aer and VFIO_FEATURE_ENABLE_REQ.
>> Depending on the returned value we set vdev->pci_aer and
>> vdev->req_enabled. An error handle is introduced for future usage
>> although not strictly useful here.
>>
>> Signed-off-by: Eric Auger 
>> ---
>>  hw/vfio/pci.c | 291 ++
>>  1 file changed, 127 insertions(+), 164 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index c0cb1ec289..c589a4e666 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -105,6 +105,95 @@ static void vfio_intx_eoi(VFIODevice *vbasedev)
>>  vfio_unmask_single_irqindex(vbasedev, VFIO_PCI_INTX_IRQ_INDEX);
>>  }
>>  
>> +/*
>> + * vfio_register_event_notifier - setup/tear down eventfd
>> + * notification and handling for IRQ indices that span over
>> + * a single IRQ
>> + *
>> + * @vdev: VFIO device handle
>> + * @index: IRQ index the eventfd/handler is associated to
> 
> s/to/with/ ?
> 
>> + * @target_state: true means notifier needs to be set up
>> + * @handler to attach if @target_state is true
>> + * @errp error handle
>> + */
>> +static int vfio_register_event_notifier(VFIOPCIDevice *vdev,
>> +int index,
>> +bool target_state,
>> +void (*handler)(void *opaque),
>> +Error **errp)
> 
> (...)
> 
>> @@ -3069,8 +2998,29 @@ static void vfio_realize(PCIDevice *pdev, Error 
>> **errp)
>>  goto out_teardown;
>>  }
>>  
>> -vfio_register_err_notifier(vdev);
>> -vfio_register_req_notifier(vdev);
>> +if (vdev->pci_aer) {
>> +/*
>> + * Registers error notifier for devices supporting error recovery.
>> + * If we encounter a failure in this function, we report an error
> 
> s/in this function/while registering it/ ?
> 
>> + * and continue after disabling error recovery support for the
>> + * device.
>> + */
>> +vdev->pci_aer =
>> +!vfio_register_event_notifier(vdev, VFIO_PCI_ERR_IRQ_INDEX, 
>> true,
>> +  vfio_err_notifier_handler, &err);
>> +if (err) {
>> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>> +}
> 
> I think you need to reset err to NULL if you want to reuse the variable.
Yes you're right. Thanks for spotting this.

> 
> Alternatively, you could keep the wrappers and define a local error
> variable there.
> 
>> +}
>> +
>> +if (vdev->features & VFIO_FEATURE_ENABLE_REQ) {
>> +vdev->req_enabled =
>> +!vfio_register_event_notifier(vdev, VFIO_PCI_REQ_IRQ_INDEX, 
>> true,
>> +  vfio_req_notifier_handler, &err);
>> +if (err) {
>> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>> +}
>> +}
>>  vfio_setup_resetfn_quirk(vdev);
>>  
>>  return;
>> @@ -3106,9 +3056,22 @@ static void vfio_instance_finalize(Object *obj)
>>  static void vfio_exitfn(PCIDevice *pdev)
>>  {
>>  VFIOPCIDevice *vdev = PCI_VFIO(pdev);
>> +Error *err = NULL;
>>  
>> -vfio_unregister_req_notifier(vdev);
>> -vfio_unregister_err_notifier(vdev);
>> +if (vdev->req_enabled) {
>> +vfio_register_event_notifier(vdev, VFIO_PCI_REQ_IRQ_INDEX,
>> + false, NULL, &err);
>> +if (err) {
>> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>> +}
> 
> Likewise.

Thank you for the review!

Eric

> 
>> +}
>> +if (vdev->pci_aer) {
>> +vfio_register_event_notifier(vdev, VFIO_PCI_ERR_IRQ_INDEX,
>> + false, NULL, &err);
>> +if (err) {
>> +warn_reportf_err(err, VFIO_MSG_PREFIX, vdev->vbasedev.name);
>> +}
>> +}
>>  pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
>>  vfio_disable_interrupts(vdev);
>>  if (vdev->intx.mmap_timer) {
> 
>

Re: [Qemu-devel] [PATCH 2/2] vfio-pci: Use vfio_register_event_notifier in vfio_intx_enable_kvm

2019-01-15 Thread Auger Eric

Hi Cornelia,

On 1/15/19 1:12 PM, Cornelia Huck wrote:
> On Fri, 11 Jan 2019 17:58:01 +0100
> Eric Auger  wrote:
> 
>> We can also use vfio_register_event_notifier() helper in
>> vfio_intx_enable_kvm to set the signalling associated to
>> VFIO_PCI_INTX_IRQ_INDEX.
>>
>> Signed-off-by: Eric Auger 
>> ---
>>  hw/vfio/pci.c | 38 +++---
>>  1 file changed, 7 insertions(+), 31 deletions(-)
>>
> 
>>  static void vfio_intx_disable(VFIOPCIDevice *vdev)
> 
> I'm wondering why the _disable path can't use the new helper. Ordering
> issues?
> 
Yes the interleaving of actions scared me a little bit. I am going to
study this a little bit further ...

Thanks

Eric

Re: [Qemu-devel] [PATCH v1] virtio: add checks for the size of the indirect table

2019-01-15 Thread Dima Stepanov

On Tue, Jan 15, 2019 at 11:40:09AM +0100, Cornelia Huck wrote:
> On Tue, 15 Jan 2019 13:08:47 +0300
> Dima Stepanov  wrote:
> 
> > The virtqueue_pop() and virtqueue_get_avail_bytes() routines can use the
> > INDIRECT table to get the data. It is possible to create a packet which
> > will lead to the assert message like:
> >   include/exec/memory.h:1995: void
> >   address_space_read_cached(MemoryRegionCache *, hwaddr, void *, int):
> >   Assertion `addr < cache->len && len <= cache->len - addr' failed.
> >   Aborted
> > To do it the first descriptor should have a link to the INDIRECT table
> > and set the size of it to 0. It doesn't look good that the guest should
> > be able to trigger the assert in qemu. Add additional check for the size
> > of the INDIRECT table, which should not be 0.
> 
> Ouch, being able to crash QEMU by a specially crafted descriptor is bad.
> 
> Looking at the virtio spec, we don't seem to explicitly disallow
> indirect descriptors with a zero-length table. So, as an alternative to
> marking the device broken, we could also skip over such a descriptor.
> Not sure whether that makes sense, though.

Hard to say what is the better option here: mark device with the
VIRTIO_CONFIG_S_NEEDS_RESET bit or just skip the descriptor. Right now
all the parsing errors are handled using the virtio_error() call. The
possible parsing errors are: wrong address, looped descriptors, invalid
size, incorrect order and so on. Some of those errors are not described
in the virtio spec. So it looks like that this error should be also
handled by calling virtio_error().

> 
> > 
> > Signed-off-by: Dima Stepanov 
> > ---
> >  hw/virtio/virtio.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 22bd1ac..a1ff647 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -646,7 +646,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
> > int *in_bytes,
> >  vring_desc_read(vdev, &desc, desc_cache, i);
> >  
> >  if (desc.flags & VRING_DESC_F_INDIRECT) {
> > -if (desc.len % sizeof(VRingDesc)) {
> > +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
> >  virtio_error(vdev, "Invalid size for indirect buffer 
> > table");
> >  goto err;
> >  }
> > @@ -902,7 +902,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
> >  desc_cache = &caches->desc;
> >  vring_desc_read(vdev, &desc, desc_cache, i);
> >  if (desc.flags & VRING_DESC_F_INDIRECT) {
> > -if (desc.len % sizeof(VRingDesc)) {
> > +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
> >  virtio_error(vdev, "Invalid size for indirect buffer table");
> >  goto done;
> >  }
>

Re: [Qemu-devel] [PULL v2 00/27] ivshmem deprecation, qtests, typedefs and gnu99

On Mon, 14 Jan 2019 at 17:45, Thomas Huth  wrote:
>
>  Hi Peter!
>
> The following changes since commit 7260438b7056469610ee166f7abe9ff8a26b8b16:
>
>   Merge remote-tracking branch 
> 'remotes/palmer/tags/riscv-for-master-3.2-part2' into staging (2019-01-14 
> 11:41:43 +)
>
> are available in the git repository at:
>
>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-01-14v2
>
> for you to fetch changes up to 650db715681ad1a042705484776e1974f288f3d4:
>
>   tests/hexloader-test: Don't pass -nographic to the QEMU under test 
> (2019-01-14 18:21:29 +0100)
>
> 
> - Remove deprecated "ivshmem" legacy device
> - Bug fix for vhost-user-test
> - Use more CONFIG Makefile switches for qtests
> - Get rid of global_qtests in some more qtests
> - typedef cleanups
> - Fixes for compiling with Clang
> - Force C standard to gnu99
> 

Hi; another compile failure on that ppc64 system, I'm afraid:

/home/pm215/qemu/qemu-seccomp.c:45:1: error: initializer element is not constant
 };
 ^
/home/pm215/qemu/qemu-seccomp.c:45:1: error: (near initialization for
‘sched_setscheduler_arg[0]’)

(I did a quick check with 'make -k' and it looks like there aren't
any more lurking after that one.)

The system libseccomp is libseccomp-2.3.1-3.el7.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 10/19] nbd/client: Split out nbd_send_one_meta_context()

12.01.2019 20:58, Eric Blake wrote:
> Refactor nbd_negotiate_simple_meta_context() to pull out the
> code that can be reused to send a LIST request for 0 or 1 query.
> No semantic change.  The old comment about 'sizeof(uint32_t)'
> being equivalent to '/* number of queries */' is no longer
> needed, now that we are computing 'sizeof(queries)' instead.
> 
> Signed-off-by: Eric Blake 
> Message-Id: <20181215135324.152629-14-ebl...@redhat.com>
> Reviewed-by: Richard W.M. Jones 
> 
> ---
> v3: Improve commit message [Rich], formatting tweak [checkpatch],
> rebase to dropped patch
> ---
>   nbd/client.c | 67 +---
>   nbd/trace-events |  2 +-
>   2 files changed, 48 insertions(+), 21 deletions(-)
> 
> diff --git a/nbd/client.c b/nbd/client.c
> index 77993890f04..3c716be2719 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -629,6 +629,49 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
>   return QIO_CHANNEL(tioc);
>   }
> 
> +/*
> + * nbd_send_one_meta_context:
> + * Send 0 or 1 set/list meta context queries.
> + * Return 0 on success, -1 with errp set for any error
> + */
> +static int nbd_send_one_meta_context(QIOChannel *ioc,
> + uint32_t opt,
> + const char *export,
> + const char *query,
> + Error **errp)
> +{
> +int ret;
> +uint32_t export_len = strlen(export);
> +uint32_t queries = !!query;

n_ or nb_ prefix may make it more clear

> +uint32_t context_len = 0;
> +uint32_t data_len;
> +char *data;
> +char *p;
> +
> +data_len = sizeof(export_len) + export_len + sizeof(queries);
> +if (query) {
> +context_len = strlen(query);

looks like it then should be query_len

> +data_len += sizeof(context_len) + context_len;
> +} else {
> +assert(opt == NBD_OPT_LIST_META_CONTEXT);
> +}
> +data = g_malloc(data_len);
> +p = data;

may use p = data = g_malloc

> +
> +trace_nbd_opt_meta_request(nbd_opt_lookup(opt), query ?: "(all)", 
> export);
> +stl_be_p(p, export_len);
> +memcpy(p += sizeof(export_len), export, export_len);
> +stl_be_p(p += export_len, queries);
> +if (query) {
> +stl_be_p(p += sizeof(uint32_t), context_len);

:), aha, please, s/uint32_t/queries, as you promised

Hmm, its my code. It's hard to read and not very comfortable to maintain..

In block/nbd-client.c we have
payload_advance* functions, to read such formatted data, I think, it should be
good to make something like this for server-part. Not about these series, of 
course.

Interesting, troubles around "don't use be64_to_cpuS, use only be64_to_cpu",
do they apply somehow to *_be_p functions family?

> +memcpy(p += sizeof(context_len), query, context_len);
> +}
> +
> +ret = nbd_send_option_request(ioc, opt, data_len, data, errp);
> +g_free(data);
> +return ret;
> +}
> +
>   /* nbd_negotiate_simple_meta_context:
>* Request the server to set the meta context for export @info->name
>* using @info->x_dirty_bitmap with a fallback to "base:allocation",
> @@ -653,26 +696,10 @@ static int nbd_negotiate_simple_meta_context(QIOChannel 
> *ioc,
>   NBDOptionReply reply;
>   const char *context = info->x_dirty_bitmap ?: "base:allocation";
>   bool received = false;
> -uint32_t export_len = strlen(info->name);
> -uint32_t context_len = strlen(context);
> -uint32_t data_len = sizeof(export_len) + export_len +
> -sizeof(uint32_t) + /* number of queries */
> -sizeof(context_len) + context_len;
> -char *data = g_malloc(data_len);
> -char *p = data;
> 
> -trace_nbd_opt_meta_request(context, info->name);
> -stl_be_p(p, export_len);
> -memcpy(p += sizeof(export_len), info->name, export_len);
> -stl_be_p(p += export_len, 1);
> -stl_be_p(p += sizeof(uint32_t), context_len);
> -memcpy(p += sizeof(context_len), context, context_len);
> -
> -ret = nbd_send_option_request(ioc, NBD_OPT_SET_META_CONTEXT, data_len, 
> data,
> -  errp);
> -g_free(data);
> -if (ret < 0) {
> -return ret;
> +if (nbd_send_one_meta_context(ioc, NBD_OPT_SET_META_CONTEXT,
> +  info->name, context, errp) < 0) {
> +return -1;
>   }
> 
>   if (nbd_receive_option_reply(ioc, NBD_OPT_SET_META_CONTEXT, &reply,
> @@ -689,7 +716,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel 
> *ioc,
>   if (reply.type == NBD_REP_META_CONTEXT) {
>   char *name;
> 
> -if (reply.length != sizeof(info->context_id) + context_len) {
> +if (reply.length != sizeof(info->context_id) + strlen(context)) {
>   error_setg(errp, "Failed to negotiate meta context '%s', server 
> "
>  "answered with unexpected length %

[Qemu-devel] Crash when deleting an iothread that is being used

2019-01-15 Thread Alberto Garcia

Here's how to reproduce the crash:

{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add", "arguments": {"driver": "null-co", "node-name": 
"hd0"}}
{ "execute": "object-add", "arguments": {"qom-type": "iothread", "id": 
"iothread0"}}
{ "execute": "x-blockdev-set-iothread", "arguments": {"node-name": "hd0", 
"iothread": "iothread0"}}
{ "execute": "object-del", "arguments": {"id": "iothread0"}}
{ "execute": "blockdev-del", "arguments": {"node-name": "hd0"}}

The problem is that bs->aio_context is the one that belonged to the
IOThread and was destroyed by the object-del call. One would need to
do x-blockdev-set-iothread(hd0, null) before deleting the thread.

The IOThread class does not have a can_be_deleted() method to prevent
threads from being deleted. One possible implementation would require
a reference count but that doesn't seem immediately trivial because
users don't use the IOThread itself but its AioContext, and not all
bdrv_set_aio_context() are related to IOThreads.

A quicker fix is of course to prevent the threads from being deleted
at all :-)

Berto

Re: [Qemu-devel] [PATCH v1] virtio: add checks for the size of the indirect table

On Tue, 15 Jan 2019 16:11:19 +0300
Dima Stepanov  wrote:

> On Tue, Jan 15, 2019 at 11:40:09AM +0100, Cornelia Huck wrote:
> > On Tue, 15 Jan 2019 13:08:47 +0300
> > Dima Stepanov  wrote:
> >   
> > > The virtqueue_pop() and virtqueue_get_avail_bytes() routines can use the
> > > INDIRECT table to get the data. It is possible to create a packet which
> > > will lead to the assert message like:
> > >   include/exec/memory.h:1995: void
> > >   address_space_read_cached(MemoryRegionCache *, hwaddr, void *, int):
> > >   Assertion `addr < cache->len && len <= cache->len - addr' failed.
> > >   Aborted
> > > To do it the first descriptor should have a link to the INDIRECT table
> > > and set the size of it to 0. It doesn't look good that the guest should
> > > be able to trigger the assert in qemu. Add additional check for the size
> > > of the INDIRECT table, which should not be 0.  
> > 
> > Ouch, being able to crash QEMU by a specially crafted descriptor is bad.
> > 
> > Looking at the virtio spec, we don't seem to explicitly disallow
> > indirect descriptors with a zero-length table. So, as an alternative to
> > marking the device broken, we could also skip over such a descriptor.
> > Not sure whether that makes sense, though.  
> 
> Hard to say what is the better option here: mark device with the
> VIRTIO_CONFIG_S_NEEDS_RESET bit or just skip the descriptor. Right now
> all the parsing errors are handled using the virtio_error() call. The
> possible parsing errors are: wrong address, looped descriptors, invalid
> size, incorrect order and so on. Some of those errors are not described
> in the virtio spec. So it looks like that this error should be also
> handled by calling virtio_error().

virtio_error() is certainly the safe option (and easiest to implement),
and handling weird descriptors is probably not worth the time.

FWIW,

Reviewed-by: Cornelia Huck 

Should this be cc:stable, as it is a guest-triggerable crash?

> 
> >   
> > > 
> > > Signed-off-by: Dima Stepanov 
> > > ---
> > >  hw/virtio/virtio.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > index 22bd1ac..a1ff647 100644
> > > --- a/hw/virtio/virtio.c
> > > +++ b/hw/virtio/virtio.c
> > > @@ -646,7 +646,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, 
> > > unsigned int *in_bytes,
> > >  vring_desc_read(vdev, &desc, desc_cache, i);
> > >  
> > >  if (desc.flags & VRING_DESC_F_INDIRECT) {
> > > -if (desc.len % sizeof(VRingDesc)) {
> > > +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
> > >  virtio_error(vdev, "Invalid size for indirect buffer 
> > > table");
> > >  goto err;
> > >  }
> > > @@ -902,7 +902,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
> > >  desc_cache = &caches->desc;
> > >  vring_desc_read(vdev, &desc, desc_cache, i);
> > >  if (desc.flags & VRING_DESC_F_INDIRECT) {
> > > -if (desc.len % sizeof(VRingDesc)) {
> > > +if (!desc.len || (desc.len % sizeof(VRingDesc))) {
> > >  virtio_error(vdev, "Invalid size for indirect buffer table");
> > >  goto done;
> > >  }  
> >

Re: [Qemu-devel] [PATCH v2] s390x/pci: Set the iommu region size mpcifc request