Re: [Qemu-devel] [PATCH 2/2] linux-user: remove #define smp_{cores, threads}

[David Gibson]

On Fri, Sep 16, 2016 at 04:36:48PM -0300, Eduardo Habkost wrote:
> On Fri, Sep 16, 2016 at 07:50:24PM +0400, Marc-André Lureau wrote:
> > Those are unneeded now that CPUState nr_{cores,threads} is always
> > initialized.
> > 
> > Signed-off-by: Marc-André Lureau 
> 
> Reviewed-by: Eduardo Habkost 
> 
> I will wait for at least an Acked-by from the PPC maintainers
> before I merge it, though.

As I've said elsewhere, I'm not entirely convinced that the vcpu
structure is the place to keep this info.

But since it's there, we might as well use it.

Acked-by: David Gibson 

> 
> > ---
> >  target-i386/cpu.c   | 8 
> >  target-ppc/translate_init.c | 3 ++-
> >  include/sysemu/cpus.h   | 5 +
> >  3 files changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 5a5299a..e863bea 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -2490,13 +2490,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t 
> > index, uint32_t count,
> >  
> >  switch (count) {
> >  case 0:
> > -*eax = apicid_core_offset(smp_cores, smp_threads);
> > -*ebx = smp_threads;
> > +*eax = apicid_core_offset(cs->nr_cores, cs->nr_threads);
> > +*ebx = cs->nr_threads;
> >  *ecx |= CPUID_TOPOLOGY_LEVEL_SMT;
> >  break;
> >  case 1:
> > -*eax = apicid_pkg_offset(smp_cores, smp_threads);
> > -*ebx = smp_cores * smp_threads;
> > +*eax = apicid_pkg_offset(cs->nr_cores, cs->nr_threads);
> > +*ebx = cs->nr_cores * cs->nr_threads;
> >  *ecx |= CPUID_TOPOLOGY_LEVEL_CORE;
> >  break;
> >  default:
> > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> > index 407ccb9..b66b40b 100644
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -9943,7 +9943,8 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, 
> > Error **errp)
> >  
> >  int ppc_get_compat_smt_threads(PowerPCCPU *cpu)
> >  {
> > -int ret = MIN(smp_threads, kvmppc_smt_threads());
> > +CPUState *cs = CPU(cpu);
> > +int ret = MIN(cs->nr_threads, kvmppc_smt_threads());
> >  
> >  switch (cpu->cpu_version) {
> >  case CPU_POWERPC_LOGICAL_2_05:
> > diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
> > index fe992a8..3728a1e 100644
> > --- a/include/sysemu/cpus.h
> > +++ b/include/sysemu/cpus.h
> > @@ -29,12 +29,9 @@ void qtest_clock_warp(int64_t dest);
> >  
> >  #ifndef CONFIG_USER_ONLY
> >  /* vl.c */
> > +/* *-user doesn't have configurable SMP topology */
> >  extern int smp_cores;
> >  extern int smp_threads;
> > -#else
> > -/* *-user doesn't have configurable SMP topology */
> > -#define smp_cores   1
> > -#define smp_threads 1
> >  #endif
> >  
> >  void list_cpus(FILE *f, fprintf_function cpu_fprintf, const char *optarg);
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices

[Dong Jia Shi]

* Jike Song  [2016-09-13 10:35:11 +0800]:

> On 09/08/2016 10:45 AM, Jike Song wrote:
> > On 08/25/2016 05:22 PM, Dong Jia wrote:
> >> On Thu, 25 Aug 2016 09:23:53 +0530
> >> Kirti Wankhede  wrote:
> >>
> >> [...]
> >>
> >> Dear Kirti,
> >>
> >> I just rebased my vfio-ccw patches to this series.
> >> With a little fix, which was pointed it out in my reply to the #3
> >> patch, it works fine.
> >>
> > 
> > Hi Jia,
> > 
> > Sorry I didn't follow a lot in previous discussion, but since
> > vfio-mdev in v7 patchset is at least PCI-agnostic, would you share
> > with us why you still need a vfio-ccw?
> 
> Kind ping :)
> 
> 
> Hi Dong Jia,
> 
> Since Kirti has confirmed that in v7 it is designed to have only one
> vfio-mdev driver for all mdev devices, would you please tell us the
> reason of your vfio-ccw? It could possibly be an architectural gap and
> the earlier we discuss it the better :)

Hi Jike,

Sorry for the late response.

I think you may mix up vfio-ccw with vfio-mccw, which is in the same
level with vfio-mpci in v6 (or vfio-mdev in v7). :>

The term of vfio-ccw is in the same semantic level with vfio-pci. You
can understand vfio-ccw as the parent device driver in the ccw case.

As you mentioned above, v7 provides an universal mdev driver (vfio-mdev)
for the mediated devices. So I don't need to provide a vfio-mccw (a
vendor specific mediated device driver) anymore, but I'd still need my
vfio-ccw (the parent device driver).

> 
> --
> Thanks,
> Jike
> 

...snip...

-- 
Dong Jia

Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class

[Nikunj A Dadhania]

Cédric Le Goater  writes:

> On 09/19/2016 08:29 AM, Nikunj A Dadhania wrote:
>> From: Benjamin Herrenschmidt 
>> 
>> The existing implementation remains same and ics-base is introduced. The
>> type name "ics" is retained, and all the related functions renamed as
>> ics_simple_*
>> 
>> This will allow different implementations for the source controllers
>> such as the MSI support of PHB3 on Power8 which uses in-memory state
>> tables for example.
>> 
>> Signed-off-by: Benjamin Herrenschmidt 
>> Signed-off-by: Nikunj A Dadhania 
>
> This patch had a Reviewed-by from David. Did you change anything 
> in v4 ? 

No changes, I missed adding reviewed-by in v4.

Regards
Nikunj

Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 3/9] ppc/xics: Make the ICSState a list

[Nikunj A Dadhania]

Cédric Le Goater  writes:

> On 09/19/2016 09:02 AM, Nikunj A Dadhania wrote:
>> Cédric Le Goater  writes:
>> 
>>>  
 +static int icp_post_load(ICPState *ss, int version_id)
 +{
 +sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 +XICSState *xics = spapr->xics;
 +uint32_t irq, i;
 +static uint32_t server_no;
 +
 +server_no++;
 +irq = XISR(ss);
 +if (!ss->cs || !irq) {
 +goto icpend;
 +}
 +
 +/* Restore the xirr_owner */
 +ss->xirr_owner = xics_find_source(xics, irq);
 +
 + icpend:
 +trace_xics_icp_post_load(server_no, ss->xirr, 
 (uint64_t)ss->xirr_owner,
 + ss->pending_priority);
 +if (xics->nr_servers != server_no) {
 +return 0;
 +}
 +
 +/* All the ICPs are processed, now restore all the state */
 +for (i = 0; i < xics->nr_servers; i++) {
 +icp_resend(xics, i);
 +}
 +server_no = 0;
 +return 0;
 +}
>>>
>>> Is the issue this change is trying to fix related to ICSState becoming 
>>> a list ? If not, It should be in another patch I think.
>> 
>> Yes, and we introduced xirr_owner to optimize. This needs to regenerated
>> at the destination after migration.
>
> Ah. this is because of the previous patch. ok. I am not sure 
> I understand the complete issue but it would better if it was 
> a bit more isolated. This patch is mixing different things and 
> doing in xics.c :
>
>   #include "hw/ppc/spapr.h"
>
> seems wrong.

Not sure, Why?

> I think David suggested to redesign the xics migration state.

That is not needed, as I have solved the problem in the previous patch.
The migration changes was needed for the issue that I had reported.

Regards
Nikunj

Re: [Qemu-devel] [PATCH v3 2/5] target-ppc: improve lxvw4x implementation

[David Gibson]

On Mon, Sep 19, 2016 at 04:06:40PM +0530, Nikunj A Dadhania wrote:
> David Gibson  writes:
> > [ Unknown signature status ]
> > On Mon, Sep 19, 2016 at 04:19:34PM +1000, David Gibson wrote:
> >> On Fri, Sep 16, 2016 at 04:21:48PM +0530, Nikunj A Dadhania wrote:
> >> > diff --git a/target-ppc/translate/vsx-impl.inc.c 
> >> > b/target-ppc/translate/vsx-impl.inc.c
> >> > index eee6052..df278df 100644
> >> > --- a/target-ppc/translate/vsx-impl.inc.c
> >> > +++ b/target-ppc/translate/vsx-impl.inc.c
> >> > @@ -75,7 +75,6 @@ static void gen_lxvdsx(DisasContext *ctx)
> >> >  static void gen_lxvw4x(DisasContext *ctx)
> >> >  {
> >> >  TCGv EA;
> >> > -TCGv_i64 tmp;
> >> >  TCGv_i64 xth = cpu_vsrh(xT(ctx->opcode));
> >> >  TCGv_i64 xtl = cpu_vsrl(xT(ctx->opcode));
> >> >  if (unlikely(!ctx->vsx_enabled)) {
> >> > @@ -84,22 +83,14 @@ static void gen_lxvw4x(DisasContext *ctx)
> >> >  }
> >> >  gen_set_access_type(ctx, ACCESS_INT);
> >> >  EA = tcg_temp_new();
> >> > -tmp = tcg_temp_new_i64();
> >> >  
> >> >  gen_addr_reg_index(ctx, EA);
> >> > -gen_qemu_ld32u_i64(ctx, tmp, EA);
> >> > -tcg_gen_addi_tl(EA, EA, 4);
> >> > -gen_qemu_ld32u_i64(ctx, xth, EA);
> >> > -tcg_gen_deposit_i64(xth, xth, tmp, 32, 32);
> >> > -
> >> > -tcg_gen_addi_tl(EA, EA, 4);
> >> > -gen_qemu_ld32u_i64(ctx, tmp, EA);
> >> > -tcg_gen_addi_tl(EA, EA, 4);
> >> > -gen_qemu_ld32u_i64(ctx, xtl, EA);
> >> > -tcg_gen_deposit_i64(xtl, xtl, tmp, 32, 32);
> >> > -
> >> > +tcg_gen_qemu_ld_i64(xth, EA, ctx->mem_idx, MO_LEQ);
> >> > +gen_helper_deposit32x2(xth, xth);
> >> > +tcg_gen_addi_tl(EA, EA, 8);
> >> > +tcg_gen_qemu_ld_i64(xtl, EA, ctx->mem_idx, MO_LEQ);
> >> > +gen_helper_deposit32x2(xtl, xtl);
> >
> > ..and I think this is wrong for BE mode.  The deposit32x2 will get the
> > words in the right order, but the bytes within each word will be wrong
> > because of the LE mode load on a BE setup.
> 
> Since lxvw4x/stxvw4x is available on POWER8. I tried running my test
> code on BE and LE Fedora24 VM. TCG Results match the POWER8 hardware.
> The order within the word is not changed. Snippet of the test code at
> the end of email. Can share full code if needed (maybe will do it in
> kvm-unit-test)

Ugh.. now I'm confused.  I would not have expected the results you've
seen from these tests.  But I still can't understand *how* the
emulation could be correct: IIUC MO_LEQ would mean it loads the 8
bytes as a single 64-bit LE integer.  Which should be the same as
loading one 32-bit LE integer into the low half of the target
register, then a 32-bit LE integer into the high half ot the target
register.

As I said above, the deposit32x2 will swap the order of the two ints,
but it won't byteswap the individual int32s which should have been BE
in memory.

Can you find the flaw in my reasoning?

> Fedora24VM BE:
> 
> [fedora@cloudimg ~]$ uname -a
> Linux cloudimg.localdomain 4.5.5-300.fc24.ppc64 #1 SMP Tue May 24 
> 12:24:54 UTC 2016 ppc64 ppc64 ppc64 GNU/Linux
> [fedora@cloudimg ~]$ ./lxv_x
> VRT32 = 00010203 20212223 30313233 40414243 
> 
> [fedora@cloudimg ~]$ ./stxv_x 
>  E0E1E2E3  E4E5E6E7  F0F1F2F3  F4F5F6F7 
> 
> 
> TCG Result BE:
> ==
> $ ./ppc64-linux-user/qemu-ppc64  -cpu POWER9 lxv_x
> VRT32 = 00010203 20212223 30313233 40414243 
> 
> $ ./ppc64-linux-user/qemu-ppc64  -cpu POWER9 stxv_x
>  E0E1E2E3  E4E5E6E7  F0F1F2F3  F4F5F6F7
> 
> 
> Fedora24VM LE:
> ==
> [fedora@cloudimg ~]$ uname -a
> Linux cloudimg.localdomain 4.5.5-300.fc24.ppc64le #1 SMP Tue May 24 
> 12:23:26 UTC 2016 ppc64le ppc64le ppc64le GNU/Linux
> [fedora@cloudimg ~]$ ./lxv_x 
> VRT32 = 40414243 30313233 20212223 00010203 
> 
> [fedora@cloudimg ~]$ ./stxv_x 
>  F4F5F6F7  F0F1F2F3  E4E5E6E7  E0E1E2E3 
> 
> TCG Result LE:
> ==
> $ ./ppc64le-linux-user/qemu-ppc64le  -cpu POWER9 lxv_x
> VRT32 = 40414243 30313233 20212223 00010203 
> 
> $ ./ppc64le-linux-user/qemu-ppc64le  -cpu POWER9 stxv_x
>  F4F5F6F7  F0F1F2F3  E4E5E6E7  E0E1E2E3 
> 
> Regards,
> Nikunj
> 
> 
> vsx.h:
> ==
> #define U32_SIZE (sizeof(__vector uint32_t) / sizeof(uint32_t))
> 
> typedef union {
> __vector uint32_t v;
> uint32_t a[U32_SIZE];
> } vuint32_t;

I am a little suspicious that whatever the compiler does to convert
the vector to an array via this union might be undoing a byte reverse.

I'd be more confident if you used VSX instructions to extract and
store separately one of the 32-bit subwords of the vector.

> 
> static void vec_put_u32(__vector uint32_t v) {
> int i;
> vuint32_t u;
> 
> for (u.v = v, i = 0; i < U32_SIZE; ++i) {
> printf("%08x ", u.a[i]);
> }
> 
> printf("\n");
> }
> 
> static void print4x4(uint32_t *p)
> {
> int i;
> if (!p)
> return;
> for(i = 0; i < 4; i++)
> printf(" %08X ", p[i]);

[Qemu-devel] [PATCH v2] add resolutions via command-line

[G 3]

Add the ability to add resolutions from the command-line. This patch  
works by
looking for a property called 'resolutions' in the options node of  
OpenBIOS.

If it is found all the resolutions are parsed and loaded.

Example command-line:

-prom-env resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900

Signed-off-by: John Arbuckle 
---
Implemented my own malloc(), strlen(), pow(), and atoi() functions.
Removed free() calls.
Changed get_set_count() to get_resolution_set_count().

 QemuVGADriver/src/QemuVga.c | 285 ++ 
+-

 1 file changed, 283 insertions(+), 2 deletions(-)

diff --git a/QemuVGADriver/src/QemuVga.c b/QemuVGADriver/src/QemuVga.c
index 4584242..cba65ba 100644
--- a/QemuVGADriver/src/QemuVga.c
+++ b/QemuVGADriver/src/QemuVga.c
@@ -18,7 +18,19 @@ static struct vMode vModes[] =  {
{ 1600, 1200 },
{ 1920, 1080 },
{ 1920, 1200 },
-   { 0,0 }
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
+   { 0,0 },
 };

 static void VgaWriteB(UInt16 port, UInt8 val)
@@ -148,11 +160,280 @@ static InterruptMemberNumber  
PCIInterruptHandler(InterruptSetMember ISTmember,

 #endif


+/* Returns the number of resolutions */
+static int get_number_of_resolutions()
+{
+   int size_of_array, num_of_resolutions, index;
+   
+   num_of_resolutions = 0;
+   size_of_array = sizeof(vModes) / sizeof(struct vMode);
+   
+   for(index = 0; index < size_of_array; index++)
+   {
+   if (vModes[index].width != 0) {
+   num_of_resolutions++;
+   }
+   }
+   
+   return num_of_resolutions;
+}
+
+
+/* strlen() implementation */
+static int strlen(const char *str)
+{
+   int count = 0;
+   
+   for( ; *str != '\0'; str++) {
+   count++;
+   }
+   
+   return count;
+}
+
+
+/* output = base^power */
+static int pow(int base, int power)
+{
+   int i, output;
+   output = 1;
+   for (i = 0; i < power; i++) {
+   output = output * base;
+   }
+   return output;
+}
+
+
+/* convert ascii string to number */
+static int atoi(const char *str)
+{
+   int result = 0, multiplier;
+
+   multiplier = strlen(str) - 1;
+   for ( ; *str != '\0'; str++) {
+   result += (*str - '0') * pow(10, multiplier);
+   multiplier--;
+   if (multiplier < 0)  /* Something might be wrong if returning 
here */
+   return result;
+   }
+
+   return result;
+}
+
+
+/* An interesting way of emulating memory allocation. */
+static char *malloc(int size)
+{
+   const int max_buffer_size = 2000;
+   static char buffer[max_buffer_size];
+   static int free_mem_pointer = 0;
+   static Boolean first_alloc = true;
+   
+   free_mem_pointer += size;
+   if (free_mem_pointer >= max_buffer_size) {
+   return NULL;
+   }
+   
+   /* For getting the very beginning of the buffer */
+   if (first_alloc == true) {
+   first_alloc = false;
+   return buffer;
+   }
+
+   return (buffer + free_mem_pointer);
+}
+
+
+/* Get the resolution set at the specified index. */
+static char *get_resolution_set(const char *resolution_set_str, int  
set_number)

+{
+   const int max_buf_size = 100;
+   char c, *buffer;
+   int index, comma_count;
+
+   buffer = (char *) malloc(max_buf_size);
+   comma_count = 0;
+   index = 0;
+   set_number++; /* Makes things easier to understand */
+
+   c = *(resolution_set_str++);
+   while (c != '\0') {
+   buffer[index++] = c;
+   c = *(resolution_set_str++);
+   if (c == ',') {
+   comma_count++;
+   if (comma_count == set_number || index >= max_buf_size) 
{
+   buffer[index] = '\0';
+   return buffer;
+   }
+   
+   else {
+   /* reset the buffer */
+   index = 0;
+   c = *(resolution_set_str++);
+   }
+   }
+   }
+   
+   buffer[index] = '\0';
+
+   return buffer;
+}
+
+
+/* Get the number of resolution sets - a set is a width by height  
pair */

+static int get_resolution_set_count(const char *resolution_sets_str)
+{
+   char c;
+   int count;
+   
+   /* Count the number of commas */
+   count = 0;
+   c = *(resolution_sets_str++);
+   while (c != '\0') {
+   if (c == ',') {
+   count++;
+   }
+   c = *(resolution_sets_str++);
+   }
+

Re: [Qemu-devel] [PATCH] linux-user: ppc64: fix ARCH_206 bit in AT_HWCAP

[David Gibson]

On Tue, Aug 16, 2016 at 03:40:50PM +0200, Michael Walle wrote:
> Only the POWER[789] CPUs should have the ARCH_206 bit set. This is what the
> linux kernel does. I guess this was also the intention of commit 0e019746.
> We have to make sure all *206 bits are set.

Hrm.. it's not clear to me how this patch fixes things.  What was
incorrect with the previous logic?

> 
> Signed-off-by: Michael Walle 
> ---
> checkpatch.pl flags one warning, but I think this is a false positive.

Yes, I think so to, but..

>  linux-user/elfload.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index f807baf..4945d48 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -742,7 +742,8 @@ static uint32_t get_elf_hwcap(void)
>  #define GET_FEATURE(flag, feature)  \
>  do { if (cpu->env.insns_flags & flag) { features |= feature; } } while 
> (0)
>  #define GET_FEATURE2(flag, feature)  \
> -do { if (cpu->env.insns_flags2 & flag) { features |= feature; } } while 
> (0)
> +do { if ((cpu->env.insns_flags2 & flag) == flag) \
> + { features |= feature; } } while (0)

..given that you're splitting this to >1 line, I think you might as
well expand it fully into a more normal indent style, which should also
shut up the stylebot.

>  GET_FEATURE(PPC_64B, QEMU_PPC_FEATURE_64);
>  GET_FEATURE(PPC_FLOAT, QEMU_PPC_FEATURE_HAS_FPU);
>  GET_FEATURE(PPC_ALTIVEC, QEMU_PPC_FEATURE_HAS_ALTIVEC);

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 19, 2016, at 6:44 PM, Benjamin Herrenschmidt wrote:


On Mon, 2016-09-19 at 08:44 -0400, G 3 wrote:

On Sep 19, 2016, at 2:24 AM, Benjamin Herrenschmidt wrote:



On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:


Add the ability to add resolutions from the command-line. This
patch
works by
looking for a property called 'resolutions' in the options node
of
OpenBIOS.
If it is found all the resolutions are parsed and loaded.

Example command-line:


You must not use the C library in the ndrv (malloc, atoi, ...)

Stick to what's in DriverServicesLib and NameRegistryLib.


I originally thought that, but was wrong. Those C library functions
work.


No, don't use them. They "seem" to work but bad thing will happen,
especially on OS X.

Cheers,
Ben.


Ok, they have been replaced in my next patch.

Something is wrong with the options node in OpenBIOS. It is  
inaccessible from Mac OS X. When trying to access the options node,  
IORegistryExplorer always crashes. The other nodes are accessible.  
I'm not certain what the problem could be.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 19, 2016, at 7:56 PM, Alfonso Gamboa wrote:


John,

http://mirror.informatimago.com/next/developer.apple.com/qa/dv/ 
dv43.html perhaps yields some insight:


Note:
An example of this is the video 'ndrv' support in Mac OS X. Mac OS  
X can load and run native video drivers from the ROM of PCI video  
cards. This allows Mac OS X to provide basic video functionality on  
any card that supports traditional Mac OS boot-time video, without  
having a real Mac OS X video driver available. In order for this to  
work your video driver must:


link with only the standard libraries (DriverServicesLib,  
NameRegistryLib, VideoServicesLib, and PCILib)

not access low-memory globals
recognize that it's possible for the logical and physical addresses  
of its PCI hardware to be different, and always access the  
appropriate addresses through the appropriate Name Registry  
properties.



Thank you very much for this.

Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices

[Jike Song]

On 09/20/2016 04:03 AM, Alex Williamson wrote:
> On Tue, 20 Sep 2016 00:43:15 +0530
> Kirti Wankhede  wrote:
> 
>> On 9/20/2016 12:06 AM, Alex Williamson wrote:
>>> On Mon, 19 Sep 2016 23:52:36 +0530
>>> Kirti Wankhede  wrote:
>>>   
 On 8/26/2016 7:43 PM, Kirti Wankhede wrote:  
> * PGP Signed: 08/26/2016 at 07:15:44 AM, Decrypted
> On 8/25/2016 2:52 PM, Dong Jia wrote:
>> On Thu, 25 Aug 2016 09:23:53 +0530
  
>>> +
>>> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
>>> + size_t count, loff_t *ppos)
>>> +{
>>> +   struct vfio_mdev *vmdev = device_data;
>>> +   struct mdev_device *mdev = vmdev->mdev;
>>> +   struct parent_device *parent = mdev->parent;
>>> +   unsigned int done = 0;
>>> +   int ret;
>>> +
>>> +   if (!parent->ops->read)
>>> +   return -EINVAL;
>>> +
>>> +   while (count) {
>> Here, I have to say sorry to you guys for that I didn't notice the
>> bad impact of this change to my patches during the v6 discussion.
>>
>> For vfio-ccw, I introduced an I/O region to input/output I/O
>> instruction parameters and results for Qemu. The @count of these data
>> currently is 140. So supporting arbitrary lengths in one shot here, and
>> also in vfio_mdev_write, seems the better option for this case.
>>
>> I believe that if the pci drivers want to iterate in a 4 bytes step, you
>> can do that in the parent read/write callbacks instead.
>>
>> What do you think?
>>
>
> I would like to know Alex's thought on this. He raised concern with this
> approach in v6 reviews:
> "But I think this is exploitable, it lets the user make the kernel
> allocate an arbitrarily sized buffer."
> 

 Read/write callbacks are for slow path, emulation of mmio region which
 are mainly device registers. I do feel it shouldn't support arbitrary
 lengths.
 Alex, I would like to know your thoughts.  
>>>
>>> The exploit was that the mdev layer allocated a buffer and copied the
>>> entire user buffer into kernel space before passing it to the vendor
>>> driver.  The solution is to pass the __user *buf to the vendor driver
>>> and let them sanitize and split it however makes sense for their
>>> device.  We shouldn't be assuming naturally aligned, up to dword
>>> accesses in the generic mdev layers.  Those sorts of semantics are
>>> defined by the device type.  This is another case where making
>>> the mdev layer as thin as possible is actually the best thing to
>>> do to make it really device type agnostic.  Thanks,
>>>   
>>
>>
>> Alex,
>>
>> These were the comments on v6 patch:
>>
> Do we really need to support arbitrary lengths in one shot?  Seems
> like
> we could just use a 4 or 8 byte variable on the stack and iterate
> until
> done.
>  

 We just want to pass the arguments to vendor driver as is here.Vendor
 driver could take care of that.  
>>
>>> But I think this is exploitable, it lets the user make the kernel
>>> allocate an arbitrarily sized buffer.  
>>
>> As per above discussion in v7 version, this module don't allocated
>> memory from heap.
>>
>> If vendor driver allocates arbitrary memory in kernel space through mdev
>> module interface, isn't that would be exploit?
> 
> Yep, my 4-8/byte chunking idea was too PCI specific.  If a vendor
> driver introduces an exploit, that's a bug in the vendor driver.  I'm
> not sure if we can or should attempt to guard against that.  Ultimately
> the vendor driver is either open source and we can inspect it for such
> exploits or it's closed source, taints the kernel, and we hope for the
> best.  It might make a good unit test to perform substantially sized
> reads/writes to the mdev device.

Can't agree more! :-)

> Perhaps the only sanity test we can
> make in the core is to verify the access doesn't exceed the size of
> the region as advertised by the vendor driver.  Thanks,

Even performing a lightweight sanity check, would require vfio-mdev
to be able to decode the ppos into a particular region, that means
information of all regions should be stored in the framework. I guess
it is not your preferred way :)

--
Thanks,
Jike

Re: [Qemu-devel] [V18 2/4] hw/i386/trace-events: Add AMD IOMMU trace events

[Michael S. Tsirkin]

On Tue, Sep 20, 2016 at 04:14:14AM +0300, David Kiarie wrote:
> 
> 
> On Tue, Sep 20, 2016 at 3:09 AM, Michael S. Tsirkin  wrote:
> 
> On Tue, Sep 20, 2016 at 03:05:02AM +0300, David Kiarie wrote:
> > Signed-off-by: David Kiarie 
> > ---
> >  hw/i386/trace-events | 29 +
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/hw/i386/trace-events b/hw/i386/trace-events
> > index 5b99eba..ddeda02 100644
> > --- a/hw/i386/trace-events
> > +++ b/hw/i386/trace-events
> > @@ -13,3 +13,32 @@ mhp_pc_dimm_assigned_address(uint64_t addr)
> "0x%"PRIx64
> >
> >  # hw/i386/x86-iommu.c
> >  x86_iommu_iec_notify(bool global, uint32_t index, uint32_t mask) 
> "Notify
> IEC invalidation: global=%d index=%" PRIu32 " mask=%" PRIu32
> > +
> > +# hw/i386/amd_iommu.c
> > +amdvi_evntlog_fail(uint64_t addr, uint32_t head) "error: fail to write
> at addr 0x%"PRIx64 " +  offset 0x%"PRIx32
> > +amdvi_cache_update(uint16_t domid, uint32_t bus, uint32_t slot, 
> uint32_t
> func, uint64_t gpa, uint64_t txaddr) " update iotlb domid 0x%"PRIx16"
> devid: %02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"PRIx64
> > +amdvi_completion_wait_fail(uint64_t addr) "error: fail to write at
> address 0x%"PRIx64
> > +amdvi_mmio_write(const char *reg, uint64_t addr, unsigned size, 
> uint64_t
> val, unsigned long offset) "%s write addr 0x%"PRIx64 ", size %d, val
> 0x%"PRIx64 ", offset 0x%"PRIx64
> 
> 
> unsigned long offset should be printed with %0lx not PRIx64.
> 
> There are a bunch of other bugs I fixed silenty in this file,
> I don't remember where exactly pls go over and and fix
> all you can find.
> 
> 
> 
> Separately resending this patch in the meantime just in case the rest of the
> patchset happens not to have any more issues.
>  
> 
> 
> > +amdvi_mmio_read(const char *reg, uint64_t addr, unsigned size, uint64_t
> offset) "%s read addr 0x%"PRIx64", size %d offset 0x%"PRIx64
> > +amdvi_command_error(uint64_t status) "error: Executing commands with
> command buffer disabled 0x%"PRIx64
> > +amdvi_command_read_fail(uint64_t addr, uint32_t head) "error: fail to
> access memory at 0x%"PRIx64" + 0x%"PRIu32
> > +amdvi_command_exec(uint32_t head, uint32_t tail, uint64_t buf) "command
> buffer head at 0x%"PRIx32 " command buffer tail at 0x%"PRIx32" command
> buffer base at 0x%" PRIx64
> > +amdvi_unhandled_command(uint8_t type) "unhandled command %d"
> > +amdvi_intr_inval(void) "Interrupt table invalidated"
> > +amdvi_iotlb_inval(void) "IOTLB pages invalidated"
> > +amdvi_prefetch_pages(void) "Pre-fetch of AMD-Vi pages requested"
> > +amdvi_pages_inval(uint16_t domid) "AMD-Vi pages for domain 0x%"PRIx16 "
> invalidated"
> > +amdvi_all_inval(void) "Invalidation of all AMD-Vi cache requested "
> > +amdvi_ppr_exec(void) "Execution of PPR queue requested "
> > +amdvi_devtab_inval(uint16_t bus, uint16_t slot, uint16_t func) "device
> table entry for devid: %02x:%02x.%x invalidated"
> > +amdvi_completion_wait(uint64_t addr, uint64_t data) "completion wait
> requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
> > +amdvi_control_status(uint64_t val) "MMIO_STATUS state 0x%"PRIx64
> > +amdvi_iotlb_reset(void) "IOTLB exceed size limit - reset "
> > +amdvi_completion_wait_exec(uint64_t addr, uint64_t data) "completion
> wait requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
> > +amdvi_dte_get_fail(uint64_t addr, uint32_t offset) "error: failed to
> access Device Entry devtab 0x%"PRIx64" offset 0x%"PRIx32
> > +amdvi_invalid_dte(uint64_t addr) "PTE entry at 0x%"PRIx64" is invalid "
> > +amdvi_get_pte_hwerror(uint64_t addr) "hardware error eccessing PTE at
> addr 0x%"PRIx64
> > +amdvi_mode_invalid(unsigned level, uint64_t addr)"error: translation
> level 0x%"PRIu8" translating addr 0x%"PRIx64

This is also wrong. Can you pls go over the traces?
Also, you can check Peter Maydell's rejects of the pulls that
included these patches.

> > +amdvi_page_fault(uint64_t addr) "error: page fault accessing guest
> physical address 0x%"PRIx64
> > +amdvi_iotlb_hit(uint16_t bus, uint16_t slot, uint16_t func, uint64_t
> addr, uint64_t txaddr) "hit iotlb devid %02x:%02x.%x gpa 0x%"PRIx64 " hpa
> 0x%"PRIx64
> > +amdvi_translation_result(uint16_t bus, uint16_t slot, uint16_t func,
> uint64_t addr, uint64_t txaddr) "devid: %02x:%02x.%x gpa 0x%"PRIx64 " hpa
> 0x%"PRIx64
> > --
> > 2.1.4
> 
> 

Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Tian, Kevin]

> From: Kirti Wankhede [mailto:kwankh...@nvidia.com]
> Sent: Tuesday, September 20, 2016 4:36 AM
> 
> 
> Hi libvirt experts,
> 
> Thanks for valuable input on v1 version of RFC.
> 
> Quick brief, VFIO based mediated device framework provides a way to
> virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
> and IBM's channel IO. This framework reuses VFIO APIs for all the
> functionalities for mediated devices which are currently being used for
> pass through devices. This framework introduces a set of new sysfs files
> for device creation and its life cycle management.
> 
> Here is the summary of discussion on v1:
> 1. Discover mediated device:
> As part of physical device initialization process, vendor driver will
> register their physical devices, which will be used to create virtual
> device (mediated device, aka mdev) to the mediated framework.
> 
> Vendor driver should specify mdev_supported_types in directory format.
> This format is class based, for example, display class directory format
> should be as below. We need to define such set for each class of devices
> which would be supported by mediated device framework.
> 
>  --- mdev_destroy
>  --- mdev_supported_types
>  |-- 11
>  |   |-- create
>  |   |-- name
>  |   |-- fb_length
>  |   |-- resolution
>  |   |-- heads
>  |   |-- max_instances
>  |   |-- params
>  |   |-- requires_group
>  |-- 12
>  |   |-- create
>  |   |-- name
>  |   |-- fb_length
>  |   |-- resolution
>  |   |-- heads
>  |   |-- max_instances
>  |   |-- params
>  |   |-- requires_group
>  |-- 13
>  |-- create
>  |-- name
>  |-- fb_length
>  |-- resolution
>  |-- heads
>  |-- max_instances
>  |-- params
>  |-- requires_group
> 
> 

Per previous discussion, I'd think below layout is more general and clearer:

  --- mdev_supported_types
  |-- ID (or name)
  |   |-- create
  |   |-- available_instances
  |   |-- requires_group
  |-- ID (or name)
  |   |-- create
  |   |-- available_instances
  |   |-- requires_group
 ...

Then under each mdev directory:
  |-- type (link to type directory)
  |-- destroy
  |-- online
  |-- reset
  |-- Destroy

Group association may be done under mdev directory too, per Alex's earlier
suggestion.

Optional vendor-agnostic attributes can be extended in the future, e.g.:
  |-- priority
  |-- weight

Vendor-specific or class-specific parameters (black string or explicit 
attributes) 
could be also extended per-mdev directory, if necessary to support.

Thanks
Kevin

Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Tian, Kevin]

> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Tuesday, September 20, 2016 5:36 AM
> 
> > In the above example directory '11' represents a type id of mdev device.
> > 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> > 'requires_group' would be Read-Only files that vendor would provide to
> > describe about that type.
> >
> > 'create':
> > Write-only file. Mandatory.
> > Accepts string to create mediated device.
> >
> > 'name':
> > Read-Only file. Mandatory.
> > Returns string, the name of that type id.
> >
> > 'fb_length':
> > Read-only file. Mandatory.
> > Returns {K,M,G}, size of framebuffer.
> 
> This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
> just one user of mediated devices.

Agree. at least it doesn't make sense for s390 passthrough usage.

> 
> >
> > 'max_instance':
> > Read-Only file. Mandatory.
> > Returns integer.  Returns maximum mdev device could be created
> > at the moment when this file is read. This count would be updated by
> > vendor driver. Before creating mdev device of this type, check if
> > max_instance is > 0.
> 
> Didn't we discuss this being being something like "available_instances"
> with a "devices" sub-directory linking current devices of that type?

Right. "available_instances" which is dynamically changed under two scenarios:

a) create a new mdev under one type decrements "available_instances" of this
type by 1;
b) create a new mdev under one type decrements "available_instances" under
other types by type-specific number, if vendor supports creating multiple types
simultaneously like in Intel solution;

> 
> > 'params'
> > Write-Only file. Optional.
> > String input. Libvirt would pass the string given in XML file to
> > this file and then create mdev device. Set empty string to clear params.
> > For example, set parameter 'frame_rate_limiter=0' to disable frame rate
> > limiter for performance benchmarking, then create device of type 11. The
> > device created would have that parameter set by vendor driver.
> 
> Might as well just call it "magic", there's absolutely no ability to
> introspect what parameters are allowed or even valid here.  Can all
> parameters be changed dynamically?  Do parameter changes apply to
> existing devices or only future devices?  This is a poorly specified
> interface.  I'd prefer this be done via module options on the vendor
> driver.

I guess it might be mdev specific for nvidia's case, then module option
is not suitable. Then this magic string should be placed under mdev
directory instead of here, otherwise there'll be race condition.

> 
> >
> > 2. Create/destroy mediated device
> >
> > With above example, vGPU device XML would look like:
> >
> >
> >  my-vgpu
> >  pci__86_00_0
> >  
> >
> >1
> >'frame_rate_limiter=0'
> >  
> >
> >
> > 'type id' is mandatory.
> 
> I was under the impression that the vendor supplied "name" was the one
> unique identifier.  We're certainly not going to create a registrar to
> hand out type ids to each vendor, so what makes type id unique?  I have
> a hard time believing that a given vendor can even allocate unique type
> ids for their own devices.  Unique type id across vendors is not
> practical.  So which attribute are we actually using to identify which
> type of mdev device we need and why would we ever specify additional
> attributes like fb_length?  Doesn't the vendor guarantee that "GRID
> M60-0B" has a fixed setup of those attributes?

I'm a bit confused about type ID and type name here. Just keeping one 
should be enough (ether a number or descriptive name), which is 
per-device scope, not necessarily to cross vendors. As long as the vendor
defines the ID or name unique cross its own generations, looks should be
sufficient (I guess cross-vendor migration is not an interest here)

> 
> > 'params' is optional field. User should set this field if extra
> > parameters need to be set for a particular vGPU device. Libvirt don't
> > need to parse these params. These are meant for vendor driver.
> 
> ie. magic black hole.  nope.

A curious question here, though it's not a mandatory requirement for
Intel's GPU virtualization solution now, but allow such extension might
be useful in the long term. Has libvirt ever supported similar black 
string in other usages? I can think it's a problem for public cloud usage,
where any configuration on the managed device should be controlled
by orchestration layer (e.g. openstack) automatically. It's not appropriate
to allow a cloud tenant specifying such parameter, so in that manner
any configuration parameter should be explicitly understood from upper
level stack to libvirt then vendor specific parameters would be hard to
handle. On the other hand, if we consider enterprise virtualization 
scenario which is more proprietary, administrator may add such vendor
specific parameters as a black string through libvirt, as long 

[Qemu-devel] [PATCH 2/4] hw/i386/trace-events: Add AMD IOMMU trace events

[David Kiarie]

Signed-off-by: David Kiarie 
---
 hw/i386/trace-events | 29 +
 1 file changed, 29 insertions(+)

diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 5b99eba..1938b98 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -13,3 +13,32 @@ mhp_pc_dimm_assigned_address(uint64_t addr) "0x%"PRIx64
 
 # hw/i386/x86-iommu.c
 x86_iommu_iec_notify(bool global, uint32_t index, uint32_t mask) "Notify IEC 
invalidation: global=%d index=%" PRIu32 " mask=%" PRIu32
+
+# hw/i386/amd_iommu.c
+amdvi_evntlog_fail(uint64_t addr, uint32_t head) "error: fail to write at addr 
0x%"PRIx64" +  offset 0x%"PRIx32
+amdvi_cache_update(uint16_t domid, uint8_t bus, uint8_t slot, uint8_t func, 
uint64_t gpa, uint64_t txaddr) " update iotlb domid 0x%"PRIx16" devid: 
%02x:%02x.%x gpa 0x%"PRIx64" hpa 0x%"PRIx64
+amdvi_completion_wait_fail(uint64_t addr) "error: fail to write at address 
0x%"PRIx64
+amdvi_mmio_write(const char *reg, uint64_t addr, unsigned size, uint64_t val, 
uint64_t offset) "%s write addr 0x%"PRIx64", size %u, val 0x%"PRIx64", offset 
0x%"PRIx64
+amdvi_mmio_read(const char *reg, uint64_t addr, unsigned size, uint64_t 
offset) "%s read addr 0x%"PRIx64", size %u offset 0x%"PRIx64
+amdvi_command_error(uint64_t status) "error: Executing commands with command 
buffer disabled 0x%"PRIx64
+amdvi_command_read_fail(uint64_t addr, uint32_t head) "error: fail to access 
memory at 0x%"PRIx64" + 0x%"PRIx32
+amdvi_command_exec(uint32_t head, uint32_t tail, uint64_t buf) "command buffer 
head at 0x%"PRIx32" command buffer tail at 0x%"PRIx32" command buffer base at 
0x%"PRIx64
+amdvi_unhandled_command(uint8_t type) "unhandled command 0x%"PRIx8
+amdvi_intr_inval(void) "Interrupt table invalidated"
+amdvi_iotlb_inval(void) "IOTLB pages invalidated"
+amdvi_prefetch_pages(void) "Pre-fetch of AMD-Vi pages requested"
+amdvi_pages_inval(uint16_t domid) "AMD-Vi pages for domain 0x%"PRIx16 " 
invalidated"
+amdvi_all_inval(void) "Invalidation of all AMD-Vi cache requested "
+amdvi_ppr_exec(void) "Execution of PPR queue requested "
+amdvi_devtab_inval(uint8_t bus, uint8_t slot, uint8_t func) "device table 
entry for devid: %02x:%02x.%x invalidated"
+amdvi_completion_wait(uint64_t addr, uint64_t data) "completion wait requested 
with store address 0x%"PRIx64" and store data 0x%"PRIx64
+amdvi_control_status(uint64_t val) "MMIO_STATUS state 0x%"PRIx64
+amdvi_iotlb_reset(void) "IOTLB exceed size limit - reset "
+amdvi_completion_wait_exec(uint64_t addr, uint64_t data) "completion wait 
requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
+amdvi_dte_get_fail(uint64_t addr, uint32_t offset) "error: failed to access 
Device Entry devtab 0x%"PRIx64" offset 0x%"PRIx32
+amdvi_invalid_dte(uint64_t addr) "PTE entry at 0x%"PRIx64" is invalid "
+amdvi_get_pte_hwerror(uint64_t addr) "hardware error eccessing PTE at addr 
0x%"PRIx64
+amdvi_mode_invalid(uint8_t level, uint64_t addr)"error: translation level 
0x%"PRIx8" translating addr 0x%"PRIx64
+amdvi_page_fault(uint64_t addr) "error: page fault accessing guest physical 
address 0x%"PRIx64
+amdvi_iotlb_hit(uint8_t bus, uint8_t slot, uint8_t func, uint64_t addr, 
uint64_t txaddr) "hit iotlb devid %02x:%02x.%x gpa 0x%"PRIx64" hpa 0x%"PRIx64
+amdvi_translation_result(uint8_t bus, uint8_t slot, uint8_t func, uint64_t 
addr, uint64_t txaddr) "devid: %02x:%02x.%x gpa 0x%"PRIx64" hpa 0x%"PRIx64
-- 
2.1.4

Re: [Qemu-devel] [V18 2/4] hw/i386/trace-events: Add AMD IOMMU trace events

[David Kiarie]

On Tue, Sep 20, 2016 at 3:09 AM, Michael S. Tsirkin  wrote:

> On Tue, Sep 20, 2016 at 03:05:02AM +0300, David Kiarie wrote:
> > Signed-off-by: David Kiarie 
> > ---
> >  hw/i386/trace-events | 29 +
> >  1 file changed, 29 insertions(+)
> >
> > diff --git a/hw/i386/trace-events b/hw/i386/trace-events
> > index 5b99eba..ddeda02 100644
> > --- a/hw/i386/trace-events
> > +++ b/hw/i386/trace-events
> > @@ -13,3 +13,32 @@ mhp_pc_dimm_assigned_address(uint64_t addr)
> "0x%"PRIx64
> >
> >  # hw/i386/x86-iommu.c
> >  x86_iommu_iec_notify(bool global, uint32_t index, uint32_t mask)
> "Notify IEC invalidation: global=%d index=%" PRIu32 " mask=%" PRIu32
> > +
> > +# hw/i386/amd_iommu.c
> > +amdvi_evntlog_fail(uint64_t addr, uint32_t head) "error: fail to write
> at addr 0x%"PRIx64 " +  offset 0x%"PRIx32
> > +amdvi_cache_update(uint16_t domid, uint32_t bus, uint32_t slot,
> uint32_t func, uint64_t gpa, uint64_t txaddr) " update iotlb domid
> 0x%"PRIx16" devid: %02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"PRIx64
> > +amdvi_completion_wait_fail(uint64_t addr) "error: fail to write at
> address 0x%"PRIx64
> > +amdvi_mmio_write(const char *reg, uint64_t addr, unsigned size,
> uint64_t val, unsigned long offset) "%s write addr 0x%"PRIx64 ", size %d,
> val 0x%"PRIx64 ", offset 0x%"PRIx64
>
>
> unsigned long offset should be printed with %0lx not PRIx64.
>
> There are a bunch of other bugs I fixed silenty in this file,
> I don't remember where exactly pls go over and and fix
> all you can find.
>
>
Separately resending this patch in the meantime just in case the rest of
the patchset happens not to have any more issues.


>
> > +amdvi_mmio_read(const char *reg, uint64_t addr, unsigned size, uint64_t
> offset) "%s read addr 0x%"PRIx64", size %d offset 0x%"PRIx64
> > +amdvi_command_error(uint64_t status) "error: Executing commands with
> command buffer disabled 0x%"PRIx64
> > +amdvi_command_read_fail(uint64_t addr, uint32_t head) "error: fail to
> access memory at 0x%"PRIx64" + 0x%"PRIu32
> > +amdvi_command_exec(uint32_t head, uint32_t tail, uint64_t buf) "command
> buffer head at 0x%"PRIx32 " command buffer tail at 0x%"PRIx32" command
> buffer base at 0x%" PRIx64
> > +amdvi_unhandled_command(uint8_t type) "unhandled command %d"
> > +amdvi_intr_inval(void) "Interrupt table invalidated"
> > +amdvi_iotlb_inval(void) "IOTLB pages invalidated"
> > +amdvi_prefetch_pages(void) "Pre-fetch of AMD-Vi pages requested"
> > +amdvi_pages_inval(uint16_t domid) "AMD-Vi pages for domain 0x%"PRIx16 "
> invalidated"
> > +amdvi_all_inval(void) "Invalidation of all AMD-Vi cache requested "
> > +amdvi_ppr_exec(void) "Execution of PPR queue requested "
> > +amdvi_devtab_inval(uint16_t bus, uint16_t slot, uint16_t func) "device
> table entry for devid: %02x:%02x.%x invalidated"
> > +amdvi_completion_wait(uint64_t addr, uint64_t data) "completion wait
> requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
> > +amdvi_control_status(uint64_t val) "MMIO_STATUS state 0x%"PRIx64
> > +amdvi_iotlb_reset(void) "IOTLB exceed size limit - reset "
> > +amdvi_completion_wait_exec(uint64_t addr, uint64_t data) "completion
> wait requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
> > +amdvi_dte_get_fail(uint64_t addr, uint32_t offset) "error: failed to
> access Device Entry devtab 0x%"PRIx64" offset 0x%"PRIx32
> > +amdvi_invalid_dte(uint64_t addr) "PTE entry at 0x%"PRIx64" is invalid "
> > +amdvi_get_pte_hwerror(uint64_t addr) "hardware error eccessing PTE at
> addr 0x%"PRIx64
> > +amdvi_mode_invalid(unsigned level, uint64_t addr)"error: translation
> level 0x%"PRIu8" translating addr 0x%"PRIx64
> > +amdvi_page_fault(uint64_t addr) "error: page fault accessing guest
> physical address 0x%"PRIx64
> > +amdvi_iotlb_hit(uint16_t bus, uint16_t slot, uint16_t func, uint64_t
> addr, uint64_t txaddr) "hit iotlb devid %02x:%02x.%x gpa 0x%"PRIx64 " hpa
> 0x%"PRIx64
> > +amdvi_translation_result(uint16_t bus, uint16_t slot, uint16_t func,
> uint64_t addr, uint64_t txaddr) "devid: %02x:%02x.%x gpa 0x%"PRIx64 " hpa
> 0x%"PRIx64
> > --
> > 2.1.4
>

Re: [Qemu-devel] [PATCH] hmp: Improve 'info mtree' with optional parm for mapinfo

[Laszlo Ersek]

On 09/20/16 02:16, Thorsten Kohfeldt wrote:
> 
> Am 15.09.2016 um 11:52 schrieb Paolo Bonzini:
>>
>> On 07/09/2016 02:48, Thorsten Kohfeldt wrote:
>>> From: Thorsten Kohfeldt 
>>> Date: Wed, 31 Aug 2016 22:43:14 +0200
>>> Subject: [PATCH] hmp: Improve 'info mtree' with optional parm for
>>> mapinfo
>>>
>>> Motivation
>>> When 'tuning' 'quirks' for VFIO imported devices, it is not easy to
>>> directly grasp the implications of the priorisation algorithms in place
>>> for the 'layered mapping' of memory regions.
>>> Even though there are rules (documented in docs/memory.txt), once in a
>>> while one might question the correctness of the actual implementation of
>>> the rules.
>>> Particularly, I believe I have uncovered a divergence of (sub-)region
>>> priorisation/order/visibility in a corner case of importing a device
>>> (which requires a 'quirk') with mmap enabled vs. mmap disabled.
>>> This modification provides a means of visualising the ACTUAL
>>> mapping/visibility/occlusion of subregions within regions, whereas the
>>> current info mtree command only lists the tree of regions (all, visible
>>> and invisible ones).
>>> It is primarily intended to provide support for easy presentation of my
>>> cause, but I strongly believe this modification also has general purpose
>>> advantages.
>>
>> It is a useful addition, but I think a simpler presentation is also
>> fine.  What about "info mtree -f" which would visit the FlatView instead
>> of the tree?  The patch would probably be much shorter.
>>
>> Thanks,
>>
>> Paolo
> 
> Paolo,
> 
> For quite some time I had the patch in use as a direct modification of
> 'info mtree', but I felt that a general purpose use must provide an ad
> hoc user selectable presentation width parameter for the map info.
> I personally use a width of 65 or even 129 characters PREFIXING the
> tree elements which the command currently responds.
> My guess is though that most users would want a width of only 9 or 17.
> So I believe that a numerical parameter is a must.
> Visit the flat view -
> I'm not sure I understand you. Do you suggest to traverse a completely
> different data structure ?
> The purpose of the suggested visualisation is to point out where
> each of the tree's memory regions are "pinched" by other regions, so
> their "native" contents is NOT visible any more throughout the full
> region length, but (fractionally) rather another regions's content.
> Thus, I personally require to traverse exactly that tree structure.
> 
> No offence, but I would rather not want to modify the patch towards
> what I feel would be a completely different purpose.
> 
> I would appreciate if someone would review the patch in its current
> functional form to get it queued for qemu 2.8.
> My intention is to be able to rely on communication partners being
> able to reproduce findings using the new command once I start to
> attack the VFIO mmap flaw I talk about in the commit comment.
> 
> 
> @ALL
> I have provided 2 patch branches in github,
> one for qemu-2.7.0 and
> one for qemu-current-master (this needed a tiny sed-conversion, see below).
> I also placed some example info mtree mapinfo output on gist.github:
> 
> 
> # patch commit for qemu-2.7 (same patch also works for qemu-2.6):
> https://github.com/Thorsten-Kohfeldt/qemu/commit/5633a3cdbf6fd7cccd098fb83f591fbb15e8d383
> 
> # in branch:
> https://github.com/Thorsten-Kohfeldt/qemu/commits/monitor_--hmp_info_mtree_mapinfo-width
> 
> 
> # PATCH (as published in mailing list) *CONVERSION* from
> qemu-2.6/qemu-2.7 to qemu-master:
> sed s/'[.]mhandler[.]cmd = '/'.cmd= '/ >qemu-master.patch
> 
> # patch commit adapted that way for qemu-master:
> https://github.com/Thorsten-Kohfeldt/qemu/commit/60b8c1be1a0119df1f1859b0d484e06e709c2ea2
> 
> # in branch:
> https://github.com/Thorsten-Kohfeldt/qemu/commits/upstream-pullrequest-mapinfo-V1
> 
> 
> 
> # sample output info mtree 9
> https://gist.github.com/Thorsten-Kohfeldt/254b5a21fef497054959f58af53a44c9
> 
> # sample output info mtree 65
> https://gist.github.com/Thorsten-Kohfeldt/39cf5c8521c1999518b3438315e439f4

I think your current output explains, for each part (that is, each "sample" of 
user-selected size) of each MemoryRegion, whether that sample is actually 
visible in the finally rendered, flat view of the address space(s). (And, if 
not, why not.)

Whereas I think Paolo's idea is to map the flat view to begin with, and 
visually associate each interval of the guest visible physical address space 
with the one region that is ultimately accessed in that interval. This too 
would explain what the guest sees where, and why. It wouldn't give much 
information about ranges that the guest can *not* access (due to occlusion by 
other regions or otherwise), but maybe the "why not" is not so important after 
all?

Regarding the FlatView thing -- QEMU already maintains a flat rendering of the 
memory region tree, so that guest accesses to the various regions can 

Re: [Qemu-devel] [PATCH] hmp: Improve 'info mtree' with optional parm for mapinfo

[Thorsten Kohfeldt]

Am 15.09.2016 um 11:52 schrieb Paolo Bonzini:


On 07/09/2016 02:48, Thorsten Kohfeldt wrote:

From: Thorsten Kohfeldt 
Date: Wed, 31 Aug 2016 22:43:14 +0200
Subject: [PATCH] hmp: Improve 'info mtree' with optional parm for mapinfo

Motivation
When 'tuning' 'quirks' for VFIO imported devices, it is not easy to
directly grasp the implications of the priorisation algorithms in place
for the 'layered mapping' of memory regions.
Even though there are rules (documented in docs/memory.txt), once in a
while one might question the correctness of the actual implementation of
the rules.
Particularly, I believe I have uncovered a divergence of (sub-)region
priorisation/order/visibility in a corner case of importing a device
(which requires a 'quirk') with mmap enabled vs. mmap disabled.
This modification provides a means of visualising the ACTUAL
mapping/visibility/occlusion of subregions within regions, whereas the
current info mtree command only lists the tree of regions (all, visible
and invisible ones).
It is primarily intended to provide support for easy presentation of my
cause, but I strongly believe this modification also has general purpose
advantages.


It is a useful addition, but I think a simpler presentation is also
fine.  What about "info mtree -f" which would visit the FlatView instead
of the tree?  The patch would probably be much shorter.

Thanks,

Paolo


Paolo,

For quite some time I had the patch in use as a direct modification of
'info mtree', but I felt that a general purpose use must provide an ad
hoc user selectable presentation width parameter for the map info.
I personally use a width of 65 or even 129 characters PREFIXING the
tree elements which the command currently responds.
My guess is though that most users would want a width of only 9 or 17.
So I believe that a numerical parameter is a must.
Visit the flat view -
I'm not sure I understand you. Do you suggest to traverse a completely
different data structure ?
The purpose of the suggested visualisation is to point out where
each of the tree's memory regions are "pinched" by other regions, so
their "native" contents is NOT visible any more throughout the full
region length, but (fractionally) rather another regions's content.
Thus, I personally require to traverse exactly that tree structure.

No offence, but I would rather not want to modify the patch towards
what I feel would be a completely different purpose.

I would appreciate if someone would review the patch in its current
functional form to get it queued for qemu 2.8.
My intention is to be able to rely on communication partners being
able to reproduce findings using the new command once I start to
attack the VFIO mmap flaw I talk about in the commit comment.


@ALL
I have provided 2 patch branches in github,
one for qemu-2.7.0 and
one for qemu-current-master (this needed a tiny sed-conversion, see below).
I also placed some example info mtree mapinfo output on gist.github:


# patch commit for qemu-2.7 (same patch also works for qemu-2.6):
https://github.com/Thorsten-Kohfeldt/qemu/commit/5633a3cdbf6fd7cccd098fb83f591fbb15e8d383
# in branch:
https://github.com/Thorsten-Kohfeldt/qemu/commits/monitor_--hmp_info_mtree_mapinfo-width

# PATCH (as published in mailing list) *CONVERSION* from qemu-2.6/qemu-2.7 to 
qemu-master:
sed s/'[.]mhandler[.]cmd = '/'.cmd= '/ qemu-master.patch

# patch commit adapted that way for qemu-master:
https://github.com/Thorsten-Kohfeldt/qemu/commit/60b8c1be1a0119df1f1859b0d484e06e709c2ea2
# in branch:
https://github.com/Thorsten-Kohfeldt/qemu/commits/upstream-pullrequest-mapinfo-V1


# sample output info mtree 9
https://gist.github.com/Thorsten-Kohfeldt/254b5a21fef497054959f58af53a44c9

# sample output info mtree 65
https://gist.github.com/Thorsten-Kohfeldt/39cf5c8521c1999518b3438315e439f4


Regards,

Thorsten

Re: [Qemu-devel] [V18 2/4] hw/i386/trace-events: Add AMD IOMMU trace events

[Michael S. Tsirkin]

On Tue, Sep 20, 2016 at 03:05:02AM +0300, David Kiarie wrote:
> Signed-off-by: David Kiarie 
> ---
>  hw/i386/trace-events | 29 +
>  1 file changed, 29 insertions(+)
> 
> diff --git a/hw/i386/trace-events b/hw/i386/trace-events
> index 5b99eba..ddeda02 100644
> --- a/hw/i386/trace-events
> +++ b/hw/i386/trace-events
> @@ -13,3 +13,32 @@ mhp_pc_dimm_assigned_address(uint64_t addr) "0x%"PRIx64
>  
>  # hw/i386/x86-iommu.c
>  x86_iommu_iec_notify(bool global, uint32_t index, uint32_t mask) "Notify IEC 
> invalidation: global=%d index=%" PRIu32 " mask=%" PRIu32
> +
> +# hw/i386/amd_iommu.c
> +amdvi_evntlog_fail(uint64_t addr, uint32_t head) "error: fail to write at 
> addr 0x%"PRIx64 " +  offset 0x%"PRIx32
> +amdvi_cache_update(uint16_t domid, uint32_t bus, uint32_t slot, uint32_t 
> func, uint64_t gpa, uint64_t txaddr) " update iotlb domid 0x%"PRIx16" devid: 
> %02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"PRIx64
> +amdvi_completion_wait_fail(uint64_t addr) "error: fail to write at address 
> 0x%"PRIx64
> +amdvi_mmio_write(const char *reg, uint64_t addr, unsigned size, uint64_t 
> val, unsigned long offset) "%s write addr 0x%"PRIx64 ", size %d, val 
> 0x%"PRIx64 ", offset 0x%"PRIx64


unsigned long offset should be printed with %0lx not PRIx64.

There are a bunch of other bugs I fixed silenty in this file,
I don't remember where exactly pls go over and and fix
all you can find.


> +amdvi_mmio_read(const char *reg, uint64_t addr, unsigned size, uint64_t 
> offset) "%s read addr 0x%"PRIx64", size %d offset 0x%"PRIx64
> +amdvi_command_error(uint64_t status) "error: Executing commands with command 
> buffer disabled 0x%"PRIx64
> +amdvi_command_read_fail(uint64_t addr, uint32_t head) "error: fail to access 
> memory at 0x%"PRIx64" + 0x%"PRIu32
> +amdvi_command_exec(uint32_t head, uint32_t tail, uint64_t buf) "command 
> buffer head at 0x%"PRIx32 " command buffer tail at 0x%"PRIx32" command buffer 
> base at 0x%" PRIx64
> +amdvi_unhandled_command(uint8_t type) "unhandled command %d"
> +amdvi_intr_inval(void) "Interrupt table invalidated"
> +amdvi_iotlb_inval(void) "IOTLB pages invalidated"
> +amdvi_prefetch_pages(void) "Pre-fetch of AMD-Vi pages requested"
> +amdvi_pages_inval(uint16_t domid) "AMD-Vi pages for domain 0x%"PRIx16 " 
> invalidated"
> +amdvi_all_inval(void) "Invalidation of all AMD-Vi cache requested "
> +amdvi_ppr_exec(void) "Execution of PPR queue requested "
> +amdvi_devtab_inval(uint16_t bus, uint16_t slot, uint16_t func) "device table 
> entry for devid: %02x:%02x.%x invalidated"
> +amdvi_completion_wait(uint64_t addr, uint64_t data) "completion wait 
> requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
> +amdvi_control_status(uint64_t val) "MMIO_STATUS state 0x%"PRIx64
> +amdvi_iotlb_reset(void) "IOTLB exceed size limit - reset "
> +amdvi_completion_wait_exec(uint64_t addr, uint64_t data) "completion wait 
> requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
> +amdvi_dte_get_fail(uint64_t addr, uint32_t offset) "error: failed to access 
> Device Entry devtab 0x%"PRIx64" offset 0x%"PRIx32
> +amdvi_invalid_dte(uint64_t addr) "PTE entry at 0x%"PRIx64" is invalid "
> +amdvi_get_pte_hwerror(uint64_t addr) "hardware error eccessing PTE at addr 
> 0x%"PRIx64
> +amdvi_mode_invalid(unsigned level, uint64_t addr)"error: translation level 
> 0x%"PRIu8" translating addr 0x%"PRIx64
> +amdvi_page_fault(uint64_t addr) "error: page fault accessing guest physical 
> address 0x%"PRIx64
> +amdvi_iotlb_hit(uint16_t bus, uint16_t slot, uint16_t func, uint64_t addr, 
> uint64_t txaddr) "hit iotlb devid %02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"PRIx64
> +amdvi_translation_result(uint16_t bus, uint16_t slot, uint16_t func, 
> uint64_t addr, uint64_t txaddr) "devid: %02x:%02x.%x gpa 0x%"PRIx64 " hpa 
> 0x%"PRIx64
> -- 
> 2.1.4

[Qemu-devel] [V18 3/4] hw/i386: Introduce AMD IOMMU

[David Kiarie]

Add AMD IOMMU emulaton to Qemu in addition to Intel IOMMU.
The IOMMU does basic translation, error checking and has a
minimal IOTLB implementation. This IOMMU bypassed the need
for target aborts by responding with IOMMU_NONE access rights
and exempts the region 0xfee0-0xfeef from translation
as it is the q35 interrupt region.

We advertise features that are not yet implemented to please
the Linux IOMMU driver.

IOTLB aims at implementing commands on real IOMMUs which is
essential for debugging and may not offer any performance
benefits

Signed-off-by: David Kiarie 
---
 hw/i386/Makefile.objs |1 +
 hw/i386/amd_iommu.c   | 1199 +
 hw/i386/amd_iommu.h   |  289 
 3 files changed, 1489 insertions(+)
 create mode 100644 hw/i386/amd_iommu.c
 create mode 100644 hw/i386/amd_iommu.h

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 90e94ff..909ead6 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -3,6 +3,7 @@ obj-y += multiboot.o
 obj-y += pc.o pc_piix.o pc_q35.o
 obj-y += pc_sysfw.o
 obj-y += x86-iommu.o intel_iommu.o
+obj-y += amd_iommu.o
 obj-$(CONFIG_XEN) += ../xenpv/ xen/
 
 obj-y += kvmvapic.o
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
new file mode 100644
index 000..6f9556e
--- /dev/null
+++ b/hw/i386/amd_iommu.c
@@ -0,0 +1,1199 @@
+/*
+ * QEMU emulation of AMD IOMMU (AMD-Vi)
+ *
+ * Copyright (C) 2011 Eduard - Gabriel Munteanu
+ * Copyright (C) 2015 David Kiarie, 
+ *
+ * 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 .
+ *
+ * Cache implementation inspired by hw/i386/intel_iommu.c
+ */
+#include "qemu/osdep.h"
+#include "hw/i386/amd_iommu.h"
+#include "trace.h"
+
+/* used AMD-Vi MMIO registers */
+const char *amdvi_mmio_low[] = {
+"AMDVI_MMIO_DEVTAB_BASE",
+"AMDVI_MMIO_CMDBUF_BASE",
+"AMDVI_MMIO_EVTLOG_BASE",
+"AMDVI_MMIO_CONTROL",
+"AMDVI_MMIO_EXCL_BASE",
+"AMDVI_MMIO_EXCL_LIMIT",
+"AMDVI_MMIO_EXT_FEATURES",
+"AMDVI_MMIO_PPR_BASE",
+"UNHANDLED"
+};
+const char *amdvi_mmio_high[] = {
+"AMDVI_MMIO_COMMAND_HEAD",
+"AMDVI_MMIO_COMMAND_TAIL",
+"AMDVI_MMIO_EVTLOG_HEAD",
+"AMDVI_MMIO_EVTLOG_TAIL",
+"AMDVI_MMIO_STATUS",
+"AMDVI_MMIO_PPR_HEAD",
+"AMDVI_MMIO_PPR_TAIL",
+"UNHANDLED"
+};
+typedef struct AMDVIAddressSpace {
+uint8_t bus_num;/* bus number   */
+uint8_t devfn;  /* device function  */
+AMDVIState *iommu_state;/* AMDVI - one per machine  */
+MemoryRegion iommu; /* Device's address translation region  */
+MemoryRegion iommu_ir;  /* Device's interrupt remapping region  */
+AddressSpace as;/* device's corresponding address space */
+} AMDVIAddressSpace;
+
+/* AMDVI cache entry */
+typedef struct AMDVIIOTLBEntry {
+uint16_t domid; /* assigned domain id  */
+uint16_t devid; /* device owning entry */
+uint64_t perms; /* access permissions  */
+uint64_t translated_addr;   /* translated address  */
+uint64_t page_mask; /* physical page size  */
+} AMDVIIOTLBEntry;
+
+/* configure MMIO registers at startup/reset */
+static void amdvi_set_quad(AMDVIState *s, hwaddr addr, uint64_t val,
+   uint64_t romask, uint64_t w1cmask)
+{
+stq_le_p(>mmior[addr], val);
+stq_le_p(>romask[addr], romask);
+stq_le_p(>w1cmask[addr], w1cmask);
+}
+
+static uint16_t amdvi_readw(AMDVIState *s, hwaddr addr)
+{
+return lduw_le_p(>mmior[addr]);
+}
+
+static uint32_t amdvi_readl(AMDVIState *s, hwaddr addr)
+{
+return ldl_le_p(>mmior[addr]);
+}
+
+static uint64_t amdvi_readq(AMDVIState *s, hwaddr addr)
+{
+return ldq_le_p(>mmior[addr]);
+}
+
+/* internal write */
+static void amdvi_writeq_raw(AMDVIState *s, uint64_t val, hwaddr addr)
+{
+stq_le_p(>mmior[addr], val);
+}
+
+/* external write */
+static void amdvi_writew(AMDVIState *s, hwaddr addr, uint16_t val)
+{
+uint16_t romask = lduw_le_p(>romask[addr]);
+uint16_t w1cmask = lduw_le_p(>w1cmask[addr]);
+uint16_t oldval = lduw_le_p(>mmior[addr]);
+stw_le_p(>mmior[addr],
+((oldval & romask) | (val & ~romask)) & ~(val & w1cmask));
+}
+
+static void amdvi_writel(AMDVIState *s, hwaddr addr, uint32_t val)

[Qemu-devel] [V18 2/4] hw/i386/trace-events: Add AMD IOMMU trace events

[David Kiarie]

Signed-off-by: David Kiarie 
---
 hw/i386/trace-events | 29 +
 1 file changed, 29 insertions(+)

diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 5b99eba..ddeda02 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -13,3 +13,32 @@ mhp_pc_dimm_assigned_address(uint64_t addr) "0x%"PRIx64
 
 # hw/i386/x86-iommu.c
 x86_iommu_iec_notify(bool global, uint32_t index, uint32_t mask) "Notify IEC 
invalidation: global=%d index=%" PRIu32 " mask=%" PRIu32
+
+# hw/i386/amd_iommu.c
+amdvi_evntlog_fail(uint64_t addr, uint32_t head) "error: fail to write at addr 
0x%"PRIx64 " +  offset 0x%"PRIx32
+amdvi_cache_update(uint16_t domid, uint32_t bus, uint32_t slot, uint32_t func, 
uint64_t gpa, uint64_t txaddr) " update iotlb domid 0x%"PRIx16" devid: 
%02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"PRIx64
+amdvi_completion_wait_fail(uint64_t addr) "error: fail to write at address 
0x%"PRIx64
+amdvi_mmio_write(const char *reg, uint64_t addr, unsigned size, uint64_t val, 
unsigned long offset) "%s write addr 0x%"PRIx64 ", size %d, val 0x%"PRIx64 ", 
offset 0x%"PRIx64
+amdvi_mmio_read(const char *reg, uint64_t addr, unsigned size, uint64_t 
offset) "%s read addr 0x%"PRIx64", size %d offset 0x%"PRIx64
+amdvi_command_error(uint64_t status) "error: Executing commands with command 
buffer disabled 0x%"PRIx64
+amdvi_command_read_fail(uint64_t addr, uint32_t head) "error: fail to access 
memory at 0x%"PRIx64" + 0x%"PRIu32
+amdvi_command_exec(uint32_t head, uint32_t tail, uint64_t buf) "command buffer 
head at 0x%"PRIx32 " command buffer tail at 0x%"PRIx32" command buffer base at 
0x%" PRIx64
+amdvi_unhandled_command(uint8_t type) "unhandled command %d"
+amdvi_intr_inval(void) "Interrupt table invalidated"
+amdvi_iotlb_inval(void) "IOTLB pages invalidated"
+amdvi_prefetch_pages(void) "Pre-fetch of AMD-Vi pages requested"
+amdvi_pages_inval(uint16_t domid) "AMD-Vi pages for domain 0x%"PRIx16 " 
invalidated"
+amdvi_all_inval(void) "Invalidation of all AMD-Vi cache requested "
+amdvi_ppr_exec(void) "Execution of PPR queue requested "
+amdvi_devtab_inval(uint16_t bus, uint16_t slot, uint16_t func) "device table 
entry for devid: %02x:%02x.%x invalidated"
+amdvi_completion_wait(uint64_t addr, uint64_t data) "completion wait requested 
with store address 0x%"PRIx64" and store data 0x%"PRIx64
+amdvi_control_status(uint64_t val) "MMIO_STATUS state 0x%"PRIx64
+amdvi_iotlb_reset(void) "IOTLB exceed size limit - reset "
+amdvi_completion_wait_exec(uint64_t addr, uint64_t data) "completion wait 
requested with store address 0x%"PRIx64" and store data 0x%"PRIx64
+amdvi_dte_get_fail(uint64_t addr, uint32_t offset) "error: failed to access 
Device Entry devtab 0x%"PRIx64" offset 0x%"PRIx32
+amdvi_invalid_dte(uint64_t addr) "PTE entry at 0x%"PRIx64" is invalid "
+amdvi_get_pte_hwerror(uint64_t addr) "hardware error eccessing PTE at addr 
0x%"PRIx64
+amdvi_mode_invalid(unsigned level, uint64_t addr)"error: translation level 
0x%"PRIu8" translating addr 0x%"PRIx64
+amdvi_page_fault(uint64_t addr) "error: page fault accessing guest physical 
address 0x%"PRIx64
+amdvi_iotlb_hit(uint16_t bus, uint16_t slot, uint16_t func, uint64_t addr, 
uint64_t txaddr) "hit iotlb devid %02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"PRIx64
+amdvi_translation_result(uint16_t bus, uint16_t slot, uint16_t func, uint64_t 
addr, uint64_t txaddr) "devid: %02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"PRIx64
-- 
2.1.4

[Qemu-devel] [V18 0/4] AMD IOMMU

[David Kiarie]

Hi all,

This patchset adds basic AMD IOMMU emulation support to Qemu. 

Changes since v17
   -removed host dependent defines in bitfields and replaced that with 
'extract64/extract32' [Peter, Michael]

Changes since v16 - this is mainly supposed to come as a ping :-)
   -minor endian-ness fixes

Changes since v15
   -Endian-ness issue fix
   -cleaned up unused macros
   -removed guest frame number(gfn) from cache entry

Changes since v14
   -MMIO register reading/write bug fix [Peter]
   -Endian-ness issue fix[Peter]
   -Bitfields layouts in IOMMU commands fix[Peter]
   -IVRS changed IVHD device entry from type 3 to 1 to save a few bytes
   -coding style issues, comment grammer and other miscellaneous fixes.

Changes since v13
   -Added an error to make AMD IOMMU incompatible with device assignment.[Alex]
   -Converted AMD IOMMU into a composite PCI and System Bus device. This helps 
with:
  -We can now inherit from X86 IOMMU base class(which is implemented as a 
System Bus device).
  -We can now reserve MMIO region for IOMMU without a BAR register and 
without a hack.

Changes since v12

   -Coding style fixes [Jan, Michael]
   -Error logging fix to avoid using a macro[Jan]
   -moved some PCI macros to PCI header[Jan]
   -Use a lookup table for MMIO register names when tracing[Jan]

Changes since V11
   -AMD IOMMU is not started with -device amd-iommu (with a dependency on 
Marcel's patches).
   -IOMMU commands are represented using bitfields which is less error prone 
and more readable[Peter]
   -Changed from debug fprintfs to tracing[Jan]

Changes since V10
 
   -Support for huge pages including some obscure AMD IOMMU feature that allows 
default page size override[Jan].
   -Fixed an issue with generation of interrupts. We noted that AMD IOMMU has 
BusMaster- and is therefore not able to generate interrupts like any other PCI 
device. 
We have resulted in writing directly to system address but this could be 
fixed by some patches which have not been merged yet.

Changes since v9

   -amd_iommu prefixes have been renamed to a shorter 'amdvi' both in the macros
and in the functions/code. The register macros have not been moved to the 
implementation file since almost the macros there are basically macros and 
I 
reckoned renaming them should suffice.
   -taken care of byte order in the use of 'dma_memory_read'[Michael]
   -Taken care of invalid DTE entries to ensure no DMA unless a device is 
configured to allow it.
   -An issue with the emulate IOMMU defaulting to AMD_IOMMU has been 
fixed[Marcel]
   
You can test[1] this patches by starting with parameters 
qemu-system-x86_64 -M -device amd-iommu -m 2G -enable-kvm -smp 4 -cpu host 
-hda file.img -soundhw ac97 
emulating whatever devices you want.

Not passing any command line parameters to linux should be enough to test this 
patches since the devices are basically
passes-through but to the 'host' (l1 guest). You can still go ahead pass 
command line parameter 'iommu=pt iommu=1'
and try to pass a device to L2 guest. This can also done without passing any 
iommu related parameters to the kernel. 

[1] https://github.com/aslaq/qemu v18

David Kiarie (4):
  hw/pci: Prepare for AMD IOMMU
  hw/i386/trace-events: Add AMD IOMMU trace events
  hw/i386: Introduce AMD IOMMU
  hw/i386: AMD IOMMU IVRS table

 hw/acpi/aml-build.c |2 +-
 hw/i386/Makefile.objs   |1 +
 hw/i386/acpi-build.c|   76 ++-
 hw/i386/amd_iommu.c | 1201 +++
 hw/i386/amd_iommu.h |  289 +++
 hw/i386/intel_iommu.c   |1 +
 hw/i386/trace-events|   29 ++
 hw/i386/x86-iommu.c |6 +
 include/hw/acpi/aml-build.h |1 +
 include/hw/i386/x86-iommu.h |   12 +
 include/hw/pci/pci.h|3 +
 11 files changed, 1611 insertions(+), 10 deletions(-)
 create mode 100644 hw/i386/amd_iommu.c
 create mode 100644 hw/i386/amd_iommu.h

-- 
2.1.4

[Qemu-devel] [V18 1/4] hw/pci: Prepare for AMD IOMMU

[David Kiarie]

Introduce PCI macros from for use by AMD IOMMU

Signed-off-by: David Kiarie 
---
 include/hw/pci/pci.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e8b83bb..772692f 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -13,9 +13,12 @@
 /* PCI bus */
 
 #define PCI_DEVFN(slot, func)   slot) & 0x1f) << 3) | ((func) & 0x07))
+#define PCI_BUS_NUM(x)  (((x) >> 8) & 0xff)
 #define PCI_SLOT(devfn) (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn) ((devfn) & 0x07)
 #define PCI_BUILD_BDF(bus, devfn) ((bus << 8) | (devfn))
+#define PCI_BUS_MAX 256
+#define PCI_DEVFN_MAX   256
 #define PCI_SLOT_MAX32
 #define PCI_FUNC_MAX8
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Alfonso Gamboa]

John,

http://mirror.informatimago.com/next/developer.apple.com/qa/dv/dv43.html
perhaps yields some insight:

*Note:*
An example of this is the video 'ndrv' support in Mac OS X. Mac OS X can
load and run native video drivers from the ROM of PCI video cards. This
allows Mac OS X to provide basic video functionality on any card that
supports traditional Mac OS boot-time video, without having a real Mac OS X
video driver available. In order for this to work your video driver must:

   - link with only the standard libraries (DriverServicesLib,
   NameRegistryLib, VideoServicesLib, and PCILib)
   - not access low-memory globals
   - recognize that it's possible for the logical and physical addresses of
   its PCI hardware to be different, and always access the appropriate
   addresses through the appropriate Name Registry properties.


On Mon, Sep 19, 2016 at 3:44 PM, Benjamin Herrenschmidt <
b...@kernel.crashing.org> wrote:

> On Mon, 2016-09-19 at 08:44 -0400, G 3 wrote:
> > On Sep 19, 2016, at 2:24 AM, Benjamin Herrenschmidt wrote:
> >
> > >
> > > On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:
> > > >
> > > > Add the ability to add resolutions from the command-line. This
> > > > patch
> > > > works by
> > > > looking for a property called 'resolutions' in the options node
> > > > of
> > > > OpenBIOS.
> > > > If it is found all the resolutions are parsed and loaded.
> > > >
> > > > Example command-line:
> > >
> > > You must not use the C library in the ndrv (malloc, atoi, ...)
> > >
> > > Stick to what's in DriverServicesLib and NameRegistryLib.
> >
> > I originally thought that, but was wrong. Those C library functions
> > work.
>
> No, don't use them. They "seem" to work but bad thing will happen,
> especially on OS X.
>
> Cheers,
> Ben.
>
> >
> > >
> > >
> > > >
> > > > -prom-env
> > > > resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900
> > > >
> > > > Signed-off-by: John Arbuckle 
> > > > ---
> > > >   QemuVGADriver/src/QemuVga.c | 227
> > > > ++
> > > > +-
> > > >   1 file changed, 225 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/QemuVGADriver/src/QemuVga.c
> > > > b/QemuVGADriver/src/QemuVga.c
> > > > index 4584242..d74fa41 100644
> > > > --- a/QemuVGADriver/src/QemuVga.c
> > > > +++ b/QemuVGADriver/src/QemuVga.c
> > > > @@ -3,6 +3,7 @@
> > > >   #include "DriverQDCalls.h"
> > > >   #include "QemuVga.h"
> > > >   #include 
> > > > +#include 
> > > >
> > > >   /* List of supported modes */
> > > >   struct vMode {
> > > > @@ -18,7 +19,21 @@ static struct vMode vModes[] =  {
> > > >   { 1600, 1200 },
> > > >   { 1920, 1080 },
> > > >   { 1920, 1200 },
> > > > - { 0,0 }
> > > > +
> > > > + /* The rest are place holders */
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > >   };
> > > >
> > > >   static void VgaWriteB(UInt16 port, UInt8 val)
> > > > @@ -147,11 +162,219 @@ static InterruptMemberNumber
> > > > PCIInterruptHandler(InterruptSetMember ISTmember,
> > > >   }
> > > >   #endif
> > > >
> > > > +/*
> > > > + * Get the resolution set at the specified index.
> > > > + * The string returned needs to be freed when no longer used.
> > > > + */
> > > > +static char *get_set(const char *resolution_set_str, int
> > > > set_number)
> > > > +{
> > > > + const int max_buf_size = 100;
> > > > + char c, *buffer;
> > > > + int index, comma_count;
> > > > +
> > > > + buffer = (char *) malloc(max_buf_size);
> > > > + comma_count = 0;
> > > > + index = 0;
> > > > + set_number++; /* Makes things easier to understand */
> > > > +
> > > > + c = *(resolution_set_str++);
> > > > + while (c != '\0') {
> > > > + buffer[index++] = c;
> > > > + c = *(resolution_set_str++);
> > > > + if (c == ',') {
> > > > + comma_count++;
> > > > + if (comma_count == set_number || index
> > > > >=
> > > > max_buf_size) {
> > > > + buffer[index] = '\0';
> > > > + return buffer;
> > > > + }
> > > > +
> > > > + else {
> > > > + /* reset the buffer */
> > > > + index = 0;
> > > > + c = *(resolution_set_str++);
> > > > + }
> > > > + }
> > > > + }
> > > > +
> > > > + buffer[index] = '\0';
> > > > +
> > > > + return buffer;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Get the number of resolution sets
> > > > + */
> > > > +
> > > > +static int get_set_count(const char *resolution_sets_str)
> > > > +{
> > > > + char c;
> > > > + int count;
> > > > +
> > > > + /* Count the number of commas */
> > > > + count = 0;
> > > > + c = *(resolution_sets_str++);
> > > > + while (c != '\0') {
> > > > + if (c 

Re: [Qemu-devel] [PATCH v2 2/3] target-i386: turn off CPU.l3-cache only for 2.7 and older machine types

[Michael S. Tsirkin]

On Mon, Sep 19, 2016 at 01:58:48PM -0300, Eduardo Habkost wrote:
> On Mon, Sep 19, 2016 at 10:32:34AM +0200, Igor Mammedov wrote:
> > commit (14c985cff target-i386: present virtual L3 cache info for vcpus)
> > misplaced compat property putting it in new 2.8 machine type
> > which would effectively to disable feature until 2.9 is released.
> > Intent of commit probably should be to disable feature for 2.7
> > and older while allowing not yet released 2.8 to have feature
> > enabled by default.
> > 
> > Signed-off-by: Igor Mammedov 
> > Reviewed-by: Marcel Apfelbaum 
> > ---
> >  include/hw/i386/pc.h | 7 +++
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index b0a61f3..29a6c9b 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -367,16 +367,15 @@ int e820_get_num_entries(void);
> >  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
> >  
> >  #define PC_COMPAT_2_8 \
> 
> PC_COMPAT_2_8 isn't even used by any code (and shouldn't be used
> until we release QEMU 2.8). Let's just delete it by now?

it's unrelated to this patch though.

> > +
> > +#define PC_COMPAT_2_7 \
> > +HW_COMPAT_2_7 \
> >  {\
> >  .driver   = TYPE_X86_CPU,\
> >  .property = "l3-cache",\
> >  .value= "off",\
> >  },
> >  
> > -
> > -#define PC_COMPAT_2_7 \
> > -HW_COMPAT_2_7
> > -
> >  #define PC_COMPAT_2_6 \
> >  HW_COMPAT_2_6 \
> >  {\
> > -- 
> > 2.7.4
> > 
> 
> -- 
> Eduardo

Re: [Qemu-devel] [PATCH v3 1/2] block: sync bdrv_co_get_block_status_above() with bdrv_is_allocated_above()

[Max Reitz]

On 2016-09-15 at 18:34, Denis V. Lunev wrote:

They should work very similar, covering same areas if backing store is
shorter than the image. This change is necessary for the followup patch
switching to bdrv_get_block_status_above() in mirror to avoid assert
in check_block.

This change should be made very carefully. Let us assume that we have
top image and 2 backing stores L0->L1->L2.
  L0: --
  L1: ---
  L2: ---===
The data marked as '=' in L2 should not appear as BDRV_BLOCK_ALLOCATED
and we should return it as filled in L0 image with properly calculated
*pnum value.

Signed-off-by: Denis V. Lunev 
CC: Stefan Hajnoczi 
CC: Fam Zheng 
CC: Kevin Wolf 
CC: Max Reitz 
CC: Jeff Cody 
---
 block/io.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/block/io.c b/block/io.c
index 420944d..067d465 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1741,18 +1741,33 @@ static int64_t coroutine_fn 
bdrv_co_get_block_status_above(BlockDriverState *bs,
 BlockDriverState **file)
 {
 BlockDriverState *p;
-int64_t ret = 0;
+int64_t ret = 0, res = nb_sectors;


It's not wrong to make res an int64_t, but an int is sufficient.



 assert(bs != base);
 for (p = bs; p != base; p = backing_bs(p)) {
-ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum, file);
-if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
-break;
+int sc;
+ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, , file);
+if (ret < 0) {
+return ret;
+} else if (ret & BDRV_BLOCK_ALLOCATED) {
+*pnum = sc;
+return ret;
+}
+
+if (res > sc && (p == bs || sector_num + sc < p->total_sectors)) {
+res = sc;


This definitely requires some comments because it took me a long time to 
figure out why we need "res" to be a separate variable from "nb_sectors" 
and why this condition is like it is.


So what I think this does is:

Basically, we want to return our final nb_sectors in *pnum. But we can 
have the constellation you noted in the commit message: A short 
intermediate layer, and the bottom layer has some data allocated beyond 
the end of that intermediate layer.


Now, when we pass through that intermediate layer, we need to shorten 
nb_sectors so that we don't query anything beyond the end of that 
intermediate layer because it doesn't matter anyway.


But we also want to remember that all of this area appears as 
unallocated to the top layer, so therefore we have to keep a second 
variable ("res") which retains this information.


Therefore, nb_sectors is always exactly the range we want to query, and 
"res" is the range we know to appear unallocated. This condition here 
tries to adjust "res" so that it conforms to that specification.


However, I'm not quite sure it actually does that. Let's take the case 
from your commit message:


L0: --
L1: ---
L2: ---===

Let's say we invoke this function in the range [0, 14]. After passing 
through L0, res is 14 and nb_sectors is 14. After L1, res is still 14, 
but nb_sectors is 7. So far, so good.


But when passing through L2, "sc" will be 7 (and it will actually always 
be 7, regardless of what comes past sector 7, because nb_sectors is 7). 
Since L2 is larger than just 7 sectors, we will now reduce res to 7 as 
well (because sector_num + sc (= 0 + 7 = 7) < p->total_sectors (= 14)).


So therefore, we will set *pnum to 7. That doesn't seem too bad to me, 
but we could have achieved the same result by just setting *pnum to 
nb_sectors and not having to track the separate "res" variable.



Thus, I'm not quite sure what the point of this is. "res" will only be 
longer than "nb_sectors" as long as the layers get shorter or stay the 
same length when going downwards. As soon as one layer is longer than 
the one above it, "res" will probably be truncated to "sc" (which is 
going to be the same value as "nb_sectors", unless 
bdrv_co_get_block_status() returns a *pnum > nb_sectors).


I'm not sure whether I'm missing something here, though.


 }
+
 /* [sector_num, pnum] unallocated on this layer, which could be only


The "pnum" here should be changed to "sc".


  * the first part of [sector_num, nb_sectors].  */
-nb_sectors = MIN(nb_sectors, *pnum);
+nb_sectors = MIN(nb_sectors, sc);
+
+if (nb_sectors == 0) {
+break;


While I can see that in this case ret would be 0, I think it wouldn't 
hurt to add an explicit "ret = 0;" here, too.


Max


+}
 }
+
+*pnum = res;
 return ret;
 }

Re: [Qemu-devel] [Bug 1625295] Re: qemu-arm dies with libarmmem inside ld.so.preload

[Stu]

- I'm on Ubuntu 16.04, and it looks like it's 2.6.1

qemu-arm version 2.6.1 (Debian 1:2.6.1+dfsg-0~16.04), Copyright (c)
2003-2008 Fabrice Bellard

Is there a PPA for qemu 2.7 somewhere ?


On 19 September 2016 at 21:27, Peter Maydell 
wrote:

> Which version of QEMU are you using? This is I think due to SETEND
> emulation, which I thought we had implemented now.
>
> If this still doesn't work on QEMU 2.7, please can you provide full
> instructions to reproduce the problem (assume I know nothing about how
> to get raspbian or run it on QEMU).
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1625295
>
> Title:
>   qemu-arm dies with libarmmem inside ld.so.preload
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1625295/+subscriptions
>

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

Title:
  qemu-arm dies with libarmmem inside ld.so.preload

Status in QEMU:
  New

Bug description:
  When running raspbian inside qemu,the user has to first comment out
  the following line from /etc/ld.so.conf:

  /usr/lib/arm-linux-gnueabihf/libarmmem.so

  
  Will future qemus will be able to work without changine /etc/ld.so.conf ?

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

Re: [Qemu-devel] build failure

[Jianjun Duan]

It seems my source tree wasn't clean. I removed the old tree and that
error is gone.

Thanks,
Jianjun

On 09/19/2016 02:51 PM, John Snow wrote:
> 
> 
> On 09/19/2016 05:44 PM, Peter Maydell wrote:
>> On 19 September 2016 at 22:38, John Snow  wrote:
>>>
>>>
>>> On 09/19/2016 05:17 PM, Jianjun Duan wrote:

 Hi,
 I ran into the follow problems when I tried to build code based on
 latest master or ppc-for-2.8:

 hw/ide/core.c: In function ‘ide_init_ioport’:
 hw/ide/core.c:2622:39: error: ‘IDEBus’ has no member named
 ‘portio_list’
  isa_register_portio_list(dev, >portio_list,
^
 hw/ide/core.c:2626:43: error: ‘IDEBus’ has no member named
 ‘portio2_list’
  isa_register_portio_list(dev, >portio2_list,
^
 make: *** [hw/ide/core.o] Error 1

 Is there a patch available for this?

 Thanks,
 Jianjun


>>>
>>> Introduced in e305a16510afa74eec20390479e349402e55ef4c.
>>>
>>> Looks related to Marc's recent work, whom I have CC'd.
>>
>> That commit adds the 'portio_list' and 'portio2_list'
>> fields to struct IDEBus, though (and it passed build tests
>> unless I messed something up testing the merge)...
>>
>> thanks
>> -- PMM
>>
> 
> Serves me right for looking quickly.
> 
> Jianjun: Can you perform a build after a make distclean? Maybe you've
> got some outdated files in your tree ...?
> 
> --js
> 

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Mon, 2016-09-19 at 08:44 -0400, G 3 wrote:
> On Sep 19, 2016, at 2:24 AM, Benjamin Herrenschmidt wrote:
> 
> > 
> > On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:
> > > 
> > > Add the ability to add resolutions from the command-line. This
> > > patch
> > > works by
> > > looking for a property called 'resolutions' in the options node
> > > of
> > > OpenBIOS.
> > > If it is found all the resolutions are parsed and loaded.
> > > 
> > > Example command-line:
> > 
> > You must not use the C library in the ndrv (malloc, atoi, ...)
> > 
> > Stick to what's in DriverServicesLib and NameRegistryLib.
> 
> I originally thought that, but was wrong. Those C library functions  
> work.

No, don't use them. They "seem" to work but bad thing will happen,
especially on OS X.

Cheers,
Ben.

> 
> > 
> > 
> > > 
> > > -prom-env
> > > resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900
> > > 
> > > Signed-off-by: John Arbuckle 
> > > ---
> > >   QemuVGADriver/src/QemuVga.c | 227
> > > ++
> > > +-
> > >   1 file changed, 225 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/QemuVGADriver/src/QemuVga.c
> > > b/QemuVGADriver/src/QemuVga.c
> > > index 4584242..d74fa41 100644
> > > --- a/QemuVGADriver/src/QemuVga.c
> > > +++ b/QemuVGADriver/src/QemuVga.c
> > > @@ -3,6 +3,7 @@
> > >   #include "DriverQDCalls.h"
> > >   #include "QemuVga.h"
> > >   #include 
> > > +#include 
> > > 
> > >   /* List of supported modes */
> > >   struct vMode {
> > > @@ -18,7 +19,21 @@ static struct vMode vModes[] =  {
> > >   { 1600, 1200 },
> > >   { 1920, 1080 },
> > >   { 1920, 1200 },
> > > - { 0,0 }
> > > + 
> > > + /* The rest are place holders */
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > > + { 0,0 },
> > >   };
> > > 
> > >   static void VgaWriteB(UInt16 port, UInt8 val)
> > > @@ -147,11 +162,219 @@ static InterruptMemberNumber
> > > PCIInterruptHandler(InterruptSetMember ISTmember,
> > >   }
> > >   #endif
> > > 
> > > +/*
> > > + * Get the resolution set at the specified index.
> > > + * The string returned needs to be freed when no longer used.
> > > + */
> > > +static char *get_set(const char *resolution_set_str, int
> > > set_number)
> > > +{
> > > + const int max_buf_size = 100;
> > > + char c, *buffer;
> > > + int index, comma_count;
> > > +
> > > + buffer = (char *) malloc(max_buf_size);
> > > + comma_count = 0;
> > > + index = 0;
> > > + set_number++; /* Makes things easier to understand */
> > > +
> > > + c = *(resolution_set_str++);
> > > + while (c != '\0') {
> > > + buffer[index++] = c;
> > > + c = *(resolution_set_str++);
> > > + if (c == ',') {
> > > + comma_count++;
> > > + if (comma_count == set_number || index
> > > >=
> > > max_buf_size) {
> > > + buffer[index] = '\0';
> > > + return buffer;
> > > + }
> > > + 
> > > + else {
> > > + /* reset the buffer */
> > > + index = 0;
> > > + c = *(resolution_set_str++);
> > > + }
> > > + }
> > > + }
> > > + 
> > > + buffer[index] = '\0';
> > > +
> > > + return buffer;
> > > +}
> > > +
> > > +/*
> > > + * Get the number of resolution sets
> > > + */
> > > +
> > > +static int get_set_count(const char *resolution_sets_str)
> > > +{
> > > + char c;
> > > + int count;
> > > + 
> > > + /* Count the number of commas */
> > > + count = 0;
> > > + c = *(resolution_sets_str++);
> > > + while (c != '\0') {
> > > + if (c == ',') {
> > > + count++;
> > > + }
> > > + c = *(resolution_sets_str++);
> > > + }
> > > +
> > > + return count + 1;
> > > +}
> > > +
> > > +/*
> > > + * Returns the width value of a resolution set
> > > + * Example:
> > > + * input: 16000x9000
> > > + * output: 16000
> > > + */
> > > +
> > > +static int get_width(const char *resolution_str)
> > > +{
> > > + int index;
> > > + char c;
> > > + const int max_buf_size = 25;
> > > + char buffer[max_buf_size];
> > > + c = *(resolution_str++);
> > > + index = 0;
> > > + while (c != 'x' && index < max_buf_size) {
> > > + buffer[index++] = c;
> > > + c = *(resolution_str++);
> > > + }
> > > + 
> > > + /* Terminate string */
> > > + buffer[index] = '\0';
> > > + 
> > > + return atoi(buffer);
> > > +}
> > > +
> > > +/*
> > > + * Returns the height value of a resolution set
> > > + * Example
> > > + * input: 16000x9000
> > > + * output: 9000
> > > + */
> > > +
> > > +static int get_height(const char *resolution_str)
> > > +{
> > > + int index;
> > > + char c;
> > > + const int max_buf_size = 25;
> > > + char buffer[max_buf_size];
> > > + 
> > > + /* skip to character 

Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Alex Williamson]

On Mon, 19 Sep 2016 23:50:56 +0200
Paolo Bonzini  wrote:

> On 19/09/2016 23:36, Alex Williamson wrote:
> > On Tue, 20 Sep 2016 02:05:52 +0530
> > Kirti Wankhede  wrote:  
> >> 'fb_length':
> >> Read-only file. Mandatory.
> >> Returns {K,M,G}, size of framebuffer.  
> > 
> > This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
> > just one user of mediated devices. [...]
> >   
> >> 'params'
> >> Write-Only file. Optional.
> >> String input. Libvirt would pass the string given in XML file to
> >> this file and then create mdev device. Set empty string to clear params.
> >> For example, set parameter 'frame_rate_limiter=0' to disable frame rate
> >> limiter for performance benchmarking, then create device of type 11. The
> >> device created would have that parameter set by vendor driver.  
> > 
> > Might as well just call it "magic", there's absolutely no ability to
> > introspect what parameters are allowed or even valid here.  Can all
> > parameters be changed dynamically?  Do parameter changes apply to
> > existing devices or only future devices?  This is a poorly specified
> > interface.  I'd prefer this be done via module options on the vendor
> > driver.  
> 
> Or additional files in the mdev's sysfs directory?

Sure, the vendor driver could certainly support that, it'd be vendor
specific of course.
 
> >> The parent device would look like:
> >>
> >>
> >>  pci__86_00_0
> >>  
> >>0
> >>134
> >>0
> >>0  
> > 
> > This is the parent device address?  
> 
> (Since this is taken from an email of mine, this is libvirt's XML
> interpretation of the udev database and the contents of sysfs.  Try
> "virsh nodedev-dumpxml pci__00_02_0" for an example that is
> unrelated to mdev).
> 
> >> 2. Create/destroy mediated device
> >>
> >> With above example, vGPU device XML would look like:
> >>
> >>
> >>  my-vgpu
> >>  pci__86_00_0
> >>  
> >>
> >>1
> >>'frame_rate_limiter=0'
> >>  
> >>
> >>
> >> 'type id' is mandatory.  
> > 
> > I was under the impression that the vendor supplied "name" was the one
> > unique identifier.  We're certainly not going to create a registrar to
> > hand out type ids to each vendor, so what makes type id unique?  
> 
> Why does it have to be unique?  It's just parroting what libvirt said in
> the  block of the parent's nodedev-dumpxml.
> 
> (Though I guess going by name could be useful as well.  Perhaps both
> could be allowed).

Do we not want to be able to migrate a VM to another host and have the
XML mean something?  Off-line or on-line, mdev device may support
migration.  Thanks,

Alex

Re: [Qemu-devel] build failure

[Jianjun Duan]

make distclean didn't solve the problem.

Thanks,
Jianjun

On 09/19/2016 02:51 PM, John Snow wrote:
> 
> 
> On 09/19/2016 05:44 PM, Peter Maydell wrote:
>> On 19 September 2016 at 22:38, John Snow  wrote:
>>>
>>>
>>> On 09/19/2016 05:17 PM, Jianjun Duan wrote:

 Hi,
 I ran into the follow problems when I tried to build code based on
 latest master or ppc-for-2.8:

 hw/ide/core.c: In function ‘ide_init_ioport’:
 hw/ide/core.c:2622:39: error: ‘IDEBus’ has no member named
 ‘portio_list’
  isa_register_portio_list(dev, >portio_list,
^
 hw/ide/core.c:2626:43: error: ‘IDEBus’ has no member named
 ‘portio2_list’
  isa_register_portio_list(dev, >portio2_list,
^
 make: *** [hw/ide/core.o] Error 1

 Is there a patch available for this?

 Thanks,
 Jianjun


>>>
>>> Introduced in e305a16510afa74eec20390479e349402e55ef4c.
>>>
>>> Looks related to Marc's recent work, whom I have CC'd.
>>
>> That commit adds the 'portio_list' and 'portio2_list'
>> fields to struct IDEBus, though (and it passed build tests
>> unless I messed something up testing the merge)...
>>
>> thanks
>> -- PMM
>>
> 
> Serves me right for looking quickly.
> 
> Jianjun: Can you perform a build after a make distclean? Maybe you've
> got some outdated files in your tree ...?
> 
> --js
> 

Re: [Qemu-devel] build failure

[John Snow]

On 09/19/2016 05:44 PM, Peter Maydell wrote:

On 19 September 2016 at 22:38, John Snow  wrote:



On 09/19/2016 05:17 PM, Jianjun Duan wrote:


Hi,
I ran into the follow problems when I tried to build code based on
latest master or ppc-for-2.8:

hw/ide/core.c: In function ‘ide_init_ioport’:
hw/ide/core.c:2622:39: error: ‘IDEBus’ has no member named ‘portio_list’
 isa_register_portio_list(dev, >portio_list,
   ^
hw/ide/core.c:2626:43: error: ‘IDEBus’ has no member named ‘portio2_list’
 isa_register_portio_list(dev, >portio2_list,
   ^
make: *** [hw/ide/core.o] Error 1

Is there a patch available for this?

Thanks,
Jianjun




Introduced in e305a16510afa74eec20390479e349402e55ef4c.

Looks related to Marc's recent work, whom I have CC'd.


That commit adds the 'portio_list' and 'portio2_list'
fields to struct IDEBus, though (and it passed build tests
unless I messed something up testing the merge)...

thanks
-- PMM



Serves me right for looking quickly.

Jianjun: Can you perform a build after a make distclean? Maybe you've 
got some outdated files in your tree ...?


--js

[Qemu-devel] [PULL v3 8/8] iotest 055: refactor and speed up

[Max Reitz]

From: Vladimir Sementsov-Ogievskiy 

Source disk is created and filled with test data before each test case.
Instead initialize it once for the whole unit.

Test disk filling patterns are merged into one pattern.

Also TestSetSpeed used different image_len for source and target (by
mistake) - this is automatically fixed here.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Message-id: 1470748523-13856-1-git-send-email-vsement...@virtuozzo.com
Reviewed-by: Pavel Butsykin 
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/055 | 52 +-
 1 file changed, 18 insertions(+), 34 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index ff4535e..1d3fd04 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -29,17 +29,24 @@ test_img = os.path.join(iotests.test_dir, 'test.img')
 target_img = os.path.join(iotests.test_dir, 'target.img')
 blockdev_target_img = os.path.join(iotests.test_dir, 'blockdev-target.img')
 
-class TestSingleDrive(iotests.QMPTestCase):
-image_len = 64 * 1024 * 1024 # MB
+image_len = 64 * 1024 * 1024 # MB
+
+def setUpModule():
+qemu_img('create', '-f', iotests.imgfmt, test_img, str(image_len))
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x11 0 64k', test_img)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x00 64k 128k', test_img)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x22 162k 32k', test_img)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
+qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x33 67043328 64k', test_img)
 
+def tearDownModule():
+os.remove(test_img)
+
+
+class TestSingleDrive(iotests.QMPTestCase):
 def setUp(self):
-# Write data to the image so we can compare later
-qemu_img('create', '-f', iotests.imgfmt, test_img, 
str(TestSingleDrive.image_len))
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 0 64k', test_img)
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 67043328 64k', 
test_img)
-qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(TestSingleDrive.image_len))
+qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(image_len))
 
 self.vm = 
iotests.VM().add_drive(test_img).add_drive(blockdev_target_img)
 if iotests.qemu_default_machine == 'pc':
@@ -48,7 +55,6 @@ class TestSingleDrive(iotests.QMPTestCase):
 
 def tearDown(self):
 self.vm.shutdown()
-os.remove(test_img)
 os.remove(blockdev_target_img)
 try:
 os.remove(target_img)
@@ -155,19 +161,14 @@ class TestSingleDrive(iotests.QMPTestCase):
 self.assert_qmp(result, 'error/class', 'GenericError')
 
 class TestSetSpeed(iotests.QMPTestCase):
-image_len = 80 * 1024 * 1024 # MB
-
 def setUp(self):
-qemu_img('create', '-f', iotests.imgfmt, test_img, 
str(TestSetSpeed.image_len))
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P1 0 512', test_img)
-qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(TestSingleDrive.image_len))
+qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(image_len))
 
 self.vm = 
iotests.VM().add_drive(test_img).add_drive(blockdev_target_img)
 self.vm.launch()
 
 def tearDown(self):
 self.vm.shutdown()
-os.remove(test_img)
 os.remove(blockdev_target_img)
 try:
 os.remove(target_img)
@@ -243,15 +244,8 @@ class TestSetSpeed(iotests.QMPTestCase):
 self.do_test_set_speed_invalid('blockdev-backup',  'drive1')
 
 class TestSingleTransaction(iotests.QMPTestCase):
-image_len = 64 * 1024 * 1024 # MB
-
 def setUp(self):
-qemu_img('create', '-f', iotests.imgfmt, test_img, 
str(TestSingleTransaction.image_len))
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 0 64k', test_img)
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
-qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 67043328 64k', 
test_img)
-qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(TestSingleDrive.image_len))
+qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, 
str(image_len))
 
 self.vm = 
iotests.VM().add_drive(test_img).add_drive(blockdev_target_img)
 if iotests.qemu_default_machine == 'pc':
@@ -260,7 +254,6 @@ class TestSingleTransaction(iotests.QMPTestCase):
 
 def tearDown(self):
 self.vm.shutdown()
-os.remove(test_img)
 

[Qemu-devel] [PULL v3 4/8] blockdev: Add dynamic generation of module_block.h

[Max Reitz]

From: Marc Mari 

To simplify the addition of new block modules, add a script that generates
module_block.h automatically from the modules' source code.

This script assumes that the QEMU coding style rules are followed.

Signed-off-by: Marc Marí 
Signed-off-by: Colin Lord 
Reviewed-by: Stefan Hajnoczi 
Message-id: 1471008424-16465-3-git-send-email-cl...@redhat.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 Makefile|   7 +++
 scripts/modules/module_block.py | 108 
 2 files changed, 115 insertions(+)
 create mode 100644 scripts/modules/module_block.py

diff --git a/Makefile b/Makefile
index 81ca388..6100567 100644
--- a/Makefile
+++ b/Makefile
@@ -76,6 +76,8 @@ GENERATED_HEADERS += trace/generated-ust-provider.h
 GENERATED_SOURCES += trace/generated-ust.c
 endif
 
+GENERATED_HEADERS += module_block.h
+
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
 Makefile: ;
@@ -352,6 +354,11 @@ ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) 
libqemuutil.a libqemustub.a
 ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) libqemuutil.a libqemustub.a
$(call LINK, $^)
 
+module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
+   $(call quiet-command,$(PYTHON) $< $@ \
+   $(addprefix $(SRC_PATH)/,$(patsubst %.mo,%.c,$(block-obj-m))), \
+   "  GEN   $@")
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h 
gen-op-arm.h
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
new file mode 100644
index 000..db4fb54
--- /dev/null
+++ b/scripts/modules/module_block.py
@@ -0,0 +1,108 @@
+#!/usr/bin/python
+#
+# Module information generator
+#
+# Copyright Red Hat, Inc. 2015 - 2016
+#
+# Authors:
+#  Marc Mari 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+
+from __future__ import print_function
+import sys
+import os
+
+def get_string_struct(line):
+data = line.split()
+
+# data[0] -> struct element name
+# data[1] -> =
+# data[2] -> value
+
+return data[2].replace('"', '')[:-1]
+
+def add_module(fheader, library, format_name, protocol_name):
+lines = []
+lines.append('.library_name = "' + library + '",')
+if format_name != "":
+lines.append('.format_name = "' + format_name + '",')
+if protocol_name != "":
+lines.append('.protocol_name = "' + protocol_name + '",')
+
+text = '\n'.join(lines)
+fheader.write('\n{\n' + text + '\n},')
+
+def process_file(fheader, filename):
+# This parser assumes the coding style rules are being followed
+with open(filename, "r") as cfile:
+found_something = False
+found_start = False
+library, _ = os.path.splitext(os.path.basename(filename))
+for line in cfile:
+if found_start:
+line = line.replace('\n', '')
+if line.find(".format_name") != -1:
+format_name = get_string_struct(line)
+elif line.find(".protocol_name") != -1:
+protocol_name = get_string_struct(line)
+elif line == "};":
+add_module(fheader, library, format_name, protocol_name)
+found_start = False
+elif line.find("static BlockDriver") != -1:
+found_something = True
+found_start = True
+format_name = ""
+protocol_name = ""
+
+if not found_something:
+print("No BlockDriver struct found in " + filename + ". \
+Is this really a module?", file=sys.stderr)
+sys.exit(1)
+
+def print_top(fheader):
+fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
+/*
+ * QEMU Block Module Infrastructure
+ *
+ * Authors:
+ *  Marc Mari   
+ */
+
+''')
+
+fheader.write('''#ifndef QEMU_MODULE_BLOCK_H
+#define QEMU_MODULE_BLOCK_H
+
+#include "qemu-common.h"
+
+static const struct {
+const char *format_name;
+const char *protocol_name;
+const char *library_name;
+} block_driver_modules[] = {''')
+
+def print_bottom(fheader):
+fheader.write('''
+};
+
+#endif
+''')
+
+# First argument: output file
+# All other arguments: modules source files (.c)
+output_file = sys.argv[1]
+with open(output_file, 'w') as fheader:
+print_top(fheader)
+
+for filename in sys.argv[2:]:
+if os.path.isfile(filename):
+process_file(fheader, filename)
+else:
+print("File " + filename + " does not exist.", file=sys.stderr)
+sys.exit(1)
+
+print_bottom(fheader)
+
+sys.exit(0)
-- 

[Qemu-devel] [PULL v3 1/8] qemu-img: add the 'dd' subcommand

[Max Reitz]

From: Reda Sallahi 

This patch adds a basic dd subcommand analogous to dd(1) to qemu-img.

For the start, this implements the bs, if, of and count options and requires
both if and of to be specified (no stdin/stdout if not specified) and doesn't
support tty, pipes, etc.

The image format must be specified with -O for the output if the raw format
is not the intended one.

Two tests are added to test qemu-img dd.

Signed-off-by: Reda Sallahi 
Message-id: 20160810024312.14544-1-fullma...@gmail.com
Reviewed-by: Stefan Hajnoczi 
[mreitz: Moved test 158 to 170]
Signed-off-by: Max Reitz 
---
 qemu-img-cmds.hx |   6 +
 qemu-img.c   | 303 ++-
 qemu-img.texi|  25 
 tests/qemu-iotests/159   |  70 +
 tests/qemu-iotests/159.out   |  87 +++
 tests/qemu-iotests/170   |  67 +
 tests/qemu-iotests/170.out   |  15 ++
 tests/qemu-iotests/common.filter |   9 ++
 tests/qemu-iotests/common.rc |   5 +-
 tests/qemu-iotests/group |   2 +
 10 files changed, 584 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/159
 create mode 100644 tests/qemu-iotests/159.out
 create mode 100755 tests/qemu-iotests/170
 create mode 100644 tests/qemu-iotests/170.out

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 7e95b2d..03bdd7a 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -45,6 +45,12 @@ STEXI
 @item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] 
[-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o 
@var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
+DEF("dd", img_dd,
+"dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] 
if=input of=output")
+STEXI
+@item dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] 
[bs=@var{block_size}] [count=@var{blocks}] if=@var{input} of=@var{output}
+ETEXI
+
 DEF("info", img_info,
 "info [--object objectdef] [--image-opts] [-f fmt] [--output=ofmt] 
[--backing-chain] filename")
 STEXI
diff --git a/qemu-img.c b/qemu-img.c
index ea52486..5bdac8d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -166,7 +166,14 @@ static void QEMU_NORETURN help(void)
"Parameters to compare subcommand:\n"
"  '-f' first image format\n"
"  '-F' second image format\n"
-   "  '-s' run in Strict mode - fail on different image size or sector 
allocation\n";
+   "  '-s' run in Strict mode - fail on different image size or sector 
allocation\n"
+   "\n"
+   "Parameters to dd subcommand:\n"
+   "  'bs=BYTES' read and write up to BYTES bytes at a time "
+   "(default: 512)\n"
+   "  'count=N' copy only N input blocks\n"
+   "  'if=FILE' read from FILE\n"
+   "  'of=FILE' write to FILE\n";
 
 printf("%s\nSupported formats:", help_msg);
 bdrv_iterate_format(format_print, NULL);
@@ -3796,6 +3803,300 @@ out:
 return 0;
 }
 
+#define C_BS  01
+#define C_COUNT   02
+#define C_IF  04
+#define C_OF  010
+
+struct DdInfo {
+unsigned int flags;
+int64_t count;
+};
+
+struct DdIo {
+int bsz;/* Block size */
+char *filename;
+uint8_t *buf;
+};
+
+struct DdOpts {
+const char *name;
+int (*f)(const char *, struct DdIo *, struct DdIo *, struct DdInfo *);
+unsigned int flag;
+};
+
+static int img_dd_bs(const char *arg,
+ struct DdIo *in, struct DdIo *out,
+ struct DdInfo *dd)
+{
+char *end;
+int64_t res;
+
+res = qemu_strtosz_suffix(arg, , QEMU_STRTOSZ_DEFSUFFIX_B);
+
+if (res <= 0 || res > INT_MAX || *end) {
+error_report("invalid number: '%s'", arg);
+return 1;
+}
+in->bsz = out->bsz = res;
+
+return 0;
+}
+
+static int img_dd_count(const char *arg,
+struct DdIo *in, struct DdIo *out,
+struct DdInfo *dd)
+{
+char *end;
+
+dd->count = qemu_strtosz_suffix(arg, , QEMU_STRTOSZ_DEFSUFFIX_B);
+
+if (dd->count < 0 || *end) {
+error_report("invalid number: '%s'", arg);
+return 1;
+}
+
+return 0;
+}
+
+static int img_dd_if(const char *arg,
+ struct DdIo *in, struct DdIo *out,
+ struct DdInfo *dd)
+{
+in->filename = g_strdup(arg);
+
+return 0;
+}
+
+static int img_dd_of(const char *arg,
+ struct DdIo *in, struct DdIo *out,
+ struct DdInfo *dd)
+{
+out->filename = g_strdup(arg);
+
+return 0;
+}
+
+static int img_dd(int argc, char **argv)
+{
+int ret = 0;
+char *arg = NULL;
+char *tmp;
+BlockDriver *drv = NULL, *proto_drv = NULL;
+BlockBackend *blk1 = NULL, *blk2 = NULL;

[Qemu-devel] [PULL v3 6/8] blockdev: Modularize nfs block driver

[Max Reitz]

From: Colin Lord 

Modularizes the nfs block driver so that it gets dynamically loaded.

Signed-off-by: Colin Lord 
Reviewed-by: Stefan Hajnoczi 
Message-id: 1471008424-16465-5-git-send-email-cl...@redhat.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 block/Makefile.objs | 1 +
 configure   | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 3da471e..cb158e9 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -29,6 +29,7 @@ block-obj-y += crypto.o
 
 common-obj-y += stream.o
 
+nfs.o-libs := $(LIBNFS_LIBS)
 iscsi.o-cflags := $(LIBISCSI_CFLAGS)
 iscsi.o-libs   := $(LIBISCSI_LIBS)
 curl.o-cflags  := $(CURL_CFLAGS)
diff --git a/configure b/configure
index 7d083bd..2efc338 100755
--- a/configure
+++ b/configure
@@ -4578,7 +4578,6 @@ if test "$libnfs" != "no" ; then
   if $pkg_config --atleast-version=1.9.3 libnfs; then
 libnfs="yes"
 libnfs_libs=$($pkg_config --libs libnfs)
-LIBS="$LIBS $libnfs_libs"
   else
 if test "$libnfs" = "yes" ; then
   feature_not_found "libnfs" "Install libnfs devel >= 1.9.3"
@@ -5351,7 +5350,8 @@ if test "$libiscsi" = "yes" ; then
 fi
 
 if test "$libnfs" = "yes" ; then
-  echo "CONFIG_LIBNFS=y" >> $config_host_mak
+  echo "CONFIG_LIBNFS=m" >> $config_host_mak
+  echo "LIBNFS_LIBS=$libnfs_libs" >> $config_host_mak
 fi
 
 if test "$seccomp" = "yes"; then
-- 
2.9.3

Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Paolo Bonzini]

On 19/09/2016 23:36, Alex Williamson wrote:
> On Tue, 20 Sep 2016 02:05:52 +0530
> Kirti Wankhede  wrote:
>> 'fb_length':
>> Read-only file. Mandatory.
>> Returns {K,M,G}, size of framebuffer.
> 
> This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
> just one user of mediated devices. [...]
> 
>> 'params'
>> Write-Only file. Optional.
>> String input. Libvirt would pass the string given in XML file to
>> this file and then create mdev device. Set empty string to clear params.
>> For example, set parameter 'frame_rate_limiter=0' to disable frame rate
>> limiter for performance benchmarking, then create device of type 11. The
>> device created would have that parameter set by vendor driver.
> 
> Might as well just call it "magic", there's absolutely no ability to
> introspect what parameters are allowed or even valid here.  Can all
> parameters be changed dynamically?  Do parameter changes apply to
> existing devices or only future devices?  This is a poorly specified
> interface.  I'd prefer this be done via module options on the vendor
> driver.

Or additional files in the mdev's sysfs directory?

>> The parent device would look like:
>>
>>
>>  pci__86_00_0
>>  
>>0
>>134
>>0
>>0
> 
> This is the parent device address?

(Since this is taken from an email of mine, this is libvirt's XML
interpretation of the udev database and the contents of sysfs.  Try
"virsh nodedev-dumpxml pci__00_02_0" for an example that is
unrelated to mdev).

>> 2. Create/destroy mediated device
>>
>> With above example, vGPU device XML would look like:
>>
>>
>>  my-vgpu
>>  pci__86_00_0
>>  
>>
>>1
>>'frame_rate_limiter=0'
>>  
>>
>>
>> 'type id' is mandatory.
> 
> I was under the impression that the vendor supplied "name" was the one
> unique identifier.  We're certainly not going to create a registrar to
> hand out type ids to each vendor, so what makes type id unique?

Why does it have to be unique?  It's just parroting what libvirt said in
the  block of the parent's nodedev-dumpxml.

(Though I guess going by name could be useful as well.  Perhaps both
could be allowed).

>> * Clear params, if set earlier:
>>
>> echo "" > /sys/../\:86\:00.0/11/params
> 
> So params only applies to the devices created while those params are
> set?  This is inherently racy.

I agree, these extra attributes should be set on the mdev, not the type
directory.

Paolo

[Qemu-devel] [PULL v3 3/8] blockdev: prepare iSCSI block driver for dynamic loading

[Max Reitz]

From: Colin Lord 

This commit moves the initialization of the QemuOptsList qemu_iscsi_opts
struct out of block/iscsi.c in order to allow the iscsi module to be
dynamically loaded.

Signed-off-by: Colin Lord 
Reviewed-by: Fam Zheng 
Reviewed-by: Stefan Hajnoczi 
Message-id: 1471008424-16465-2-git-send-email-cl...@redhat.com
Reviewed-by: Max Reitz 
Signed-off-by: Max Reitz 
---
 block/iscsi.c | 36 
 vl.c  | 40 
 2 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 95ce9e1..c4a0937 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2010,45 +2010,9 @@ static BlockDriver bdrv_iscsi = {
 .bdrv_attach_aio_context = iscsi_attach_aio_context,
 };
 
-static QemuOptsList qemu_iscsi_opts = {
-.name = "iscsi",
-.head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
-.desc = {
-{
-.name = "user",
-.type = QEMU_OPT_STRING,
-.help = "username for CHAP authentication to target",
-},{
-.name = "password",
-.type = QEMU_OPT_STRING,
-.help = "password for CHAP authentication to target",
-},{
-.name = "password-secret",
-.type = QEMU_OPT_STRING,
-.help = "ID of the secret providing password for CHAP "
-"authentication to target",
-},{
-.name = "header-digest",
-.type = QEMU_OPT_STRING,
-.help = "HeaderDigest setting. "
-"{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
-},{
-.name = "initiator-name",
-.type = QEMU_OPT_STRING,
-.help = "Initiator iqn name to use when connecting",
-},{
-.name = "timeout",
-.type = QEMU_OPT_NUMBER,
-.help = "Request timeout in seconds (default 0 = no timeout)",
-},
-{ /* end of list */ }
-},
-};
-
 static void iscsi_block_init(void)
 {
 bdrv_register(_iscsi);
-qemu_add_opts(_iscsi_opts);
 }
 
 block_init(iscsi_block_init);
diff --git a/vl.c b/vl.c
index fca0487..fd321e2 100644
--- a/vl.c
+++ b/vl.c
@@ -507,6 +507,43 @@ static QemuOptsList qemu_fw_cfg_opts = {
 },
 };
 
+#ifdef CONFIG_LIBISCSI
+static QemuOptsList qemu_iscsi_opts = {
+.name = "iscsi",
+.head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
+.desc = {
+{
+.name = "user",
+.type = QEMU_OPT_STRING,
+.help = "username for CHAP authentication to target",
+},{
+.name = "password",
+.type = QEMU_OPT_STRING,
+.help = "password for CHAP authentication to target",
+},{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
+.help = "ID of the secret providing password for CHAP "
+"authentication to target",
+},{
+.name = "header-digest",
+.type = QEMU_OPT_STRING,
+.help = "HeaderDigest setting. "
+"{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
+},{
+.name = "initiator-name",
+.type = QEMU_OPT_STRING,
+.help = "Initiator iqn name to use when connecting",
+},{
+.name = "timeout",
+.type = QEMU_OPT_NUMBER,
+.help = "Request timeout in seconds (default 0 = no timeout)",
+},
+{ /* end of list */ }
+},
+};
+#endif
+
 /**
  * Get machine options
  *
@@ -3017,6 +3054,9 @@ int main(int argc, char **argv, char **envp)
 qemu_add_opts(_icount_opts);
 qemu_add_opts(_semihosting_config_opts);
 qemu_add_opts(_fw_cfg_opts);
+#ifdef CONFIG_LIBISCSI
+qemu_add_opts(_iscsi_opts);
+#endif
 module_call_init(MODULE_INIT_OPTS);
 
 runstate_init();
-- 
2.9.3

[Qemu-devel] [PULL v3 0/8] Block layer patches

[Max Reitz]

The following changes since commit 33e1666b4289313306371fee0740f5c85517e406:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-09-19' into 
staging (2016-09-19 18:06:52 +0100)

are available in the git repository at:

  git://github.com/XanClic/qemu.git tags/pull-block-2016-09-19

for you to fetch changes up to 203bfabbcc0d681ebb4a8f0a0d08b077d4e53eae:

  iotest 055: refactor and speed up (2016-09-19 23:13:40 +0200)


Block patches for 2.8


Alberto Garcia (1):
  commit: get the overlay node before manipulating the backing chain

Colin Lord (2):
  blockdev: prepare iSCSI block driver for dynamic loading
  blockdev: Modularize nfs block driver

Marc Mari (2):
  blockdev: Add dynamic generation of module_block.h
  blockdev: Add dynamic module loading for block drivers

Reda Sallahi (2):
  qemu-img: add the 'dd' subcommand
  qemu-img: add skip option to dd

Vladimir Sementsov-Ogievskiy (1):
  iotest 055: refactor and speed up

 Makefile |  10 +-
 block.c  |  62 ++-
 block/Makefile.objs  |   4 +-
 block/commit.c   |   3 +-
 block/iscsi.c|  36 
 configure|   4 +-
 include/qemu/module.h|   3 +
 qemu-img-cmds.hx |   6 +
 qemu-img.c   | 343 ++-
 qemu-img.texi|  27 +++
 scripts/modules/module_block.py  | 108 
 tests/qemu-iotests/055   |  52 ++
 tests/qemu-iotests/159   |  70 
 tests/qemu-iotests/159.out   |  87 ++
 tests/qemu-iotests/160   |  72 
 tests/qemu-iotests/160.out   |  51 ++
 tests/qemu-iotests/170   |  67 
 tests/qemu-iotests/170.out   |  15 ++
 tests/qemu-iotests/common.filter |   9 +
 tests/qemu-iotests/common.rc |   5 +-
 tests/qemu-iotests/group |   3 +
 util/module.c|  38 ++---
 vl.c |  40 +
 23 files changed, 997 insertions(+), 118 deletions(-)
 create mode 100644 scripts/modules/module_block.py
 create mode 100755 tests/qemu-iotests/159
 create mode 100644 tests/qemu-iotests/159.out
 create mode 100755 tests/qemu-iotests/160
 create mode 100644 tests/qemu-iotests/160.out
 create mode 100755 tests/qemu-iotests/170
 create mode 100644 tests/qemu-iotests/170.out

-- 
2.9.3

[Qemu-devel] [PULL v3 5/8] blockdev: Add dynamic module loading for block drivers

[Max Reitz]

From: Marc Mari 

Extend the current module interface to allow for block drivers to be
loaded dynamically on request. The only block drivers that can be
converted into modules are the drivers that don't perform any init
operation except for registering themselves.

In addition, only the protocol drivers are being modularized, as they
are the only ones which see significant performance benefits. The format
drivers do not generally link to external libraries, so modularizing
them is of no benefit from a performance perspective.

All the necessary module information is located in a new structure found
in module_block.h

This spoils the purpose of 5505e8b76f (block/dmg: make it modular).

Before this patch, if module build is enabled, block-dmg.so is linked to
libbz2, whereas the main binary is not. In downstream, theoretically, it
means only the qemu-block-extra package depends on libbz2, while the
main QEMU package needn't to. With this patch, we (temporarily) change
the case so that the main QEMU depends on libbz2 again.

Signed-off-by: Marc Marí 
Signed-off-by: Colin Lord 
Reviewed-by: Stefan Hajnoczi 
Message-id: 1471008424-16465-4-git-send-email-cl...@redhat.com
Reviewed-by: Max Reitz 
[mreitz: Replaced size_t i by int i, so an empty block_driver_modules[]
 array will not cause a build error]
Signed-off-by: Max Reitz 
---
 Makefile  |  3 ---
 block.c   | 62 +--
 block/Makefile.objs   |  3 +--
 include/qemu/module.h |  3 +++
 util/module.c | 38 +--
 5 files changed, 70 insertions(+), 39 deletions(-)

diff --git a/Makefile b/Makefile
index 6100567..b9ed405 100644
--- a/Makefile
+++ b/Makefile
@@ -247,9 +247,6 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 libqemustub.a: $(stub-obj-y)
 libqemuutil.a: $(util-obj-y)
 
-block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) NULL
-util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
-
 ##
 
 qemu-img.o: qemu-img-cmds.h
diff --git a/block.c b/block.c
index 66ed1c0..e5384f7 100644
--- a/block.c
+++ b/block.c
@@ -27,6 +27,7 @@
 #include "block/blockjob.h"
 #include "block/nbd.h"
 #include "qemu/error-report.h"
+#include "module_block.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qbool.h"
@@ -242,17 +243,40 @@ BlockDriverState *bdrv_new(void)
 return bs;
 }
 
-BlockDriver *bdrv_find_format(const char *format_name)
+static BlockDriver *bdrv_do_find_format(const char *format_name)
 {
 BlockDriver *drv1;
+
 QLIST_FOREACH(drv1, _drivers, list) {
 if (!strcmp(drv1->format_name, format_name)) {
 return drv1;
 }
 }
+
 return NULL;
 }
 
+BlockDriver *bdrv_find_format(const char *format_name)
+{
+BlockDriver *drv1;
+int i;
+
+drv1 = bdrv_do_find_format(format_name);
+if (drv1) {
+return drv1;
+}
+
+/* The driver isn't registered, maybe we need to load a module */
+for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+if (!strcmp(block_driver_modules[i].format_name, format_name)) {
+block_module_load_one(block_driver_modules[i].library_name);
+break;
+}
+}
+
+return bdrv_do_find_format(format_name);
+}
+
 static int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
 {
 static const char *whitelist_rw[] = {
@@ -461,6 +485,19 @@ static BlockDriver *find_hdev_driver(const char *filename)
 return drv;
 }
 
+static BlockDriver *bdrv_do_find_protocol(const char *protocol)
+{
+BlockDriver *drv1;
+
+QLIST_FOREACH(drv1, _drivers, list) {
+if (drv1->protocol_name && !strcmp(drv1->protocol_name, protocol)) {
+return drv1;
+}
+}
+
+return NULL;
+}
+
 BlockDriver *bdrv_find_protocol(const char *filename,
 bool allow_protocol_prefix,
 Error **errp)
@@ -469,6 +506,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 char protocol[128];
 int len;
 const char *p;
+int i;
 
 /* TODO Drivers without bdrv_file_open must be specified explicitly */
 
@@ -495,15 +533,25 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 len = sizeof(protocol) - 1;
 memcpy(protocol, filename, len);
 protocol[len] = '\0';
-QLIST_FOREACH(drv1, _drivers, list) {
-if (drv1->protocol_name &&
-!strcmp(drv1->protocol_name, protocol)) {
-return drv1;
+
+drv1 = bdrv_do_find_protocol(protocol);
+if (drv1) {
+return drv1;
+}
+
+for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+if (block_driver_modules[i].protocol_name &&
+!strcmp(block_driver_modules[i].protocol_name, protocol)) {
+  

[Qemu-devel] [PULL v3 2/8] qemu-img: add skip option to dd

[Max Reitz]

From: Reda Sallahi 

This adds the skip option which allows qemu-img dd to skip a number of blocks
before copying the input.

A test case was added to test the skip option.

Signed-off-by: Reda Sallahi 
Message-id: 20160810141609.32727-1-fullma...@gmail.com
Signed-off-by: Max Reitz 
---
 qemu-img-cmds.hx   |  4 +--
 qemu-img.c | 50 
 qemu-img.texi  |  4 ++-
 tests/qemu-iotests/160 | 72 ++
 tests/qemu-iotests/160.out | 51 
 tests/qemu-iotests/group   |  1 +
 6 files changed, 174 insertions(+), 8 deletions(-)
 create mode 100755 tests/qemu-iotests/160
 create mode 100644 tests/qemu-iotests/160.out

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 03bdd7a..f054599 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -46,9 +46,9 @@ STEXI
 ETEXI
 
 DEF("dd", img_dd,
-"dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] 
if=input of=output")
+"dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] 
[skip=blocks] if=input of=output")
 STEXI
-@item dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] 
[bs=@var{block_size}] [count=@var{blocks}] if=@var{input} of=@var{output}
+@item dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] 
[bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} 
of=@var{output}
 ETEXI
 
 DEF("info", img_info,
diff --git a/qemu-img.c b/qemu-img.c
index 5bdac8d..ceffefe 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -173,7 +173,8 @@ static void QEMU_NORETURN help(void)
"(default: 512)\n"
"  'count=N' copy only N input blocks\n"
"  'if=FILE' read from FILE\n"
-   "  'of=FILE' write to FILE\n";
+   "  'of=FILE' write to FILE\n"
+   "  'skip=N' skip N bs-sized blocks at the start of input\n";
 
 printf("%s\nSupported formats:", help_msg);
 bdrv_iterate_format(format_print, NULL);
@@ -3807,6 +3808,7 @@ out:
 #define C_COUNT   02
 #define C_IF  04
 #define C_OF  010
+#define C_SKIP020
 
 struct DdInfo {
 unsigned int flags;
@@ -3817,6 +3819,7 @@ struct DdIo {
 int bsz;/* Block size */
 char *filename;
 uint8_t *buf;
+int64_t offset;
 };
 
 struct DdOpts {
@@ -3877,6 +3880,22 @@ static int img_dd_of(const char *arg,
 return 0;
 }
 
+static int img_dd_skip(const char *arg,
+   struct DdIo *in, struct DdIo *out,
+   struct DdInfo *dd)
+{
+char *end;
+
+in->offset = qemu_strtosz_suffix(arg, , QEMU_STRTOSZ_DEFSUFFIX_B);
+
+if (in->offset < 0 || *end) {
+error_report("invalid number: '%s'", arg);
+return 1;
+}
+
+return 0;
+}
+
 static int img_dd(int argc, char **argv)
 {
 int ret = 0;
@@ -3900,12 +3919,14 @@ static int img_dd(int argc, char **argv)
 struct DdIo in = {
 .bsz = 512, /* Block size is by default 512 bytes */
 .filename = NULL,
-.buf = NULL
+.buf = NULL,
+.offset = 0
 };
 struct DdIo out = {
 .bsz = 512,
 .filename = NULL,
-.buf = NULL
+.buf = NULL,
+.offset = 0
 };
 
 const struct DdOpts options[] = {
@@ -3913,6 +3934,7 @@ static int img_dd(int argc, char **argv)
 { "count", img_dd_count, C_COUNT },
 { "if", img_dd_if, C_IF },
 { "of", img_dd_of, C_OF },
+{ "skip", img_dd_skip, C_SKIP },
 { NULL, NULL, 0 }
 };
 const struct option long_options[] = {
@@ -4032,7 +4054,14 @@ static int img_dd(int argc, char **argv)
 size = dd.count * in.bsz;
 }
 
-qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, _abort);
+/* Overflow means the specified offset is beyond input image's size */
+if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
+  size < in.bsz * in.offset)) {
+qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, _abort);
+} else {
+qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
+size - in.bsz * in.offset, _abort);
+}
 
 ret = bdrv_create(drv, out.filename, opts, _err);
 if (ret < 0) {
@@ -4051,9 +4080,20 @@ static int img_dd(int argc, char **argv)
 goto out;
 }
 
+if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
+  size < in.offset * in.bsz)) {
+/* We give a warning if the skip option is bigger than the input
+ * size and create an empty output disk image (i.e. like dd(1)).
+ */
+error_report("%s: cannot skip to specified offset", in.filename);
+in_pos = size;
+} else {
+in_pos = in.offset * in.bsz;
+}
+
 in.buf = g_new(uint8_t, in.bsz);
 
-for (in_pos = 0, out_pos = 0; in_pos < size; block_count++) {
+for (out_pos = 0; in_pos < size; block_count++) 

[Qemu-devel] [PULL v3 7/8] commit: get the overlay node before manipulating the backing chain

[Max Reitz]

From: Alberto Garcia 

The 'block-commit' command has a 'top' parameter to specify the
topmost node from which the data is going to be copied.

   [E] <- [D] <- [C] <- [B] <- [A]

In this case if [C] is the top node then this is the result:

   [E] <- [B] <- [A]

[B] must be modified so its backing image string points to [E] instead
of [C]. commit_start() takes care of reopening [B] in read-write
mode, and commit_complete() puts it back in read-only mode once the
operation has finished.

In order to find [B] (the overlay node) we look for the node that has
[C] (the top node) as its backing image. However in commit_complete()
we're doing it after [C] has been removed from the chain, so [B] is
never found and remains in read-write mode.

This patch gets the overlay node before the backing chain is
manipulated.

Signed-off-by: Alberto Garcia 
Message-id: 1471836963-28548-1-git-send-email-be...@igalia.com
Signed-off-by: Max Reitz 
---
 block/commit.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index 553e18d..a02539b 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -83,7 +83,7 @@ static void commit_complete(BlockJob *job, void *opaque)
 BlockDriverState *active = s->active;
 BlockDriverState *top = blk_bs(s->top);
 BlockDriverState *base = blk_bs(s->base);
-BlockDriverState *overlay_bs;
+BlockDriverState *overlay_bs = bdrv_find_overlay(active, top);
 int ret = data->ret;
 
 if (!block_job_is_cancelled(>common) && ret == 0) {
@@ -97,7 +97,6 @@ static void commit_complete(BlockJob *job, void *opaque)
 if (s->base_flags != bdrv_get_flags(base)) {
 bdrv_reopen(base, s->base_flags, NULL);
 }
-overlay_bs = bdrv_find_overlay(active, top);
 if (overlay_bs && s->orig_overlay_flags != bdrv_get_flags(overlay_bs)) {
 bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
 }
-- 
2.9.3

Re: [Qemu-devel] build failure

[Peter Maydell]

On 19 September 2016 at 22:38, John Snow  wrote:
>
>
> On 09/19/2016 05:17 PM, Jianjun Duan wrote:
>>
>> Hi,
>> I ran into the follow problems when I tried to build code based on
>> latest master or ppc-for-2.8:
>>
>> hw/ide/core.c: In function ‘ide_init_ioport’:
>> hw/ide/core.c:2622:39: error: ‘IDEBus’ has no member named ‘portio_list’
>>  isa_register_portio_list(dev, >portio_list,
>>^
>> hw/ide/core.c:2626:43: error: ‘IDEBus’ has no member named ‘portio2_list’
>>  isa_register_portio_list(dev, >portio2_list,
>>^
>> make: *** [hw/ide/core.o] Error 1
>>
>> Is there a patch available for this?
>>
>> Thanks,
>> Jianjun
>>
>>
>
> Introduced in e305a16510afa74eec20390479e349402e55ef4c.
>
> Looks related to Marc's recent work, whom I have CC'd.

That commit adds the 'portio_list' and 'portio2_list'
fields to struct IDEBus, though (and it passed build tests
unless I messed something up testing the merge)...

thanks
-- PMM

Re: [Qemu-devel] build failure

[John Snow]

On 09/19/2016 05:17 PM, Jianjun Duan wrote:

Hi,
I ran into the follow problems when I tried to build code based on
latest master or ppc-for-2.8:

hw/ide/core.c: In function ‘ide_init_ioport’:
hw/ide/core.c:2622:39: error: ‘IDEBus’ has no member named ‘portio_list’
 isa_register_portio_list(dev, >portio_list,
   ^
hw/ide/core.c:2626:43: error: ‘IDEBus’ has no member named ‘portio2_list’
 isa_register_portio_list(dev, >portio2_list,
   ^
make: *** [hw/ide/core.o] Error 1

Is there a patch available for this?

Thanks,
Jianjun




Introduced in e305a16510afa74eec20390479e349402e55ef4c.

Looks related to Marc's recent work, whom I have CC'd.

--js

Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Alex Williamson]

On Tue, 20 Sep 2016 02:05:52 +0530
Kirti Wankhede  wrote:

> Hi libvirt experts,
> 
> Thanks for valuable input on v1 version of RFC.
> 
> Quick brief, VFIO based mediated device framework provides a way to
> virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
> and IBM's channel IO. This framework reuses VFIO APIs for all the
> functionalities for mediated devices which are currently being used for
> pass through devices. This framework introduces a set of new sysfs files
> for device creation and its life cycle management.
> 
> Here is the summary of discussion on v1:
> 1. Discover mediated device:
> As part of physical device initialization process, vendor driver will
> register their physical devices, which will be used to create virtual
> device (mediated device, aka mdev) to the mediated framework.
> 
> Vendor driver should specify mdev_supported_types in directory format.
> This format is class based, for example, display class directory format
> should be as below. We need to define such set for each class of devices
> which would be supported by mediated device framework.
> 
>  --- mdev_destroy

I thought we replaced mdev_destroy with a remove attribute for each
mdev device.

>  --- mdev_supported_types
>  |-- 11
>  |   |-- create
>  |   |-- name
>  |   |-- fb_length
>  |   |-- resolution
>  |   |-- heads
>  |   |-- max_instances
>  |   |-- params
>  |   |-- requires_group
>  |-- 12
>  |   |-- create
>  |   |-- name
>  |   |-- fb_length
>  |   |-- resolution
>  |   |-- heads
>  |   |-- max_instances
>  |   |-- params
>  |   |-- requires_group
>  |-- 13
>  |-- create
>  |-- name
>  |-- fb_length
>  |-- resolution
>  |-- heads
>  |-- max_instances
>  |-- params
>  |-- requires_group
> 
> 
> In the above example directory '11' represents a type id of mdev device.
> 'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
> 'requires_group' would be Read-Only files that vendor would provide to
> describe about that type.
> 
> 'create':
> Write-only file. Mandatory.
> Accepts string to create mediated device.
> 
> 'name':
> Read-Only file. Mandatory.
> Returns string, the name of that type id.
> 
> 'fb_length':
> Read-only file. Mandatory.
> Returns {K,M,G}, size of framebuffer.

This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
just one user of mediated devices.

> 
> 'resolution':
> Read-Only file. Mandatory.
> Returns 'hres x vres' format. Maximum supported resolution.

Same.

> 
> 'heads':
> Read-Only file. Mandatory.
> Returns integer. Number of maximum heads supported.

Same.

> 
> 'max_instance':
> Read-Only file. Mandatory.
> Returns integer.  Returns maximum mdev device could be created
> at the moment when this file is read. This count would be updated by
> vendor driver. Before creating mdev device of this type, check if
> max_instance is > 0.

Didn't we discuss this being being something like "available_instances"
with a "devices" sub-directory linking current devices of that type?
 
> 'params'
> Write-Only file. Optional.
> String input. Libvirt would pass the string given in XML file to
> this file and then create mdev device. Set empty string to clear params.
> For example, set parameter 'frame_rate_limiter=0' to disable frame rate
> limiter for performance benchmarking, then create device of type 11. The
> device created would have that parameter set by vendor driver.

Might as well just call it "magic", there's absolutely no ability to
introspect what parameters are allowed or even valid here.  Can all
parameters be changed dynamically?  Do parameter changes apply to
existing devices or only future devices?  This is a poorly specified
interface.  I'd prefer this be done via module options on the vendor
driver.

> 'requires_group'
> Read-Only file. Optional.
> This should be provided by vendor driver if vendor driver need to
> group mdev devices in one domain so that vendor driver can use 'first
> open' to commit resources of all mdev devices associated to that domain
> and 'last close' to free those.
> 
> The parent device would look like:
> 
>
>  pci__86_00_0
>  
>0
>134
>0
>0

This is the parent device address?  (I think we'd re-use the
specification for assigned devices)  Is this optional?  Couldn't
libvirt choose to pick any parent device supplying the specified type
if not supplied?

>
>  
>  
>
>GRID M60-0B
>512M
>2560x1600
>2
>16
>1
>  
>
>GRID M60
>NVIDIA

What are these specifying?

>  
>
> 
> 2. Create/destroy mediated device
> 
> With above example, vGPU device XML would look like:
> 
>
>  my-vgpu
>  

Re: [Qemu-devel] [PATCH v4 0/2] block: allow flush on devices with open tray

[Max Reitz]

On 19.09.2016 23:21, John Snow wrote:
> 
> 
> On 09/19/2016 04:58 PM, Max Reitz wrote:
>> On 19.09.2016 18:44, John Snow wrote:
>>> Final re-send for wording.
>>>
>>> The move to blk_flush altered the behavior of migration and flushing
>>> nodes that are not reachable via the guest, but are still reachable
>>> via QEMU and may or may not need to be flushed.
>>>
>>> This is likely the simplest solution for now until we nail down our
>>> policy a bit more.
>>>
>>> This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt
>>> et al being unable to migrate QEMU when the CDROM tray is open.
>>>
>>> v4:
>>>  Commit message update.
>>>
>>> v3:
>>>  Trying to take a hint from Kevin, reinstating bdrv_flush_all.
>>>  If it's not what we want, we can try moving back to v2,
>>>  acknowledging that a nicer solution in the future:
>>>  (A) Can skip flushing on devices that just don't need it, and
>>>  (B) Optionally institutes some sort of flush-on-eject policy.
>>>
>>> 
>>>
>>>
>>> For convenience, this branch is available at:
>>> https://github.com/jnsnow/qemu.git branch atapi-tray-migfix
>>> https://github.com/jnsnow/qemu/tree/atapi-tray-migfix
>>>
>>> This version is tagged atapi-tray-migfix-v4:
>>> https://github.com/jnsnow/qemu/releases/tag/atapi-tray-migfix-v4
>>>
>>> John Snow (2):
>>>   block: reintroduce bdrv_flush_all
>>>   qemu: use bdrv_flush_all for vm_stop et al
>>>
>>>  block/io.c| 25 +
>>>  cpus.c|  4 ++--
>>>  include/block/block.h |  1 +
>>>  3 files changed, 28 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Max Reitz 
>>
> 
> http://i.imgur.com/DNltmfT.jpg

http://bit.ly/2cLX1rm



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 0/2] block: allow flush on devices with open tray

[John Snow]

On 09/19/2016 04:58 PM, Max Reitz wrote:

On 19.09.2016 18:44, John Snow wrote:

Final re-send for wording.

The move to blk_flush altered the behavior of migration and flushing
nodes that are not reachable via the guest, but are still reachable
via QEMU and may or may not need to be flushed.

This is likely the simplest solution for now until we nail down our
policy a bit more.

This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt
et al being unable to migrate QEMU when the CDROM tray is open.

v4:
 Commit message update.

v3:
 Trying to take a hint from Kevin, reinstating bdrv_flush_all.
 If it's not what we want, we can try moving back to v2,
 acknowledging that a nicer solution in the future:
 (A) Can skip flushing on devices that just don't need it, and
 (B) Optionally institutes some sort of flush-on-eject policy.



For convenience, this branch is available at:
https://github.com/jnsnow/qemu.git branch atapi-tray-migfix
https://github.com/jnsnow/qemu/tree/atapi-tray-migfix

This version is tagged atapi-tray-migfix-v4:
https://github.com/jnsnow/qemu/releases/tag/atapi-tray-migfix-v4

John Snow (2):
  block: reintroduce bdrv_flush_all
  qemu: use bdrv_flush_all for vm_stop et al

 block/io.c| 25 +
 cpus.c|  4 ++--
 include/block/block.h |  1 +
 3 files changed, 28 insertions(+), 2 deletions(-)


Reviewed-by: Max Reitz 



http://i.imgur.com/DNltmfT.jpg

Re: [Qemu-devel] [PULL v2 0/8] Block layer patches

[Max Reitz]

On 19.09.2016 13:39, Peter Maydell wrote:
> On 17 September 2016 at 23:05, Max Reitz  wrote:
>> The following changes since commit e3571ae30cd26d19efd4554c25e32ef64d6a36b3:
>>
>>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20160916' into 
>> staging (2016-09-16 16:54:50 +0100)
>>
>> are available in the git repository at:
>>
>>   git://github.com/XanClic/qemu.git tags/pull-block-2016-09-17
>>
>> for you to fetch changes up to 61601533ace153922fec04460f0b9b168b244968:
>>
>>   iotest 055: refactor and speed up (2016-09-17 23:51:18 +0200)
>>
>> 
>> Block patches for 2.8
> 
> I'm afraid this fails to build with the win32 compiler:
> /home/petmay01/linaro/qemu-for-merges/block.c: In function ‘bdrv_find_format’:
> /home/petmay01/linaro/qemu-for-merges/block.c:270:19: error:
> comparison of unsigned expression < 0 is always false
> [-Werror=type-limits]
>  for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
>^
> /home/petmay01/linaro/qemu-for-merges/block.c: In function 
> ‘bdrv_find_protocol’:
> /home/petmay01/linaro/qemu-for-merges/block.c:542:19: error:
> comparison of unsigned expression < 0 is always false
> [-Werror=type-limits]
>  for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
>^
> cc1: all warnings being treated as errors

Thanks, I'll fix it. Let's hope third time's a charm. :-/

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] build failure

[Jianjun Duan]

Hi,
I ran into the follow problems when I tried to build code based on
latest master or ppc-for-2.8:

hw/ide/core.c: In function ‘ide_init_ioport’:
hw/ide/core.c:2622:39: error: ‘IDEBus’ has no member named ‘portio_list’
 isa_register_portio_list(dev, >portio_list,
   ^
hw/ide/core.c:2626:43: error: ‘IDEBus’ has no member named ‘portio2_list’
 isa_register_portio_list(dev, >portio2_list,
   ^
make: *** [hw/ide/core.o] Error 1

Is there a patch available for this?

Thanks,
Jianjun

Re: [Qemu-devel] [libvirt] [PATCH v3 0/5] Add runnability info to query-cpu-definitions

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [libvirt] [PATCH v3 0/5] Add runnability info to query-cpu-definitions
Message-id: 1474314175-24374-1-git-send-email-ehabk...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/1474314175-24374-1-git-send-email-ehabk...@redhat.com -> 
patchew/1474314175-24374-1-git-send-email-ehabk...@redhat.com
 * [new tag] patchew/c1a178ce-1c5e-2c26-0f55-6a4fbfe21...@tuxfamily.org 
-> patchew/c1a178ce-1c5e-2c26-0f55-6a4fbfe21...@tuxfamily.org
Switched to a new branch 'test'
43f49c7 target-i386: Return runnability information on query-cpu-definitions
17a9434 qmp: Add runnability information to query-cpu-definitions
405e270 target-i386: Define CPUID filtering functions before x86_cpu_list()
dc099e7 target-i386: Move warning code outside x86_cpu_filter_features()
21da10b target-i386: List CPU models using subclass list

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
=== OUTPUT END ===

Abort: command timeout (>3600 seconds)


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

Re: [Qemu-devel] [PATCH v7 1/4] vfio: Mediated device Core driver

[Alex Williamson]

On Tue, 20 Sep 2016 01:39:19 +0530
Kirti Wankhede  wrote:

> On 9/19/2016 11:41 PM, Alex Williamson wrote:
> > On Mon, 19 Sep 2016 22:59:34 +0530
> > Kirti Wankhede  wrote:
> >   
> >> On 9/12/2016 9:23 PM, Alex Williamson wrote:  
> >>> On Mon, 12 Sep 2016 13:19:11 +0530
> >>> Kirti Wankhede  wrote:
> >>> 
>  On 9/12/2016 10:40 AM, Jike Song wrote:
> > On 09/10/2016 03:55 AM, Kirti Wankhede wrote:  
> >> On 9/10/2016 12:12 AM, Alex Williamson wrote:  
> >>> On Fri, 9 Sep 2016 23:18:45 +0530
> >>> Kirti Wankhede  wrote:
> >>>  
> >> +
> >> +static struct parent_device *mdev_get_parent_from_dev(struct 
> >> device *dev)
> >> +{
> >> +  struct parent_device *parent;
> >> +
> >> +  mutex_lock(_list_lock);
> >> +  parent = mdev_get_parent(__find_parent_device(dev));
> >> +  mutex_unlock(_list_lock);
> >> +
> >> +  return parent;
> >> +}
> >
> > As we have demonstrated, all these refs and locks and release 
> > workqueue are not necessary,
> > as long as you have an independent device associated with the mdev 
> > host device
> > ("parent" device here).
> >
> 
>  I don't think every lock will go away with that. This also changes 
>  how
>  mdev devices entries are created in sysfs. It adds an extra 
>  directory.  
> >>>
> >>> Exposing the parent-child relationship through sysfs is a desirable
> >>> feature, so I'm not sure how this is a negative.  This part of Jike's
> >>> conversion was a big improvement, I thought.  Thanks,
> >>>  
> >>
> >> Jike's suggestion is to introduced a fake device over parent device 
> >> i.e.
> >> mdev-host, and then all mdev devices are children of 'mdev-host' not
> >> children of real parent.
> >>  
> >
> > It really depends on how you define 'real parent' :)
> >
> > With a physical-host-mdev hierarchy, the parent of mdev devices is the 
> > host
> > device, the parent of host device is the physical device. e.g.
> >
> > pdevmdev_host   mdev_device
> > dev >   parent  parent
> >
> > Figure 1: device hierarchy
> >   
> 
>  Right, mdev-host device doesn't represent physical device nor any mdev
>  device. Then what is the need of such device?
> >>>
> >>> Is there anything implicitly wrong with using a device node to host the
> >>> mdev child devices?  Is the argument against it only that it's
> >>> unnecessary?  Can we make use of the device-core parent/child
> >>> dependencies as Jike has done w/o that extra node?
> >>>
> >>
> >> I do feel that mdev core module would get simplified with the new sysfs
> >> interface without having extra node.  
> > 
> > Can you provide an example of why that is?
> >
> 
> 'online' or earlier 'start'/'stop' interface is removed and would add
> open() and close() callbacks. ref count from struct mdev_device and
> mdev_get_device()/mdev_put_device() were added for this interface, these
> would go away.
> Using device-core parent/child dependencies between physical device and
> mdev device, other functions would get simplified.

Yes, we've refined the interface over time and I'm glad that you're
incorporating the device-core simplifications, but this doesn't really
argue for or against the extra device node.
 
> >> For example, directory structure we have now is:
> >> /sys/bus/pci/devices/\:85\:00.0/
> >>
> >> mdev devices are in real parents directory.
> >>
> >> By introducing fake device it would be:
> >> /sys/bus/pci/devices/\:85\:00.0/mdev-host/
> >>
> >> mdev devices are in fake device's directory.
> >>  
> >
> > Yes, this is the wanted directory.
> >   
> 
>  I don't think so.
> >>>
> >>> Why?
> >>> 
> >>
> >> This directory is not mandatory. right?  
> > 
> > Clearly you've done an implementation without it, so it's not
> > functionally mandatory, but Jike has made significant code reduction
> > and simplification with it.  Those are desirable things.
> >   
> >> Lock would be still required, to handle the race conditions like
> >> 'mdev_create' is still in process and parent device is unregistered by
> >> vendor driver/ parent device is unbind from vendor driver.
> >>  
> >
> > locks are provided to protect resources, would you elaborate more on
> > what is the exact resource you want to protect by a lock in mdev_create?
> >   
> 
>  Simple, in your suggestion mdev-host device. Fake device will go away if
>  vendor driver unregisters the device from mdev 

Re: [Qemu-devel] [PATCH] Avoid additional GET_FEATURES call on vhost-user

[Felipe Franciosi]

Gentle ping.

Thanks,
Felipe

From: Marc-André Lureau 
Date: Thursday, 25 August 2016 at 12:14
To: Felipe Franciosi , "qemu-devel@nongnu.org" 
, "Michael S. Tsirkin" 
Subject: Re: [Qemu-devel] [PATCH] Avoid additional GET_FEATURES call on 
vhost-user

Hi

On Sat, Aug 20, 2016 at 3:28 AM Felipe Franciosi 
> wrote:
Vhost-user requires an early GET_FEATURES call to determine if the
slave supports protocol feature negotiation. An extra GET_FEATURES
call is made after vhost_backend_init() to actually set the device
features.

This patch moves the actual setting of the device features to both
implementations (kernel/user) of vhost_backend_init(), therefore
eliminating the need for a second call.

Signed-off-by: Felipe Franciosi >


Reviewed-by: Marc-André Lureau 
>

---
 hw/virtio/vhost-backend.c | 27 ++-
 hw/virtio/vhost-user.c|  1 +
 hw/virtio/vhost.c |  9 -
 3 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 7681f15..a519fe2 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -25,15 +25,6 @@ static int vhost_kernel_call(struct vhost_dev *dev, unsigned 
long int request,
 return ioctl(fd, request, arg);
 }

-static int vhost_kernel_init(struct vhost_dev *dev, void *opaque)
-{
-assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL);
-
-dev->opaque = opaque;
-
-return 0;
-}
-
 static int vhost_kernel_cleanup(struct vhost_dev *dev)
 {
 int fd = (uintptr_t) dev->opaque;
@@ -172,6 +163,24 @@ static int vhost_kernel_get_vq_index(struct vhost_dev 
*dev, int idx)
 return idx - dev->vq_index;
 }

+static int vhost_kernel_init(struct vhost_dev *dev, void *opaque)
+{
+uint64_t features;
+int r;
+
+assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL);
+
+dev->opaque = opaque;
+
+r = vhost_kernel_get_features(dev, );
+if (r < 0) {
+return r;
+}
+dev->features = features;
+
+return 0;
+}
+
 static const VhostOps kernel_ops = {
 .backend_type = VHOST_BACKEND_TYPE_KERNEL,
 .vhost_backend_init = vhost_kernel_init,
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b57454a..684f6d7 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -578,6 +578,7 @@ static int vhost_user_init(struct vhost_dev *dev, void 
*opaque)
 if (err < 0) {
 return err;
 }
+dev->features = features;

 if (virtio_has_feature(features, VHOST_USER_F_PROTOCOL_FEATURES)) {
 dev->backend_features |= 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 3d0c807..cb9870a 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1037,7 +1037,6 @@ static void vhost_virtqueue_cleanup(struct 
vhost_virtqueue *vq)
 int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type, uint32_t busyloop_timeout)
 {
-uint64_t features;
 int i, r, n_initialized_vqs = 0;

 hdev->migration_blocker = NULL;
@@ -1063,12 +1062,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 goto fail;
 }

-r = hdev->vhost_ops->vhost_get_features(hdev, );
-if (r < 0) {
-VHOST_OPS_DEBUG("vhost_get_features failed");
-goto fail;
-}
-
 for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) {
 r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i);
 if (r < 0) {
@@ -1086,8 +1079,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
 }
 }

-hdev->features = features;
-
 hdev->memory_listener = (MemoryListener) {
 .begin = vhost_begin,
 .commit = vhost_commit,
--
1.9.5

--
Marc-André Lureau

Re: [Qemu-devel] [PATCH v4 0/2] block: allow flush on devices with open tray

[Max Reitz]

On 19.09.2016 18:44, John Snow wrote:
> Final re-send for wording.
> 
> The move to blk_flush altered the behavior of migration and flushing
> nodes that are not reachable via the guest, but are still reachable
> via QEMU and may or may not need to be flushed.
> 
> This is likely the simplest solution for now until we nail down our
> policy a bit more.
> 
> This is intended for 2.6.2 and/or 2.7.1, to fix problems with libvirt
> et al being unable to migrate QEMU when the CDROM tray is open.
> 
> v4:
>  Commit message update.
> 
> v3:
>  Trying to take a hint from Kevin, reinstating bdrv_flush_all.
>  If it's not what we want, we can try moving back to v2,
>  acknowledging that a nicer solution in the future:
>  (A) Can skip flushing on devices that just don't need it, and
>  (B) Optionally institutes some sort of flush-on-eject policy.
> 
> 
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch atapi-tray-migfix
> https://github.com/jnsnow/qemu/tree/atapi-tray-migfix
> 
> This version is tagged atapi-tray-migfix-v4:
> https://github.com/jnsnow/qemu/releases/tag/atapi-tray-migfix-v4
> 
> John Snow (2):
>   block: reintroduce bdrv_flush_all
>   qemu: use bdrv_flush_all for vm_stop et al
> 
>  block/io.c| 25 +
>  cpus.c|  4 ++--
>  include/block/block.h |  1 +
>  3 files changed, 28 insertions(+), 2 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 1/2] block: sync bdrv_co_get_block_status_above() with bdrv_is_allocated_above()

[Eric Blake]

On 09/18/2016 11:37 PM, Denis V. Lunev wrote:
> On 09/19/2016 04:21 AM, Fam Zheng wrote:
>> On Thu, 09/15 19:34, Denis V. Lunev wrote:
>>> They should work very similar, covering same areas if backing store is
>>> shorter than the image. This change is necessary for the followup patch
>>> switching to bdrv_get_block_status_above() in mirror to avoid assert
>>> in check_block.
>>>
>>> This change should be made very carefully. Let us assume that we have
>>> top image and 2 backing stores L0->L1->L2.
>> Stupid question: which one is top and which are backing?
> L0 is top, L2 is at bottom.

I typically write this as:

L2 <- L1 <- L0

(read "L2 backs L1, which in turn backs L0") with the active on the
right.  So I understand the confusion in Fam's question where you were
using the opposite direction.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Kirti Wankhede]

Hi libvirt experts,

Thanks for valuable input on v1 version of RFC.

Quick brief, VFIO based mediated device framework provides a way to
virtualize their devices without SR-IOV, like NVIDIA vGPU, Intel KVMGT
and IBM's channel IO. This framework reuses VFIO APIs for all the
functionalities for mediated devices which are currently being used for
pass through devices. This framework introduces a set of new sysfs files
for device creation and its life cycle management.

Here is the summary of discussion on v1:
1. Discover mediated device:
As part of physical device initialization process, vendor driver will
register their physical devices, which will be used to create virtual
device (mediated device, aka mdev) to the mediated framework.

Vendor driver should specify mdev_supported_types in directory format.
This format is class based, for example, display class directory format
should be as below. We need to define such set for each class of devices
which would be supported by mediated device framework.

 --- mdev_destroy
 --- mdev_supported_types
 |-- 11
 |   |-- create
 |   |-- name
 |   |-- fb_length
 |   |-- resolution
 |   |-- heads
 |   |-- max_instances
 |   |-- params
 |   |-- requires_group
 |-- 12
 |   |-- create
 |   |-- name
 |   |-- fb_length
 |   |-- resolution
 |   |-- heads
 |   |-- max_instances
 |   |-- params
 |   |-- requires_group
 |-- 13
 |-- create
 |-- name
 |-- fb_length
 |-- resolution
 |-- heads
 |-- max_instances
 |-- params
 |-- requires_group


In the above example directory '11' represents a type id of mdev device.
'name', 'fb_length', 'resolution', 'heads', 'max_instance' and
'requires_group' would be Read-Only files that vendor would provide to
describe about that type.

'create':
Write-only file. Mandatory.
Accepts string to create mediated device.

'name':
Read-Only file. Mandatory.
Returns string, the name of that type id.

'fb_length':
Read-only file. Mandatory.
Returns {K,M,G}, size of framebuffer.

'resolution':
Read-Only file. Mandatory.
Returns 'hres x vres' format. Maximum supported resolution.

'heads':
Read-Only file. Mandatory.
Returns integer. Number of maximum heads supported.

'max_instance':
Read-Only file. Mandatory.
Returns integer.  Returns maximum mdev device could be created
at the moment when this file is read. This count would be updated by
vendor driver. Before creating mdev device of this type, check if
max_instance is > 0.

'params'
Write-Only file. Optional.
String input. Libvirt would pass the string given in XML file to
this file and then create mdev device. Set empty string to clear params.
For example, set parameter 'frame_rate_limiter=0' to disable frame rate
limiter for performance benchmarking, then create device of type 11. The
device created would have that parameter set by vendor driver.

'requires_group'
Read-Only file. Optional.
This should be provided by vendor driver if vendor driver need to
group mdev devices in one domain so that vendor driver can use 'first
open' to commit resources of all mdev devices associated to that domain
and 'last close' to free those.

The parent device would look like:

   
 pci__86_00_0
 
   0
   134
   0
   0
   
 
 
   
   GRID M60-0B
   512M
   2560x1600
   2
   16
   1
 
   
   GRID M60
   NVIDIA
 
   

2. Create/destroy mediated device

With above example, vGPU device XML would look like:

   
 my-vgpu
 pci__86_00_0
 
   
   1
   'frame_rate_limiter=0'
 
   

'type id' is mandatory.
'group' is optional. It should be a unique number in the system among
all the groups created for mdev devices. Its usage is:
  - not needed if single vGPU device is being assigned to a domain.
  - only need to be set if multiple vGPUs need to be assigned to a
domain and vendor driver have 'requires_group' file in type id directory.
  - if type id directory include 'requires_group' and user tries to
assign multiple vGPUs to a domain without having  field in XML,
it will create single vGPU.

'params' is optional field. User should set this field if extra
parameters need to be set for a particular vGPU device. Libvirt don't
need to parse these params. These are meant for vendor driver.

Libvirt need to follow the sequence to create device:
* Read /sys/../\:86\:00.0/11/max_instances. If it is greater than 0,
then only proceed else fail.

* Set extra params if 'params' field exist in device XML and 'params'
file exist in type id directory

echo "frame_rate_limiter=0" > /sys/../\:86\:00.0/11/params

* Autogenerate UUID
* Create device:

echo "$UUID:" > /sys/../\:86\:00.0/11/create

where  is optional. Group should be unique number among all
the 

[Qemu-devel] [Bug 1625295] Re: qemu-arm dies with libarmmem inside ld.so.preload

[Peter Maydell]

Which version of QEMU are you using? This is I think due to SETEND
emulation, which I thought we had implemented now.

If this still doesn't work on QEMU 2.7, please can you provide full
instructions to reproduce the problem (assume I know nothing about how
to get raspbian or run it on QEMU).

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

Title:
  qemu-arm dies with libarmmem inside ld.so.preload

Status in QEMU:
  New

Bug description:
  When running raspbian inside qemu,the user has to first comment out
  the following line from /etc/ld.so.conf:

  /usr/lib/arm-linux-gnueabihf/libarmmem.so

  
  Will future qemus will be able to work without changine /etc/ld.so.conf ?

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

Re: [Qemu-devel] [PATCH v8 10/12] uuid: Tighten uuid parse

[Eric Blake]

On 09/17/2016 11:25 PM, Fam Zheng wrote:
> sscanf is relatively loose (tolerate) on some invalid formats that we
> should fail instead of generating a wrong uuid structure, like with
> whitespaces and short strings.
> 
> Add and use a helper function to first check the format.
> 
> Signed-off-by: Fam Zheng 
> ---
>  util/uuid.c | 24 +++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 

>  
> +static bool qemu_uuid_is_valid(const char *str)
> +{
> +int i;
> +
> +for (i = 0; i < strlen(str); i++) {
> +const char c = str[i];
> +if (i == 8 || i == 13 || i == 18 || i == 23) {
> +if (str[i] != '-') {
> +return false;
> +}
> +} else {
> +if ((c >= '0' && c <= '9') ||
> +(c >= 'A' && c <= 'F') ||
> +(c >= 'a' && c <= 'f')) {
> +continue;
> +}
> +return false;
> +}
> +}
> +return i == 36;
> +}

Quite verbose, compared to my earlier suggestion of just checking that
all bytes in the string are valid (but not worrying about positions,
because sscanf mostly does that):

 strspn(str, "0123456789abcdefABCDEF-") == 36 && !str[36]

and then tightening sscanf() (now that we've rejected whitespace via
strspn(), all that remains is to ensure we parsed as much as we were
expecting), as in:

 sscanf(str, UUID_FMT "%n", [0], ... [15], )

and then validating that len == 36.

But while my approach is a (cryptic) three-line change, yours is easier
to check that it is obviously correct.  So unless you want to respin
because you like playing golf when writing C expressions,

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] [PULL v2 0/8] Merge qcrypto 2016/09/19

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Type: series
Subject: [Qemu-devel] [PULL v2 0/8] Merge qcrypto 2016/09/19
Message-id: 1474299237-1054-1-git-send-email-berra...@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
e82628e crypto: add trace points for TLS cert verification
9d93d38 crypto: support more hash algorithms for pbkdf
08053a6 crypto: increase default pbkdf2 time for luks to 2 seconds
657e1cd crypto: remove bogus /= 2 for pbkdf iterations
d3206e1 crypto: use correct derived key size when timing pbkdf
435e4e2 crypto: clear out buffer after timing pbkdf algorithm
cce8751 crypto: make PBKDF iterations configurable for LUKS format
2f624b1 crypto: use uint64_t for pbkdf iteration count parameters

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
=== OUTPUT END ===

Abort: command timeout (>3600 seconds)


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

Re: [Qemu-devel] [PATCH v7 1/4] vfio: Mediated device Core driver

[Kirti Wankhede]

On 9/19/2016 11:41 PM, Alex Williamson wrote:
> On Mon, 19 Sep 2016 22:59:34 +0530
> Kirti Wankhede  wrote:
> 
>> On 9/12/2016 9:23 PM, Alex Williamson wrote:
>>> On Mon, 12 Sep 2016 13:19:11 +0530
>>> Kirti Wankhede  wrote:
>>>   
 On 9/12/2016 10:40 AM, Jike Song wrote:  
> On 09/10/2016 03:55 AM, Kirti Wankhede wrote:
>> On 9/10/2016 12:12 AM, Alex Williamson wrote:
>>> On Fri, 9 Sep 2016 23:18:45 +0530
>>> Kirti Wankhede  wrote:
>>>
>> +
>> +static struct parent_device *mdev_get_parent_from_dev(struct device 
>> *dev)
>> +{
>> +struct parent_device *parent;
>> +
>> +mutex_lock(_list_lock);
>> +parent = mdev_get_parent(__find_parent_device(dev));
>> +mutex_unlock(_list_lock);
>> +
>> +return parent;
>> +}  
>
> As we have demonstrated, all these refs and locks and release 
> workqueue are not necessary,
> as long as you have an independent device associated with the mdev 
> host device
> ("parent" device here).
>  

 I don't think every lock will go away with that. This also changes how
 mdev devices entries are created in sysfs. It adds an extra directory. 

>>>
>>> Exposing the parent-child relationship through sysfs is a desirable
>>> feature, so I'm not sure how this is a negative.  This part of Jike's
>>> conversion was a big improvement, I thought.  Thanks,
>>>
>>
>> Jike's suggestion is to introduced a fake device over parent device i.e.
>> mdev-host, and then all mdev devices are children of 'mdev-host' not
>> children of real parent.
>>
>
> It really depends on how you define 'real parent' :)
>
> With a physical-host-mdev hierarchy, the parent of mdev devices is the 
> host
> device, the parent of host device is the physical device. e.g.
>
> pdevmdev_host   mdev_device
> dev   parent  parent
>
> Figure 1: device hierarchy
> 

 Right, mdev-host device doesn't represent physical device nor any mdev
 device. Then what is the need of such device?  
>>>
>>> Is there anything implicitly wrong with using a device node to host the
>>> mdev child devices?  Is the argument against it only that it's
>>> unnecessary?  Can we make use of the device-core parent/child
>>> dependencies as Jike has done w/o that extra node?
>>>  
>>
>> I do feel that mdev core module would get simplified with the new sysfs
>> interface without having extra node.
> 
> Can you provide an example of why that is?
>  

'online' or earlier 'start'/'stop' interface is removed and would add
open() and close() callbacks. ref count from struct mdev_device and
mdev_get_device()/mdev_put_device() were added for this interface, these
would go away.
Using device-core parent/child dependencies between physical device and
mdev device, other functions would get simplified.

>> For example, directory structure we have now is:
>> /sys/bus/pci/devices/\:85\:00.0/
>>
>> mdev devices are in real parents directory.
>>
>> By introducing fake device it would be:
>> /sys/bus/pci/devices/\:85\:00.0/mdev-host/
>>
>> mdev devices are in fake device's directory.
>>
>
> Yes, this is the wanted directory.
> 

 I don't think so.  
>>>
>>> Why?
>>>   
>>
>> This directory is not mandatory. right?
> 
> Clearly you've done an implementation without it, so it's not
> functionally mandatory, but Jike has made significant code reduction
> and simplification with it.  Those are desirable things.
> 
>> Lock would be still required, to handle the race conditions like
>> 'mdev_create' is still in process and parent device is unregistered by
>> vendor driver/ parent device is unbind from vendor driver.
>>
>
> locks are provided to protect resources, would you elaborate more on
> what is the exact resource you want to protect by a lock in mdev_create?
> 

 Simple, in your suggestion mdev-host device. Fake device will go away if
 vendor driver unregisters the device from mdev module, right.  
>>>
>>> I don't follow the reply here, but aiui there's ordering implicit in
>>> the device core that Jike is trying to take advantage of that
>>> simplifies the mdev layer significantly.  In the case of an
>>> mdev_create, the device core needs to take a reference to the parent
>>> object, the mdev-host I'd guess in Jike's version, the created mdev
>>> device would also have a reference to that object, so the physical host
>>> device could not be removed so long as there are outstanding
>>> references.  It's just a matter of managing 

[Qemu-devel] MAINTAINERS: Add some SPARC machine related files

[Thomas Huth]

And while we're at it, remove Blue Swirl from the list
of maintainers. Blue has apparently been inactive for
quite a while now, so I assume he's unfortunately
not available as maintainer anymore.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index a9fab46..c6297c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -650,22 +650,27 @@ F: hw/sh4/shix.c
 SPARC Machines
 --
 Sun4m
-M: Blue Swirl 
 M: Mark Cave-Ayland 
 S: Maintained
 F: hw/sparc/sun4m.c
+F: hw/dma/sparc32_dma.c
+F: hw/dma/sun4m_iommu.c
+F: include/hw/sparc/sparc32_dma.h
+F: include/hw/sparc/sun4m.h
+F: pc-bios/openbios-sparc32
 
 Sun4u
-M: Blue Swirl 
 M: Mark Cave-Ayland 
 S: Maintained
 F: hw/sparc64/sun4u.c
+F: pc-bios/openbios-sparc64
 
 Leon3
 M: Fabien Chouteau 
 S: Maintained
 F: hw/sparc/leon3.c
 F: hw/*/grlib*
+F: include/hw/sparc/grlib.h
 
 S390 Machines
 -
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/6] iotests: throw away test timings if args change

[Eric Blake]

On 09/19/2016 12:35 PM, Daniel P. Berrange wrote:
> The 'check' program records timings for each test that
> is run. These timings are only valid, however, for a
> particular format/protocol combination. So if frequently
> running 'check' with a variety of different formats or
> protocols, the times printed can be very misleading.
> 
> Record the protocol/format in the check.time file and
> throw it away if it doesn't mach the current run args.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  tests/qemu-iotests/check | 21 +++--
>  1 file changed, 19 insertions(+), 2 deletions(-)

Rather than completely throwing things away, would it be worth updating
the check.time file format to track multiple entries? For every distinct
args seen, track a timing for that combination of args, then when
starting a test, if a line in the file contains the current args, report
that old time; if not, then append a line with the new args.  The file
grows according to how many distinct args combinations you use, and it's
probably easier to make 'a b' and 'b a' report as different timings even
if they have the same effect and could share a timing.  We'd also want
an operation to clean out timings without running tests, particularly if
timings can otherwise grow huge due to every possible args combination.

What do you think?

-- 
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 v7 2/4] vfio: VFIO driver for mediated devices

[Alex Williamson]

On Tue, 20 Sep 2016 00:43:15 +0530
Kirti Wankhede  wrote:

> On 9/20/2016 12:06 AM, Alex Williamson wrote:
> > On Mon, 19 Sep 2016 23:52:36 +0530
> > Kirti Wankhede  wrote:
> >   
> >> On 8/26/2016 7:43 PM, Kirti Wankhede wrote:  
> >>> * PGP Signed: 08/26/2016 at 07:15:44 AM, Decrypted
> >>> On 8/25/2016 2:52 PM, Dong Jia wrote:
>  On Thu, 25 Aug 2016 09:23:53 +0530
> >>  
> > +
> > +static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
> > + size_t count, loff_t *ppos)
> > +{
> > +   struct vfio_mdev *vmdev = device_data;
> > +   struct mdev_device *mdev = vmdev->mdev;
> > +   struct parent_device *parent = mdev->parent;
> > +   unsigned int done = 0;
> > +   int ret;
> > +
> > +   if (!parent->ops->read)
> > +   return -EINVAL;
> > +
> > +   while (count) {
>  Here, I have to say sorry to you guys for that I didn't notice the
>  bad impact of this change to my patches during the v6 discussion.
> 
>  For vfio-ccw, I introduced an I/O region to input/output I/O
>  instruction parameters and results for Qemu. The @count of these data
>  currently is 140. So supporting arbitrary lengths in one shot here, and
>  also in vfio_mdev_write, seems the better option for this case.
> 
>  I believe that if the pci drivers want to iterate in a 4 bytes step, you
>  can do that in the parent read/write callbacks instead.
> 
>  What do you think?
> 
> >>>
> >>> I would like to know Alex's thought on this. He raised concern with this
> >>> approach in v6 reviews:
> >>> "But I think this is exploitable, it lets the user make the kernel
> >>> allocate an arbitrarily sized buffer."
> >>> 
> >>
> >> Read/write callbacks are for slow path, emulation of mmio region which
> >> are mainly device registers. I do feel it shouldn't support arbitrary
> >> lengths.
> >> Alex, I would like to know your thoughts.  
> > 
> > The exploit was that the mdev layer allocated a buffer and copied the
> > entire user buffer into kernel space before passing it to the vendor
> > driver.  The solution is to pass the __user *buf to the vendor driver
> > and let them sanitize and split it however makes sense for their
> > device.  We shouldn't be assuming naturally aligned, up to dword
> > accesses in the generic mdev layers.  Those sorts of semantics are
> > defined by the device type.  This is another case where making
> > the mdev layer as thin as possible is actually the best thing to
> > do to make it really device type agnostic.  Thanks,
> >   
> 
> 
> Alex,
> 
> These were the comments on v6 patch:
> 
> >>> Do we really need to support arbitrary lengths in one shot?  Seems
> >>> like
> >>> we could just use a 4 or 8 byte variable on the stack and iterate
> >>> until
> >>> done.
> >>>  
> >>
> >> We just want to pass the arguments to vendor driver as is here.Vendor
> >> driver could take care of that.  
> 
> > But I think this is exploitable, it lets the user make the kernel
> > allocate an arbitrarily sized buffer.  
> 
> As per above discussion in v7 version, this module don't allocated
> memory from heap.
> 
> If vendor driver allocates arbitrary memory in kernel space through mdev
> module interface, isn't that would be exploit?

Yep, my 4-8/byte chunking idea was too PCI specific.  If a vendor
driver introduces an exploit, that's a bug in the vendor driver.  I'm
not sure if we can or should attempt to guard against that.  Ultimately
the vendor driver is either open source and we can inspect it for such
exploits or it's closed source, taints the kernel, and we hope for the
best.  It might make a good unit test to perform substantially sized
reads/writes to the mdev device.  Perhaps the only sanity test we can
make in the core is to verify the access doesn't exceed the size of
the region as advertised by the vendor driver.  Thanks,

Alex

Re: [Qemu-devel] [PATCH v2 07/10] block: Accept device model name for eject

[Eric Blake]

On 09/19/2016 11:54 AM, Kevin Wolf wrote:
> In order to remove the necessity to use BlockBackend names in the
> external API, we want to allow qdev device names in all device related
> commands.
> 
> This converts eject to accept a qdev device name.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c  | 10 +++---
>  hmp.c   |  2 +-
>  qapi/block.json |  9 +++--
>  qmp-commands.hx | 10 ++
>  4 files changed, 21 insertions(+), 10 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

[Qemu-devel] [PATCH] hw/arm: Fix Integrator/CP memsz initialization

[Jakub Jermář]

* Do not assume memsz is already initialized in integratorcm_init
* Calculate memsz directly from MachineState
* Get rid of the now unused memsz property

Signed-off-by: Jakub Jermar 
---
 hw/arm/integratorcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 96dc150..3d88369 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -247,7 +247,9 @@ static void integratorcm_init(Object *obj)
 {
 IntegratorCMState *s = INTEGRATOR_CM(obj);
 SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+MachineState *machine = MACHINE(qdev_get_machine());
 
+s->memsz = machine->ram_size >> 20;
 s->cm_osc = 0x0148;
 /* ??? What should the high bits of this value be?  */
 s->cm_auxosc = 0x0007feff;
@@ -574,7 +576,6 @@ static void integratorcp_init(MachineState *machine)
 memory_region_add_subregion(address_space_mem, 0x8000, ram_alias);
 
 dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
-qdev_prop_set_uint32(dev, "memsz", ram_size >> 20);
 qdev_init_nofail(dev);
 sysbus_mmio_map((SysBusDevice *)dev, 0, 0x1000);
 
@@ -624,7 +625,6 @@ static void integratorcp_machine_init(MachineClass *mc)
 DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
 
 static Property core_properties[] = {
-DEFINE_PROP_UINT32("memsz", IntegratorCMState, memsz, 0),
 DEFINE_PROP_END_OF_LIST(),
 };
 

Re: [Qemu-devel] [PATCH v2 08/10] block: Accept device model name for blockdev-change-medium

[Eric Blake]

On 09/19/2016 11:54 AM, Kevin Wolf wrote:
> In order to remove the necessity to use BlockBackend names in the
> external API, we want to allow qdev device names in all device related
> commands.
> 
> This converts blockdev-change-medium to accept a qdev device name.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c   | 18 +++---
>  hmp.c|  5 +++--
>  qapi/block-core.json |  8 ++--
>  qmp-commands.hx  | 12 +++-
>  qmp.c|  4 ++--
>  5 files changed, 29 insertions(+), 18 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 06/10] block: Accept device model name for x-blockdev-remove-medium

[Eric Blake]

On 09/19/2016 11:54 AM, Kevin Wolf wrote:
> In order to remove the necessity to use BlockBackend names in the
> external API, we want to allow qdev device names in all device related
> commands.
> 
> This converts x-blockdev-remove-medium to accept a qdev device name.
> 
> As the command is experimental, we can still remove the 'device' option
> that uses the BlockBackend name. This requires some test case changes
> and is left for another series.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c   | 28 
>  qapi/block-core.json |  7 +--
>  qmp-commands.hx  | 14 --
>  3 files changed, 29 insertions(+), 20 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 09/10] block: Accept device model name for block_set_io_throttle

[Eric Blake]

On 09/19/2016 11:54 AM, Kevin Wolf wrote:
> In order to remove the necessity to use BlockBackend names in the
> external API, we want to allow qdev device names in all device related
> commands.
> 
> This converts block_set_io_throttle to accept a qdev device name.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c   | 12 +++-
>  qapi/block-core.json |  8 +---
>  qmp-commands.hx  |  8 +---
>  3 files changed, 17 insertions(+), 11 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

[Qemu-devel] [Bug 1624896] Re: [PPC] SegFault due to Stack Overflow in E500

[T. Huth]

** Tags added: ppc

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

Title:
  [PPC] SegFault due to Stack Overflow in E500

Status in QEMU:
  New

Bug description:
  
  I am getting a Segmentation Fault while simulating a PowerPC e500. I've tried 
to debug the problem and I've found that it occurs when you have a 0 value 
decrementer. The function trace is the following:

  1) __cpu_ppc_store_decr (ppc.c) is called with value = 0 and 
raise_excp=booke_decr_cb;
  2) Since value < 3, booke_decr_cb is called;
  3) booke_decr_cb then calls booke_update_irq() and cpu_ppc_store_decr();
  4) cpu_ppc_store_decr calls __cpu_ppc_store_decr

  You're stuck on this infinite cycle until your stack overflows
  eventually.

  Command Line:
  qemu-system-ppc -cpu e500v2 -d guest_errors,unimp -m 2048 -M ppce500 
-nographic -bios ../cc/share/qem
  u/u-boot.e500 -kernel XKYAPP.exe

  Platform where the bug occured: Bash ubuntu on Windows;

  Revision where the bug was found:
  e3571ae30cd26d19efd4554c25e32ef64d6a36b3 (16 Set 2016)


  Thanks!

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

[Qemu-devel] [PATCH v3 0/5] Add runnability info to query-cpu-definitions

[Eduardo Habkost]

This series extends query-cpu-definitions to include an extra
field: "unavailable-features". The new field can be used to find
out reasons that prevent the CPU model from running in the
current host.

This will return information based on the current machine and
accelerator only. In the future we may extend these mechanisms to
allow querying other machines and other accelerators without
restarting QEMU, but it will require some reorganization of
QEMU's main code.

Changes v2 -> v3:
* Small documentation reword
  * Suggested-by: Markus Armbruster 
* Create a x86_cpu_feature_name() function, to
  isolate the code that returns the property name

Changes v1 -> v2:
* Fixed documentation to say "(since 2.7)"
* Removed @runnable field, improved documentation

Example command output:

{ "return": [
{
"unavailable-features": [],
"static": false,
"name": "host"
},
{
"unavailable-features": [],
"static": false,
"name": "qemu64"
},
{
"unavailable-features": [],
"static": false,
"name": "qemu32"
},
{
"unavailable-features": ["npt", "sse4a", "3dnow", "3dnowext", 
"fxsr-opt", "mmxext"],
"static": false,
"name": "phenom"
},
{
"unavailable-features": [],
"static": false,
"name": "pentium3"
},
{
"unavailable-features": [],
"static": false,
"name": "pentium2"
},
{
"unavailable-features": [],
"static": false,
"name": "pentium"
},
{
"unavailable-features": [],
"static": false,
"name": "n270"
},
{
"unavailable-features": [],
"static": false,
"name": "kvm64"
},
{
"unavailable-features": [],
"static": false,
"name": "kvm32"
},
{
"unavailable-features": [],
"static": false,
"name": "coreduo"
},
{
"unavailable-features": [],
"static": false,
"name": "core2duo"
},
{
"unavailable-features": ["3dnow", "3dnowext", "mmxext"],
"static": false,
"name": "athlon"
},
{
"unavailable-features": [],
"static": false,
"name": "Westmere"
},
{
"unavailable-features": ["xgetbv1", "xsavec", "3dnowprefetch", 
"smap", "adx", "rdseed", "mpx", "rtm", "hle"],
"static": false,
"name": "Skylake-Client"
},
{
"unavailable-features": [],
"static": false,
"name": "SandyBridge"
},
{
"unavailable-features": [],
"static": false,
"name": "Penryn"
},
{
"unavailable-features": ["tbm", "fma4", "xop", "3dnowprefetch", 
"misalignsse", "sse4a"],
"static": false,
"name": "Opteron_G5"
},
{
"unavailable-features": ["fma4", "xop", "3dnowprefetch", 
"misalignsse", "sse4a"],
"static": false,
"name": "Opteron_G4"
},
{
"unavailable-features": ["misalignsse", "sse4a"],
"static": false,
"name": "Opteron_G3"
},
{
"unavailable-features": [],
"static": false,
"name": "Opteron_G2"
},
{
"unavailable-features": [],
"static": false,
"name": "Opteron_G1"
},
{
"unavailable-features": [],
"static": false,
"name": "Nehalem"
},
{
"unavailable-features": [],
"static": false,
"name": "IvyBridge"
},
{
"unavailable-features": ["rtm", "hle"],
"static": false,
"name": "Haswell"
},
{
"unavailable-features": [],
"static": false,
"name": "Haswell-noTSX"
},
{
"unavailable-features": [],
"static": false,
"name": "Conroe"
},
{
"unavailable-features": ["3dnowprefetch", "smap", "adx", "rdseed", 
"rtm", "hle"],
"static": false,
"name": "Broadwell"
},
{
"unavailable-features": ["3dnowprefetch", "smap", "adx", "rdseed"],
"static": false,
"name": "Broadwell-noTSX"
},
{
"unavailable-features": [],
"static": false,
"name": "486"
}
]}

Cc: David Hildenbrand 
Cc: Michael Mueller 
Cc: Christian Borntraeger 

Re: [Qemu-devel] [Xen-devel] stack size limit issues with xen + qemu + rbd

[Konrad Rzeszutek Wilk]

On Fri, Sep 16, 2016 at 04:55:17PM -0400, Chris Patterson wrote:
> I have spent some time investigating a case where qemu is failing to
> register xenstore watches for a PV guest once I enable vfb (and
> thereby triggering the creation of a qemu instance).
> 
> The qemu logs show something along the lines of:
> xen be core: xen be core: xen be: watching backend path
> (backend/console/3) failed
> xen be: watching backend path (backend/console/3) failed
> xen be core: xen be core: xen be: watching backend path (backend/vkbd/3) 
> failed
> xen be: watching backend path (backend/vkbd/3) failed
> xen be core: xen be core: xen be: watching backend path (backend/qdisk/3) 
> failed
> xen be: watching backend path (backend/qdisk/3) failed
> xen be core: xen be core: xen be: watching backend path (backend/qusb/3) 
> failed
> xen be: watching backend path (backend/qusb/3) failed
> xen be core: xen be core: xen be: watching backend path (backend/vfb/3) failed
> xen be: watching backend path (backend/vfb/3) failed
> xen be core: xen be core: xen be: watching backend path (backend/qnic/3) 
> failed
> xen be: watching backend path (backend/qnic/3) failed
> 
> I have tested qemu master, qemu-xen in the master xen tree, as well as
> a few tags all with the same issue.
> 
> I came across a similar issue reported by Juergen Gross:
> https://lists.nongnu.org/archive/html/qemu-devel/2016-07/msg03341.html
> 
> Sure enough, the thread stack size was the culprit.  I had been
> testing with qemu with the associated fix "vnc-tight: fix regression
> with libxenstore" as it is in master, so that wasn't it...
> 
> I did some basic analysis of the qemu binary and the libraries it is pulling 
> in:
> 
> for lib in $(ldd /usr/local/bin/qemu-system-i386 | grep -o '/.* '); do
> echo "lib=$lib"; readelf -S "$lib" | grep -e tbss -e tdata -A1 ; done
> 
> The largest consumers were:
> lib=/usr/lib/x86_64-linux-gnu/librbd.so.1
>   [17] .tbss NOBITS   0088fed0  0068fed0
>1820   WAT   0 0 8
> lib=/usr/lib/x86_64-linux-gnu/librados.so.2
>   [17] .tbss NOBITS   00718600  00518600
>0aa0   WAT   0 0 8
> 
> IIUC, librbd & librados are using nearly 9k of the 16k alone (I am
> assuming this thread-local storage must be consumed as part of the
> thread's stack)?
> 
> I narrowed that down to Ceph's usage of __thread in stringify() in
> src/include/stringify.h.
> 
> To make things functional, the options were either to:
> (a) disable rbd at configure time for qemu
> (b) reduce the level of thread-local storage in dependencies
> (particularly ceph's stringify)
> (c) increase the stack size specified in xenstore's xs.c


I would say c) for now and focus on b) long-term until c) can be
reverted?

> 
> Is there is any precedent/policy with regards to expected TLS and/or
> stack usage for dependencies?  Is the best course of action (b)? Or

No precendent/policy that I know of..

> perhaps reconsider the default size for (c)?
> 
> Thoughts? :)
> 
> ___
> Xen-devel mailing list
> xen-de...@lists.xen.org
> https://lists.xen.org/xen-devel

[Qemu-devel] [PATCH v3 2/5] target-i386: Move warning code outside x86_cpu_filter_features()

[Eduardo Habkost]

x86_cpu_filter_features() will be reused by code that shouldn't
print any warning. Move the warning code to a new
x86_cpu_report_filtered_features() function, and call it from
x86_cpu_realizefn().

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 24682aa..1a42233 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2218,9 +2218,6 @@ static int x86_cpu_filter_features(X86CPU *cpu)
 env->features[w] &= host_feat;
 cpu->filtered_features[w] = requested_features & ~env->features[w];
 if (cpu->filtered_features[w]) {
-if (cpu->check_cpuid || cpu->enforce_cpuid) {
-report_unavailable_features(w, cpu->filtered_features[w]);
-}
 rv = 1;
 }
 }
@@ -2228,6 +2225,15 @@ static int x86_cpu_filter_features(X86CPU *cpu)
 return rv;
 }
 
+static void x86_cpu_report_filtered_features(X86CPU *cpu)
+{
+FeatureWord w;
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+report_unavailable_features(w, cpu->filtered_features[w]);
+}
+}
+
 static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
 {
 PropValue *pv;
@@ -3042,12 +3048,16 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 env->cpuid_level = 7;
 }
 
-if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
-error_setg(_err,
-   kvm_enabled() ?
-   "Host doesn't support requested features" :
-   "TCG doesn't support requested features");
-goto out;
+if (x86_cpu_filter_features(cpu) &&
+(cpu->check_cpuid || cpu->enforce_cpuid)) {
+x86_cpu_report_filtered_features(cpu);
+if (cpu->enforce_cpuid) {
+error_setg(_err,
+   kvm_enabled() ?
+   "Host doesn't support requested features" :
+   "TCG doesn't support requested features");
+goto out;
+}
 }
 
 /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 10/10] qemu-iotests/118: Test media change with qdev name

[Eric Blake]

On 09/19/2016 11:54 AM, Kevin Wolf wrote:
> We just added the option to use qdev device names in all device related
> block QMP commands. This patch converts some of the test cases in 118 to
> use qdev device names instead of BlockBackend names to cover the new
> way. It converts cases for each of the media change commands, but only
> for CD-ROM and not everywhere, so that the old way is still tested, too.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  tests/qemu-iotests/118| 85 
> ++-
>  tests/qemu-iotests/iotests.py |  5 +++
>  2 files changed, 73 insertions(+), 17 deletions(-)
> 

> @@ -90,7 +99,10 @@ class GeneralChangeTestsBaseClass(ChangeBaseClass):
>  self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
>  
>  def test_eject(self):
> -result = self.vm.qmp('eject', device='drive0', force=True)
> +if self.device_name is not None:

I guess writing it like this instead of "if not self.device_name:"
allows us to write a test where self.device_name is "", if we wanted to
make sure that error message is sane.  Probably not worth changing
anything based on that observation, so keep the patch as-is, and add:

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

[Qemu-devel] [PATCH v3 5/5] target-i386: Return runnability information on query-cpu-definitions

[Eduardo Habkost]

Fill the "unavailable-features" field on the x86 implementation
of query-cpu-definitions.

Cc: Jiri Denemark 
Cc: libvir-l...@redhat.com
Signed-off-by: Eduardo Habkost 
---
Changes v2 -> v3:
* Create a x86_cpu_feature_name() function, to
  isolate the code that returns the property name

Changes v1 -> v2:
* Updated to the new schema: no @runnable field, and
  always report @unavailable-features as present
---
 target-i386/cpu.c | 55 +++
 1 file changed, 55 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a755309..9b53ce5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1987,6 +1987,24 @@ static inline void feat2prop(char *s)
 }
 }
 
+/* Return the feature property name for a feature flag bit */
+static char *x86_cpu_feature_name(FeatureWord w, int bitnr)
+{
+char **parts, *r;
+
+/*TODO: This can be simplified later, by:
+ * 1) Moving the aliases outside the feat_names array
+ *(making g_strsplit() unnecessary)
+ * 2) Replacing "_" with "-" on all entries
+ *(making feat2prop() unnecessary)
+ */
+parts = g_strsplit(feature_word_info[w].feat_names[bitnr], "|", 2);
+r = g_strdup(parts[0]);
+feat2prop(r);
+g_strfreev(parts);
+return r;
+}
+
 /* Compatibily hack to maintain legacy +-feat semantic,
  * where +-feat overwrites any feature set by
  * feat=on|feat even if the later is parsed after +-feat
@@ -2105,6 +2123,41 @@ static void x86_cpu_report_filtered_features(X86CPU *cpu)
 }
 }
 
+/* Check for missing features that may prevent the CPU class from
+ * running using the current machine and accelerator.
+ */
+static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
+ strList **missing_feats)
+{
+X86CPU *xc;
+FeatureWord w;
+
+if (xcc->kvm_required && !kvm_enabled()) {
+strList *new = g_new0(strList, 1);
+new->value = g_strdup("kvm");;
+*missing_feats = new;
+return;
+}
+
+xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc;
+if (x86_cpu_filter_features(xc)) {
+for (w = 0; w < FEATURE_WORDS; w++) {
+uint32_t filtered = xc->filtered_features[w];
+int i;
+for (i = 0; i < 32; i++) {
+if (filtered & (1UL << i)) {
+strList *new = g_new0(strList, 1);
+new->value = x86_cpu_feature_name(w, i);
+new->next = *missing_feats;
+*missing_feats = new;
+}
+}
+}
+}
+
+object_unref(OBJECT(xc));
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2197,6 +2250,8 @@ static void x86_cpu_definition_entry(gpointer data, 
gpointer user_data)
 
 info = g_malloc0(sizeof(*info));
 info->name = x86_cpu_class_get_model_name(cc);
+x86_cpu_class_check_missing_features(cc, >unavailable_features);
+info->has_unavailable_features = true;
 
 entry = g_malloc0(sizeof(*entry));
 entry->value = info;
-- 
2.7.4

[Qemu-devel] [PATCH v3 4/5] qmp: Add runnability information to query-cpu-definitions

[Eduardo Habkost]

Add a new optional field to query-cpu-definitions schema:
"unavailable-features". It will contain a list of QOM properties
that prevent the CPU model from running in the current host.

Cc: David Hildenbrand 
Cc: Michael Mueller 
Cc: Christian Borntraeger 
Cc: Cornelia Huck 
Cc: Jiri Denemark 
Cc: libvir-l...@redhat.com
Signed-off-by: Eduardo Habkost 
---
Changes v2 -> v3:
* Small documentation reword
  * Suggested-by: Markus Armbruster 

Changes v1 -> v2:
* Remove @runnable field, non-empty @unavailable-features is
  enough to report CPU model as not runnable.
* Documentation updates:
  * Changed to "(since 2.7)";
  * Add more details about the exact meaning of
unavailable-features, and what it would mean to see
read-only QOM properties in the list, and that
implementations can return "type" if there's
no extra information available;
---
 qapi-schema.json | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index e507061..057c63d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3111,10 +3111,31 @@
 #  QEMU version, machine type, machine options and accelerator options.
 #  A static model is always migration-safe. (since 2.8)
 #
+# @unavailable-features: #optional List of properties that prevent
+#the CPU model from running in the current
+#host. (since 2.7)
+#
+# @unavailable-features is a list of QOM property names that
+# represent CPU model attributes that prevent the CPU from running.
+# If the QOM property is read-only, that means there's no known
+# way to make the CPU model run in the current host. Implementations
+# that choose not to provide specific information return the
+# property name "type".
+# If the property is read-write, it means that it MAY be possible
+# to run the CPU model in the current host if that property is
+# changed. Management software can use it as hints to suggest or
+# choose an alternative for the user, or just to generate meaningful
+# error messages explaining why the CPU model can't be used.
+# If @unavailable-features is an empty list, the CPU model is
+# runnable using the current host and machine-type.
+# If @unavailable-features is not present, runnability
+# information for the CPU is not available.
+#
 # Since: 1.2.0
 ##
 { 'struct': 'CpuDefinitionInfo',
-  'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool' } }
+  'data': { 'name': 'str', '*migration-safe': 'bool', 'static': 'bool',
+'*unavailable-features': [ 'str' ] } }
 
 ##
 # @query-cpu-definitions:
-- 
2.7.4

[Qemu-devel] [PATCH v3 3/5] target-i386: Define CPUID filtering functions before x86_cpu_list()

[Eduardo Habkost]

Just move code to another place so the it can be reused by the
query-cpu-definitions code.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 68 +++
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1a42233..a755309 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2071,6 +2071,40 @@ static void x86_cpu_parse_featurestr(const char 
*typename, char *features,
 }
 }
 
+/*
+ * Filters CPU feature words based on host availability of each feature.
+ *
+ * Returns: 0 if all flags are supported by the host, non-zero otherwise.
+ */
+static int x86_cpu_filter_features(X86CPU *cpu)
+{
+CPUX86State *env = >env;
+FeatureWord w;
+int rv = 0;
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+uint32_t host_feat =
+x86_cpu_get_supported_feature_word(w, cpu->migratable);
+uint32_t requested_features = env->features[w];
+env->features[w] &= host_feat;
+cpu->filtered_features[w] = requested_features & ~env->features[w];
+if (cpu->filtered_features[w]) {
+rv = 1;
+}
+}
+
+return rv;
+}
+
+static void x86_cpu_report_filtered_features(X86CPU *cpu)
+{
+FeatureWord w;
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+report_unavailable_features(w, cpu->filtered_features[w]);
+}
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2200,40 +2234,6 @@ static uint32_t 
x86_cpu_get_supported_feature_word(FeatureWord w,
 return r;
 }
 
-/*
- * Filters CPU feature words based on host availability of each feature.
- *
- * Returns: 0 if all flags are supported by the host, non-zero otherwise.
- */
-static int x86_cpu_filter_features(X86CPU *cpu)
-{
-CPUX86State *env = >env;
-FeatureWord w;
-int rv = 0;
-
-for (w = 0; w < FEATURE_WORDS; w++) {
-uint32_t host_feat =
-x86_cpu_get_supported_feature_word(w, cpu->migratable);
-uint32_t requested_features = env->features[w];
-env->features[w] &= host_feat;
-cpu->filtered_features[w] = requested_features & ~env->features[w];
-if (cpu->filtered_features[w]) {
-rv = 1;
-}
-}
-
-return rv;
-}
-
-static void x86_cpu_report_filtered_features(X86CPU *cpu)
-{
-FeatureWord w;
-
-for (w = 0; w < FEATURE_WORDS; w++) {
-report_unavailable_features(w, cpu->filtered_features[w]);
-}
-}
-
 static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
 {
 PropValue *pv;
-- 
2.7.4

[Qemu-devel] [PATCH v3 1/5] target-i386: List CPU models using subclass list

[Eduardo Habkost]

Instead of using the builtin_x86_defs array, use the QOM subclass
list to list CPU models on "-cpu ?" and "query-cpu-definitions".

Signed-off-by: Andreas Färber 
[ehabkost: copied code from a patch by Andreas:
 "target-i386: QOM'ify CPU", from March 2012]
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu-qom.h |   4 ++
 target-i386/cpu.c | 103 --
 2 files changed, 78 insertions(+), 29 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 5dde658..e724004 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -63,6 +63,10 @@ typedef struct X86CPUClass {
 
 bool kvm_required;
 
+/* Optional description of CPU model.
+ * If unavailable, cpu_def->model_id is used */
+const char *model_description;
+
 DeviceRealize parent_realize;
 void (*parent_reset)(CPUState *cpu);
 } X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5a5299a..24682aa 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1616,6 +1616,9 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void 
*data)
 cpu_x86_fill_model_id(host_cpudef.model_id);
 
 xcc->cpu_def = _cpudef;
+xcc->model_description =
+"KVM processor with all supported host features "
+"(only available in KVM mode)";
 
 /* level, xlevel, xlevel2, and the feature words are initialized on
  * instance_init, because they require KVM to be initialized.
@@ -2083,23 +2086,62 @@ static void listflags(FILE *f, fprintf_function print, 
const char **featureset)
 }
 }
 
-/* generate CPU information. */
+/* Sort alphabetically by type name, listing kvm_required models last. */
+static gint x86_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+ObjectClass *class_a = (ObjectClass *)a;
+ObjectClass *class_b = (ObjectClass *)b;
+X86CPUClass *cc_a = X86_CPU_CLASS(class_a);
+X86CPUClass *cc_b = X86_CPU_CLASS(class_b);
+const char *name_a, *name_b;
+
+if (cc_a->kvm_required != cc_b->kvm_required) {
+/* kvm_required items go last */
+return cc_a->kvm_required ? 1 : -1;
+} else {
+name_a = object_class_get_name(class_a);
+name_b = object_class_get_name(class_b);
+return strcmp(name_a, name_b);
+}
+}
+
+static GSList *get_sorted_cpu_model_list(void)
+{
+GSList *list = object_class_get_list(TYPE_X86_CPU, false);
+list = g_slist_sort(list, x86_cpu_list_compare);
+return list;
+}
+
+static void x86_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+X86CPUClass *cc = X86_CPU_CLASS(oc);
+CPUListState *s = user_data;
+char *name = x86_cpu_class_get_model_name(cc);
+const char *desc = cc->model_description;
+if (!desc) {
+desc = cc->cpu_def->model_id;
+}
+
+(*s->cpu_fprintf)(s->file, "x86 %16s  %-48s\n",
+  name, desc);
+g_free(name);
+}
+
+/* list available CPU models and flags */
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
-X86CPUDefinition *def;
-char buf[256];
 int i;
+CPUListState s = {
+.file = f,
+.cpu_fprintf = cpu_fprintf,
+};
+GSList *list;
 
-for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
-def = _x86_defs[i];
-snprintf(buf, sizeof(buf), "%s", def->name);
-(*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
-}
-#ifdef CONFIG_KVM
-(*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
-   "KVM processor with all supported host features "
-   "(only available in KVM mode)");
-#endif
+(*cpu_fprintf)(f, "Available CPUs:\n");
+list = get_sorted_cpu_model_list();
+g_slist_foreach(list, x86_cpu_list_entry, );
+g_slist_free(list);
 
 (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
 for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
@@ -2111,26 +2153,29 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 }
 }
 
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
 {
-CpuDefinitionInfoList *cpu_list = NULL;
-X86CPUDefinition *def;
-int i;
+ObjectClass *oc = data;
+X86CPUClass *cc = X86_CPU_CLASS(oc);
+CpuDefinitionInfoList **cpu_list = user_data;
+CpuDefinitionInfoList *entry;
+CpuDefinitionInfo *info;
 
-for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
-CpuDefinitionInfoList *entry;
-CpuDefinitionInfo *info;
-
-def = _x86_defs[i];
-info = g_malloc0(sizeof(*info));
-info->name = g_strdup(def->name);
+info = g_malloc0(sizeof(*info));
+info->name = x86_cpu_class_get_model_name(cc);
 
-entry = g_malloc0(sizeof(*entry));
-entry->value = info;
-entry->next = cpu_list;
-cpu_list = entry;
-}
+entry = g_malloc0(sizeof(*entry));
+

[Qemu-devel] [Bug 1625295] [NEW] qemu-arm dies with libarmmem inside ld.so.preload

[Stu]

Public bug reported:

When running raspbian inside qemu,the user has to first comment out the
following line from /etc/ld.so.conf:

/usr/lib/arm-linux-gnueabihf/libarmmem.so


Will future qemus will be able to work without changine /etc/ld.so.conf ?

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu-arm dies with libarmmem inside ld.so.preload

Status in QEMU:
  New

Bug description:
  When running raspbian inside qemu,the user has to first comment out
  the following line from /etc/ld.so.conf:

  /usr/lib/arm-linux-gnueabihf/libarmmem.so

  
  Will future qemus will be able to work without changine /etc/ld.so.conf ?

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

Re: [Qemu-devel] [PATCH v2 04/10] block: Accept device model name for blockdev-open/close-tray

[Eric Blake]

On 09/19/2016 11:54 AM, Kevin Wolf wrote:
> In order to remove the necessity to use BlockBackend names in the
> external API, we want to allow qdev device names in all device related
> commands.
> 
> This converts blockdev-open/close-tray to accept a qdev device name.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c   | 61 
> +++-
>  qapi/block-core.json | 14 
>  qmp-commands.hx  | 16 --
>  3 files changed, 66 insertions(+), 25 deletions(-)
> 

> +++ b/qmp-commands.hx
> @@ -4295,7 +4295,7 @@ EQMP
>  
>  {
>  .name   = "blockdev-open-tray",
> -.args_type  = "device:s,force:b?",
> +.args_type  = "device:s?,id:s?,force:b?",
>  .mhandler.cmd_new = qmp_marshal_blockdev_open_tray,
>  },

Let the merge conflicts begin! Commit 33e1666b has landed.

> @@ -4329,7 +4331,7 @@ Arguments:
>  Example:
>  
>  -> { "execute": "blockdev-open-tray",
> - "arguments": { "device": "ide1-cd0" } }
> + "arguments": { "id": "ide0-1-0" } }

The changes to the examples look good.  Overall, I'm fine with:

Reviewed-by: Eric Blake 

but I'm afraid it may be easiest for you to respin v3 and do the rebase
work here and elsewhere in the series.

-- 
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 v3 00/10] virtio: avoid exit() when device enters invalid states

[Laszlo Ersek]

On 09/19/16 19:51, Michael S. Tsirkin wrote:
> On Mon, Sep 19, 2016 at 06:07:40PM +0200, Cornelia Huck wrote:
>> On Tue, 12 Apr 2016 14:25:24 +0100
>> Stefan Hajnoczi  wrote:
>>
>>> v3:
>>>  * Patch 1: Fix typo and clarify commit description [Markus]
>>>  * Use virtio_set_status() instead of open coding assignment [Cornelia]
>>>  * Add live migration
>>>
>>> v2:
>>>  * Add VIRTIO_CONFIG_S_NEEDS_RESET notification for VIRTIO 1.0 [Cornelia]
>>>(Note I've sent a Linux virtio_config.h patch to get the constant added 
>>> to
>>>the headers.)
>>>  * Split int -> unsigned int change into separate commit [Fam]
>>>  * Fix double "index" typo in commit description [Fam]
>>>
>>> The virtio code calls exit() when the device enters an invalid state.  This
>>> means invalid vring indices and descriptor chains kill the VM.  See the 
>>> patch
>>> descriptions for why this is a bad thing.
>>>
>>> When the virtio device is in the broken state calls to virtqueue_pop() and
>>> friends will pretend the virtqueue is empty.  This means the device will 
>>> become
>>> isolated from guest activity until it is reset again.
>>>
>>> RFC because two things are missing:
>>> 1. Live migration support (subsection for broken flag?)
>>> 2. Auditing devices and replacing exit() calls there too
>>>
>>> Stefan Hajnoczi (10):
>>>   virtio: fix stray tab character
>>>   include: update virtio_config.h Linux header
>>>   virtio: stop virtqueue processing if device is broken
>>>   virtio: migrate vdev->broken flag
>>>   virtio: handle virtqueue_map_desc() errors
>>>   virtio: handle virtqueue_get_avail_bytes() errors
>>>   virtio: use unsigned int for virtqueue_get_avail_bytes() index
>>>   virtio: handle virtqueue_read_next_desc() errors
>>>   virtio: handle virtqueue_num_heads() errors
>>>   virtio: handle virtqueue_get_head() errors
>>>
>>>  hw/virtio/virtio.c | 223 
>>> +++--
>>>  include/hw/virtio/virtio.h |   3 +
>>>  include/standard-headers/linux/virtio_config.h |   2 +
>>>  3 files changed, 181 insertions(+), 47 deletions(-)
>>>
>>
>> As the exit-in-virtio question has popped up several times in the
>> recent past: I think we should go forward with this series, even if we
>> still need to look at the individual devices. Do you have a version
>> that fits on current master?
> 
> I agree.
> 

NB, Prasad just posted a patch (v3 being the latest) that adds another
such exit(1), at my suggestion.

[Qemu-devel] [PATCH v3] virtio: add check for descriptor's mapped
address

So a rebase of this series should likely consider that patch as well.
(But Stefan is aware anyway.)

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH v2 00/10] block: Accept qdev IDs in device level QMP commands

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH v2 00/10] block: Accept qdev IDs in device level 
QMP commands
Message-id: 1474304097-5790-1-git-send-email-kw...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/1474304097-5790-1-git-send-email-kw...@redhat.com 
-> patchew/1474304097-5790-1-git-send-email-kw...@redhat.com
 * [new tag] 
patchew/1474306544-24708-1-git-send-email-berra...@redhat.com -> 
patchew/1474306544-24708-1-git-send-email-berra...@redhat.com
Switched to a new branch 'test'
d1089ee qemu-iotests/118: Test media change with qdev name
5d9b8a9 block: Accept device model name for block_set_io_throttle
67e0070 block: Accept device model name for blockdev-change-medium
8f4431c block: Accept device model name for eject
e47c17c block: Accept device model name for x-blockdev-remove-medium
f0c7593 block: Accept device model name for x-blockdev-insert-medium
8c7f2d8 block: Accept device model name for blockdev-open/close-tray
d690c73 qdev-monitor: Add blk_by_qdev_id()
2b53d88 qdev-monitor: Factor out find_device_state()
a531e02 block: Add blk_by_dev()

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
=== OUTPUT END ===

Abort: command timeout (>3600 seconds)


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

Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices

[Kirti Wankhede]

On 9/20/2016 12:06 AM, Alex Williamson wrote:
> On Mon, 19 Sep 2016 23:52:36 +0530
> Kirti Wankhede  wrote:
> 
>> On 8/26/2016 7:43 PM, Kirti Wankhede wrote:
>>> * PGP Signed: 08/26/2016 at 07:15:44 AM, Decrypted
>>> On 8/25/2016 2:52 PM, Dong Jia wrote:  
 On Thu, 25 Aug 2016 09:23:53 +0530  
>>
> +
> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
> +   size_t count, loff_t *ppos)
> +{
> + struct vfio_mdev *vmdev = device_data;
> + struct mdev_device *mdev = vmdev->mdev;
> + struct parent_device *parent = mdev->parent;
> + unsigned int done = 0;
> + int ret;
> +
> + if (!parent->ops->read)
> + return -EINVAL;
> +
> + while (count) {  
 Here, I have to say sorry to you guys for that I didn't notice the
 bad impact of this change to my patches during the v6 discussion.

 For vfio-ccw, I introduced an I/O region to input/output I/O
 instruction parameters and results for Qemu. The @count of these data
 currently is 140. So supporting arbitrary lengths in one shot here, and
 also in vfio_mdev_write, seems the better option for this case.

 I believe that if the pci drivers want to iterate in a 4 bytes step, you
 can do that in the parent read/write callbacks instead.

 What do you think?
  
>>>
>>> I would like to know Alex's thought on this. He raised concern with this
>>> approach in v6 reviews:
>>> "But I think this is exploitable, it lets the user make the kernel
>>> allocate an arbitrarily sized buffer."
>>>   
>>
>> Read/write callbacks are for slow path, emulation of mmio region which
>> are mainly device registers. I do feel it shouldn't support arbitrary
>> lengths.
>> Alex, I would like to know your thoughts.
> 
> The exploit was that the mdev layer allocated a buffer and copied the
> entire user buffer into kernel space before passing it to the vendor
> driver.  The solution is to pass the __user *buf to the vendor driver
> and let them sanitize and split it however makes sense for their
> device.  We shouldn't be assuming naturally aligned, up to dword
> accesses in the generic mdev layers.  Those sorts of semantics are
> defined by the device type.  This is another case where making
> the mdev layer as thin as possible is actually the best thing to
> do to make it really device type agnostic.  Thanks,
> 


Alex,

These were the comments on v6 patch:

>>> Do we really need to support arbitrary lengths in one shot?  Seems
>>> like
>>> we could just use a 4 or 8 byte variable on the stack and iterate
>>> until
>>> done.
>>>
>>
>> We just want to pass the arguments to vendor driver as is here.Vendor
>> driver could take care of that.

> But I think this is exploitable, it lets the user make the kernel
> allocate an arbitrarily sized buffer.

As per above discussion in v7 version, this module don't allocated
memory from heap.

If vendor driver allocates arbitrary memory in kernel space through mdev
module interface, isn't that would be exploit?

Thanks,
Kirti

Re: [Qemu-devel] [PATCH v4 0/2] block: allow flush on devices with open tray

[John Snow]

On 09/19/2016 02:15 PM, 
no-re...@ec2-52-6-146-230.compute-1.amazonaws.com wrote:

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.



lies and slander!


Subject: [Qemu-devel] [PATCH v4 0/2] block: allow flush on devices with open 
tray
Message-id: 1474303471-12509-1-git-send-email-js...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
647d07e qemu: use bdrv_flush_all for vm_stop et al
5989f9b block: reintroduce bdrv_flush_all

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/src/tests/docker/install
BIOS directory/tmp/qemu-test/src/tests/docker/install/share/qemu
binary directory  /tmp/qemu-test/src/tests/docker/install/bin
library directory /tmp/qemu-test/src/tests/docker/install/lib
module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
include directory /tmp/qemu-test/src/tests/docker/install/include
config directory  /tmp/qemu-test/src/tests/docker/install/etc
local state directory   /tmp/qemu-test/src/tests/docker/install/var
Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g
QEMU_CFLAGS   -I/usr/include/pixman-1-fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no
GTK GL supportno
VTE support   no
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw)
Block whitelist (ro)
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
uuid support  no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backendslog
spice support no
rbd support   no
xfsctl supportno
smartcard support no
libusbno
usb net redir no
OpenGL supportno
OpenGL dmabufsno
libiscsi support  no
libnfs supportno
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine poolyes
GlusterFS support no
Archipelago support no
gcov  gcov
gcov enabled  no
TPM support   yes
libssh2 support   no
TPM passthrough   yes
QOM debugging yes
vhdx  no
lzo support   no
snappy supportno
bzip2 support no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
replication support yes
  GEN   x86_64-softmmu/config-devices.mak.tmp
  GEN   aarch64-softmmu/config-devices.mak.tmp
  GEN   config-host.h
  GEN   qemu-options.def
  GEN   qmp-commands.h
  GEN   qapi-types.h
  GEN   qapi-visit.h
 

Re: [Qemu-devel] [RFC 7/8] util/qht: atomically set b->hashes

[Emilio G. Cota]

On Mon, Sep 19, 2016 at 20:37:06 +0200, Paolo Bonzini wrote:
> On 19/09/2016 20:06, Emilio G. Cota wrote:
> > On Mon, Sep 19, 2016 at 16:51:38 +0100, Alex Bennée wrote:
> >> > ThreadSanitizer detects a possible race between reading/writing the
> >> > hashes. As ordering semantics are already documented for qht we just
> >> > need to ensure a race can't tear the hash value so we can use the
> >> > relaxed atomic_set/read functions.
> > This was discussed here:
> > 
> > https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg03658.html
> > 
> > To reiterate: reading torn hash values is fine, since the retry will
> > happen regardless (and all pointers[] remain valid through the RCU
> > read-critical section).
> 
> True, but C11 says data races are undefined, not merely unspecified.
> seqlock-protected data requires a relaxed read and write, because they
> are read concurrently in the read and write sides.

Ah I see.

Let me then just point out that this comes at a small perf loss.

Running 'taskset -c 0 tests/qht-bench -n 1 -d 10' (i.e. all lookups) 10 times,
we get:

before the patch:
 $ ./mean.pl 34.04 34.24 34.38 34.25 34.18 34.51 34.46 34.44 34.29 34.08
 34.287 +- 0.160072900059109
after:
 $ ./mean.pl 33.94 34.00 33.52 33.46 33.55 33.71 34.27 34.06 34.28 34.58
 33.937 +- 0.374731014640279

But hey we can live with that.

Cheers,

E.

Re: [Qemu-devel] [PATCH v5 5/8] linux-user: Fix msgrcv() and msgsnd() syscalls support

[Aleksandar Markovic]

Hello, Lorain.

I really appreciate your reviewing these patches. This was a great opportunity 
for me to learn a lot. I will address all your concerns about this series in 
its next version, which is planned to be posted by the end of this week. 
Regarding another series on Mips-specific issues, its v7 was posted on 
qemu-devel today.

Thanks,
Aleksandar


From: Laurent Vivier [laur...@vivier.eu]
Sent: Saturday, September 17, 2016 4:43 PM
To: Aleksandar Markovic; qemu-devel@nongnu.org; riku.voi...@iki.fi; 
peter.mayd...@linaro.org; Petar Jovanovic; Miodrag Dinic; Aleksandar Rikalo; 
Aleksandar Markovic
Subject: Re: [Qemu-devel] [PATCH v5 5/8] linux-user: Fix msgrcv() and msgsnd() 
syscalls support

Le 14/09/2016 à 22:19, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic 
>
> If syscalls msgrcv() and msgsnd() fail, they return E2BIG, EACCES,
> EAGAIN, EFAULT, EIDRM, EINTR, EINVAL, ENOMEM, or ENOMSG.
>
> By examining negative scenarios of these syscalls for Mips, it was
> established that ENOMSG does not have the same value accross all
> platforms, but it is nevertheless not included for conversion in
> the correspondant conversion table defined in linux-user/syscall.c.
> This is certainly a bug, since it leads to the incorrect emulation
> of msgrcv() and msgsnd() for scenarios involving ENOMSG.
>
> This patch fixes this by extending the conversion table to include
> ENOMSG.
>
> Also, LTP test msgrcv04 will be fixed for some platforms.
>
> Signed-off-by: Aleksandar Markovic 

Reviewed-by: Laurent Vivier 

> ---
>  linux-user/syscall.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 7f8ae41..bdc12ae 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -750,6 +750,9 @@ static uint16_t 
> host_to_target_errno_table[ERRNO_TABLE_SIZE] = {
>  #ifdef ENOTRECOVERABLE
>  [ENOTRECOVERABLE]= TARGET_ENOTRECOVERABLE,
>  #endif
> +#ifdef ENOMSG
> +[ENOMSG]= TARGET_ENOMSG,
> +#endif
>  };
>
>  static inline int host_to_target_errno(int err)
>

Re: [Qemu-devel] [PATCH v3] virtio: add check for descriptor's mapped address

[Laszlo Ersek]

On 09/19/16 20:25, P J P wrote:
> From: Prasad J Pandit 
> 
> virtio back end uses set of buffers to facilitate I/O operations.
> If its size is too large, 'cpu_physical_memory_map' could return
> a null address. This would result in a null dereference while
> un-mapping descriptors. Add check to avoid it.
> 
> Reported-by: Qinghao Tang 
> Signed-off-by: Prasad J Pandit 
> ---
>  hw/virtio/virtio.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> Update per:
>   -> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04329.html
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 15ee3a7..a4ebf8c 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -472,6 +472,11 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, 
> hwaddr *addr, struct iove
>  }
>  
>  iov[num_sg].iov_base = cpu_physical_memory_map(pa, , is_write);
> +if (!iov[num_sg].iov_base) {
> +error_report("virtio: bogus descriptor or out of resources");
> +exit(1);
> +}
> +
>  iov[num_sg].iov_len = len;
>  addr[num_sg] = pa;
>  
> 

Reviewed-by: Laszlo Ersek 

[Qemu-devel] [Bug 1624726] Re: Integrator/CP regression after QOM'ification of integratorcp.c

[Jakub Jermar]

Turns out integratorcm_init() depends on the memsz property being
already set, but that unfortunately is not the case as setting of memsz
depends on integratorcm_init() having completed:

dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);<= calls 
integratorcm_init(), needs memsz
qdev_prop_set_uint32(dev, "memsz", ram_size >> 20); <= memsz set here, 
needs dev

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

Title:
  Integrator/CP regression after QOM'ification of integratorcp.c

Status in HelenOS branches:
  New
Status in QEMU:
  New

Bug description:
  The following command line no longer works (i.e. the guest does not
  boot) with QEMU 2.7.0:

  qemu-system-arm -M integratorcp -m 128M -kernel
  HelenOS-0.6.0-arm32-integratorcp.boot

  The HelenOS image can be downloaded here:

  http://www.helenos.org/releases/HelenOS-0.6.0-arm32-integratorcp.boot

  I did git bisect and came to this revision:

  a1f42e0c9abc1028a8bb8686dbb3749fcd2d18e8 is the first bad commit
  commit a1f42e0c9abc1028a8bb8686dbb3749fcd2d18e8
  Author: xiaoqiang.zhao 
  Date:   Mon Mar 7 15:05:44 2016 +0800

  hw/arm: QOM'ify integratorcp.c
  
  * Drop the use of old SysBus init function and use instance_init
  * Remove the empty 'icp_pic_class_init' from Typeinfo
  
  Signed-off-by: xiaoqiang zhao 
  Reviewed-by: Peter Maydell 
  Signed-off-by: Peter Maydell 

  :04 04 b73418ea3fb69ed72438776e78786456fe4c414c
  b483e8579037fdae7d136b2f4ada3147bdde92f1 M  hw

  Upon closer inspection, I discovered that for some reason s->memsz in
  integratorcm_init() is zero. In the last good revision, this value was
  128. As a temporary workaround, hardcoding it to this expected value
  fixes the problem.

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

[Qemu-devel] [PULL 2/3] kvm/apic: drop debugging

[Eduardo Habkost]

From: "Michael S. Tsirkin" 

commit 78d6a05d2f69cbfa6e95f0a4a24a2c934969913b
("x86/lapic: Load LAPIC state at post_load")
has some debugging leftovers.

Drop them.

Cc: Dr. David Alan Gilbert 
Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Eduardo Habkost 
---
 hw/i386/kvm/apic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 5d140b9..feb0002 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -141,7 +141,6 @@ static void kvm_apic_put(void *data)
 
 static void kvm_apic_post_load(APICCommonState *s)
 {
-fprintf(stderr, "%s: Yeh\n", __func__);
 run_on_cpu(CPU(s->cpu), kvm_apic_put, s);
 }
 
-- 
2.7.4

[Qemu-devel] [PULL 0/3] x86 queue, 2016-09-19

[Eduardo Habkost]

The following changes since commit 33e1666b4289313306371fee0740f5c85517e406:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-09-19' into 
staging (2016-09-19 18:06:52 +0100)

are available in the git repository at:

  git://github.com/ehabkost/qemu.git tags/x86-pull-request

for you to fetch changes up to fa5376dd8adb5ad104e5773f09c15af2e8a76738:

  linux-user-i386: Fix crash on cpuid (2016-09-19 15:34:35 -0300)


x86 queue, 2016-09-19



Marc-André Lureau (1):
  linux-user-i386: Fix crash on cpuid

Michael S. Tsirkin (1):
  kvm/apic: drop debugging

Richard Henderson (1):
  target-i386: Use struct X86XSaveArea in fpu_helper.c

 hw/i386/kvm/apic.c   |   1 -
 qom/cpu.c|   5 +++
 target-i386/cpu.c|   7 ++-
 target-i386/cpu.h|  10 +
 target-i386/fpu_helper.c | 108 ++-
 5 files changed, 72 insertions(+), 59 deletions(-)

-- 
2.7.4

[Qemu-devel] [PULL 1/3] target-i386: Use struct X86XSaveArea in fpu_helper.c

[Eduardo Habkost]

From: Richard Henderson 

This avoids a double hand-full of magic numbers in the
xsave and xrstor helper functions.

Signed-off-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c|   7 ++-
 target-i386/cpu.h|  10 +
 target-i386/fpu_helper.c | 108 ++-
 3 files changed, 67 insertions(+), 58 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 5a5299a..db12728 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -538,7 +538,12 @@ static const X86RegisterInfo32 
x86_reg_info_32[CPU_NB_REGS32] = {
 };
 #undef REGISTER
 
-const ExtSaveArea x86_ext_save_areas[] = {
+typedef struct ExtSaveArea {
+uint32_t feature, bits;
+uint32_t offset, size;
+} ExtSaveArea;
+
+static const ExtSaveArea x86_ext_save_areas[] = {
 [XSTATE_YMM_BIT] =
   { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
 .offset = offsetof(X86XSaveArea, avx_state),
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 58e43b6..27af9c3 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -877,7 +877,8 @@ typedef union X86LegacyXSaveArea {
 typedef struct X86XSaveHeader {
 uint64_t xstate_bv;
 uint64_t xcomp_bv;
-uint8_t reserved[48];
+uint64_t reserve0;
+uint8_t reserved[40];
 } X86XSaveHeader;
 
 /* Ext. save area 2: AVX State */
@@ -1392,13 +1393,6 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
void *puc);
 
 /* cpu.c */
-typedef struct ExtSaveArea {
-uint32_t feature, bits;
-uint32_t offset, size;
-} ExtSaveArea;
-
-extern const ExtSaveArea x86_ext_save_areas[];
-
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx);
diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index 929489b..2049a8c 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -1110,6 +1110,8 @@ void cpu_x86_frstor(CPUX86State *env, target_ulong ptr, 
int data32)
 }
 #endif
 
+#define XO(X)  offsetof(X86XSaveArea, X)
+
 static void do_xsave_fpu(CPUX86State *env, target_ulong ptr, uintptr_t ra)
 {
 int fpus, fptag, i;
@@ -1120,17 +1122,18 @@ static void do_xsave_fpu(CPUX86State *env, target_ulong 
ptr, uintptr_t ra)
 for (i = 0; i < 8; i++) {
 fptag |= (env->fptags[i] << i);
 }
-cpu_stw_data_ra(env, ptr, env->fpuc, ra);
-cpu_stw_data_ra(env, ptr + 2, fpus, ra);
-cpu_stw_data_ra(env, ptr + 4, fptag ^ 0xff, ra);
+
+cpu_stw_data_ra(env, ptr + XO(legacy.fcw), env->fpuc, ra);
+cpu_stw_data_ra(env, ptr + XO(legacy.fsw), fpus, ra);
+cpu_stw_data_ra(env, ptr + XO(legacy.ftw), fptag ^ 0xff, ra);
 
 /* In 32-bit mode this is eip, sel, dp, sel.
In 64-bit mode this is rip, rdp.
But in either case we don't write actual data, just zeros.  */
-cpu_stq_data_ra(env, ptr + 0x08, 0, ra); /* eip+sel; rip */
-cpu_stq_data_ra(env, ptr + 0x10, 0, ra); /* edp+sel; rdp */
+cpu_stq_data_ra(env, ptr + XO(legacy.fpip), 0, ra); /* eip+sel; rip */
+cpu_stq_data_ra(env, ptr + XO(legacy.fpdp), 0, ra); /* edp+sel; rdp */
 
-addr = ptr + 0x20;
+addr = ptr + XO(legacy.fpregs);
 for (i = 0; i < 8; i++) {
 floatx80 tmp = ST(i);
 helper_fstt(env, tmp, addr, ra);
@@ -1140,8 +1143,8 @@ static void do_xsave_fpu(CPUX86State *env, target_ulong 
ptr, uintptr_t ra)
 
 static void do_xsave_mxcsr(CPUX86State *env, target_ulong ptr, uintptr_t ra)
 {
-cpu_stl_data_ra(env, ptr + 0x18, env->mxcsr, ra); /* mxcsr */
-cpu_stl_data_ra(env, ptr + 0x1c, 0x, ra); /* mxcsr_mask */
+cpu_stl_data_ra(env, ptr + XO(legacy.mxcsr), env->mxcsr, ra);
+cpu_stl_data_ra(env, ptr + XO(legacy.mxcsr_mask), 0x, ra);
 }
 
 static void do_xsave_sse(CPUX86State *env, target_ulong ptr, uintptr_t ra)
@@ -1155,7 +1158,7 @@ static void do_xsave_sse(CPUX86State *env, target_ulong 
ptr, uintptr_t ra)
 nb_xmm_regs = 8;
 }
 
-addr = ptr + 0xa0;
+addr = ptr + XO(legacy.xmm_regs);
 for (i = 0; i < nb_xmm_regs; i++) {
 cpu_stq_data_ra(env, addr, env->xmm_regs[i].ZMM_Q(0), ra);
 cpu_stq_data_ra(env, addr + 8, env->xmm_regs[i].ZMM_Q(1), ra);
@@ -1163,8 +1166,9 @@ static void do_xsave_sse(CPUX86State *env, target_ulong 
ptr, uintptr_t ra)
 }
 }
 
-static void do_xsave_bndregs(CPUX86State *env, target_ulong addr, uintptr_t ra)
+static void do_xsave_bndregs(CPUX86State *env, target_ulong ptr, uintptr_t ra)
 {
+target_ulong addr = ptr + offsetof(XSaveBNDREG, bnd_regs);
 int i;
 
 for (i = 0; i < 4; i++, addr += 16) {
@@ -1173,15 +1177,17 @@ static void do_xsave_bndregs(CPUX86State *env, 
target_ulong addr, uintptr_t ra)
 }
 }
 
-static void do_xsave_bndcsr(CPUX86State *env, target_ulong addr, uintptr_t ra)
+static void do_xsave_bndcsr(CPUX86State *env, 

[Qemu-devel] [PULL 3/3] linux-user-i386: Fix crash on cpuid

[Eduardo Habkost]

From: Marc-André Lureau 

Running cpuid instructions with a simple run like:
i386-linux-user/qemu-i386 tests/tcg/sha1-i386

Results in the following assert:
 #0  0x764246f5 in raise () from /lib64/libc.so.6
 #1  0x764262fa in abort () from /lib64/libc.so.6
 #2  0x77937ec5 in g_assertion_message () from /lib64/libglib-2.0.so.0
 #3  0x77937f5a in g_assertion_message_expr () from 
/lib64/libglib-2.0.so.0
 #4  0x5561b54c in apicid_bitwidth_for_count (count=0) at 
/home/elmarco/src/qemu/include/hw/i386/topology.h:58
 #5  0x5561b58a in apicid_smt_width (nr_cores=0, nr_threads=0) at 
/home/elmarco/src/qemu/include/hw/i386/topology.h:67
 #6  0x5561b5c3 in apicid_core_offset (nr_cores=0, nr_threads=0) at 
/home/elmarco/src/qemu/include/hw/i386/topology.h:82
 #7  0x5561b5e3 in apicid_pkg_offset (nr_cores=0, nr_threads=0) at 
/home/elmarco/src/qemu/include/hw/i386/topology.h:89
 #8  0x5561dd86 in cpu_x86_cpuid (env=0x57999550, index=4, count=3, 
eax=0x7fffcae8, ebx=0x7fffcaec, ecx=0x7fffcaf0, edx=0x7fffcaf4) 
at /home/elmarco/src/qemu/target-i386/cpu.c:2405
 #9  0x55638e8e in helper_cpuid (env=0x57999550) at 
/home/elmarco/src/qemu/target-i386/misc_helper.c:106
 #10 0x5599dc5e in static_code_gen_buffer ()
 #11 0x555952f8 in cpu_tb_exec (cpu=0x579912d0, itb=0x74371ab0) 
at /home/elmarco/src/qemu/cpu-exec.c:166
 #12 0x55595c8e in cpu_loop_exec_tb (cpu=0x579912d0, 
tb=0x74371ab0, last_tb=0x7fffd088, tb_exit=0x7fffd084, 
sc=0x7fffd0a0) at /home/elmarco/src/qemu/cpu-exec.c:517
 #13 0x55595e50 in cpu_exec (cpu=0x579912d0) at 
/home/elmarco/src/qemu/cpu-exec.c:612
 #14 0x555c065b in cpu_loop (env=0x57999550) at 
/home/elmarco/src/qemu/linux-user/main.c:297
 #15 0x555c25b2 in main (argc=2, argv=0x7fffd848, 
envp=0x7fffd860) at /home/elmarco/src/qemu/linux-user/main.c:4803

The fields are set in qemu_init_vcpu() with softmmu, but it's a stub
with linux-user.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Eduardo Habkost 
---
 qom/cpu.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/qom/cpu.c b/qom/cpu.c
index 2553247..f783b5a 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -342,6 +342,11 @@ static void cpu_common_initfn(Object *obj)
 
 cpu->cpu_index = UNASSIGNED_CPU_INDEX;
 cpu->gdb_num_regs = cpu->gdb_num_g_regs = cc->gdb_num_core_regs;
+/* *-user doesn't have configurable SMP topology */
+/* the default value is changed by qemu_init_vcpu() for softmmu */
+cpu->nr_cores = 1;
+cpu->nr_threads = 1;
+
 qemu_mutex_init(>work_mutex);
 QTAILQ_INIT(>breakpoints);
 QTAILQ_INIT(>watchpoints);
-- 
2.7.4

Re: [Qemu-devel] [RFC 7/8] util/qht: atomically set b->hashes

[Paolo Bonzini]

On 19/09/2016 20:06, Emilio G. Cota wrote:
> On Mon, Sep 19, 2016 at 16:51:38 +0100, Alex Bennée wrote:
>> > ThreadSanitizer detects a possible race between reading/writing the
>> > hashes. As ordering semantics are already documented for qht we just
>> > need to ensure a race can't tear the hash value so we can use the
>> > relaxed atomic_set/read functions.
> This was discussed here:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg03658.html
> 
> To reiterate: reading torn hash values is fine, since the retry will
> happen regardless (and all pointers[] remain valid through the RCU
> read-critical section).

True, but C11 says data races are undefined, not merely unspecified.
seqlock-protected data requires a relaxed read and write, because they
are read concurrently in the read and write sides.

Paolo

Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices

[Alex Williamson]

On Mon, 19 Sep 2016 23:52:36 +0530
Kirti Wankhede  wrote:

> On 8/26/2016 7:43 PM, Kirti Wankhede wrote:
> > * PGP Signed: 08/26/2016 at 07:15:44 AM, Decrypted
> > On 8/25/2016 2:52 PM, Dong Jia wrote:  
> >> On Thu, 25 Aug 2016 09:23:53 +0530  
> 
> >>> +
> >>> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
> >>> +   size_t count, loff_t *ppos)
> >>> +{
> >>> + struct vfio_mdev *vmdev = device_data;
> >>> + struct mdev_device *mdev = vmdev->mdev;
> >>> + struct parent_device *parent = mdev->parent;
> >>> + unsigned int done = 0;
> >>> + int ret;
> >>> +
> >>> + if (!parent->ops->read)
> >>> + return -EINVAL;
> >>> +
> >>> + while (count) {  
> >> Here, I have to say sorry to you guys for that I didn't notice the
> >> bad impact of this change to my patches during the v6 discussion.
> >>
> >> For vfio-ccw, I introduced an I/O region to input/output I/O
> >> instruction parameters and results for Qemu. The @count of these data
> >> currently is 140. So supporting arbitrary lengths in one shot here, and
> >> also in vfio_mdev_write, seems the better option for this case.
> >>
> >> I believe that if the pci drivers want to iterate in a 4 bytes step, you
> >> can do that in the parent read/write callbacks instead.
> >>
> >> What do you think?
> >>  
> > 
> > I would like to know Alex's thought on this. He raised concern with this
> > approach in v6 reviews:
> > "But I think this is exploitable, it lets the user make the kernel
> > allocate an arbitrarily sized buffer."
> >   
> 
> Read/write callbacks are for slow path, emulation of mmio region which
> are mainly device registers. I do feel it shouldn't support arbitrary
> lengths.
> Alex, I would like to know your thoughts.

The exploit was that the mdev layer allocated a buffer and copied the
entire user buffer into kernel space before passing it to the vendor
driver.  The solution is to pass the __user *buf to the vendor driver
and let them sanitize and split it however makes sense for their
device.  We shouldn't be assuming naturally aligned, up to dword
accesses in the generic mdev layers.  Those sorts of semantics are
defined by the device type.  This is another case where making
the mdev layer as thin as possible is actually the best thing to
do to make it really device type agnostic.  Thanks,

Alex

Re: [Qemu-devel] [PATCH v2] virtio: add check for descriptor's mapped address

[P J P]

+-- On Mon, 19 Sep 2016, Laszlo Ersek wrote --+
| Prasad, can you please send a v3 then, to address Stefan's feedback?

Yes, sent patch v3. Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

[Qemu-devel] [PATCH v3] virtio: add check for descriptor's mapped address

[P J P]

From: Prasad J Pandit 

virtio back end uses set of buffers to facilitate I/O operations.
If its size is too large, 'cpu_physical_memory_map' could return
a null address. This would result in a null dereference while
un-mapping descriptors. Add check to avoid it.

Reported-by: Qinghao Tang 
Signed-off-by: Prasad J Pandit 
---
 hw/virtio/virtio.c | 5 +
 1 file changed, 5 insertions(+)

Update per:
  -> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg04329.html

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 15ee3a7..a4ebf8c 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -472,6 +472,11 @@ static void virtqueue_map_desc(unsigned int *p_num_sg, 
hwaddr *addr, struct iove
 }
 
 iov[num_sg].iov_base = cpu_physical_memory_map(pa, , is_write);
+if (!iov[num_sg].iov_base) {
+error_report("virtio: bogus descriptor or out of resources");
+exit(1);
+}
+
 iov[num_sg].iov_len = len;
 addr[num_sg] = pa;
 
-- 
2.5.5

Re: [Qemu-devel] [PATCH v3 18/18] trace: introduce a formal group name for trace events

[Lluís Vilanova]

Daniel P Berrange writes:

> The declarations in the generated-tracers.h file are
> assuming there's only ever going to be one instance
> of this header, as they are not namespaced. When we
> have one header per event group, if a single source
> file needs to include multiple sets of trace events,
> the symbols will all clash.

> This change thus introduces a '--group NAME' arg to the
> 'tracetool' program. This will cause all the symbols in
> the generated header files to be given a unique namespace.

> If no group is given, the group name 'common' is used,
> which is suitable for the current usage where there is
> only one global trace-events file used for code generation.

> Signed-off-by: Daniel P. Berrange 
> ---
>  scripts/tracetool.py | 24 
> +++-
>  scripts/tracetool/__init__.py|  6 --
>  scripts/tracetool/backend/__init__.py| 12 ++--
>  scripts/tracetool/backend/dtrace.py  |  4 ++--
>  scripts/tracetool/backend/ftrace.py  |  5 +++--
>  scripts/tracetool/backend/log.py |  4 ++--
>  scripts/tracetool/backend/simple.py  |  8 
>  scripts/tracetool/backend/syslog.py  |  4 ++--
>  scripts/tracetool/backend/ust.py |  4 ++--
>  scripts/tracetool/format/__init__.py |  4 ++--
>  scripts/tracetool/format/c.py| 18 ++
>  scripts/tracetool/format/d.py|  2 +-
>  scripts/tracetool/format/h.py| 14 +++---
>  scripts/tracetool/format/simpletrace_stap.py |  2 +-
>  scripts/tracetool/format/stap.py |  2 +-
>  scripts/tracetool/format/tcg_h.py|  2 +-
>  scripts/tracetool/format/tcg_helper_c.py |  2 +-
>  scripts/tracetool/format/tcg_helper_h.py |  2 +-
>  scripts/tracetool/format/tcg_helper_wrapper_h.py |  2 +-
>  scripts/tracetool/format/ust_events_c.py |  2 +-
>  scripts/tracetool/format/ust_events_h.py |  8 
>  21 files changed, 79 insertions(+), 52 deletions(-)

> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index f66e767..0ee66f3 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py
> @@ -15,6 +15,8 @@ __email__  = "stefa...@linux.vnet.ibm.com"
 
>  import sys
>  import getopt
> +import os.path
> +import re
 
>  from tracetool import error_write, out
>  import tracetool.backend
> @@ -60,6 +62,24 @@ Options:
>  else:
>  sys.exit(1)
 
> +def find_git_root(dirname):
> +while dirname != "":
> +git = os.path.join(dirname, ".git")
> +if os.path.exists(git):
> +return dirname
> +dirname = os.path.dirname(dirname)
> +
> +raise ValueError("Cannot find .git directory for %s",
> + filename)
> +

Some people (distributions?) distribute source tarballs without the VCS
files/directories, so this might break.

Even if ugly, it might be easier to just hardcode it based on the script's
relative location:

  os.path.abspath(os.path.join(os.dirname(___file___), os.pardir))


> +def make_group_name(filename):
> +dirname = os.path.dirname(filename)
> +gitdir = find_git_root(dirname)
> +dirname = dirname[len(gitdir):]
> +
> +if dirname == "":
> +return "common"
> +return re.sub(r"/|-", "_", dirname)
 
>  def main(args):
>  global _SCRIPT
> @@ -134,8 +154,10 @@ def main(args):
>  with open(args[0], "r") as fh:
>  events = tracetool.read_events(fh)
 
> +group = make_group_name(args[0])
> +
>  try:
> -tracetool.generate(events, arg_format, arg_backends,
> +tracetool.generate(events, group, arg_format, arg_backends,
> binary=binary, probe_prefix=probe_prefix)
>  except tracetool.TracetoolError as e:
>  error_opt(str(e))
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index ed668a1..be8a360 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -363,7 +363,7 @@ def try_import(mod_name, attr_name=None, 
> attr_default=None):
>  return False, None
 
 
> -def generate(events, format, backends,
> +def generate(events, group, format, backends,
>   binary=None, probe_prefix=None):
>  """Generate the output for the given (format, backends) pair.
 
> @@ -371,6 +371,8 @@ def generate(events, format, backends,
>  --
>  events : list
>  list of Event objects to generate for
> +group: str
> +Name of the tracing group
>  format : str
>  Output format name.
>  backends : list
> @@ -400,4 +402,4 @@ def generate(events, format, backends,
>  tracetool.backend.dtrace.BINARY = binary
>  tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
 
> -tracetool.format.generate(events, format, backend)
> +tracetool.format.generate(events, 

Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices

[Kirti Wankhede]

On 8/26/2016 7:43 PM, Kirti Wankhede wrote:
> * PGP Signed: 08/26/2016 at 07:15:44 AM, Decrypted
> On 8/25/2016 2:52 PM, Dong Jia wrote:
>> On Thu, 25 Aug 2016 09:23:53 +0530

>>> +
>>> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf,
>>> + size_t count, loff_t *ppos)
>>> +{
>>> +   struct vfio_mdev *vmdev = device_data;
>>> +   struct mdev_device *mdev = vmdev->mdev;
>>> +   struct parent_device *parent = mdev->parent;
>>> +   unsigned int done = 0;
>>> +   int ret;
>>> +
>>> +   if (!parent->ops->read)
>>> +   return -EINVAL;
>>> +
>>> +   while (count) {
>> Here, I have to say sorry to you guys for that I didn't notice the
>> bad impact of this change to my patches during the v6 discussion.
>>
>> For vfio-ccw, I introduced an I/O region to input/output I/O
>> instruction parameters and results for Qemu. The @count of these data
>> currently is 140. So supporting arbitrary lengths in one shot here, and
>> also in vfio_mdev_write, seems the better option for this case.
>>
>> I believe that if the pci drivers want to iterate in a 4 bytes step, you
>> can do that in the parent read/write callbacks instead.
>>
>> What do you think?
>>
> 
> I would like to know Alex's thought on this. He raised concern with this
> approach in v6 reviews:
> "But I think this is exploitable, it lets the user make the kernel
> allocate an arbitrarily sized buffer."
> 

Read/write callbacks are for slow path, emulation of mmio region which
are mainly device registers. I do feel it shouldn't support arbitrary
lengths.
Alex, I would like to know your thoughts.

Thanks,
Kirti

Re: [Qemu-devel] [PATCH for-2.8 v4 0/6] Add support for Cadence GEM priority queues

[Alistair Francis]

On Mon, Sep 19, 2016 at 11:08 AM, Peter Maydell
 wrote:
> On 28 July 2016 at 20:00, Alistair Francis  
> wrote:
>>
>> This patch series adds initial priority queue support to the Cadence GEM
>> device. This is based on original work by Peter C, that has been ported
>> to the latest version of QEMU.
>>
>> There is more GEM work that I'd like to upstream after this, but I
>> figured this a good place to start. I have done limited testing on the
>> Xilinx machine, including running some of our GEM regressions, although
>> they don't cover everything. I can't really think of too many other test
>> cases that I can run at the moment.
>
> Hi -- I just found this in a random mail folder I forgot to check
> very often -- I'm guessing it's still relevant and I should review/apply
> it to target-arm.next?

Yes, you fully reviewed it during the 2.7 freeze. It should be ready
to apply to master now.

Thanks,

Alistair

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v3 16/18] trace: push reading of events up a level to tracetool main

[Lluís Vilanova]

Daniel P Berrange writes:

> Move the reading of events out of the 'tracetool.generate'
> method and into tracetool.main, so that the latter is not
> tied to generating from a single source of events.

> Signed-off-by: Daniel P. Berrange 

Reviewed-by: Lluís Vilanova 


> ---
>  scripts/tracetool.py  | 4 +++-
>  scripts/tracetool/__init__.py | 8 +++-
>  2 files changed, 6 insertions(+), 6 deletions(-)

> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index 7b82959..6accbbf 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py
> @@ -129,8 +129,10 @@ def main(args):
>  if probe_prefix is None:
>  probe_prefix = ".".join(["qemu", target_type, target_name])
 
> +events = tracetool.read_events(sys.stdin)
> +
>  try:
> -tracetool.generate(sys.stdin, arg_format, arg_backends,
> +tracetool.generate(events, arg_format, arg_backends,
> binary=binary, probe_prefix=probe_prefix)
>  except tracetool.TracetoolError as e:
>  error_opt(str(e))
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 1bb3886..ed668a1 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -363,14 +363,14 @@ def try_import(mod_name, attr_name=None, 
> attr_default=None):
>  return False, None
 
 
> -def generate(fevents, format, backends,
> +def generate(events, format, backends,
>   binary=None, probe_prefix=None):
>  """Generate the output for the given (format, backends) pair.
 
>  Parameters
>  --
> -fevents : file
> -Event description file.
> +events : list
> +list of Event objects to generate for
>  format : str
>  Output format name.
>  backends : list
> @@ -400,6 +400,4 @@ def generate(fevents, format, backends,
>  tracetool.backend.dtrace.BINARY = binary
>  tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
 
> -events = read_events(fevents)
> -
>  tracetool.format.generate(events, format, backend)
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH v4 0/2] block: allow flush on devices with open tray

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH v4 0/2] block: allow flush on devices with open 
tray
Message-id: 1474303471-12509-1-git-send-email-js...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
647d07e qemu: use bdrv_flush_all for vm_stop et al
5989f9b block: reintroduce bdrv_flush_all

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY RUNNER
  RUN test-quick in centos6
No C++ compiler available; disabling C++ specific optional code
Install prefix/tmp/qemu-test/src/tests/docker/install
BIOS directory/tmp/qemu-test/src/tests/docker/install/share/qemu
binary directory  /tmp/qemu-test/src/tests/docker/install/bin
library directory /tmp/qemu-test/src/tests/docker/install/lib
module directory  /tmp/qemu-test/src/tests/docker/install/lib/qemu
libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
include directory /tmp/qemu-test/src/tests/docker/install/include
config directory  /tmp/qemu-test/src/tests/docker/install/etc
local state directory   /tmp/qemu-test/src/tests/docker/install/var
Manual directory  /tmp/qemu-test/src/tests/docker/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
uuid support  no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backendslog
spice support no 
rbd support   no
xfsctl supportno
smartcard support no
libusbno
usb net redir no
OpenGL supportno
OpenGL dmabufsno
libiscsi support  no
libnfs supportno
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine poolyes
GlusterFS support no
Archipelago support no
gcov  gcov
gcov enabled  no
TPM support   yes
libssh2 support   no
TPM passthrough   yes
QOM debugging yes
vhdx  no
lzo support   no
snappy supportno
bzip2 support no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
replication support yes
  GEN   x86_64-softmmu/config-devices.mak.tmp
  GEN   aarch64-softmmu/config-devices.mak.tmp
  GEN   config-host.h
  GEN   qemu-options.def
  GEN   qmp-commands.h
  GEN   qapi-types.h
  GEN   qapi-visit.h
  GEN   qapi-event.h
  GEN   x86_64-softmmu/config-devices.mak
  GEN   

Re: [Qemu-devel] [PATCH v3 14/18] trace: get rid of generated-events.h/generated-events.c

[Lluís Vilanova]

Daniel P Berrange writes:

> Currently the generated-events.[ch] files contain the
> event dstates, constants and TraceEvent structs, while the
> generated-tracers.[ch] files contain the actual trace
> probe logic. With the removal of usage of the event enums
> from the API there is no longer any compelling reason for
> the separation between these files. The generated-events.h
> content is only ever be needed from the generated-tracers.[ch]
> files.

> The enums/constants/structs from generated-events.[ch] are
> thus moved into the generated-tracers.[ch], so that there
> is one less file to be generated.

> Signed-off-by: Daniel P. Berrange 
> ---
>  Makefile  |  3 ---
>  include/trace-tcg.h   |  1 -
>  include/trace.h   |  1 -
>  scripts/tracetool/format/c.py | 51 
> ++-
>  scripts/tracetool/format/h.py | 19 
>  trace/Makefile.objs   | 28 
>  trace/control.h   |  2 +-
>  trace/simple.h|  4 
>  8 files changed, 70 insertions(+), 39 deletions(-)

> diff --git a/Makefile b/Makefile
> index 50b4b3a..16a35b0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -56,9 +56,6 @@ GENERATED_SOURCES += qmp-marshal.c qapi-types.c 
> qapi-visit.c qapi-event.c
>  GENERATED_HEADERS += qmp-introspect.h
>  GENERATED_SOURCES += qmp-introspect.c
 
> -GENERATED_HEADERS += trace/generated-events.h
> -GENERATED_SOURCES += trace/generated-events.c
> -
>  GENERATED_HEADERS += trace/generated-tracers.h
>  ifeq ($(findstring dtrace,$(TRACE_BACKENDS)),dtrace)
>  GENERATED_HEADERS += trace/generated-tracers-dtrace.h
> diff --git a/include/trace-tcg.h b/include/trace-tcg.h
> index edab4b1..da68608 100644
> --- a/include/trace-tcg.h
> +++ b/include/trace-tcg.h
> @@ -2,6 +2,5 @@
>  #define TRACE_TCG_H
 
>  #include "trace/generated-tcg-tracers.h"
> -#include "trace/generated-events.h"
 
>  #endif /* TRACE_TCG_H */
> diff --git a/include/trace.h b/include/trace.h
> index 9a01e44..ac9ff3d 100644
> --- a/include/trace.h
> +++ b/include/trace.h
> @@ -2,6 +2,5 @@
>  #define TRACE_H
 
>  #include "trace/generated-tracers.h"
> -#include "trace/generated-events.h"
 
>  #endif /* TRACE_H */
> diff --git a/scripts/tracetool/format/c.py b/scripts/tracetool/format/c.py
> index 699598f..24f8d2e 100644
> --- a/scripts/tracetool/format/c.py
> +++ b/scripts/tracetool/format/c.py
> @@ -17,12 +17,53 @@ from tracetool import out
 
 
>  def generate(events, backend):
> -events = [e for e in events
> -  if "disable" not in e.properties]
> +active_events = [e for e in events
> + if "disable" not in e.properties]
 
>  out('/* This file is autogenerated by tracetool, do not edit. */',
> +'',
> +'#include "qemu/osdep.h"',
> +'#include "trace.h"',
>  '')
> -backend.generate_begin(events)
> -for event in events:
> +
> +for e in events:
> +out('uint16_t %s;' % e.api(e.QEMU_DSTATE))
> +
> +for e in events:
> +if "vcpu" in e.properties:
> +vcpu_id = 0
> +else:
> +vcpu_id = "TRACE_VCPU_EVENT_NONE"
> +out('TraceEvent %(event)s = {',
> +'  .id = 0,',
> +'  .vcpu_id = %(vcpu_id)s,',
> +'  .name = \"%(name)s\",',
> +'  .sstate = %(sstate)s,',
> +'  .dstate = &%(dstate)s ',
> +'};',
> +event = "TRACE_" + e.name.upper() + "_EV",

Event.api() again.


> +vcpu_id = vcpu_id,
> +name = e.name,
> +sstate = "TRACE_%s_ENABLED" % e.name.upper(),
> +dstate = e.api(e.QEMU_DSTATE))
> +
> +out('TraceEvent *trace_events[] = {')
> +
> +for e in events:
> +out('&%(event)s,',
> +event = "TRACE_" + e.name.upper() + "_EV")
> +
> +out('  NULL,',
> +'};',
> +'')
> +
> +out('static void trace_register_events(void)',
> +'{',
> +'trace_event_register_group(trace_events);',
> +'}',
> +'trace_init(trace_register_events)')
> +
> +backend.generate_begin(active_events)
> +for event in active_events:
>  backend.generate(event)
> -backend.generate_end(events)
> +backend.generate_end(active_events)
> diff --git a/scripts/tracetool/format/h.py b/scripts/tracetool/format/h.py
> index 64a6680..f1dc493 100644
> --- a/scripts/tracetool/format/h.py
> +++ b/scripts/tracetool/format/h.py
> @@ -26,6 +26,25 @@ def generate(events, backend):
>  '#include "trace/control.h"',
>  '')
 
> +for e in events:
> +out('extern TraceEvent TRACE_%s_EV;' % e.name.upper())
> +
> +for e in events:
> +out('extern uint16_t %s;' % e.api(e.QEMU_DSTATE))
> +
> +# static state
> +for e in events:
> +if 'disable' in e.properties:
> +enabled = 0
> +else:
> +enabled = 1
> +if "tcg-exec" 

Re: [Qemu-devel] [PATCH v3 15/18] trace: rename _read_events to read_events

[Lluís Vilanova]

Daniel P Berrange writes:

> The _read_events method is used by callers outside of
> its module, so should be a public method, not private.

> Signed-off-by: Daniel P. Berrange 


Reviewed-by: Lluís Vilanova 


> ---
>  scripts/simpletrace.py|  6 +++---
>  scripts/tracetool/__init__.py | 14 --
>  2 files changed, 15 insertions(+), 5 deletions(-)

> diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
> index 98abb7a..5dc71d3 100755
> --- a/scripts/simpletrace.py
> +++ b/scripts/simpletrace.py
> @@ -12,7 +12,7 @@
>  import struct
>  import re
>  import inspect
> -from tracetool import _read_events, Event
> +from tracetool import read_events, Event
>  from tracetool.backend.simple import is_string
 
>  header_event_id = 0x
> @@ -128,7 +128,7 @@ class Analyzer(object):
>  def process(events, log, analyzer, read_header=True):
>  """Invoke an analyzer on each event in a log."""
>  if isinstance(events, str):
> -events = _read_events(open(events, 'r'))
> +events = read_events(open(events, 'r'))
>  if isinstance(log, str):
>  log = open(log, 'rb')
 
> @@ -187,7 +187,7 @@ def run(analyzer):
>   '\n' % sys.argv[0])
>  sys.exit(1)
 
> -events = _read_events(open(sys.argv[1], 'r'))
> +events = read_events(open(sys.argv[1], 'r'))
>  process(events, sys.argv[2], analyzer, read_header=read_header)
 
>  if __name__ == '__main__':
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 5191df9..1bb3886 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -281,7 +281,17 @@ class Event(object):
>   self)
 
 
> -def _read_events(fobj):
> +def read_events(fobj):
> +"""Generate the output for the given (format, backends) pair.
> +
> +Parameters
> +--
> +fobj : file
> +Event description file.
> +
> +Returns a list of Event objects
> +"""
> +
>  events = []
>  for line in fobj:
>  if not line.strip():
> @@ -390,6 +400,6 @@ def generate(fevents, format, backends,
>  tracetool.backend.dtrace.BINARY = binary
>  tracetool.backend.dtrace.PROBEPREFIX = probe_prefix
 
> -events = _read_events(fevents)
> +events = read_events(fevents)
 
>  tracetool.format.generate(events, format, backend)
> -- 
> 2.7.4