Re: [Qemu-devel] [PATCH 0/7] Delete 16 *_cpu_class_by_name() functions

[Markus Armbruster]

Eduardo Habkost  writes:

> On Wed, May 08, 2019 at 10:34:44AM +0200, Markus Armbruster wrote:
>> Eduardo Habkost  writes:
>> 
>> > On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote:
>> >> Eduardo Habkost  writes:
>> >> 
>> >> > This series adds a new CPUClass::class_name_format field, which
>> >> > allows us to delete 16 of the 21 *_cpu_class_by_name() functions
>> >> > that exist today.
>> >> 
>> >> Which five remain, and why?
>> >
>> > alpha_cpu_class_by_name:
>> > * Translates aliases based on alpha_cpu_aliases;
>> > * Falls back to "ev67" unconditionally
>> >   (there's a "TODO: remove match everything nonsense" comment).
>> >
>> > cris_cpu_class_by_name:
>> > * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY.
>> >
>> > ppc_cpu_class_by_name:
>> > * Supports lookup by PVR if CPU model is a 8 digit hex number;
>> > * Converts CPU model to lowercase.
>> >
>> > superh_cpu_class_by_name:
>> > * Translates "any" alias to TYPE_SH7750R_CPU.
>> >
>> > sparc_cpu_class_by_name:
>> > * Replaces whitespaces with '-' on CPU model name.
>> 
>> I'm of course asking because I wonder whether we can dumb down this CPU
>> naming business to something simpler and more regular.
>
> We can, but that's not on my list of priorities.  Any volunteers?

Fair enough.  Except for...

>> 
> [...]
>> Observations:
>> 
>> * The CPU type name format is generally "%s-T-cpu", where T is either
>>or 64.
>> 
>>   Exceptions:
>> 
>>   - openrisc, sh4 uses or1k, superh instead.  Looks pointless to me.
>> 
>>   - i386 uses x86_64 instead of i38664.  Makes sense.
>> 
>>   - hppa, microblaze, nios2 and tilegx use CPU type name format "T-cpu",
>> ignoring the user's cpu model.  These exceptions looks pointless to
>> me.
>> 
>> * The user's CPU model is generally the "%s" part of the format.
>> 
>>   Exceptions:
>> 
>>   - alpha additionaly recognizes full type names.  If that's useful for
>> alpha (I'm not sure it is), why isn't it useful for all other
>> targets?
>> 
>>   - cris and sh4 additionaly recognize an "any" alias, cris only #ifdef
>> CONFIG_USER_ONLY.
>> 
>> Until PATCH 4, arm also recognizes an "any" alias #ifdef
>> CONFIG_USER_ONLY.  PATCH 4 drops that, because it's redundant with
>> the "any" CPU, which is a copy instead of an alias.  Sure we want to
>> do have different targets do "any" in different ways?
>> 
>> See aliases below.
>> 
>>   - ppc additionaly recognizes PVR aliases and additional (case
>> insensitive) aliases.  Feels overengineered to me.  See aliases
>> below.
>> 
>>   - sparc additionally recognizes aliases with ' ' instead of '-'.
>> Feels pointless to me.  See aliases below.

... this, perhaps:

>> * What about deprecating pointless exceptions?

Deprecating unwanted stuff now is likely to make a later cleanup so much
easier.

>> * Aliases
>> 
>>   We have several targets roll their own CPU name aliases code.
>>   Assuming aliases are here to stay (i.e. we're not deprecating all of
>>   them): what about letting each CPU type specify a set of aliases, so
>>   we can recognize them in generic code?
>
> Yes.  I considered adding alias support to generic code, but
> decided to do this one step at a time.

Okay.  Consider adding suitable TODO comments.

Re: [Qemu-devel] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device

[Yan Zhao]

On Thu, May 09, 2019 at 11:38:34AM +0800, Alex Williamson wrote:
> On Wed, 8 May 2019 23:10:55 -0400
> Yan Zhao  wrote:
> 
> > On Thu, May 09, 2019 at 05:22:42AM +0800, Alex Williamson wrote:
> > > On Wed, 8 May 2019 07:27:40 -0400
> > > Yan Zhao  wrote:
> > >   
> > > > On Wed, May 08, 2019 at 05:18:26AM +0800, Alex Williamson wrote:  
> > > > > On Sun,  5 May 2019 21:49:04 -0400
> > > > > Yan Zhao  wrote:
> > > > > 
> > > > > > version attribute is used to check two mdev devices' compatibility.
> > > > > > 
> > > > > > The key point of this version attribute is that it's rw.
> > > > > > User space has no need to understand internal of device version and 
> > > > > > no
> > > > > > need to compare versions by itself.
> > > > > > Compared to reading version strings from both two mdev devices being
> > > > > > checked, user space only reads from one mdev device's version 
> > > > > > attribute.
> > > > > > After getting its version string, user space writes this string 
> > > > > > into the
> > > > > > other mdev device's version attribute. Vendor driver of mdev device
> > > > > > whose version attribute being written will check device 
> > > > > > compatibility of
> > > > > > the two mdev devices for user space and return success for 
> > > > > > compatibility
> > > > > > or errno for incompatibility.
> > > > > > So two readings of version attributes + checking in user space are 
> > > > > > now
> > > > > > changed to one reading + one writing of version attributes + 
> > > > > > checking in
> > > > > > vendor driver.
> > > > > > Format and length of version strings are now private to vendor 
> > > > > > driver
> > > > > > who can define them freely.
> > > > > > 
> > > > > >  __ user space
> > > > > >   /\  \
> > > > > >  / \write
> > > > > > / read  \
> > > > > >  __/__   ___\|/___
> > > > > > | version | | version |-->check compatibility
> > > > > > --- ---
> > > > > > mdev device A   mdev device B
> > > > > > 
> > > > > > This version attribute is optional. If a mdev device does not 
> > > > > > provide
> > > > > > with a version attribute, this mdev device is incompatible to all 
> > > > > > other
> > > > > > mdev devices.
> > > > > > 
> > > > > > Live migration is able to take advantage of this version attribute.
> > > > > > Before user space actually starts live migration, it can first check
> > > > > > whether two mdev devices are compatible.
> > > > > > 
> > > > > > v2:
> > > > > > 1. added detailed intent and usage
> > > > > > 2. made definition of version string completely private to vendor 
> > > > > > driver
> > > > > >(Alex Williamson)
> > > > > > 3. abandoned changes to sample mdev drivers (Alex Williamson)
> > > > > > 4. mandatory --> optional (Cornelia Huck)
> > > > > > 5. added description for errno (Cornelia Huck)
> > > > > > 
> > > > > > Cc: Alex Williamson 
> > > > > > Cc: Erik Skultety 
> > > > > > Cc: "Dr. David Alan Gilbert" 
> > > > > > Cc: Cornelia Huck 
> > > > > > Cc: "Tian, Kevin" 
> > > > > > Cc: Zhenyu Wang 
> > > > > > Cc: "Wang, Zhi A" 
> > > > > > Cc: Neo Jia 
> > > > > > Cc: Kirti Wankhede 
> > > > > > Cc: Daniel P. Berrangé 
> > > > > > Cc: Christophe de Dinechin 
> > > > > > 
> > > > > > Signed-off-by: Yan Zhao 
> > > > > > ---
> > > > > >  Documentation/vfio-mediated-device.txt | 140 
> > > > > > +
> > > > > >  1 file changed, 140 insertions(+)
> > > > > > 
> > > > > > diff --git a/Documentation/vfio-mediated-device.txt 
> > > > > > b/Documentation/vfio-mediated-device.txt
> > > > > > index c3f69bcaf96e..013a764968eb 100644
> > > > > > --- a/Documentation/vfio-mediated-device.txt
> > > > > > +++ b/Documentation/vfio-mediated-device.txt
> > > > > > @@ -202,6 +202,7 @@ Directories and files under the sysfs for Each 
> > > > > > Physical Device
> > > > > >| |   |--- available_instances
> > > > > >| |   |--- device_api
> > > > > >| |   |--- description
> > > > > > +  | |   |--- version
> > > > > >| |   |--- [devices]
> > > > > >| |--- []
> > > > > >| |   |--- create
> > > > > > @@ -209,6 +210,7 @@ Directories and files under the sysfs for Each 
> > > > > > Physical Device
> > > > > >| |   |--- available_instances
> > > > > >| |   |--- device_api
> > > > > >| |   |--- description
> > > > > > +  | |   |--- version
> > > > > >| |   |--- [devices]
> > > > > >| |--- []
> > > > > >|  |--- create
> > > > > > @@ -216,6 +218,7 @@ Directories and files under the sysfs for Each 
> > > > > > Physical Device
> > > > > >|  |--- available_instances
> > > > > >|  |--- device_api
> > > > > >|  |--- description
> > > > > > +  |  |--- version
> > > > > >|  |--- [devices]
> > > > > 
> > > > > I thought there was a request to make this more specific to 

Re: [Qemu-devel] [PATCH] configure: Require python3 >= 3.5

[Markus Armbruster]

Eduardo Habkost  writes:

> The oldest python3 version in distros that will be supported by
> QEMU 4.1 is 3.5.3 (the one in Debian Stretch).  Error out if
> running python3 < 3.5.
>
> We have a .travis.yml job configured to use Python 3.4.  Change
> it to use Python 3.5.
>
> Signed-off-by: Eduardo Habkost 
> ---
>  configure   | 5 +++--
>  .travis.yml | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 6b3ed8c532..520c207d66 100755
> --- a/configure
> +++ b/configure
> @@ -1841,8 +1841,9 @@ fi
>  
>  # Note that if the Python conditional here evaluates True we will exit
>  # with status 1 which is a shell 'false' value.
> -if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> -  error_exit "Cannot use '$python', Python 2 >= 2.7 or Python 3 is 
> required." \
> +if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7) or \
> +  (3,0) <= sys.version_info < (3,5))'; 
> then
> +  error_exit "Cannot use '$python', Python 2 >= 2.7 or Python 3 >= 3.5 is 
> required." \
>"Use --python=/path/to/python to specify a supported Python."
>  fi
>  
> diff --git a/.travis.yml b/.travis.yml
> index 66448d99d6..0f6986b3f1 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -211,7 +211,7 @@ matrix:
>  - CONFIG="--target-list=x86_64-softmmu"
>language: python
>python:
> -- "3.4"
> +- "3.5"
>  
>  
>  - env:

Easily missed, good work.

My grep for similar references to Python versions we don't support found
a few 2.x, x < 7.  Not this patch's problem, of course, but let me show
them anyway:

* scripts/qapi/common.py

# re.subn() lacks flags support before Python 2.7, use re.compile()

  I'll clean this up.

* tests/image-fuzzer/

  docs/image-fuzzer.txt "Fuzzer requirements" item "17. Should be
  compatible with python version 2.4-2.7".

  Stefan, does the fuzzer need porting to Python 3?

  Two spots in the code are marked as 2.4 work-arounds:

tests/image-fuzzer/qcow2/fuzz.py:in Python 2.4
tests/image-fuzzer/runner.py:# Python 2.4 doesn't support 'finally' 
and 'except' in the same 'try'

Grep also found tests/vm/netbsd and tests/vm/openbsd pass
--python=python2.7 to configure.  Eduardo, should they be upgraded to a
suitable version of Python 3?  Possibly in your "[PATCH] Deprecate
Python 2 support"?

If yes, then https://wiki.qemu.org/Hosts/BSD also needs an update.

Re: [Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB

[David Gibson]

On Wed, May 08, 2019 at 07:19:45PM +0200, Cédric Le Goater wrote:
> The high order bits of the address of the OS event queue is stored in
> bits [4-31] of word2 of the XIVE END internal structures and the low
> order bits in word3. This structure is using Big Endian ordering and
> computing the value requires some simple arithmetic which happens to
> be wrong. The mask removing bits [0-3] of word2 is applied to the
> wrong value and the resulting address is bogus when above 64GB.
> 
> Guests with more than 64GB of RAM will allocate pages for the OS event
> queues which will reside above the 64GB limit. In this case, the XIVE
> device model will wake up the CPUs in case of a notification, such as
> IPIs, but the update of the event queue will be written at the wrong
> place in memory. The result is uncertain as the guest memory is
> trashed and IPI are not delivered.
> 
> Introduce a helper xive_end_qaddr() to compute this value correctly in
> all places where it is used.
> 
> Signed-off-by: Cédric Le Goater 

Applied, thanks.

> ---
>  include/hw/ppc/xive_regs.h | 6 ++
>  hw/intc/spapr_xive.c   | 3 +--
>  hw/intc/xive.c | 9 +++--
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
> index bf36678a242c..1a8c5b5e64f0 100644
> --- a/include/hw/ppc/xive_regs.h
> +++ b/include/hw/ppc/xive_regs.h
> @@ -208,6 +208,12 @@ typedef struct XiveEND {
>  #define xive_end_is_backlog(end)  (be32_to_cpu((end)->w0) & END_W0_BACKLOG)
>  #define xive_end_is_escalate(end) (be32_to_cpu((end)->w0) & 
> END_W0_ESCALATE_CTL)
>  
> +static inline uint64_t xive_end_qaddr(XiveEND *end)
> +{
> +return ((uint64_t) be32_to_cpu(end->w2) & 0x0fff) << 32 |
> +be32_to_cpu(end->w3);
> +}
> +
>  /* Notification Virtual Target (NVT) */
>  typedef struct XiveNVT {
>  uint32_tw0;
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 666e24e9b447..810435c30cc7 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -1150,8 +1150,7 @@ static target_ulong h_int_get_queue_config(PowerPCCPU 
> *cpu,
>  }
>  
>  if (xive_end_is_enqueue(end)) {
> -args[1] = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
> -| be32_to_cpu(end->w3);
> +args[1] = xive_end_qaddr(end);
>  args[2] = xive_get_field32(END_W0_QSIZE, end->w0) + 12;
>  } else {
>  args[1] = 0;
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index a0b87001da25..dcf2fcd10893 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -1042,8 +1042,7 @@ static const TypeInfo xive_source_info = {
>  
>  void xive_end_queue_pic_print_info(XiveEND *end, uint32_t width, Monitor 
> *mon)
>  {
> -uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
> -| be32_to_cpu(end->w3);
> +uint64_t qaddr_base = xive_end_qaddr(end);
>  uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
>  uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
>  uint32_t qentries = 1 << (qsize + 10);
> @@ -1072,8 +1071,7 @@ void xive_end_queue_pic_print_info(XiveEND *end, 
> uint32_t width, Monitor *mon)
>  
>  void xive_end_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon)
>  {
> -uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
> -| be32_to_cpu(end->w3);
> +uint64_t qaddr_base = xive_end_qaddr(end);
>  uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
>  uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1);
>  uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
> @@ -1101,8 +1099,7 @@ void xive_end_pic_print_info(XiveEND *end, uint32_t 
> end_idx, Monitor *mon)
>  
>  static void xive_end_enqueue(XiveEND *end, uint32_t data)
>  {
> -uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
> -| be32_to_cpu(end->w3);
> +uint64_t qaddr_base = xive_end_qaddr(end);
>  uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
>  uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
>  uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1);

-- 
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 v2] target/ppc: Fix xxspltib

[David Gibson]

On Thu, May 09, 2019 at 06:17:33AM +1000, Anton Blanchard wrote:
> xxspltib raises a VMX or a VSX exception depending on the register
> set it is operating on. We had a check, but it was backwards.
> 
> Fixes: f113283525a4 ("target-ppc: add xxspltib instruction")
> Signed-off-by: Anton Blanchard 

Applied, thanks.

> ---
>  target/ppc/translate/vsx-impl.inc.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/ppc/translate/vsx-impl.inc.c 
> b/target/ppc/translate/vsx-impl.inc.c
> index 4d8ca7cf32..4812a374aa 100644
> --- a/target/ppc/translate/vsx-impl.inc.c
> +++ b/target/ppc/translate/vsx-impl.inc.c
> @@ -1355,13 +1355,13 @@ static void gen_xxspltib(DisasContext *ctx)
>  int rt = xT(ctx->opcode);
>  
>  if (rt < 32) {
> -if (unlikely(!ctx->altivec_enabled)) {
> -gen_exception(ctx, POWERPC_EXCP_VPU);
> +if (unlikely(!ctx->vsx_enabled)) {
> +gen_exception(ctx, POWERPC_EXCP_VSXU);
>  return;
>  }
>  } else {
> -if (unlikely(!ctx->vsx_enabled)) {
> -gen_exception(ctx, POWERPC_EXCP_VSXU);
> +if (unlikely(!ctx->altivec_enabled)) {
> +gen_exception(ctx, POWERPC_EXCP_VPU);
>  return;
>  }
>  }

-- 
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 1/3] spapr/xive: EQ page should be naturally aligned

[David Gibson]

On Wed, May 08, 2019 at 07:19:44PM +0200, Cédric Le Goater wrote:
> When the OS configures the EQ page in which to receive event
> notifications from the XIVE interrupt controller, the page should be
> naturally aligned. Add this check.
> 
> Signed-off-by: Cédric Le Goater 

Applied, thanks.

> ---
>  hw/intc/spapr_xive.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 097f88d4608d..666e24e9b447 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -993,6 +993,12 @@ static target_ulong h_int_set_queue_config(PowerPCCPU 
> *cpu,
>  case 16:
>  case 21:
>  case 24:
> +if (!QEMU_IS_ALIGNED(qpage, 1ul << qsize)) {
> +qemu_log_mask(LOG_GUEST_ERROR, "XIVE: EQ @0x%" HWADDR_PRIx
> +  " is not naturally aligned with %" HWADDR_PRIx 
> "\n",
> +  qpage, 1ul << qsize);
> +return H_P4;
> +}
>  end.w2 = cpu_to_be32((qpage >> 32) & 0x0fff);
>  end.w3 = cpu_to_be32(qpage & 0x);
>  end.w0 |= cpu_to_be32(END_W0_ENQUEUE);

-- 
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 3/3] spapr/xive: print out the EQ page address in the monitor

[David Gibson]

On Wed, May 08, 2019 at 07:19:46PM +0200, Cédric Le Goater wrote:
> This proved to be a useful information when debugging issues with OS
> event queues allocated above 64GB.
> 
> Signed-off-by: Cédric Le Goater 

Applied, thanks.

> ---
>  hw/intc/spapr_xive.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index 810435c30cc7..7faf03b1fb7c 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -120,6 +120,7 @@ static int spapr_xive_target_to_end(uint32_t target, 
> uint8_t prio,
>  static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
>Monitor *mon)
>  {
> +uint64_t qaddr_base = xive_end_qaddr(end);
>  uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
>  uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1);
>  uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
> @@ -127,9 +128,9 @@ static void spapr_xive_end_pic_print_info(SpaprXive 
> *xive, XiveEND *end,
>  uint32_t nvt = xive_get_field32(END_W6_NVT_INDEX, end->w6);
>  uint8_t priority = xive_get_field32(END_W7_F0_PRIORITY, end->w7);
>  
> -monitor_printf(mon, "%3d/%d % 6d/%5d ^%d",
> +monitor_printf(mon, "%3d/%d % 6d/%5d @%"PRIx64" ^%d",
> spapr_xive_nvt_to_target(0, nvt),
> -   priority, qindex, qentries, qgen);
> +   priority, qindex, qentries, qaddr_base, qgen);
>  
>  xive_end_queue_pic_print_info(end, 6, mon);
>  monitor_printf(mon, "]");

-- 
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 4/9] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x

[David Gibson]

On Thu, May 09, 2019 at 10:33:24AM +1000, Anton Blanchard wrote:
> Hi Mark,
> 
> > Following on from this I've just gone through the load/store
> > operations once again and spotted two things:
> > 
> > 
> > 1) VSX_LOAD_SCALAR_DS has an extra get_cpu_vsrh() which can be removed
> > 
> > diff --git a/target/ppc/translate/vsx-impl.inc.c
> > b/target/ppc/translate/vsx-impl.inc.c index 11d9b75d01..004ea56c4f
> > 100644 --- a/target/ppc/translate/vsx-impl.inc.c
> > +++ b/target/ppc/translate/vsx-impl.inc.c
> > @@ -329,7 +329,6 @@ static void gen_##name(DisasContext
> > *ctx) \
> > return;
> > \ } \ xth
> > = tcg_temp_new_i64(); \
> > -get_cpu_vsrh(xth, rD(ctx->opcode) + 32);  \
> >  gen_set_access_type(ctx, ACCESS_INT); \
> >  EA = tcg_temp_new();  \
> >  gen_addr_imm_index(ctx, EA, 0x03);\
> 
> Looks good. I also noticed we had two stores that needed to be fixed:
> 
> VSX_LOAD_SCALAR_DS(stxsd, st64_i64)
> VSX_LOAD_SCALAR_DS(stxssp, st32fs)
> 
> > 2) VSX_VECTOR_LOAD_STORE is confusing and should be split into
> > separate VSX_VECTOR_LOAD and VSX_VECTOR_STORE macros
> 
> Good idea. I also removed (what I assume) are redundant set_cpu_vsr*
> and get_cpu_vsr* calls.
> 
> > Does that sound reasonable? I'm also thinking that we should consider
> > adding a CC to stable for patches 4, 5 and 9 in this series since
> > these are genuine regressions.
> 
> Fine with me. If David agrees, I'm not sure if he can rebase them or
> if I can send them manually if they have been already committed.

Usually going to stable is just a matter of pinging Mike Roth and
getting him to include it.  I can queue if somewhere if you like, but
the stable cadance is so slow it tends to get forgotten a bit.

-- 
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] configure: Require python3 >= 3.5

[Thomas Huth]

On 08/05/2019 20.23, Eduardo Habkost wrote:
> The oldest python3 version in distros that will be supported by
> QEMU 4.1 is 3.5.3 (the one in Debian Stretch).  Error out if
> running python3 < 3.5.
> 
> We have a .travis.yml job configured to use Python 3.4.  Change
> it to use Python 3.5.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  configure   | 5 +++--
>  .travis.yml | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index 6b3ed8c532..520c207d66 100755
> --- a/configure
> +++ b/configure
> @@ -1841,8 +1841,9 @@ fi
>  
>  # Note that if the Python conditional here evaluates True we will exit
>  # with status 1 which is a shell 'false' value.
> -if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
> -  error_exit "Cannot use '$python', Python 2 >= 2.7 or Python 3 is 
> required." \
> +if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7) or \
> +  (3,0) <= sys.version_info < (3,5))'; 
> then
> +  error_exit "Cannot use '$python', Python 2 >= 2.7 or Python 3 >= 3.5 is 
> required." \

Nit: There won't be a Python2 > 2.7 anymore, so you could also replace
"2 >= 2.7" with "2.7" here. But well, it will go away next year anyway, so:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PULL v2 00/28] Kconfig for Arm machines

[Thomas Huth]

On 08/05/2019 18.45, Philippe Mathieu-Daudé wrote:
> [clicked ctrl+enter too fast]
> 
> On Wed, May 8, 2019 at 6:43 PM Philippe Mathieu-Daudé  
> wrote:
>> On 5/8/19 5:33 PM, Thomas Huth wrote:
>>> On 08/05/2019 17.09, Peter Maydell wrote:
 On Tue, 7 May 2019 at 14:45, Thomas Huth  wrote:
>
>  Hi Peter,
>
> the following changes since commit 
> a6ae23831b05a11880b40f7d58e332c45a6b04f7:
>
>   Merge remote-tracking branch 
> 'remotes/ehabkost/tags/python-next-pull-request' into staging (2019-05-03 
> 15:26:09 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-05-07
>
> for you to fetch changes up to 69f879e9fefab9aaf24893fe4ce23e07756d703c:
>
>   hw/arm: Remove hard-enablement of the remaining PCI devices (2019-05-07 
> 15:01:47 +0200)
>
> 
> Kconfig settings for the Arm machines
> (v2: Fix the dependency of q35 to AHCI_ICH9 in the second patch)
> 

 Hi -- this is still failing in the build test where I 'make clean'
>>>
>>> Very weird. What is running before the "make clean"? Could you provide
>>> me with the content of i386-softmmu/config-devices.mak please?
>>
>> It worked for me after running 'git fetch --tags', maybe because Thomas
>> used the same tag?
> 
> Maybe because Thomas used the same tag you are still trying the
> previous version?

I did not use the same tag. v1 had pull-request-2019-05-05 while v2 has
pull-request-2019-05-07. So this can not be the reason.

 Thomas

Re: [Qemu-devel] [PATCH 26/26] tcg: Use tlb_fill probe from tlb_vaddr_to_host

[Richard Henderson]

On 4/29/19 10:41 AM, Peter Maydell wrote:
> On Wed, 3 Apr 2019 at 05:05, Richard Henderson
>  wrote:
>>
>> Most of the existing users would continue around a loop which
>> would fault the tlb entry in via a normal load/store.  But for
>> SVE we have a true non-faulting case which requires the new
>> probing form of tlb_fill.
> 
> So am I right in thinking that this fixes a bug where we
> previously would mark a load as faulted if the memory happened
> not to be in the TLB, whereas now we will correctly pull in the
> TLB entry and do the load ?

Yes.

> (Since guest code ought to be handling the "non-first-load
> faulted" case by looping round or otherwise arranging to
> retry, nothing in practice would have noticed this bug, right?)

Yes.

The only case with changed behaviour is (expected to be) SVE no-fault, where
the loop you mention would have produced different incorrect results.


r~

Re: [Qemu-devel] [PULL 10/19] tests/boot_linux_console: increase timeout

[Gerd Hoffmann]

  Hi,

> > Tests can also timeout due to slow downloads of test kernels.
> > Any chance to run the downloads without timeout?
> 
> I acknowledge this is an issue, and have thought about two possible
> ways to solve it:
> 
>  1) Downloading/caching/checking all the test assets in a job "pre-tests"
> plugin.

>  2) Report the test phase (say, setUp()) to the test runner, which
> would allow the runner to:

> I'm very much interested in your opinion so we can evolve the idea into
> implementation.

(1) is the better approach I think.  That way you can give better
feedback to the user, along the lines of "download 2 of 5 in progress".
Also it allows for better management of the assets, you can have tools
to list them / fetch them / copy them from one to another test machine
/ find & cleanup obsolete assets (i.e. Fedora 28 kernel after switching
tests to Fedora 29).

(2) is probably still useful, to report phases of longer running
tests and maybe have separate timeouts for each phase.  But for
assets I think approach (1) is better than a "download asset"
phase.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device

[Alex Williamson]

On Wed, 8 May 2019 23:10:55 -0400
Yan Zhao  wrote:

> On Thu, May 09, 2019 at 05:22:42AM +0800, Alex Williamson wrote:
> > On Wed, 8 May 2019 07:27:40 -0400
> > Yan Zhao  wrote:
> >   
> > > On Wed, May 08, 2019 at 05:18:26AM +0800, Alex Williamson wrote:  
> > > > On Sun,  5 May 2019 21:49:04 -0400
> > > > Yan Zhao  wrote:
> > > > 
> > > > > version attribute is used to check two mdev devices' compatibility.
> > > > > 
> > > > > The key point of this version attribute is that it's rw.
> > > > > User space has no need to understand internal of device version and no
> > > > > need to compare versions by itself.
> > > > > Compared to reading version strings from both two mdev devices being
> > > > > checked, user space only reads from one mdev device's version 
> > > > > attribute.
> > > > > After getting its version string, user space writes this string into 
> > > > > the
> > > > > other mdev device's version attribute. Vendor driver of mdev device
> > > > > whose version attribute being written will check device compatibility 
> > > > > of
> > > > > the two mdev devices for user space and return success for 
> > > > > compatibility
> > > > > or errno for incompatibility.
> > > > > So two readings of version attributes + checking in user space are now
> > > > > changed to one reading + one writing of version attributes + checking 
> > > > > in
> > > > > vendor driver.
> > > > > Format and length of version strings are now private to vendor driver
> > > > > who can define them freely.
> > > > > 
> > > > >  __ user space
> > > > >   /\  \
> > > > >  / \write
> > > > > / read  \
> > > > >  __/__   ___\|/___
> > > > > | version | | version |-->check compatibility
> > > > > --- ---
> > > > > mdev device A   mdev device B
> > > > > 
> > > > > This version attribute is optional. If a mdev device does not provide
> > > > > with a version attribute, this mdev device is incompatible to all 
> > > > > other
> > > > > mdev devices.
> > > > > 
> > > > > Live migration is able to take advantage of this version attribute.
> > > > > Before user space actually starts live migration, it can first check
> > > > > whether two mdev devices are compatible.
> > > > > 
> > > > > v2:
> > > > > 1. added detailed intent and usage
> > > > > 2. made definition of version string completely private to vendor 
> > > > > driver
> > > > >(Alex Williamson)
> > > > > 3. abandoned changes to sample mdev drivers (Alex Williamson)
> > > > > 4. mandatory --> optional (Cornelia Huck)
> > > > > 5. added description for errno (Cornelia Huck)
> > > > > 
> > > > > Cc: Alex Williamson 
> > > > > Cc: Erik Skultety 
> > > > > Cc: "Dr. David Alan Gilbert" 
> > > > > Cc: Cornelia Huck 
> > > > > Cc: "Tian, Kevin" 
> > > > > Cc: Zhenyu Wang 
> > > > > Cc: "Wang, Zhi A" 
> > > > > Cc: Neo Jia 
> > > > > Cc: Kirti Wankhede 
> > > > > Cc: Daniel P. Berrangé 
> > > > > Cc: Christophe de Dinechin 
> > > > > 
> > > > > Signed-off-by: Yan Zhao 
> > > > > ---
> > > > >  Documentation/vfio-mediated-device.txt | 140 
> > > > > +
> > > > >  1 file changed, 140 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/vfio-mediated-device.txt 
> > > > > b/Documentation/vfio-mediated-device.txt
> > > > > index c3f69bcaf96e..013a764968eb 100644
> > > > > --- a/Documentation/vfio-mediated-device.txt
> > > > > +++ b/Documentation/vfio-mediated-device.txt
> > > > > @@ -202,6 +202,7 @@ Directories and files under the sysfs for Each 
> > > > > Physical Device
> > > > >| |   |--- available_instances
> > > > >| |   |--- device_api
> > > > >| |   |--- description
> > > > > +  | |   |--- version
> > > > >| |   |--- [devices]
> > > > >| |--- []
> > > > >| |   |--- create
> > > > > @@ -209,6 +210,7 @@ Directories and files under the sysfs for Each 
> > > > > Physical Device
> > > > >| |   |--- available_instances
> > > > >| |   |--- device_api
> > > > >| |   |--- description
> > > > > +  | |   |--- version
> > > > >| |   |--- [devices]
> > > > >| |--- []
> > > > >|  |--- create
> > > > > @@ -216,6 +218,7 @@ Directories and files under the sysfs for Each 
> > > > > Physical Device
> > > > >|  |--- available_instances
> > > > >|  |--- device_api
> > > > >|  |--- description
> > > > > +  |  |--- version
> > > > >|  |--- [devices]
> > > > 
> > > > I thought there was a request to make this more specific to migration
> > > > by renaming it to something like migration_version.  Also, as an
> > > >
> > > so this attribute may not only include a mdev device's parent device info 
> > > and
> > > mdev type, but also include numeric software version of vendor specific
> > > migration code, right?  
> > 
> > It's a vendor defined string, it should be 

Re: [Qemu-devel] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device

[Yan Zhao]

On Thu, May 09, 2019 at 05:22:42AM +0800, Alex Williamson wrote:
> On Wed, 8 May 2019 07:27:40 -0400
> Yan Zhao  wrote:
> 
> > On Wed, May 08, 2019 at 05:18:26AM +0800, Alex Williamson wrote:
> > > On Sun,  5 May 2019 21:49:04 -0400
> > > Yan Zhao  wrote:
> > >   
> > > > version attribute is used to check two mdev devices' compatibility.
> > > > 
> > > > The key point of this version attribute is that it's rw.
> > > > User space has no need to understand internal of device version and no
> > > > need to compare versions by itself.
> > > > Compared to reading version strings from both two mdev devices being
> > > > checked, user space only reads from one mdev device's version attribute.
> > > > After getting its version string, user space writes this string into the
> > > > other mdev device's version attribute. Vendor driver of mdev device
> > > > whose version attribute being written will check device compatibility of
> > > > the two mdev devices for user space and return success for compatibility
> > > > or errno for incompatibility.
> > > > So two readings of version attributes + checking in user space are now
> > > > changed to one reading + one writing of version attributes + checking in
> > > > vendor driver.
> > > > Format and length of version strings are now private to vendor driver
> > > > who can define them freely.
> > > > 
> > > >  __ user space
> > > >   /\  \
> > > >  / \write
> > > > / read  \
> > > >  __/__   ___\|/___
> > > > | version | | version |-->check compatibility
> > > > --- ---
> > > > mdev device A   mdev device B
> > > > 
> > > > This version attribute is optional. If a mdev device does not provide
> > > > with a version attribute, this mdev device is incompatible to all other
> > > > mdev devices.
> > > > 
> > > > Live migration is able to take advantage of this version attribute.
> > > > Before user space actually starts live migration, it can first check
> > > > whether two mdev devices are compatible.
> > > > 
> > > > v2:
> > > > 1. added detailed intent and usage
> > > > 2. made definition of version string completely private to vendor driver
> > > >(Alex Williamson)
> > > > 3. abandoned changes to sample mdev drivers (Alex Williamson)
> > > > 4. mandatory --> optional (Cornelia Huck)
> > > > 5. added description for errno (Cornelia Huck)
> > > > 
> > > > Cc: Alex Williamson 
> > > > Cc: Erik Skultety 
> > > > Cc: "Dr. David Alan Gilbert" 
> > > > Cc: Cornelia Huck 
> > > > Cc: "Tian, Kevin" 
> > > > Cc: Zhenyu Wang 
> > > > Cc: "Wang, Zhi A" 
> > > > Cc: Neo Jia 
> > > > Cc: Kirti Wankhede 
> > > > Cc: Daniel P. Berrangé 
> > > > Cc: Christophe de Dinechin 
> > > > 
> > > > Signed-off-by: Yan Zhao 
> > > > ---
> > > >  Documentation/vfio-mediated-device.txt | 140 +
> > > >  1 file changed, 140 insertions(+)
> > > > 
> > > > diff --git a/Documentation/vfio-mediated-device.txt 
> > > > b/Documentation/vfio-mediated-device.txt
> > > > index c3f69bcaf96e..013a764968eb 100644
> > > > --- a/Documentation/vfio-mediated-device.txt
> > > > +++ b/Documentation/vfio-mediated-device.txt
> > > > @@ -202,6 +202,7 @@ Directories and files under the sysfs for Each 
> > > > Physical Device
> > > >| |   |--- available_instances
> > > >| |   |--- device_api
> > > >| |   |--- description
> > > > +  | |   |--- version
> > > >| |   |--- [devices]
> > > >| |--- []
> > > >| |   |--- create
> > > > @@ -209,6 +210,7 @@ Directories and files under the sysfs for Each 
> > > > Physical Device
> > > >| |   |--- available_instances
> > > >| |   |--- device_api
> > > >| |   |--- description
> > > > +  | |   |--- version
> > > >| |   |--- [devices]
> > > >| |--- []
> > > >|  |--- create
> > > > @@ -216,6 +218,7 @@ Directories and files under the sysfs for Each 
> > > > Physical Device
> > > >|  |--- available_instances
> > > >|  |--- device_api
> > > >|  |--- description
> > > > +  |  |--- version
> > > >|  |--- [devices]  
> > > 
> > > I thought there was a request to make this more specific to migration
> > > by renaming it to something like migration_version.  Also, as an
> > >  
> > so this attribute may not only include a mdev device's parent device info 
> > and
> > mdev type, but also include numeric software version of vendor specific
> > migration code, right?
> 
> It's a vendor defined string, it should be considered opaque to the
> user, the vendor can include whatever they feel is relevant.
> 
> > This actually makes sense.
> > So, do I need to add a disclaimer in this doc like:
> > vendor driver should be responsible by itself for a mdev device's migration
> > compatibility. 
> 
> I thought that was the purpose of this attribute.
> 
> > During migration setup 

Re: [Qemu-devel] [PATCH 00/11] kvm/migration: support KVM_CLEAR_DIRTY_LOG

[Peter Xu]

On Wed, May 08, 2019 at 01:55:07PM +0200, Paolo Bonzini wrote:
> On 08/05/19 06:39, Peter Xu wrote:
> >> The disadvantage of this is that you won't clear in the kernel those
> >> dirty bits that come from other sources (e.g. vhost or
> >> address_space_map).  This can lead to double-copying of pages.
> >>
> >> Migration already makes a local copy in rb->bmap, and
> >> memory_region_snapshot_and_clear_dirty can also do the clear.  Would it
> >> be possible to invoke the clear using rb->bmap instead of the KVMSlot's
> >> new bitmap?
> >
> > Actually that's what I did in the first version before I post the work
> > but I noticed that there seems to have a race condition with the
> > design.  The problem is we have multiple copies of the same dirty
> > bitmap from KVM and the race can happen with those multiple users
> > (bitmaps of the users can be a merged version containing KVM and other
> > sources like vhost, address_space_map, etc. but let's just make it
> > simpler to not have them yet).
> 
> I see now.  And in fact the same double-copying inefficiency happens
> already without this series, so you are improving the situation anyway.
> 
> Have you done any kind of benchmarking already?

Not yet.  I posted the series for some initial reviews first before
moving on with performance tests.

My plan of the test scenario could be:

- find a guest with relatively large memory (I would guess it is
  better to have memory like 64G or even more to make some big
  difference)

- run random dirty memory workload upon most of the mem, with dirty
  rate X Bps.

- setup the migration bandwidth to Y Bps (Y should be bigger than X
  but not that big.  One could be X=800M and Y=1G to emulate 10G nic
  with a workload that we can still converge with precopy only) and
  start precopy migration.

- measure total migration time with CLEAR_LOG on & off. We should
  expect the guest to have these with CLEAR_LOG: (1) not hang during
  log_sync, and (2) migration should complete faster.

Does above test plan makes sense?

If both the QEMU/KVM changes looks ok in general, I can at least try
this on some smaller guests (I can manage ~10G mem guests with my own
hosts, but I can also try to find some bigger ones).

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 17/26] target/s390x: Convert to CPUClass::tlb_fill

[Richard Henderson]

On 4/3/19 4:17 AM, David Hildenbrand wrote:
>> +/*
>> + * Note that handle_mmu_fault sets ilen to either 2 (for code)
> This comment no longer matches.
> 
>> + * or AUTO (for data).  We can resolve AUTO now, as if it was
>> + * set to UNWIND -- that will have been done via assignment
>> + * in cpu_restore_state.  Otherwise re-examine access_type.
>> + */
>> +if (access_type == MMU_INST_FETCH) {
>> +CPUS390XState *env = cs->env_ptr;
>> +env->int_pgm_ilen = 2;
>> +}

Indeed it doesn't.  It's also confusingly written.
I've tried again as

/*
 * The ILC value for code accesses is undefined.  The important
 * thing here is to *not* leave env->int_pgm_ilen set to ILEN_AUTO,
 * which would cause do_program_interrupt to attempt to read from
 * env->psw.addr again.  C.f. the condition in trigger_page_fault,
 * but is not universally applied.
 *
 * ??? If we remove ILEN_AUTO, by moving the computation of ILEN
 * into cpu_restore_state, then we may remove this entirely.
 */
if (access_type == MMU_INST_FETCH) {
env->int_pgm_ilen = 2;
}

I'll just note in passing that the ??? part of the comment alludes to

https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg00063.html

to which I ought to return at some point.


r~

[Qemu-devel] [PATCH v2] target/ppc: Fix xvabs[sd]p, xvnabs[sd]p, xvneg[sd]p, xvcpsgn[sd]p

[Anton Blanchard]

We were using set_cpu_vsr*() when we should have used get_cpu_vsr*().

Fixes: 8b3b2d75c7c0 ("introduce get_cpu_vsr{l,h}() and set_cpu_vsr{l,h}() 
helpers for VSR register access")
Signed-off-by: Anton Blanchard 
---
 target/ppc/translate/vsx-impl.inc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index b487136d52..4b7627f53b 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -859,8 +859,8 @@ static void glue(gen_, name)(DisasContext *ctx) 
 \
 xbh = tcg_temp_new_i64();\
 xbl = tcg_temp_new_i64();\
 sgm = tcg_temp_new_i64();\
-set_cpu_vsrh(xB(ctx->opcode), xbh);  \
-set_cpu_vsrl(xB(ctx->opcode), xbl);  \
+get_cpu_vsrh(xbh, xB(ctx->opcode));  \
+get_cpu_vsrl(xbl, xB(ctx->opcode));  \
 tcg_gen_movi_i64(sgm, sgn_mask); \
 switch (op) {\
 case OP_ABS: {   \
-- 
2.20.1

Re: [Qemu-devel] [PATCH 23/26] target/xtensa: Convert to CPUClass::tlb_fill

[Max Filippov]

On Tue, Apr 30, 2019 at 2:07 PM Max Filippov  wrote:
> On Tue, Apr 30, 2019 at 11:14 AM Max Filippov  wrote:
> > On Tue, Apr 30, 2019 at 10:44 AM Richard Henderson
> > > And Peter's right that I should have kept EXC_USER.
>
> It appears to work as is: the EXC_USER is set up by the
> exception_cause helper because there's always PS_U in
> the PS, PS_EXCM is cleared in the cpu_loop and the
> current PC is preserved by the xtensa_cpu_tlb_fill.
> I'll play with it some more...

I've run gcc/uclibc testsuites for xtensa-linux with this series as is,
got no new regressions.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 4/9] target/ppc: Fix lxvw4x, lxvh8x and lxvb16x

[Anton Blanchard]

Hi Mark,

> Following on from this I've just gone through the load/store
> operations once again and spotted two things:
> 
> 
> 1) VSX_LOAD_SCALAR_DS has an extra get_cpu_vsrh() which can be removed
> 
> diff --git a/target/ppc/translate/vsx-impl.inc.c
> b/target/ppc/translate/vsx-impl.inc.c index 11d9b75d01..004ea56c4f
> 100644 --- a/target/ppc/translate/vsx-impl.inc.c
> +++ b/target/ppc/translate/vsx-impl.inc.c
> @@ -329,7 +329,6 @@ static void gen_##name(DisasContext
> *ctx) \
> return;
> \ } \ xth
> = tcg_temp_new_i64(); \
> -get_cpu_vsrh(xth, rD(ctx->opcode) + 32);  \
>  gen_set_access_type(ctx, ACCESS_INT); \
>  EA = tcg_temp_new();  \
>  gen_addr_imm_index(ctx, EA, 0x03);\

Looks good. I also noticed we had two stores that needed to be fixed:

VSX_LOAD_SCALAR_DS(stxsd, st64_i64)
VSX_LOAD_SCALAR_DS(stxssp, st32fs)

> 2) VSX_VECTOR_LOAD_STORE is confusing and should be split into
> separate VSX_VECTOR_LOAD and VSX_VECTOR_STORE macros

Good idea. I also removed (what I assume) are redundant set_cpu_vsr*
and get_cpu_vsr* calls.

> Does that sound reasonable? I'm also thinking that we should consider
> adding a CC to stable for patches 4, 5 and 9 in this series since
> these are genuine regressions.

Fine with me. If David agrees, I'm not sure if he can rebase them or
if I can send them manually if they have been already committed.

Thanks,
Anton

[Qemu-devel] [PATCH] target/ppc: Optimise VSX_LOAD_SCALAR_DS and VSX_VECTOR_LOAD_STORE

[Anton Blanchard]

A few small optimisations:

In VSX_LOAD_SCALAR_DS() we can don't need to read the VSR via
get_cpu_vsrh().

Split VSX_VECTOR_LOAD_STORE() into two functions. Loads only need to
write the VSRs (set_cpu_vsr*()) and stores only need to read the VSRs
(get_cpu_vsr*())

Thanks to Mark Cave-Ayland for the suggestions.

Signed-off-by: Anton Blanchard 
---
 target/ppc/translate/vsx-impl.inc.c | 68 -
 1 file changed, 58 insertions(+), 10 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index 4b7627f53b..cdb44b8b70 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -228,7 +228,7 @@ static void gen_lxvb16x(DisasContext *ctx)
 tcg_temp_free_i64(xtl);
 }
 
-#define VSX_VECTOR_LOAD_STORE(name, op, indexed)\
+#define VSX_VECTOR_LOAD(name, op, indexed)  \
 static void gen_##name(DisasContext *ctx)   \
 {   \
 int xt; \
@@ -255,8 +255,6 @@ static void gen_##name(DisasContext *ctx)   
\
 }   \
 xth = tcg_temp_new_i64();   \
 xtl = tcg_temp_new_i64();   \
-get_cpu_vsrh(xth, xt);  \
-get_cpu_vsrl(xtl, xt);  \
 gen_set_access_type(ctx, ACCESS_INT);   \
 EA = tcg_temp_new();\
 if (indexed) {  \
@@ -282,10 +280,61 @@ static void gen_##name(DisasContext *ctx) 
  \
 tcg_temp_free_i64(xtl); \
 }
 
-VSX_VECTOR_LOAD_STORE(lxv, ld_i64, 0)
-VSX_VECTOR_LOAD_STORE(stxv, st_i64, 0)
-VSX_VECTOR_LOAD_STORE(lxvx, ld_i64, 1)
-VSX_VECTOR_LOAD_STORE(stxvx, st_i64, 1)
+VSX_VECTOR_LOAD(lxv, ld_i64, 0)
+VSX_VECTOR_LOAD(lxvx, ld_i64, 1)
+
+#define VSX_VECTOR_STORE(name, op, indexed) \
+static void gen_##name(DisasContext *ctx)   \
+{   \
+int xt; \
+TCGv EA;\
+TCGv_i64 xth;   \
+TCGv_i64 xtl;   \
+\
+if (indexed) {  \
+xt = xT(ctx->opcode);   \
+} else {\
+xt = DQxT(ctx->opcode); \
+}   \
+\
+if (xt < 32) {  \
+if (unlikely(!ctx->vsx_enabled)) {  \
+gen_exception(ctx, POWERPC_EXCP_VSXU);  \
+return; \
+}   \
+} else {\
+if (unlikely(!ctx->altivec_enabled)) {  \
+gen_exception(ctx, POWERPC_EXCP_VPU);   \
+return; \
+}   \
+}   \
+xth = tcg_temp_new_i64();   \
+xtl = tcg_temp_new_i64();   \
+get_cpu_vsrh(xth, xt);  \
+get_cpu_vsrl(xtl, xt);  \
+gen_set_access_type(ctx, ACCESS_INT);   \
+EA = tcg_temp_new();\
+if (indexed) {  \
+gen_addr_reg_index(ctx, EA);\
+} else {\
+gen_addr_imm_index(ctx, EA, 0x0F);  \
+}   \
+if (ctx->le_mode) { \
+tcg_gen_qemu_##op(xtl, EA, ctx->mem_idx, MO_LEQ);   \
+tcg_gen_addi_tl(EA, EA, 8); \
+tcg_gen_qemu_##op(xth, EA, ctx->mem_idx, MO_LEQ);   \
+} else {\
+tcg_gen_qemu_##op(xth, EA, ctx->mem_idx, MO_BEQ);   \
+tcg_gen_addi_tl(EA, EA, 8); \
+tcg_gen_qemu_##op(xtl, EA, ctx->mem_idx, MO_BEQ);   \
+}   \
+tcg_temp_free(EA);  \
+tcg_temp_free_i64(xth);   

[Qemu-devel] [PATCH] Multiple ramfb enhancements

[Hou Qiming]

Pulled back the `qemu_create_displaysurface_guestmem` function to create
the display surface so that the guest memory gets properly unmaped.

Only allow one resolution change per guest boot, which prevents a
crash when the guest writes garbage to the configuration space (e.g.
when rebooting).

Write an initial resolution to the configuration space on guest reset,
which a later BIOS / OVMF patch can take advantage of.

Signed-off-by: HOU Qiming 
---
 hw/display/ramfb-standalone.c | 12 -
 hw/display/ramfb.c| 91 +--
 hw/vfio/display.c |  4 +-
 hw/vfio/pci.c |  6 ++-
 include/hw/display/ramfb.h|  2 +-
 stubs/ramfb.c |  2 +-
 6 files changed, 96 insertions(+), 21 deletions(-)

diff --git a/hw/display/ramfb-standalone.c b/hw/display/ramfb-standalone.c
index da3229a..6441449 100644
--- a/hw/display/ramfb-standalone.c
+++ b/hw/display/ramfb-standalone.c
@@ -1,6 +1,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "hw/loader.h"
+#include "hw/isa/isa.h"
 #include "hw/display/ramfb.h"
 #include "ui/console.h"
 #include "sysemu/sysemu.h"
@@ -11,6 +12,8 @@ typedef struct RAMFBStandaloneState {
 SysBusDevice parent_obj;
 QemuConsole *con;
 RAMFBState *state;
+uint32_t xres;
+uint32_t yres;
 } RAMFBStandaloneState;

 static void display_update_wrapper(void *dev)
@@ -33,15 +36,22 @@ static void ramfb_realizefn(DeviceState *dev, Error
**errp)
 RAMFBStandaloneState *ramfb = RAMFB(dev);

 ramfb->con = graphic_console_init(dev, 0, _ops, dev);
-ramfb->state = ramfb_setup(errp);
+ramfb->state = ramfb_setup(dev, errp);
 }

+static Property ramfb_properties[] = {
+DEFINE_PROP_UINT32("xres", RAMFBStandaloneState, xres, 0),
+DEFINE_PROP_UINT32("yres", RAMFBStandaloneState, yres, 0),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void ramfb_class_initfn(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);

 set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
 dc->realize = ramfb_realizefn;
+dc->props = ramfb_properties;
 dc->desc = "ram framebuffer standalone device";
 dc->user_creatable = true;
 }
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index 25c8ad7..0033ac8 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -12,6 +12,7 @@
  */
 #include "qemu/osdep.h"
 #include "qapi/error.h"
+#include "qemu/option.h"
 #include "hw/loader.h"
 #include "hw/display/ramfb.h"
 #include "ui/console.h"
@@ -29,18 +30,57 @@ struct QEMU_PACKED RAMFBCfg {
 struct RAMFBState {
 DisplaySurface *ds;
 uint32_t width, height;
+uint32_t starting_width, starting_height;
+hwaddr addr, length;
 struct RAMFBCfg cfg;
+bool locked;
 };

+static void qemu_unmap_displaysurface_guestmem(pixman_image_t *image,
+   void *unused)
+{
+void *data = pixman_image_get_data(image);
+uint32_t size = pixman_image_get_stride(image) *
+pixman_image_get_height(image);
+cpu_physical_memory_unmap(data, size, 0, 0);
+}
+
+static DisplaySurface *qemu_create_displaysurface_guestmem(
+int width, int height,
+pixman_format_code_t format,
+int linesize, uint64_t addr)
+{
+DisplaySurface *surface;
+hwaddr size;
+void *data;
+
+if (linesize == 0) {
+linesize = width * PIXMAN_FORMAT_BPP(format) / 8;
+}
+
+size = (hwaddr)linesize * height;
+data = cpu_physical_memory_map(addr, , 0);
+if (size != (hwaddr)linesize * height) {
+cpu_physical_memory_unmap(data, size, 0, 0);
+return NULL;
+}
+
+surface = qemu_create_displaysurface_from
+(width, height, format, linesize, data);
+pixman_image_set_destroy_function
+(surface->image, qemu_unmap_displaysurface_guestmem, NULL);
+
+return surface;
+}
+
 static void ramfb_fw_cfg_write(void *dev, off_t offset, size_t len)
 {
 RAMFBState *s = dev;
-void *framebuffer;
-uint32_t fourcc, format;
+uint32_t fourcc, format, width, height;
 hwaddr stride, addr, length;

-s->width  = be32_to_cpu(s->cfg.width);
-s->height = be32_to_cpu(s->cfg.height);
+width = be32_to_cpu(s->cfg.width);
+height= be32_to_cpu(s->cfg.height);
 stride= be32_to_cpu(s->cfg.stride);
 fourcc= be32_to_cpu(s->cfg.fourcc);
 addr  = be64_to_cpu(s->cfg.addr);
@@ -48,17 +88,18 @@ static void ramfb_fw_cfg_write(void *dev, off_t offset,
size_t len)
 format= qemu_drm_format_to_pixman(fourcc);

 fprintf(stderr, "%s: %dx%d @ 0x%" PRIx64 "\n", __func__,
-s->width, s->height, addr);
-framebuffer = address_space_map(_space_memory,
-addr, , false,
-MEMTXATTRS_UNSPECIFIED);
-if (!framebuffer || length < stride * s->height) {
-s->width = 0;
-s->height = 0;
+width, height, addr);
+if (s->locked) {
+

Re: [Qemu-devel] [PATCH] blockdev-backup: don't check aio_context too early

[John Snow]

On 5/7/19 4:50 AM, Kevin Wolf wrote:
> Am 06.05.2019 um 22:33 hat John Snow geschrieben:
>> in blockdev_backup_prepare, we check to make sure that the target is
>> associated with a compatible aio context. However, do_blockdev_backup is
>> called later and has some logic to move the target to a compatible
>> aio_context. The transaction version will fail certain commands
>> needlessly early as a result.
>>
>> Allow blockdev_backup_prepare to simply call do_blockdev_backup, which
>> will ultimately decide if the contexts are compatible or not.
>>
>> Note: the transaction version has always disallowed this operation since
>> its initial commit bd8baecd (2014), whereas the version of
>> qmp_blockdev_backup at the time, from commit c29c1dd312f, tried to
>> enforce the aio_context switch instead. It's not clear, and I can't see
>> from the mailing list archives at the time, why the two functions take a
>> different approach. It wasn't until later in efd7556708b (2016) that the
>> standalone version tried to determine if it could set the context or
>> not.
>>
>> Reported-by: aihua liang 
>> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1683498
> 
> Signed-off-by is missing, and a testcase, too. :-)
> 
>> diff --git a/blockdev.c b/blockdev.c
>> index 79fbac8450..a81d88980c 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -1872,10 +1872,6 @@ static void blockdev_backup_prepare(BlkActionState 
>> *common, Error **errp)
>>  }
>>  
>>  aio_context = bdrv_get_aio_context(bs);
>> -if (aio_context != bdrv_get_aio_context(target)) {
>> -error_setg(errp, "Backup between two IO threads is not 
>> implemented");
>> -return;
>> -}
>>  aio_context_acquire(aio_context);
>>  state->bs = bs;
> 
> The actual change looks good to me.
> 
> Kevin
> 

(See the Red Hat BZ for details on the test I am more or less trying to
replicate in iotests -- but the jist of it is using an iothread on one
device and not specifying one for the other, then backing up both to two
blockdev-add created nodes not attached to any device.)

Is there some trick to getting this to fail with accel=qtest? I'm
noticing that if the VM is paused or if I use accel=qtest, the iothread
contexts for the drives appear to go ... unresolved? and the test passes.

I've only had luck (so far) reproducing this with accel=kvm on a running
VM (the drives can be empty) and I don't actually know why that is just
yet -- I guess these aio_context objects get resolved at runtime?

Anyway, this seems to be a little tricky so far, maybe you have some
advice for me?

--js

(Also note: doing a full backup and an incremental backup for two
perfectly empty 64GB qcow2 drives takes over 6 seconds in total. It
probably shouldn't, right? There's something about the initial backup
that appears to take a pretty long time.)

Re: [Qemu-devel] [PATCH v3 18/39] target/mips: Use env_cpu, env_archcpu

[Philippe Mathieu-Daudé]

Hi Richard, Aleksandar.

On 5/8/19 4:32 PM, Richard Henderson wrote:
> On 5/8/19 1:15 AM, Aleksandar Markovic wrote:
>>
>> On May 8, 2019 2:19 AM, "Richard Henderson" > > wrote:
>>>
>>>
>>>
>>
>> This commit message doesnˊt explain the reason for the change, and why is 
>> this
>> an improvement. The underlyng reason for distingishing between  env_cpu and
>> env_archcpu cases is not explained too.
> 
> It's certainly explained in the preceeding patches that introduce those 
> functions.
> 
> Are you suggesting that it is beneficial to copy-and-paste a common block
> explanation into 21 commit messages for each of target/foo/?


*) Richard:

I tried to put myself in Aleksandar shoes. I believe Aleksandar is
worried about his MIPS maintainer duty, wanting to Ack-by this patch.

It is true that out of the context of the series, it is hard to see what
is the problem you try to solve.

You could copy/paste the explanation you used previously,
with s/$arch/mips/:

"Cleanup in the boilerplate that each target must define."

"Combined uses of CPU(mips_env_get_cpu()) were failures to use
the more proper, ENV_GET_CPU macro, now replaced by env_cpu."

Now to clearly understand this patch we still need to look at the
previous two arch-generic patches
- "cpu: Replace ENV_GET_CPU with env_cpu" and
- "cpu: Introduce env_archcpu".

Also, it is tedious to copy/paste the same explanation, but thinking of
forks or stable branch that cherry-pick not all but some commits of a
series, it might be useful.

Another guess is Aleksandar might have looked at the series cover, which
is not well explained as your v2:
https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg07635.html
I think you mistakenly copied the v1 blurb instead of the v2 one.

So at some point I can understand Aleksandar frustation.


*) Aleksandar:

This series fall under the "Overall Guest CPU cores (TCG)" section
maintained by Richard and Paolo. I think you have to see this series as
a whole to understand the benefits of it.

With the same reasoning, I believe you shouldn't worry to not give your
Ack if you don't feel comfortable.

I think Richard sent this v3 to simply address comments raised by the
previous reviewer during v1/v2, where there was some discussions: I took
it as "this is the last round before getting merged" (unless someone
object).

It is hard to make everybody happy on a such big project, with so many
areas, lines of code, people, culture, etc... I believe we all try to
give our best, neither the commiters nor the reviewers are perfect, but
slowly we help this project to improve :)


Best regards,

Phil.

Re: [Qemu-devel] [PATCH v2 7/7] block: Ignore loosening perm restrictions failures

[Max Reitz]

On 08.05.19 20:25, Max Reitz wrote:
> We generally assume that loosening permission restrictions can never
> fail.  We have seen in the past that this assumption is wrong.  This has
> led to crashes because we generally pass _abort when loosening
> permissions.
> 
> However, a failure in such a case should actually be handled in quite
> the opposite way: It is very much not fatal, so qemu may report it, but
> still consider the operation successful.  The only realistic problem is
> that qemu may then retain permissions and thus locks on images it
> actually does not require.  But again, that is not fatal.
> 
> To implement this behavior, we make all functions that change
> permissions and that pass _abort to the initiating function
> (bdrv_check_perm() or bdrv_child_check_perm()) evaluate the
> @loosen_restrictions value introduced in the previous patch.  If it is
> true and an error did occur, we abort the permission update, report
> the error, and report success to the caller.

Hm, I just noticed that while make check passes, it emits two of these
warnings:

TEST: tests/i440fx-test... (pid=23021)
qemu-system-x86_64: warning: Failed to loosen restrictions: Could not
reopen file: No such file or directory
qemu-system-x86_64: warning: Failed to loosen restrictions: Could not
reopen file: No such file or directory
PASS: tests/i440fx-test

This is because the test creates temporary files which it unlinks after
qemu has opened them.  The bdrv_close_all() then attempts to drop the
WRITE permission, which requires reopening the file, which fails.

(Reproducer:

$ touch /tmp/foo; x86_64-softmmu/qemu-system-x86_64 -hda /tmp/foo &; \
  sleep 1; rm /tmp/foo; kill %1
[1] 23236
WARNING: Image format was not specified for '/tmp/foo' and probing
guessed raw.
 Automatically detecting the format is dangerous for raw images,
write operations on block 0 will be restricted.
 Specify the 'raw' format explicitly to remove the restrictions.
qemu-system-x86_64: terminating on signal 15 from pid 7922 (-zsh)



qemu-system-x86_64: warning: Failed to loosen restrictions: Could not
reopen file: No such file or directory
qemu-system-x86_64: warning: Failed to loosen restrictions: Could not
reopen file: No such file or directory
[1]  + 23236 done   x86_64-softmmu/qemu-system-x86_64 -hda /tmp/foo

)

Should I just drop the warning?

Max

> bdrv_child_try_set_perm() itself does not pass _abort, but it is
> the only public function to change permissions.  As such, callers may
> pass _abort to it, expecting dropping permission restrictions to
> never fail.
> 
> Signed-off-by: Max Reitz 
> ---
>  block.c | 40 ++--
>  1 file changed, 34 insertions(+), 6 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 8f517be5b4..a5a03906d0 100644
> --- a/block.c
> +++ b/block.c
> @@ -2088,11 +2088,20 @@ static void bdrv_child_abort_perm_update(BdrvChild *c)
>  int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
>  Error **errp)
>  {
> +Error *local_err = NULL;
>  int ret;
> +bool tighten_restrictions;
>  
> -ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL, NULL, errp);
> +ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL,
> +_restrictions, _err);
>  if (ret < 0) {
>  bdrv_child_abort_perm_update(c);
> +if (tighten_restrictions) {
> +error_propagate(errp, local_err);
> +} else {
> +warn_reportf_err(local_err, "Failed to loosen restrictions: ");
> +ret = 0;
> +}
>  return ret;
>  }
>  
> @@ -2275,10 +2284,20 @@ static void bdrv_replace_child(BdrvChild *child, 
> BlockDriverState *new_bs)
>  /* Update permissions for old node. This is guaranteed to succeed
>   * because we're just taking a parent away, so we're loosening
>   * restrictions. */
> +bool tighten_restrictions;
> +Error *local_err = NULL;
> +int ret;
> +
>  bdrv_get_cumulative_perm(old_bs, , _perm);
> -bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL,
> -NULL, _abort);
> -bdrv_set_perm(old_bs, perm, shared_perm);
> +ret = bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL,
> +  _restrictions, _err);
> +assert(tighten_restrictions == false);
> +if (ret < 0) {
> +warn_reportf_err(local_err, "Failed to loosen restrictions: ");
> +bdrv_abort_perm_update(old_bs);
> +} else {
> +bdrv_set_perm(old_bs, perm, shared_perm);
> +}
>  }
>  }
>  
> @@ -5322,7 +5341,9 @@ static bool bdrv_has_bds_parent(BlockDriverState *bs, 
> bool only_active)
>  static int bdrv_inactivate_recurse(BlockDriverState *bs)
>  {
>  BdrvChild *child, *parent;
> +bool tighten_restrictions;
>  uint64_t perm, shared_perm;
> +Error *local_err = NULL;
>   

[Qemu-devel] [PATCH v3 2/5] iotests.py: Add qemu_nbd_early_pipe()

[Max Reitz]

qemu_nbd_pipe() currently unconditionally reads qemu-nbd's output.  That
is not ideal because qemu-nbd may keep stderr open after the parent
process has exited.

Currently, the only user of qemu_nbd_pipe() is 147, which discards the
whole output if the parent process returned success and only evaluates
it on error.  Therefore, we can replace qemu_nbd_pipe() by
qemu_nbd_early_pipe() that does the same: Discard the output on success,
and return it on error.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/147| 4 ++--
 tests/qemu-iotests/iotests.py | 9 ++---
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
index 82513279b0..2d84fddb01 100755
--- a/tests/qemu-iotests/147
+++ b/tests/qemu-iotests/147
@@ -24,7 +24,7 @@ import socket
 import stat
 import time
 import iotests
-from iotests import cachemode, imgfmt, qemu_img, qemu_nbd, qemu_nbd_pipe
+from iotests import cachemode, imgfmt, qemu_img, qemu_nbd, qemu_nbd_early_pipe
 
 NBD_PORT_START  = 32768
 NBD_PORT_END= NBD_PORT_START + 1024
@@ -93,7 +93,7 @@ class QemuNBD(NBDBlockdevAddBase):
 pass
 
 def _try_server_up(self, *args):
-status, msg = qemu_nbd_pipe('-f', imgfmt, test_img, *args)
+status, msg = qemu_nbd_early_pipe('-f', imgfmt, test_img, *args)
 if status == 0:
 return True
 if 'Address already in use' in msg:
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f811f69135..ce21d83182 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -204,9 +204,9 @@ def qemu_nbd(*args):
 '''Run qemu-nbd in daemon mode and return the parent's exit code'''
 return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
 
-def qemu_nbd_pipe(*args):
+def qemu_nbd_early_pipe(*args):
 '''Run qemu-nbd in daemon mode and return both the parent's exit code
-   and its output'''
+   and its output in case of an error'''
 subp = subprocess.Popen(qemu_nbd_args + ['--fork'] + list(args),
 stdout=subprocess.PIPE,
 stderr=subprocess.STDOUT,
@@ -216,7 +216,10 @@ def qemu_nbd_pipe(*args):
 sys.stderr.write('qemu-nbd received signal %i: %s\n' %
  (-exitcode,
   ' '.join(qemu_nbd_args + ['--fork'] + list(args
-return exitcode, subp.communicate()[0]
+if exitcode == 0:
+return exitcode, ''
+else:
+return exitcode, subp.communicate()[0]
 
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
 '''Return True if two image files are identical'''
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device

[Alex Williamson]

On Wed, 8 May 2019 07:27:40 -0400
Yan Zhao  wrote:

> On Wed, May 08, 2019 at 05:18:26AM +0800, Alex Williamson wrote:
> > On Sun,  5 May 2019 21:49:04 -0400
> > Yan Zhao  wrote:
> >   
> > > version attribute is used to check two mdev devices' compatibility.
> > > 
> > > The key point of this version attribute is that it's rw.
> > > User space has no need to understand internal of device version and no
> > > need to compare versions by itself.
> > > Compared to reading version strings from both two mdev devices being
> > > checked, user space only reads from one mdev device's version attribute.
> > > After getting its version string, user space writes this string into the
> > > other mdev device's version attribute. Vendor driver of mdev device
> > > whose version attribute being written will check device compatibility of
> > > the two mdev devices for user space and return success for compatibility
> > > or errno for incompatibility.
> > > So two readings of version attributes + checking in user space are now
> > > changed to one reading + one writing of version attributes + checking in
> > > vendor driver.
> > > Format and length of version strings are now private to vendor driver
> > > who can define them freely.
> > > 
> > >  __ user space
> > >   /\  \
> > >  / \write
> > > / read  \
> > >  __/__   ___\|/___
> > > | version | | version |-->check compatibility
> > > --- ---
> > > mdev device A   mdev device B
> > > 
> > > This version attribute is optional. If a mdev device does not provide
> > > with a version attribute, this mdev device is incompatible to all other
> > > mdev devices.
> > > 
> > > Live migration is able to take advantage of this version attribute.
> > > Before user space actually starts live migration, it can first check
> > > whether two mdev devices are compatible.
> > > 
> > > v2:
> > > 1. added detailed intent and usage
> > > 2. made definition of version string completely private to vendor driver
> > >(Alex Williamson)
> > > 3. abandoned changes to sample mdev drivers (Alex Williamson)
> > > 4. mandatory --> optional (Cornelia Huck)
> > > 5. added description for errno (Cornelia Huck)
> > > 
> > > Cc: Alex Williamson 
> > > Cc: Erik Skultety 
> > > Cc: "Dr. David Alan Gilbert" 
> > > Cc: Cornelia Huck 
> > > Cc: "Tian, Kevin" 
> > > Cc: Zhenyu Wang 
> > > Cc: "Wang, Zhi A" 
> > > Cc: Neo Jia 
> > > Cc: Kirti Wankhede 
> > > Cc: Daniel P. Berrangé 
> > > Cc: Christophe de Dinechin 
> > > 
> > > Signed-off-by: Yan Zhao 
> > > ---
> > >  Documentation/vfio-mediated-device.txt | 140 +
> > >  1 file changed, 140 insertions(+)
> > > 
> > > diff --git a/Documentation/vfio-mediated-device.txt 
> > > b/Documentation/vfio-mediated-device.txt
> > > index c3f69bcaf96e..013a764968eb 100644
> > > --- a/Documentation/vfio-mediated-device.txt
> > > +++ b/Documentation/vfio-mediated-device.txt
> > > @@ -202,6 +202,7 @@ Directories and files under the sysfs for Each 
> > > Physical Device
> > >| |   |--- available_instances
> > >| |   |--- device_api
> > >| |   |--- description
> > > +  | |   |--- version
> > >| |   |--- [devices]
> > >| |--- []
> > >| |   |--- create
> > > @@ -209,6 +210,7 @@ Directories and files under the sysfs for Each 
> > > Physical Device
> > >| |   |--- available_instances
> > >| |   |--- device_api
> > >| |   |--- description
> > > +  | |   |--- version
> > >| |   |--- [devices]
> > >| |--- []
> > >|  |--- create
> > > @@ -216,6 +218,7 @@ Directories and files under the sysfs for Each 
> > > Physical Device
> > >|  |--- available_instances
> > >|  |--- device_api
> > >|  |--- description
> > > +  |  |--- version
> > >|  |--- [devices]  
> > 
> > I thought there was a request to make this more specific to migration
> > by renaming it to something like migration_version.  Also, as an
> >  
> so this attribute may not only include a mdev device's parent device info and
> mdev type, but also include numeric software version of vendor specific
> migration code, right?

It's a vendor defined string, it should be considered opaque to the
user, the vendor can include whatever they feel is relevant.

> This actually makes sense.
> So, do I need to add a disclaimer in this doc like:
> vendor driver should be responsible by itself for a mdev device's migration
> compatibility. 

I thought that was the purpose of this attribute.

> During migration setup phase, general migration code in user space VFIO only
> checks this version of VFIO migration region, and will not check software 
> version
> of vendor specific migration code.

What is "software version of vendor specific migration code"?  If
you're asking whether anything will check for parent device

[Qemu-devel] [PATCH v3 5/5] iotests: Let 233 run concurrently

[Max Reitz]

common.nbd's nbd_server_set_tcp_port() tries to find a free port, and
then uses it for the whole test run.  However, this is racy because even
if the port was free at the beginning, there is no guarantee it will
continue to be available.  Therefore, 233 currently cannot reliably be
run concurrently with other NBD TCP tests.

This patch addresses the problem by dropping nbd_server_set_tcp_port(),
and instead finding a new port every time nbd_server_start_tcp_socket()
is invoked.  For this, we run qemu-nbd with --fork and on error evaluate
the output to see whether it contains "Address already in use".  If so,
we try the next port.

On success, we still want to continually redirect the output from
qemu-nbd to stderr.  To achieve both, we redirect qemu-nbd's stderr to a
FIFO that we then open in bash.  If the parent process exits with status
0 (which means that the server has started successfully), we launch a
background cat process that copies the FIFO to stderr.  On failure, we
read the whole content into a variable and then evaluate it.

While at it, use --fork in nbd_server_start_unix_socket(), too.  Doing
so allows us to drop nbd_server_wait_for_*_socket().

Note that the reason common.nbd did not use --fork before is that
qemu-nbd did not have --pid-file.

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 tests/qemu-iotests/233|  1 -
 tests/qemu-iotests/common.nbd | 94 ---
 2 files changed, 42 insertions(+), 53 deletions(-)

diff --git a/tests/qemu-iotests/233 b/tests/qemu-iotests/233
index b8b6c8cc4c..8682ea277c 100755
--- a/tests/qemu-iotests/233
+++ b/tests/qemu-iotests/233
@@ -50,7 +50,6 @@ _supported_proto file
 _supported_os Linux
 _require_command QEMU_NBD
 
-nbd_server_set_tcp_port
 tls_x509_init
 
 echo
diff --git a/tests/qemu-iotests/common.nbd b/tests/qemu-iotests/common.nbd
index 25fc9ffaa4..24b01b60aa 100644
--- a/tests/qemu-iotests/common.nbd
+++ b/tests/qemu-iotests/common.nbd
@@ -22,6 +22,11 @@
 nbd_unix_socket="${TEST_DIR}/qemu-nbd.sock"
 nbd_tcp_addr="127.0.0.1"
 nbd_pid_file="${TEST_DIR}/qemu-nbd.pid"
+nbd_stderr_fifo="${TEST_DIR}/qemu-nbd.fifo"
+
+# If bash version is >= 4.1, this will be overwritten by a dynamically
+# assigned file descriptor value.
+nbd_fifo_fd=10
 
 nbd_server_stop()
 {
@@ -33,77 +38,62 @@ nbd_server_stop()
 kill "$NBD_PID"
 fi
 fi
-rm -f "$nbd_unix_socket"
-}
-
-nbd_server_wait_for_unix_socket()
-{
-pid=$1
-
-for ((i = 0; i < 300; i++))
-do
-if [ -r "$nbd_unix_socket" ]; then
-return
-fi
-kill -s 0 $pid 2>/dev/null
-if test $? != 0
-then
-echo "qemu-nbd unexpectedly quit"
-exit 1
-fi
-sleep 0.1
-done
-echo "Failed in check of unix socket created by qemu-nbd"
-exit 1
+rm -f "$nbd_unix_socket" "$nbd_stderr_fifo"
 }
 
 nbd_server_start_unix_socket()
 {
 nbd_server_stop
-$QEMU_NBD -v -t -k "$nbd_unix_socket" "$@" &
-nbd_server_wait_for_unix_socket $!
+$QEMU_NBD -v -t -k "$nbd_unix_socket" --fork "$@"
 }
 
-nbd_server_set_tcp_port()
+nbd_server_start_tcp_socket()
 {
-(ss --help) >/dev/null 2>&1 || _notrun "ss utility not found, skipping 
test"
+nbd_server_stop
 
+mkfifo "$nbd_stderr_fifo"
 for ((port = 10809; port <= 10909; port++))
 do
-if ! ss -tln | grep -sqE ":$port\b"; then
+# Redirect stderr to FIFO, so we can later decide whether we
+# want to read it or to redirect it to our stderr, depending
+# on whether the command fails or not
+$QEMU_NBD -v -t -b $nbd_tcp_addr -p $port --fork "$@" \
+2> "$nbd_stderr_fifo" &
+
+# Taken from common.qemu
+if [[ "${BASH_VERSINFO[0]}" -ge "5" ||
+("${BASH_VERSINFO[0]}" -ge "4" && "${BASH_VERSINFO[1]}" -ge "1") ]]
+then
+exec {nbd_fifo_fd}<"$nbd_stderr_fifo"
+else
+let _nbd_fifo_fd++
+eval "exec ${_nbd_fifo_fd}<'$nbd_stderr_fifo'"
+fi
+wait $!
+
+if test $? == 0
+then
+# Success, redirect qemu-nbd's stderr to our stderr
 nbd_tcp_port=$port
+(cat <&$nbd_fifo_fd >&2) &
+eval "exec $nbd_fifo_fd>&-"
 return
 fi
-done
 
-echo "Cannot find free TCP port for nbd in range 10809-10909"
-exit 1
-}
-
-nbd_server_wait_for_tcp_socket()
-{
-pid=$1
+# Failure, read the output
+output=$(cat <&$nbd_fifo_fd)
+eval "exec $nbd_fifo_fd>&-"
 
-for ((i = 0; i < 300; i++))
-do
-if ss -tln | grep -sqE ":$nbd_tcp_port\b"; then
-return
-fi
-kill -s 0 $pid 2>/dev/null
-if test $? != 0
+if ! echo "$output" | grep -q "Address already in use"
 then
-echo "qemu-nbd unexpectedly quit"
+# Unknown error, print it
+echo "$output" >&2
+rm -f 

[Qemu-devel] [PATCH v3 4/5] iotests: Use qemu-nbd's --pid-file

[Max Reitz]

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.rc | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 93f87389b6..5502c3da2f 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -105,10 +105,8 @@ _qemu_io_wrapper()
 
 _qemu_nbd_wrapper()
 {
-(
-echo $BASHPID > "${QEMU_TEST_DIR}/qemu-nbd.pid"
-exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@"
-)
+"$QEMU_NBD_PROG" --pid-file="${QEMU_TEST_DIR}/qemu-nbd.pid" \
+ $QEMU_NBD_OPTIONS "$@"
 }
 
 _qemu_vxhs_wrapper()
-- 
2.21.0

[Qemu-devel] [PATCH v3 1/5] qemu-nbd: Add --pid-file option

[Max Reitz]

--fork is a bit boring if there is no way to get the child's PID.  This
option helps.

Signed-off-by: Max Reitz 
---
 qemu-nbd.c| 11 +++
 qemu-nbd.texi |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index dca9e72cee..edb5195208 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -59,6 +59,7 @@
 #define QEMU_NBD_OPT_IMAGE_OPTS262
 #define QEMU_NBD_OPT_FORK  263
 #define QEMU_NBD_OPT_TLSAUTHZ  264
+#define QEMU_NBD_OPT_PID_FILE  265
 
 #define MBR_SIZE 512
 
@@ -111,6 +112,7 @@ static void usage(const char *name)
 "specify tracing options\n"
 "  --forkfork off the server process and exit the parent\n"
 "once the server is running\n"
+"  --pid-file=PATH   store the server's process ID in the given file\n"
 #if HAVE_NBD_DEVICE
 "\n"
 "Kernel NBD client support:\n"
@@ -651,6 +653,7 @@ int main(int argc, char **argv)
 { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
 { "trace", required_argument, NULL, 'T' },
 { "fork", no_argument, NULL, QEMU_NBD_OPT_FORK },
+{ "pid-file", required_argument, NULL, QEMU_NBD_OPT_PID_FILE },
 { NULL, 0, NULL, 0 }
 };
 int ch;
@@ -677,6 +680,7 @@ int main(int argc, char **argv)
 bool list = false;
 int old_stderr = -1;
 unsigned socket_activation;
+const char *pid_file_name = NULL;
 
 /* The client thread uses SIGTERM to interrupt the server.  A signal
  * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -876,6 +880,9 @@ int main(int argc, char **argv)
 case 'L':
 list = true;
 break;
+case QEMU_NBD_OPT_PID_FILE:
+pid_file_name = optarg;
+break;
 }
 }
 
@@ -1196,6 +1203,10 @@ int main(int argc, char **argv)
 
 nbd_update_server_watch();
 
+if (pid_file_name) {
+qemu_write_pidfile(pid_file_name, _fatal);
+}
+
 /* now when the initialization is (almost) complete, chdir("/")
  * to free any busy filesystems */
 if (chdir("/") < 0) {
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index de342c76b8..7f55657722 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -117,6 +117,8 @@ option; or provide the credentials needed for connecting as 
a client
 in list mode.
 @item --fork
 Fork off the server process and exit the parent once the server is running.
+@item --pid-file=PATH
+Store the server's process ID in the given file.
 @item --tls-authz=ID
 Specify the ID of a qauthz object previously created with the
 --object option. This will be used to authorize connecting users
-- 
2.21.0

[Qemu-devel] [PATCH v3 0/5] iotests: Let 233 run concurrently

[Max Reitz]

Currently, 233 cannot reliably run concurrently to other NBD TCP tests.
When it starts, it looks for a free port and then attempts to use that
for the whole duration of the test run.  This is a TOCTTOU race
condition: It does not reserve that port, so another NBD TCP test that
runs in parallel can grab it.

To fix this, we must not use the same port all the time, but always
choose a new one when qemu-nbd is started.  We cannot check whether it
is free, but must let qemu-nbd do so and take it atomically.  We can
achieve this by using the existing --fork option.

There are two problems with --fork, however.  First, it does not give us
a chance to reliably get the server’s PID, which we need.  We can change
that by letting qemu-nbd (optionally) write a PID file, though.  (Which
makes sense if we have a daemon mode.)

Second, it currently discards all output after the server has been
started.  That looks like an accident to me, because we clearly try to
restore the old stderr channel after having redirected all startup
messages to the parent process.  If it is a bug, we can fix it.


v3:
- Patch 1: Dropped “pid_file” variable, so it actually compiles...


git backport-diff of v3 against v2:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/5:[0001] [FC] 'qemu-nbd: Add --pid-file option'
002/5:[] [--] 'iotests.py: Add qemu_nbd_early_pipe()'
003/5:[] [--] 'qemu-nbd: Do not close stderr'
004/5:[] [--] 'iotests: Use qemu-nbd's --pid-file'
005/5:[] [--] 'iotests: Let 233 run concurrently'


v2:
- Patch 1:
  - Use qemu_write_pidfile() [Dan]
  - %s/pid_path/pid_filename/ [Eric]
- Patch 4: Drop the now superfluous subshell [Eric]
  (Didn’t touch _qemu_img_wrapper, because, well, it doesn’t belong in
  this series?)
- Patch 5:
  - s/racey/racy/ [Eric]
  - Unite the “rm -f”s [Eric]
  (Did not address the “FIFO filling up” problem, because 64 kB of FIFO
  space ought to be enough.  Also, cat-ing around that felt weird.)


git backport-diff of v2 against v1:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/5:[0025] [FC] 'qemu-nbd: Add --pid-file option'
002/5:[] [--] 'iotests.py: Add qemu_nbd_early_pipe()'
003/5:[] [--] 'qemu-nbd: Do not close stderr'
004/5:[0006] [FC] 'iotests: Use qemu-nbd's --pid-file'
005/5:[0003] [FC] 'iotests: Let 233 run concurrently'


Max Reitz (5):
  qemu-nbd: Add --pid-file option
  iotests.py: Add qemu_nbd_early_pipe()
  qemu-nbd: Do not close stderr
  iotests: Use qemu-nbd's --pid-file
  iotests: Let 233 run concurrently

 qemu-nbd.c| 14 +-
 qemu-nbd.texi |  2 +
 tests/qemu-iotests/147|  4 +-
 tests/qemu-iotests/233|  1 -
 tests/qemu-iotests/common.nbd | 94 ---
 tests/qemu-iotests/common.rc  |  6 +--
 tests/qemu-iotests/iotests.py |  9 ++--
 7 files changed, 67 insertions(+), 63 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v3 3/5] qemu-nbd: Do not close stderr

[Max Reitz]

We kept old_stderr specifically so we could keep emitting error message
on stderr.  However, qemu_daemon() closes stderr.  Therefore, we need to
dup() stderr to old_stderr before invoking qemu_daemon().

Signed-off-by: Max Reitz 
Reviewed-by: Eric Blake 
---
 qemu-nbd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index edb5195208..e4446d479c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1014,10 +1014,11 @@ int main(int argc, char **argv)
 exit(EXIT_FAILURE);
 } else if (pid == 0) {
 close(stderr_fd[0]);
+
+old_stderr = dup(STDERR_FILENO);
 ret = qemu_daemon(1, 0);
 
 /* Temporarily redirect stderr to the parent's pipe...  */
-old_stderr = dup(STDERR_FILENO);
 dup2(stderr_fd[1], STDERR_FILENO);
 if (ret < 0) {
 error_report("Failed to daemonize: %s", strerror(errno));
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2 1/5] qemu-nbd: Add --pid-file option

[Max Reitz]

On 08.05.19 15:22, Max Reitz wrote:
> --fork is a bit boring if there is no way to get the child's PID.  This
> option helps.
> 
> Signed-off-by: Max Reitz 
> ---
>  qemu-nbd.c| 12 
>  qemu-nbd.texi |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index dca9e72cee..4866042160 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c

[...]

> @@ -677,6 +680,8 @@ int main(int argc, char **argv)
>  bool list = false;
>  int old_stderr = -1;
>  unsigned socket_activation;
> +const char *pid_file_name = NULL;
> +FILE *pid_file;

Great to see how well I test my patches.  This shouldn’t be here
anymore, of course.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] target/riscv: Only flush TLB if SATP.ASID changes

[Palmer Dabbelt]

On Wed, 08 May 2019 10:38:35 PDT (-0700), jonat...@fintelia.io wrote:

There is an analogous change for ARM here:
https://patchwork.kernel.org/patch/10649857

Signed-off-by: Jonathan Behrens 
---
 target/riscv/csr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6083c782a1..1ec1222da1 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -732,7 +732,9 @@ static int write_satp(CPURISCVState *env, int csrno, 
target_ulong val)
 if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
 return -1;
 } else {
-tlb_flush(CPU(riscv_env_get_cpu(env)));
+if((val ^ env->satp) & SATP_ASID) {
+tlb_flush(CPU(riscv_env_get_cpu(env)));
+}
 env->satp = val;
 }
 }


Thanks!  I've taken this into my for-master branch, pending some testing I'll
send it up.

Re: [Qemu-devel] [Question] Memory hotplug clarification for Qemu ARM/virt

[Laszlo Ersek]

On 05/08/19 14:50, Robin Murphy wrote:
> Hi Shameer,
> 
> On 08/05/2019 11:15, Shameerali Kolothum Thodi wrote:
>> Hi,
>>
>> This series here[0] attempts to add support for PCDIMM in QEMU for
>> ARM/Virt platform and has stumbled upon an issue as it is not clear(at
>> least
>> from Qemu/EDK2 point of view) how in physical world the hotpluggable
>> memory is handled by kernel.
>>
>> The proposed implementation in Qemu, builds the SRAT and DSDT parts
>> and uses GED device to trigger the hotplug. This works fine.
>>
>> But when we added the DT node corresponding to the PCDIMM(cold plug
>> scenario), we noticed that Guest kernel see this memory during early boot
>> even if we are booting with ACPI. Because of this, hotpluggable memory
>> may end up in zone normal and make it non-hot-un-pluggable even if Guest
>> boots with ACPI.
>>
>> Further discussions[1] revealed that, EDK2 UEFI has no means to
>> interpret the
>> ACPI content from Qemu(this is designed to do so) and uses DT info to
>> build the GetMemoryMap(). To solve this, introduced "hotpluggable"
>> property
>> to DT memory node(patches #7 & #8 from [0]) so that UEFI can
>> differentiate
>> the nodes and exclude the hotpluggable ones from GetMemoryMap().
>>
>> But then Laszlo rightly pointed out that in order to accommodate the
>> changes
>> into UEFI we need to know how exactly Linux expects/handles all the
>> hotpluggable memory scenarios. Please find the discussion here[2].
>>
>> For ease, I am just copying the relevant comment from Laszlo below,
>>
>> /**
>> "Given patches #7 and #8, as I understand them, the firmware cannot
>> distinguish
>>   hotpluggable & present, from hotpluggable & absent. The firmware can
>> only
>>   skip both hotpluggable cases. That's fine in that the firmware will
>> hog neither
>>   type -- but is that OK for the OS as well, for both ACPI boot and DT
>> boot?
>>
>> Consider in particular the "hotpluggable & present, ACPI boot" case.
>> Assuming
>> we modify the firmware to skip "hotpluggable" altogether, the UEFI memmap
>> will not include the range despite it being present at boot.
>> Presumably, ACPI
>> will refer to the range somehow, however. Will that not confuse the OS?
>>
>> When Igor raised this earlier, I suggested that
>> hotpluggable-and-present should
>> be added by the firmware, but also allocated immediately, as
>> EfiBootServicesData
>> type memory. This will prevent other drivers in the firmware from
>> allocating AcpiNVS
>> or Reserved chunks from the same memory range, the UEFI memmap will
>> contain
>> the range as EfiBootServicesData, and then the OS can release that
>> allocation in
>> one go early during boot.
>>
>> But this really has to be clarified from the Linux kernel's
>> expectations. Please
>> formalize all of the following cases:
>>
>> OS boot (DT/ACPI)  hotpluggable & ...  GetMemoryMap() should report
>> as  DT/ACPI should report as
>> -  -- 
>> ---  
>> DT present ?    ?
>> DT absent  ?    ?
>> ACPI   present ?    ?
>> ACPI   absent  ?    ?
>>
>> Again, this table is dictated by Linux."
>>
>> **/
>>
>> Could you please take a look at this and let us know what is expected
>> here from
>> a Linux kernel view point.
> 
> For arm64, so far we've not even been considering DT-based hotplug - as
> far as I'm aware there would still be a big open question there around
> notification mechanisms and how to describe them. The DT stuff so far
> has come from the PowerPC folks, so it's probably worth seeing what
> their ideas are.
> 
> ACPI-wise I've always assumed/hoped that hotplug-related things should
> be sufficiently well-specified in UEFI that "do whatever x86/IA-64 do"
> would be enough for us.

As far as I can see in UEFI v2.8 -- and I had checked the spec before
dumping the table with the many question marks on Shameer --, all the
hot-plug language in the spec refers to USB and PCI hot-plug in the
preboot environment. There is not a single word about hot-plug at OS
runtime (regarding any device or component type), nor about memory
hot-plug (at any time).

Looking to x86 appears valid -- so what does the Linux kernel expect on
that architecture, in the "ACPI" rows of the table?

Shameer: if you (Huawei) are represented on the USWG / ASWG, I suggest
re-raising the question on those lists too; at least the "ACPI" rows of
the table.

Thanks!
Laszlo

> 
> Robin.
> 
>> (Hi Laszlo/Igor/Eric, please feel free to add/change if I have missed
>> any valid
>> points above).
>>
>> Thanks,
>> Shameer
>> [0] https://patchwork.kernel.org/cover/10890919/
>> [1] https://patchwork.kernel.org/patch/10863299/
>> [2] https://patchwork.kernel.org/patch/10890937/
>>
>>

Re: [Qemu-devel] [PATCH 0/3] Export machine type deprecation info through QMP

[Eduardo Habkost]

On Wed, May 08, 2019 at 11:16:50AM +0200, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > On Tue, May 07, 2019 at 07:07:04AM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost  writes:
> >> 
> >> > This series adds machine type deprecation information to the
> >> > output of the `query-machines` QMP command.  With this, libvirt
> >> > and management software will be able to show this information to
> >> > users and/or suggest changes to VM configuration to avoid
> >> > deprecated machine types.
> >> 
> >> This overlaps with something I want to try, namely using Kevin's
> >> proposed QAPI feature flags for deprecation markings.  Let's compare the
> >> two.
> >> 
> >> To mark something as deprecated with your patches, you add a
> >> @support-status member somewhere, where "somewhere" is related to
> >> "something" by "provides information on".
> >> 
> >> Example: MachineInfo (returned by query-machines) provides information
> >> on possible values of -machine parameter type.  If -machine was
> >> QAPIfied, it would provide information on possible values of a QAPI
> >> object type's member.  The type might be anonymous.  The member should
> >> be an enum (we currently use 'str' in MachineInfo).
> >
> > QAPIfying -machine, -cpu, and -device would be wonderful.
> >
> >> 
> >> Example: say we want to deprecate block driver "vfat",
> >> i.e. BlockdevDriver member @vfat.  Type BlockdevDriver is used in
> >> multiple places; let's ignore all but BlockdevOptions.  We need to add
> >> @support-status to something that provides information on
> >> BlockdevDriver, or maybe on BlockdevOptions.  There is no ad hoc query
> >> providing information on either of the two, because QAPI/QMP
> >> introspection has been sufficient.  What now?
> >> 
> >> Can we add deprecation information to (general) QAPI/QMP introspection
> >
> > Yes, we can.  I think it's a good idea.  But:
> >
> >> instead of ad hoc queries?
> >
> > I'm not sure about the "instead of" part.  I don't want perfect
> > to be the enemy of done, and I don't want QAPIfication of
> > -machine to be a requirement to start reporting machine type
> > deprecation information.
> 
> Valid point.  Still, I believe we should at least try to predict how the
> pieces we create now would fit with the pieces we plan to create later
> on.

Sure.

> 
> Note that full QAPIfication of -machine isn't necessary to make QAPI
> feature "deprecated" work for machine types.  Turning MachineInfo member
> @name into an enum, so we can tack "deprecated" onto its values, would
> suffice.
> 
> Such a QAPIfication of machine types is still hard: QOM types are
> defined at compile time just like the QAPI schema, but their definition
> is distributed, and collected into one place only at run time.  I
> discussed this on slide 39 of my "QEMU interface introspection: From
> hacks to solutions" talk (KVM Form 2015).  Just for device_add, but it's
> just a special case of QOM.  Choices listed there:
> 
> * Collect drivers at compile time? Hard...
> * Make QAPI schema dynamic? Hard...
> * Forgo driver-specific arguments in schema?
>   Defeats introspection...
> 
> I'd like to add to the last item:
> 
>   Provide QOM introspection on par with QAPI schema introspection
> 
> The QOM introspection we have (qom-list-types etc. is not on par.

Agreed, but do we really want to do it?  We have been avoiding
exposing QOM internals to the outside on purpose.  I believe
there are at least two reasons for that:

1) Not every QOM type/property is supposed to be visible to the
   outside (and nobody really knows what's the full set of
   supported external QOM interfaces);
2) QAPI is our preferred interface interface specification/introspection
   mechanism.

> 
> Back to exposing machine type deprecation.
> 
> I'm doubtful your proposed solution can be applied widely.  It relies on
> adding @support-status to something that provides information on
> whatever is deprecated.  The initial use is with a something that is an
> ad hoc query, namely query-machines.  To use it, the management
> application needs to understand what query-machines' @support-status
> applies to.  Certainly feasible.  But I fear every use will be a special
> case.  Furthermore, a suitable ad hoc query need not exist.  What then?
> Create suitable ad hoc queries just for communicating deprecation?
> 
> Instead, I'd like us to think about a more genral solution.  Or perhaps
> two: one for properly QAPIfied stuff, and one for QOM.

Should we really spend our time designing a second solution, if
we could build this on top of QAPI abstractions?  Making the QAPI
schema dynamic might be hard, but reinventing QAPI and
maintaining the two systems in parallel seems harder.


> 
> >> Kevin's proposed QAPI feature flags[*] extend the QAPI language so that
> >> struct types can optionally have a list of feature flags, which are
> >> strings.  Struct types suffice for his immediate needs.  I'd like to use
> >> feature flags to mark 

Re: [Qemu-devel] [PATCH for-4.1 0/2] hw/alpha: Add the CY82C693UB southbridge in Kconfig

[Philippe Mathieu-Daudé]

Paolo, Thomas,

On 4/29/19 1:29 PM, Philippe Mathieu-Daudé wrote:
> CC'ing Thomas who is a Kconfig expert.
> 
> On 3/17/19 12:44 AM, Philippe Mathieu-Daudé wrote:
>> Explicit the CY82C693UB southbridge used by the 264DP.
>>
>> Philippe Mathieu-Daudé (2):
>>   hw/isa/southbridge: Add the Cypress 82C693UB chipset
>>   hw/alpha/Kconfig: The 264DP machine use a CY82C693UB southbridge

This series does not fix anything, but makes the kconfig graph cleaner.

Regards,

Phil.

Re: [Qemu-devel] [PATCH] target/riscv: Only flush TLB if SATP.ASID changes

[Alistair Francis]

On Wed, May 8, 2019 at 10:39 AM Jonathan Behrens  wrote:
>
> There is an analogous change for ARM here:
> https://patchwork.kernel.org/patch/10649857
>
> Signed-off-by: Jonathan Behrens 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/csr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 6083c782a1..1ec1222da1 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -732,7 +732,9 @@ static int write_satp(CPURISCVState *env, int csrno, 
> target_ulong val)
>  if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
>  return -1;
>  } else {
> -tlb_flush(CPU(riscv_env_get_cpu(env)));
> +if((val ^ env->satp) & SATP_ASID) {
> +tlb_flush(CPU(riscv_env_get_cpu(env)));
> +}
>  env->satp = val;
>  }
>  }
> --
> 2.20.1
>

[Qemu-devel] [PATCH v2] target/ppc: Fix xxspltib

[Anton Blanchard]

xxspltib raises a VMX or a VSX exception depending on the register
set it is operating on. We had a check, but it was backwards.

Fixes: f113283525a4 ("target-ppc: add xxspltib instruction")
Signed-off-by: Anton Blanchard 
---
 target/ppc/translate/vsx-impl.inc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/translate/vsx-impl.inc.c 
b/target/ppc/translate/vsx-impl.inc.c
index 4d8ca7cf32..4812a374aa 100644
--- a/target/ppc/translate/vsx-impl.inc.c
+++ b/target/ppc/translate/vsx-impl.inc.c
@@ -1355,13 +1355,13 @@ static void gen_xxspltib(DisasContext *ctx)
 int rt = xT(ctx->opcode);
 
 if (rt < 32) {
-if (unlikely(!ctx->altivec_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VPU);
+if (unlikely(!ctx->vsx_enabled)) {
+gen_exception(ctx, POWERPC_EXCP_VSXU);
 return;
 }
 } else {
-if (unlikely(!ctx->vsx_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VSXU);
+if (unlikely(!ctx->altivec_enabled)) {
+gen_exception(ctx, POWERPC_EXCP_VPU);
 return;
 }
 }
-- 
2.20.1

Re: [Qemu-devel] [Question] Memory hotplug clarification for Qemu ARM/virt

[Laszlo Ersek]

On 05/08/19 12:15, Shameerali Kolothum Thodi wrote:
> Hi,
> 
> This series here[0] attempts to add support for PCDIMM in QEMU for
> ARM/Virt platform and has stumbled upon an issue as it is not clear(at least
> from Qemu/EDK2 point of view) how in physical world the hotpluggable
> memory is handled by kernel.
> 
> The proposed implementation in Qemu, builds the SRAT and DSDT parts
> and uses GED device to trigger the hotplug. This works fine.
> 
> But when we added the DT node corresponding to the PCDIMM(cold plug
> scenario), we noticed that Guest kernel see this memory during early boot
> even if we are booting with ACPI. Because of this, hotpluggable memory
> may end up in zone normal and make it non-hot-un-pluggable even if Guest
> boots with ACPI.
> 
> Further discussions[1] revealed that, EDK2 UEFI has no means to interpret the
> ACPI content from Qemu(this is designed to do so) and uses DT info to
> build the GetMemoryMap(). To solve this, introduced "hotpluggable" property
> to DT memory node(patches #7 & #8 from [0]) so that UEFI can differentiate
> the nodes and exclude the hotpluggable ones from GetMemoryMap().
> 
> But then Laszlo rightly pointed out that in order to accommodate the changes
> into UEFI we need to know how exactly Linux expects/handles all the 
> hotpluggable memory scenarios. Please find the discussion here[2].
> 
> For ease, I am just copying the relevant comment from Laszlo below,
> 
> /**
> "Given patches #7 and #8, as I understand them, the firmware cannot 
> distinguish
>  hotpluggable & present, from hotpluggable & absent. The firmware can only
>  skip both hotpluggable cases. That's fine in that the firmware will hog 
> neither
>  type -- but is that OK for the OS as well, for both ACPI boot and DT boot?
> 
> Consider in particular the "hotpluggable & present, ACPI boot" case. Assuming
> we modify the firmware to skip "hotpluggable" altogether, the UEFI memmap
> will not include the range despite it being present at boot. Presumably, ACPI
> will refer to the range somehow, however. Will that not confuse the OS?
> 
> When Igor raised this earlier, I suggested that hotpluggable-and-present 
> should
> be added by the firmware, but also allocated immediately, as 
> EfiBootServicesData
> type memory. This will prevent other drivers in the firmware from allocating 
> AcpiNVS
> or Reserved chunks from the same memory range, the UEFI memmap will contain
> the range as EfiBootServicesData, and then the OS can release that allocation 
> in
> one go early during boot.
> 
> But this really has to be clarified from the Linux kernel's expectations. 
> Please
> formalize all of the following cases:
> 
> OS boot (DT/ACPI)  hotpluggable & ...  GetMemoryMap() should report as  
> DT/ACPI should report as
> -  --  ---  
> 
> DT present ??
> DT absent  ??
> ACPI   present ??
> ACPI   absent  ??
> 
> Again, this table is dictated by Linux."
> 
> **/
> 
> Could you please take a look at this and let us know what is expected here 
> from
> a Linux kernel view point.
> 
> (Hi Laszlo/Igor/Eric, please feel free to add/change if I have missed any 
> valid
> points above).

I'm happy with your summary, thank you!
Laszlo

> 
> Thanks,
> Shameer
> [0] https://patchwork.kernel.org/cover/10890919/
> [1] https://patchwork.kernel.org/patch/10863299/
> [2] https://patchwork.kernel.org/patch/10890937/
> 
> 

Re: [Qemu-devel] [PATCH v3] i386: Add some MSR based features on Cascadelake-Server CPU model

[Eduardo Habkost]

On Wed, May 08, 2019 at 09:31:53AM +0800, Tao Xu wrote:
> As noted in "c7a88b52f6 i386: Add new model of Cascadelake-Server"
> Because MSR based feature has been supported by QEMU, we add
> CPUID_7_0_EDX_ARCH_CAPABILITIES on Cascadelake-Server CPU model,
> and add IA32_ARCH_CAPABILITIES MSR based features (RDCL_NO,
> IBRS_ALL and SKIP_L1DFL_VMENTRY).
> 
> And "014018e19b i386: Make arch_capabilities migratable" has been
> in QEMU upstream, the CPUID_7_0_EDX_ARCH_CAPABILITIES can be
> safely added into CPU Model.
> 
> Signed-off-by: Tao Xu 

CPUID_7_0_EDX_ARCH_CAPABILITIES requires Linux >= v4.16.  This
means the patch will make "-cpu Cascadelake-Server" stop working
on Linux < v4.16 hosts, doesn't it?

For additional details, see:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg603291.html
"[PATCH 2/2] i386: Add some MSR based features on
Cascadelake-Server CPU model"

and:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg606373.html
"Cascadelake-Server missing MSR based features ?"

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 25/31] target/cris: Use tcg_gen_abs_tl

[David Hildenbrand]

On 04.05.19 07:52, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/cris/translate.c | 9 +
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index b005a5c20e..31b40a57f9 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -1686,18 +1686,11 @@ static int dec_cmp_r(CPUCRISState *env, DisasContext 
> *dc)
>  
>  static int dec_abs_r(CPUCRISState *env, DisasContext *dc)
>  {
> -TCGv t0;
> -
>  LOG_DIS("abs $r%u, $r%u\n",
>  dc->op1, dc->op2);
>  cris_cc_mask(dc, CC_MASK_NZ);
>  
> -t0 = tcg_temp_new();
> -tcg_gen_sari_tl(t0, cpu_R[dc->op1], 31);
> -tcg_gen_xor_tl(cpu_R[dc->op2], cpu_R[dc->op1], t0);
> -tcg_gen_sub_tl(cpu_R[dc->op2], cpu_R[dc->op2], t0);
> -tcg_temp_free(t0);
> -
> +tcg_gen_abs_tl(cpu_R[dc->op2], cpu_R[dc->op1]);
>  cris_alu(dc, CC_OP_MOVE,
>  cpu_R[dc->op2], cpu_R[dc->op2], cpu_R[dc->op2], 4);
>  return 2;
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 0/7] Delete 16 *_cpu_class_by_name() functions

[Eduardo Habkost]

On Wed, May 08, 2019 at 10:34:44AM +0200, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > On Mon, May 06, 2019 at 01:53:28PM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost  writes:
> >> 
> >> > This series adds a new CPUClass::class_name_format field, which
> >> > allows us to delete 16 of the 21 *_cpu_class_by_name() functions
> >> > that exist today.
> >> 
> >> Which five remain, and why?
> >
> > alpha_cpu_class_by_name:
> > * Translates aliases based on alpha_cpu_aliases;
> > * Falls back to "ev67" unconditionally
> >   (there's a "TODO: remove match everything nonsense" comment).
> >
> > cris_cpu_class_by_name:
> > * Translates "any" alias to "crisv32" if CONFIG_USER_ONLY.
> >
> > ppc_cpu_class_by_name:
> > * Supports lookup by PVR if CPU model is a 8 digit hex number;
> > * Converts CPU model to lowercase.
> >
> > superh_cpu_class_by_name:
> > * Translates "any" alias to TYPE_SH7750R_CPU.
> >
> > sparc_cpu_class_by_name:
> > * Replaces whitespaces with '-' on CPU model name.
> 
> I'm of course asking because I wonder whether we can dumb down this CPU
> naming business to something simpler and more regular.

We can, but that's not on my list of priorities.  Any volunteers?

> 
[...]
> * Aliases
> 
>   We have several targets roll their own CPU name aliases code.
>   Assuming aliases are here to stay (i.e. we're not deprecating all of
>   them): what about letting each CPU type specify a set of aliases, so
>   we can recognize them in generic code?

Yes.  I considered adding alias support to generic code, but
decided to do this one step at a time.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 11/13] tests/vm: netbsd autoinstall, using serial console

[Kamil Rytarowski]

On 08.05.2019 10:56, Gerd Hoffmann wrote:
> Instead of fetching the prebuilt image from patchew download the install
> iso and prepare the image locally.  Install to disk, using the serial
> console.  Create qemu user, configure ssh login.  Install packages
> needed for qemu builds.
> 

I recommend to add one extra step into generated image:

echo security.pax.mprotect.enabled=0 >> /etc/sysctl.conf

Alternatively (and preferably) enhance qemu to handle RWX allocation for
JIT on NetBSD.

Example in libffi.

https://github.com/libffi/libffi/commit/2bfcd29955c02b67fa10a68cc4200f6838181e0f

> Signed-off-by: Gerd Hoffmann 
> ---
>  tests/vm/netbsd | 178 +---
>  1 file changed, 169 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/vm/netbsd b/tests/vm/netbsd
> index 4c6624ea5ed5..eaf0ae21db42 100755
> --- a/tests/vm/netbsd
> +++ b/tests/vm/netbsd
> @@ -13,32 +13,192 @@
>  
>  import os
>  import sys
> +import time
>  import subprocess
>  import basevm
>  
>  class NetBSDVM(basevm.BaseVM):
>  name = "netbsd"
>  arch = "x86_64"
> +
> +link = 
> "https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.0/images/NetBSD-8.0-amd64.iso;
> +size = "20G"
> +pkgs = [
> +# tools
> +"git",
> +"pkgconf",
> +"bzip2", "xz",
> +
> +# gnu tools
> +"bash",
> +"gmake",
> +"gsed",
> +"flex", "bison",
> +
> +# libs: crypto
> +"gnutls",
> +
> +# libs: images
> +"jpeg",
> +"png",
> +
> + # libs: ui
> +"SDL2",
> +"gtk3+",
> +"libxkbcommon",
> +]
> +
>  BUILD_SCRIPT = """
>  set -e;
> -rm -rf /var/tmp/qemu-test.*
> -cd $(mktemp -d /var/tmp/qemu-test.XX);
> +rm -rf /home/qemu/qemu-test.*
> +cd $(mktemp -d /home/qemu/qemu-test.XX);
> +mkdir src build; cd src;
>  tar -xf /dev/rld1a;
> -./configure --python=python2.7 {configure_opts};
> +cd ../build
> +../src/configure --python=python2.7 --disable-opengl 
> {configure_opts};
>  gmake --output-sync -j{jobs} {target} {verbose};
>  """
>  
>  def build_image(self, img):
> -cimg = 
> self._download_with_cache("http://download.patchew.org/netbsd-7.1-amd64.img.xz;,
> - 
> sha256sum='b633d565b0eac3d02015cd0c81440bd8a7a8df8512615ac1ee05d318be015732')
> -img_tmp_xz = img + ".tmp.xz"
> +cimg = self._download_with_cache(self.link)
>  img_tmp = img + ".tmp"
> -sys.stderr.write("Extracting the image...\n")
> -subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
> -subprocess.check_call(["xz", "-dvf", img_tmp_xz])
> +iso = img + ".install.iso"
> +
> +self.print_step("Preparing iso and disk image")
> +subprocess.check_call(["cp", "-f", cimg, iso])
> +subprocess.check_call(["qemu-img", "create", "-f", "qcow2",
> +   img_tmp, self.size])
> +
> +self.print_step("Booting installer")
> +self.boot(img_tmp, extra_args = [
> +"-device", "VGA",
> +"-machine", "graphics=off",
> +"-cdrom", iso
> +])
> +self.console_init()
> +self.console_wait("Primary Bootstrap")
> +
> +# serial console boot menu output doesn't work for some
> +# reason, so we have to fly blind ...
> +for char in list("5consdev com0\n"):
> +time.sleep(0.2)
> +self.console_send(char)
> +self.console_wait("")
> +self.console_wait_send("> ", "boot\n")
> +
> +self.console_wait_send("Terminal type","xterm\n")
> +self.console_wait_send("a: Installation messages", "a\n")
> +self.console_wait_send("b: US-English","b\n")
> +self.console_wait_send("a: Install NetBSD","a\n")
> +self.console_wait("Shall we continue?")
> +self.console_wait_send("b: Yes",   "b\n")
> +
> +self.console_wait_send("a: ld0",   "a\n")
> +self.console_wait_send("a: This is the correct",   "a\n")
> +self.console_wait_send("b: Use the entire disk",   "b\n")
> +self.console_wait("NetBSD bootcode")
> +self.console_wait_send("a: Yes",   "a\n")
> +self.console_wait_send("b: Use existing part", "b\n")
> +self.console_wait_send("x: Partition sizes ok","x\n")
> +self.console_wait_send("for your NetBSD disk", "\n")
> +self.console_wait("Shall we continue?")
> +self.console_wait_send("b: Yes",   "b\n")
> +
> +self.console_wait_send("b: Use serial port com0",  "b\n")
> +self.console_wait_send("f: Set serial baud rate",  "f\n")
> +self.console_wait_send("a: 9600",  "a\n")
> +self.console_wait_send("x: Exit",  

Re: [Qemu-devel] Update *BSD images with gnu-sed and bash

[Kamil Rytarowski]

On 08.05.2019 10:07, Thomas Huth wrote:
> On 08/05/2019 09.06, Kamil Rytarowski wrote:
>> On 06.05.2019 12:12, Thomas Huth wrote:
> [...]
>>>  Kamil,
>>>
>>> could you maybe help with the NetBSD image and the tests/vm/netbsd script?
>>>
>>
>> Please be more specific what am I expected to do.
> 
> We have some VMs (including NetBSD) available that are used during
> Peter's regression tests when somebody sends him a PULL requests. You
> can run them also locally with:
> 
>  make BUILD_TARGET=check vm-build-netbsd
> 
> From time to time, we've got to update these images, either to a newer
> version or to add some missing packages (like bash and gnu-sed in this
> case).

The process has been documented on wiki.

https://wiki.qemu.org/Hosts/BSD#NetBSD

> However, many people (including me) don't have a clue about the various
> *BSD flavours, so also no clue about how to update these images easily.
> That's why I was hoping you could help here.
> 
> But looks like Gerd is already working on a way to generate these images
> in a more automated way, so let's hope that he'll find some spare time
> to finish that work soon.
> 

I recommend to upgrade to 8.0.

One extra step is to disable PaX MPROTECT (tcg violates W^X):

This should be addressed in qemu with an extension flag to
mmap(2)/mremap(2). I still have this on my TODO list.

sysctl -w security.pax.mprotect.enabled=1

NetBSD 9.0 will be released sooner than later and soon after that
NetBSD-7.x will be EOL.

>  Thomas
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v7 2/6] virtio-pmem: Add virtio pmem driver

[Jakub Staroń via Qemu-devel]

On 5/8/19 4:12 AM, Pankaj Gupta wrote:
> 
>>
>> On 4/25/19 10:00 PM, Pankaj Gupta wrote:
>>
>>> +void host_ack(struct virtqueue *vq)
>>> +{
>>> +   unsigned int len;
>>> +   unsigned long flags;
>>> +   struct virtio_pmem_request *req, *req_buf;
>>> +   struct virtio_pmem *vpmem = vq->vdev->priv;
>>> +
>>> +   spin_lock_irqsave(>pmem_lock, flags);
>>> +   while ((req = virtqueue_get_buf(vq, )) != NULL) {
>>> +   req->done = true;
>>> +   wake_up(>host_acked);
>>> +
>>> +   if (!list_empty(>req_list)) {
>>> +   req_buf = list_first_entry(>req_list,
>>> +   struct virtio_pmem_request, list);
>>> +   list_del(>req_list);
>>
>> Shouldn't it be rather `list_del(vpmem->req_list.next)`? We are trying to
>> unlink
>> first element of the list and `vpmem->req_list` is just the list head.
> 
> This looks correct. We are not deleting head but first entry in 'req_list'
> which is device corresponding list of pending requests.
> 
> Please see below:
> 
> /**
>  * Retrieve the first list entry for the given list pointer.
>  *
>  * Example:
>  * struct foo *first;
>  * first = list_first_entry(>list_of_foos, struct foo, list_of_foos);
>  *
>  * @param ptr The list head
>  * @param type Data type of the list element to retrieve
>  * @param member Member name of the struct list_head field in the list 
> element.
>  * @return A pointer to the first list element.
>  */
> #define list_first_entry(ptr, type, member) \
> list_entry((ptr)->next, type, member)

Please look at this StackOverflow question:
https://stackoverflow.com/questions/19675419/deleting-first-element-of-a-list-h-list

Author asks about deleting first element of the queue. In our case
(and also in the question's author case), `vpmem->req_list` is not element
of any request struct and not an element of the list. It's just a list head 
storing 
`next` and `prev` pointers which are then pointing to respectively first and
last element of the list. We want to unlink the first element of the list,
so we need to pass pointer to the first element of the list to
the `list_del` function - that is, the `vpmem->req_list.next`.

Thank you,
Jakub Staron

[Qemu-devel] [PATCH v2 5/7] block: Fix order in bdrv_replace_child()

[Max Reitz]

We have to start by applying the permission restrictions to new_bs
before we can loosen them on old_bs.  See the comment for the
explanation.

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
---
 block.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index be11504940..da7da8cc6c 100644
--- a/block.c
+++ b/block.c
@@ -2205,6 +2205,19 @@ static void bdrv_replace_child(BdrvChild *child, 
BlockDriverState *new_bs)
 
 bdrv_replace_child_noperm(child, new_bs);
 
+/*
+ * Start with the new node's permissions.  If @new_bs is a (direct
+ * or indirect) child of @old_bs, we must complete the permission
+ * update on @new_bs before we loosen the restrictions on @old_bs.
+ * Otherwise, bdrv_check_perm() on @old_bs would re-initiate
+ * updating the permissions of @new_bs, and thus not purely loosen
+ * restrictions.
+ */
+if (new_bs) {
+bdrv_get_cumulative_perm(new_bs, , _perm);
+bdrv_set_perm(new_bs, perm, shared_perm);
+}
+
 if (old_bs) {
 /* Update permissions for old node. This is guaranteed to succeed
  * because we're just taking a parent away, so we're loosening
@@ -2213,11 +2226,6 @@ static void bdrv_replace_child(BdrvChild *child, 
BlockDriverState *new_bs)
 bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL, _abort);
 bdrv_set_perm(old_bs, perm, shared_perm);
 }
-
-if (new_bs) {
-bdrv_get_cumulative_perm(new_bs, , _perm);
-bdrv_set_perm(new_bs, perm, shared_perm);
-}
 }
 
 BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
-- 
2.20.1

[Qemu-devel] [PATCH v2 6/7] block: Add *tighten_restrictions to *check*_perm()

[Max Reitz]

This patch makes three functions report whether the necessary permission
change tightens restrictions or not.  These functions are:
- bdrv_check_perm()
- bdrv_check_update_perm()
- bdrv_child_check_perm()

Callers can use this result to decide whether a failure is fatal or not
(see the next patch).

Signed-off-by: Max Reitz 
---
 block.c | 89 ++---
 1 file changed, 72 insertions(+), 17 deletions(-)

diff --git a/block.c b/block.c
index da7da8cc6c..8f517be5b4 100644
--- a/block.c
+++ b/block.c
@@ -1686,9 +1686,12 @@ static int bdrv_fill_options(QDict **options, const char 
*filename,
 
 static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q,
  uint64_t perm, uint64_t shared,
- GSList *ignore_children, Error **errp);
+ GSList *ignore_children,
+ bool *tighten_restrictions, Error **errp);
 static void bdrv_child_abort_perm_update(BdrvChild *c);
 static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);
+static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm,
+ uint64_t *shared_perm);
 
 typedef struct BlockReopenQueueEntry {
  bool prepared;
@@ -1759,18 +1762,43 @@ static void bdrv_child_perm(BlockDriverState *bs, 
BlockDriverState *child_bs,
  * permissions of all its parents. This involves checking whether all necessary
  * permission changes to child nodes can be performed.
  *
+ * Will set *tighten_restrictions to true if and only if new permissions have 
to
+ * be taken or currently shared permissions are to be unshared.  Otherwise,
+ * errors are not fatal as long as the caller accepts that the restrictions
+ * remain tighter than they need to be.  The caller still has to abort the
+ * transaction.
+ * @tighten_restrictions cannot be used together with @q: When reopening, we 
may
+ * encounter fatal errors even though no restrictions are to be tightened.  For
+ * example, changing a node from RW to RO will fail if the WRITE permission is
+ * to be kept.
+ *
  * A call to this function must always be followed by a call to bdrv_set_perm()
  * or bdrv_abort_perm_update().
  */
 static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q,
uint64_t cumulative_perms,
uint64_t cumulative_shared_perms,
-   GSList *ignore_children, Error **errp)
+   GSList *ignore_children,
+   bool *tighten_restrictions, Error **errp)
 {
 BlockDriver *drv = bs->drv;
 BdrvChild *c;
 int ret;
 
+assert(!q || !tighten_restrictions);
+
+if (tighten_restrictions) {
+uint64_t current_perms, current_shared;
+uint64_t added_perms, removed_shared_perms;
+
+bdrv_get_cumulative_perm(bs, _perms, _shared);
+
+added_perms = cumulative_perms & ~current_perms;
+removed_shared_perms = current_shared & ~cumulative_shared_perms;
+
+*tighten_restrictions = added_perms || removed_shared_perms;
+}
+
 /* Write permissions never work with read-only images */
 if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) &&
 !bdrv_is_writable_after_reopen(bs, q))
@@ -1798,11 +1826,18 @@ static int bdrv_check_perm(BlockDriverState *bs, 
BlockReopenQueue *q,
 /* Check all children */
 QLIST_FOREACH(c, >children, next) {
 uint64_t cur_perm, cur_shared;
+bool child_tighten_restr;
+
 bdrv_child_perm(bs, c->bs, c, c->role, q,
 cumulative_perms, cumulative_shared_perms,
 _perm, _shared);
-ret = bdrv_child_check_perm(c, q, cur_perm, cur_shared,
-ignore_children, errp);
+ret = bdrv_child_check_perm(c, q, cur_perm, cur_shared, 
ignore_children,
+tighten_restrictions ? _tighten_restr
+ : NULL,
+errp);
+if (tighten_restrictions) {
+*tighten_restrictions |= child_tighten_restr;
+}
 if (ret < 0) {
 return ret;
 }
@@ -1926,17 +1961,23 @@ char *bdrv_perm_names(uint64_t perm)
  * set, the BdrvChild objects in this list are ignored in the calculations;
  * this allows checking permission updates for an existing reference.
  *
+ * See bdrv_check_perm() for the semantics of @tighten_restrictions.
+ *
  * Needs to be followed by a call to either bdrv_set_perm() or
  * bdrv_abort_perm_update(). */
 static int bdrv_check_update_perm(BlockDriverState *bs, BlockReopenQueue *q,
   uint64_t new_used_perm,
   uint64_t new_shared_perm,
-  GSList *ignore_children, Error **errp)
+

[Qemu-devel] [PATCH v2 4/7] block/commit: Drop bdrv_child_try_set_perm()

[Max Reitz]

commit_top_bs never requests or unshares any permissions.  There is no
reason to make this so explicit here.

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
---
 block/commit.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/block/commit.c b/block/commit.c
index 14e5bb394c..44b3083b84 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -110,8 +110,6 @@ static void commit_abort(Job *job)
  * XXX Can (or should) we somehow keep 'consistent read' blocked even
  * after the failed/cancelled commit job is gone? If we already wrote
  * something to base, the intermediate images aren't valid any more. */
-bdrv_child_try_set_perm(s->commit_top_bs->backing, 0, BLK_PERM_ALL,
-_abort);
 bdrv_replace_node(s->commit_top_bs, backing_bs(s->commit_top_bs),
   _abort);
 
-- 
2.20.1

[Qemu-devel] [PATCH v2 2/7] block: Add bdrv_child_refresh_perms()

[Max Reitz]

If a block node uses bdrv_child_try_set_perm() to change the permission
it takes on its child, the result may be very short-lived.  If anything
makes the block layer recalculate the permissions internally, it will
invoke the node driver's .bdrv_child_perm() implementation.  The
permission/shared permissions masks that returns will then override the
values previously passed to bdrv_child_try_set_perm().

If drivers want a child edge to have specific values for the
permissions/shared permissions mask, it must return them in
.bdrv_child_perm().  Consequentially, there is no need for them to pass
the same values to bdrv_child_try_set_perm() then: It is better to have
a function that invokes .bdrv_child_perm() and calls
bdrv_child_try_set_perm() with the result.  This patch adds such a
function under the name of bdrv_child_refresh_perms().

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
---
 include/block/block_int.h | 15 +++
 block.c   | 12 
 2 files changed, 27 insertions(+)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 94d45c9708..5522e58201 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1184,9 +1184,24 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState 
*child_bs,
   void *opaque, Error **errp);
 void bdrv_root_unref_child(BdrvChild *child);
 
+/**
+ * Sets a BdrvChild's permissions.  Avoid if the parent is a BDS; use
+ * bdrv_child_refresh_perms() instead and make the parent's
+ * .bdrv_child_perm() implementation return the correct values.
+ */
 int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
 Error **errp);
 
+/**
+ * Calls bs->drv->bdrv_child_perm() and updates the child's permission
+ * masks with the result.
+ * Drivers should invoke this function whenever an event occurs that
+ * makes their .bdrv_child_perm() implementation return different
+ * values than before, but which will not result in the block layer
+ * automatically refreshing the permissions.
+ */
+int bdrv_child_refresh_perms(BlockDriverState *bs, BdrvChild *c, Error **errp);
+
 /* Default implementation for BlockDriver.bdrv_child_perm() that can be used by
  * block filters: Forward CONSISTENT_READ, WRITE, WRITE_UNCHANGED and RESIZE to
  * all children */
diff --git a/block.c b/block.c
index 7778e0dd89..be11504940 100644
--- a/block.c
+++ b/block.c
@@ -2048,6 +2048,18 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, 
uint64_t shared,
 return 0;
 }
 
+int bdrv_child_refresh_perms(BlockDriverState *bs, BdrvChild *c, Error **errp)
+{
+uint64_t parent_perms, parent_shared;
+uint64_t perms, shared;
+
+bdrv_get_cumulative_perm(bs, _perms, _shared);
+bdrv_child_perm(bs, c->bs, c, c->role, NULL, parent_perms, parent_shared,
+, );
+
+return bdrv_child_try_set_perm(c, perms, shared, errp);
+}
+
 void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c,
const BdrvChildRole *role,
BlockReopenQueue *reopen_queue,
-- 
2.20.1

[Qemu-devel] [PATCH v2 3/7] block/mirror: Fix child permissions

[Max Reitz]

We cannot use bdrv_child_try_set_perm() to give up all restrictions on
the child edge, and still have bdrv_mirror_top_child_perm() request
BLK_PERM_WRITE.  Fix this by making bdrv_mirror_top_child_perm() return
0/BLK_PERM_ALL when we want to give up all permissions, and replacing
bdrv_child_try_set_perm() by bdrv_child_refresh_perms().

The bdrv_child_try_set_perm() before removing the node with
bdrv_replace_node() is then unnecessary.  No permissions have changed
since the previous invocation of bdrv_child_try_set_perm().

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
---
 block/mirror.c | 32 +++-
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index ff15cfb197..e15adce98e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -85,6 +85,7 @@ typedef struct MirrorBlockJob {
 
 typedef struct MirrorBDSOpaque {
 MirrorBlockJob *job;
+bool stop;
 } MirrorBDSOpaque;
 
 struct MirrorOp {
@@ -656,8 +657,9 @@ static int mirror_exit_common(Job *job)
 
 /* We don't access the source any more. Dropping any WRITE/RESIZE is
  * required before it could become a backing file of target_bs. */
-bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
-_abort);
+bs_opaque->stop = true;
+bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
+ _abort);
 if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
 BlockDriverState *backing = s->is_none_mode ? src : s->base;
 if (backing_bs(target_bs) != backing) {
@@ -704,13 +706,12 @@ static int mirror_exit_common(Job *job)
 g_free(s->replaces);
 bdrv_unref(target_bs);
 
-/* Remove the mirror filter driver from the graph. Before this, get rid of
+/*
+ * Remove the mirror filter driver from the graph. Before this, get rid of
  * the blockers on the intermediate nodes so that the resulting state is
- * valid. Also give up permissions on mirror_top_bs->backing, which might
- * block the removal. */
+ * valid.
+ */
 block_job_remove_all_bdrv(bjob);
-bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
-_abort);
 bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), _abort);
 
 /* We just changed the BDS the job BB refers to (with either or both of the
@@ -1468,6 +1469,18 @@ static void bdrv_mirror_top_child_perm(BlockDriverState 
*bs, BdrvChild *c,
uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared)
 {
+MirrorBDSOpaque *s = bs->opaque;
+
+if (s->stop) {
+/*
+ * If the job is to be stopped, we do not need to forward
+ * anything to the real image.
+ */
+*nperm = 0;
+*nshared = BLK_PERM_ALL;
+return;
+}
+
 /* Must be able to forward guest writes to the real image */
 *nperm = 0;
 if (perm & BLK_PERM_WRITE) {
@@ -1687,8 +1700,9 @@ fail:
 job_early_fail(>common.job);
 }
 
-bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
-_abort);
+bs_opaque->stop = true;
+bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
+ _abort);
 bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), _abort);
 
 bdrv_unref(mirror_top_bs);
-- 
2.20.1

[Qemu-devel] [PATCH v2 7/7] block: Ignore loosening perm restrictions failures

[Max Reitz]

We generally assume that loosening permission restrictions can never
fail.  We have seen in the past that this assumption is wrong.  This has
led to crashes because we generally pass _abort when loosening
permissions.

However, a failure in such a case should actually be handled in quite
the opposite way: It is very much not fatal, so qemu may report it, but
still consider the operation successful.  The only realistic problem is
that qemu may then retain permissions and thus locks on images it
actually does not require.  But again, that is not fatal.

To implement this behavior, we make all functions that change
permissions and that pass _abort to the initiating function
(bdrv_check_perm() or bdrv_child_check_perm()) evaluate the
@loosen_restrictions value introduced in the previous patch.  If it is
true and an error did occur, we abort the permission update, report
the error, and report success to the caller.

bdrv_child_try_set_perm() itself does not pass _abort, but it is
the only public function to change permissions.  As such, callers may
pass _abort to it, expecting dropping permission restrictions to
never fail.

Signed-off-by: Max Reitz 
---
 block.c | 40 ++--
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 8f517be5b4..a5a03906d0 100644
--- a/block.c
+++ b/block.c
@@ -2088,11 +2088,20 @@ static void bdrv_child_abort_perm_update(BdrvChild *c)
 int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared,
 Error **errp)
 {
+Error *local_err = NULL;
 int ret;
+bool tighten_restrictions;
 
-ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL, NULL, errp);
+ret = bdrv_child_check_perm(c, NULL, perm, shared, NULL,
+_restrictions, _err);
 if (ret < 0) {
 bdrv_child_abort_perm_update(c);
+if (tighten_restrictions) {
+error_propagate(errp, local_err);
+} else {
+warn_reportf_err(local_err, "Failed to loosen restrictions: ");
+ret = 0;
+}
 return ret;
 }
 
@@ -2275,10 +2284,20 @@ static void bdrv_replace_child(BdrvChild *child, 
BlockDriverState *new_bs)
 /* Update permissions for old node. This is guaranteed to succeed
  * because we're just taking a parent away, so we're loosening
  * restrictions. */
+bool tighten_restrictions;
+Error *local_err = NULL;
+int ret;
+
 bdrv_get_cumulative_perm(old_bs, , _perm);
-bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL,
-NULL, _abort);
-bdrv_set_perm(old_bs, perm, shared_perm);
+ret = bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL,
+  _restrictions, _err);
+assert(tighten_restrictions == false);
+if (ret < 0) {
+warn_reportf_err(local_err, "Failed to loosen restrictions: ");
+bdrv_abort_perm_update(old_bs);
+} else {
+bdrv_set_perm(old_bs, perm, shared_perm);
+}
 }
 }
 
@@ -5322,7 +5341,9 @@ static bool bdrv_has_bds_parent(BlockDriverState *bs, 
bool only_active)
 static int bdrv_inactivate_recurse(BlockDriverState *bs)
 {
 BdrvChild *child, *parent;
+bool tighten_restrictions;
 uint64_t perm, shared_perm;
+Error *local_err = NULL;
 int ret;
 
 if (!bs->drv) {
@@ -5358,8 +5379,15 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs)
 
 /* Update permissions, they may differ for inactive nodes */
 bdrv_get_cumulative_perm(bs, , _perm);
-bdrv_check_perm(bs, NULL, perm, shared_perm, NULL, NULL, _abort);
-bdrv_set_perm(bs, perm, shared_perm);
+ret = bdrv_check_perm(bs, NULL, perm, shared_perm, NULL,
+  _restrictions, _err);
+assert(tighten_restrictions == false);
+if (ret < 0) {
+warn_reportf_err(local_err, "Failed to loosen restrictions: ");
+bdrv_abort_perm_update(bs);
+} else {
+bdrv_set_perm(bs, perm, shared_perm);
+}
 
 
 /* Recursively inactivate children */
-- 
2.20.1

[Qemu-devel] [PATCH v2 0/7] block: Ignore loosening perm restrictions failures

[Max Reitz]

Hi,

This series is mainly a fix for
https://bugzilla.redhat.com/show_bug.cgi?id=1703793.  The problem
described there is that mirroring to a gluster volume, then switching
off the volume makes qemu crash.  There are two problems here:

(1) file-posix reopens the FD all the time because it thinks the FD it
has is RDONLY.  It actually isn’t after the first reopen, we just
forgot to change the internal flags.  That’s what patch 1 is for.

(2) Even then, when mirror completes, it drops its write permission on
the FD.  This requires a reopen, which will fail if the volume is
down.  Mirror doesn’t expect that.  Nobody ever expects that
dropping permissions can fail, and rightfully so because that’s what
I think we have generally agreed on.
Therefore, the block layer should hide this error.  This is what the
last two patches are for.

The last patch adds two assertions: bdrv_replace_child() (for the old
BDS) and bdrv_inactivate_recurse() assume they only ever drop
assertions.  This is now substantiated by these new assertions.
It turns out that this assumption was just plain wrong.  Patches 3 to 5
make it right.


v2:
- Patch 1: Set s->perm_change_flags for reopen, too [Kevin]
- Patch 6:
  - Rename loosen_restrictions to tighten_restrictions and kind of
invert its meaning [Kevin]
  - Assert and document that we cannot return useful information about
whether restrictions are loosened or tightened if the caller wants
to reopen the node [Kevin]
- Patch 7: Handle loosen_restrictions -> tighten_restrictions fallout


git backport-diff output against v1:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/7:[0001] [FC] 'file-posix: Update open_flags in raw_set_perm()'
002/7:[] [--] 'block: Add bdrv_child_refresh_perms()'
003/7:[] [--] 'block/mirror: Fix child permissions'
004/7:[] [--] 'block/commit: Drop bdrv_child_try_set_perm()'
005/7:[0018] [FC] 'block: Fix order in bdrv_replace_child()'
    Confuses my v1 patch with 8aecf1d1bd250a, should be:
  [] [--]
006/7:[down] 'block: Add *tighten_restrictions to *check*_perm()'
    Commit title has changed, but should be something like:
  [0061] [FC]
007/7:[0022] [FC] 'block: Ignore loosening perm restrictions failures'


Max Reitz (7):
  file-posix: Update open_flags in raw_set_perm()
  block: Add bdrv_child_refresh_perms()
  block/mirror: Fix child permissions
  block/commit: Drop bdrv_child_try_set_perm()
  block: Fix order in bdrv_replace_child()
  block: Add *tighten_restrictions to *check*_perm()
  block: Ignore loosening perm restrictions failures

 include/block/block_int.h |  15 
 block.c   | 151 --
 block/commit.c|   2 -
 block/file-posix.c|   4 +
 block/mirror.c|  32 +---
 5 files changed, 169 insertions(+), 35 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v2 1/7] file-posix: Update open_flags in raw_set_perm()

[Max Reitz]

raw_check_perm() + raw_set_perm() can change the flags associated with
the current FD.  If so, we have to update BDRVRawState.open_flags
accordingly.  Otherwise, we may keep reopening the FD even though the
current one already has the correct flags.

Signed-off-by: Max Reitz 
Reviewed-by: Kevin Wolf 
---
 block/file-posix.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index 1cf4ee49eb..3e859c4b9f 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -145,6 +145,7 @@ typedef struct BDRVRawState {
 uint64_t locked_shared_perm;
 
 int perm_change_fd;
+int perm_change_flags;
 BDRVReopenState *reopen_state;
 
 #ifdef CONFIG_XFS
@@ -2754,6 +2755,7 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t 
perm, uint64_t shared,
 assert(s->reopen_state->shared_perm == shared);
 rs = s->reopen_state->opaque;
 s->perm_change_fd = rs->fd;
+s->perm_change_flags = rs->open_flags;
 } else {
 /* We may need a new fd if auto-read-only switches the mode */
 ret = raw_reconfigure_getfd(bs, bs->open_flags, _flags, perm,
@@ -2762,6 +2764,7 @@ static int raw_check_perm(BlockDriverState *bs, uint64_t 
perm, uint64_t shared,
 return ret;
 } else if (ret != s->fd) {
 s->perm_change_fd = ret;
+s->perm_change_flags = open_flags;
 }
 }
 
@@ -2800,6 +2803,7 @@ static void raw_set_perm(BlockDriverState *bs, uint64_t 
perm, uint64_t shared)
 if (s->perm_change_fd && s->fd != s->perm_change_fd) {
 qemu_close(s->fd);
 s->fd = s->perm_change_fd;
+s->open_flags = s->perm_change_flags;
 }
 s->perm_change_fd = 0;
 
-- 
2.20.1

[Qemu-devel] [PATCH] configure: Require python3 >= 3.5

[Eduardo Habkost]

The oldest python3 version in distros that will be supported by
QEMU 4.1 is 3.5.3 (the one in Debian Stretch).  Error out if
running python3 < 3.5.

We have a .travis.yml job configured to use Python 3.4.  Change
it to use Python 3.5.

Signed-off-by: Eduardo Habkost 
---
 configure   | 5 +++--
 .travis.yml | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 6b3ed8c532..520c207d66 100755
--- a/configure
+++ b/configure
@@ -1841,8 +1841,9 @@ fi
 
 # Note that if the Python conditional here evaluates True we will exit
 # with status 1 which is a shell 'false' value.
-if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7))'; then
-  error_exit "Cannot use '$python', Python 2 >= 2.7 or Python 3 is required." \
+if ! $python -c 'import sys; sys.exit(sys.version_info < (2,7) or \
+  (3,0) <= sys.version_info < (3,5))'; then
+  error_exit "Cannot use '$python', Python 2 >= 2.7 or Python 3 >= 3.5 is 
required." \
   "Use --python=/path/to/python to specify a supported Python."
 fi
 
diff --git a/.travis.yml b/.travis.yml
index 66448d99d6..0f6986b3f1 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -211,7 +211,7 @@ matrix:
 - CONFIG="--target-list=x86_64-softmmu"
   language: python
   python:
-- "3.4"
+- "3.5"
 
 
 - env:
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [Bug 1828272] Re: 4.0 breaks keyboard autorepeat in guests with xserver

[Frederick Metzengerstein]

** Description changed:

  Description:
  In a linux/bsd guest within X, pressing and holding a key for a short time 
causes an endless repeat of that key in the guest. The release of the key gets 
ignored.
  Example 1: pressing and holding 'a' for a few seconds results in typing of 
'...' endlessly.
  Example 2: pressing and holding 'Backspace' for a few seconds results in 
deleting all your previously typed text.
  
  It doesn't happen within a VT in the guest. It also doesn't happen with
  guests that run windows, reactos or haiku for example.
  
- The problem goes away, when disabling xorgs autorepeat function via "xset -r" 
in the host.
+ The problem goes away when disabling xorgs autorepeat function via "xset -r" 
in the host.
  Normally, this setting should not have any effect on the guest, since it has 
it's own autorepeat setting. So there is some conflict here.
  
  Steps to reproduce:
  Start any linux/bsd guest system with xserver, open a terminal, press and 
hold a key for a short time: Look how it gets typed endlessly (Try a few times 
if it doesn't happen immediately).
  The easiest way is to run a linux live cd, like this (Link to example iso 
:http://download.grml.org/grml64-full_2018.12.iso)
  $ qemu-system-x86_64 -enable-kvm -m 512 -net none -boot d -cdrom 
grml64-full_2018.12.iso
  
  Qemu version info:
  QEMU emulator version 4.0.0
  Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers
  
  System info:
  Linux  5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 
x86_64 GNU/Linux

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

Title:
  4.0 breaks keyboard autorepeat in guests with xserver

Status in QEMU:
  New

Bug description:
  Description:
  In a linux/bsd guest within X, pressing and holding a key for a short time 
causes an endless repeat of that key in the guest. The release of the key gets 
ignored.
  Example 1: pressing and holding 'a' for a few seconds results in typing of 
'...' endlessly.
  Example 2: pressing and holding 'Backspace' for a few seconds results in 
deleting all your previously typed text.

  It doesn't happen within a VT in the guest. It also doesn't happen
  with guests that run windows, reactos or haiku for example.

  The problem goes away when disabling xorgs autorepeat function via "xset -r" 
in the host.
  Normally, this setting should not have any effect on the guest, since it has 
it's own autorepeat setting. So there is some conflict here.

  Steps to reproduce:
  Start any linux/bsd guest system with xserver, open a terminal, press and 
hold a key for a short time: Look how it gets typed endlessly (Try a few times 
if it doesn't happen immediately).
  The easiest way is to run a linux live cd, like this (Link to example iso 
:http://download.grml.org/grml64-full_2018.12.iso)
  $ qemu-system-x86_64 -enable-kvm -m 512 -net none -boot d -cdrom 
grml64-full_2018.12.iso

  Qemu version info:
  QEMU emulator version 4.0.0
  Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers

  System info:
  Linux  5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 
x86_64 GNU/Linux

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

Re: [Qemu-devel] [PATCH] include/exec/poison: Mark TARGET_FMT_lu as poisoned, too

[Richard Henderson]

On 5/8/19 8:06 AM, Thomas Huth wrote:
> We already poison TARGET_FMT_lx and TARGET_FMT_ld, but apparently
> forgot to poison TARGET_FMT_lu, too. Do it now.
> 
> Signed-off-by: Thomas Huth 
> ---
>  include/exec/poison.h | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [Bug 1828272] Re: 4.0 breaks keyboard autorepeat in guests with xserver

[Frederick Metzengerstein]

** Description changed:

  Description:
  In a linux/bsd guest within X, pressing and holding a key for a short time 
causes an endless repeat of that key in the guest. The release of the key gets 
ignored.
  Example 1: pressing and holding 'a' for a few seconds results in typing of 
'...' endlessly.
  Example 2: pressing and holding 'Backspace' for a few seconds results in 
deleting all your previously typed text.
  
  It doesn't happen within a VT in the guest. It also doesn't happen with
  guests that run windows, reactos or haiku for example.
  
  The problem goes away, when disabling xorgs autorepeat function via "xset -r" 
in the host.
  Normally, this setting should not have any effect on the guest, since it has 
it's own autorepeat setting. So there is some conflict here.
  
  Steps to reproduce:
  Start any linux/bsd guest system with xserver, open a terminal, press and 
hold a key for a short time: Look how it gets typed endlessly (Try a few times 
if it doesn't happen immediately).
  The easiest way is to run a linux live cd, like this (Link to example iso 
:http://download.grml.org/grml64-full_2018.12.iso)
- $ qemu-system-x86_64 -enable-kvm -m 512 -boot d -cdrom grml64-full_2018.12.iso
+ $ qemu-system-x86_64 -enable-kvm -m 512 -net none -boot d -cdrom 
grml64-full_2018.12.iso
  
  Qemu version info:
  QEMU emulator version 4.0.0
  Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers
  
  System info:
  Linux  5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 
x86_64 GNU/Linux

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

Title:
  4.0 breaks keyboard autorepeat in guests with xserver

Status in QEMU:
  New

Bug description:
  Description:
  In a linux/bsd guest within X, pressing and holding a key for a short time 
causes an endless repeat of that key in the guest. The release of the key gets 
ignored.
  Example 1: pressing and holding 'a' for a few seconds results in typing of 
'...' endlessly.
  Example 2: pressing and holding 'Backspace' for a few seconds results in 
deleting all your previously typed text.

  It doesn't happen within a VT in the guest. It also doesn't happen
  with guests that run windows, reactos or haiku for example.

  The problem goes away, when disabling xorgs autorepeat function via "xset -r" 
in the host.
  Normally, this setting should not have any effect on the guest, since it has 
it's own autorepeat setting. So there is some conflict here.

  Steps to reproduce:
  Start any linux/bsd guest system with xserver, open a terminal, press and 
hold a key for a short time: Look how it gets typed endlessly (Try a few times 
if it doesn't happen immediately).
  The easiest way is to run a linux live cd, like this (Link to example iso 
:http://download.grml.org/grml64-full_2018.12.iso)
  $ qemu-system-x86_64 -enable-kvm -m 512 -net none -boot d -cdrom 
grml64-full_2018.12.iso

  Qemu version info:
  QEMU emulator version 4.0.0
  Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers

  System info:
  Linux  5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 
x86_64 GNU/Linux

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

[Qemu-devel] [Bug 1828272] [NEW] 4.0 breaks keyboard autorepeat in guests with xserver

[Frederick Metzengerstein]

Public bug reported:

Description:
In a linux/bsd guest within X, pressing and holding a key for a short time 
causes an endless repeat of that key in the guest. The release of the key gets 
ignored.
Example 1: pressing and holding 'a' for a few seconds results in typing of 
'...' endlessly.
Example 2: pressing and holding 'Backspace' for a few seconds results in 
deleting all your previously typed text.

It doesn't happen within a VT in the guest. It also doesn't happen with
guests that run windows, reactos or haiku for example.

The problem goes away, when disabling xorgs autorepeat function via "xset -r" 
in the host.
Normally, this setting should not have any effect on the guest, since it has 
it's own autorepeat setting. So there is some conflict here.

Steps to reproduce:
Start any linux/bsd guest system with xserver, open a terminal, press and hold 
a key for a short time: Look how it gets typed endlessly (Try a few times if it 
doesn't happen immediately).
The easiest way is to run a linux live cd, like this (Link to example iso 
:http://download.grml.org/grml64-full_2018.12.iso)
$ qemu-system-x86_64 -enable-kvm -m 512 -net none -boot d -cdrom 
grml64-full_2018.12.iso

Qemu version info:
QEMU emulator version 4.0.0
Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers

System info:
Linux  5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 
x86_64 GNU/Linux

** Affects: qemu
 Importance: Undecided
 Status: New

** Description changed:

  Description:
  In a linux/bsd guest within X, pressing and holding a key for a short time 
causes an endless repeat of that key in the guest. The release of the key gets 
ignored.
  Example 1: pressing and holding 'a' for a few seconds results in typing of 
'...' endlessly.
  Example 2: pressing and holding 'Backspace' for a few seconds results in 
deleting all your previously typed text.
  
  It doesn't happen within a VT in the guest. It also doesn't happen with
  guests that run windows, reactos or haiku for example.
  
  The problem goes away, when disabling xorgs autorepeat function via "xset -r" 
in the host.
  Normally, this setting should not have any effect on the guest, since it has 
it's own autorepeat setting. So there is some conflict here.
  
  Steps to reproduce:
  Start any linux/bsd guest system with xserver, open a terminal, press and 
hold a key for a short time: Look how it gets typed endlessly (Try a few times 
if it doesn't happen immediately).
  The easiest way is to run a linux live cd, like this (Link to example iso 
:http://download.grml.org/grml64-full_2018.12.iso)
  $ qemu-system-x86_64 -enable-kvm -m 512 -boot d -cdrom grml64-full_2018.12.iso
  
- 
  Qemu version info:
  QEMU emulator version 4.0.0
  Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers
+ 
+ System info:
+ Linux  5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 
x86_64 GNU/Linux

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

Title:
  4.0 breaks keyboard autorepeat in guests with xserver

Status in QEMU:
  New

Bug description:
  Description:
  In a linux/bsd guest within X, pressing and holding a key for a short time 
causes an endless repeat of that key in the guest. The release of the key gets 
ignored.
  Example 1: pressing and holding 'a' for a few seconds results in typing of 
'...' endlessly.
  Example 2: pressing and holding 'Backspace' for a few seconds results in 
deleting all your previously typed text.

  It doesn't happen within a VT in the guest. It also doesn't happen
  with guests that run windows, reactos or haiku for example.

  The problem goes away, when disabling xorgs autorepeat function via "xset -r" 
in the host.
  Normally, this setting should not have any effect on the guest, since it has 
it's own autorepeat setting. So there is some conflict here.

  Steps to reproduce:
  Start any linux/bsd guest system with xserver, open a terminal, press and 
hold a key for a short time: Look how it gets typed endlessly (Try a few times 
if it doesn't happen immediately).
  The easiest way is to run a linux live cd, like this (Link to example iso 
:http://download.grml.org/grml64-full_2018.12.iso)
  $ qemu-system-x86_64 -enable-kvm -m 512 -net none -boot d -cdrom 
grml64-full_2018.12.iso

  Qemu version info:
  QEMU emulator version 4.0.0
  Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers

  System info:
  Linux  5.0.13-arch1-1-ARCH #1 SMP PREEMPT Sun May 5 18:05:41 UTC 2019 
x86_64 GNU/Linux

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

Re: [Qemu-devel] [PATCH] Deprecate Python 2 support

[Eduardo Habkost]

On Wed, May 08, 2019 at 02:50:00PM +0200, Markus Armbruster wrote:
> Daniel P. Berrangé  writes:
> 
> > On Tue, May 07, 2019 at 12:38:14PM +0200, Kevin Wolf wrote:
> >> Am 03.05.2019 um 21:37 hat Eduardo Habkost geschrieben:
> >> > Python 2 will reach end of life in January 1 2020.  Declare it as
> >> > deprecated.
> >> > 
> >> > Signed-off-by: Eduardo Habkost 
> >> > ---
> >> >  configure| 8 
> >> >  qemu-deprecated.texi | 8 
> >> >  2 files changed, 16 insertions(+)
> >> > 
> >> > diff --git a/configure b/configure
> >> > index 5b183c2e39..50385061ed 100755
> >> > --- a/configure
> >> > +++ b/configure
> >> > @@ -6461,6 +6461,14 @@ if test "$supported_os" = "no"; then
> >> >  echo "us upstream at qemu-devel@nongnu.org."
> >> >  fi
> >> >  
> >> > +# Note that if the Python conditional here evaluates True we will exit
> >> > +# with status 1 which is a shell 'false' value.
> >> > +if ! $python -c 'import sys; sys.exit(sys.version_info < (3,0))'; then
> >> > +  echo
> >> > +  echo "WARNING: Python 2 support is deprecated" >&2
> >> > +  echo "WARNING: Python 3 will be required for building future versions 
> >> > of QEMU" >&2
> >> > +fi
> >> 
> >> While it's clear that we want to get rid of Python 2, did we actually
> >> discuss how to decide what the new minimum Python version is? I don't
> >> think any major distribution uses 3.0, which was released in 2008, so
> >> this doesn't seem to make a lot of sense to me as the new minimum.
> 
> Good point.
> 
> >> Currently, 3.6 seems to be a commonly available version. It looks like
> >> Debian stable is at 3.5 still, though it might become oldstable before
> >> the next QEMU release. Do we need to support anything older than that?
> >
> > Per our support build platform doc, the oldest distros we care about will
> > be RHEL-7 and Debian Jessie.  Except we can drop Jessie 2 years after
> > Stretch was released. IOW, we can drop Jessie in June this year, which
> > is before our next releasee. So we don't need to care about the 3.4
> > version in Jessie.
> >
> > RHEL-7 doesn't have py3 at all in standard distros, but it can be obtained
> > via software collections and this has 3.6
> >
> > Debian Strech has 3.5.3, so 3.5 looks like our min viable version.
> 
> Eduardo, care to update configure accordingly?

I'll do it as a separate patch, because updating the minimum
Python 3.x version (which is 3.0 right now) is independent from
Python 2 deprecation.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] tests: Force Python I/O encoding for check-qapi-schema

[Eduardo Habkost]

On Wed, May 08, 2019 at 03:04:43PM +0200, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > On Tue, May 07, 2019 at 03:13:45PM +0100, Daniel P. Berrangé wrote:
> >> On Mon, May 06, 2019 at 06:38:17PM -0300, Eduardo Habkost wrote:
> >> > test-qapi.py doesn't force a specific encoding for stderr or
> >> > stdout, but the reference files used by check-qapi-schema are in
> >> > UTF-8.  This breaks check-qapi-schema under certain circumstances
> >> > (e.g. if using the C locale and Python < 3.7).
> >> > 
> >> > We need to make sure test-qapi.py always generate UTF-8 output
> >> > somehow.  On Python 3.7+ we can do it using
> >> > `sys.stdout.reconfigure(...)`, but we need a solution that works
> >> > with older Python versions.
> >> > 
> >> > Instead of trying a hack like reopening sys.stdout and
> >> > sys.stderr, we can just tell Python to use UTF-8 for I/O encoding
> >> > when running test-qapi.py.  Do it by setting PYTHONIOENCODING.
> >> > 
> >> > Reported-by: Thomas Huth 
> >> > Tested-by: Thomas Huth 
> >> > Signed-off-by: Eduardo Habkost 
> >> > ---
> >> >  tests/Makefile.include | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> > 
> >> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> >> > index 7c8b9c84b2..af88ab6f8b 100644
> >> > --- a/tests/Makefile.include
> >> > +++ b/tests/Makefile.include
> >> > @@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: 
> >> > tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
> >> >  .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
> >> >  $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: 
> >> > $(SRC_PATH)/%.json
> >> >  $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
> >> > -$(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> >> > +PYTHONIOENCODING=utf-8 $(PYTHON) 
> >> > $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
> >> 
> >> I see PYTHONIOENCODING exists since 2.6 which is nice.
> >> 
> >> How about we actually change $(PYTHON) so that it always includes
> >> PYTHONIOENCODING=utf-8 ?
> >> 
> >> That way we avoid continuing to play whack-a-mole with more utf-8
> >> bugs in future.
> >> 
> >> It would also let us revert this:
> >> 
> >>   commit de685ae5e9a4b523513033bd6cadc8187a227170
> >>   Author: Markus Armbruster 
> >>   Date:   Mon Jun 18 19:59:57 2018 +0200
> >> 
> >> qapi: Open files with encoding='utf-8'
> >> 
> >> which had to provide separate logic for py2 vs py3 :-(
> 
> The separate logic will soon be history.  I'd welcome getting rid of the
> remainder anyway.

Which remainder?  Do you mean the encoding='utf-8' arguments to
open()?

> 
> > Not every Python script in the QEMU tree is run by our makefiles
> > and scripts using $(PYTHON).  We need to ensure our scripts and
> > modules won't break when run directly from the command line, too.
> > Setting PYTHONIOENCODING everywhere would just hide these bugs
> > from us.
> 
> I agree for Python scripts that are meant to be run that way (assuming
> such scripts exist).  [...]

All scripts inside ./scripts are meant to be run directly from
the command line, aren't they?


>[...]  For all the others (including all the QAPI-related
> scripts), I'd be quite fine with
> 
> 1. Our build system runs all Python scripts with the
> PYTHONIOENCODING=utf-8
> 
> 2. If you run a Python script yourself, you get to specify the
> PYTHONIOENCODING=utf-8, or use a suitable locale.  Enabling UTF-8 mode
> with PYTHONUTF8=1 or -X utf8 could also work.

I'm OK if we don't actively try to fix those bugs and just expect
people to set PYTHONIOENCODING.  But I don't think we should
reject patches that make the Python code work with non-utf8
locales if it's an easy fix.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v7 2/2] hw/arm: Add arm SBSA reference machine, devices part

[Radoslaw Biernacki]

On Wed, 8 May 2019 at 13:30, Hongbo Zhang  wrote:

> On Tue, 30 Apr 2019 at 22:17, Peter Maydell 
> wrote:
> >
> > On Thu, 18 Apr 2019 at 05:05, Hongbo Zhang 
> wrote:
> > >
> > > Following the previous patch, this patch adds peripheral devices to the
> > > newly introduced SBSA-ref machine.
> > >
> > > Signed-off-by: Hongbo Zhang 
> > > ---
> > >  hw/arm/sbsa-ref.c | 451
> ++
> > >  1 file changed, 451 insertions(+)
> >
> > Some fairly minor comments on this one.
> >
> > > +static void create_flash(const SBSAMachineState *vms,
> > > + MemoryRegion *sysmem,
> > > + MemoryRegion *secure_sysmem)
> > > +{
> > > +/*
> > > + * Create one secure and nonsecure flash devices to fill
> SBSA_FLASH
> > > + * space in the memmap, file passed via -bios goes in the first
> one.
> > > + */
> > > +hwaddr flashsize = vms->memmap[SBSA_FLASH].size / 2;
> > > +hwaddr flashbase = vms->memmap[SBSA_FLASH].base;
> > > +
> > > +create_one_flash("sbsa-ref.flash0", flashbase, flashsize,
> > > + bios_name, secure_sysmem);
> > > +create_one_flash("sbsa-ref.flash1", flashbase + flashsize,
> flashsize,
> > > + NULL, sysmem);
> > > +}
> >
> > I think Markus might have an opinion on the best way to create
> > flash devices on a new board model. Is "just create two flash
> > devices the way the virt board does" the right thing?
> >
> For the firmware part, we are using two flashes, one secure and
> another non-secure.
>
> > > +static void create_ahci(const SBSAMachineState *vms, qemu_irq *pic)
> > > +{
> > > +hwaddr base = vms->memmap[SBSA_AHCI].base;
> > > +int irq = vms->irqmap[SBSA_AHCI];
> > > +DeviceState *dev;
> > > +DriveInfo *hd[NUM_SATA_PORTS];
> > > +SysbusAHCIState *sysahci;
> > > +AHCIState *ahci;
> > > +int i;
> > > +
> > > +dev = qdev_create(NULL, "sysbus-ahci");
> > > +qdev_prop_set_uint32(dev, "num-ports", NUM_SATA_PORTS);
> > > +qdev_init_nofail(dev);
> > > +sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> > > +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irq]);
> > > +
> > > +sysahci = SYSBUS_AHCI(dev);
> > > +ahci = >ahci;
> > > +ide_drive_get(hd, ARRAY_SIZE(hd));
> > > +for (i = 0; i < ahci->ports; i++) {
> > > +if (hd[i] == NULL) {
> > > +continue;
> > > +}
> > > +ide_create_drive(>dev[i].port, 0, hd[i]);
> > > +}
> > > +}
> > > +
> > > +static void create_ehci(const SBSAMachineState *vms, qemu_irq *pic)
> > > +{
> > > +hwaddr base = vms->memmap[SBSA_EHCI].base;
> > > +int irq = vms->irqmap[SBSA_EHCI];
> > > +USBBus *usb_bus;
> > > +
> > > +sysbus_create_simple("platform-ehci-usb", base, pic[irq]);
> > > +
> > > +usb_bus = usb_bus_find(-1);
> > > +usb_create_simple(usb_bus, "usb-kbd");
> > > +usb_create_simple(usb_bus, "usb-mouse");
> >
> > I don't think we should automatically create the usb keyboard
> > and mouse devices. The user can do it on the command line if they
> > want them.
> >
> OK.
>

Actually I need to rise an objection to this one.
As we trying to make SBSA machine as close as possible to real machine, we
should have keyboard and mouse.
Those have the same requirement as for VGA. It's just an expected piece of
HW when you for e.g. installing a server.
We also do a lot of FW work so it is expected to have keyboard (and even
mouse) in UEFI.


>
> > >  static void sbsa_ref_init(MachineState *machine)
> > >  {
> > >  SBSAMachineState *vms = SBSA_MACHINE(machine);
> > > @@ -125,6 +552,7 @@ static void sbsa_ref_init(MachineState *machine)
> > >  bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
> > >  const CPUArchIdList *possible_cpus;
> > >  int n, sbsa_max_cpus;
> > > +qemu_irq pic[NUM_IRQS];
> > >
> > >  if (strcmp(machine->cpu_type, ARM_CPU_TYPE_NAME("cortex-a57"))) {
> > >  error_report("sbsa-ref: CPU type other than the built-in "
> > > @@ -209,11 +637,34 @@ static void sbsa_ref_init(MachineState *machine)
> > >   machine->ram_size);
> > >  memory_region_add_subregion(sysmem, vms->memmap[SBSA_MEM].base,
> ram);
> > >
> > > +create_fdt(vms);
> > > +
> > > +create_flash(vms, sysmem, secure_sysmem ? secure_sysmem : sysmem);
> > > +
> > > +create_secure_ram(vms, secure_sysmem);
> > > +
> > > +create_gic(vms, pic);
> > > +
> > > +create_uart(vms, pic, SBSA_UART, sysmem, serial_hd(0));
> > > +create_uart(vms, pic, SBSA_SECURE_UART, secure_sysmem,
> serial_hd(1));
> > > +create_uart(vms, pic, SBSA_SECURE_UART_MM, secure_sysmem,
> serial_hd(2));
> >
> > What's the third UART for (ie what is the name intended to mean)?
> > Should we have more than one non-secure UART?
> >
> Yes, this is called " Standalone Management Mode", I will add comment
> for it, this is needed by server RAS feature.
>
> > 

Re: [Qemu-devel] [PATCH] target/riscv: Only flush TLB if SATP.ASID changes

[Richard Henderson]

On 5/8/19 10:38 AM, Jonathan Behrens wrote:
> There is an analogous change for ARM here:
> https://patchwork.kernel.org/patch/10649857
> 
> Signed-off-by: Jonathan Behrens 
> ---
>  target/riscv/csr.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v5 03/15] tests/tcg/aarch64: add system boot.S

[Alex Bennée]

Richard Henderson  writes:

> On 4/30/19 9:52 AM, Alex Bennée wrote:
>> +.error:
>> +.string "Terminated by exception.\n"
>
> Put it in .rodata just because we can?
>
>> +/* Page table setup (identity mapping).  */
>> +adrpx0, ttb
>> +add x0, x0, :lo12:ttb
>
> You are in control of the layout of the executable,
> and adr has a 1MB range.  Why use adrp+add?

Well I have to now as I've aligned .data with:

/* align r/w section to next 2mb */
. = ALIGN(1 << 21);

>
>> +/* Create some (big) pages */
>> +adr x1, .   /* phys address */
>> +bic x1, x1, #(1 << 30) - 1  /* 1GB block alignment */
>
> Do you really want 1GB pages?  You'll pretty much only be able to test valid
> memory operations with that.  Which is also true until there's something other
> than an exit for the exception vector... but ya know what I mean.

Not using it for testing but I'm trying to set-up a 2 stage translation
so we get:

  1gb->1gb+2mb = .text/.rodata
  1gb+2mb->1gb+4mb = .data/.bss

>
>> +/* Setup some stack space and enter the test code.
>> + * Assume everthing except the return value is garbage when we
>> + * return, we won't need it.
>> + */
>> +adrpx0, stack
>> +add x0, x0, :lo12:stack
>> +mov  sp, x0
>
> You need a pointer to the end of the stack, not the beginning.
> Again, I think this could be just
>
>   adr sp, stack_end
>
> Also, there's tab/space confusion all through this file.
> IMO, this is assembly, so it *should* be tabs.

I'm adding an entry to editorconfig and fixing up the damage.

>
>> @@ -0,0 +1,22 @@
>> +ENTRY(__start)
>> +
>> +SECTIONS
>> +{
>> +/* virt machine, RAM starts at 1gb */
>> +. = (1 << 30);
>> +.text : {
>> +*(.text)
>> +}
>> +.data : {
>> +*(.data)
>> +}
>> +.rodata : {
>> +*(.rodata)
>> +}
>
> If you ever wanted to make this read-only, swap .rodata before .data, so that
> it's next to .text.

done.

>
>
> r~


--
Alex Bennée

[Qemu-devel] [PATCH] target/riscv: Only flush TLB if SATP.ASID changes

[Jonathan Behrens]

There is an analogous change for ARM here:
https://patchwork.kernel.org/patch/10649857

Signed-off-by: Jonathan Behrens 
---
 target/riscv/csr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6083c782a1..1ec1222da1 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -732,7 +732,9 @@ static int write_satp(CPURISCVState *env, int csrno, 
target_ulong val)
 if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
 return -1;
 } else {
-tlb_flush(CPU(riscv_env_get_cpu(env)));
+if((val ^ env->satp) & SATP_ASID) {
+tlb_flush(CPU(riscv_env_get_cpu(env)));
+}
 env->satp = val;
 }
 }
-- 
2.20.1

Re: [Qemu-devel] [PATCH v10 13/13] qemu/bitops.h: Add extract8 and extract16

[Richard Henderson]

On 5/8/19 7:56 AM, Yoshinori Sato wrote:
> +static inline uint16_t extract16(uint32_t value, int start, int length)

s/uint32_t/uint16_t/

Aside from the possible value of the more restrictive asserts, I'm not sure
what advantage you see in these routines.  All arithmetic in C is promoted to
type "int", which for all supported hosts is 32-bits.

This suggests an implementation for these functions as

assert(...);
return extract32(value, start, length);

because otherwise the (32 - length) subexpression might at first glance appear
to be a cut-and-paste bug.  Whereas it's really required by the larger
subexpression.


r~

Re: [Qemu-devel] [PATCH v10 12/13] hw/registerfields.h: Add 8bit and 16bit register macros.

[Richard Henderson]

On 5/8/19 7:56 AM, Yoshinori Sato wrote:
> Some RX peripheral using 8bit and 16bit registers.
> Added 8bit and 16bit APIs.
> 
> Signed-off-by: Yoshinori Sato 
> ---
>  include/hw/registerfields.h | 32 +++-
>  1 file changed, 31 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v10 10/13] Add rx-softmmu

[Richard Henderson]

On 5/8/19 7:56 AM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 
> ---
>  configure  | 8 
>  default-configs/rx-softmmu.mak | 7 +++
>  include/sysemu/arch_init.h | 1 +
>  arch_init.c| 2 ++
>  hw/Kconfig | 1 +
>  5 files changed, 19 insertions(+)
>  create mode 100644 default-configs/rx-softmmu.mak

This patch must be last (aside from MAINTAINERS, which doesn't have code).

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v10 05/13] target/rx: Miscellaneous files

[Richard Henderson]

On 5/8/19 7:56 AM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/gdbstub.c | 112 
> 
>  target/rx/monitor.c |  38 
>  target/rx/Makefile.objs |  12 ++
>  3 files changed, 162 insertions(+)
>  create mode 100644 target/rx/gdbstub.c
>  create mode 100644 target/rx/monitor.c
>  create mode 100644 target/rx/Makefile.objs

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v10 00/13] Add RX archtecture support

[Richard Henderson]

On 5/8/19 7:55 AM, Yoshinori Sato wrote:
> Changes for v9.
> - Fix "mov.l dsp5[rs],rd".

You need the same change to the disassembler.


r~

Re: [Qemu-devel] [PATCH v10 04/13] target/rx: RX disassembler

[Richard Henderson]

On 5/8/19 7:56 AM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 
> ---
>  include/disas/dis-asm.h |5 +
>  target/rx/disas.c   | 1480 
> +++
>  2 files changed, 1485 insertions(+)
>  create mode 100644 target/rx/disas.c

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH 1/3] spapr/xive: EQ page should be naturally aligned

[Cédric Le Goater]

When the OS configures the EQ page in which to receive event
notifications from the XIVE interrupt controller, the page should be
naturally aligned. Add this check.

Signed-off-by: Cédric Le Goater 
---
 hw/intc/spapr_xive.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 097f88d4608d..666e24e9b447 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -993,6 +993,12 @@ static target_ulong h_int_set_queue_config(PowerPCCPU *cpu,
 case 16:
 case 21:
 case 24:
+if (!QEMU_IS_ALIGNED(qpage, 1ul << qsize)) {
+qemu_log_mask(LOG_GUEST_ERROR, "XIVE: EQ @0x%" HWADDR_PRIx
+  " is not naturally aligned with %" HWADDR_PRIx "\n",
+  qpage, 1ul << qsize);
+return H_P4;
+}
 end.w2 = cpu_to_be32((qpage >> 32) & 0x0fff);
 end.w3 = cpu_to_be32(qpage & 0x);
 end.w0 |= cpu_to_be32(END_W0_ENQUEUE);
-- 
2.20.1

Re: [Qemu-devel] [PATCH v10 03/13] target/rx: CPU definition

[Richard Henderson]

On 5/8/19 7:56 AM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/cpu-qom.h |  52 
>  target/rx/cpu.h | 196 ++
>  target/rx/cpu.c | 222 
> 
>  3 files changed, 470 insertions(+)
>  create mode 100644 target/rx/cpu-qom.h
>  create mode 100644 target/rx/cpu.h
>  create mode 100644 target/rx/cpu.c

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH 0/3] spapr/xive: fixes on EQ page addresses

[Cédric Le Goater]

Hello

Here is a small series adding a check on the EQ page address alignment
and fixing a severe issue when addresses are above 64GB.

Thanks,

C.

Cédric Le Goater (3):
  spapr/xive: EQ page should be naturally aligned
  spapr/xive: fix EQ page addresses above 64GB
  spapr/xive: print out the EQ page address in the monitor

 include/hw/ppc/xive_regs.h |  6 ++
 hw/intc/spapr_xive.c   | 14 ++
 hw/intc/xive.c |  9 +++--
 3 files changed, 19 insertions(+), 10 deletions(-)

-- 
2.20.1

Re: [Qemu-devel] [PATCH v10 02/13] target/rx: TCG helper

[Richard Henderson]

On 5/8/19 7:56 AM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/helper.h|  31 
>  target/rx/helper.c| 148 
>  target/rx/op_helper.c | 481 
> ++
>  3 files changed, 660 insertions(+)
>  create mode 100644 target/rx/helper.h
>  create mode 100644 target/rx/helper.c
>  create mode 100644 target/rx/op_helper.c

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH 3/3] spapr/xive: print out the EQ page address in the monitor

[Cédric Le Goater]

This proved to be a useful information when debugging issues with OS
event queues allocated above 64GB.

Signed-off-by: Cédric Le Goater 
---
 hw/intc/spapr_xive.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 810435c30cc7..7faf03b1fb7c 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -120,6 +120,7 @@ static int spapr_xive_target_to_end(uint32_t target, 
uint8_t prio,
 static void spapr_xive_end_pic_print_info(SpaprXive *xive, XiveEND *end,
   Monitor *mon)
 {
+uint64_t qaddr_base = xive_end_qaddr(end);
 uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
 uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1);
 uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
@@ -127,9 +128,9 @@ static void spapr_xive_end_pic_print_info(SpaprXive *xive, 
XiveEND *end,
 uint32_t nvt = xive_get_field32(END_W6_NVT_INDEX, end->w6);
 uint8_t priority = xive_get_field32(END_W7_F0_PRIORITY, end->w7);
 
-monitor_printf(mon, "%3d/%d % 6d/%5d ^%d",
+monitor_printf(mon, "%3d/%d % 6d/%5d @%"PRIx64" ^%d",
spapr_xive_nvt_to_target(0, nvt),
-   priority, qindex, qentries, qgen);
+   priority, qindex, qentries, qaddr_base, qgen);
 
 xive_end_queue_pic_print_info(end, 6, mon);
 monitor_printf(mon, "]");
-- 
2.20.1

Re: [Qemu-devel] [PATCH v10 01/13] target/rx: TCG translation

[Richard Henderson]

On 5/8/19 7:55 AM, Yoshinori Sato wrote:
> +static bool trans_XCHG_mr(DisasContext *ctx, arg_XCHG_mr *a)
> +{
> +TCGv mem, addr;
> +mem = tcg_temp_new();
> +switch (a->mi) {
> +case 0: /* dsp[rs].b */
> +case 1: /* dsp[rs].w */
> +case 2: /* dsp[rs].l */
> +addr = rx_index_addr(ctx, mem, a->ld, a->mi, a->rs);
> +break;
> +case 3: /* dsp[rs].uw */
> +case 4: /* dsp[rs].ub */
> +addr = rx_index_addr(ctx, mem, a->ld, 4 - a->mi, a->rs);
> +break;
> +}

You need

default:
g_assert_not_reached();

to avoid the compilation error pointed out by Phil.

Otherwise,
Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH 2/3] spapr/xive: fix EQ page addresses above 64GB

[Cédric Le Goater]

The high order bits of the address of the OS event queue is stored in
bits [4-31] of word2 of the XIVE END internal structures and the low
order bits in word3. This structure is using Big Endian ordering and
computing the value requires some simple arithmetic which happens to
be wrong. The mask removing bits [0-3] of word2 is applied to the
wrong value and the resulting address is bogus when above 64GB.

Guests with more than 64GB of RAM will allocate pages for the OS event
queues which will reside above the 64GB limit. In this case, the XIVE
device model will wake up the CPUs in case of a notification, such as
IPIs, but the update of the event queue will be written at the wrong
place in memory. The result is uncertain as the guest memory is
trashed and IPI are not delivered.

Introduce a helper xive_end_qaddr() to compute this value correctly in
all places where it is used.

Signed-off-by: Cédric Le Goater 
---
 include/hw/ppc/xive_regs.h | 6 ++
 hw/intc/spapr_xive.c   | 3 +--
 hw/intc/xive.c | 9 +++--
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/hw/ppc/xive_regs.h b/include/hw/ppc/xive_regs.h
index bf36678a242c..1a8c5b5e64f0 100644
--- a/include/hw/ppc/xive_regs.h
+++ b/include/hw/ppc/xive_regs.h
@@ -208,6 +208,12 @@ typedef struct XiveEND {
 #define xive_end_is_backlog(end)  (be32_to_cpu((end)->w0) & END_W0_BACKLOG)
 #define xive_end_is_escalate(end) (be32_to_cpu((end)->w0) & 
END_W0_ESCALATE_CTL)
 
+static inline uint64_t xive_end_qaddr(XiveEND *end)
+{
+return ((uint64_t) be32_to_cpu(end->w2) & 0x0fff) << 32 |
+be32_to_cpu(end->w3);
+}
+
 /* Notification Virtual Target (NVT) */
 typedef struct XiveNVT {
 uint32_tw0;
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 666e24e9b447..810435c30cc7 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -1150,8 +1150,7 @@ static target_ulong h_int_get_queue_config(PowerPCCPU 
*cpu,
 }
 
 if (xive_end_is_enqueue(end)) {
-args[1] = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
-| be32_to_cpu(end->w3);
+args[1] = xive_end_qaddr(end);
 args[2] = xive_get_field32(END_W0_QSIZE, end->w0) + 12;
 } else {
 args[1] = 0;
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index a0b87001da25..dcf2fcd10893 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -1042,8 +1042,7 @@ static const TypeInfo xive_source_info = {
 
 void xive_end_queue_pic_print_info(XiveEND *end, uint32_t width, Monitor *mon)
 {
-uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
-| be32_to_cpu(end->w3);
+uint64_t qaddr_base = xive_end_qaddr(end);
 uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
 uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
 uint32_t qentries = 1 << (qsize + 10);
@@ -1072,8 +1071,7 @@ void xive_end_queue_pic_print_info(XiveEND *end, uint32_t 
width, Monitor *mon)
 
 void xive_end_pic_print_info(XiveEND *end, uint32_t end_idx, Monitor *mon)
 {
-uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
-| be32_to_cpu(end->w3);
+uint64_t qaddr_base = xive_end_qaddr(end);
 uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
 uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1);
 uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
@@ -1101,8 +1099,7 @@ void xive_end_pic_print_info(XiveEND *end, uint32_t 
end_idx, Monitor *mon)
 
 static void xive_end_enqueue(XiveEND *end, uint32_t data)
 {
-uint64_t qaddr_base = (uint64_t) be32_to_cpu(end->w2 & 0x0fff) << 32
-| be32_to_cpu(end->w3);
+uint64_t qaddr_base = xive_end_qaddr(end);
 uint32_t qsize = xive_get_field32(END_W0_QSIZE, end->w0);
 uint32_t qindex = xive_get_field32(END_W1_PAGE_OFF, end->w1);
 uint32_t qgen = xive_get_field32(END_W1_GENERATION, end->w1);
-- 
2.20.1

Re: [Qemu-devel] [PULL v2 00/28] Kconfig for Arm machines

[Philippe Mathieu-Daudé]

[clicked ctrl+enter too fast]

On Wed, May 8, 2019 at 6:43 PM Philippe Mathieu-Daudé  wrote:
> On 5/8/19 5:33 PM, Thomas Huth wrote:
> > On 08/05/2019 17.09, Peter Maydell wrote:
> >> On Tue, 7 May 2019 at 14:45, Thomas Huth  wrote:
> >>>
> >>>  Hi Peter,
> >>>
> >>> the following changes since commit 
> >>> a6ae23831b05a11880b40f7d58e332c45a6b04f7:
> >>>
> >>>   Merge remote-tracking branch 
> >>> 'remotes/ehabkost/tags/python-next-pull-request' into staging (2019-05-03 
> >>> 15:26:09 +0100)
> >>>
> >>> are available in the Git repository at:
> >>>
> >>>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-05-07
> >>>
> >>> for you to fetch changes up to 69f879e9fefab9aaf24893fe4ce23e07756d703c:
> >>>
> >>>   hw/arm: Remove hard-enablement of the remaining PCI devices (2019-05-07 
> >>> 15:01:47 +0200)
> >>>
> >>> 
> >>> Kconfig settings for the Arm machines
> >>> (v2: Fix the dependency of q35 to AHCI_ICH9 in the second patch)
> >>> 
> >>
> >> Hi -- this is still failing in the build test where I 'make clean'
> >
> > Very weird. What is running before the "make clean"? Could you provide
> > me with the content of i386-softmmu/config-devices.mak please?
>
> It worked for me after running 'git fetch --tags', maybe because Thomas
> used the same tag?

Maybe because Thomas used the same tag you are still trying the
previous version?

Re: [Qemu-devel] [PULL v2 00/28] Kconfig for Arm machines

[Philippe Mathieu-Daudé]

On 5/8/19 5:33 PM, Thomas Huth wrote:
> On 08/05/2019 17.09, Peter Maydell wrote:
>> On Tue, 7 May 2019 at 14:45, Thomas Huth  wrote:
>>>
>>>  Hi Peter,
>>>
>>> the following changes since commit a6ae23831b05a11880b40f7d58e332c45a6b04f7:
>>>
>>>   Merge remote-tracking branch 
>>> 'remotes/ehabkost/tags/python-next-pull-request' into staging (2019-05-03 
>>> 15:26:09 +0100)
>>>
>>> are available in the Git repository at:
>>>
>>>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-05-07
>>>
>>> for you to fetch changes up to 69f879e9fefab9aaf24893fe4ce23e07756d703c:
>>>
>>>   hw/arm: Remove hard-enablement of the remaining PCI devices (2019-05-07 
>>> 15:01:47 +0200)
>>>
>>> 
>>> Kconfig settings for the Arm machines
>>> (v2: Fix the dependency of q35 to AHCI_ICH9 in the second patch)
>>> 
>>
>> Hi -- this is still failing in the build test where I 'make clean'
> 
> Very weird. What is running before the "make clean"? Could you provide
> me with the content of i386-softmmu/config-devices.mak please?

It worked for me after running 'git fetch --tags', maybe because Thomas
used the same tag?

Re: [Qemu-devel] [PATCH v10 07/13] hw/timer: RX62N internal timer modules

[Philippe Mathieu-Daudé]

On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> renesas_tmr: 8bit timer modules.
> renesas_cmt: 16bit compare match timer modules.
> This part use many renesas's CPU.
> Hardware manual.
> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> 
> Signed-off-by: Yoshinori Sato 
> ---
>  include/hw/timer/renesas_cmt.h |  33 +++
>  include/hw/timer/renesas_tmr.h |  46 +
>  hw/timer/renesas_cmt.c | 277 +
>  hw/timer/renesas_tmr.c | 458 
> +
>  hw/timer/Kconfig   |   6 +
>  hw/timer/Makefile.objs |   3 +
>  6 files changed, 823 insertions(+)
>  create mode 100644 include/hw/timer/renesas_cmt.h
>  create mode 100644 include/hw/timer/renesas_tmr.h
>  create mode 100644 hw/timer/renesas_cmt.c
>  create mode 100644 hw/timer/renesas_tmr.c

Errors on 32bit host:

  CC  hw/timer/renesas_tmr.o
In file included from qemu/hw/timer/renesas_tmr.c:24:0:
qemu/hw/timer/renesas_tmr.c: In function 'tmr_read':
qemu/hw/timer/renesas_tmr.c:186:23: error: format '%lx' expects argument
of type 'long unsigned int', but argument 2 has type 'hwaddr {aka long
long unsigned int}' [-Werror=format=]
   "renesas_tmr: Invalid read size %08lx.\n", offset);
   ^
qemu/include/qemu/log.h:85:22: note: in definition of macro 'qemu_log_mask'
 qemu_log(FMT, ## __VA_ARGS__);  \
  ^
qemu/hw/timer/renesas_tmr.c:239:23: error: format '%lx' expects argument
of type 'long unsigned int', but argument 2 has type 'hwaddr {aka long
long unsigned int}' [-Werror=format=]
   "renesas_tmr: Register %08lx not implemented\n",
   ^
qemu/include/qemu/log.h:85:22: note: in definition of macro 'qemu_log_mask'
 qemu_log(FMT, ## __VA_ARGS__);  \
  ^
qemu/hw/timer/renesas_tmr.c: In function 'tmr_write':
qemu/hw/timer/renesas_tmr.c:267:23: error: format '%lx' expects argument
of type 'long unsigned int', but argument 2 has type 'hwaddr {aka long
long unsigned int}' [-Werror=format=]
   "renesas_tmr: Invalid write size %08lx.\n", offset);
   ^
qemu/include/qemu/log.h:85:22: note: in definition of macro 'qemu_log_mask'
 qemu_log(FMT, ## __VA_ARGS__);  \
  ^
qemu/hw/timer/renesas_tmr.c:291:23: error: format '%lx' expects argument
of type 'long unsigned int', but argument 2 has type 'hwaddr {aka long
long unsigned int}' [-Werror=format=]
   "renesas_tmr: Register %08lx not implemented\n",
   ^
qemu/include/qemu/log.h:85:22: note: in definition of macro 'qemu_log_mask'
 qemu_log(FMT, ## __VA_ARGS__);  \
  ^
  CC  hw/timer/renesas_cmt.o
In file included from qemu/hw/timer/renesas_cmt.c:24:0:
qemu/hw/timer/renesas_cmt.c: In function 'cmt_read':
qemu/hw/timer/renesas_cmt.c:127:19: error: format '%lx' expects argument
of type 'long unsigned int', but argument 2 has type 'hwaddr {aka long
long unsigned int}' [-Werror=format=]
   "renesas_cmt: Register %08lx not implemented\n",
   ^
qemu/include/qemu/log.h:85:22: note: in definition of macro 'qemu_log_mask'
 qemu_log(FMT, ## __VA_ARGS__);  \
  ^
qemu/hw/timer/renesas_cmt.c: In function 'cmt_write':
qemu/hw/timer/renesas_cmt.c:171:27: error: format '%lx' expects argument
of type 'long unsigned int', but argument 2 has type 'hwaddr {aka long
long unsigned int}' [-Werror=format=]
   "renesas_cmt: Register %08lx not implemented\n",
   ^
qemu/include/qemu/log.h:85:22: note: in definition of macro 'qemu_log_mask'
 qemu_log(FMT, ## __VA_ARGS__);  \
  ^
cc1: all warnings being treated as errors

Re: [Qemu-devel] [PATCH 0/3] acpi: More trace points

[Michael S. Tsirkin]

tagged, thanks!

On Wed, May 08, 2019 at 01:19:47PM +0200, Markus Armbruster wrote:
> Ping?
> 
> Markus Armbruster  writes:
> 
> > I wrote these patches to help me debug an unplug failure.  I expect
> > them to be helpful for others, too.
> >
> > Markus Armbruster (3):
> >   acpi/piix4: Convert debug printf()s to trace events
> >   acpi/pcihp: Convert debug printf()s to trace events
> >   acpi/pcihp: Add a few more trace points related to unplug
> >
> >  hw/acpi/pcihp.c  | 32 +++-
> >  hw/acpi/piix4.c  | 14 +++---
> >  hw/acpi/trace-events | 16 
> >  3 files changed, 34 insertions(+), 28 deletions(-)

Re: [Qemu-devel] [PATCH v10 01/13] target/rx: TCG translation

[Philippe Mathieu-Daudé]

On 5/8/19 4:55 PM, Yoshinori Sato wrote:
> This part only supported RXv1 instructions.
> Instruction manual.
> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01us0032ej0120_rxsm.pdf
> 
> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/translate.c  | 2432 
> 
>  target/rx/insns.decode |  617 
>  2 files changed, 3049 insertions(+)
>  create mode 100644 target/rx/translate.c
>  create mode 100644 target/rx/insns.decode

I got this error (haven't look at it):

qemu/target/rx/translate.c: In function 'trans_XCHG_mr':
qemu/target/rx/translate.c:744:5: error: 'addr' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 tcg_gen_atomic_xchg_i32(cpu_regs[a->rd], addr, cpu_regs[a->rd],
 ^
cc1: all warnings being treated as errors
qemu/rules.mak:69: recipe for target 'target/rx/translate.o' failed

Re: [Qemu-devel] [PATCH v10 06/13] hw/intc: RX62N interrupt controller (ICUa)

[Philippe Mathieu-Daudé]

On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> This implementation supported only ICUa.
> Hardware manual.
> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> 
> Signed-off-by: Yoshinori Sato 
> ---
>  include/hw/intc/rx_icu.h |  49 +++
>  hw/intc/rx_icu.c | 375 
> +++
>  hw/intc/Makefile.objs|   1 +
>  3 files changed, 425 insertions(+)
>  create mode 100644 include/hw/intc/rx_icu.h
>  create mode 100644 hw/intc/rx_icu.c
> 
> diff --git a/include/hw/intc/rx_icu.h b/include/hw/intc/rx_icu.h
> new file mode 100644
> index 00..bc46b3079b
> --- /dev/null
> +++ b/include/hw/intc/rx_icu.h
> @@ -0,0 +1,49 @@
> +#ifndef RX_ICU_H
> +#define RX_ICU_H
> +
> +#include "qemu-common.h"
> +#include "hw/irq.h"
> +
> +struct IRQSource {
> +int sense;

Hmm can you use the enum?

> +int level;
> +};
> +
> +struct RXICUState {
> +SysBusDevice parent_obj;
> +
> +MemoryRegion memory;
> +struct IRQSource src[256];
> +char *icutype;
> +uint32_t nr_irqs;
> +uint32_t *map;
> +uint32_t nr_sense;
> +uint32_t *init_sense;
> +
> +uint8_t ir[256];
> +uint8_t dtcer[256];
> +uint8_t ier[32];
> +uint8_t ipr[142];
> +uint8_t dmasr[4];
> +uint16_t fir;
> +uint8_t nmisr;
> +uint8_t nmier;
> +uint8_t nmiclr;
> +uint8_t nmicr;
> +int req_irq;
> +qemu_irq _irq;
> +qemu_irq _fir;
> +qemu_irq _swi;
> +};
> +typedef struct RXICUState RXICUState;
> +
> +#define TYPE_RXICU "rxicu"
> +#define RXICU(obj) OBJECT_CHECK(RXICUState, (obj), TYPE_RXICU)
> +
> +#define SWI 27

enum {

> +#define TRG_LEVEL 0
> +#define TRG_NEDGE 1
> +#define TRG_PEDGE 2
> +#define TRG_BEDGE 3

};

> +
> +#endif /* RX_ICU_H */
> diff --git a/hw/intc/rx_icu.c b/hw/intc/rx_icu.c
> new file mode 100644
> index 00..345e047a45
> --- /dev/null
> +++ b/hw/intc/rx_icu.c
> @@ -0,0 +1,375 @@
> +/*
> + * RX Interrupt control unit

"Interrupt Control Unit"

"Warning: Only ICUa is supported."

> + *
> + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> + * (Rev.1.40 R01UH0033EJ0140)
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "cpu.h"
> +#include "hw/hw.h"
> +#include "hw/sysbus.h"
> +#include "hw/registerfields.h"
> +#include "hw/intc/rx_icu.h"
> +#include "qemu/error-report.h"
> +
> +REG8(IR, 0)
> +  FIELD(IR, IR,  0, 1)
> +REG8(DTCER, 0x100)
> +  FIELD(DTCER, DTCE,  0, 1)
> +REG8(IER, 0x200)
> +REG8(SWINTR, 0x2e0)
> +  FIELD(SWINTR, SWINT, 0, 1)
> +REG16(FIR, 0x2f0)
> +  FIELD(FIR, FVCT, 0, 8)
> +  FIELD(FIR, FIEN, 15, 1)
> +REG8(IPR, 0x300)
> +  FIELD(IPR, IPR, 0, 4)
> +REG8(DMRSR, 0x400)
> +REG8(IRQCR, 0x500)
> +  FIELD(IRQCR, IRQMD, 2, 2)
> +REG8(NMISR, 0x580)
> +  FIELD(NMISR, NMIST, 0, 1)
> +  FIELD(NMISR, LVDST, 1, 1)
> +  FIELD(NMISR, OSTST, 2, 1)
> +REG8(NMIER, 0x581)
> +  FIELD(NMIER, NMIEN, 0, 1)
> +  FIELD(NMIER, LVDEN, 1, 1)
> +  FIELD(NMIER, OSTEN, 2, 1)
> +REG8(NMICLR, 0x582)
> +  FIELD(NMICLR, NMICLR, 0, 1)
> +  FIELD(NMICLR, OSTCLR, 2, 1)
> +REG8(NMICR, 0x583)
> +  FIELD(NMICR, NMIMD, 3, 1)
> +
> +#define request(icu, n) (icu->ipr[icu->map[n]] << 8 | n)
> +
> +static qemu_irq *rxicu_pin(RXICUState *icu, int n_IRQ)
> +{
> +if ((icu->fir & R_FIR_FIEN_MASK) &&
> +(icu->fir & R_FIR_FVCT_MASK) == n_IRQ) {
> +return >_fir;
> +} else {
> +return >_irq;
> +}
> +}
> +
> +static void rxicu_request(RXICUState *icu, int n_IRQ)
> +{
> +int enable;
> +
> +enable = icu->ier[n_IRQ / 8] & (1 << (n_IRQ & 7));
> +if (n_IRQ > 0 && enable != 0 && atomic_read(>req_irq) < 0) {
> +atomic_set(>req_irq, n_IRQ);
> +qemu_set_irq(*rxicu_pin(icu, n_IRQ), request(icu, n_IRQ));
> +}
> +}
> +
> +static void rxicu_set_irq(void *opaque, int n_IRQ, int level)
> +{
> +RXICUState *icu = opaque;
> +struct IRQSource *src;
> +int issue;
> +
> +if (n_IRQ >= 256) {

OK this should be RX_IRQ_COUNT (noted in later patch).

> +error_report("%s: IRQ %d out of range", __func__, n_IRQ);
> +return;
> +}
> +
> +src = >src[n_IRQ];
> +
> +level = (level != 0);
> +switch (src->sense) {
> +case TRG_LEVEL:
> +/* 

Re: [Qemu-devel] [PATCH 4/6] fsdev: Error out when unsupported option is passed

[Eric Blake]

On 5/7/19 3:45 AM, Greg Kurz wrote:
> Each fsdriver only supports a subset of the options that can be passed
> to -fsdev. Unsupported options are simply ignored. This could cause the
> user to erroneously think QEMU has a bug.
> 
> Enforce strict checking of supported options for all fsdrivers. This
> shouldn't impact libvirt, since it doesn't know about he synth and

s/he/the/

> proxy fsdrivers.
> 
> Signed-off-by: Greg Kurz 
> ---
>  fsdev/qemu-fsdev.c |   74 
> ++--
>  1 file changed, 71 insertions(+), 3 deletions(-)
> 

> 
> +#define COMMON_FS_DRIVER_OPTIONS "id", "fsdriver", "readonly"
> +
>  static FsDriverTable FsDrivers[] = {
> -{ .name = "local", .ops = _ops},
> -{ .name = "synth", .ops = _ops},
> -{ .name = "proxy", .ops = _ops},
> +{
> +.name = "local",
> +.ops = _ops,
> +.opts = (const char * []) {
> +COMMON_FS_DRIVER_OPTIONS,
> +"security_model",


> +static int validate_opt(void *opaque, const char *name, const char *value,
> +Error **errp)
> +{
> +FsDriverTable *drv = opaque;
> +const char **opt;
> +
> +for (opt = drv->opts; *opt; opt++) {
> +if (!strcmp(*opt, name)) {
> +return 0;
> +}
> +}
> +
> +error_setg(errp, "'%s' is invalid for fsdriver '%s'", name, drv->name);
> +return -1;
> +}

When we ever reach command-line QAPIfication, this might go away. In the
meantime, this is an improvement.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v10 08/13] hw/char: RX62N serial communication interface (SCI)

[Philippe Mathieu-Daudé]

On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> This module supported only non FIFO type.
> Hardware manual.
> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> 
> Signed-off-by: Yoshinori Sato 
> ---
>  include/hw/char/renesas_sci.h |  45 ++
>  hw/char/renesas_sci.c | 340 
> ++
>  hw/char/Kconfig   |   3 +
>  hw/char/Makefile.objs |   2 +-
>  4 files changed, 389 insertions(+), 1 deletion(-)
>  create mode 100644 include/hw/char/renesas_sci.h
>  create mode 100644 hw/char/renesas_sci.c
> 
> diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
> new file mode 100644
> index 00..50d1336944
> --- /dev/null
> +++ b/include/hw/char/renesas_sci.h
> @@ -0,0 +1,45 @@
> +/*
> + * Renesas Serial Communication Interface
> + *
> + * Copyright (c) 2018 Yoshinori Sato
> + *
> + * This code is licensed under the GPL version 2 or later.
> + *
> + */
> +
> +#include "chardev/char-fe.h"
> +#include "qemu/timer.h"
> +#include "hw/sysbus.h"
> +
> +#define TYPE_RENESAS_SCI "renesas-sci"
> +#define RSCI(obj) OBJECT_CHECK(RSCIState, (obj), TYPE_RENESAS_SCI)
> +
> +enum {
> +ERI = 0,
> +RXI = 1,
> +TXI = 2,
> +TEI = 3,
> +SCI_NR_IRQ = 4,
> +};
> +
> +typedef struct {
> +SysBusDevice parent_obj;
> +MemoryRegion memory;
> +
> +uint8_t smr;
> +uint8_t brr;
> +uint8_t scr;
> +uint8_t tdr;
> +uint8_t ssr;
> +uint8_t rdr;
> +uint8_t scmr;
> +uint8_t semr;
> +
> +uint8_t read_ssr;
> +int64_t trtime;
> +int64_t rx_next;
> +QEMUTimer *timer;
> +CharBackend chr;
> +uint64_t input_freq;
> +qemu_irq irq[SCI_NR_IRQ];
> +} RSCIState;
> diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
> new file mode 100644
> index 00..2e7c3e460e
> --- /dev/null
> +++ b/hw/char/renesas_sci.c
> @@ -0,0 +1,340 @@
> +/*
> + * Renesas Serial Communication Interface
> + *
> + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> + * (Rev.1.40 R01UH0033EJ0140)
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "hw/hw.h"
> +#include "hw/sysbus.h"
> +#include "hw/registerfields.h"
> +#include "hw/char/renesas_sci.h"
> +#include "qemu/error-report.h"
> +
> +/* SCI register map */
> +REG8(SMR, 0)
> +  FIELD(SMR, CKS,  0, 2)
> +  FIELD(SMR, MP,   2, 1)
> +  FIELD(SMR, STOP, 3, 1)
> +  FIELD(SMR, PM,   4, 1)
> +  FIELD(SMR, PE,   5, 1)
> +  FIELD(SMR, CHR,  6, 1)
> +  FIELD(SMR, CM,   7, 1)
> +REG8(BRR, 1)
> +REG8(SCR, 2)
> +  FIELD(SCR, CKE, 0, 2)
> +  FIELD(SCR, TEIE, 2, 1)
> +  FIELD(SCR, MPIE, 3, 1)
> +  FIELD(SCR, RE,   4, 1)
> +  FIELD(SCR, TE,   5, 1)
> +  FIELD(SCR, RIE,  6, 1)
> +  FIELD(SCR, TIE,  7, 1)
> +REG8(TDR, 3)
> +REG8(SSR, 4)
> +  FIELD(SSR, MPBT, 0, 1)
> +  FIELD(SSR, MPB,  1, 1)
> +  FIELD(SSR, TEND, 2, 1)
> +  FIELD(SSR, ERR, 3, 3)
> +FIELD(SSR, PER,  3, 1)
> +FIELD(SSR, FER,  4, 1)
> +FIELD(SSR, ORER, 5, 1)
> +  FIELD(SSR, RDRF, 6, 1)
> +  FIELD(SSR, TDRE, 7, 1)
> +REG8(RDR, 5)
> +REG8(SCMR, 6)
> +  FIELD(SCMR, SMIF, 0, 1)
> +  FIELD(SCMR, SINV, 2, 1)
> +  FIELD(SCMR, SDIR, 3, 1)
> +  FIELD(SCMR, BCP2, 7, 1)
> +REG8(SEMR, 7)
> +  FIELD(SEMR, ACS0, 0, 1)
> +  FIELD(SEMR, ABCS, 4, 1)
> +
> +static int can_receive(void *opaque)
> +{
> +RSCIState *sci = RSCI(opaque);
> +if (sci->rx_next > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
> +return 0;
> +} else {
> +return FIELD_EX8(sci->scr, SCR, RE);
> +}
> +}
> +
> +static void receive(void *opaque, const uint8_t *buf, int size)
> +{
> +RSCIState *sci = RSCI(opaque);
> +sci->rx_next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime;
> +if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
> +sci->ssr = FIELD_DP8(sci->ssr, SSR, ORER, 1);
> +if (FIELD_EX8(sci->scr, SCR, RIE)) {
> +qemu_set_irq(sci->irq[ERI], 1);
> +}
> +} else {
> +sci->rdr = buf[0];
> +sci->ssr = FIELD_DP8(sci->ssr, SSR, RDRF, 1);
> +if (FIELD_EX8(sci->scr, SCR, RIE)) {
> +qemu_irq_pulse(sci->irq[RXI]);
> +}
> +}
> +}
> +
> +static void send_byte(RSCIState *sci)
> +{
> +if 

Re: [Qemu-devel] [PULL 0/5] NBD patches for 2019-05-07

[Peter Maydell]

On Tue, 7 May 2019 at 16:03, Eric Blake  wrote:
>
> The following changes since commit 19eb2d4e736dc895f31fbd6b520e514f10cc08e0:
>
>   Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into 
> staging (2019-05-07 10:43:32 +0100)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2019-05-07
>
> for you to fetch changes up to 8fabb8be37775ebb32b0d78bc7be815a29b8a107:
>
>   iotests: Make 182 do without device_add (2019-05-07 09:43:42 -0500)
>
> 
> nbd patches for 2019-05-07
>
> - iotest improvements
>

Applied, thanks.

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

-- PMM

Re: [Qemu-devel] [PATCH v10 01/13] target/rx: TCG translation

[Philippe Mathieu-Daudé]

On 5/8/19 4:55 PM, Yoshinori Sato wrote:
> This part only supported RXv1 instructions.
> Instruction manual.
> https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01us0032ej0120_rxsm.pdf
> 
> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/translate.c  | 2432 
> 
>  target/rx/insns.decode |  617 
>  2 files changed, 3049 insertions(+)
>  create mode 100644 target/rx/translate.c
>  create mode 100644 target/rx/insns.decode
> 
> diff --git a/target/rx/translate.c b/target/rx/translate.c
> new file mode 100644
> index 00..51e8f88aca
> --- /dev/null
> +++ b/target/rx/translate.c
> @@ -0,0 +1,2432 @@
> +/*
> + *  RX translation
> + *
> + *  Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/bswap.h"
> +#include "qemu/qemu-print.h"
> +#include "cpu.h"
> +#include "disas/disas.h"

Why include "disas.h" here?

> +#include "exec/exec-all.h"
> +#include "tcg-op.h"
> +#include "exec/cpu_ldst.h"
> +#include "exec/helper-proto.h"
> +#include "exec/helper-gen.h"
> +#include "exec/translator.h"
> +#include "hw/registerfields.h"

^ this include is not needed.

> +#include "trace-tcg.h"
> +#include "exec/log.h"
> +
> +typedef struct DisasContext {
> +DisasContextBase base;
> +CPURXState *env;
> +uint32_t pc;
> +} DisasContext;
> +
> +typedef struct DisasCompare {
> +TCGv value;
> +TCGv temp;
> +TCGCond cond;
> +} DisasCompare;
> +
> +const char rx_crname[][6] = {
> +"psw", "pc", "usp", "fpsw", "", "", "", "",
> +"bpsw", "bpc", "isp", "fintv", "intb", "", "", "",
> +};
> +
> +/* Target-specific values for dc->base.is_jmp.  */
> +#define DISAS_JUMPDISAS_TARGET_0
> +#define DISAS_UPDATE  DISAS_TARGET_1
> +#define DISAS_EXITDISAS_TARGET_2
> +
> +/* global register indexes */
> +static TCGv cpu_regs[16];
> +static TCGv cpu_psw_o, cpu_psw_s, cpu_psw_z, cpu_psw_c;
> +static TCGv cpu_psw_i, cpu_psw_pm, cpu_psw_u, cpu_psw_ipl;
> +static TCGv cpu_usp, cpu_fpsw, cpu_bpsw, cpu_bpc, cpu_isp;
> +static TCGv cpu_fintv, cpu_intb, cpu_pc;
> +static TCGv_i64 cpu_acc;
> +
> +#define cpu_sp cpu_regs[0]
> +
> +#include "exec/gen-icount.h"
> +
> +/* decoder helper */
> +static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t insn,
> +   int i, int n)
> +{
> +while (++i <= n) {
> +uint8_t b = cpu_ldub_code(ctx->env, ctx->base.pc_next++);
> +insn |= b << (32 - i * 8);
> +}
> +return insn;
> +}
> +
> +static uint32_t li(DisasContext *ctx, int sz)
> +{
> +int32_t tmp, addr;
> +CPURXState *env = ctx->env;
> +addr = ctx->base.pc_next;
> +
> +tcg_debug_assert(sz < 4);
> +switch (sz) {
> +case 1:
> +ctx->base.pc_next += 1;
> +return cpu_ldsb_code(env, addr);
> +case 2:
> +ctx->base.pc_next += 2;
> +return cpu_ldsw_code(env, addr);
> +case 3:
> +ctx->base.pc_next += 3;
> +tmp = cpu_ldsb_code(env, addr + 2) << 16;
> +tmp |= cpu_lduw_code(env, addr) & 0x;
> +return tmp;
> +case 0:
> +ctx->base.pc_next += 4;
> +return cpu_ldl_code(env, addr);
> +}
> +return 0;
> +}
> +
> +static int bdsp_s(DisasContext *ctx, int d)
> +{
> +/*
> + * 0 -> 8
> + * 1 -> 9
> + * 2 -> 10
> + * 3 -> 3
> + * :
> + * 7 -> 7
> + */
> +if (d < 3) {
> +d += 8;
> +}
> +return d;
> +}
> +
> +/* Include the auto-generated decoder. */
> +#include "decode.inc.c"
> +
> +void rx_cpu_dump_state(CPUState *cs, FILE *f, int flags)
> +{
> +RXCPU *cpu = RXCPU(cs);
> +CPURXState *env = >env;
> +int i;
> +uint32_t psw;
> +
> +psw = rx_cpu_pack_psw(env);
> +qemu_fprintf(f, "pc=0x%08x psw=0x%08x\n",
> + env->pc, psw);
> +for (i = 0; i < 16; i += 4) {
> +qemu_fprintf(f, "r%d=0x%08x r%d=0x%08x r%d=0x%08x r%d=0x%08x\n",
> + i, env->regs[i], i + 1, env->regs[i + 1],
> + i + 2, env->regs[i + 2], i + 3, env->regs[i + 3]);
> +}
> +}
> +
> +static bool use_goto_tb(DisasContext *dc, target_ulong dest)
> +{
> +if (unlikely(dc->base.singlestep_enabled)) {
> +return false;
> +} else {
> +return true;
> +}
> +}
> +
> +static void gen_goto_tb(DisasContext *dc, int n, 

Re: [Qemu-devel] [PATCH v10 05/13] target/rx: Miscellaneous files

[Philippe Mathieu-Daudé]

On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/gdbstub.c | 112 
> 
>  target/rx/monitor.c |  38 
>  target/rx/Makefile.objs |  12 ++
>  3 files changed, 162 insertions(+)
>  create mode 100644 target/rx/gdbstub.c
>  create mode 100644 target/rx/monitor.c
>  create mode 100644 target/rx/Makefile.objs
> 
> diff --git a/target/rx/gdbstub.c b/target/rx/gdbstub.c
> new file mode 100644
> index 00..d76ca52e82
> --- /dev/null
> +++ b/target/rx/gdbstub.c
> @@ -0,0 +1,112 @@
> +/*
> + * RX gdb server stub
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "exec/gdbstub.h"
> +
> +int rx_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> +{
> +RXCPU *cpu = RXCPU(cs);
> +CPURXState *env = >env;
> +
> +switch (n) {
> +case 0 ... 15:
> +return gdb_get_regl(mem_buf, env->regs[n]);
> +case 16:
> +return gdb_get_regl(mem_buf, (env->psw_u) ? env->regs[0] : env->usp);
> +case 17:
> +return gdb_get_regl(mem_buf, (!env->psw_u) ? env->regs[0] : 
> env->isp);
> +case 18:
> +return gdb_get_regl(mem_buf, rx_cpu_pack_psw(env));
> +case 19:
> +return gdb_get_regl(mem_buf, env->pc);
> +case 20:
> +return gdb_get_regl(mem_buf, env->intb);
> +case 21:
> +return gdb_get_regl(mem_buf, env->bpsw);
> +case 22:
> +return gdb_get_regl(mem_buf, env->bpc);
> +case 23:
> +return gdb_get_regl(mem_buf, env->fintv);
> +case 24:
> +return gdb_get_regl(mem_buf, env->fpsw);
> +case 25:
> +return 0;
> +}
> +return 0;
> +}
> +
> +int rx_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> +{
> +RXCPU *cpu = RXCPU(cs);
> +CPURXState *env = >env;
> +uint32_t psw;
> +switch (n) {
> +case 0 ... 15:
> +env->regs[n] = ldl_p(mem_buf);
> +if (n == 0) {
> +if (env->psw_u) {
> +env->usp = env->regs[0];
> +} else {
> +env->isp = env->regs[0];
> +}
> +}
> +break;
> +case 16:
> +env->usp = ldl_p(mem_buf);
> +if (env->psw_u) {
> +env->regs[0] = ldl_p(mem_buf);
> +}
> +break;
> +case 17:
> +env->isp = ldl_p(mem_buf);
> +if (!env->psw_u) {
> +env->regs[0] = ldl_p(mem_buf);
> +}
> +break;
> +case 18:
> +psw = ldl_p(mem_buf);
> +rx_cpu_unpack_psw(env, psw, 1);
> +break;
> +case 19:
> +env->pc = ldl_p(mem_buf);
> +break;
> +case 20:
> +env->intb = ldl_p(mem_buf);
> +break;
> +case 21:
> +env->bpsw = ldl_p(mem_buf);
> +break;
> +case 22:
> +env->bpc = ldl_p(mem_buf);
> +break;
> +case 23:
> +env->fintv = ldl_p(mem_buf);
> +break;
> +case 24:
> +env->fpsw = ldl_p(mem_buf);
> +break;
> +case 25:
> +return 8;
> +default:
> +return 0;
> +}
> +
> +return 4;
> +}
> diff --git a/target/rx/monitor.c b/target/rx/monitor.c
> new file mode 100644
> index 00..5d7a1e58b5
> --- /dev/null
> +++ b/target/rx/monitor.c
> @@ -0,0 +1,38 @@
> +/*
> + * QEMU monitor
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * 

Re: [Qemu-devel] [PATCH v10 10/13] Add rx-softmmu

[Philippe Mathieu-Daudé]

On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 
> ---
>  configure  | 8 
>  default-configs/rx-softmmu.mak | 7 +++
>  include/sysemu/arch_init.h | 1 +
>  arch_init.c| 2 ++
>  hw/Kconfig | 1 +
>  5 files changed, 19 insertions(+)
>  create mode 100644 default-configs/rx-softmmu.mak
> 
> diff --git a/configure b/configure
> index 5b183c2e39..40132c5391 100755
> --- a/configure
> +++ b/configure
> @@ -7547,6 +7547,11 @@ case "$target_name" in
>  gdb_xml_files="riscv-64bit-cpu.xml riscv-64bit-fpu.xml 
> riscv-64bit-csr.xml"
>  target_compiler=$cross_cc_riscv64
>;;
> +  rx)
> +TARGET_ARCH=rx
> +bflt="yes"
> +target_compiler=$cross_cc_rx
> +  ;;
>sh4|sh4eb)
>  TARGET_ARCH=sh4
>  bflt="yes"
> @@ -7767,6 +7772,9 @@ for i in $ARCH $TARGET_BASE_ARCH ; do
>riscv*)
>  disas_config "RISCV"
>;;
> +  rx)
> +disas_config "RX"
> +  ;;
>s390*)
>  disas_config "S390"
>;;
> diff --git a/default-configs/rx-softmmu.mak b/default-configs/rx-softmmu.mak
> new file mode 100644
> index 00..3f62f04e9b
> --- /dev/null
> +++ b/default-configs/rx-softmmu.mak
> @@ -0,0 +1,7 @@
> +# Default configuration for rx-softmmu
> +
> +CONFIG_SERIAL=y
> +CONFIG_RX=y
> +CONFIG_RENESAS_SCI=y
> +CONFIG_RENESAS_TMR=y
> +CONFIG_RENESAS_CMT=y

See previous patch comments, you should only use CONFIG_RX=y here.

Tested-by: Philippe Mathieu-Daudé 

Using only CONFIG_RX=y:
Reviewed-by: Philippe Mathieu-Daudé 

> diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
> index 10cbafe970..3f4f844f7b 100644
> --- a/include/sysemu/arch_init.h
> +++ b/include/sysemu/arch_init.h
> @@ -25,6 +25,7 @@ enum {
>  QEMU_ARCH_NIOS2 = (1 << 17),
>  QEMU_ARCH_HPPA = (1 << 18),
>  QEMU_ARCH_RISCV = (1 << 19),
> +QEMU_ARCH_RX = (1 << 20),
>  };
>  
>  extern const uint32_t arch_type;
> diff --git a/arch_init.c b/arch_init.c
> index f4f3f610c8..cc25ddd7ca 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -74,6 +74,8 @@ int graphic_depth = 32;
>  #define QEMU_ARCH QEMU_ARCH_PPC
>  #elif defined(TARGET_RISCV)
>  #define QEMU_ARCH QEMU_ARCH_RISCV
> +#elif defined(TARGET_RX)
> +#define QEMU_ARCH QEMU_ARCH_RX
>  #elif defined(TARGET_S390X)
>  #define QEMU_ARCH QEMU_ARCH_S390X
>  #elif defined(TARGET_SH4)
> diff --git a/hw/Kconfig b/hw/Kconfig
> index 88b9f15007..63a071092e 100644
> --- a/hw/Kconfig
> +++ b/hw/Kconfig
> @@ -53,6 +53,7 @@ source nios2/Kconfig
>  source openrisc/Kconfig
>  source ppc/Kconfig
>  source riscv/Kconfig
> +source rx/Kconfig
>  source s390x/Kconfig
>  source sh4/Kconfig
>  source sparc/Kconfig
> 

Re: [Qemu-devel] [PATCH v10 09/13] hw/rx: RX Target hardware definition

[Philippe Mathieu-Daudé]

Hi Yoshinori,

"v10"... I'm a bit late at reviewing this, sorry.

On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> rx62n - RX62N cpu.

cpu -> MCU (or SoC)

> rxqemu - QEMU virtual target.

You declare it with MACHINE_TYPE_NAME("rx-qemu"), please use one or
another to be consistent. Why not name it "virt"?

> 
> Signed-off-by: Yoshinori Sato 
> ---
>  include/hw/rx/rx.h|   7 ++
>  include/hw/rx/rx62n.h |  54 
>  hw/rx/rx62n.c | 240 
> ++
>  hw/rx/rxqemu.c| 100 +
>  hw/rx/Kconfig |   2 +
>  hw/rx/Makefile.objs   |   1 +
>  6 files changed, 404 insertions(+)
>  create mode 100644 include/hw/rx/rx.h
>  create mode 100644 include/hw/rx/rx62n.h
>  create mode 100644 hw/rx/rx62n.c
>  create mode 100644 hw/rx/rxqemu.c
>  create mode 100644 hw/rx/Kconfig
>  create mode 100644 hw/rx/Makefile.objs
> 
> diff --git a/include/hw/rx/rx.h b/include/hw/rx/rx.h
> new file mode 100644
> index 00..ff5924b81f
> --- /dev/null
> +++ b/include/hw/rx/rx.h
> @@ -0,0 +1,7 @@
> +#ifndef QEMU_RX_H
> +#define QEMU_RX_H
> +/* Definitions for RX board emulation.  */
> +
> +#include "target/rx/cpu-qom.h"

Do you really need that file?

> +
> +#endif
> diff --git a/include/hw/rx/rx62n.h b/include/hw/rx/rx62n.h
> new file mode 100644
> index 00..8c15399ce0
> --- /dev/null
> +++ b/include/hw/rx/rx62n.h
> @@ -0,0 +1,54 @@
> +/*
> + * RX62N Object

"RX62N MCU"

> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#ifndef HW_RX_RX62N_H
> +#define HW_RX_RX62N_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/rx/rx.h"
> +#include "hw/intc/rx_icu.h"
> +#include "hw/timer/renesas_tmr.h"
> +#include "hw/timer/renesas_cmt.h"
> +#include "hw/char/renesas_sci.h"
> +
> +#define TYPE_RX62N "rx62n"
> +#define TYPE_RX62N_CPU RX_CPU_TYPE_NAME(TYPE_RX62N)
> +#define RX62N(obj) OBJECT_CHECK(RX62NState, (obj), TYPE_RX62N)
> +
> +typedef struct RX62NState {
> +SysBusDevice parent_obj;
> +
> +RXCPU *cpu;
> +RXICUState *icu;
> +RTMRState *tmr[2];
> +RCMTState *cmt[2];
> +RSCIState *sci[6];

Again too late, so I'm OK with this patch using the QDEV API, but I'd
recommend using the SysBus API for new design (Peter, do you confirm?).
You can reserve space for the full object in the container, rather than
using pointer to allocated objects.

I.E. the could would be:

   RSCIState sci[6];

and later you initialize it with:

   object_initialize_child(OBJECT(s), "sci0", >sci[0], ...

> +
> +MemoryRegion *sysmem;
> +bool kernel;
> +
> +MemoryRegion iram;
> +MemoryRegion iomem1;
> +MemoryRegion d_flash;
> +MemoryRegion iomem2;
> +MemoryRegion iomem3;
> +MemoryRegion c_flash;
> +qemu_irq irq[256];
> +} RX62NState;
> +
> +#endif
> diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
> new file mode 100644
> index 00..1dca5dd84c
> --- /dev/null
> +++ b/hw/rx/rx62n.c
> @@ -0,0 +1,240 @@
> +/*
> + * RX62N device
> + *
> + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> + * (Rev.1.40 R01UH0033EJ0140)
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/hw.h"
> +#include "hw/rx/rx62n.h"
> +#include "hw/loader.h"
> +#include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
> +#include "cpu.h"
> +#include "exec/exec-all.h"
> +#include "exec/address-spaces.h"
> +
> +/*
> + * IRQ -> IPR mapping table
> + * 0x00 - 0x91: IPR no (IPR00 to IPR91)
> + * 0xff: IPR not assigned
> + * See "11.3.1 Interrupt Vector Table" in hardware manual.
> + */

#define IRQ_COUNT 256

> +static const int ipr_table[] = {

static const int ipr_table[IRQ_COUNT] = {

> +

Re: [Qemu-devel] [PATCH 6/6] virtfs: Fix documentation of -fsdev and -virtfs

[Thomas Huth]

On 07/05/2019 10.45, Greg Kurz wrote:
> This fixes several things:
> - add "id" description to -virtfs documentation
> - split the description into several lines in both usage and documentation
>   for accurateness and clarity
> - add documentation and usage of the synth fsdriver
> - add "throttling.*" description to -fsdev local
> - add some missing periods
> 
> Buglink: https://bugs.launchpad.net/qemu/+bug/1581976
> Signed-off-by: Greg Kurz 
> ---
>  qemu-options.hx |   84 
> +++
>  1 file changed, 60 insertions(+), 24 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 9c5cc2e6bf70..975342dfbd66 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1232,26 +1232,35 @@ the write back by pressing @key{C-a s} 
> (@pxref{disk_images}).
>  ETEXI
>  
>  DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
> -"-fsdev 
> fsdriver,id=id[,path=path,][security_model={mapped-xattr|mapped-file|passthrough|none}]\n"
> -" 
> [,writeout=immediate][,readonly][,socket=socket|sock_fd=sock_fd][,fmode=fmode][,dmode=dmode]\n"
> +"-fsdev 
> local,id=id,path=path,security_model=mapped-xattr|mapped-file|passthrough|none\n"
> +" [,writeout=immediate][,readonly][,fmode=fmode][,dmode=dmode]\n"
>  " 
> [[,throttling.bps-total=b]|[[,throttling.bps-read=r][,throttling.bps-write=w]]]\n"
>  " 
> [[,throttling.iops-total=i]|[[,throttling.iops-read=r][,throttling.iops-write=w]]]\n"
>  " 
> [[,throttling.bps-total-max=bm]|[[,throttling.bps-read-max=rm][,throttling.bps-write-max=wm]]]\n"
>  " 
> [[,throttling.iops-total-max=im]|[[,throttling.iops-read-max=irm][,throttling.iops-write-max=iwm]]]\n"
> -" [[,throttling.iops-size=is]]\n",
> +" [[,throttling.iops-size=is]]\n"
> +"-fsdev proxy,id=id,socket=socket[,writeout=immediate][,readonly]\n"
> +"-fsdev proxy,id=id,sock_fd=sock_fd[,writeout=immediate][,readonly]\n"
> +"-fsdev synth,id=id\n",
>  QEMU_ARCH_ALL)
>  
>  STEXI
>  
> -@item -fsdev 
> @var{fsdriver},id=@var{id},path=@var{path},[security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}][,fmode=@var{fmode}][,dmode=@var{dmode}]
> +@item -fsdev 
> local,id=@var{id},path=@var{path},security_model=@var{security_model} 
> [,writeout=@var{writeout}][,readonly][,fmode=@var{fmode}][,dmode=@var{dmode}] 
> [,throttling.@var{option}=@var{value}[,throttling.@var{option}=@var{value}[,...]]]
> +@itemx -fsdev 
> proxy,id=@var{id},socket=@var{socket}[,writeout=@var{writeout}][,readonly]
> +@itemx -fsdev 
> proxy,id=@var{id},sock_fd=@var{sock_fd}[,writeout=@var{writeout}][,readonly]
> +@itemx -fsdev synth,id=@var{id}[,readonly]
>  @findex -fsdev
>  Define a new file system device. Valid options are:
>  @table @option
> -@item @var{fsdriver}
> -This option specifies the fs driver backend to use.
> -Currently "local" and "proxy" file system drivers are supported.
> +@item local
> +Accesses to the filesystem are done by QEMU.
> +@item proxy
> +Accesses to the filesystem are done by virtfs-proxy-helper(1).
> +@item synth
> +Synthetic filesystem, only used by QTests.
>  @item id=@var{id}
> -Specifies identifier for this device
> +Specifies identifier for this device.
>  @item path=@var{path}
>  Specifies the export path for the file system device. Files under
>  this path will be available to the 9p client on the guest.
> @@ -1279,17 +1288,33 @@ Enables exporting 9p share as a readonly mount for 
> guests. By default
>  read-write access is given.
>  @item socket=@var{socket}
>  Enables proxy filesystem driver to use passed socket file for communicating
> -with virtfs-proxy-helper
> +with virtfs-proxy-helper(1).

Why did you add a "(1)" after each virtfs-proxy-helper?

... apart from that, the modifications look fine to me (but as mentioned
earlier, I'm not an expert in this area, so not sure whether that counts
;-))

 Thomas

[Qemu-devel] [PATCH v2] block: Use bdrv_unref_child() for all children in bdrv_close()

[Alberto Garcia]

bdrv_unref_child() does the following things:

  - Updates the child->bs->inherits_from pointer.
  - Calls bdrv_detach_child() to remove the BdrvChild from bs->children.
  - Calls bdrv_unref() to unref the child BlockDriverState.

When bdrv_unref_child() was introduced in commit 33a604075c it was not
used in bdrv_close() because the drivers that had additional children
(like quorum or blkverify) had already called bdrv_unref() on their
children during their own close functions.

This was changed later (in 0bd6e91a7e for quorum, in 3e586be0b2 for
blkverify) so there's no reason not to use bdrv_unref_child() in
bdrv_close() anymore.

After this there's also no need to remove bs->backing and bs->file
separately from the rest of the children, so bdrv_close() can be
simplified.

This patch also updates a couple of tests that were doing their own
bdrv_unref().

Signed-off-by: Alberto Garcia 
---
 block.c | 16 +++-
 tests/test-bdrv-drain.c |  6 --
 tests/test-bdrv-graph-mod.c |  1 -
 3 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/block.c b/block.c
index 9ae5c0ed2f..96f8e431da 100644
--- a/block.c
+++ b/block.c
@@ -3843,22 +3843,12 @@ static void bdrv_close(BlockDriverState *bs)
 bs->drv = NULL;
 }
 
-bdrv_set_backing_hd(bs, NULL, _abort);
-
-if (bs->file != NULL) {
-bdrv_unref_child(bs, bs->file);
-bs->file = NULL;
-}
-
 QLIST_FOREACH_SAFE(child, >children, next, next) {
-/* TODO Remove bdrv_unref() from drivers' close function and use
- * bdrv_unref_child() here */
-if (child->bs->inherits_from == bs) {
-child->bs->inherits_from = NULL;
-}
-bdrv_detach_child(child);
+bdrv_unref_child(bs, child);
 }
 
+bs->backing = NULL;
+bs->file = NULL;
 g_free(bs->opaque);
 bs->opaque = NULL;
 atomic_set(>copy_on_read, 0);
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
index eda90750eb..5534c2adf9 100644
--- a/tests/test-bdrv-drain.c
+++ b/tests/test-bdrv-drain.c
@@ -1436,12 +1436,6 @@ static void test_detach_indirect(bool by_parent_cb)
 bdrv_unref(parent_b);
 blk_unref(blk);
 
-/* XXX Once bdrv_close() unref's children instead of just detaching them,
- * this won't be necessary any more. */
-bdrv_unref(a);
-bdrv_unref(a);
-bdrv_unref(c);
-
 g_assert_cmpint(a->refcnt, ==, 1);
 g_assert_cmpint(b->refcnt, ==, 1);
 g_assert_cmpint(c->refcnt, ==, 1);
diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
index 283dc84869..747c0bf8fc 100644
--- a/tests/test-bdrv-graph-mod.c
+++ b/tests/test-bdrv-graph-mod.c
@@ -116,7 +116,6 @@ static void test_update_perm_tree(void)
 g_assert_nonnull(local_err);
 error_free(local_err);
 
-bdrv_unref(bs);
 blk_unref(root);
 }
 
-- 
2.11.0

Re: [Qemu-devel] [PULL v2 00/28] Kconfig for Arm machines

[Thomas Huth]

On 08/05/2019 17.09, Peter Maydell wrote:
> On Tue, 7 May 2019 at 14:45, Thomas Huth  wrote:
>>
>>  Hi Peter,
>>
>> the following changes since commit a6ae23831b05a11880b40f7d58e332c45a6b04f7:
>>
>>   Merge remote-tracking branch 
>> 'remotes/ehabkost/tags/python-next-pull-request' into staging (2019-05-03 
>> 15:26:09 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-05-07
>>
>> for you to fetch changes up to 69f879e9fefab9aaf24893fe4ce23e07756d703c:
>>
>>   hw/arm: Remove hard-enablement of the remaining PCI devices (2019-05-07 
>> 15:01:47 +0200)
>>
>> 
>> Kconfig settings for the Arm machines
>> (v2: Fix the dependency of q35 to AHCI_ICH9 in the second patch)
>> 
> 
> Hi -- this is still failing in the build test where I 'make clean'

Very weird. What is running before the "make clean"? Could you provide
me with the content of i386-softmmu/config-devices.mak please?

 Thomas

Re: [Qemu-devel] [PATCH v10 04/13] target/rx: RX disassembler

[Philippe Mathieu-Daudé]

On 5/8/19 4:56 PM, Yoshinori Sato wrote:
> Signed-off-by: Yoshinori Sato 

Tested-by: Philippe Mathieu-Daudé 

> ---
>  include/disas/dis-asm.h |5 +
>  target/rx/disas.c   | 1480 
> +++
>  2 files changed, 1485 insertions(+)
>  create mode 100644 target/rx/disas.c
> 
> diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
> index 9240ec32c2..de17792e88 100644
> --- a/include/disas/dis-asm.h
> +++ b/include/disas/dis-asm.h
> @@ -226,6 +226,10 @@ enum bfd_architecture
>  #define bfd_mach_nios2r22
>bfd_arch_lm32,   /* Lattice Mico32 */
>  #define bfd_mach_lm32 1
> +  bfd_arch_rx,   /* Renesas RX */
> +#define bfd_mach_rx0x75
> +#define bfd_mach_rx_v2 0x76
> +#define bfd_mach_rx_v3 0x77
>bfd_arch_last
>};
>  #define bfd_mach_s390_31 31
> @@ -433,6 +437,7 @@ int print_insn_little_nios2 (bfd_vma, 
> disassemble_info*);
>  int print_insn_xtensa   (bfd_vma, disassemble_info*);
>  int print_insn_riscv32  (bfd_vma, disassemble_info*);
>  int print_insn_riscv64  (bfd_vma, disassemble_info*);
> +int print_insn_rx(bfd_vma, disassemble_info *);
>  
>  #if 0
>  /* Fetch the disassembler for a given BFD, if that support is available.  */
> diff --git a/target/rx/disas.c b/target/rx/disas.c
> new file mode 100644
> index 00..3dce2dd864
> --- /dev/null
> +++ b/target/rx/disas.c
> @@ -0,0 +1,1480 @@
> +/*
> + * Renesas RX Disassembler
> + *
> + * Copyright (c) 2019 Yoshinori Sato 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "disas/dis-asm.h"
> +#include "qemu/bitops.h"
> +#include "cpu.h"
> +
> +typedef struct DisasContext {
> +disassemble_info *dis;
> +uint32_t addr;
> +uint32_t pc;
> +} DisasContext;
> +
> +
> +static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t insn,
> +   int i, int n)
> +{
> +bfd_byte buf;
> +while (++i <= n) {
> +ctx->dis->read_memory_func(ctx->addr++, , 1, ctx->dis);
> +insn |= buf << (32 - i * 8);
> +}
> +return insn;
> +}
> +
> +static int32_t li(DisasContext *ctx, int sz)
> +{
> +int32_t addr;
> +bfd_byte buf[4];
> +addr = ctx->addr;
> +
> +switch (sz) {
> +case 1:
> +ctx->addr += 1;
> +ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
> +return (int8_t)buf[0];
> +case 2:
> +ctx->addr += 2;
> +ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
> +return ldsw_le_p(buf);
> +case 3:
> +ctx->addr += 3;
> +ctx->dis->read_memory_func(addr, buf, 3, ctx->dis);
> +return (int8_t)buf[2] << 16 | lduw_le_p(buf);
> +case 0:
> +ctx->addr += 4;
> +ctx->dis->read_memory_func(addr, buf, 4, ctx->dis);
> +return ldl_le_p(buf);
> +default:
> +g_assert_not_reached();
> +}
> +}
> +
> +static int bdsp_s(DisasContext *ctx, int d)
> +{
> +/*
> + * 0 -> 8
> + * 1 -> 9
> + * 2 -> 10
> + * 3 -> 3
> + * :
> + * 7 -> 7
> + */
> +if (d < 3) {
> +d += 8;
> +}
> +return d;
> +}
> +
> +/* Include the auto-generated decoder.  */
> +#include "decode.inc.c"
> +
> +#define prt(...) (ctx->dis->fprintf_func)((ctx->dis->stream), __VA_ARGS__)
> +
> +#define RX_MEMORY_BYTE 0
> +#define RX_MEMORY_WORD 1
> +#define RX_MEMORY_LONG 2
> +
> +#define RX_IM_BYTE 0
> +#define RX_IM_WORD 1
> +#define RX_IM_LONG 2
> +#define RX_IM_UWORD 3
> +
> +static const char size[] = {'b', 'w', 'l'};
> +static const char cond[][4] = {
> +"eq", "ne", "c", "nc", "gtu", "leu", "pz", "n",
> +"ge", "lt", "gt", "le", "o", "no", "ra", "f"
> +};
> +static const char psw[] = {
> +'c', 'z', 's', 'o', 0, 0, 0, 0,
> +'i', 'u', 0, 0, 0, 0, 0, 0,
> +};
> +
> +static uint32_t rx_index_addr(int ld, int size, DisasContext *ctx)
> +{
> +bfd_byte buf[2];
> +switch (ld) {
> +case 0:
> +return 0;
> +case 1:
> +ctx->dis->read_memory_func(ctx->addr, buf, 1, ctx->dis);
> +ctx->addr += 1;
> +return ((uint8_t)buf[0]) << size;
> +case 2:
> +ctx->dis->read_memory_func(ctx->addr, buf, 2, ctx->dis);
> +ctx->addr += 2;
> +return lduw_le_p(buf) << size;
> +}
> +g_assert_not_reached();
> +}
> +
> +static void operand(DisasContext *ctx, 

Re: [Qemu-devel] [PATCH v7 2/6] virtio-pmem: Add virtio pmem driver

[Pankaj Gupta]

> > 
> > > +int virtio_pmem_flush(struct nd_region *nd_region)
> > > +{
> > > +int err;
> > > +unsigned long flags;
> > > +struct scatterlist *sgs[2], sg, ret;
> > > +struct virtio_device *vdev = nd_region->provider_data;
> > > +struct virtio_pmem *vpmem = vdev->priv;
> > > +struct virtio_pmem_request *req;
> > > +
> > > +might_sleep();
> > > +req = kmalloc(sizeof(*req), GFP_KERNEL);
> > > +if (!req)
> > > +return -ENOMEM;
> > > +
> > > +req->done = req->wq_buf_avail = false;
> > > +strcpy(req->name, "FLUSH");
> > > +init_waitqueue_head(>host_acked);
> > > +init_waitqueue_head(>wq_buf);
> > > +sg_init_one(, req->name, strlen(req->name));
> > > +sgs[0] = 
> > > +sg_init_one(, >ret, sizeof(req->ret));
> > > +sgs[1] = 
> > > +
> > > +spin_lock_irqsave(>pmem_lock, flags);
> > > +err = virtqueue_add_sgs(vpmem->req_vq, sgs, 1, 1, req, 
> > > GFP_ATOMIC);
> > > +if (err) {
> > > +dev_err(>dev, "failed to send command to virtio 
> > > pmem device\n");
> > > +
> > > +list_add_tail(>req_list, >list);
> > > +spin_unlock_irqrestore(>pmem_lock, flags);
> > > +
> > > +/* When host has read buffer, this completes via 
> > > host_ack */
> > > +wait_event(req->wq_buf, req->wq_buf_avail);
> > > +spin_lock_irqsave(>pmem_lock, flags);
> > > +}
> > 
> > Aren't the arguments in `list_add_tail` swapped? The element we are adding
> 

Yes, arguments for 'list_add_tail' should be swapped.

list_add_tail(>list, >req_list);


Thank you,
Pankaj

Re: [Qemu-devel] [PATCH 0/2] fix qmp-cont vs migration-finish race-crash

[Thomas Huth]

On 24/01/2019 13.25, Vladimir Sementsov-Ogievskiy wrote:
> Hi.
> 
> It's a simple fix for problems reported in "Aborts in iotest 169"
> by Max:
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg05907.html
> 
> In thread Kevin described that a problem itself is bigger and needs
> more effort:
> https://lists.gnu.org/archive/html/qemu-devel/2019-01/msg06136.html
> 
> So, we may continue discussion in "Aborts in iotest 169", and in
> parallel apply these patches at least as a temporary fix.
> 
> The problem of this fix is that we finally have a bit weird interface:
> 
> User gets event MIGRATION_COMPLETED, and after it he can get error
> message "Migration is not finalized yet". 
> 
> But it is better than crash, anyway.
> 
> Vladimir Sementsov-Ogievskiy (2):
>   qmp: forbid qmp_cont in RUN_STATE_FINISH_MIGRATE
>   iotest: fix 169: do not run qmp_cont in RUN_STATE_FINISH_MIGRATE

What happened to these two patches? As far as I can see, they've never
got applied? Has another fix for 169 included instead?

 Thomas

[Qemu-devel] [PATCH v10 01/13] target/rx: TCG translation

[Yoshinori Sato]

This part only supported RXv1 instructions.
Instruction manual.
https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01us0032ej0120_rxsm.pdf

Signed-off-by: Yoshinori Sato 
---
 target/rx/translate.c  | 2432 
 target/rx/insns.decode |  617 
 2 files changed, 3049 insertions(+)
 create mode 100644 target/rx/translate.c
 create mode 100644 target/rx/insns.decode

diff --git a/target/rx/translate.c b/target/rx/translate.c
new file mode 100644
index 00..51e8f88aca
--- /dev/null
+++ b/target/rx/translate.c
@@ -0,0 +1,2432 @@
+/*
+ *  RX translation
+ *
+ *  Copyright (c) 2019 Yoshinori Sato
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bswap.h"
+#include "qemu/qemu-print.h"
+#include "cpu.h"
+#include "disas/disas.h"
+#include "exec/exec-all.h"
+#include "tcg-op.h"
+#include "exec/cpu_ldst.h"
+#include "exec/helper-proto.h"
+#include "exec/helper-gen.h"
+#include "exec/translator.h"
+#include "hw/registerfields.h"
+#include "trace-tcg.h"
+#include "exec/log.h"
+
+typedef struct DisasContext {
+DisasContextBase base;
+CPURXState *env;
+uint32_t pc;
+} DisasContext;
+
+typedef struct DisasCompare {
+TCGv value;
+TCGv temp;
+TCGCond cond;
+} DisasCompare;
+
+const char rx_crname[][6] = {
+"psw", "pc", "usp", "fpsw", "", "", "", "",
+"bpsw", "bpc", "isp", "fintv", "intb", "", "", "",
+};
+
+/* Target-specific values for dc->base.is_jmp.  */
+#define DISAS_JUMPDISAS_TARGET_0
+#define DISAS_UPDATE  DISAS_TARGET_1
+#define DISAS_EXITDISAS_TARGET_2
+
+/* global register indexes */
+static TCGv cpu_regs[16];
+static TCGv cpu_psw_o, cpu_psw_s, cpu_psw_z, cpu_psw_c;
+static TCGv cpu_psw_i, cpu_psw_pm, cpu_psw_u, cpu_psw_ipl;
+static TCGv cpu_usp, cpu_fpsw, cpu_bpsw, cpu_bpc, cpu_isp;
+static TCGv cpu_fintv, cpu_intb, cpu_pc;
+static TCGv_i64 cpu_acc;
+
+#define cpu_sp cpu_regs[0]
+
+#include "exec/gen-icount.h"
+
+/* decoder helper */
+static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t insn,
+   int i, int n)
+{
+while (++i <= n) {
+uint8_t b = cpu_ldub_code(ctx->env, ctx->base.pc_next++);
+insn |= b << (32 - i * 8);
+}
+return insn;
+}
+
+static uint32_t li(DisasContext *ctx, int sz)
+{
+int32_t tmp, addr;
+CPURXState *env = ctx->env;
+addr = ctx->base.pc_next;
+
+tcg_debug_assert(sz < 4);
+switch (sz) {
+case 1:
+ctx->base.pc_next += 1;
+return cpu_ldsb_code(env, addr);
+case 2:
+ctx->base.pc_next += 2;
+return cpu_ldsw_code(env, addr);
+case 3:
+ctx->base.pc_next += 3;
+tmp = cpu_ldsb_code(env, addr + 2) << 16;
+tmp |= cpu_lduw_code(env, addr) & 0x;
+return tmp;
+case 0:
+ctx->base.pc_next += 4;
+return cpu_ldl_code(env, addr);
+}
+return 0;
+}
+
+static int bdsp_s(DisasContext *ctx, int d)
+{
+/*
+ * 0 -> 8
+ * 1 -> 9
+ * 2 -> 10
+ * 3 -> 3
+ * :
+ * 7 -> 7
+ */
+if (d < 3) {
+d += 8;
+}
+return d;
+}
+
+/* Include the auto-generated decoder. */
+#include "decode.inc.c"
+
+void rx_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+{
+RXCPU *cpu = RXCPU(cs);
+CPURXState *env = >env;
+int i;
+uint32_t psw;
+
+psw = rx_cpu_pack_psw(env);
+qemu_fprintf(f, "pc=0x%08x psw=0x%08x\n",
+ env->pc, psw);
+for (i = 0; i < 16; i += 4) {
+qemu_fprintf(f, "r%d=0x%08x r%d=0x%08x r%d=0x%08x r%d=0x%08x\n",
+ i, env->regs[i], i + 1, env->regs[i + 1],
+ i + 2, env->regs[i + 2], i + 3, env->regs[i + 3]);
+}
+}
+
+static bool use_goto_tb(DisasContext *dc, target_ulong dest)
+{
+if (unlikely(dc->base.singlestep_enabled)) {
+return false;
+} else {
+return true;
+}
+}
+
+static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
+{
+if (use_goto_tb(dc, dest)) {
+tcg_gen_goto_tb(n);
+tcg_gen_movi_i32(cpu_pc, dest);
+tcg_gen_exit_tb(dc->base.tb, n);
+} else {
+tcg_gen_movi_i32(cpu_pc, dest);
+if (dc->base.singlestep_enabled) {
+gen_helper_debug(cpu_env);
+} else {
+tcg_gen_lookup_and_goto_ptr();
+}
+}
+dc->base.is_jmp = DISAS_NORETURN;
+}
+
+/* generic 

Re: [Qemu-devel] [PULL v2 00/28] Kconfig for Arm machines

[Peter Maydell]

On Tue, 7 May 2019 at 14:45, Thomas Huth  wrote:
>
>  Hi Peter,
>
> the following changes since commit a6ae23831b05a11880b40f7d58e332c45a6b04f7:
>
>   Merge remote-tracking branch 
> 'remotes/ehabkost/tags/python-next-pull-request' into staging (2019-05-03 
> 15:26:09 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/huth/qemu.git tags/pull-request-2019-05-07
>
> for you to fetch changes up to 69f879e9fefab9aaf24893fe4ce23e07756d703c:
>
>   hw/arm: Remove hard-enablement of the remaining PCI devices (2019-05-07 
> 15:01:47 +0200)
>
> 
> Kconfig settings for the Arm machines
> (v2: Fix the dependency of q35 to AHCI_ICH9 in the second patch)
> 

Hi -- this is still failing in the build test where I 'make clean'
before doing the build:

MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=i386-softmmu/qemu-system-i386
QTEST_QEMU_IMG=qemu-img tests/a
hci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="ahci-test"
qemu-system-i386: Unknown device 'ich9-ahci' for bus 'PCIE'
Broken pipe

thanks
-- PMM

[Qemu-devel] [PATCH] include/exec/poison: Mark TARGET_FMT_lu as poisoned, too

[Thomas Huth]

We already poison TARGET_FMT_lx and TARGET_FMT_ld, but apparently
forgot to poison TARGET_FMT_lu, too. Do it now.

Signed-off-by: Thomas Huth 
---
 include/exec/poison.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 1a7a57baae..b862320fa6 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -44,6 +44,7 @@
 #pragma GCC poison TARGET_LONG_BITS
 #pragma GCC poison TARGET_FMT_lx
 #pragma GCC poison TARGET_FMT_ld
+#pragma GCC poison TARGET_FMT_lu
 
 #pragma GCC poison TARGET_PAGE_SIZE
 #pragma GCC poison TARGET_PAGE_MASK
-- 
2.21.0

[Qemu-devel] [PATCH v10 04/13] target/rx: RX disassembler

[Yoshinori Sato]

Signed-off-by: Yoshinori Sato 
---
 include/disas/dis-asm.h |5 +
 target/rx/disas.c   | 1480 +++
 2 files changed, 1485 insertions(+)
 create mode 100644 target/rx/disas.c

diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h
index 9240ec32c2..de17792e88 100644
--- a/include/disas/dis-asm.h
+++ b/include/disas/dis-asm.h
@@ -226,6 +226,10 @@ enum bfd_architecture
 #define bfd_mach_nios2r22
   bfd_arch_lm32,   /* Lattice Mico32 */
 #define bfd_mach_lm32 1
+  bfd_arch_rx,   /* Renesas RX */
+#define bfd_mach_rx0x75
+#define bfd_mach_rx_v2 0x76
+#define bfd_mach_rx_v3 0x77
   bfd_arch_last
   };
 #define bfd_mach_s390_31 31
@@ -433,6 +437,7 @@ int print_insn_little_nios2 (bfd_vma, 
disassemble_info*);
 int print_insn_xtensa   (bfd_vma, disassemble_info*);
 int print_insn_riscv32  (bfd_vma, disassemble_info*);
 int print_insn_riscv64  (bfd_vma, disassemble_info*);
+int print_insn_rx(bfd_vma, disassemble_info *);
 
 #if 0
 /* Fetch the disassembler for a given BFD, if that support is available.  */
diff --git a/target/rx/disas.c b/target/rx/disas.c
new file mode 100644
index 00..3dce2dd864
--- /dev/null
+++ b/target/rx/disas.c
@@ -0,0 +1,1480 @@
+/*
+ * Renesas RX Disassembler
+ *
+ * Copyright (c) 2019 Yoshinori Sato 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 .
+ */
+
+#include "qemu/osdep.h"
+#include "disas/dis-asm.h"
+#include "qemu/bitops.h"
+#include "cpu.h"
+
+typedef struct DisasContext {
+disassemble_info *dis;
+uint32_t addr;
+uint32_t pc;
+} DisasContext;
+
+
+static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t insn,
+   int i, int n)
+{
+bfd_byte buf;
+while (++i <= n) {
+ctx->dis->read_memory_func(ctx->addr++, , 1, ctx->dis);
+insn |= buf << (32 - i * 8);
+}
+return insn;
+}
+
+static int32_t li(DisasContext *ctx, int sz)
+{
+int32_t addr;
+bfd_byte buf[4];
+addr = ctx->addr;
+
+switch (sz) {
+case 1:
+ctx->addr += 1;
+ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
+return (int8_t)buf[0];
+case 2:
+ctx->addr += 2;
+ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
+return ldsw_le_p(buf);
+case 3:
+ctx->addr += 3;
+ctx->dis->read_memory_func(addr, buf, 3, ctx->dis);
+return (int8_t)buf[2] << 16 | lduw_le_p(buf);
+case 0:
+ctx->addr += 4;
+ctx->dis->read_memory_func(addr, buf, 4, ctx->dis);
+return ldl_le_p(buf);
+default:
+g_assert_not_reached();
+}
+}
+
+static int bdsp_s(DisasContext *ctx, int d)
+{
+/*
+ * 0 -> 8
+ * 1 -> 9
+ * 2 -> 10
+ * 3 -> 3
+ * :
+ * 7 -> 7
+ */
+if (d < 3) {
+d += 8;
+}
+return d;
+}
+
+/* Include the auto-generated decoder.  */
+#include "decode.inc.c"
+
+#define prt(...) (ctx->dis->fprintf_func)((ctx->dis->stream), __VA_ARGS__)
+
+#define RX_MEMORY_BYTE 0
+#define RX_MEMORY_WORD 1
+#define RX_MEMORY_LONG 2
+
+#define RX_IM_BYTE 0
+#define RX_IM_WORD 1
+#define RX_IM_LONG 2
+#define RX_IM_UWORD 3
+
+static const char size[] = {'b', 'w', 'l'};
+static const char cond[][4] = {
+"eq", "ne", "c", "nc", "gtu", "leu", "pz", "n",
+"ge", "lt", "gt", "le", "o", "no", "ra", "f"
+};
+static const char psw[] = {
+'c', 'z', 's', 'o', 0, 0, 0, 0,
+'i', 'u', 0, 0, 0, 0, 0, 0,
+};
+
+static uint32_t rx_index_addr(int ld, int size, DisasContext *ctx)
+{
+bfd_byte buf[2];
+switch (ld) {
+case 0:
+return 0;
+case 1:
+ctx->dis->read_memory_func(ctx->addr, buf, 1, ctx->dis);
+ctx->addr += 1;
+return ((uint8_t)buf[0]) << size;
+case 2:
+ctx->dis->read_memory_func(ctx->addr, buf, 2, ctx->dis);
+ctx->addr += 2;
+return lduw_le_p(buf) << size;
+}
+g_assert_not_reached();
+}
+
+static void operand(DisasContext *ctx, int ld, int mi, int rs, int rd)
+{
+int dsp;
+static const char sizes[][4] = {".b", ".w", ".l", ".uw", ".ub"};
+if (ld < 3) {
+switch (mi) {
+case 4:
+/* dsp[rs].ub */
+dsp = rx_index_addr(ld, RX_MEMORY_BYTE, ctx);
+break;
+case 3:
+/* dsp[rs].uw */
+dsp = rx_index_addr(ld, RX_MEMORY_WORD, ctx);
+break;
+