Re: [Qemu-devel] [PATCH v2] target-ppc: Add MMU model check for booke machines

[Edgar E. Iglesias]

On Wed, Jan 25, 2017 at 02:50:30PM +, Valentin Plotkin wrote:
> Machines bamboo, e500 and virtex-ml507 assume a certain MMU model,
> otherwise resulting in unpredictable behavior. Add apropriate checks
> into *_init functions.

The Virtex part looks OK.
I couldn't apply the patch with git am though...



> 
> Signed-off-by: Valentin 
> ---
>  hw/ppc/e500.c  | 6 ++
>  hw/ppc/ppc440_bamboo.c | 6 ++
>  hw/ppc/virtex_ml507.c  | 7 +++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index cf8b122..5b1958c 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -827,6 +827,12 @@ void ppce500_init(MachineState *machine, PPCE500Params 
> *params)
>  env = &cpu->env;
>  cs = CPU(cpu);
> 
> +if (env->mmu_model != POWERPC_MMU_BOOKE206) {
> +fprintf(stderr, "MMU model %i not supported by this machine.\n",
> +env->mmu_model);
> +exit(1);
> +}
> +
>  if (!firstenv) {
>  firstenv = env;
>  }
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 5c535b1..9d997bf 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -193,6 +193,12 @@ static void bamboo_init(MachineState *machine)
>  }
>  env = &cpu->env;
> 
> +if (env->mmu_model != POWERPC_MMU_BOOKE) {
> +fprintf(stderr, "MMU model %i not supported by this machine.\n",
> +env->mmu_model);
> +exit(1);
> +}
> +
>  qemu_register_reset(main_cpu_reset, cpu);
>  ppc_booke_timers_init(cpu, 4, 0);
>  ppc_dcr_init(env, NULL, NULL);
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index b97d966..fdbcf22 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -221,6 +221,13 @@ static void virtex_init(MachineState *machine)
> 
>  cpu = ppc440_init_xilinx(&ram_size, 1, machine->cpu_model, 4);
>  env = &cpu->env;
> +
> +if (env->mmu_model != POWERPC_MMU_BOOKE) {
> +fprintf(stderr, "MMU model %i not supported by this machine.\n",
> +env->mmu_model);
> +exit(1);
> +}
> +
>  qemu_register_reset(main_cpu_reset, cpu);
> 
>  memory_region_allocate_system_memory(phys_ram, NULL, "ram", ram_size);
> -- 
> 2.5.5
> 
> 
> This fixes 'qemu-system-ppc -nographic -cpu G2leGP3 -M ppce500; bug from
> BiteSizedTasks.
> 
> Version 2: fixed style, moved checks from mmubooke_create_initial_mapping
> to *_init. Thanks to Thomas Huth .

Re: [Qemu-devel] [PATCH v2 30/30] target-sparc: fix up niagara machine

[Markus Armbruster]

niagara_init() does something naughty, which conflicts with Max's
"[PATCH v6 0/9] block: Drop BDS.filename".  Details inline.

Artyom Tarasenko  writes:

> Remove the Niagara stub implementation from sun4u.c and add a machine,
> compatible with Legion simulator from the OpenSPARC T1 project.
>
> The machine uses the firmware supplied with the OpenSPARC T1 project,
> http://download.oracle.com/technetwork/systems/opensparc/OpenSPARCT1_Arch.1.5.tar.bz2
> in the directory S10image/, and is able to boot the supplied Solaris 10 image.
>
> Note that for compatibility with the naming conventions for SPARC machines
> the new machine name is lowercase niagara.
>
> Signed-off-by: Artyom Tarasenko 
> Reviewed-by: Richard Henderson 
> ---
>  MAINTAINERS |  13 +--
>  default-configs/sparc64-softmmu.mak |   2 +
>  hw/sparc64/Makefile.objs|   1 +
>  hw/sparc64/niagara.c| 177 
> 
>  hw/sparc64/sun4u.c  |  31 ---
>  qemu-doc.texi   |  14 ++-
>  6 files changed, 199 insertions(+), 39 deletions(-)
>  create mode 100644 hw/sparc64/niagara.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 54588e5..b5ebfab 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -725,6 +725,13 @@ S: Maintained
>  F: hw/sparc64/sun4u.c
>  F: pc-bios/openbios-sparc64
>  
> +Sun4v
> +M: Artyom Tarasenko 
> +S: Maintained
> +F: hw/sparc64/sun4v.c
> +F: hw/timer/sun4v-rtc.c
> +F: include/hw/timer/sun4v-rtc.h
> +
>  Leon3
>  M: Fabien Chouteau 
>  S: Maintained
> @@ -1098,12 +1105,6 @@ F: hw/nvram/chrp_nvram.c
>  F: include/hw/nvram/chrp_nvram.h
>  F: tests/prom-env-test.c
>  
> -sun4v RTC
> -M: Artyom Tarasenko 
> -S: Maintained
> -F: hw/timer/sun4v-rtc.c
> -F: include/hw/timer/sun4v-rtc.h
> -
>  Subsystems
>  --
>  Audio
> diff --git a/default-configs/sparc64-softmmu.mak 
> b/default-configs/sparc64-softmmu.mak
> index c0cdd64..c581e61 100644
> --- a/default-configs/sparc64-softmmu.mak
> +++ b/default-configs/sparc64-softmmu.mak
> @@ -13,3 +13,5 @@ CONFIG_IDE_CMD646=y
>  CONFIG_PCI_APB=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
> +CONFIG_EMPTY_SLOT=y
> +CONFIG_SUN4V_RTC=y
> diff --git a/hw/sparc64/Makefile.objs b/hw/sparc64/Makefile.objs
> index a96b1f8..cf9de21 100644
> --- a/hw/sparc64/Makefile.objs
> +++ b/hw/sparc64/Makefile.objs
> @@ -1,2 +1,3 @@
>  obj-y += sparc64.o
>  obj-y += sun4u.o
> +obj-y += niagara.o
> \ No newline at end of file
> diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
> new file mode 100644
> index 000..b55d4bb
> --- /dev/null
> +++ b/hw/sparc64/niagara.c
> @@ -0,0 +1,177 @@
> +/*
> + * QEMU Sun4v/Niagara System Emulator
> + *
> + * Copyright (c) 2016 Artyom Tarasenko
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "hw/hw.h"
> +#include "hw/boards.h"
> +#include "hw/char/serial.h"
> +#include "hw/empty_slot.h"
> +#include "hw/loader.h"
> +#include "hw/sparc/sparc64.h"
> +#include "hw/timer/sun4v-rtc.h"
> +#include "exec/address-spaces.h"
> +#include "sysemu/block-backend.h"
> +
> +
> +typedef struct NiagaraBoardState {
> +MemoryRegion hv_ram;
> +MemoryRegion partition_ram;
> +MemoryRegion nvram;
> +MemoryRegion md_rom;
> +MemoryRegion hv_rom;
> +MemoryRegion vdisk_ram;
> +MemoryRegion prom;
> +} NiagaraBoardState;
> +
> +#define NIAGARA_HV_RAM_BASE 0x10ULL
> +#define NIAGARA_HV_RAM_SIZE 0x3f0ULL /* 63 MiB */
> +
> +#define NIAGARA_PARTITION_RAM_BASE 0x8000ULL
> +
> +#define NIAGARA_UART_BASE   0x1f1000ULL
> +
> +#define NIAGARA_NVRAM_BASE  0x1f1100ULL
> +#define NIAGARA_NVRAM_SIZE  0x2000
> +
> +#define NIAGARA_MD_ROM_BASE 0x1f1200ULL
> +#define NIAGARA_MD_ROM_SIZE 0x2000
> +
> +#define NIAGARA_HV_ROM_BASE 0x1f1208ULL
> +#d

Re: [Qemu-devel] [PATCH v5 00/18] VT-d: vfio enablement and misc enhances

[Peter Xu]

On Wed, Jan 25, 2017 at 04:26:59PM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 25, 2017 at 12:48:37PM +0800, Peter Xu wrote:
> > On Tue, Jan 24, 2017 at 04:48:49PM +0200, Michael S. Tsirkin wrote:
> > > On Tue, Jan 24, 2017 at 07:49:10PM +0800, Peter Xu wrote:
> > > > On Tue, Jan 24, 2017 at 06:25:53PM +0800, Peter Xu wrote:
> > > > > This is v5 of vt-d vfio enablement series.
> > > > > 
> > > > > Most of the changes in v5 is not functionally, but related to
> > > > > comments, error_report()s, debugging, squashing patches, etc. (which
> > > > > are confirmed changes in v4), besides a tiny tweak when unmapping a
> > > > > whole address space (please see below changelog). There are still
> > > > > discussions in v4 thread, we can just continue there (or here), and
> > > > > from this version I'll remove RFC tag.
> > > > > 
> > > > > I didn't rebase to master since current master failed to build on my
> > > > > laptop (with a "vl.c/hax..." error), however this series should be
> > > > > okay to be applied cleanly upon it.
> > > > 
> > > > Sorry I forgot to append the todo list (please help adding in in case
> > > > I missed anything):
> > > > 
> > > > - don't need to notify IOTLB (psi/gsi/global) invalidations to devices
> > > >   that with ATS enabled
> > > > - investigate when guest map page while mask contains existing mapped
> > > >   pages (e.g. map 12k-16k first, then map 0-12k)
> > > > - coalesce unmap during page walk (currently, we send it once per
> > > >   page)
> > > > - when do PSI for unmap, whether we can send one notify directly
> > > >   instead of walking over the page table?
> > > > 
> > > > Above does not contain those that are still during discussion. And,
> > > > some of the entries still need tests to further prove it's
> > > > feasibility.
> > > > 
> > > > Thanks,
> > > > 
> > > > -- peterx
> > > 
> > > While valid I don't think above need to block merging.
> > 
> > Sorry for the delay of this series.
> > 
> > Looks like we still need to wait until we got explicit acks for vfio
> > patch 2-3 from Alex/Paolo/David since that'll be required by the
> > following series. (I'll address all Jason's new comments as well in
> > next post)
> > 
> > Do you like to merge the iommu cleanups first? Or you can also wait
> > for above to be confirmed, and I'll repost for a final version then
> > (if we don't have further comments).
> > 
> > Thanks,
> > 
> > -- peterx
> 
> I can do that. If you want me to, pls post just the patches
> that are ready separately.

Thanks for the offer!

Since I see that currently the only uncertainty of this series would
be the impact that patch 3 might bring to Power, I'll prefer we wait
for several more days until David's back (I suppose it's next Monday
or so), to see whether I can send a complete final version.

Thanks,

-- peterx

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Peter Xu]

On Wed, Jan 25, 2017 at 06:11:37PM +0100, Paolo Bonzini wrote:

[...]

> > The comment from v4 still needs input from Paolo, is it valid to make
> > use of vaddr (based on address_space_translate ->
> > memory_region_get_ram_ptr) outside of the rcu read lock or could future
> > BQL reduction efforts allow this to race?
> 
> You need to keep a reference to the MemoryRegion if you do
> rcu_read_unlock.  But it's simpler to call vfio_get_vaddr within
> rcu_read_lock, and keep the lock/unlock in vfio_iommu_map_notify.
> 
> You probably should also put a comment about why VFIO does *not* need to
> keep a reference between vfio_dma_map and vfio_dma_unmap (which doesn't
> sound easy to do either).

How about this one?

/*
 * Here, we need to have the lock not only for vfio_get_vaddr(),
 * but also needs to make sure that the vaddr will be valid for
 * further operations.
 *
 * When we map new pages, we need the lock to make sure that vaddr
 * is valid along the way we build up the IO page table (via
 * vfio_dma_map()). Then, as long as the mapping is set up, we can
 * unlock since those pages will be pinned in kernel (which makes
 * sure that the RAM backend of vaddr will always be there, even
 * if the memory object is destroyed and RAM released).
 *
 * For unmapping case, we don't really need the protection since
 * the pages are in all cases locked in kernel, so we'll probably
 * be safe even without the lock. However, it won't hurt we have
 * the lock as well here.
 */

Thanks,

-- peterx

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Fam Zheng]

On Thu, 01/26 07:29, Markus Armbruster wrote:
> With QEMU sources, you can have any tab width, as long as it is eight.

That is very permissive!

Fam

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Peter Xu]

On Wed, Jan 25, 2017 at 01:09:47PM -0700, Alex Williamson wrote:
> On Wed, 25 Jan 2017 20:42:19 +0100
> Paolo Bonzini  wrote:
> 
> > On 25/01/2017 19:36, Alex Williamson wrote:
> > >> It depends of what happens if they aren't.  I think it's fine (see other
> > >> message), but taking a reference for each mapping entry isn't so easy
> > >> because the unmap case doesn't know the old memory region.  
> > > If we held a reference to the memory region from the mapping path and
> > > walk the IOMMU page table to generate the unmap, then we really should
> > > get to the same original memory region, right?  The vfio iommu notifier
> > > should only be mapping native page sizes of the IOMMU, 4k/2M/1G.  The
> > > problem is that it's a lot of overhead to flush the entire address
> > > space that way vs the single invalidation Peter is trying to enable
> > > here.  It's actually similar to how the type1 iommu works in the kernel
> > > though, we can unmap by iova because we ask the iommu for the iova->pfn
> > > translation in order to unpin the page.  
> > 
> > But in the kernel you can trust the IOMMU page tables because you build
> > them, here instead it's the guest's page tables that you'd walk, right?
> > You cannot trust the guest.
> 
> Yes, you're right, we're not shadowing the vt-d page tables, we're
> working on the explicit invalidation model.  So there could be
> anything, or nothing in the page tables when we go to try to lookup the
> unref.  So clearly taking that reference without a shadow page table
> would be the wrong approach.  Thanks,

IIUC of above discussion, moving rcu read lock/unlock out of
vfio_get_vaddr() would be the nicest approach here.

Thanks to you both on helping verify and confirm the problem!

-- peterx

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Markus Armbruster]

Fam Zheng  writes:

> On Wed, 01/25 18:36, Phil Dennis-Jordan wrote:
>> On 25 January 2017 at 18:27,   wrote:
>> > Your series seems to have some coding style problems. See output below for
>> > more information:
>> >
>> > Type: series
>> > Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest 
>> > compatibility of usb-tablet
>> > Message-id: 1485365075-32702-1-git-send-email-p...@philjordan.eu
>> >
>> > === OUTPUT BEGIN ===
>> > Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of 
>> > usb-tablet...
>> > ERROR: code indent should never use tabs
>> > #43: FILE: hw/usb/dev-hid.c:490:
>> > +0x09, 0x02,^I^I/* Usage (Mouse) */$
>> 
>> Interestingly, the surrounding array initialisation already uses tabs,
>> so replacing them with spaces on only the line I edited seems wrong as
>> it'll mis-render in editors configured with a different tab width.
>> Please let me know if I need to take action on this issue, and if so
>> what to do. (I can add a whitespace-only patch to fix the surrounding
>> area, for example. Coding guidelines suggest this might not be
>> desirable though.)
>
> File level tab consistency should override checkpatch.pl in this case, and as
> you said whitespace patches are usually not advisable. Let's just ignore the
> complain.

Ignoring is okay here.  Expanding the tabs would also be okay, and we
routinely do that when we touch lines.

With QEMU sources, you can have any tab width, as long as it is eight.

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Peter Xu]

On Wed, Jan 25, 2017 at 06:16:44PM +0100, Paolo Bonzini wrote:
> 
> 
> On 25/01/2017 17:43, Alex Williamson wrote:
> > On Wed, 25 Jan 2017 12:32:21 +0800
> > Peter Xu  wrote:
> >> I have similar question as well above - IIUC the RCU read lock
> >> protects us from not losing the references of memory objects, however
> >> in our case even after we release the lock, we are still using the
> >> backend ram (vaddr) since we have set it up inside kernel to build up
> >> the IO page table. After that, the kernel/device should be able to
> >> write to addresses of that backend ram any time.
> 
> I don't think that's what happens.  As far as I understand, VFIO pins
> the pages corresponding to vaddr, not vaddr itself.  The memory backend
> is mmap-ed memory; when you hot-unplug it the munmap releases the VMA
> and loses the connection between QEMU's virtual address space and the
> pages.  However, the pages stay pinned and stay mapped into VFIO's own
> IOMMU page tables.
> 
> So if a guest does a memory hot-unplug without IOMMU unmap, it would
> keep on seeing the content of the hot-unplugged memory, and the host
> could not release the pages, but the guest cannot overwrite QEMU data
> structures.

Sounds reasonable. I forgot that these pages are pinned if without an
explicit unmap.

Thanks Paolo. :)

-- peterx

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2] target-ppc: Add MMU model check for booke machines

[Thomas Huth]

On 25.01.2017 15:50, Valentin Plotkin wrote:
> Machines bamboo, e500 and virtex-ml507 assume a certain MMU model,
> otherwise resulting in unpredictable behavior. Add apropriate checks
> into *_init functions.
> 
> Signed-off-by: Valentin 

Missing last name? --^

Apart from that, the patch looks fine to me now, thanks for the update!

 Thomas


> ---
>  hw/ppc/e500.c  | 6 ++
>  hw/ppc/ppc440_bamboo.c | 6 ++
>  hw/ppc/virtex_ml507.c  | 7 +++
>  3 files changed, 19 insertions(+)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index cf8b122..5b1958c 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -827,6 +827,12 @@ void ppce500_init(MachineState *machine,
> PPCE500Params *params)
>  env = &cpu->env;
>  cs = CPU(cpu);
> 
> +if (env->mmu_model != POWERPC_MMU_BOOKE206) {
> +fprintf(stderr, "MMU model %i not supported by this
> machine.\n",
> +env->mmu_model);
> +exit(1);
> +}
> +
>  if (!firstenv) {
>  firstenv = env;
>  }
> diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
> index 5c535b1..9d997bf 100644
> --- a/hw/ppc/ppc440_bamboo.c
> +++ b/hw/ppc/ppc440_bamboo.c
> @@ -193,6 +193,12 @@ static void bamboo_init(MachineState *machine)
>  }
>  env = &cpu->env;
> 
> +if (env->mmu_model != POWERPC_MMU_BOOKE) {
> +fprintf(stderr, "MMU model %i not supported by this machine.\n",
> +env->mmu_model);
> +exit(1);
> +}
> +
>  qemu_register_reset(main_cpu_reset, cpu);
>  ppc_booke_timers_init(cpu, 4, 0);
>  ppc_dcr_init(env, NULL, NULL);
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index b97d966..fdbcf22 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -221,6 +221,13 @@ static void virtex_init(MachineState *machine)
> 
>  cpu = ppc440_init_xilinx(&ram_size, 1, machine->cpu_model, 4);
>  env = &cpu->env;
> +
> +if (env->mmu_model != POWERPC_MMU_BOOKE) {
> +fprintf(stderr, "MMU model %i not supported by this machine.\n",
> +env->mmu_model);
> +exit(1);
> +}
> +
>  qemu_register_reset(main_cpu_reset, cpu);
> 
>  memory_region_allocate_system_memory(phys_ram, NULL, "ram", ram_size);

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Fix crash when running without CPU

[Thomas Huth]

On 26.01.2017 00:26, Alistair Francis wrote:
> On Wed, Jan 25, 2017 at 12:52 PM, Laurent Vivier  wrote:
>> Le 25/01/2017 à 21:45, Thomas Huth a écrit :
>>> When running QEMU with "-M none -device loader,file=kernel.elf", it
>>> currently crashes with a segmentation fault, because the "none"-machine
>>> does not have any CPU by default and the generic loader code tries
>>> to dereference s->cpu. Fix it by adding an appropriate check for a
>>> NULL pointer.
>>>
>>> Reported-by: Laurent Vivier 
>>> Signed-off-by: Thomas Huth 
>>> ---
>>>  hw/core/generic-loader.c | 9 +
>>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
>>> index 58f1f02..4601267 100644
>>> --- a/hw/core/generic-loader.c
>>> +++ b/hw/core/generic-loader.c
>>> @@ -137,20 +137,21 @@ static void generic_loader_realize(DeviceState *dev, 
>>> Error **errp)
>>>  #endif
>>>
>>>  if (s->file) {
>>> +AddressSpace *as = s->cpu ? s->cpu->as :  NULL;
>>
>> Should we just abort if s->cpu is NULL?
> 
> I agree, what is the use case where you are loading images without a CPU?
> 
> If there is a use case (maybe some KVM thing?) then this patch looks fine to 
> me.

I think there is no real use case yet. But this fix is 1) simpler than
doing an error_report() + exit() here, and 2) maybe the vision of
constructing machines on the fly with QEMU will eventually come true one
day in the distant future, so with that patch here, the code would
already be prepared for the case when QEMU gets started without CPUs and
the CPUs are then later added via QOM...
Well, I don't mind ... if you prefer an error message instead, feel free
to suggest another patch. I'm fine as long as we do not simply crash
with a segmentation fault here.

 Thomas

Re: [Qemu-devel] [PATCH v4 1/9] ACPI: Add a function for building named qword entries

[Ben Warren]

> On Jan 25, 2017, at 4:48 PM, Laszlo Ersek  wrote:
> 
> On 01/25/17 19:35, Michael S. Tsirkin wrote:
>> On Wed, Jan 25, 2017 at 09:36:52AM -0800, Ben Warren wrote:
>>> Hi Laszlo,
>>> 
>>> 
>>>On Jan 24, 2017, at 7:55 PM, Laszlo Ersek  wrote:
>>> 
>>>Hi Ben,
>>> 
>>>sorry about being late to reviewing this series. I hope I can now spend
>>>more time on it.
>>> 
>>>- Please do not try to address my comments immediately. It's very
>>>possible (even likely) that Igor, MST and myself could have different
>>>opinions on things, so first please await agreement between your 
>>> reviewers.
>>> 
>>> 
>>> Thanks for the very detailed review.  I’ll give it a couple of days and then
>>> start work on the suggested changes.
>>> 
>>> 
>>>- I think you should have CC'd Igor and Michael directly. I'm adding
>>>them to this reply; hopefully that will be enough for them to monitor
>>>this series.
>>> 
>>>- I'll likely be unable to review everything with 100% coverage; so
>>>addressing (or sufficiently refuting) my comments might not guarantee
>>>that the next version will be merged at once.
>>> 
>>>With all that said:
>>> 
>>>On 01/25/17 02:43, b...@skyportsystems.com wrote:
>>> 
>>>From: Ben Warren 
>>> 
>>>This is initially used to patch a 64-bit address into the VM 
>>> Generation
>>>ID SSDT
>>> 
>>> 
>>>(1) I think this commit message line is overlong; I think we wrap at 74
>>>chars or so. Not critical, but worth mentioning.
>>> 
>>> 
>>> 
>>>Signed-off-by: Ben Warren 
>>>---
>>>hw/acpi/aml-build.c | 28 
>>>include/hw/acpi/aml-build.h |  4 
>>>2 files changed, 32 insertions(+)
>>> 
>>>diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>>index b2a1e40..dc4edc2 100644
>>>--- a/hw/acpi/aml-build.c
>>>+++ b/hw/acpi/aml-build.c
>>>@@ -285,6 +285,34 @@ build_append_named_dword(GArray *array, const 
>>> char
>>>*name_format, ...)
>>>return offset;
>>>}
>>> 
>>>+/*
>>>+ * Build NAME(, 0x) where 0x is encoded as a
>>>qword,
>>>+ * and return the offset to 0x for runtime patching.
>>>+ *
>>>+ * Warning: runtime patching is best avoided. Only use this as
>>>+ * a replacement for DataTableRegion (for guests that don't
>>>+ * support it).
>>>+ */
>> 
>> only one comment: QWords first appeared in ACPI 2.0 and
>> XP does not like them. Not strictly a blocker as people can
>> avoid using the feature, but nice to have.
> 
> Does XP have a driver for VMGENID?
> 
> If not, then I'd prefer to stick with the qword VGIA.
> 
>> Will either UEFI or seabios allocate
>> memory outside 4G range? If not you do not need a qword.
> 
> Good point (assuming XP has a driver for VMGENID).
> 
> OVMF keeps all such allocations (i.e., for COMMAND_ALLOCATE and the
> upcoming COMMAND_ALLOCATE_RETURN_ADDR) under 4GB, so as far as OVMF is
> concerned, using a dword for the VGIA named object should be fine.
> Accordingly, a 4-byte wide ADD_POINTER command should be used for
> patching VGIA.
> 
> Considering the fw_cfg file that receives the address, and
> COMMAND_ALLOCATE_RETURN_ADDR more generally, I'd still prefer if those
> stayed 8-byte wide, regardless of XP's support for VMGENID.
> 
> 
> Hm... It looks like VMGENID *can* be consumed on Windows XP SP3, as long
> as "Hyper-V integration services" are installed:
> 
> https://msdn.microsoft.com/en-us/library/jj643357(v=vs.85).aspx 
> 
> 
>The virtual machine must be running a guest operating system that
>has support for the virtual machine generation identifier.
>Currently, these are the following.
>The following operating systems have native support for the virtual
>machine generation identifier.
>  [...]
> 
>The following operating can be used as the guest operating system
>if the Hyper-V integration services from Windows 8 or Windows
>Server 2012 are installed.
> 
>  [...]
>  * Windows XP with Service Pack 3 (SP3)
> 
> Additionally, under
>  >:
> 
>Supported Windows client guest operating systems
> 
>Windows XP with   [...] Install the integration  [...]
>Service Pack 3 (SP3)services after you set
>up the operating system
>in the virtual machine.
> 
> This seems to be consistent with the VMGENID spec requirement that the
> ADDR method return a package of two 32-bit integers, not a single 64-bit
> one.
> 
> So, I agree with using a dword for VGIA (and a matching 4-byte wide
> ADD_POINTER command).
> 
> But, again, I'd like to keep COMMAND

Re: [Qemu-devel] [PATCH 1/1] block: add missed BDRV_O_NOCACHE when block device is opened without file

[Denis V. Lunev]

On 01/25/2017 08:59 PM, Max Reitz wrote:
> [CC-ing John]
>
> On 25.01.2017 17:42, Denis V. Lunev wrote:
>> Technically there is a problem when the guest DVD is created by libvirt
>> with AIO mode 'native' on Linux. Current QEMU is unable to start the
>> domain configured as follows:
>> 
>>   
>>   
>>   
>> 
>> The problem comes from the combination of 'cache' and 'io' options.
>>
>> 'io' option is common option and it is removed from block driver
>> specific options. 'cache' originally is not. The patch makes 'cache'
>> option common. This works fine as long as cdrom media insertion
>> later on.
>>
>> Signed-off-by: Denis V. Lunev 
>> CC: Kevin Wolf 
>> CC: Max Reitz 
>> ---
>>  blockdev.c | 18 +++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
> There was a Red Hat BZ for this:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1342999
>
> There is now a corresponding BZ for libvirt:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1377321
>
> The gist is that it was determined to be a problem with libvirt.
>
> RHEL has a downstream commit to work around this issue by ignoring the
> cache mode set for empty CD-ROMs -- but that is only because that was
> simpler than fixing libvirt.
>
> (Note that it's ignoring the cache mode instead of actually evaluating it.)
>
This is what I have exactly started from:
http://ftp.redhat.com/pub/redhat/linux/enterprise/7Server/en/RHEV/SRPMS/qemu-kvm-rhev-2.6.0-27.el7.src.rpm

This package starts VM well for the above mentioned configuration:


  
  
  


but the problem comes later at 'change' moment. It reports

'Details: internal error: unable to execute QEMU command 'change':
aio=native was specified, but it requires cache.direct=on, which
was not specified.)'


Thus partial solution implemented by the RedHat is really
partial and does not work at the second stage. I have started from

diff --git a/blockdev.c b/blockdev.c
index d280dc4..e2c9053 100644   
--- a/blockdev.c
+++ b/blockdev.c
@@ -608,6 +608,13 @@ static BlockBackend *blockdev_init(const char
*file, QDict *bs_opts,
 goto early_err;
 }
 
+if (qdict_haskey(bs_opts, BDRV_OPT_CACHE_DIRECT)) {
+bdrv_flags |= BDRV_O_NOCACHE;
+}
+if (qdict_haskey(bs_opts, BDRV_OPT_CACHE_NO_FLUSH)) {
+bdrv_flags |= BDRV_O_NO_FLUSH;
+}
+
 blk_rs = blk_get_root_state(blk);
 blk_rs->open_flags= bdrv_flags;
 blk_rs->read_only = !(bdrv_flags & BDRV_O_RDWR);

The problem for us is that we have such previously valid configurations
in the field.

>> May be this has already discussed, but still. AIO=native for CDROM without
>> media seems important case.
> First, I personally don't find aio=native very important for CD-ROM
> drives. Yes, we shouldn't make IDE CD-ROM slower than necessary, but I
> don't think aio=native will make the difference between "Slow like
> CD-ROMs are in reality" and "As fast as virtio".
the problem for me is that each clone() call is costly and counts. That
is why we are trying to avoid it whenever possible. That is why 'native'
mode is MUCH better. Also it would be very nice not to use cached
IO, which is very good for memory overcommit situations.

Here I am fighting not with the performance, which does not make
any real sense, but with a memory footprint.

> Second, all this patch does is revert some changes done by commit
> 91a097e7478940483e76d52217f05bc05b98d5a5, which was very deliberate.
>
> Third, you may then be asking for the recommended way to put an
> aio=native medium into a CD-ROM drive. Good thing you ask, because we
> have a way that we want to recommend but can't because it's still
> considered experimental:
>
> The BDS is added using blockdev-add, with all of the appropriate caching
> and aio options. Then it's inserted into the drive using the
> x-blockdev-insert-medium command, and the drive is closed using
> blockdev-close-tray.
the problem, again, is that with x-blockdev-insert-medium I can not
deal with block driver options, or may be I am missing something.

> There are a couple of issues with this: First, blockdev-add and
> x-blockdev-insert-medium are still experimental. The good news are that
> I think the reason why the latter is experimental has actually
> disappeared and we can just drop the x- by now. The
> not-so-good-but-not-so-bad-either news are that we plan to get
> blockdev-add declared non-experimental for 2.9. Let's see how it goes.
Does this mean that x-blockdev-del would also lose x- prefix?

> The other issue is that of course it's more cumbersome to use than a
> simple 'change' via HMP. I argue that if someone communicates with a VM
> by hand, they either have to deal with this added complexity or they
> cannot use aio=native for CD-ROM drives -- which, as I said, I don't
> think is too bad.
this is how 'virsh change-media' works at the moment, at least with
latest downstreams.
That is why this is not that cle

[Qemu-devel] [PATCH v2 2/2] virtio-scsi: Implement fc_host feature

[Fam Zheng]

This patch add qdev properties to allow enabling the fc_host feature of
virtio-scsi, and fill config fields.

For migration, the destination QEMU should be started with the same
wwnn and wwpn, and a inverted primary_active. When migration is done,
the config change interrupt will allow guest to discover the toggling.

Signed-off-by: Fam Zheng 
---
 hw/scsi/virtio-scsi.c   | 72 +++--
 include/hw/virtio/virtio-scsi.h | 10 ++
 2 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index ce19eff..c303f46 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -656,6 +656,11 @@ static void virtio_scsi_get_config(VirtIODevice *vdev,
 virtio_stw_p(vdev, &scsiconf->max_channel, VIRTIO_SCSI_MAX_CHANNEL);
 virtio_stw_p(vdev, &scsiconf->max_target, VIRTIO_SCSI_MAX_TARGET);
 virtio_stl_p(vdev, &scsiconf->max_lun, VIRTIO_SCSI_MAX_LUN);
+stq_be_p(&scsiconf->primary_wwpn, s->conf.primary_wwpn);
+stq_be_p(&scsiconf->primary_wwnn, s->conf.primary_wwnn);
+stq_be_p(&scsiconf->secondary_wwpn, s->conf.secondary_wwpn);
+stq_be_p(&scsiconf->secondary_wwnn, s->conf.secondary_wwnn);
+scsiconf->primary_active =  s->conf.primary_active;
 }
 
 static void virtio_scsi_set_config(VirtIODevice *vdev,
@@ -870,18 +875,68 @@ void virtio_scsi_common_realize(DeviceState *dev, Error 
**errp,
 }
 }
 
+static void virtio_scsi_vm_state_change(void *opaque, int running,
+RunState state)
+{
+VirtIOSCSI *s = opaque;
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+if (!running || !s->config_change_pending) {
+return;
+}
+s->config_change_pending = false;
+virtio_notify_config(vdev);
+}
+
 static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOSCSI *s = VIRTIO_SCSI(dev);
+VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 Error *err = NULL;
 
+if (vs->conf.fc_host) {
+if (!strcmp(vs->conf.fc_host, "off")) {
+vs->conf.primary_wwpn = 0;
+vs->conf.primary_wwnn = 0;
+vs->conf.secondary_wwpn = 0;
+vs->conf.secondary_wwnn = 0;
+} else if (!strcmp(vs->conf.fc_host, "primary") ||
+   !strcmp(vs->conf.fc_host, "secondary")) {
+virtio_add_feature(&vdev->host_features, VIRTIO_SCSI_F_FC_HOST);
+vs->conf.primary_active = !strcmp(vs->conf.fc_host, "primary");
+if (!vs->conf.primary_wwpn) {
+error_setg(errp, "fc_host enabled but primary_wwpn not set");
+return;
+}
+if (!vs->conf.primary_wwnn) {
+error_setg(errp, "fc_host enabled but primary_wwnn not set");
+return;
+}
+if (!vs->conf.secondary_wwpn) {
+error_setg(errp, "fc_host enabled but secondary_wwpn not set");
+return;
+}
+if (!vs->conf.secondary_wwnn) {
+error_setg(errp, "fc_host enabled but secondary_wwnn not set");
+return;
+}
+s->vm_state_change =
+qemu_add_vm_change_state_handler(virtio_scsi_vm_state_change, 
s);
+s->config_change_pending = runstate_check(RUN_STATE_INMIGRATE);
+} else {
+error_setg(errp, "Invalid fc_host option. "
+ "Must be 'off', 'primary' or 'secondary'");
+return;
+}
+}
+
 virtio_scsi_common_realize(dev, &err, virtio_scsi_handle_ctrl,
virtio_scsi_handle_event,
virtio_scsi_handle_cmd);
 if (err != NULL) {
 error_propagate(errp, err);
-return;
+goto fail;
 }
 
 scsi_bus_new(&s->bus, sizeof(s->bus), dev,
@@ -893,11 +948,14 @@ static void virtio_scsi_device_realize(DeviceState *dev, 
Error **errp)
 scsi_bus_legacy_handle_cmdline(&s->bus, &err);
 if (err != NULL) {
 error_propagate(errp, err);
-return;
+goto fail;
 }
 }
 
 virtio_scsi_dataplane_setup(s, errp);
+return;
+fail:
+qemu_del_vm_change_state_handler(s->vm_state_change);
 }
 
 static void virtio_scsi_instance_init(Object *obj)
@@ -921,6 +979,11 @@ void virtio_scsi_common_unrealize(DeviceState *dev, Error 
**errp)
 
 static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
 {
+VirtIOSCSI *s = VIRTIO_SCSI(dev);
+
+if (s->vm_state_change) {
+qemu_del_vm_change_state_handler(s->vm_state_change);
+}
 virtio_scsi_common_unrealize(dev, errp);
 }
 
@@ -934,6 +997,11 @@ static Property virtio_scsi_properties[] = {
VIRTIO_SCSI_F_HOTPLUG, true),
 DEFINE_PROP_BIT("param_change", VirtIOSCSI, host_features,

[Qemu-devel] [PATCH v2 1/2] manual update linux header for virtio scsi fc_host

[Fam Zheng]

XXX: bring these changes in by syncing once it's added to linux.

Signed-off-by: Fam Zheng 
---
 include/standard-headers/linux/virtio_scsi.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/standard-headers/linux/virtio_scsi.h 
b/include/standard-headers/linux/virtio_scsi.h
index ab66166..b37c395 100644
--- a/include/standard-headers/linux/virtio_scsi.h
+++ b/include/standard-headers/linux/virtio_scsi.h
@@ -113,6 +113,11 @@ struct virtio_scsi_config {
uint16_t max_channel;
uint16_t max_target;
uint32_t max_lun;
+   uint8_t  primary_wwpn[8];
+   uint8_t  primary_wwnn[8];
+   uint8_t  secondary_wwpn[8];
+   uint8_t  secondary_wwnn[8];
+   uint8_t  primary_active;
 } QEMU_PACKED;
 
 /* Feature Bits */
@@ -120,6 +125,7 @@ struct virtio_scsi_config {
 #define VIRTIO_SCSI_F_HOTPLUG  1
 #define VIRTIO_SCSI_F_CHANGE   2
 #define VIRTIO_SCSI_F_T10_PI   3
+#define VIRTIO_SCSI_F_FC_HOST  4
 
 /* Response codes */
 #define VIRTIO_SCSI_S_OK   0
-- 
2.9.3

[Qemu-devel] [PATCH v2 0/2] virtio-scsi: Implement fc_host feature

[Fam Zheng]

v2: Use big endian for WWNN/WWPN fields. [Paolo]
Clean up vm state change notifier in error/unrealize. [Stefan]

This implements a WIP feature extention being proposed on virtio-scsi.

We assign a set of Fibre Channel properties, WWNN and WWPN (world wide node
name and port name, respectively) to the device so the virtual device presents
a FC transport in guest. They are useful to identify a host side virtual port
on an FC host, in a NPIV (N_Port ID Virtualization) senario.

The linux driver changes are submitted as:

https://lkml.org/lkml/2017/1/16/439

Fam Zheng (2):
  manual update linux header for virtio scsi fc_host
  virtio-scsi: Implement fc_host feature

 hw/scsi/virtio-scsi.c| 72 +++-
 include/hw/virtio/virtio-scsi.h  | 10 
 include/standard-headers/linux/virtio_scsi.h |  6 +++
 3 files changed, 86 insertions(+), 2 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] [PATCH V6 0/2] Add new qmp commands to suppurt Xen COLO

[Jason Wang]

On 2017年01月26日 11:04, Zhang Chen wrote:

Hi~

No news for a week.

We need comments ~~

Ping...



Cc David who is one of the maintainer of migration.




Thanks

Zhang Chen


On 01/19/2017 05:57 PM, Zhang Chen wrote:

Hi~~

Anyone have any comments?

Ping


Thanks

Zhang Chen


On 01/05/2017 02:08 PM, Zhang Chen wrote:

Xen COLO depend on qemu COLO replication function.
So, We need new qmp commands for Xen to use qemu replication.

Corresponding libxl patches already in xen.git.
Commit ID:

ed37ef1f91c20f0ab162ce60f8c38400b917fa64
COLO: introduce new API to prepare/start/do/get_error/stop replication

a0ddc0b359375b2418213966dfbdbfab593ecc6f
tools/libxl: introduction of libxl__qmp_restore to load qemu state


Zhang Chen (2):
   Add a new qmp command to start/stop replication
   Add a new qmp command to do checkpoint, query xen replication status

  docs/qmp-commands.txt | 42 +++
  migration/colo.c  | 40 +
  qapi-schema.json  | 69 
+++

  3 files changed, 151 insertions(+)

Re: [Qemu-devel] [PATCH V6 0/2] Add new qmp commands to suppurt Xen COLO

[Zhang Chen]

Hi~

No news for a week.

We need comments ~~

Ping...


Thanks

Zhang Chen


On 01/19/2017 05:57 PM, Zhang Chen wrote:

Hi~~

Anyone have any comments?

Ping


Thanks

Zhang Chen


On 01/05/2017 02:08 PM, Zhang Chen wrote:

Xen COLO depend on qemu COLO replication function.
So, We need new qmp commands for Xen to use qemu replication.

Corresponding libxl patches already in xen.git.
Commit ID:

ed37ef1f91c20f0ab162ce60f8c38400b917fa64
COLO: introduce new API to prepare/start/do/get_error/stop replication

a0ddc0b359375b2418213966dfbdbfab593ecc6f
tools/libxl: introduction of libxl__qmp_restore to load qemu state


Zhang Chen (2):
   Add a new qmp command to start/stop replication
   Add a new qmp command to do checkpoint, query xen replication status

  docs/qmp-commands.txt | 42 +++
  migration/colo.c  | 40 +
  qapi-schema.json  | 69 
+++

  3 files changed, 151 insertions(+)





--
Thanks
Zhang Chen

[Qemu-devel] [PULL 0/2] HBitmap patches

[Fam Zheng]

The following changes since commit c7f1cf01b8245762ca5864e835d84f6677ae8b1f:

  Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging 
(2017-01-25 17:54:14 +)

are available in the git repository at:

  git://github.com/famz/qemu.git tags/for-upstream

for you to fetch changes up to 7cdc49b9a2ee127b2403f6ce98ce3f96afb41733:

  test-hbitmap: Add hbitmap_is_serializable() calls (2017-01-26 10:25:01 +0800)



Two patches by Max submitted back in 2.8 timeframe.



Max Reitz (2):
  hbitmap: Add hbitmap_is_serializable()
  test-hbitmap: Add hbitmap_is_serializable() calls

 include/qemu/hbitmap.h | 13 +
 tests/test-hbitmap.c   | 11 +++
 util/hbitmap.c | 22 +++---
 3 files changed, 43 insertions(+), 3 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL 1/2] hbitmap: Add hbitmap_is_serializable()

[Fam Zheng]

From: Max Reitz 

Bitmaps with a granularity of 58 or above can be neither serialized nor
deserialized (see the comment in the function added in this series for
an explanation). This patch adds a function so that we can check whether
a bitmap actually can be (de-)serialized at all, thus avoiding failing
the necessary assertion in hbitmap_serialization_granularity().

Signed-off-by: Max Reitz 
Message-Id: <20161115225746.3590-2-mre...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Fam Zheng 
---
 include/qemu/hbitmap.h | 13 +
 util/hbitmap.c | 22 +++---
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index eb46475..9239fe5 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -146,6 +146,19 @@ void hbitmap_reset_all(HBitmap *hb);
 bool hbitmap_get(const HBitmap *hb, uint64_t item);
 
 /**
+ * hbitmap_is_serializable:
+ * @hb: HBitmap which should be (de-)serialized.
+ *
+ * Returns whether the bitmap can actually be (de-)serialized. Other
+ * (de-)serialization functions may only be invoked if this function returns
+ * true.
+ *
+ * Calling (de-)serialization functions does not affect a bitmap's
+ * (de-)serializability.
+ */
+bool hbitmap_is_serializable(const HBitmap *hb);
+
+/**
  * hbitmap_serialization_granularity:
  * @hb: HBitmap to operate on.
  *
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 9f691b7..35088e1 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -387,6 +387,24 @@ void hbitmap_reset_all(HBitmap *hb)
 hb->count = 0;
 }
 
+bool hbitmap_is_serializable(const HBitmap *hb)
+{
+/* Every serialized chunk must be aligned to 64 bits so that endianness
+ * requirements can be fulfilled on both 64 bit and 32 bit hosts.
+ * We have hbitmap_serialization_granularity() which converts this
+ * alignment requirement from bitmap bits to items covered (e.g. sectors).
+ * That value is:
+ *64 << hb->granularity
+ * Since this value must not exceed UINT64_MAX, hb->granularity must be
+ * less than 58 (== 64 - 6, where 6 is ld(64), i.e. 1 << 6 == 64).
+ *
+ * In order for hbitmap_serialization_granularity() to always return a
+ * meaningful value, bitmaps that are to be serialized must have a
+ * granularity of less than 58. */
+
+return hb->granularity < 58;
+}
+
 bool hbitmap_get(const HBitmap *hb, uint64_t item)
 {
 /* Compute position and bit in the last layer.  */
@@ -399,9 +417,7 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item)
 
 uint64_t hbitmap_serialization_granularity(const HBitmap *hb)
 {
-/* Must hold true so that the shift below is defined
- * (ld(64) == 6, i.e. 1 << 6 == 64) */
-assert(hb->granularity < 64 - 6);
+assert(hbitmap_is_serializable(hb));
 
 /* Require at least 64 bit granularity to be safe on both 64 bit and 32 bit
  * hosts. */
-- 
2.9.3

[Qemu-devel] [PULL 2/2] test-hbitmap: Add hbitmap_is_serializable() calls

[Fam Zheng]

From: Max Reitz 

Add calls to hbitmap_is_serializable() (asserting that it returns true)
where necessary (i.e. before every series of (de-)serialization function
invocations).

Signed-off-by: Max Reitz 
Message-Id: <20161115225746.3590-3-mre...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Fam Zheng 
---
 tests/test-hbitmap.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
index 9b7495c..23773d2 100644
--- a/tests/test-hbitmap.c
+++ b/tests/test-hbitmap.c
@@ -744,6 +744,8 @@ static void 
test_hbitmap_serialize_granularity(TestHBitmapData *data,
 int r;
 
 hbitmap_test_init(data, L3 * 2, 3);
+g_assert(hbitmap_is_serializable(data->hb));
+
 r = hbitmap_serialization_granularity(data->hb);
 g_assert_cmpint(r, ==, 64 << 3);
 }
@@ -768,6 +770,8 @@ static void hbitmap_test_serialize_range(TestHBitmapData 
*data,
 if (count) {
 hbitmap_set(data->hb, pos, count);
 }
+
+g_assert(hbitmap_is_serializable(data->hb));
 hbitmap_serialize_part(data->hb, buf, 0, data->size);
 
 /* Serialized buffer is inherently LE, convert it back manually to test */
@@ -788,6 +792,8 @@ static void hbitmap_test_serialize_range(TestHBitmapData 
*data,
 memset(buf, 0, buf_size);
 hbitmap_serialize_part(data->hb, buf, 0, data->size);
 hbitmap_reset_all(data->hb);
+
+g_assert(hbitmap_is_serializable(data->hb));
 hbitmap_deserialize_part(data->hb, buf, 0, data->size, true);
 
 for (i = 0; i < data->size; i++) {
@@ -810,6 +816,7 @@ static void test_hbitmap_serialize_basic(TestHBitmapData 
*data,
 int num_positions = sizeof(positions) / sizeof(positions[0]);
 
 hbitmap_test_init(data, L3, 0);
+g_assert(hbitmap_is_serializable(data->hb));
 buf_size = hbitmap_serialization_size(data->hb, 0, data->size);
 buf = g_malloc0(buf_size);
 
@@ -841,6 +848,8 @@ static void test_hbitmap_serialize_part(TestHBitmapData 
*data,
 hbitmap_set(data->hb, positions[i], 1);
 }
 
+g_assert(hbitmap_is_serializable(data->hb));
+
 for (i = 0; i < data->size; i += buf_size) {
 unsigned long *el = (unsigned long *)buf;
 hbitmap_serialize_part(data->hb, buf, i, buf_size);
@@ -879,6 +888,8 @@ static void test_hbitmap_serialize_zeroes(TestHBitmapData 
*data,
 hbitmap_set(data->hb, positions[i], L1);
 }
 
+g_assert(hbitmap_is_serializable(data->hb));
+
 for (i = 0; i < num_positions; i++) {
 hbitmap_deserialize_zeroes(data->hb, positions[i], min_l1, true);
 hbitmap_iter_init(&iter, data->hb, 0);
-- 
2.9.3

[Qemu-devel] [PATCH v7 wave 2 1/3] hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg

[Laszlo Ersek]

Introduce the following fw_cfg files:

- "etc/smi/supported-features": a little endian uint64_t feature bitmap,
  presenting the features known by the host to the guest. Read-only for
  the guest.

  The content of this file will be determined via bit-granularity ICH9-LPC
  device properties, to be introduced later. For now, the bitmask is left
  zeroed. The bits will be set from machine type compat properties and on
  the QEMU command line, hence this file is not migrated.

- "etc/smi/requested-features": a little endian uint64_t feature bitmap,
  representing the features the guest would like to request. Read-write
  for the guest.

  The guest can freely (re)write this file, it has no direct consequence.
  Initial value is zero. A nonzero value causes the SMI-related fw_cfg
  files and fields that are under guest influence to be migrated.

- "etc/smi/features-ok": contains a uint8_t value, and it is read-only for
  the guest. When the guest selects the associated fw_cfg key, the guest
  features are validated against the host features. In case of error, the
  negotiation doesn't proceed, and the "features-ok" file remains zero. In
  case of success, the "features-ok" file becomes (uint8_t)1, and the
  negotiated features are locked down internally (to which no further
  changes are possible until reset).

  The initial value is zero.  A nonzero value causes the SMI-related
  fw_cfg files and fields that are under guest influence to be migrated.

The C-language fields backing the "supported-features" and
"requested-features" files are uint8_t arrays. This is because they carry
guest-side representation (our choice is little endian), while
VMSTATE_UINT64() assumes / implies host-side endianness for any uint64_t
fields. If we migrate a guest between hosts with different endiannesses
(which is possible with TCG), then the host-side value is preserved, and
the host-side representation is translated. This would be visible to the
guest through fw_cfg, unless we used plain byte arrays. So we do.

Cc: "Michael S. Tsirkin" 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Paolo Bonzini 
Signed-off-by: Laszlo Ersek 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Igor Mammedov 
---

Notes:
v7:
- no changes, pick up Igor's R-b

v6:
- no changes, pick up Michael's R-b

v5:
- rename the "etc/smi/host-features" fw_cfg file to
  "etc/smi/supported-features" [Igor]

- rename the "etc/smi/guest-features" fw_cfg file to
  "etc/smi/requested-features" [Igor]

- suffix the names of the "ICH9LPCState.smi_host_features" and
  "ICH9LPCState.smi_guest_features" array fields with "_le" for
  representing their guest-visible encoding [Igor]

- Replace the "smi_host_features" parameter of ich9_lpc_pm_init() --
  which was meant in v4 to be set by  board code -- with a new
  "ICH9LPCState.smi_host_features" field, of type uint64_t.
  Bit-granularity ICH9-LPC device properties will be carved out of this
  field. [Igor]

- Given the "ICH9LPCState.smi_host_features" uint64_t field, we can now
  use that directly for feature validation in
  smi_features_ok_callback(). Converting the (otherwise guest-read-only)
  "ICH9LPCState.smi_host_features_le" array back to CPU endianness just
  for this is no longer necessary.

 include/hw/i386/ich9.h | 10 +++
 hw/isa/lpc_ich9.c  | 79 ++
 2 files changed, 89 insertions(+)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index 5fd7e97d2347..da1118727146 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -64,6 +64,16 @@ typedef struct ICH9LPCState {
 uint8_t rst_cnt;
 MemoryRegion rst_cnt_mem;
 
+/* SMI feature negotiation via fw_cfg */
+uint64_t smi_host_features;   /* guest-invisible, host endian */
+uint8_t smi_host_features_le[8];  /* guest-visible, read-only, little
+   * endian uint64_t */
+uint8_t smi_guest_features_le[8]; /* guest-visible, read-write, little
+   * endian uint64_t */
+uint8_t smi_features_ok;  /* guest-visible, read-only; selecting it
+   * triggers feature lockdown */
+uint64_t smi_negotiated_features; /* guest-invisible, host endian */
+
 /* isa bus */
 ISABus *isa_bus;
 MemoryRegion rcrb_mem; /* root complex register block */
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 10d1ee8b9310..376b7801a42c 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -48,6 +48,8 @@
 #include "exec/address-spaces.h"
 #include "sysemu/sysemu.h"
 #include "qom/cpu.h"
+#include "hw/nvram/fw_cfg.h"
+#include "qemu/cutils.h"
 
 /*/
 /* ICH9 LPC PCI to ISA bridge */
@@ -360,13 +362,62 @@ static void ich9_set_sci(void *opaque, int irq_num, int 
level)
 }
 }

[Qemu-devel] [PATCH v7 wave 2 3/3] hw/isa/lpc_ich9: negotiate SMI broadcast on pc-q35-2.9+ machine types

[Laszlo Ersek]

Cc: "Michael S. Tsirkin" 
Cc: Eduardo Habkost 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Paolo Bonzini 
Signed-off-by: Laszlo Ersek 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Igor Mammedov 
---

Notes:
v7:
- no functional changes, pick up Igor's R-b
- rebase to master, resolve conflict with commit abc62c89f319 ("pc.h:
  move x-mach-use-reliable-get-clock compat entry to PC_COMPAT_2_8",
  2017-01-18)

v6:
- rename "smi_broadcast" to "x-smi-broadcast" [Eduardo]
- pick up Eduardo's R-b
- pick up Michael's R-b

v5:
- replace the v4 patch "hw/i386/pc_q35: advertise broadcast SMI if VCPU
  hotplug is turned off" with a simple device property and compat
  setting [Igor]

 include/hw/i386/pc.h | 6 ++
 hw/isa/lpc_ich9.c| 2 ++
 2 files changed, 8 insertions(+)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 5a20c5e38ef5..b89441d2c49f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -381,6 +381,12 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 .property = "x-mach-use-reliable-get-clock",\
 .value= "off",\
 },\
+{\
+.driver   = "ICH9-LPC",\
+.property = "x-smi-broadcast",\
+.value= "off",\
+},\
+
 
 #define PC_COMPAT_2_7 \
 HW_COMPAT_2_7 \
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index ced6f803a4f2..59930dd9d09d 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -776,6 +776,8 @@ static const VMStateDescription vmstate_ich9_lpc = {
 
 static Property ich9_lpc_properties[] = {
 DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true),
+DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features,
+  ICH9_LPC_SMI_F_BROADCAST_BIT, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.9.3

[Qemu-devel] [PATCH v7 wave 2 2/3] hw/isa/lpc_ich9: add broadcast SMI feature

[Laszlo Ersek]

The generic edk2 SMM infrastructure prefers
EFI_SMM_CONTROL2_PROTOCOL.Trigger() to inject an SMI on each processor. If
Trigger() only brings the current processor into SMM, then edk2 handles it
in the following ways:

(1) If Trigger() is executed by the BSP (which is guaranteed before
ExitBootServices(), but is not necessarily true at runtime), then:

(a) If edk2 has been configured for "traditional" SMM synchronization,
then the BSP sends directed SMIs to the APs with APIC delivery,
bringing them into SMM individually. Then the BSP runs the SMI
handler / dispatcher.

(b) If edk2 has been configured for "relaxed" SMM synchronization,
then the APs that are not already in SMM are not brought in, and
the BSP runs the SMI handler / dispatcher.

(2) If Trigger() is executed by an AP (which is possible after
ExitBootServices(), and can be forced e.g. by "taskset -c 1
efibootmgr"), then the AP in question brings in the BSP with a
directed SMI, and the BSP runs the SMI handler / dispatcher.

The smaller problem with (1a) and (2) is that the BSP and AP
synchronization is slow. For example, the "taskset -c 1 efibootmgr"
command from (2) can take more than 3 seconds to complete, because
efibootmgr accesses non-volatile UEFI variables intensively.

The larger problem is that QEMU's current behavior diverges from the
behavior usually seen on physical hardware, and that keeps exposing
obscure corner cases, race conditions and other instabilities in edk2,
which generally expects / prefers a software SMI to affect all CPUs at
once.

Therefore introduce the "broadcast SMI" feature that causes QEMU to inject
the SMI on all VCPUs.

While the original posting of this patch

only intended to speed up (2), based on our recent "stress testing" of SMM
this patch actually provides functional improvements.

Cc: "Michael S. Tsirkin" 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Paolo Bonzini 
Signed-off-by: Laszlo Ersek 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Igor Mammedov 
---

Notes:
v7:
- no changes, pick up Igor's R-b

v6:
- no changes, pick up Michael's R-b

v5:
- replace the ICH9_LPC_SMI_F_BROADCAST bit value with the
  ICH9_LPC_SMI_F_BROADCAST_BIT bit position (necessary for
  DEFINE_PROP_BIT() in the next patch)

 include/hw/i386/ich9.h |  3 +++
 hw/isa/lpc_ich9.c  | 10 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index da1118727146..18dcca7ebcbf 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -250,4 +250,7 @@ Object *ich9_lpc_find(void);
 #define ICH9_SMB_HST_D1 0x06
 #define ICH9_SMB_HOST_BLOCK_DB  0x07
 
+/* bit positions used in fw_cfg SMI feature negotiation */
+#define ICH9_LPC_SMI_F_BROADCAST_BIT0
+
 #endif /* HW_ICH9_H */
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 376b7801a42c..ced6f803a4f2 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -437,7 +437,15 @@ static void ich9_apm_ctrl_changed(uint32_t val, void *arg)
 
 /* SMI_EN = PMBASE + 30. SMI control and enable register */
 if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN) {
-cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
+if (lpc->smi_negotiated_features &
+(UINT64_C(1) << ICH9_LPC_SMI_F_BROADCAST_BIT)) {
+CPUState *cs;
+CPU_FOREACH(cs) {
+cpu_interrupt(cs, CPU_INTERRUPT_SMI);
+}
+} else {
+cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
+}
 }
 }
 
-- 
2.9.3

[Qemu-devel] [PATCH v7 wave 2 0/3] q35: add negotiable broadcast SMI

[Laszlo Ersek]

Hi Paolo,

I'm sending v7 with no functional changes; I've only picked up Igor's
R-b's from v6 so you don't have to, and while rebasing to current
master, I resolved a conflict in patch #3 against recent commit
abc62c89f319 ("pc.h: move x-mach-use-reliable-get-clock compat entry to
PC_COMPAT_2_8", 2017-01-18).

The series is fully reviewed and is mergeable as-is.

v6 wave 2 was at
.

Cc: "Michael S. Tsirkin" 
Cc: Eduardo Habkost 
Cc: Gerd Hoffmann 
Cc: Igor Mammedov 
Cc: Paolo Bonzini 

Thanks
Laszlo

Laszlo Ersek (3):
  hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg
  hw/isa/lpc_ich9: add broadcast SMI feature
  hw/isa/lpc_ich9: negotiate SMI broadcast on pc-q35-2.9+ machine types

 include/hw/i386/ich9.h | 13 
 include/hw/i386/pc.h   |  6 
 hw/isa/lpc_ich9.c  | 91 +-
 3 files changed, 109 insertions(+), 1 deletion(-)

-- 
2.9.3

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Fam Zheng]

On Wed, 01/25 18:36, Phil Dennis-Jordan wrote:
> On 25 January 2017 at 18:27,   wrote:
> > Your series seems to have some coding style problems. See output below for
> > more information:
> >
> > Type: series
> > Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest 
> > compatibility of usb-tablet
> > Message-id: 1485365075-32702-1-git-send-email-p...@philjordan.eu
> >
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of 
> > usb-tablet...
> > ERROR: code indent should never use tabs
> > #43: FILE: hw/usb/dev-hid.c:490:
> > +0x09, 0x02,^I^I/* Usage (Mouse) */$
> 
> Interestingly, the surrounding array initialisation already uses tabs,
> so replacing them with spaces on only the line I edited seems wrong as
> it'll mis-render in editors configured with a different tab width.
> Please let me know if I need to take action on this issue, and if so
> what to do. (I can add a whitespace-only patch to fix the surrounding
> area, for example. Coding guidelines suggest this might not be
> desirable though.)

File level tab consistency should override checkpatch.pl in this case, and as
you said whitespace patches are usually not advisable. Let's just ignore the
complain.

Fam

[Qemu-devel] [PATCH v2 3/3] qemu-iotest: test to lookup protocol-based image with relative backing

[Jeff Cody]

This test uses NFS and block-stream to force a lookup of a backing
image that has a relative filename, but a full backing image name
with the protocol path intact.

Signed-off-by: Jeff Cody 
---
 tests/qemu-iotests/173 | 97 ++
 tests/qemu-iotests/173.out | 12 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 110 insertions(+)
 create mode 100755 tests/qemu-iotests/173
 create mode 100644 tests/qemu-iotests/173.out

diff --git a/tests/qemu-iotests/173 b/tests/qemu-iotests/173
new file mode 100755
index 000..bdaa092
--- /dev/null
+++ b/tests/qemu-iotests/173
@@ -0,0 +1,97 @@
+#!/bin/bash
+#
+# Test QAPI commands looking up protocol based images with relative
+# filename backing strings
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# 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 .
+#
+# creator
+owner=jc...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+status=1# failure is the default!
+
+_cleanup()
+{
+_cleanup_qemu
+rm  -f "${QEMU_TEST_DIR}/image.base" "${QEMU_TEST_DIR}/image.snp1"
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt qcow2
+_supported_proto nfs
+_supported_os Linux
+
+size=100M
+
+BASE_IMG="${TEST_DIR}/image.base"
+TOP_IMG="${TEST_DIR}/image.snp1"
+
+TEST_IMG="${BASE_IMG}" _make_test_img $size
+
+TEST_IMG="${TOP_IMG}" _make_test_img $size
+
+echo
+echo === Running QEMU, using block-stream to find backing image ===
+echo
+
+qemu_comm_method="qmp"
+_launch_qemu -drive file="${BASE_IMG}",if=virtio,id=disk2
+h=$QEMU_HANDLE
+
+_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return"
+
+_send_qemu_cmd $h "{ 'arguments': {
+'device': 'disk2',
+'format': '${IMGFMT}',
+'mode': 'existing',
+'snapshot-file': '${TOP_IMG}',
+'snapshot-node-name': 'snp1'
+ },
+ 'execute': 'blockdev-snapshot-sync'
+   }" "return"
+
+
+_send_qemu_cmd $h "{ 'arguments': {
+'backing-file': 'image.base',
+'device': 'disk2',
+'image-node-name': 'snp1'
+ },
+ 'execute': 'change-backing-file'
+   }" "return"
+
+_send_qemu_cmd $h "{ 'arguments': {
+'base': '${BASE_IMG}',
+'device': 'disk2'
+  },
+  'execute': 'block-stream'
+   }" "BLOCK_JOB_COMPLETED"
+
+_cleanup_qemu
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/173.out b/tests/qemu-iotests/173.out
new file mode 100644
index 000..f477a00
--- /dev/null
+++ b/tests/qemu-iotests/173.out
@@ -0,0 +1,12 @@
+QA output created by 173
+Formatting 'TEST_DIR/image.base', fmt=IMGFMT size=104857600
+Formatting 'TEST_DIR/image.snp1', fmt=IMGFMT size=104857600
+
+=== Running QEMU, using block-stream to find backing image ===
+
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
"BLOCK_JOB_COMPLETED", "data": {"device": "disk2", "len": 104857600, "offset": 
104857600, "speed": 0, "type": "stream"}}
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 866c1a0..5a93ba9 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -165,3 +165,4 @@
 170 rw auto quick
 171 rw auto quick
 172 auto
+173 rw auto
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/3] qemu-iotests: Don't create fifos / pidfiles with protocol paths

[Jeff Cody]

Trying to create, use, and remove fifos and pidfiles on protocol paths
(e.g. nfs://localhost/scratch/qemu-nbd.pid) is obviously broken.

Use the local $TEST_DIR path before it is 'protocolized' for these
files.

Signed-off-by: Jeff Cody 
---
 tests/qemu-iotests/common.config |  6 --
 tests/qemu-iotests/common.qemu   | 10 +-
 tests/qemu-iotests/common.rc |  6 +++---
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index f6384fb..55527aa 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -109,7 +109,7 @@ _qemu_wrapper()
 {
 (
 if [ -n "${QEMU_NEED_PID}" ]; then
-echo $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
+echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
 fi
 exec "$QEMU_PROG" $QEMU_OPTIONS "$@"
 )
@@ -151,7 +151,7 @@ _qemu_io_wrapper()
 _qemu_nbd_wrapper()
 {
 (
-echo $BASHPID > "${TEST_DIR}/qemu-nbd.pid"
+echo $BASHPID > "${QEMU_TEST_DIR}/qemu-nbd.pid"
 exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@"
 )
 }
@@ -186,6 +186,8 @@ if [ -z "$TEST_DIR" ]; then
 TEST_DIR=`pwd`/scratch
 fi
 
+QEMU_TEST_DIR="${TEST_DIR}"
+
 if [ ! -e "$TEST_DIR" ]; then
 mkdir "$TEST_DIR"
 fi
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index e657361..4278789 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -27,8 +27,8 @@
 
 QEMU_COMM_TIMEOUT=10
 
-QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$"
-QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$"
+QEMU_FIFO_IN="${QEMU_TEST_DIR}/qmp-in-$$"
+QEMU_FIFO_OUT="${QEMU_TEST_DIR}/qmp-out-$$"
 
 QEMU_HANDLE=0
 
@@ -204,9 +204,9 @@ function _cleanup_qemu()
 for i in "${!QEMU_OUT[@]}"
 do
 local QEMU_PID
-if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then
-read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid"
-rm -f "${TEST_DIR}/qemu-${i}.pid"
+if [ -f "${QEMU_TEST_DIR}/qemu-${i}.pid" ]; then
+read QEMU_PID < "${QEMU_TEST_DIR}/qemu-${i}.pid"
+rm -f "${QEMU_TEST_DIR}/qemu-${i}.pid"
 if [ -z "${wait}" ] && [ -n "${QEMU_PID}" ]; then
 kill -KILL ${QEMU_PID} 2>/dev/null
 fi
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 3213765..4bb9b77 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -193,11 +193,11 @@ _cleanup_test_img()
 case "$IMGPROTO" in
 
 nbd)
-if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then
+if [ -f "${QEMU_TEST_DIR}/qemu-nbd.pid" ]; then
 local QEMU_NBD_PID
-read QEMU_NBD_PID < "${TEST_DIR}/qemu-nbd.pid"
+read QEMU_NBD_PID < "${QEMU_TEST_DIR}/qemu-nbd.pid"
 kill ${QEMU_NBD_PID}
-rm -f "${TEST_DIR}/qemu-nbd.pid"
+rm -f "${QEMU_TEST_DIR}/qemu-nbd.pid"
 fi
 rm -f "$TEST_IMG_FILE"
 ;;
-- 
2.9.3

[Qemu-devel] [PATCH v2 0/3] block: check full backing filename when searching protocol filenames

[Jeff Cody]

Differences from v1:

Patch 1: set local_error = NULL after freeing (Thanks Eric)
Patch 2: new patch, groundwork for qemu-iotest in patch 3
Patch 3: qemu-iotest, as requested (Thanks Max)


Jeff Cody (3):
  block: check full backing filename when searching protocol filenames
  qemu-iotests: Don't create fifos / pidfiles with protocol paths
  qemu-iotest: test to lookup protocol-based image with relative backing

 block.c  | 13 ++
 tests/qemu-iotests/173   | 97 
 tests/qemu-iotests/173.out   | 12 +
 tests/qemu-iotests/common.config |  6 ++-
 tests/qemu-iotests/common.qemu   | 10 ++---
 tests/qemu-iotests/common.rc |  6 +--
 tests/qemu-iotests/group |  1 +
 7 files changed, 135 insertions(+), 10 deletions(-)
 create mode 100755 tests/qemu-iotests/173
 create mode 100644 tests/qemu-iotests/173.out

-- 
2.9.3

[Qemu-devel] [PATCH v2 1/3] block: check full backing filename when searching protocol filenames

[Jeff Cody]

In bdrv_find_backing_image(), if we are searching an image for a backing
file that contains a protocol, we currently only compare unmodified
paths.

However, some management software will change the backing filename to be
a relative filename in a path.  QEMU is able to handle this fine,
because internally it will use path_combine to put together the full
protocol URI.

However, this can lead to an inability to match an image during a QAPI
command that needs to use bdrv_find_backing_image() to find the image,
when it is searched by the full URI.

When searching for a protocol filename, if the straight comparison
fails, this patch will also compare against the full backing filename to
see if that is a match.

Signed-off-by: Jeff Cody 
---
 block.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/block.c b/block.c
index 39ddea3..d97c009 100644
--- a/block.c
+++ b/block.c
@@ -3145,6 +3145,7 @@ BlockDriverState 
*bdrv_find_backing_image(BlockDriverState *bs,
 int is_protocol = 0;
 BlockDriverState *curr_bs = NULL;
 BlockDriverState *retval = NULL;
+Error *local_error = NULL;
 
 if (!bs || !bs->drv || !backing_file) {
 return NULL;
@@ -3165,6 +3166,18 @@ BlockDriverState 
*bdrv_find_backing_image(BlockDriverState *bs,
 retval = curr_bs->backing->bs;
 break;
 }
+/* Also check against the full backing filename for the image */
+bdrv_get_full_backing_filename(curr_bs, backing_file_full, 
PATH_MAX,
+   &local_error);
+if (local_error == NULL) {
+if (strcmp(backing_file, backing_file_full) == 0) {
+retval = curr_bs->backing->bs;
+break;
+}
+} else {
+error_free(local_error);
+local_error = NULL;
+}
 } else {
 /* If not an absolute filename path, make it relative to the 
current
  * image's filename path */
-- 
2.9.3

Re: [Qemu-devel] [PATCH v4 1/9] ACPI: Add a function for building named qword entries

[Laszlo Ersek]

On 01/25/17 19:35, Michael S. Tsirkin wrote:
> On Wed, Jan 25, 2017 at 09:36:52AM -0800, Ben Warren wrote:
>> Hi Laszlo,
>>
>>
>> On Jan 24, 2017, at 7:55 PM, Laszlo Ersek  wrote:
>>
>> Hi Ben,
>>
>> sorry about being late to reviewing this series. I hope I can now spend
>> more time on it.
>>
>> - Please do not try to address my comments immediately. It's very
>> possible (even likely) that Igor, MST and myself could have different
>> opinions on things, so first please await agreement between your 
>> reviewers.
>>
>>
>> Thanks for the very detailed review.  I’ll give it a couple of days and then
>> start work on the suggested changes.
>>
>>
>> - I think you should have CC'd Igor and Michael directly. I'm adding
>> them to this reply; hopefully that will be enough for them to monitor
>> this series.
>>
>> - I'll likely be unable to review everything with 100% coverage; so
>> addressing (or sufficiently refuting) my comments might not guarantee
>> that the next version will be merged at once.
>>
>> With all that said:
>>
>> On 01/25/17 02:43, b...@skyportsystems.com wrote:
>>
>> From: Ben Warren 
>>
>> This is initially used to patch a 64-bit address into the VM 
>> Generation
>> ID SSDT
>>
>>
>> (1) I think this commit message line is overlong; I think we wrap at 74
>> chars or so. Not critical, but worth mentioning.
>>
>>
>>
>> Signed-off-by: Ben Warren 
>> ---
>> hw/acpi/aml-build.c | 28 
>> include/hw/acpi/aml-build.h |  4 
>> 2 files changed, 32 insertions(+)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index b2a1e40..dc4edc2 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -285,6 +285,34 @@ build_append_named_dword(GArray *array, const 
>> char
>> *name_format, ...)
>> return offset;
>> }
>>
>> +/*
>> + * Build NAME(, 0x) where 0x is encoded as a
>> qword,
>> + * and return the offset to 0x for runtime patching.
>> + *
>> + * Warning: runtime patching is best avoided. Only use this as
>> + * a replacement for DataTableRegion (for guests that don't
>> + * support it).
>> + */
> 
> only one comment: QWords first appeared in ACPI 2.0 and
> XP does not like them. Not strictly a blocker as people can
> avoid using the feature, but nice to have.

Does XP have a driver for VMGENID?

If not, then I'd prefer to stick with the qword VGIA.

> Will either UEFI or seabios allocate
> memory outside 4G range? If not you do not need a qword.

Good point (assuming XP has a driver for VMGENID).

OVMF keeps all such allocations (i.e., for COMMAND_ALLOCATE and the
upcoming COMMAND_ALLOCATE_RETURN_ADDR) under 4GB, so as far as OVMF is
concerned, using a dword for the VGIA named object should be fine.
Accordingly, a 4-byte wide ADD_POINTER command should be used for
patching VGIA.

Considering the fw_cfg file that receives the address, and
COMMAND_ALLOCATE_RETURN_ADDR more generally, I'd still prefer if those
stayed 8-byte wide, regardless of XP's support for VMGENID.


Hm... It looks like VMGENID *can* be consumed on Windows XP SP3, as long
as "Hyper-V integration services" are installed:

https://msdn.microsoft.com/en-us/library/jj643357(v=vs.85).aspx

The virtual machine must be running a guest operating system that
has support for the virtual machine generation identifier.
Currently, these are the following.
The following operating systems have native support for the virtual
machine generation identifier.
  [...]

The following operating can be used as the guest operating system
if the Hyper-V integration services from Windows 8 or Windows
Server 2012 are installed.

  [...]
  * Windows XP with Service Pack 3 (SP3)

Additionally, under
:

Supported Windows client guest operating systems

Windows XP with   [...] Install the integration  [...]
Service Pack 3 (SP3)services after you set
up the operating system
in the virtual machine.

This seems to be consistent with the VMGENID spec requirement that the
ADDR method return a package of two 32-bit integers, not a single 64-bit
one.

So, I agree with using a dword for VGIA (and a matching 4-byte wide
ADD_POINTER command).

But, again, I'd like to keep COMMAND_ALLOCATE_RETURN_ADDR 8-byte wide.
In the future we might introduce more allocation hints (for the "zone"
field) that would enable the firmware to allocate from the full 64-bit
address space.

NB, I didn't check SeaBIOS (should be easy for Ben); AFAIR it only uses
16-bit and 32-bit modes.

Thanks!
Laszlo

> 
> 
> 
> 
>>
>> (2) Since we're addin

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Fix crash when running without CPU

[Alistair Francis]

On Wed, Jan 25, 2017 at 12:52 PM, Laurent Vivier  wrote:
> Le 25/01/2017 à 21:45, Thomas Huth a écrit :
>> When running QEMU with "-M none -device loader,file=kernel.elf", it
>> currently crashes with a segmentation fault, because the "none"-machine
>> does not have any CPU by default and the generic loader code tries
>> to dereference s->cpu. Fix it by adding an appropriate check for a
>> NULL pointer.
>>
>> Reported-by: Laurent Vivier 
>> Signed-off-by: Thomas Huth 
>> ---
>>  hw/core/generic-loader.c | 9 +
>>  1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
>> index 58f1f02..4601267 100644
>> --- a/hw/core/generic-loader.c
>> +++ b/hw/core/generic-loader.c
>> @@ -137,20 +137,21 @@ static void generic_loader_realize(DeviceState *dev, 
>> Error **errp)
>>  #endif
>>
>>  if (s->file) {
>> +AddressSpace *as = s->cpu ? s->cpu->as :  NULL;
>
> Should we just abort if s->cpu is NULL?

I agree, what is the use case where you are loading images without a CPU?

If there is a use case (maybe some KVM thing?) then this patch looks fine to me.

Thanks,

Alistair

>
> Laurent
>
>

Re: [Qemu-devel] [PATCH] qxl: switch to constants within BUILD_BUG_ON

[Eric Blake]

On 01/25/2017 03:52 PM, Michael S. Tsirkin wrote:
> We are switching BUILD_BUG_ON to verify that it's parameter is a
> compile-time constant, and it turns out that some gcc versions
> (specifically gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609) are
> not smart enough to figure it out for expressions involving local
> variables. This is harmless but means that the check is ineffective for
> these platforms.  To fix, replace variables with macros.
> 
> Reported-by: Peter Maydell 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/display/qxl.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)

Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 3/3] linux-user: define correct UTS machine name for hppa

[Peter Maydell]

On 25 January 2017 at 21:56, Laurent Vivier  wrote:
> the correct UTS machine name (as expected by systemd) is "parisc",
> not "hppa".
>
> Signed-off-by: Laurent Vivier 
> ---
>  linux-user/uname.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/linux-user/uname.c b/linux-user/uname.c
> index 313b79d..daad002 100644
> --- a/linux-user/uname.c
> +++ b/linux-user/uname.c
> @@ -63,6 +63,8 @@ const char *cpu_to_uname_machine(void *cpu_env)
>  return "i586";
>  }
>  return "i686";
> +#elif defined(TARGET_HPPA)
> +return "parisc";
>  #else
>  /* default is #define-d in each arch/ subdir */
>  return UNAME_MACHINE;

This function is for when the uname string has to be
determined at runtime. If this target has a fixed
string, then you should just set UNAME_MACHINE correctly
in linux-user/$ARCH/target_syscall.h.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/3] linux-user: some patches for hppa

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v2 0/3] linux-user: some patches for hppa
Message-id: 20170125215637.2992-1-laur...@vivier.eu

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20170125215637.2992-1-laur...@vivier.eu -> 
patchew/20170125215637.2992-1-laur...@vivier.eu
Switched to a new branch 'test'
c426c84 linux-user: define correct UTS machine name for hppa
1566132 linux-user: fix "apt-get update" on linux-user hppa
97a848f linux-user: add hppa magic numbers in qemu-binfmt-conf.sh

=== OUTPUT BEGIN ===
Checking PATCH 1/3: linux-user: add hppa magic numbers in qemu-binfmt-conf.sh...
ERROR: line over 90 characters
#34: FILE: scripts/qemu-binfmt-conf.sh:95:
+hppa_magic='\x7f\x45\x4c\x46\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x0f'

ERROR: line over 90 characters
#35: FILE: scripts/qemu-binfmt-conf.sh:96:
+hppa_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'

total: 2 errors, 0 warnings, 22 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/3: linux-user: fix "apt-get update" on linux-user hppa...
Checking PATCH 3/3: linux-user: define correct UTS machine name for hppa...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PULL 00/16] virtio, vhost, pci: fixes, features

[Michael S. Tsirkin]

On Wed, Jan 25, 2017 at 07:24:03PM +, Peter Maydell wrote:
> On 25 January 2017 at 18:08, Peter Maydell  wrote:
> > On 25 January 2017 at 18:02, Marcel Apfelbaum
> >  wrote:
> >> Strange, I re-based in the same day I posted and compiled just fine...
> >> However, I get the same error using today's master.
> >>
> >> I'll fix and re-post the series.
> >
> > mst has done a rebase of the pull request which I'll retry later.
> 
> Results:
> /home/petmay01/linaro/qemu-for-merges/hw/display/qxl.c: In function
> ‘qxl_rom_size’:
> /home/petmay01/linaro/qemu-for-merges/hw/display/qxl.c:313:20: error:
> bit-field ‘’ width not an integer constant
>  QEMU_BUILD_BUG_ON(required_rom_size > rom_size);
> ^
> 
> x86-64 linux host; I guess the usage in this file conflicts
> with your series' rephrasing of the macro for some versions
> of gcc. This is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609

I guess what happened previously is that gcc did not actually
check this size at all. I posted a patch to rewrite using macros.
Trivial but I still will give maintainer a bit of time to take a look.


> If I rewrite the build-assert in qxl.c as
> QEMU_BUILD_BUG_ON(sizeof(QXLRom) + sizeof(QXLModes) +
> sizeof(qxl_modes) > 8192);
> it compiles OK (and the build then runs into the error below).
> 
> 
> All the other build hosts fell over like this:
> 
> /home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:9: error:
> implicit declaration of function ‘VMSTATE_PCIE_DEVICE’
> [-Werror=implicit-function-declaration]
>  VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
>  ^
> /home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:29: error:
> ‘parent_obj’ undeclared here (not in a function)
>  VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
>  ^
> /home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:63: error:
> expected expression before ‘PCIESlot’
>  VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
> 
> thanks
> -- PMM

[Qemu-devel] [PATCH v2 3/3] linux-user: define correct UTS machine name for hppa

[Laurent Vivier]

the correct UTS machine name (as expected by systemd) is "parisc",
not "hppa".

Signed-off-by: Laurent Vivier 
---
 linux-user/uname.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/linux-user/uname.c b/linux-user/uname.c
index 313b79d..daad002 100644
--- a/linux-user/uname.c
+++ b/linux-user/uname.c
@@ -63,6 +63,8 @@ const char *cpu_to_uname_machine(void *cpu_env)
 return "i586";
 }
 return "i686";
+#elif defined(TARGET_HPPA)
+return "parisc";
 #else
 /* default is #define-d in each arch/ subdir */
 return UNAME_MACHINE;
-- 
2.9.3

[Qemu-devel] [PATCH v2 2/3] linux-user: fix "apt-get update" on linux-user hppa

[Laurent Vivier]

apt-get was hanging on linux-user hppa.

strace has shown the netlink data stream was not correctly byte swapped.

It appears the fd translator function is unregistered just after it
has been registered, so the translator function is not called.

This patch removes the fd_trans_unregister() after the do_socket()
in the TARGET_NR_socket case.

This fd_trans_unregister() was added by commit
e36800c linux-user: add signalfd/signalfd4 syscalls
when do_socket() was not registering any fd translator.
And as now it is, we must remove this fd_trans_unregister() to keep them.

Reported-by: John Paul Adrian Glaubitz 
Signed-off-by: Laurent Vivier 
Tested-by: John Paul Adrian Glaubitz 
---
 linux-user/syscall.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11a311f..9be8e95 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9343,7 +9343,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_socket
 case TARGET_NR_socket:
 ret = do_socket(arg1, arg2, arg3);
-fd_trans_unregister(ret);
 break;
 #endif
 #ifdef TARGET_NR_socketpair
-- 
2.9.3

[Qemu-devel] [PATCH v2 1/3] linux-user: add hppa magic numbers in qemu-binfmt-conf.sh

[Laurent Vivier]

As we have now a linux-user HPPA target, we can add it to the list of
supported targets in qemu-binfmt-conf.sh

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
---
 scripts/qemu-binfmt-conf.sh | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index de4d1c1..0f1aa63 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -1,9 +1,10 @@
 #!/bin/sh
-# enable automatic i386/ARM/M68K/MIPS/SPARC/PPC/s390 program execution by the 
kernel
+# enable automatic i386/ARM/M68K/MIPS/SPARC/PPC/s390/HPPA
+# program execution by the kernel
 
 qemu_target_list="i386 i486 alpha arm sparc32plus ppc ppc64 ppc64le m68k \
 mips mipsel mipsn32 mipsn32el mips64 mips64el \
-sh4 sh4eb s390x aarch64"
+sh4 sh4eb s390x aarch64 hppa"
 
 
i386_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00'
 
i386_mask='\xff\xff\xff\xff\xff\xfe\xfe\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'
@@ -91,6 +92,10 @@ 
aarch64_magic='\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x
 
aarch64_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'
 aarch64_family=arm
 
+hppa_magic='\x7f\x45\x4c\x46\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x0f'
+hppa_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
+hppa_family=hppa
+
 qemu_get_family() {
 cpu=${HOST_ARCH:-$(uname -m)}
 case "$cpu" in
-- 
2.9.3

[Qemu-devel] [PATCH v2 0/3] linux-user: some patches for hppa

[Laurent Vivier]

This short series enables hppa in qemu-binfmt-conf.sh, and fixes
a bug in the netlink functions that has been found by Adrian Glaubitz
while he was testing a debian chroot with qemu-hppa. I think the problem
doesn't appear on other architectures I have tested as they should use
NR_socketcall syscall instead of NR_socket syscall.

v2:
added the hppa UTS machine name fix
added R-b and T-b

Laurent Vivier (3):
  linux-user: add hppa magic numbers in qemu-binfmt-conf.sh
  linux-user: fix "apt-get update" on linux-user hppa
  linux-user: define correct UTS machine name for hppa

 linux-user/syscall.c| 1 -
 linux-user/uname.c  | 2 ++
 scripts/qemu-binfmt-conf.sh | 9 +++--
 3 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH] qxl: switch to constants within BUILD_BUG_ON

[Michael S. Tsirkin]

We are switching BUILD_BUG_ON to verify that it's parameter is a
compile-time constant, and it turns out that some gcc versions
(specifically gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609) are
not smart enough to figure it out for expressions involving local
variables. This is harmless but means that the check is ineffective for
these platforms.  To fix, replace variables with macros.

Reported-by: Peter Maydell 
Signed-off-by: Michael S. Tsirkin 
---
 hw/display/qxl.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 62d0c80..af4c0ca 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -306,12 +306,11 @@ void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
 
 static ram_addr_t qxl_rom_size(void)
 {
-uint32_t required_rom_size = sizeof(QXLRom) + sizeof(QXLModes) +
- sizeof(qxl_modes);
-uint32_t rom_size = 8192; /* two pages */
+#define QXL_REQUIRED_SZ (sizeof(QXLRom) + sizeof(QXLModes) + sizeof(qxl_modes))
+#define QXL_ROM_SZ 8192
 
-QEMU_BUILD_BUG_ON(required_rom_size > rom_size);
-return rom_size;
+QEMU_BUILD_BUG_ON(QXL_REQUIRED_SZ > QXL_ROM_SZ);
+return QXL_ROM_SZ;
 }
 
 static void init_qxl_rom(PCIQXLDevice *d)
-- 
MST

Re: [Qemu-devel] [PATCH 1/2] linux-user: add hppa magic numbers in qemu-binfmt-conf.sh

[Richard Henderson]

On 01/25/2017 12:38 PM, Laurent Vivier wrote:
> As we have now a linux-user HPPA target, we can add it to the list of
> supported targets in qemu-binfmt-conf.sh
> 
> Signed-off-by: Laurent Vivier 
> ---
>  scripts/qemu-binfmt-conf.sh | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH] hw/core/generic-loader: Fix crash when running without CPU

[Laurent Vivier]

Le 25/01/2017 à 21:45, Thomas Huth a écrit :
> When running QEMU with "-M none -device loader,file=kernel.elf", it
> currently crashes with a segmentation fault, because the "none"-machine
> does not have any CPU by default and the generic loader code tries
> to dereference s->cpu. Fix it by adding an appropriate check for a
> NULL pointer.
> 
> Reported-by: Laurent Vivier 
> Signed-off-by: Thomas Huth 
> ---
>  hw/core/generic-loader.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
> index 58f1f02..4601267 100644
> --- a/hw/core/generic-loader.c
> +++ b/hw/core/generic-loader.c
> @@ -137,20 +137,21 @@ static void generic_loader_realize(DeviceState *dev, 
> Error **errp)
>  #endif
>  
>  if (s->file) {
> +AddressSpace *as = s->cpu ? s->cpu->as :  NULL;

Should we just abort if s->cpu is NULL?

Laurent

Re: [Qemu-devel] [PATCH 0/2] linux-user: some patches for hppa

[Laurent Vivier]

Le 25/01/2017 à 21:42, no-re...@patchew.org a écrit :
> Hi,
> 
> Your series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Subject: [Qemu-devel] [PATCH 0/2] linux-user: some patches for hppa
> Message-id: 20170125203852.19254-1-laur...@vivier.eu
...
> Checking PATCH 1/2: linux-user: add hppa magic numbers in 
> qemu-binfmt-conf.sh...
> ERROR: line over 90 characters
> #33: FILE: scripts/qemu-binfmt-conf.sh:95:
> +hppa_magic='\x7f\x45\x4c\x46\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x0f'
> 
> ERROR: line over 90 characters
> #34: FILE: scripts/qemu-binfmt-conf.sh:96:
> +hppa_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
>

This error is not relevant for this file...

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 2/2] linux-user: fix "apt-get update" on linux-user hppa

[John Paul Adrian Glaubitz]

On 01/25/2017 09:38 PM, Laurent Vivier wrote:
> Reported-by: John Paul Adrian Glaubitz 
> Signed-off-by: Laurent Vivier 

Tested-by: John Paul Adrian Glaubitz 

CC'ing Helge Deller so he can verify this as well.

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

[Qemu-devel] [PATCH] hw/core/generic-loader: Fix crash when running without CPU

[Thomas Huth]

When running QEMU with "-M none -device loader,file=kernel.elf", it
currently crashes with a segmentation fault, because the "none"-machine
does not have any CPU by default and the generic loader code tries
to dereference s->cpu. Fix it by adding an appropriate check for a
NULL pointer.

Reported-by: Laurent Vivier 
Signed-off-by: Thomas Huth 
---
 hw/core/generic-loader.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
index 58f1f02..4601267 100644
--- a/hw/core/generic-loader.c
+++ b/hw/core/generic-loader.c
@@ -137,20 +137,21 @@ static void generic_loader_realize(DeviceState *dev, 
Error **errp)
 #endif
 
 if (s->file) {
+AddressSpace *as = s->cpu ? s->cpu->as :  NULL;
+
 if (!s->force_raw) {
 size = load_elf_as(s->file, NULL, NULL, &entry, NULL, NULL,
-   big_endian, 0, 0, 0, s->cpu->as);
+   big_endian, 0, 0, 0, as);
 
 if (size < 0) {
 size = load_uimage_as(s->file, &entry, NULL, NULL, NULL, NULL,
-  s->cpu->as);
+  as);
 }
 }
 
 if (size < 0 || s->force_raw) {
 /* Default to the maximum size being the machine's ram size */
-size = load_image_targphys_as(s->file, s->addr, ram_size,
-  s->cpu->as);
+size = load_image_targphys_as(s->file, s->addr, ram_size, as);
 } else {
 s->addr = entry;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/2] linux-user: some patches for hppa

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH 0/2] linux-user: some patches for hppa
Message-id: 20170125203852.19254-1-laur...@vivier.eu

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20170125203852.19254-1-laur...@vivier.eu -> 
patchew/20170125203852.19254-1-laur...@vivier.eu
Switched to a new branch 'test'
4995cf7 linux-user: fix "apt-get update" on linux-user hppa
2d71add linux-user: add hppa magic numbers in qemu-binfmt-conf.sh

=== OUTPUT BEGIN ===
Checking PATCH 1/2: linux-user: add hppa magic numbers in qemu-binfmt-conf.sh...
ERROR: line over 90 characters
#33: FILE: scripts/qemu-binfmt-conf.sh:95:
+hppa_magic='\x7f\x45\x4c\x46\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x0f'

ERROR: line over 90 characters
#34: FILE: scripts/qemu-binfmt-conf.sh:96:
+hppa_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'

total: 2 errors, 0 warnings, 22 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/2: linux-user: fix "apt-get update" on linux-user hppa...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PATCH 0/2] linux-user: some patches for hppa

[Laurent Vivier]

This short series enables hppa in qemu-binfmt-conf.sh, and fixes
a bug in the netlink functions that has been found by Adrian Glaubitz
while he was testing a debian chroot with qemu-hppa. I think the problem
doesn't appear on other architectures I have tested as they should use
NR_socketcall syscall instead of NR_socket syscall.

Laurent Vivier (2):
  linux-user: add hppa magic numbers in qemu-binfmt-conf.sh
  linux-user: fix "apt-get update" on linux-user hppa

 linux-user/syscall.c| 1 -
 scripts/qemu-binfmt-conf.sh | 9 +++--
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 2/2] linux-user: fix "apt-get update" on linux-user hppa

[Laurent Vivier]

apt-get was hanging on linux-user hppa.

strace has shown the netlink data stream was not correctly byte swapped.

It appears the fd translator function is unregistered just after it
has been registered, so the translator function is not called.

This patch removes the fd_trans_unregister() after the do_socket()
in the TARGET_NR_socket case.

This fd_trans_unregister() was added by commit
e36800c linux-user: add signalfd/signalfd4 syscalls
when do_socket() was not registering any fd translator.
And as now it is, we must remove this fd_trans_unregister() to keep them.

Reported-by: John Paul Adrian Glaubitz 
Signed-off-by: Laurent Vivier 
---
 linux-user/syscall.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 11a311f..9be8e95 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9343,7 +9343,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 #ifdef TARGET_NR_socket
 case TARGET_NR_socket:
 ret = do_socket(arg1, arg2, arg3);
-fd_trans_unregister(ret);
 break;
 #endif
 #ifdef TARGET_NR_socketpair
-- 
2.9.3

[Qemu-devel] [PATCH 1/2] linux-user: add hppa magic numbers in qemu-binfmt-conf.sh

[Laurent Vivier]

As we have now a linux-user HPPA target, we can add it to the list of
supported targets in qemu-binfmt-conf.sh

Signed-off-by: Laurent Vivier 
---
 scripts/qemu-binfmt-conf.sh | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index de4d1c1..0f1aa63 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -1,9 +1,10 @@
 #!/bin/sh
-# enable automatic i386/ARM/M68K/MIPS/SPARC/PPC/s390 program execution by the 
kernel
+# enable automatic i386/ARM/M68K/MIPS/SPARC/PPC/s390/HPPA
+# program execution by the kernel
 
 qemu_target_list="i386 i486 alpha arm sparc32plus ppc ppc64 ppc64le m68k \
 mips mipsel mipsn32 mipsn32el mips64 mips64el \
-sh4 sh4eb s390x aarch64"
+sh4 sh4eb s390x aarch64 hppa"
 
 
i386_magic='\x7fELF\x01\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x03\x00'
 
i386_mask='\xff\xff\xff\xff\xff\xfe\xfe\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'
@@ -91,6 +92,10 @@ 
aarch64_magic='\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x
 
aarch64_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff'
 aarch64_family=arm
 
+hppa_magic='\x7f\x45\x4c\x46\x01\x02\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x0f'
+hppa_mask='\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff'
+hppa_family=hppa
+
 qemu_get_family() {
 cpu=${HOST_ARCH:-$(uname -m)}
 case "$cpu" in
-- 
2.9.3

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Alex Williamson]

On Wed, 25 Jan 2017 20:42:19 +0100
Paolo Bonzini  wrote:

> On 25/01/2017 19:36, Alex Williamson wrote:
> >> It depends of what happens if they aren't.  I think it's fine (see other
> >> message), but taking a reference for each mapping entry isn't so easy
> >> because the unmap case doesn't know the old memory region.  
> > If we held a reference to the memory region from the mapping path and
> > walk the IOMMU page table to generate the unmap, then we really should
> > get to the same original memory region, right?  The vfio iommu notifier
> > should only be mapping native page sizes of the IOMMU, 4k/2M/1G.  The
> > problem is that it's a lot of overhead to flush the entire address
> > space that way vs the single invalidation Peter is trying to enable
> > here.  It's actually similar to how the type1 iommu works in the kernel
> > though, we can unmap by iova because we ask the iommu for the iova->pfn
> > translation in order to unpin the page.  
> 
> But in the kernel you can trust the IOMMU page tables because you build
> them, here instead it's the guest's page tables that you'd walk, right?
> You cannot trust the guest.

Yes, you're right, we're not shadowing the vt-d page tables, we're
working on the explicit invalidation model.  So there could be
anything, or nothing in the page tables when we go to try to lookup the
unref.  So clearly taking that reference without a shadow page table
would be the wrong approach.  Thanks,

Alex

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Paolo Bonzini]

On 25/01/2017 19:36, Alex Williamson wrote:
>> It depends of what happens if they aren't.  I think it's fine (see other
>> message), but taking a reference for each mapping entry isn't so easy
>> because the unmap case doesn't know the old memory region.
> If we held a reference to the memory region from the mapping path and
> walk the IOMMU page table to generate the unmap, then we really should
> get to the same original memory region, right?  The vfio iommu notifier
> should only be mapping native page sizes of the IOMMU, 4k/2M/1G.  The
> problem is that it's a lot of overhead to flush the entire address
> space that way vs the single invalidation Peter is trying to enable
> here.  It's actually similar to how the type1 iommu works in the kernel
> though, we can unmap by iova because we ask the iommu for the iova->pfn
> translation in order to unpin the page.

But in the kernel you can trust the IOMMU page tables because you build
them, here instead it's the guest's page tables that you'd walk, right?
You cannot trust the guest.

Paolo

Re: [Qemu-devel] [PATCH V4 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

On 01/25/2017 09:15 PM, Michael S. Tsirkin wrote:

On Wed, Jan 25, 2017 at 09:03:55PM +0200, Marcel Apfelbaum wrote:

v3 -> v4:
  - Rebased on master.



No need for this. Pls check that my pci branch is ok.



I see hw/pci-bridge/gen_pcie_root_port.c still uses the macro 
VMSTATE_PCIE_DEVICE
removed by the commit 20daa90 (PCI/migration merge vmstate_pci_device and 
vmstate_pcie_device),
so it can't work.

I couldn't check the issue because the pci tree compilation fails on:

qemu/hw/display/qxl.c: In function ‘qxl_rom_size’:
qemu/hw/display/qxl.c:313:20: error: bit-field ‘’ width not an 
integer constant
 QEMU_BUILD_BUG_ON(required_rom_size > rom_size);
^
qemu/rules.mak:64: recipe for target 'hw/display/qxl.o' failed

Thanks,
Marcel




v2 -> v3:
 - Keep only the root port base class code in pcie_root_port.c (Michael)
 - Use msix for the generic root port implementation (Michael and Gerd)
   - The task required some refactoring like having some common
 init/uninit interrupts functions to be implemented by both
 generic and Intel Root Ports.

v1 -> v2:
 - Rebased on master.

The Generic Root Port behaves the same as the
Intel's IOH device with id 3420, without having
Intel specific attributes.

The device has two purposes:
 (1) Can be used on both X86 and ARM machines.
 (2) It will allow us to tweak the behaviour
(e.g add vendor-specific PCI capabilities)
 - something that obviously cannot be done
   on a known device.

Patch 1/3: Introduce a base class for Root Ports - most of the code
   is migrated from IOH3420 implementation.
Patch 2/3: Derives the IOH3420 from the new base class
Patch 3/3: Introduces the generic Root Port.

Tested with Linux and Windows guests only on x86 hosts.

Marcel Apfelbaum (3):
  hw/pcie: Introduce a base class for PCI Express Root Ports
  hw/ioh3420: derive from PCI Express Root Port base class
  hw/pcie: Introduce Generic PCI Express Root Port

 default-configs/arm-softmmu.mak|   1 +
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/pci-bridge/Makefile.objs|   1 +
 hw/pci-bridge/gen_pcie_root_port.c |  88 +++
 hw/pci-bridge/ioh3420.c| 121 --
 hw/pci-bridge/pcie_root_port.c | 171 +
 include/hw/pci/pci.h   |   1 +
 include/hw/pci/pcie_port.h |  19 +
 9 files changed, 298 insertions(+), 106 deletions(-)
 create mode 100644 hw/pci-bridge/gen_pcie_root_port.c
 create mode 100644 hw/pci-bridge/pcie_root_port.c

--
2.5.5

Re: [Qemu-devel] [PULL 00/16] virtio, vhost, pci: fixes, features

[Peter Maydell]

On 25 January 2017 at 18:08, Peter Maydell  wrote:
> On 25 January 2017 at 18:02, Marcel Apfelbaum
>  wrote:
>> Strange, I re-based in the same day I posted and compiled just fine...
>> However, I get the same error using today's master.
>>
>> I'll fix and re-post the series.
>
> mst has done a rebase of the pull request which I'll retry later.

Results:
/home/petmay01/linaro/qemu-for-merges/hw/display/qxl.c: In function
‘qxl_rom_size’:
/home/petmay01/linaro/qemu-for-merges/hw/display/qxl.c:313:20: error:
bit-field ‘’ width not an integer constant
 QEMU_BUILD_BUG_ON(required_rom_size > rom_size);
^

x86-64 linux host; I guess the usage in this file conflicts
with your series' rephrasing of the macro for some versions
of gcc. This is gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609

If I rewrite the build-assert in qxl.c as
QEMU_BUILD_BUG_ON(sizeof(QXLRom) + sizeof(QXLModes) +
sizeof(qxl_modes) > 8192);
it compiles OK (and the build then runs into the error below).


All the other build hosts fell over like this:

/home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:9: error:
implicit declaration of function ‘VMSTATE_PCIE_DEVICE’
[-Werror=implicit-function-declaration]
 VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
 ^
/home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:29: error:
‘parent_obj’ undeclared here (not in a function)
 VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
 ^
/home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:63: error:
expected expression before ‘PCIESlot’
 VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),

thanks
-- PMM

Re: [Qemu-devel] [PATCH V4 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Michael S. Tsirkin]

On Wed, Jan 25, 2017 at 09:03:55PM +0200, Marcel Apfelbaum wrote:
> v3 -> v4:
>   - Rebased on master.


No need for this. Pls check that my pci branch is ok.


> v2 -> v3:
>  - Keep only the root port base class code in pcie_root_port.c (Michael)
>  - Use msix for the generic root port implementation (Michael and Gerd)
>- The task required some refactoring like having some common
>  init/uninit interrupts functions to be implemented by both
>  generic and Intel Root Ports.
> 
> v1 -> v2:
>  - Rebased on master.
> 
> The Generic Root Port behaves the same as the
> Intel's IOH device with id 3420, without having
> Intel specific attributes.
> 
> The device has two purposes:
>  (1) Can be used on both X86 and ARM machines.
>  (2) It will allow us to tweak the behaviour
> (e.g add vendor-specific PCI capabilities)
>  - something that obviously cannot be done
>on a known device.
> 
> Patch 1/3: Introduce a base class for Root Ports - most of the code
>is migrated from IOH3420 implementation.
> Patch 2/3: Derives the IOH3420 from the new base class
> Patch 3/3: Introduces the generic Root Port.
> 
> Tested with Linux and Windows guests only on x86 hosts.
> 
> Marcel Apfelbaum (3):
>   hw/pcie: Introduce a base class for PCI Express Root Ports
>   hw/ioh3420: derive from PCI Express Root Port base class
>   hw/pcie: Introduce Generic PCI Express Root Port
> 
>  default-configs/arm-softmmu.mak|   1 +
>  default-configs/i386-softmmu.mak   |   1 +
>  default-configs/x86_64-softmmu.mak |   1 +
>  hw/pci-bridge/Makefile.objs|   1 +
>  hw/pci-bridge/gen_pcie_root_port.c |  88 +++
>  hw/pci-bridge/ioh3420.c| 121 --
>  hw/pci-bridge/pcie_root_port.c | 171 
> +
>  include/hw/pci/pci.h   |   1 +
>  include/hw/pci/pcie_port.h |  19 +
>  9 files changed, 298 insertions(+), 106 deletions(-)
>  create mode 100644 hw/pci-bridge/gen_pcie_root_port.c
>  create mode 100644 hw/pci-bridge/pcie_root_port.c
> 
> -- 
> 2.5.5

[Qemu-devel] [PATCH V4 2/3] hw/ioh3420: derive from PCI Express Root Port base class

[Marcel Apfelbaum]

Preserve only Intel specific details.

Tested-by: Andrea Bolognani 
Signed-off-by: Marcel Apfelbaum 
---
 hw/pci-bridge/ioh3420.c | 121 ++--
 1 file changed, 15 insertions(+), 106 deletions(-)

diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index 0eef87a..da4e5bd 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -61,119 +61,28 @@ static uint8_t ioh3420_aer_vector(const PCIDevice *d)
 return 0;
 }
 
-static void ioh3420_aer_vector_update(PCIDevice *d)
+static int ioh3420_interrupts_init(PCIDevice *d, Error **errp)
 {
-pcie_aer_root_set_vector(d, ioh3420_aer_vector(d));
-}
-
-static void ioh3420_write_config(PCIDevice *d,
-   uint32_t address, uint32_t val, int len)
-{
-uint32_t root_cmd =
-pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
-
-pci_bridge_write_config(d, address, val, len);
-ioh3420_aer_vector_update(d);
-pcie_cap_slot_write_config(d, address, val, len);
-pcie_aer_write_config(d, address, val, len);
-pcie_aer_root_write_config(d, address, val, len, root_cmd);
-}
-
-static void ioh3420_reset(DeviceState *qdev)
-{
-PCIDevice *d = PCI_DEVICE(qdev);
-
-ioh3420_aer_vector_update(d);
-pcie_cap_root_reset(d);
-pcie_cap_deverr_reset(d);
-pcie_cap_slot_reset(d);
-pcie_cap_arifwd_reset(d);
-pcie_aer_root_reset(d);
-pci_bridge_reset(qdev);
-pci_bridge_disable_base_limit(d);
-}
-
-static int ioh3420_initfn(PCIDevice *d)
-{
-PCIEPort *p = PCIE_PORT(d);
-PCIESlot *s = PCIE_SLOT(d);
 int rc;
-Error *err = NULL;
-
-pci_config_set_interrupt_pin(d->config, 1);
-pci_bridge_initfn(d, TYPE_PCIE_BUS);
-pcie_port_init_reg(d);
-
-rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
-   IOH_EP_SSVID_SVID, IOH_EP_SSVID_SSID);
-if (rc < 0) {
-goto err_bridge;
-}
+Error *local_err = NULL;
 
 rc = msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR,
   IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
-  IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT, &err);
+  IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
+  &local_err);
 if (rc < 0) {
 assert(rc == -ENOTSUP);
-error_report_err(err);
-goto err_bridge;
+error_propagate(errp, local_err);
 }
 
-rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, p->port);
-if (rc < 0) {
-goto err_msi;
-}
-
-pcie_cap_arifwd_init(d);
-pcie_cap_deverr_init(d);
-pcie_cap_slot_init(d, s->slot);
-pcie_cap_root_init(d);
-
-pcie_chassis_create(s->chassis);
-rc = pcie_chassis_add_slot(s);
-if (rc < 0) {
-goto err_pcie_cap;
-}
-
-rc = pcie_aer_init(d, PCI_ERR_VER, IOH_EP_AER_OFFSET,
-   PCI_ERR_SIZEOF, &err);
-if (rc < 0) {
-error_report_err(err);
-goto err;
-}
-pcie_aer_root_init(d);
-ioh3420_aer_vector_update(d);
-
-return 0;
-
-err:
-pcie_chassis_del_slot(s);
-err_pcie_cap:
-pcie_cap_exit(d);
-err_msi:
-msi_uninit(d);
-err_bridge:
-pci_bridge_exitfn(d);
 return rc;
 }
 
-static void ioh3420_exitfn(PCIDevice *d)
+static void ioh3420_interrupts_uninit(PCIDevice *d)
 {
-PCIESlot *s = PCIE_SLOT(d);
-
-pcie_aer_exit(d);
-pcie_chassis_del_slot(s);
-pcie_cap_exit(d);
 msi_uninit(d);
-pci_bridge_exitfn(d);
 }
 
-static Property ioh3420_props[] = {
-DEFINE_PROP_BIT(COMPAT_PROP_PCP, PCIDevice, cap_present,
-QEMU_PCIE_SLTCAP_PCP_BITNR, true),
-DEFINE_PROP_END_OF_LIST()
-};
-
 static const VMStateDescription vmstate_ioh3420 = {
 .name = "ioh-3240-express-root-port",
 .version_id = 1,
@@ -191,25 +100,25 @@ static void ioh3420_class_init(ObjectClass *klass, void 
*data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
 
-k->is_express = 1;
-k->is_bridge = 1;
-k->config_write = ioh3420_write_config;
-k->init = ioh3420_initfn;
-k->exit = ioh3420_exitfn;
 k->vendor_id = PCI_VENDOR_ID_INTEL;
 k->device_id = PCI_DEVICE_ID_IOH_EPORT;
 k->revision = PCI_DEVICE_ID_IOH_REV;
-set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 dc->desc = "Intel IOH device id 3420 PCIE Root Port";
-dc->reset = ioh3420_reset;
 dc->vmsd = &vmstate_ioh3420;
-dc->props = ioh3420_props;
+rpc->aer_vector = ioh3420_aer_vector;
+rpc->interrupts_init = ioh3420_interrupts_init;
+rpc->interrupts_uninit = ioh3420_interrupts_uninit;
+rpc->exp_offset = IOH_EP_EXP_OFFSET;
+rpc->aer_offset = IOH_EP_AER_OFFSET;
+rpc->ssvid_offset = IOH_EP_SSVID_OFFSET;
+rpc->ssid = IOH_EP_SSVID_SSID;
 }
 
 static const TypeInfo ioh3420_info = {
 .name  = "ioh3420",
-.parent 

[Qemu-devel] [PATCH V4 3/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

The Generic Root Port behaves almost the same as the
Intel's IOH device with id 3420, without having
Intel specific attributes.

The device has two purposes:
 (1) Can be used on both X86 and ARM machines.
 (2) It will allow us to tweak the behaviour
(e.g add vendor-specific PCI capabilities)
 - something that obviously cannot be done
   on a known device.

Tested-by: Andrea Bolognani 
Signed-off-by: Marcel Apfelbaum 
---
 hw/pci-bridge/Makefile.objs|  2 +-
 hw/pci-bridge/gen_pcie_root_port.c | 88 ++
 include/hw/pci/pci.h   |  1 +
 3 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 hw/pci-bridge/gen_pcie_root_port.c

diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
index 4f2039f..85e8e39 100644
--- a/hw/pci-bridge/Makefile.objs
+++ b/hw/pci-bridge/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-y += pci_bridge_dev.o
 common-obj-y += pci_expander_bridge.o
-common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
+common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o gen_pcie_root_port.o
 common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
 common-obj-$(CONFIG_IOH3420) += ioh3420.o
 common-obj-$(CONFIG_I82801B11) += i82801b11.o
diff --git a/hw/pci-bridge/gen_pcie_root_port.c 
b/hw/pci-bridge/gen_pcie_root_port.c
new file mode 100644
index 000..185fd90
--- /dev/null
+++ b/hw/pci-bridge/gen_pcie_root_port.c
@@ -0,0 +1,88 @@
+/*
+ * Generic PCI Express Root Port emulation
+ *
+ * Copyright (C) 2017 Red Hat Inc
+ *
+ * Authors:
+ *   Marcel Apfelbaum 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/pci/msix.h"
+#include "hw/pci/pcie_port.h"
+
+#define TYPE_GEN_PCIE_ROOT_PORT"pcie-root-port"
+
+#define GEN_PCIE_ROOT_PORT_AER_OFFSET   0x100
+#define GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR   1
+
+static uint8_t gen_rp_aer_vector(const PCIDevice *d)
+{
+return 0;
+}
+
+static int gen_rp_interrupts_init(PCIDevice *d, Error **errp)
+{
+int rc;
+
+rc = msix_init_exclusive_bar(d, GEN_PCIE_ROOT_PORT_MSIX_NR_VECTOR, 0);
+
+if (rc < 0) {
+assert(rc == -ENOTSUP);
+error_setg(errp, "Unable to init msix vectors");
+} else {
+msix_vector_use(d, 0);
+}
+
+return rc;
+}
+
+static void gen_rp_interrupts_uninit(PCIDevice *d)
+{
+msix_uninit_exclusive_bar(d);
+}
+
+static const VMStateDescription vmstate_rp_dev = {
+.name = "pcie-root-port",
+.version_id = 1,
+.minimum_version_id = 1,
+.post_load = pcie_cap_slot_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
+VMSTATE_STRUCT(parent_obj.parent_obj.parent_obj.exp.aer_log,
+   PCIESlot, 0, vmstate_pcie_aer_log, PCIEAERLog),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static void gen_rp_dev_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+PCIERootPortClass *rpc = PCIE_ROOT_PORT_CLASS(klass);
+
+k->vendor_id = PCI_VENDOR_ID_REDHAT;
+k->device_id = PCI_DEVICE_ID_REDHAT_PCIE_RP;
+dc->desc = "PCI Express Root Port";
+dc->vmsd = &vmstate_rp_dev;
+rpc->aer_vector = gen_rp_aer_vector;
+rpc->interrupts_init = gen_rp_interrupts_init;
+rpc->interrupts_uninit = gen_rp_interrupts_uninit;
+rpc->aer_offset = GEN_PCIE_ROOT_PORT_AER_OFFSET;
+}
+
+static const TypeInfo gen_rp_dev_info = {
+.name  = TYPE_GEN_PCIE_ROOT_PORT,
+.parent= TYPE_PCIE_ROOT_PORT,
+.class_init= gen_rp_dev_class_init,
+};
+
+ static void gen_rp_register_types(void)
+ {
+type_register_static(&gen_rp_dev_info);
+ }
+ type_init(gen_rp_register_types)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 772692f..cbc1fdf 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -96,6 +96,7 @@
 #define PCI_DEVICE_ID_REDHAT_PXB 0x0009
 #define PCI_DEVICE_ID_REDHAT_BRIDGE_SEAT 0x000a
 #define PCI_DEVICE_ID_REDHAT_PXB_PCIE0x000b
+#define PCI_DEVICE_ID_REDHAT_PCIE_RP 0x000c
 #define PCI_DEVICE_ID_REDHAT_QXL 0x0100
 
 #define FMT_PCIBUS  PRIx64
-- 
2.5.5

[Qemu-devel] [PATCH V4 1/3] hw/pcie: Introduce a base class for PCI Express Root Ports

[Marcel Apfelbaum]

The 'base' PCI Express Root Port includes
the common code to be re-used for all
Root Ports implementations. Most of the code
was taken from the current implementation
of Intel's IOH 3420 Root Port.

Tested-by: Andrea Bolognani 
Signed-off-by: Marcel Apfelbaum 
---
 default-configs/arm-softmmu.mak|   1 +
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/pci-bridge/Makefile.objs|   1 +
 hw/pci-bridge/pcie_root_port.c | 171 +
 include/hw/pci/pcie_port.h |  19 +
 6 files changed, 194 insertions(+)
 create mode 100644 hw/pci-bridge/pcie_root_port.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 6de3e16..6f2a180 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -108,6 +108,7 @@ CONFIG_FSL_IMX25=y
 
 CONFIG_IMX_I2C=y
 
+CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak
index 0b51360..9288838 100644
--- a/default-configs/i386-softmmu.mak
+++ b/default-configs/i386-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_NVDIMM=y
 CONFIG_ACPI_NVDIMM=y
+CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
diff --git a/default-configs/x86_64-softmmu.mak 
b/default-configs/x86_64-softmmu.mak
index 7f89503..7d2c2d4 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -51,6 +51,7 @@ CONFIG_PVPANIC=y
 CONFIG_MEM_HOTPLUG=y
 CONFIG_NVDIMM=y
 CONFIG_ACPI_NVDIMM=y
+CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
diff --git a/hw/pci-bridge/Makefile.objs b/hw/pci-bridge/Makefile.objs
index f2adfe3..4f2039f 100644
--- a/hw/pci-bridge/Makefile.objs
+++ b/hw/pci-bridge/Makefile.objs
@@ -1,5 +1,6 @@
 common-obj-y += pci_bridge_dev.o
 common-obj-y += pci_expander_bridge.o
+common-obj-$(CONFIG_PCIE_PORT) += pcie_root_port.o
 common-obj-$(CONFIG_XIO3130) += xio3130_upstream.o xio3130_downstream.o
 common-obj-$(CONFIG_IOH3420) += ioh3420.o
 common-obj-$(CONFIG_I82801B11) += i82801b11.o
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
new file mode 100644
index 000..cf36318
--- /dev/null
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -0,0 +1,171 @@
+/*
+ * Base class for PCI Express Root Ports
+ *
+ * Copyright (C) 2017 Red Hat Inc
+ *
+ * Authors:
+ *   Marcel Apfelbaum 
+ *
+ * Most of the code was migrated from hw/pci-bridge/ioh3420.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/pci/pcie_port.h"
+
+static void rp_aer_vector_update(PCIDevice *d)
+{
+PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+
+if (rpc->aer_vector) {
+pcie_aer_root_set_vector(d, rpc->aer_vector(d));
+}
+}
+
+static void rp_write_config(PCIDevice *d, uint32_t address,
+uint32_t val, int len)
+{
+uint32_t root_cmd =
+pci_get_long(d->config + d->exp.aer_cap + PCI_ERR_ROOT_COMMAND);
+
+pci_bridge_write_config(d, address, val, len);
+rp_aer_vector_update(d);
+pcie_cap_slot_write_config(d, address, val, len);
+pcie_aer_write_config(d, address, val, len);
+pcie_aer_root_write_config(d, address, val, len, root_cmd);
+}
+
+static void rp_reset(DeviceState *qdev)
+{
+PCIDevice *d = PCI_DEVICE(qdev);
+
+rp_aer_vector_update(d);
+pcie_cap_root_reset(d);
+pcie_cap_deverr_reset(d);
+pcie_cap_slot_reset(d);
+pcie_cap_arifwd_reset(d);
+pcie_aer_root_reset(d);
+pci_bridge_reset(qdev);
+pci_bridge_disable_base_limit(d);
+}
+
+static void rp_realize(PCIDevice *d, Error **errp)
+{
+PCIEPort *p = PCIE_PORT(d);
+PCIESlot *s = PCIE_SLOT(d);
+PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
+PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
+int rc;
+Error *local_err = NULL;
+
+pci_config_set_interrupt_pin(d->config, 1);
+pci_bridge_initfn(d, TYPE_PCIE_BUS);
+pcie_port_init_reg(d);
+
+rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
+if (rc < 0) {
+error_setg(errp, "Can't init SSV ID, error %d", rc);
+goto err_bridge;
+}
+
+if (rpc->interrupts_init) {
+rc = rpc->interrupts_init(d, &local_err);
+if (rc < 0) {
+error_propagate(errp, local_err);
+goto err_bridge;
+}
+}
+
+rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
+if (rc < 0) {
+error_setg(errp, "Can't add Root Port capability, error %d", rc);
+goto err_int;
+}
+
+pcie_cap_arifwd_init(d);
+pcie_cap_deverr_init(d);
+pcie_cap_slot_init(d, s->slot);
+pcie_cap_root_init(d);
+
+pcie_chassis_create(s->chassis);
+rc = pcie_chassis

[Qemu-devel] [PATCH V4 0/3] hw/pcie: Introduce Generic PCI Express Root Port

[Marcel Apfelbaum]

v3 -> v4:
  - Rebased on master.

v2 -> v3:
 - Keep only the root port base class code in pcie_root_port.c (Michael)
 - Use msix for the generic root port implementation (Michael and Gerd)
   - The task required some refactoring like having some common
 init/uninit interrupts functions to be implemented by both
 generic and Intel Root Ports.

v1 -> v2:
 - Rebased on master.

The Generic Root Port behaves the same as the
Intel's IOH device with id 3420, without having
Intel specific attributes.

The device has two purposes:
 (1) Can be used on both X86 and ARM machines.
 (2) It will allow us to tweak the behaviour
(e.g add vendor-specific PCI capabilities)
 - something that obviously cannot be done
   on a known device.

Patch 1/3: Introduce a base class for Root Ports - most of the code
   is migrated from IOH3420 implementation.
Patch 2/3: Derives the IOH3420 from the new base class
Patch 3/3: Introduces the generic Root Port.

Tested with Linux and Windows guests only on x86 hosts.

Marcel Apfelbaum (3):
  hw/pcie: Introduce a base class for PCI Express Root Ports
  hw/ioh3420: derive from PCI Express Root Port base class
  hw/pcie: Introduce Generic PCI Express Root Port

 default-configs/arm-softmmu.mak|   1 +
 default-configs/i386-softmmu.mak   |   1 +
 default-configs/x86_64-softmmu.mak |   1 +
 hw/pci-bridge/Makefile.objs|   1 +
 hw/pci-bridge/gen_pcie_root_port.c |  88 +++
 hw/pci-bridge/ioh3420.c| 121 --
 hw/pci-bridge/pcie_root_port.c | 171 +
 include/hw/pci/pci.h   |   1 +
 include/hw/pci/pcie_port.h |  19 +
 9 files changed, 298 insertions(+), 106 deletions(-)
 create mode 100644 hw/pci-bridge/gen_pcie_root_port.c
 create mode 100644 hw/pci-bridge/pcie_root_port.c

-- 
2.5.5

Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Jeff Cody]

On Wed, Jan 25, 2017 at 07:27:07PM +0100, Max Reitz wrote:
> On 25.01.2017 18:22, Jeff Cody wrote:
> > In bdrv_find_backing_image(), if we are searching an image for a backing
> > file that contains a protocol, we currently only compare unmodified
> > paths.
> > 
> > However, some management software will change the backing filename to be
> > a relative filename in a path.  QEMU is able to handle this fine,
> > because internally it will use path_combine to put together the full
> > protocol URI.
> > 
> > However, this can lead to an inability to match an image during a QAPI
> > command that needs to use bdrv_find_backing_image() to find the image,
> > when it is searched by the full URI.
> > 
> > When searching for a protocol filename, if the straight comparison
> > fails, this patch will also compare against the full backing filename to
> > see if that is a match.
> > 
> > Signed-off-by: Jeff Cody 
> > ---
> >  block.c | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/block.c b/block.c
> > index 39ddea3..a173afc 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -3145,6 +3145,7 @@ BlockDriverState 
> > *bdrv_find_backing_image(BlockDriverState *bs,
> >  int is_protocol = 0;
> >  BlockDriverState *curr_bs = NULL;
> >  BlockDriverState *retval = NULL;
> > +Error *local_error = NULL;
> >  
> >  if (!bs || !bs->drv || !backing_file) {
> >  return NULL;
> > @@ -3165,6 +3166,17 @@ BlockDriverState 
> > *bdrv_find_backing_image(BlockDriverState *bs,
> >  retval = curr_bs->backing->bs;
> >  break;
> >  }
> > +/* Also check against the full backing filename for the image 
> > */
> > +bdrv_get_full_backing_filename(curr_bs, backing_file_full, 
> > PATH_MAX,
> > +   &local_error);
> > +if (local_error == NULL) {
> > +if (strcmp(backing_file, backing_file_full) == 0) {
> > +retval = curr_bs->backing->bs;
> > +break;
> > +}
> > +} else {
> > +error_free(local_error);
> 
> Oops, I was a bit too quick there. You should either follow Eric's
> remark about the scope, or you have to reset local_error to NULL here.
>

OK. I'll submit a v2, and add an iotest to that as well.

> 
> > +}
> >  } else {
> >  /* If not an absolute filename path, make it relative to the 
> > current
> >   * image's filename path */
> > 
> 
> 

Re: [Qemu-devel] [PULL 00/16] virtio, vhost, pci: fixes, features

[Marcel Apfelbaum]

On 01/25/2017 08:08 PM, Peter Maydell wrote:

On 25 January 2017 at 18:02, Marcel Apfelbaum
 wrote:

Strange, I re-based in the same day I posted and compiled just fine...
However, I get the same error using today's master.

I'll fix and re-post the series.


mst has done a rebase of the pull request which I'll retry later.



Understood, anyway the problem is that commit
20daa90 (PCI/migration merge vmstate_pci_device and vmstate_pcie_device)
removed the VMSTATE_PCIE_DEVICE macro.

I'll re-post anyway and Michael can decide if he tweaks the macro name
or use the updated series.

Thanks,
Marcel



thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Jeff Cody]

On Wed, Jan 25, 2017 at 07:24:48PM +0100, Max Reitz wrote:
> On 25.01.2017 18:22, Jeff Cody wrote:
> > In bdrv_find_backing_image(), if we are searching an image for a backing
> > file that contains a protocol, we currently only compare unmodified
> > paths.
> > 
> > However, some management software will change the backing filename to be
> > a relative filename in a path.  QEMU is able to handle this fine,
> > because internally it will use path_combine to put together the full
> > protocol URI.
> > 
> > However, this can lead to an inability to match an image during a QAPI
> > command that needs to use bdrv_find_backing_image() to find the image,
> > when it is searched by the full URI.
> > 
> > When searching for a protocol filename, if the straight comparison
> > fails, this patch will also compare against the full backing filename to
> > see if that is a match.
> > 
> > Signed-off-by: Jeff Cody 
> > ---
> >  block.c | 12 
> >  1 file changed, 12 insertions(+)
> 
> Thanks, applied to my block tree:
> 
> https://github.com/XanClic/qemu/commits/block
>

Thanks!

> 
> How much would you mind writing an iotest?

I don't mind, I can do that.

-Jeff

Re: [Qemu-devel] [PATCH v4 1/9] ACPI: Add a function for building named qword entries

[Michael S. Tsirkin]

On Wed, Jan 25, 2017 at 09:36:52AM -0800, Ben Warren wrote:
> Hi Laszlo,
> 
> 
> On Jan 24, 2017, at 7:55 PM, Laszlo Ersek  wrote:
> 
> Hi Ben,
> 
> sorry about being late to reviewing this series. I hope I can now spend
> more time on it.
> 
> - Please do not try to address my comments immediately. It's very
> possible (even likely) that Igor, MST and myself could have different
> opinions on things, so first please await agreement between your 
> reviewers.
> 
> 
> Thanks for the very detailed review.  I’ll give it a couple of days and then
> start work on the suggested changes.
> 
> 
> - I think you should have CC'd Igor and Michael directly. I'm adding
> them to this reply; hopefully that will be enough for them to monitor
> this series.
> 
> - I'll likely be unable to review everything with 100% coverage; so
> addressing (or sufficiently refuting) my comments might not guarantee
> that the next version will be merged at once.
> 
> With all that said:
> 
> On 01/25/17 02:43, b...@skyportsystems.com wrote:
> 
> From: Ben Warren 
> 
> This is initially used to patch a 64-bit address into the VM 
> Generation
> ID SSDT
> 
> 
> (1) I think this commit message line is overlong; I think we wrap at 74
> chars or so. Not critical, but worth mentioning.
> 
> 
> 
> Signed-off-by: Ben Warren 
> ---
> hw/acpi/aml-build.c | 28 
> include/hw/acpi/aml-build.h |  4 
> 2 files changed, 32 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index b2a1e40..dc4edc2 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -285,6 +285,34 @@ build_append_named_dword(GArray *array, const 
> char
> *name_format, ...)
> return offset;
> }
> 
> +/*
> + * Build NAME(, 0x) where 0x is encoded as a
> qword,
> + * and return the offset to 0x for runtime patching.
> + *
> + * Warning: runtime patching is best avoided. Only use this as
> + * a replacement for DataTableRegion (for guests that don't
> + * support it).
> + */

only one comment: QWords first appeared in ACPI 2.0 and
XP does not like them. Not strictly a blocker as people can
avoid using the feature, but nice to have.
Will either UEFI or seabios allocate
memory outside 4G range? If not you do not need a qword.




> 
> (2) Since we're adding a qword (8-byte integer), the hexadecimal
> constant should have 16 nibbles: 0x. (After copying the
> comment from build_append_named_dword(), it should be actualized.)
> 
> (3) Normally the functions in this file that create AML operators carry
> a note about the ACPI spec version and exact location that defines the
> operator. I see that commit f20354910893 ("acpi: add
> build_append_named_dword, returning an offset in buffer", 2016-03-01)
> missed that too.
> 
> Unless this tradition has been willfully abandoned, I suggest that you
> add the right reference here, and also (in retrospect) to
> build_append_named_dword().
> 
> 
> +int
> +build_append_named_qword(GArray *array, const char *name_format, ...)
> +{
> +int offset;
> +va_list ap;
> +
> +build_append_byte(array, 0x08); /* NameOp */
> +va_start(ap, name_format);
> +build_append_namestringv(array, name_format, ap);
> +va_end(ap);
> +
> +build_append_byte(array, 0x0E); /* QWordPrefix */
> +
> +offset = array->len;
> +build_append_int_noprefix(array, 0x, 8);
> 
> 
> (4) I guess the constant should be updated here too, for consistency
> with the comment.
> 
> The rest looks okay. (I didn't verify 0x0E == QWordPrefix specifically,
> because an error there would break the functionality immediately, and
> you'd notice.)
> 
> Thanks!
> Laszlo
> 
> 
> +assert(array->len == offset + 8);
> +
> +return offset;
> +}
> +
> static GPtrArray *alloc_list;
> 
> static Aml *aml_alloc(void)
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 559326c..dbf63cf 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -385,6 +385,10 @@ int
> build_append_named_dword(GArray *array, const char *name_format, ...)
> GCC_FMT_ATTR(2, 3);
> 
> +int
> +build_append_named_qword(GArray *array, const char *name_format, ...)
> +GCC_FMT_ATTR(2, 3);
> +
> void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>uint64_t len, int node, Memor

Re: [Qemu-devel] [01/15] postcopy: Transmit and compare individual page sizes

[Dr. David Alan Gilbert]

* Alexey Perevalov (a.pereva...@samsung.com) wrote:
> Hi David,

Hi Alexey,

> I checked you whole patch set with Andrea's kernel
> git://git.kernel.org/pub/scm/linux/kernel/git/andrea/aa.git
> 
> It works and really gives sufficient decreasing of the downtime.
> 
> I'm newby in qemu and here in the mailing list.

Welcome!

> I have some remarks on current patch.
> 
> On both client and server side post copy capability should be enabled
> 
> migrate_set_capability postcopy-ram on
> 
> and serialization/deserialization relies on it.
> 
>  So if destination host
> doesn't set post copy capability ram_load will skip reading of
> remote_page_size and in case of multiple RAMBlocks the next read of len
> will be incorrect. Hopefully usually len is 0, but it could be bigger and
> overrun buffer ;).
> Maybe it's better to pass post copy capability attribute from host to
> destination to avoid
> such assumption.

We already pass an 'advise' command from the source to the destination
to tell it we're using postcopy - that's effectively us passing the
capability.  You can see in the 2nd patch I modify it's contents;
that happens before the RAM code we see here, so we
so we should always be safe in the case of the source having enabled
postcopy but the destination not having done.

'len' is always read as a byte, and the buffer is 256 bytes long - so
even if we read garbage off the stream we can never overrun the buffer.

However, you have made me think of a related case;  if the destination
has postcopy capability set, but the source does NOT have it set
then we'll incorrectly read this data.
I can fix that by changing the migrate_postcopy_ram() on
the receive side to:
  postcopy_state_get() >= POSTCOPY_INCOMING_ADVISE

so we'll only do it if the source has it enabled.

Thanks,

Dave

> 
> 
> On 01/06/2017 09:28 PM, Dave Gilbert wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > When using postcopy with hugepages, we require the source
> > and destination page sizes for any RAMBlock to match.
> > 
> > Transmit them as part of the RAM information header and
> > fail if there's a difference.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >   migration/ram.c | 15 +++
> >   1 file changed, 15 insertions(+)
> > 
> > diff --git a/migration/ram.c b/migration/ram.c
> > index a1c8089..39998f5 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -1970,6 +1970,9 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> >   qemu_put_byte(f, strlen(block->idstr));
> >   qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
> >   qemu_put_be64(f, block->used_length);
> > +if (migrate_postcopy_ram() && block->page_size != 
> > qemu_host_page_size) {
> > +qemu_put_be64(f, block->page_size);
> > +}
> >   }
> >   rcu_read_unlock();
> > @@ -2536,6 +2539,18 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> > version_id)
> >   error_report_err(local_err);
> >   }
> >   }
> > +/* For postcopy we need to check hugepage sizes match 
> > */
> > +if (migrate_postcopy_ram() &&
> > +block->page_size != qemu_host_page_size) {
> > +uint64_t remote_page_size = qemu_get_be64(f);
> > +if (remote_page_size != block->page_size) {
> > +error_report("Mismatched RAM page size %s "
> > + "(local) %" PRId64 "!= %" PRId64,
> > + id, block->page_size,
> > + remote_page_size);
> > +ret = -EINVAL;
> > +}
> > +}
> >   ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG,
> > block->idstr);
> >   } else {
> 
> 
> -- 
> Best regards,
> Alexey Perevalov
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PULL 0/5] 9p patches 20170125

[Peter Maydell]

On 25 January 2017 at 13:47, Greg Kurz  wrote:
> The following changes since commit a9e404600a9bd1e6a26431fc89e5069092e67f14:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170124' into 
> staging (2017-01-24 17:26:26 +)
>
> are available in the git repository at:
>
>   https://github.com/gkurz/qemu.git tags/for-upstream
>
> for you to fetch changes up to fa0eb5c512d17a223d9f9bac45da48d78d12f584:
>
>   9pfs: fix offset error in v9fs_xattr_read() (2017-01-25 09:34:35 +0100)
>
> 
> This pull request fixes a 2.9 regression and a long standing bug that can
> cause 9p clients to hang. Other patches are minor enhancements.
>
> 
> Greg Kurz (5):
>   9pfs: add missing coroutine_fn annotations
>   tests: virtio-9p: improve error reporting
>   9pfs: fix off-by-one error in PDU free list
>   9pfs: local: trivial cosmetic fix in pwritev op
>   9pfs: fix offset error in v9fs_xattr_read()

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Alex Williamson]

On Wed, 25 Jan 2017 18:40:56 +0100
Paolo Bonzini  wrote:

> On 25/01/2017 18:36, Alex Williamson wrote:
> >> You probably should also put a comment about why VFIO does *not* need to
> >> keep a reference between vfio_dma_map and vfio_dma_unmap (which doesn't
> >> sound easy to do either).  Would any well-behaved guest invalidate the
> >> IOMMU page tables before a memory hot-unplug?  
> >
> > Hmm, we do take a reference in vfio_listener_region_add(), but this is
> > of course to the iommu region not to the RAM region we're translating.
> > In the non-vIOMMU case we would be holding a reference to the memory
> > region backing a DMA mapping.  I would expect a well behaved guest to
> > evacuate DMA mappings targeting a hotplug memory region before it gets
> > ejected, but how much do we want to rely on well behaved guests.  
> 
> It depends of what happens if they aren't.  I think it's fine (see other
> message), but taking a reference for each mapping entry isn't so easy
> because the unmap case doesn't know the old memory region.

If we held a reference to the memory region from the mapping path and
walk the IOMMU page table to generate the unmap, then we really should
get to the same original memory region, right?  The vfio iommu notifier
should only be mapping native page sizes of the IOMMU, 4k/2M/1G.  The
problem is that it's a lot of overhead to flush the entire address
space that way vs the single invalidation Peter is trying to enable
here.  It's actually similar to how the type1 iommu works in the kernel
though, we can unmap by iova because we ask the iommu for the iova->pfn
translation in order to unpin the page.

I do agree with your description in the other message about how things
would work for a memory hot-unplug w/o unmap though, which does seem to
imply that we don't need that reference.  Thanks,

Alex

Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Eric Blake]

On 01/25/2017 11:22 AM, Jeff Cody wrote:
> In bdrv_find_backing_image(), if we are searching an image for a backing
> file that contains a protocol, we currently only compare unmodified
> paths.
> 
> However, some management software will change the backing filename to be
> a relative filename in a path.  QEMU is able to handle this fine,
> because internally it will use path_combine to put together the full
> protocol URI.
> 
> However, this can lead to an inability to match an image during a QAPI
> command that needs to use bdrv_find_backing_image() to find the image,
> when it is searched by the full URI.
> 
> When searching for a protocol filename, if the straight comparison
> fails, this patch will also compare against the full backing filename to
> see if that is a match.
> 
> Signed-off-by: Jeff Cody 
> ---
>  block.c | 12 
>  1 file changed, 12 insertions(+)

Does this overlap with Max' work on 'block: Fix some filename generation
issues'?  But in isolation, it looks okay to me,

Reviewed-by: Eric Blake 

> 
> diff --git a/block.c b/block.c
> index 39ddea3..a173afc 100644
> --- a/block.c
> +++ b/block.c
> @@ -3145,6 +3145,7 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>  int is_protocol = 0;
>  BlockDriverState *curr_bs = NULL;
>  BlockDriverState *retval = NULL;
> +Error *local_error = NULL;

I might have scoped this...

>  
>  if (!bs || !bs->drv || !backing_file) {
>  return NULL;
> @@ -3165,6 +3166,17 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>  retval = curr_bs->backing->bs;
>  break;
>  }

...as the first line within the if that uses it. But no big deal.

> +/* Also check against the full backing filename for the image */
> +bdrv_get_full_backing_filename(curr_bs, backing_file_full, 
> PATH_MAX,
> +   &local_error);
> +if (local_error == NULL) {
> +if (strcmp(backing_file, backing_file_full) == 0) {
> +retval = curr_bs->backing->bs;
> +break;
> +}
> +} else {
> +error_free(local_error);
> +}
>  } else {
>  /* If not an absolute filename path, make it relative to the 
> current
>   * image's filename path */
> 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/3] xen-platform: add support for unplugging NVMe disks...

[Stefano Stabellini]

On Wed, 25 Jan 2017, Paul Durrant wrote:
> On 25 January 2017, at 17:54, Stefano Stabellini  
> wrote:
> 
> >
> >
> >On Wed, 25 Jan 2017, Paul Durrant wrote:
> >> > -Original Message-
> >> > From: Stefano Stabellini [mailto:sstabell...@kernel.org]
> >> > Sent: 24 January 2017 23:49
> >> > To: Paul Durrant 
> >> > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Stefano
> >> > Stabellini ; Anthony Perard
> >> > ; Michael S. Tsirkin ; Paolo
> >> > Bonzini ; Richard Henderson ;
> >> > Eduardo Habkost ; o...@aepfle.de
> >> > Subject: Re: [PATCH v2 2/3] xen-platform: add support for unplugging NVMe
> >> > disks...
> >> >
> >> > On Tue, 24 Jan 2017, Paul Durrant wrote:
> >> > > ...not just IDE and SCSI.
> >> > >
> >> > > This patch allows the Xen tool-stack to fully support of NVMe as an
> >> > > emulated disk type.
> >> > >
> >> > > Signed-off-by: Paul Durrant 
> >> >
> >> > Please update docs/misc/hvm-emulated-unplug.markdown in the Xen
> >> > repository first. It might be also worth clarifying that `1` actually
> >> > means all disks, not just IDE disks. Then, please add a reference to
> >> > that commit in the description of this patch.
> >> >
> >>
> >> Patch posted to remove 'IDE' from the documentation for value '1'. 
> >> Awaiting ack.
> >Done.
> >When you repost this patch, could you also add to the description a
> >reference to the commit that enables NVMe in QEMU with Xen? I guess it
> >is a libxl commit?
> 
> I think there is a chicken and egg issue here. Wei wanted to ensure that QEMU 
> is able to unplug
> NVMe drives before accepting my patch to libxl.

No problems. In that case, please add a link to the xen-devel email
thread.


> >> > > ---
> >> > > Cc: Stefano Stabellini 
> >> > > Cc: Anthony Perard 
> >> > > Cc: "Michael S. Tsirkin" 
> >> > > Cc: Paolo Bonzini 
> >> > > Cc: Richard Henderson 
> >> > > Cc: Eduardo Habkost 
> >> > > ---
> >> > >  hw/i386/xen/xen_platform.c | 1 +
> >> > >  1 file changed, 1 insertion(+)
> >> > >
> >> > > diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> >> > > index f50915f..7d41ebb 100644
> >> > > --- a/hw/i386/xen/xen_platform.c
> >> > > +++ b/hw/i386/xen/xen_platform.c
> >> > > @@ -120,6 +120,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d,
> >> > void *o)
> >> > >  break;
> >> > >
> >> > >  case PCI_CLASS_STORAGE_SCSI:
> >> > > +    case PCI_CLASS_STORAGE_EXPRESS:
> >> > >  object_unparent(OBJECT(d));
> >> > >  break;
> >> > >
> >> > > --
> >> > > 2.1.4
> >> > >
> >>
> >
> 
> 
> 

Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Max Reitz]

On 25.01.2017 18:22, Jeff Cody wrote:
> In bdrv_find_backing_image(), if we are searching an image for a backing
> file that contains a protocol, we currently only compare unmodified
> paths.
> 
> However, some management software will change the backing filename to be
> a relative filename in a path.  QEMU is able to handle this fine,
> because internally it will use path_combine to put together the full
> protocol URI.
> 
> However, this can lead to an inability to match an image during a QAPI
> command that needs to use bdrv_find_backing_image() to find the image,
> when it is searched by the full URI.
> 
> When searching for a protocol filename, if the straight comparison
> fails, this patch will also compare against the full backing filename to
> see if that is a match.
> 
> Signed-off-by: Jeff Cody 
> ---
>  block.c | 12 
>  1 file changed, 12 insertions(+)

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block


How much would you mind writing an iotest?

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Max Reitz]

On 25.01.2017 18:22, Jeff Cody wrote:
> In bdrv_find_backing_image(), if we are searching an image for a backing
> file that contains a protocol, we currently only compare unmodified
> paths.
> 
> However, some management software will change the backing filename to be
> a relative filename in a path.  QEMU is able to handle this fine,
> because internally it will use path_combine to put together the full
> protocol URI.
> 
> However, this can lead to an inability to match an image during a QAPI
> command that needs to use bdrv_find_backing_image() to find the image,
> when it is searched by the full URI.
> 
> When searching for a protocol filename, if the straight comparison
> fails, this patch will also compare against the full backing filename to
> see if that is a match.
> 
> Signed-off-by: Jeff Cody 
> ---
>  block.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 39ddea3..a173afc 100644
> --- a/block.c
> +++ b/block.c
> @@ -3145,6 +3145,7 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>  int is_protocol = 0;
>  BlockDriverState *curr_bs = NULL;
>  BlockDriverState *retval = NULL;
> +Error *local_error = NULL;
>  
>  if (!bs || !bs->drv || !backing_file) {
>  return NULL;
> @@ -3165,6 +3166,17 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>  retval = curr_bs->backing->bs;
>  break;
>  }
> +/* Also check against the full backing filename for the image */
> +bdrv_get_full_backing_filename(curr_bs, backing_file_full, 
> PATH_MAX,
> +   &local_error);
> +if (local_error == NULL) {
> +if (strcmp(backing_file, backing_file_full) == 0) {
> +retval = curr_bs->backing->bs;
> +break;
> +}
> +} else {
> +error_free(local_error);

Oops, I was a bit too quick there. You should either follow Eric's
remark about the scope, or you have to reset local_error to NULL here.

Max

> +}
>  } else {
>  /* If not an absolute filename path, make it relative to the 
> current
>   * image's filename path */
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Max Reitz]

On 25.01.2017 19:24, Eric Blake wrote:
> On 01/25/2017 11:22 AM, Jeff Cody wrote:
>> In bdrv_find_backing_image(), if we are searching an image for a backing
>> file that contains a protocol, we currently only compare unmodified
>> paths.
>>
>> However, some management software will change the backing filename to be
>> a relative filename in a path.  QEMU is able to handle this fine,
>> because internally it will use path_combine to put together the full
>> protocol URI.
>>
>> However, this can lead to an inability to match an image during a QAPI
>> command that needs to use bdrv_find_backing_image() to find the image,
>> when it is searched by the full URI.
>>
>> When searching for a protocol filename, if the straight comparison
>> fails, this patch will also compare against the full backing filename to
>> see if that is a match.
>>
>> Signed-off-by: Jeff Cody 
>> ---
>>  block.c | 12 
>>  1 file changed, 12 insertions(+)
> 
> Does this overlap with Max' work on 'block: Fix some filename generation
> issues'?  But in isolation, it looks okay to me,

Yes, it does, but it's not too hard to fix up in my series. Better get
this in first and then do the trivial change on my side.

> Reviewed-by: Eric Blake 

Noted :-)

Max

>>
>> diff --git a/block.c b/block.c
>> index 39ddea3..a173afc 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3145,6 +3145,7 @@ BlockDriverState 
>> *bdrv_find_backing_image(BlockDriverState *bs,
>>  int is_protocol = 0;
>>  BlockDriverState *curr_bs = NULL;
>>  BlockDriverState *retval = NULL;
>> +Error *local_error = NULL;
> 
> I might have scoped this...
> 
>>  
>>  if (!bs || !bs->drv || !backing_file) {
>>  return NULL;
>> @@ -3165,6 +3166,17 @@ BlockDriverState 
>> *bdrv_find_backing_image(BlockDriverState *bs,
>>  retval = curr_bs->backing->bs;
>>  break;
>>  }
> 
> ...as the first line within the if that uses it. But no big deal.
> 
>> +/* Also check against the full backing filename for the image */
>> +bdrv_get_full_backing_filename(curr_bs, backing_file_full, 
>> PATH_MAX,
>> +   &local_error);
>> +if (local_error == NULL) {
>> +if (strcmp(backing_file, backing_file_full) == 0) {
>> +retval = curr_bs->backing->bs;
>> +break;
>> +}
>> +} else {
>> +error_free(local_error);
>> +}
>>  } else {
>>  /* If not an absolute filename path, make it relative to the 
>> current
>>   * image's filename path */
>>
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/3] xen-platform: add support for unplugging NVMe disks...

[Paul Durrant]

On 25 January 2017, at 17:54, Stefano Stabellini  wrote:

>
>
>On Wed, 25 Jan 2017, Paul Durrant wrote:
>> > -Original Message-
>> > From: Stefano Stabellini [mailto:sstabell...@kernel.org]
>> > Sent: 24 January 2017 23:49
>> > To: Paul Durrant 
>> > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Stefano
>> > Stabellini ; Anthony Perard
>> > ; Michael S. Tsirkin ; Paolo
>> > Bonzini ; Richard Henderson ;
>> > Eduardo Habkost ; o...@aepfle.de
>> > Subject: Re: [PATCH v2 2/3] xen-platform: add support for unplugging NVMe
>> > disks...
>> >
>> > On Tue, 24 Jan 2017, Paul Durrant wrote:
>> > > ...not just IDE and SCSI.
>> > >
>> > > This patch allows the Xen tool-stack to fully support of NVMe as an
>> > > emulated disk type.
>> > >
>> > > Signed-off-by: Paul Durrant 
>> >
>> > Please update docs/misc/hvm-emulated-unplug.markdown in the Xen
>> > repository first. It might be also worth clarifying that `1` actually
>> > means all disks, not just IDE disks. Then, please add a reference to
>> > that commit in the description of this patch.
>> >
>>
>> Patch posted to remove 'IDE' from the documentation for value '1'. Awaiting 
>> ack.
>Done.
>When you repost this patch, could you also add to the description a
>reference to the commit that enables NVMe in QEMU with Xen? I guess it
>is a libxl commit?

I think there is a chicken and egg issue here. Wei wanted to ensure that QEMU 
is able to unplug NVMe drives before accepting my patch to libxl.

  Paul

>> > > ---
>> > > Cc: Stefano Stabellini 
>> > > Cc: Anthony Perard 
>> > > Cc: "Michael S. Tsirkin" 
>> > > Cc: Paolo Bonzini 
>> > > Cc: Richard Henderson 
>> > > Cc: Eduardo Habkost 
>> > > ---
>> > >  hw/i386/xen/xen_platform.c | 1 +
>> > >  1 file changed, 1 insertion(+)
>> > >
>> > > diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
>> > > index f50915f..7d41ebb 100644
>> > > --- a/hw/i386/xen/xen_platform.c
>> > > +++ b/hw/i386/xen/xen_platform.c
>> > > @@ -120,6 +120,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d,
>> > void *o)
>> > >  break;
>> > >
>> > >  case PCI_CLASS_STORAGE_SCSI:
>> > > +case PCI_CLASS_STORAGE_EXPRESS:
>> > >  object_unparent(OBJECT(d));
>> > >  break;
>> > >
>> > > --
>> > > 2.1.4
>> > >
>>
>

Re: [Qemu-devel] [PATCH 0/2] migration capability to discard the migrated ram pages

[Denis V. Lunev]

On 01/16/2017 06:51 PM, Pavel Butsykin wrote:
> This feature frees the migrated memory on the source during postcopy-ram
> migration. In the second step of postcopy-ram migration when the source vm
> is put on pause we can free unnecessary memory. It will allow, in particular,
> to start relaxing the memory stress on the source host in a load-balancing
> scenario.
>
> Pavel Butsykin (2):
>   add 'discard-ram' migrate capability
>   migration: discard non-dirty ram pages after the start of postcopy
>
>  include/migration/migration.h |  2 ++
>  include/migration/qemu-file.h |  3 ++-
>  migration/migration.c | 11 
>  migration/qemu-file.c | 59 
> ++-
>  migration/ram.c   | 49 ++-
>  qapi-schema.json  |  5 +++-
>  6 files changed, 119 insertions(+), 10 deletions(-)
>
ping

Re: [Qemu-devel] [Bug 1622547] Re: qemu-system-sparc fatal error Trap 0x29 on Solaris 2.6

[Artyom Tarasenko]

Richard, can you please look at it?

The test case:

qemu-system-sparc -bios ./ss20_v2.25_rom -M SS-20 -nographic -m 512
-cpu "TI SuperSparc 60"

Kind regards,
Artyom

On Wed, Jan 25, 2017 at 5:16 PM, m...@papersolve.com
 wrote:
> That was fun! And we have a result:
>
> fbb4bbb62e5603c991b880e25dc4bb30d342b944 is the first bad commit
> commit fbb4bbb62e5603c991b880e25dc4bb30d342b944
> Author: Richard Henderson 
> Date:   Tue Jul 12 15:38:13 2016 -0700
>
> target-sparc: Implement ldstub_asi inline
>
> Tested-by: Mark Cave-Ayland 
> Signed-off-by: Richard Henderson 
>
> :04 04 670db498d49d38bc878efccd55e39d03f074cadf
> 5052ce1f32ddf00646aaa9e37bb73e38b4e750f1 M  target-sparc
>
>
> I verified that the last good commit not only boots the BIOS but also boots 
> the OS properly (and faster than 2.7.1).
>
> --
> You received this bug notification because you are a member of qemu-
> devel-ml, which is subscribed to QEMU.
> https://bugs.launchpad.net/bugs/1622547
>
> Title:
>   qemu-system-sparc fatal error Trap 0x29 on Solaris 2.6
>
> Status in QEMU:
>   New
>
> Bug description:
>   When trying to install Solaris 2.6 from original CDROM, qemu fail with
>   the following error :
>
>   qemu: fatal: Trap 0x29 while interrupts disabled, Error state
>   pc: f0041280  npc: f0041284
>   %g0-7:  f0281800 0800   f0243b88 0001 
> f0244020
>   %o0-7: 40400ce2 40400ce2  404000e2 f0243b88  f023ffd8 
> f0057914
>   %l0-7: 4cc2 f009645c f0096460 0002 0209 0004 0007 
> f023ff90
>   %i0-7: 0042 404000e3  404000e3 e000 f028192a f0240038 
> f0096448
>   %f00:     
>   %f08:     
>   %f16:     
>   %f24:     
>   psr: 40400cc2 (icc: -Z-- SPE: SP-) wim: 0002
>   fsr:  y: 
>
>   The command line was :
>
>   qemu-system-sparc -nographic -bios ./openbios-sparc32 -M SS-20 -hda
>   ./36G.disk -m 512 -cdrom Solaris_2.6_Software_05_98.img -boot d
>   -serial telnet:0.0.0.0:3000,server -smp 2,cores=2 -monitor null
>
>   It fails with a similar output when using bios ss20_v2.25_rom.
>
>   ▶ qemu-system-sparc --version
>   QEMU emulator version 2.7.0, Copyright (c) 2003-2016 Fabrice Bellard and 
> the QEMU Project developers
>
>   ▶ uname -a
>   Linux xxx 4.7.1-1-ARCH #1 SMP PREEMPT Wed Aug 17 08:13:35 CEST 2016 x86_64 
> GNU/Linux
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1622547/+subscriptions
>



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [Qemu-devel] [PULL 00/16] virtio, vhost, pci: fixes, features

[Marcel Apfelbaum]

On 01/25/2017 06:36 PM, Peter Maydell wrote:

On 24 January 2017 at 22:36, Michael S. Tsirkin  wrote:

The following changes since commit a9e404600a9bd1e6a26431fc89e5069092e67f14:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170124' into 
staging (2017-01-24 17:26:26 +)

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 b562d945fc840131d290d4bf5af8d6cc3bb98a2b:

  vhost-user: delete chardev on cleanup (2017-01-24 21:49:25 +0200)


virtio, vhost, pci: fixes, features

generic pci root port support
fixes and cleanups all over the place

Signed-off-by: Michael S. Tsirkin 


Hi; I'm afraid this fails to build:

/home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:9: error:
implicit declaration of function ‘VMSTATE_PCIE_DEVICE’
[-Werror=implicit-function-declaration]
 VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
 ^
/home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:29: error:
‘parent_obj’ undeclared here (not in a function)
 VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),
 ^
/home/pm215/qemu/hw/pci-bridge/gen_pcie_root_port.c:54:63: error:
expected expression before ‘PCIESlot’
 VMSTATE_PCIE_DEVICE(parent_obj.parent_obj.parent_obj, PCIESlot),



Hi Peter,

Strange, I re-based in the same day I posted and compiled just fine...
However, I get the same error using today's master.

I'll fix and re-post the series.

Thanks,
Marcel
^


thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 1/2] move vm_start to cpus.c

[Paolo Bonzini]

On 25/01/2017 18:53, Claudio Imbrenda wrote:
> On 25/01/17 11:21, Paolo Bonzini wrote:
>>
>>
>> On 28/10/2016 19:15, Claudio Imbrenda wrote:
>>> * moves the call to qemu_clock_enable away from resume_all_vcpus, and
>>>   add an explicit call to it before each instance of resume_all_vcpus
>>>   in the code.
>>
>> This change adds useless duplication, and isn't matched by a similar
>> change to pause_all_vcpus.  You need to justify it; I suppose it is
>> because the next patch will not call resume_all_cpus?
> 
> Actually it is because the next patch used to call resume_all_vcpus, and
> we didn't want it to restart the clock too. But then I ended up not
> using resume_all_vcpus.
> 
> And now I have actually no idea why we didn't want to restart the clock.
> Maybe we should? After all some CPUs are running. I can patch
> gdb_continue_partial to check if any CPU was actually restarted, and if
> so start the clock.
> 
> So in the end it should still be correct, but the changes would be kept
> small.

Yeah, this sounds like a good solution.  Thanks!

Paolo

>> Most of the callers of pause_all_vcpus/resume_all_vcpus don't let timers
>> run, so the clock need not be disabled and enabled.  Maybe the right
>> places to call qemu_clock_enable are cpu_disable_ticks and
>> cpu_enable_ticks?  That should work for you.
>>
>> In that case, please make the first patch the qemu_clock_enable
>> movement; the second patch the introduction of vm_prepare_start; the
>> third patch the gdbstub change.
>>
>>> diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
>>> index 3728a1e..5fa074b 100644
>>> --- a/include/sysemu/cpus.h
>>> +++ b/include/sysemu/cpus.h
>>> @@ -5,6 +5,7 @@
>>>  bool qemu_in_vcpu_thread(void);
>>>  void qemu_init_cpu_loop(void);
>>>  void resume_all_vcpus(void);
>>> +void resume_some_vcpus(CPUState **cpus);
>>>  void pause_all_vcpus(void);
>>>  void cpu_stop_current(void);
>>>  void cpu_ticks_init(void);
>>
>> This function doesn't exist.
> 
> oops.
> 
> 
> Claudio
> 

Re: [Qemu-devel] [PULL 00/16] virtio, vhost, pci: fixes, features

[Peter Maydell]

On 25 January 2017 at 18:02, Marcel Apfelbaum
 wrote:
> Strange, I re-based in the same day I posted and compiled just fine...
> However, I get the same error using today's master.
>
> I'll fix and re-post the series.

mst has done a rebase of the pull request which I'll retry later.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support
Message-id: cover.1485365834.git.jc...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/cover.1485365834.git.jc...@redhat.com -> 
patchew/cover.1485365834.git.jc...@redhat.com
Switched to a new branch 'test'
b51085c QAPI: Fix blockdev-add example documentation
f3be4b6 iscsi: Add blockdev-add support
714b5f9 iscsi: Add timeout option
c5ee895 iscsi: Add header-digest option
8047699 iscsi: Add initiator-name option
a9e68ca iscsi: Handle -iscsi user/password in bdrv_parse_filename()
c38bdab iscsi: Split URL into individual options

=== OUTPUT BEGIN ===
Checking PATCH 1/7: iscsi: Split URL into individual options...
ERROR: switch and case should be at the same indent
#56: FILE: block/iscsi.c:1608:
+switch (iscsi_url->transport) {
+case TCP_TRANSPORT: transport_name = "tcp"; break;
+case ISER_TRANSPORT:transport_name = "iser"; break;
+default:

ERROR: trailing statements should be on next line
#57: FILE: block/iscsi.c:1609:
+case TCP_TRANSPORT: transport_name = "tcp"; break;

ERROR: trailing statements should be on next line
#58: FILE: block/iscsi.c:1610:
+case ISER_TRANSPORT:transport_name = "iser"; break;

total: 3 errors, 0 warnings, 289 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/7: iscsi: Handle -iscsi user/password in 
bdrv_parse_filename()...
Checking PATCH 3/7: iscsi: Add initiator-name option...
Checking PATCH 4/7: iscsi: Add header-digest option...
Checking PATCH 5/7: iscsi: Add timeout option...
Checking PATCH 6/7: iscsi: Add blockdev-add support...
Checking PATCH 7/7: QAPI: Fix blockdev-add example documentation...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH v2 2/3] xen-platform: add support for unplugging NVMe disks...

[Stefano Stabellini]

On Wed, 25 Jan 2017, Paul Durrant wrote:
> > -Original Message-
> > From: Stefano Stabellini [mailto:sstabell...@kernel.org]
> > Sent: 24 January 2017 23:49
> > To: Paul Durrant 
> > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Stefano
> > Stabellini ; Anthony Perard
> > ; Michael S. Tsirkin ; Paolo
> > Bonzini ; Richard Henderson ;
> > Eduardo Habkost ; o...@aepfle.de
> > Subject: Re: [PATCH v2 2/3] xen-platform: add support for unplugging NVMe
> > disks...
> > 
> > On Tue, 24 Jan 2017, Paul Durrant wrote:
> > > ...not just IDE and SCSI.
> > >
> > > This patch allows the Xen tool-stack to fully support of NVMe as an
> > > emulated disk type.
> > >
> > > Signed-off-by: Paul Durrant 
> > 
> > Please update docs/misc/hvm-emulated-unplug.markdown in the Xen
> > repository first. It might be also worth clarifying that `1` actually
> > means all disks, not just IDE disks. Then, please add a reference to
> > that commit in the description of this patch.
> >
> 
> Patch posted to remove 'IDE' from the documentation for value '1'. Awaiting 
> ack.

Done.

When you repost this patch, could you also add to the description a
reference to the commit that enables NVMe in QEMU with Xen? I guess it
is a libxl commit?


> > > ---
> > > Cc: Stefano Stabellini 
> > > Cc: Anthony Perard 
> > > Cc: "Michael S. Tsirkin" 
> > > Cc: Paolo Bonzini 
> > > Cc: Richard Henderson 
> > > Cc: Eduardo Habkost 
> > > ---
> > >  hw/i386/xen/xen_platform.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
> > > index f50915f..7d41ebb 100644
> > > --- a/hw/i386/xen/xen_platform.c
> > > +++ b/hw/i386/xen/xen_platform.c
> > > @@ -120,6 +120,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d,
> > void *o)
> > >  break;
> > >
> > >  case PCI_CLASS_STORAGE_SCSI:
> > > +case PCI_CLASS_STORAGE_EXPRESS:
> > >  object_unparent(OBJECT(d));
> > >  break;
> > >
> > > --
> > > 2.1.4
> > >
> 

Re: [Qemu-devel] [PATCH v3 2/2] gdbstub: Fix vCont behaviour

[Claudio Imbrenda]

I applied all your suggestions


Claudio

Re: [Qemu-devel] [PATCH 1/1] block: add missed BDRV_O_NOCACHE when block device is opened without file

[Max Reitz]

[CC-ing John]

On 25.01.2017 17:42, Denis V. Lunev wrote:
> Technically there is a problem when the guest DVD is created by libvirt
> with AIO mode 'native' on Linux. Current QEMU is unable to start the
> domain configured as follows:
> 
>   
>   
>   
> 
> The problem comes from the combination of 'cache' and 'io' options.
> 
> 'io' option is common option and it is removed from block driver
> specific options. 'cache' originally is not. The patch makes 'cache'
> option common. This works fine as long as cdrom media insertion
> later on.
> 
> Signed-off-by: Denis V. Lunev 
> CC: Kevin Wolf 
> CC: Max Reitz 
> ---
>  blockdev.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

There was a Red Hat BZ for this:

https://bugzilla.redhat.com/show_bug.cgi?id=1342999

There is now a corresponding BZ for libvirt:

https://bugzilla.redhat.com/show_bug.cgi?id=1377321

The gist is that it was determined to be a problem with libvirt.

RHEL has a downstream commit to work around this issue by ignoring the
cache mode set for empty CD-ROMs -- but that is only because that was
simpler than fixing libvirt.

(Note that it's ignoring the cache mode instead of actually evaluating it.)

> May be this has already discussed, but still. AIO=native for CDROM without
> media seems important case.

First, I personally don't find aio=native very important for CD-ROM
drives. Yes, we shouldn't make IDE CD-ROM slower than necessary, but I
don't think aio=native will make the difference between "Slow like
CD-ROMs are in reality" and "As fast as virtio".

Second, all this patch does is revert some changes done by commit
91a097e7478940483e76d52217f05bc05b98d5a5, which was very deliberate.

Third, you may then be asking for the recommended way to put an
aio=native medium into a CD-ROM drive. Good thing you ask, because we
have a way that we want to recommend but can't because it's still
considered experimental:

The BDS is added using blockdev-add, with all of the appropriate caching
and aio options. Then it's inserted into the drive using the
x-blockdev-insert-medium command, and the drive is closed using
blockdev-close-tray.

There are a couple of issues with this: First, blockdev-add and
x-blockdev-insert-medium are still experimental. The good news are that
I think the reason why the latter is experimental has actually
disappeared and we can just drop the x- by now. The
not-so-good-but-not-so-bad-either news are that we plan to get
blockdev-add declared non-experimental for 2.9. Let's see how it goes.

The other issue is that of course it's more cumbersome to use than a
simple 'change' via HMP. I argue that if someone communicates with a VM
by hand, they either have to deal with this added complexity or they
cannot use aio=native for CD-ROM drives -- which, as I said, I don't
think is too bad.

However, there is a reason why you cannot set cache/aio for an empty
drive and it's simply that it doesn't make sense.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/1] block: check full backing filename when searching protocol filenames

[Jeff Cody]

Forgot to cc qemu-block, added.

On Wed, Jan 25, 2017 at 12:22:02PM -0500, Jeff Cody wrote:
> In bdrv_find_backing_image(), if we are searching an image for a backing
> file that contains a protocol, we currently only compare unmodified
> paths.
> 
> However, some management software will change the backing filename to be
> a relative filename in a path.  QEMU is able to handle this fine,
> because internally it will use path_combine to put together the full
> protocol URI.
> 
> However, this can lead to an inability to match an image during a QAPI
> command that needs to use bdrv_find_backing_image() to find the image,
> when it is searched by the full URI.
> 
> When searching for a protocol filename, if the straight comparison
> fails, this patch will also compare against the full backing filename to
> see if that is a match.
> 
> Signed-off-by: Jeff Cody 
> ---
>  block.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 39ddea3..a173afc 100644
> --- a/block.c
> +++ b/block.c
> @@ -3145,6 +3145,7 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>  int is_protocol = 0;
>  BlockDriverState *curr_bs = NULL;
>  BlockDriverState *retval = NULL;
> +Error *local_error = NULL;
>  
>  if (!bs || !bs->drv || !backing_file) {
>  return NULL;
> @@ -3165,6 +3166,17 @@ BlockDriverState 
> *bdrv_find_backing_image(BlockDriverState *bs,
>  retval = curr_bs->backing->bs;
>  break;
>  }
> +/* Also check against the full backing filename for the image */
> +bdrv_get_full_backing_filename(curr_bs, backing_file_full, 
> PATH_MAX,
> +   &local_error);
> +if (local_error == NULL) {
> +if (strcmp(backing_file, backing_file_full) == 0) {
> +retval = curr_bs->backing->bs;
> +break;
> +}
> +} else {
> +error_free(local_error);
> +}
>  } else {
>  /* If not an absolute filename path, make it relative to the 
> current
>   * image's filename path */
> -- 
> 2.9.3
> 

[Qemu-devel] [PATCH v2 5/7] iscsi: Add timeout option

[Jeff Cody]

From: Kevin Wolf 

This was previously only available with -iscsi. Again, after this patch,
the -iscsi option only takes effect if an URL is given. New users are
supposed to use the new driver-specific option.

All -iscsi options have a corresponding driver-specific option for the
iscsi block driver now.

Reviewed-by: Daniel P. Berrange 
Signed-off-by: Kevin Wolf 
Signed-off-by: Jeff Cody 
---
 block/iscsi.c | 37 +++--
 1 file changed, 11 insertions(+), 26 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index a989b52..4701a27 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1318,29 +1318,6 @@ static char *get_initiator_name(QemuOpts *opts)
 return iscsi_name;
 }
 
-static int parse_timeout(const char *target)
-{
-QemuOptsList *list;
-QemuOpts *opts;
-const char *timeout;
-
-list = qemu_find_opts("iscsi");
-if (list) {
-opts = qemu_opts_find(list, target);
-if (!opts) {
-opts = QTAILQ_FIRST(&list->head);
-}
-if (opts) {
-timeout = qemu_opt_get(opts, "timeout");
-if (timeout) {
-return atoi(timeout);
-}
-}
-}
-
-return 0;
-}
-
 static void iscsi_nop_timed_event(void *opaque)
 {
 IscsiLun *iscsilun = opaque;
@@ -1549,7 +1526,7 @@ static void iscsi_parse_iscsi_option(const char *target, 
QDict *options)
 QemuOptsList *list;
 QemuOpts *opts;
 const char *user, *password, *password_secret, *initiator_name,
-   *header_digest;
+   *header_digest, *timeout;
 
 list = qemu_find_opts("iscsi");
 if (!list) {
@@ -1588,6 +1565,11 @@ static void iscsi_parse_iscsi_option(const char *target, 
QDict *options)
 if (header_digest) {
 qdict_set_default_str(options, "header-digest", header_digest);
 }
+
+timeout = qemu_opt_get(opts, "timeout");
+if (timeout) {
+qdict_set_default_str(options, "timeout", timeout);
+}
 }
 
 /*
@@ -1639,7 +1621,6 @@ static void iscsi_parse_filename(const char *filename, 
QDict *options,
 iscsi_destroy_url(iscsi_url);
 }
 
-/* TODO Add -iscsi options */
 static QemuOptsList runtime_opts = {
 .name = "iscsi",
 .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
@@ -1680,6 +1661,10 @@ static QemuOptsList runtime_opts = {
 .name = "header-digest",
 .type = QEMU_OPT_STRING,
 },
+{
+.name = "timeout",
+.type = QEMU_OPT_NUMBER,
+},
 { /* end of list */ }
 },
 };
@@ -1780,7 +1765,7 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 
 /* timeout handling is broken in libiscsi before 1.15.0 */
-timeout = parse_timeout(target);
+timeout = qemu_opt_get_number(opts, "timeout", 0);
 #if LIBISCSI_API_VERSION >= 20150621
 iscsi_set_timeout(iscsi, timeout);
 #else
-- 
2.9.3

Re: [Qemu-devel] [PATCH v3 1/2] move vm_start to cpus.c

[Claudio Imbrenda]

On 25/01/17 11:21, Paolo Bonzini wrote:
> 
> 
> On 28/10/2016 19:15, Claudio Imbrenda wrote:
>> * moves the call to qemu_clock_enable away from resume_all_vcpus, and
>>   add an explicit call to it before each instance of resume_all_vcpus
>>   in the code.
> 
> This change adds useless duplication, and isn't matched by a similar
> change to pause_all_vcpus.  You need to justify it; I suppose it is
> because the next patch will not call resume_all_cpus?

Actually it is because the next patch used to call resume_all_vcpus, and
we didn't want it to restart the clock too. But then I ended up not
using resume_all_vcpus.

And now I have actually no idea why we didn't want to restart the clock.
Maybe we should? After all some CPUs are running. I can patch
gdb_continue_partial to check if any CPU was actually restarted, and if
so start the clock.

So in the end it should still be correct, but the changes would be kept
small.

> Most of the callers of pause_all_vcpus/resume_all_vcpus don't let timers
> run, so the clock need not be disabled and enabled.  Maybe the right
> places to call qemu_clock_enable are cpu_disable_ticks and
> cpu_enable_ticks?  That should work for you.
> 
> In that case, please make the first patch the qemu_clock_enable
> movement; the second patch the introduction of vm_prepare_start; the
> third patch the gdbstub change.
> 
>> diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
>> index 3728a1e..5fa074b 100644
>> --- a/include/sysemu/cpus.h
>> +++ b/include/sysemu/cpus.h
>> @@ -5,6 +5,7 @@
>>  bool qemu_in_vcpu_thread(void);
>>  void qemu_init_cpu_loop(void);
>>  void resume_all_vcpus(void);
>> +void resume_some_vcpus(CPUState **cpus);
>>  void pause_all_vcpus(void);
>>  void cpu_stop_current(void);
>>  void cpu_ticks_init(void);
> 
> This function doesn't exist.

oops.


Claudio

Re: [Qemu-devel] [PATCH 1/1] Migration: libvirt live migration over RDMA of ipv6 addr failed

[David Z. Dai]

On Tue, 2017-01-24 at 17:36 +, Daniel P. Berrange wrote:
> On Tue, Jan 24, 2017 at 09:44:33AM -0600, David Dai wrote:
> > Using libvirt to do live migration over RDMA via ip v6 address failed. 
> > For example:
> > # virsh migrate  --live --migrateuri rdma://[deba::]:49152  \
> >   rhel73_host1_guest1 qemu+ssh://[deba::]/system --verbose
> > root@deba::'s password:
> > error: internal error: unable to execute QEMU command 'migrate': RDMA 
> > ERROR:  
> >   could not rdma_getaddrinfo address deba
> > 
> > As we can see, the ip v6 address used by rdma_getaddrinfo() has only "deba" 
> > part. It should be "deba::".
> > 
> > 1) According to rfc 3986, a literal ip v6 address should be enclosed 
> > in '[' and ']'.
> > When using virsh command to do live migration via ip v6 addresss, user
> > will input the ip v6 address with brackets (i.e. rdma://[deba::]:49152).
> > libvirt will parse command line option by calling virURIParse(). 
> > Inside it calls virStringStripIPv6Brackets() to strip off the brackets.
> > The uri passed in to virURIParse()  is:
> >"uri = rdma://[deba::]:49152"
> > Inside virURIParse() routine, it will strip off the bracket '[' and ']' if
> > it's ip v6 address. Then save the ip v6 address in this format "deba::" 
> > in the virURI->server field, and to be passed to qemu.
> > 
> > 2) At the beginning of migration, in qemu's qemu_rdma_data_init(host_port) 
> > routine, it calls inet_parse(host_port) routine to parse the ip v6 address 
> > and 
> > port string obtained from libvirt.
> > The input string host_port passed to qemu_rdma_data_init() can be:
> > "hostname:port", or
> > "ipv4address:port", or
> > "[ipv6address]:port" (i.e "[deba::]:49152"), or
> > "ipv6address:port" (i.e "deba:::49152").
> > Existing qemu api inet_parse() can handle the above first 3 cases properly, 
> >  
> > but didn't handle the last case ("ipv6address:port") correctly.
> > In this live migration over rdma via ip v6 address case, the server ip v6 
> > address obtained from libvirt doesn't contain the brackets '[' and ']' 
> > (i.e. "deba:::49152"). It caused inet_parse() to parse only "deba" 
> > part, 
> > and stopped at the 1st colon ':'. As the result, the subsequent 
> > rdma_getaddrinfo() with ip address "deba" will fail.
> > 
> > If we don't strip off brackets '[' and ']' for an ip v6 address in 
> > libvirt's 
> > virURIParse(), it will cause libvirt ipv6 ssh authentication failure.
> > 
> > NOTE:
> > If using libvirt to do live migration over TCP via ip v6 address:
> > # virsh migrate  --live --migrateuri tcp://[deba::]:49152  \
> >   rhel73_host1_guest1 qemu+ssh://[deba::]/system --verbose
> > It works fine.
> > In migrateuri of tcp case, libvirt will call virNetSocketNewConnectTCP()
> > directly to connect to remote "deba:::49152" after it strips off
> > the bracket '[' and ']' for an ip v6 address. 
> > On qemu side, fd_start_outgoing_migration() will be called to do migration.
> > It doesn't call inet_parse(). So we don't see issue in tcp case.
> > 
> > Solution:
> > I choose to fix the code in qemu's inet_parse() routine to parse the
> > ip v6 addresss w/o brackets properly (i.e. "deba:::49152" format).
> 
> That is not right - "deba::222:49152" is *not* a valid address. It
> is mandatory to use [] when providing a numeric IPv6 address. So
> there's nothing broken in QEMU here - libvirt needs fixing to pass
> correct data to QEMU
> 
> Regards,
> Daniel
Hi, Daniel:
   Just learned you are one of the libvirt maintainer from Dr. Gilbert.

   In Previous test, I changed libvirtd code in src/util/viruri.c file,
 virURIParse() routine to block
 "virStringStripIPv6Brackets(ret->server);" call. It caused ssh ipv6
authentication failure.
  
   You said libvirt needs to be changed to pass correct data to QEMU.
   
   As one of the solution on libvirt side: if I add this condition in
 virURIParse() routine:
+  if (strcmp(ret->scheme, "rdma")) {
/* Strip square bracket from an IPv6 address.
 * The function modifies the string in-place. Even after such
 * modification, it is OK to free the URI with xmlFreeURI. */
virStringStripIPv6Brackets(ret->server);
+  }

   Now both the virsh live migration with tcp and rdma via ipv6 address
work fine.
  
   Do you think this is the acceptable solution?

Thanks! - David Dai

Re: [Qemu-devel] [PATCH 1/1] Migration: libvirt live migration over RDMA of ipv6 addr failed

[Daniel P. Berrange]

On Wed, Jan 25, 2017 at 11:47:45AM -0600, David Z. Dai wrote:
>In Previous test, I changed libvirtd code in src/util/viruri.c file,
>  virURIParse() routine to block
>  "virStringStripIPv6Brackets(ret->server);" call. It caused ssh ipv6
> authentication failure.
>   
>You said libvirt needs to be changed to pass correct data to QEMU.
>
>As one of the solution on libvirt side: if I add this condition in
>  virURIParse() routine:
> +  if (strcmp(ret->scheme, "rdma")) {
> /* Strip square bracket from an IPv6 address.
>  * The function modifies the string in-place. Even after such
>  * modification, it is OK to free the URI with xmlFreeURI. */
> virStringStripIPv6Brackets(ret->server);
> +  }
> 
>Now both the virsh live migration with tcp and rdma via ipv6 address
> work fine.
>   
>Do you think this is the acceptable solution?

No, the viruri code is a general purpose URI parsing API and should
not be hacked with QEMU specific knowledge in this way.

You need to modify the QEMU driver code in libvirt to adding [] when
calling the QEMU migrate monitor command, and when creating the
-incoming CLI arg. eg modify various places in the src/qemu/qemu_*.c
files

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [PULL 00/14] target/xtensa updates

[Peter Maydell]

On 25 January 2017 at 00:37, Max Filippov  wrote:
> Hi Peter,
>
> please pull the following batch of updates for target/xtensa.
>
> The following changes since commit a470b33259bf82ef2336bfcd5d07640562d3f63b:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> staging (2016-12-22 19:23:51 +)
>
> are available in the git repository at:
>
>   git://github.com/OSLL/qemu-xtensa.git tags/20170124-xtensa
>
> for you to fetch changes up to 3a3c9dc4ca2eaa612cbd5d4c85d674b15eadfb02:
>
>   target-xtensa: implement RER/WER instructions (2017-01-16 19:19:03 -0800)
>
> 
> target/xtensa updates:
>
> - refactor CCOUNT/CCOMPARE (use QEMU timers instead of instruction counting);
> - support icount; run target/xtensa TCG tests with icount;
> - implement SMP prerequisites: static vector selection, RUNSTALL and RER/WER.
>

Applied, thanks.

-- PMM

[Qemu-devel] [PATCH v2 2/7] iscsi: Handle -iscsi user/password in bdrv_parse_filename()

[Jeff Cody]

From: Kevin Wolf 

This splits the logic in the old parse_chap() function into a part that
parses the -iscsi options into the new driver-specific options, and
another part that actually applies those options (called apply_chap()
now).

Note that this means that username and password specified with -iscsi
only take effect when a URL is provided. This is intentional, -iscsi is
a legacy interface only supported for compatibility, new users should
use the proper driver-specific options.

Signed-off-by: Kevin Wolf 
Signed-off-by: Jeff Cody 
---
 block/iscsi.c | 78 +--
 1 file changed, 44 insertions(+), 34 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 97cff30..fc91d0f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1236,29 +1236,14 @@ retry:
 return 0;
 }
 
-static void parse_chap(struct iscsi_context *iscsi, const char *target,
+static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
Error **errp)
 {
-QemuOptsList *list;
-QemuOpts *opts;
 const char *user = NULL;
 const char *password = NULL;
 const char *secretid;
 char *secret = NULL;
 
-list = qemu_find_opts("iscsi");
-if (!list) {
-return;
-}
-
-opts = qemu_opts_find(list, target);
-if (opts == NULL) {
-opts = QTAILQ_FIRST(&list->head);
-if (!opts) {
-return;
-}
-}
-
 user = qemu_opt_get(opts, "user");
 if (!user) {
 return;
@@ -1587,6 +1572,41 @@ out:
 }
 }
 
+static void iscsi_parse_iscsi_option(const char *target, QDict *options)
+{
+QemuOptsList *list;
+QemuOpts *opts;
+const char *user, *password, *password_secret;
+
+list = qemu_find_opts("iscsi");
+if (!list) {
+return;
+}
+
+opts = qemu_opts_find(list, target);
+if (opts == NULL) {
+opts = QTAILQ_FIRST(&list->head);
+if (!opts) {
+return;
+}
+}
+
+user = qemu_opt_get(opts, "user");
+if (user) {
+qdict_set_default_str(options, "user", user);
+}
+
+password = qemu_opt_get(opts, "password");
+if (password) {
+qdict_set_default_str(options, "password", password);
+}
+
+password_secret = qemu_opt_get(opts, "password-secret");
+if (password_secret) {
+qdict_set_default_str(options, "password-secret", password_secret);
+}
+}
+
 /*
  * We support iscsi url's on the form
  * iscsi://[%@][:]//
@@ -1625,6 +1645,9 @@ static void iscsi_parse_filename(const char *filename, 
QDict *options,
 qdict_set_default_str(options, "lun", lun_str);
 g_free(lun_str);
 
+/* User/password from -iscsi take precedence over those from the URL */
+iscsi_parse_iscsi_option(iscsi_url->target, options);
+
 if (iscsi_url->user[0] != '\0') {
 qdict_set_default_str(options, "user", iscsi_url->user);
 qdict_set_default_str(options, "password", iscsi_url->passwd);
@@ -1659,6 +1682,10 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 },
 {
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
+},
+{
 .name = "lun",
 .type = QEMU_OPT_NUMBER,
 },
@@ -1678,7 +1705,6 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 QemuOpts *opts;
 Error *local_err = NULL;
 const char *transport_name, *portal, *target;
-const char *user, *password;
 #if LIBISCSI_API_VERSION >= (20160603)
 enum iscsi_transport_type transport;
 #endif
@@ -1695,8 +1721,6 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 transport_name = qemu_opt_get(opts, "transport");
 portal = qemu_opt_get(opts, "portal");
 target = qemu_opt_get(opts, "target");
-user = qemu_opt_get(opts, "user");
-password = qemu_opt_get(opts, "password");
 lun = qemu_opt_get_number(opts, "lun", 0);
 
 if (!transport_name || !portal || !target) {
@@ -1704,11 +1728,6 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 ret = -EINVAL;
 goto out;
 }
-if (user && !password) {
-error_setg(errp, "If a user name is given, a password is required");
-ret = -EINVAL;
-goto out;
-}
 
 if (!strcmp(transport_name, "tcp")) {
 #if LIBISCSI_API_VERSION >= (20160603)
@@ -1747,17 +1766,8 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto out;
 }
 
-if (user) {
-ret = iscsi_set_initiator_username_pwd(iscsi, user, password);
-if (ret != 0) {
-error_setg(errp, "Failed to set initiator username and password");
-ret = -EINVAL;
-goto out;
-}
-}
-
 /* check if we got CHAP username/password via the options */
-parse_chap(iscsi, target, &local_err);
+apply_chap(iscsi, opts, &local_err);
 if (local_err != NULL) {
 error

[Qemu-devel] [PATCH v2 1/7] iscsi: Split URL into individual options

[Jeff Cody]

From: Kevin Wolf 

This introduces a .bdrv_parse_filename handler for iscsi which parses an
URL if given and translates it to individual options.

Signed-off-by: Kevin Wolf 
Signed-off-by: Jeff Cody 
---
 block/iscsi.c | 189 ++
 1 file changed, 136 insertions(+), 53 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 6aeeb9e..97cff30 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1470,20 +1470,6 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, 
Error **errp)
 }
 }
 
-/* TODO Convert to fine grained options */
-static QemuOptsList runtime_opts = {
-.name = "iscsi",
-.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
-.desc = {
-{
-.name = "filename",
-.type = QEMU_OPT_STRING,
-.help = "URL to the iscsi image",
-},
-{ /* end of list */ }
-},
-};
-
 static struct scsi_task *iscsi_do_inquiry(struct iscsi_context *iscsi, int lun,
   int evpd, int pc, void **inq, Error 
**errp)
 {
@@ -1605,20 +1591,98 @@ out:
  * We support iscsi url's on the form
  * iscsi://[%@][:]//
  */
+static void iscsi_parse_filename(const char *filename, QDict *options,
+ Error **errp)
+{
+struct iscsi_url *iscsi_url;
+const char *transport_name;
+char *lun_str;
+
+iscsi_url = iscsi_parse_full_url(NULL, filename);
+if (iscsi_url == NULL) {
+error_setg(errp, "Failed to parse URL : %s", filename);
+return;
+}
+
+#if LIBISCSI_API_VERSION >= (20160603)
+switch (iscsi_url->transport) {
+case TCP_TRANSPORT: transport_name = "tcp"; break;
+case ISER_TRANSPORT:transport_name = "iser"; break;
+default:
+error_setg(errp, "Unknown transport type (%d)",
+   iscsi_url->transport);
+return;
+}
+#else
+transport_name = "tcp";
+#endif
+
+qdict_set_default_str(options, "transport", transport_name);
+qdict_set_default_str(options, "portal", iscsi_url->portal);
+qdict_set_default_str(options, "target", iscsi_url->target);
+
+lun_str = g_strdup_printf("%d", iscsi_url->lun);
+qdict_set_default_str(options, "lun", lun_str);
+g_free(lun_str);
+
+if (iscsi_url->user[0] != '\0') {
+qdict_set_default_str(options, "user", iscsi_url->user);
+qdict_set_default_str(options, "password", iscsi_url->passwd);
+}
+
+iscsi_destroy_url(iscsi_url);
+}
+
+/* TODO Add -iscsi options */
+static QemuOptsList runtime_opts = {
+.name = "iscsi",
+.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+.desc = {
+{
+.name = "transport",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "portal",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "target",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "user",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "password",
+.type = QEMU_OPT_STRING,
+},
+{
+.name = "lun",
+.type = QEMU_OPT_NUMBER,
+},
+{ /* end of list */ }
+},
+};
+
 static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
   Error **errp)
 {
 IscsiLun *iscsilun = bs->opaque;
 struct iscsi_context *iscsi = NULL;
-struct iscsi_url *iscsi_url = NULL;
 struct scsi_task *task = NULL;
 struct scsi_inquiry_standard *inq = NULL;
 struct scsi_inquiry_supported_pages *inq_vpd;
 char *initiator_name = NULL;
 QemuOpts *opts;
 Error *local_err = NULL;
-const char *filename;
-int i, ret = 0, timeout = 0;
+const char *transport_name, *portal, *target;
+const char *user, *password;
+#if LIBISCSI_API_VERSION >= (20160603)
+enum iscsi_transport_type transport;
+#endif
+int i, ret = 0, timeout = 0, lun;
 
 opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
 qemu_opts_absorb_qdict(opts, options, &local_err);
@@ -1628,18 +1692,41 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto out;
 }
 
-filename = qemu_opt_get(opts, "filename");
+transport_name = qemu_opt_get(opts, "transport");
+portal = qemu_opt_get(opts, "portal");
+target = qemu_opt_get(opts, "target");
+user = qemu_opt_get(opts, "user");
+password = qemu_opt_get(opts, "password");
+lun = qemu_opt_get_number(opts, "lun", 0);
 
-iscsi_url = iscsi_parse_full_url(iscsi, filename);
-if (iscsi_url == NULL) {
-error_setg(errp, "Failed to parse URL : %s", filename);
+if (!transport_name || !portal || !target) {
+error_setg(errp, "Need all of transport, portal and target options");
+ret = -EINVAL;
+goto out;
+}
+if (user && !password) {
+error_setg(errp, "If a user n

[Qemu-devel] [PATCH v2 7/7] QAPI: Fix blockdev-add example documentation

[Jeff Cody]

Signed-off-by: Jeff Cody 
---
 qapi/block-core.json | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4ebb8d8..adc089f 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2909,21 +2909,24 @@
 # 1.
 # -> { "execute": "blockdev-add",
 #  "arguments": {
-#  "options" : { "driver": "qcow2",
-#"file": { "driver": "file",
-#  "filename": "test.qcow2" } } } }
+#   "driver": "qcow2",
+#   "node-name": "test1",
+#   "file": {
+#   "driver": "file",
+#   "filename": "test.qcow2"
+#}
+#   }
+# }
 # <- { "return": {} }
 #
 # 2.
 # -> { "execute": "blockdev-add",
 #  "arguments": {
-#  "options": {
 #"driver": "qcow2",
 #"node-name": "node0",
 #"discard": "unmap",
 #"cache": {
-#"direct": true,
-#"writeback": true
+#"direct": true
 #},
 #"file": {
 #"driver": "file",
@@ -2936,7 +2939,6 @@
 #"filename": "/dev/fdset/4"
 #}
 #}
-#  }
 #}
 #  }
 #
@@ -2964,14 +2966,12 @@
 #
 # -> { "execute": "blockdev-add",
 #  "arguments": {
-#  "options": {
 #  "driver": "qcow2",
 #  "node-name": "node0",
 #  "file": {
 #  "driver": "file",
 #  "filename": "test.qcow2"
 #  }
-#  }
 #  }
 #}
 # <- { "return": {} }
-- 
2.9.3

[Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support

[Jeff Cody]

From: Kevin Wolf 

This adds blockdev-add support for iscsi devices.

Reviewed-by: Daniel P. Berrange 
Signed-off-by: Kevin Wolf 
Signed-off-by: Jeff Cody 
---
 block/iscsi.c| 14 ++
 qapi/block-core.json | 74 
 2 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 4701a27..65484f0 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1282,13 +1282,13 @@ static void apply_header_digest(struct iscsi_context 
*iscsi, QemuOpts *opts,
 digest = qemu_opt_get(opts, "header-digest");
 if (!digest) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
-} else if (!strcmp(digest, "CRC32C")) {
+} else if (!strcmp(digest, "crc32c")) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
-} else if (!strcmp(digest, "NONE")) {
+} else if (!strcmp(digest, "none")) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
-} else if (!strcmp(digest, "CRC32C-NONE")) {
+} else if (!strcmp(digest, "crc32c-none")) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C_NONE);
-} else if (!strcmp(digest, "NONE-CRC32C")) {
+} else if (!strcmp(digest, "none-crc32c")) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
 } else {
 error_setg(errp, "Invalid header-digest setting : %s", digest);
@@ -1563,7 +1563,11 @@ static void iscsi_parse_iscsi_option(const char *target, 
QDict *options)
 
 header_digest = qemu_opt_get(opts, "header-digest");
 if (header_digest) {
-qdict_set_default_str(options, "header-digest", header_digest);
+/* -iscsi takes upper case values, but QAPI only supports lower case
+ * enum constant names, so we have to convert here. */
+char *qapi_value = g_ascii_strdown(header_digest, -1);
+qdict_set_default_str(options, "header-digest", qapi_value);
+g_free(qapi_value);
 }
 
 timeout = qemu_opt_get(opts, "timeout");
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 1b3e6eb..4ebb8d8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2116,10 +2116,10 @@
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
-'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio',
-'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
-'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc',
-'vvfat' ] }
+'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
+'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
+'quorum', 'raw', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk',
+'vpc', 'vvfat' ] }
 
 ##
 # @BlockdevOptionsFile:
@@ -2601,6 +2601,70 @@
 '*logfile': 'str' } }
 
 ##
+# @IscsiTransport:
+#
+# An enumeration of libiscsi transport types
+#
+# Since: 2.9
+##
+{ 'enum': 'IscsiTransport',
+  'data': [ 'tcp', 'iser' ] }
+
+##
+# @IscsiHeaderDigest:
+#
+# An enumeration of header digests supported by libiscsi
+#
+# Since: 2.9
+##
+{ 'enum': 'IscsiHeaderDigest',
+  'prefix': 'QAPI_ISCSI_HEADER_DIGEST',
+  'data': [ 'crc32c', 'none', 'crc32c-none', 'none-crc32c' ] }
+
+##
+# @BlockdevOptionsIscsi:
+#
+# @transportThe iscsi transport type
+#
+# @portal   The address of the iscsi portal
+#
+# @target   The target iqn name
+#
+# @lun  #optional LUN to connect to. Defaults to 0.
+#
+# @user #optional User name to log in with. If omitted, no CHAP
+#   authentication is performed.
+#
+# @password-secret  #optional The ID of a QCryptoSecret object providing
+#   the password for the login. This option is required if
+#   @user is specified.
+#
+# @initiator-name   #optional The iqn name we want to identify to the target
+#   as. If this option is not specified, an initiator name is
+#   generated automatically.
+#
+# @header-digest#optional The desired header digest. Defaults to
+#   none-crc32c.
+#
+# @timeout  #optional Timeout in seconds after which a request will
+#   timeout. 0 means no timeout and is the default.
+#
+# Driver specific block device options for iscsi
+#
+# Since: 2.9
+##
+{ 'struct': 'BlockdevOptionsIscsi',
+  'data': { 'transport': 'IscsiTransport',
+'portal': 'str',
+'target': 'str',
+'*lun': 'int',
+'*user': 'str',
+'*password-secret': 'str',
+'*initiator-name': 'str',
+'*header-digest': 'IscsiHeaderDigest',
+'*timeout': 'int' } }
+
+##
 # @ReplicationMode:
 #
 # An enumeration of replication modes.
@@ -2786,7 +2850,7 @@
   'host_device':'BlockdevOptionsFile',
   

[Qemu-devel] [PATCH v2 3/7] iscsi: Add initiator-name option

[Jeff Cody]

From: Kevin Wolf 

This was previously only available with -iscsi. Again, after this patch,
the -iscsi option only takes effect if an URL is given. New users are
supposed to use the new driver-specific option.

Reviewed-by: Daniel P. Berrange 
Signed-off-by: Kevin Wolf 
Signed-off-by: Jeff Cody 
---
 block/iscsi.c | 32 +++-
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index fc91d0f..3401b7e 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1312,26 +1312,15 @@ static void parse_header_digest(struct iscsi_context 
*iscsi, const char *target,
 }
 }
 
-static char *parse_initiator_name(const char *target)
+static char *get_initiator_name(QemuOpts *opts)
 {
-QemuOptsList *list;
-QemuOpts *opts;
 const char *name;
 char *iscsi_name;
 UuidInfo *uuid_info;
 
-list = qemu_find_opts("iscsi");
-if (list) {
-opts = qemu_opts_find(list, target);
-if (!opts) {
-opts = QTAILQ_FIRST(&list->head);
-}
-if (opts) {
-name = qemu_opt_get(opts, "initiator-name");
-if (name) {
-return g_strdup(name);
-}
-}
+name = qemu_opt_get(opts, "initiator-name");
+if (name) {
+return g_strdup(name);
 }
 
 uuid_info = qmp_query_uuid(NULL);
@@ -1576,7 +1565,7 @@ static void iscsi_parse_iscsi_option(const char *target, 
QDict *options)
 {
 QemuOptsList *list;
 QemuOpts *opts;
-const char *user, *password, *password_secret;
+const char *user, *password, *password_secret, *initiator_name;
 
 list = qemu_find_opts("iscsi");
 if (!list) {
@@ -1605,6 +1594,11 @@ static void iscsi_parse_iscsi_option(const char *target, 
QDict *options)
 if (password_secret) {
 qdict_set_default_str(options, "password-secret", password_secret);
 }
+
+initiator_name = qemu_opt_get(opts, "initiator-name");
+if (initiator_name) {
+qdict_set_default_str(options, "initiator-name", initiator_name);
+}
 }
 
 /*
@@ -1689,6 +1683,10 @@ static QemuOptsList runtime_opts = {
 .name = "lun",
 .type = QEMU_OPT_NUMBER,
 },
+{
+.name = "initiator-name",
+.type = QEMU_OPT_STRING,
+},
 { /* end of list */ }
 },
 };
@@ -1745,7 +1743,7 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 memset(iscsilun, 0, sizeof(IscsiLun));
 
-initiator_name = parse_initiator_name(target);
+initiator_name = get_initiator_name(opts);
 
 iscsi = iscsi_create_context(initiator_name);
 if (iscsi == NULL) {
-- 
2.9.3

[Qemu-devel] [PATCH v2 4/7] iscsi: Add header-digest option

[Jeff Cody]

From: Kevin Wolf 

This was previously only available with -iscsi. Again, after this patch,
the -iscsi option only takes effect if an URL is given. New users are
supposed to use the new driver-specific option.

Reviewed-by: Daniel P. Berrange 
Signed-off-by: Kevin Wolf 
Signed-off-by: Jeff Cody 
---
 block/iscsi.c | 39 +++
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3401b7e..a989b52 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1274,32 +1274,15 @@ static void apply_chap(struct iscsi_context *iscsi, 
QemuOpts *opts,
 g_free(secret);
 }
 
-static void parse_header_digest(struct iscsi_context *iscsi, const char 
*target,
+static void apply_header_digest(struct iscsi_context *iscsi, QemuOpts *opts,
 Error **errp)
 {
-QemuOptsList *list;
-QemuOpts *opts;
 const char *digest = NULL;
 
-list = qemu_find_opts("iscsi");
-if (!list) {
-return;
-}
-
-opts = qemu_opts_find(list, target);
-if (opts == NULL) {
-opts = QTAILQ_FIRST(&list->head);
-if (!opts) {
-return;
-}
-}
-
 digest = qemu_opt_get(opts, "header-digest");
 if (!digest) {
-return;
-}
-
-if (!strcmp(digest, "CRC32C")) {
+iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
+} else if (!strcmp(digest, "CRC32C")) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
 } else if (!strcmp(digest, "NONE")) {
 iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
@@ -1565,7 +1548,8 @@ static void iscsi_parse_iscsi_option(const char *target, 
QDict *options)
 {
 QemuOptsList *list;
 QemuOpts *opts;
-const char *user, *password, *password_secret, *initiator_name;
+const char *user, *password, *password_secret, *initiator_name,
+   *header_digest;
 
 list = qemu_find_opts("iscsi");
 if (!list) {
@@ -1599,6 +1583,11 @@ static void iscsi_parse_iscsi_option(const char *target, 
QDict *options)
 if (initiator_name) {
 qdict_set_default_str(options, "initiator-name", initiator_name);
 }
+
+header_digest = qemu_opt_get(opts, "header-digest");
+if (header_digest) {
+qdict_set_default_str(options, "header-digest", header_digest);
+}
 }
 
 /*
@@ -1687,6 +1676,10 @@ static QemuOptsList runtime_opts = {
 .name = "initiator-name",
 .type = QEMU_OPT_STRING,
 },
+{
+.name = "header-digest",
+.type = QEMU_OPT_STRING,
+},
 { /* end of list */ }
 },
 };
@@ -1778,10 +1771,8 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto out;
 }
 
-iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
-
 /* check if we got HEADER_DIGEST via the options */
-parse_header_digest(iscsi, target, &local_err);
+apply_header_digest(iscsi, opts, &local_err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
 ret = -EINVAL;
-- 
2.9.3

[Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support

[Jeff Cody]

This adds blockdev-add support to the iscsi block driver.

Picked this series up from Kevin.  I've tested it on my local iscsi setup.

There are only a few minor changes:

* In patch 2, fixed the segfault pointed out by Daniel
* In patch 6, placed the ':' after the command header as now required
* New patch 7, to fix some out of date documentation in the qapi schema


Jeff Cody (1):
  QAPI: Fix blockdev-add example documentation

Kevin Wolf (6):
  iscsi: Split URL into individual options
  iscsi: Handle -iscsi user/password in bdrv_parse_filename()
  iscsi: Add initiator-name option
  iscsi: Add header-digest option
  iscsi: Add timeout option
  iscsi: Add blockdev-add support

 block/iscsi.c| 349 +++
 qapi/block-core.json |  92 +++---
 2 files changed, 288 insertions(+), 153 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Paolo Bonzini]

On 25/01/2017 18:36, Alex Williamson wrote:
>> You probably should also put a comment about why VFIO does *not* need to
>> keep a reference between vfio_dma_map and vfio_dma_unmap (which doesn't
>> sound easy to do either).  Would any well-behaved guest invalidate the
>> IOMMU page tables before a memory hot-unplug?
>
> Hmm, we do take a reference in vfio_listener_region_add(), but this is
> of course to the iommu region not to the RAM region we're translating.
> In the non-vIOMMU case we would be holding a reference to the memory
> region backing a DMA mapping.  I would expect a well behaved guest to
> evacuate DMA mappings targeting a hotplug memory region before it gets
> ejected, but how much do we want to rely on well behaved guests.

It depends of what happens if they aren't.  I think it's fine (see other
message), but taking a reference for each mapping entry isn't so easy
because the unmap case doesn't know the old memory region.

Paolo


> Perhaps we should be taking a reference for each mapping entry, though
> this makes Peter's plans to invalidate the entire address space much
> more difficult.  Thanks,

Re: [Qemu-devel] [PATCH v5 02/18] vfio: introduce vfio_get_vaddr()

[Alex Williamson]

On Wed, 25 Jan 2017 18:11:37 +0100
Paolo Bonzini  wrote:

> On 24/01/2017 17:29, Alex Williamson wrote:
> > On Tue, 24 Jan 2017 18:25:55 +0800
> > Peter Xu  wrote:
> >   
> >> A cleanup for vfio_iommu_map_notify(). Should have no functional change,
> >> just to make the function shorter and easier to understand.
> >>
> >> Signed-off-by: Peter Xu 
> >> ---
> >>  hw/vfio/common.c | 58 
> >> +---
> >>  1 file changed, 38 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> >> index 174f351..ce55dff 100644
> >> --- a/hw/vfio/common.c
> >> +++ b/hw/vfio/common.c
> >> @@ -294,25 +294,14 @@ static bool 
> >> vfio_listener_skipped_section(MemoryRegionSection *section)
> >> section->offset_within_address_space & (1ULL << 63);
> >>  }
> >>  
> >> -static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> >> +static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
> >> +   bool *read_only)
> >>  {
> >> -VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> >> -VFIOContainer *container = giommu->container;
> >> -hwaddr iova = iotlb->iova + giommu->iommu_offset;
> >>  MemoryRegion *mr;
> >>  hwaddr xlat;
> >>  hwaddr len = iotlb->addr_mask + 1;
> >> -void *vaddr;
> >> -int ret;
> >> -
> >> -trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : 
> >> "MAP",
> >> -iova, iova + iotlb->addr_mask);
> >> -
> >> -if (iotlb->target_as != &address_space_memory) {
> >> -error_report("Wrong target AS \"%s\", only system memory is 
> >> allowed",
> >> - iotlb->target_as->name ? iotlb->target_as->name : 
> >> "none");
> >> -return;
> >> -}
> >> +bool ret = false;
> >> +bool writable = iotlb->perm & IOMMU_WO;
> >>  
> >>  /*
> >>   * The IOMMU TLB entry we have just covers translation through
> >> @@ -322,12 +311,13 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
> >> IOMMUTLBEntry *iotlb)
> >>  rcu_read_lock();
> >>  mr = address_space_translate(&address_space_memory,
> >>   iotlb->translated_addr,
> >> - &xlat, &len, iotlb->perm & IOMMU_WO);
> >> + &xlat, &len, writable);
> >>  if (!memory_region_is_ram(mr)) {
> >>  error_report("iommu map to non memory area %"HWADDR_PRIx"",
> >>   xlat);
> >>  goto out;
> >>  }
> >> +
> >>  /*
> >>   * Translation truncates length to the IOMMU page size,
> >>   * check that it did not truncate too much.
> >> @@ -337,11 +327,41 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
> >> IOMMUTLBEntry *iotlb)
> >>  goto out;
> >>  }
> >>  
> >> +*vaddr = memory_region_get_ram_ptr(mr) + xlat;
> >> +*read_only = !writable || mr->readonly;
> >> +ret = true;
> >> +
> >> +out:
> >> +rcu_read_unlock();
> >> +return ret;
> >> +}
> >> +
> >> +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> >> +{
> >> +VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> >> +VFIOContainer *container = giommu->container;
> >> +hwaddr iova = iotlb->iova + giommu->iommu_offset;
> >> +bool read_only;
> >> +void *vaddr;
> >> +int ret;
> >> +
> >> +trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : 
> >> "MAP",
> >> +iova, iova + iotlb->addr_mask);
> >> +
> >> +if (iotlb->target_as != &address_space_memory) {
> >> +error_report("Wrong target AS \"%s\", only system memory is 
> >> allowed",
> >> + iotlb->target_as->name ? iotlb->target_as->name : 
> >> "none");
> >> +return;
> >> +}
> >> +
> >> +if (!vfio_get_vaddr(iotlb, &vaddr, &read_only)) {
> >> +return;
> >> +}
> >> +
> >>  if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
> >> -vaddr = memory_region_get_ram_ptr(mr) + xlat;
> >>  ret = vfio_dma_map(container, iova,
> >> iotlb->addr_mask + 1, vaddr,
> >> -   !(iotlb->perm & IOMMU_WO) || mr->readonly);
> >> +   read_only);
> >>  if (ret) {
> >>  error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
> >>   "0x%"HWADDR_PRIx", %p) = %d (%m)",
> >> @@ -357,8 +377,6 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
> >> IOMMUTLBEntry *iotlb)
> >>   iotlb->addr_mask + 1, ret);
> >>  }
> >>  }
> >> -out:
> >> -rcu_read_unlock();  
> > 
> > The comment from v4 still needs input from Paolo, is it valid to make
> > use of vaddr (based on address_space_translate ->
> > memory_region_get_ram_ptr) outside of the rcu read lock or could future
> > BQL reduction efforts allow this to race?  
> 
> You need 

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[Phil Dennis-Jordan]

On 25 January 2017 at 18:27,   wrote:
> Your series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility 
> of usb-tablet
> Message-id: 1485365075-32702-1-git-send-email-p...@philjordan.eu
>
> === OUTPUT BEGIN ===
> Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of 
> usb-tablet...
> ERROR: code indent should never use tabs
> #43: FILE: hw/usb/dev-hid.c:490:
> +0x09, 0x02,^I^I/* Usage (Mouse) */$

Interestingly, the surrounding array initialisation already uses tabs,
so replacing them with spaces on only the line I edited seems wrong as
it'll mis-render in editors configured with a different tab width.
Please let me know if I need to take action on this issue, and if so
what to do. (I can add a whitespace-only patch to fix the surrounding
area, for example. Coding guidelines suggest this might not be
desirable though.)

Re: [Qemu-devel] [PATCH v4 1/9] ACPI: Add a function for building named qword entries

[Ben Warren]

Hi Laszlo,

> On Jan 24, 2017, at 7:55 PM, Laszlo Ersek  wrote:
> 
> Hi Ben,
> 
> sorry about being late to reviewing this series. I hope I can now spend
> more time on it.
> 
> - Please do not try to address my comments immediately. It's very
> possible (even likely) that Igor, MST and myself could have different
> opinions on things, so first please await agreement between your reviewers.
> 
Thanks for the very detailed review.  I’ll give it a couple of days and then 
start work on the suggested changes.

> - I think you should have CC'd Igor and Michael directly. I'm adding
> them to this reply; hopefully that will be enough for them to monitor
> this series.
> 
> - I'll likely be unable to review everything with 100% coverage; so
> addressing (or sufficiently refuting) my comments might not guarantee
> that the next version will be merged at once.
> 
> With all that said:
> 
> On 01/25/17 02:43, b...@skyportsystems.com  
> wrote:
>> From: Ben Warren mailto:b...@skyportsystems.com>>
>> 
>> This is initially used to patch a 64-bit address into the VM Generation ID 
>> SSDT
> 
> (1) I think this commit message line is overlong; I think we wrap at 74
> chars or so. Not critical, but worth mentioning.
> 
>> 
>> Signed-off-by: Ben Warren 
>> ---
>> hw/acpi/aml-build.c | 28 
>> include/hw/acpi/aml-build.h |  4 
>> 2 files changed, 32 insertions(+)
>> 
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index b2a1e40..dc4edc2 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -285,6 +285,34 @@ build_append_named_dword(GArray *array, const char 
>> *name_format, ...)
>> return offset;
>> }
>> 
>> +/*
>> + * Build NAME(, 0x) where 0x is encoded as a qword,
>> + * and return the offset to 0x for runtime patching.
>> + *
>> + * Warning: runtime patching is best avoided. Only use this as
>> + * a replacement for DataTableRegion (for guests that don't
>> + * support it).
>> + */
> 
> (2) Since we're adding a qword (8-byte integer), the hexadecimal
> constant should have 16 nibbles: 0x. (After copying the
> comment from build_append_named_dword(), it should be actualized.)
> 
> (3) Normally the functions in this file that create AML operators carry
> a note about the ACPI spec version and exact location that defines the
> operator. I see that commit f20354910893 ("acpi: add
> build_append_named_dword, returning an offset in buffer", 2016-03-01)
> missed that too.
> 
> Unless this tradition has been willfully abandoned, I suggest that you
> add the right reference here, and also (in retrospect) to
> build_append_named_dword().
> 
>> +int
>> +build_append_named_qword(GArray *array, const char *name_format, ...)
>> +{
>> +int offset;
>> +va_list ap;
>> +
>> +build_append_byte(array, 0x08); /* NameOp */
>> +va_start(ap, name_format);
>> +build_append_namestringv(array, name_format, ap);
>> +va_end(ap);
>> +
>> +build_append_byte(array, 0x0E); /* QWordPrefix */
>> +
>> +offset = array->len;
>> +build_append_int_noprefix(array, 0x, 8);
> 
> (4) I guess the constant should be updated here too, for consistency
> with the comment.
> 
> The rest looks okay. (I didn't verify 0x0E == QWordPrefix specifically,
> because an error there would break the functionality immediately, and
> you'd notice.)
> 
> Thanks!
> Laszlo
> 
>> +assert(array->len == offset + 8);
>> +
>> +return offset;
>> +}
>> +
>> static GPtrArray *alloc_list;
>> 
>> static Aml *aml_alloc(void)
>> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
>> index 559326c..dbf63cf 100644
>> --- a/include/hw/acpi/aml-build.h
>> +++ b/include/hw/acpi/aml-build.h
>> @@ -385,6 +385,10 @@ int
>> build_append_named_dword(GArray *array, const char *name_format, ...)
>> GCC_FMT_ATTR(2, 3);
>> 
>> +int
>> +build_append_named_qword(GArray *array, const char *name_format, ...)
>> +GCC_FMT_ATTR(2, 3);
>> +
>> void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
>>uint64_t len, int node, MemoryAffinityFlags flags);

—Ben



smime.p7s
Description: S/MIME cryptographic signature

Re: [Qemu-devel] [PATCH v2 0/3] x86-kvm: Fix Mac guest timekeeping by exposing TSC frequency in CPUID

[Paolo Bonzini]

On 25/01/2017 18:26, Phil Dennis-Jordan wrote:
> On 24 January 2017 at 12:58, Paolo Bonzini  wrote:
>> Looks good, thanks!  Queued patches 1 and 3 for 2.9 (patch 2 is in already).
> 
> Awesome, thanks for your help!
> 
> As there are now some minor merge conflicts with upstream, I've
> rebased the remaining 2 patches on latest master, they're in the
> vmware-cpuid-freq branch at https://github.com/pmj/qemu.git - in case
> that makes your job of merging them a bit easier.

I'll compare them with my own resolution, thanks!

Paolo

Re: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of usb-tablet

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v3] hw/usb/dev-hid: Improve guest compatibility of 
usb-tablet
Message-id: 1485365075-32702-1-git-send-email-p...@philjordan.eu

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/1485297322-10595-1-git-send-email-...@redhat.com 
-> patchew/1485297322-10595-1-git-send-email-...@redhat.com
 * [new tag] 
patchew/1485365075-32702-1-git-send-email-p...@philjordan.eu -> 
patchew/1485365075-32702-1-git-send-email-p...@philjordan.eu
 - [tag update]  patchew/20170124110151.937-1-berra...@redhat.com -> 
patchew/20170124110151.937-1-berra...@redhat.com
 - [tag update]  patchew/20170125121710.13561-1-cornelia.h...@de.ibm.com -> 
patchew/20170125121710.13561-1-cornelia.h...@de.ibm.com
Switched to a new branch 'test'
bdba2e5 hw/usb/dev-hid: Improve guest compatibility of usb-tablet

=== OUTPUT BEGIN ===
Checking PATCH 1/1: hw/usb/dev-hid: Improve guest compatibility of usb-tablet...
ERROR: code indent should never use tabs
#43: FILE: hw/usb/dev-hid.c:490:
+0x09, 0x02,^I^I/* Usage (Mouse) */$

total: 1 errors, 0 warnings, 24 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH] target-openrisc: Fix exception handling status registers

[Richard Henderson]

On 01/25/2017 04:34 AM, Stafford Horne wrote:
> Hmm, I just tried your qemu branch and mine:
> 
>   g...@github.com:stffrdhrn/qemu.git or1k-fix-sigill
> 
> Both of them were able to boot fine.
> 
> The opencores,or1200-rtlsvn481 cpu node is in the or1ksim device tree
> definition.  Are you sure your kernel config has this in it?
> 
>   CONFIG_OPENRISC_BUILTIN_DTB="or1ksim"
> 
> Could you send your kernel config?
> 
> I have attached mine, just in case.  For next-20170124

Curious.  The entire diff between our configs is just

--- ../or-config2017-01-24 09:14:51.918059107 -0800
+++ .config 2017-01-25 09:18:14.153999754 -0800
@@ -18,7 +18,7 @@
 #
 CONFIG_BROKEN_ON_SMP=y
 CONFIG_INIT_ENV_ARG_LIMIT=32
-CONFIG_CROSS_COMPILE="or1k-linux-musl-"
+CONFIG_CROSS_COMPILE="or1k-musl-linux-"
 # CONFIG_COMPILE_TEST is not set
 CONFIG_LOCALVERSION=""
 CONFIG_LOCALVERSION_AUTO=y

So you're saying you *do* get serial port output?

Perhaps you can just send me your kernel...


r~