date:20201119

On 19/11/2020 22.11, Eric Farman wrote:
> 
> 
> On 11/19/20 3:20 PM, Thomas Huth wrote:
>> On 19/11/2020 17.57, Eric Farman wrote:
>>> Let's look at the Reset PSW first instead of the contents of memory.
>>> It might be leftover from an earlier system boot when processing
>>> a chreipl.
>>>
>>> Signed-off-by: Eric Farman 
>>> ---
>>>   pc-bios/s390-ccw/jump2ipl.c | 20 ++--
>>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
>>> index fbae45b03c..67b4afb6a0 100644
>>> --- a/pc-bios/s390-ccw/jump2ipl.c
>>> +++ b/pc-bios/s390-ccw/jump2ipl.c
>>> @@ -72,16 +72,6 @@ void jump_to_IPL_code(uint64_t address)
>>>     void jump_to_low_kernel(void)
>>>   {
>>> -    /*
>>> - * If it looks like a Linux binary, i.e. there is the "S390EP" magic
>>> from
>>> - * arch/s390/kernel/head.S here, then let's jump to the well-known
>>> Linux
>>> - * kernel start address (when jumping to the PSW-at-zero address
>>> instead,
>>> - * the kernel startup code fails when we booted from a network device).
>>> - */
>>> -    if (!memcmp((char *)0x10008, "S390EP", 6)) {
>>> -    jump_to_IPL_code(KERN_IMAGE_START);
>>> -    }
>>> -
>>>   /* Trying to get PSW at zero address */
>>>   if (*((uint64_t *)0) & RESET_PSW_MASK) {
>>>   /*
>>> @@ -92,6 +82,16 @@ void jump_to_low_kernel(void)
>>>   jump_to_IPL_code(0);
>>>   }
>>>   +    /*
>>> + * If it looks like a Linux binary, i.e. there is the "S390EP" magic
>>> from
>>> + * arch/s390/kernel/head.S here, then let's jump to the well-known
>>> Linux
>>> + * kernel start address (when jumping to the PSW-at-zero address
>>> instead,
>>> + * the kernel startup code fails when we booted from a network device).
>>> + */
>>> +    if (!memcmp((char *)0x10008, "S390EP", 6)) {
>>> +    jump_to_IPL_code(KERN_IMAGE_START);
>>> +    }
>>
>> That feels a little bit dangerous ... I assume the order has been that way
>> for a reason, e.g. I think we had to jump to KERN_IMAGE_START for some older
>> versions of the Linux kernel since the startup code that was referenced by
>> the PSW at address zero was not working in KVM...
> 
> Makes sense.  It does seem like a precarious piece of code.
> 
>>
>> What do you think about this alternate idea instead: Clear the memory at
>> location 0x10008 at the very beginning of the main() function (or maybe in
>> boot_setup())? 
> 
> This seems to work too (I put it in boot_setup(), prior to call to
> store_iplb()).

Great, can you send the patch before your holidays next week? (If you don't
have enough time, that's ok, too, it's trivial enough, so I think I could
write such a patch, too)

 Thomas

Re: [PATCH v2 3/8] qnum: QNumValue type for QNum value literals

2020-11-19 Thread Markus Armbruster

Eduardo Habkost  writes:

> On Thu, Nov 19, 2020 at 11:24:52AM +0100, Markus Armbruster wrote:
>> Marc-André Lureau  writes:
>> 
>> > On Tue, Nov 17, 2020 at 6:42 PM Eduardo Habkost  
>> > wrote:
>> >
>> >> On Tue, Nov 17, 2020 at 12:37:56PM +0400, Marc-André Lureau wrote:
>> >> > On Tue, Nov 17, 2020 at 2:43 AM Eduardo Habkost 
>> >> wrote:
>> >> >
>> >> > > Provide a separate QNumValue type that can be used for QNum value
>> >> > > literals without the referencing counting and memory allocation
>> >> > > features provided by QObject.
>> >> > >
>> >> > > Signed-off-by: Eduardo Habkost 
>> >> > > ---
>> >> > > Changes v1 -> v2:
>> >> > > * Fix "make check" failure, by updating check-qnum unit test to
>> >> > >   use the new struct fields
>> >> > > ---
>> >> > >  include/qapi/qmp/qnum.h | 40 +++--
>> >> > >  qobject/qnum.c  | 78 
>> >> > > -
>> >> > >  tests/check-qnum.c  | 14 
>> >> > >  3 files changed, 84 insertions(+), 48 deletions(-)
>> >> > >
>> >> > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
>> >> > > index 55c27b1c24..62fbdfda68 100644
>> >> > > --- a/include/qapi/qmp/qnum.h
>> >> > > +++ b/include/qapi/qmp/qnum.h
>> >> > > @@ -46,20 +46,56 @@ typedef enum {
>> >> > >   * in range: qnum_get_try_int() / qnum_get_try_uint() check range and
>> >> > >   * convert under the hood.
>> >> > >   */
>> >> > > -struct QNum {
>> >> > > -struct QObjectBase_ base;
>> >> > > +
>> >> > > +/**
>> >> > > + * struct QNumValue: the value of a QNum
>> >> > > + *
>> >> > > + * QNumValue literals can be constructed using the `QNUM_VAL_INT`,
>> >> > > + * `QNUM_VAL_UINT`, and `QNUM_VAL_DOUBLE` macros.
>> >> > > + */
>> >> > > +typedef struct QNumValue {
>> >> > > +/* private: */
>> 
>> Do we care?
>
> Are you asking if we want to make them private, or if we want to
> document them as private (assuming they are).
>
> The answer to the latter is yes ("private:" is an indication to
> kernel-doc).  The answer to the former seems to be "no", based on
> your other comments.
>
> Or maybe `kind` should be public and `u` should be private?

You're factoring out a part of struct QNum into new struct QNumValue.
struct QNum is not private before the patch.  I see no need to start
making it or parts of it private now.

When the structure of a data type is to be kept away from its users, I
prefer to keep it out of the public header, so the compiler enforces the
encapsulation.

[...]

[PATCH v2] virtio-blk: seg_max do not subtract 2 if host has VIRTIO_RING_F_INDIRECT_DESC feature

2020-11-19 Thread zhaoxin\RockCuioc

v1 -> v2:
fix codestyle checked by checkpatch.pl

This patch modify virtio-blk seg_max when host has VIRTIO_RING_F_INDIRECT_DESC 
feature, when read/write virtio-blk disk in direct mode, 
this patch can make the bio reach 512k but not 504k if the user buffer physical 
segments are all discontinuous,when use ceph the size of 504k 
will affect performance.This patch should be used in guest kernel 
version>=4.14, kernel after this version virtio driver does not judge 
total_sg and vring num if the host supports indirect descriptor tables.  

Signed-off-by: zhaoxin\RockCuioc 
---
 hw/block/virtio-blk.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index bac2d6f..40bbbd7 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -932,7 +932,11 @@ static void virtio_blk_update_config(VirtIODevice *vdev, 
uint8_t *config)
 blk_get_geometry(s->blk, );
 memset(, 0, sizeof(blkcfg));
 virtio_stq_p(vdev, , capacity);
-virtio_stl_p(vdev, _max,
+if virtio_host_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC)
+virtio_stl_p(vdev, _max,
+ s->conf.seg_max_adjust ? s->conf.queue_size : 128);
+else
+virtio_stl_p(vdev, _max,
  s->conf.seg_max_adjust ? s->conf.queue_size - 2 : 128 - 2);
 virtio_stw_p(vdev, , conf->cyls);
 virtio_stl_p(vdev, _size, blk_size);
-- 
2.10.1.windows.1

Re: [PATCH V13 2/9] meson.build: Re-enable KVM support for MIPS

2020-11-19 Thread Huacai Chen

Hi, Philippe,

On Wed, Nov 18, 2020 at 1:17 AM Philippe Mathieu-Daudé  wrote:
>
> Hi Huacai,
>
> On 10/7/20 10:39 AM, Huacai Chen wrote:
> > After converting from configure to meson, KVM support is lost for MIPS,
> > so re-enable it in meson.build.
> >
> > Fixes: fdb75aeff7c212e1afaaa3a43 ("configure: remove target configuration")
> > Fixes: 8a19980e3fc42239aae054bc9 ("configure: move accelerator logic to 
> > meson")
> > Cc: aolo Bonzini 
> > Signed-off-by: Huacai Chen 
> > ---
> >  meson.build | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/meson.build b/meson.build
> > index 17c89c8..b407ff4 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -59,6 +59,8 @@ elif cpu == 's390x'
> >kvm_targets = ['s390x-softmmu']
> >  elif cpu in ['ppc', 'ppc64']
> >kvm_targets = ['ppc-softmmu', 'ppc64-softmmu']
> > +elif cpu in ['mips', 'mips64']
> > +  kvm_targets = ['mips-softmmu', 'mipsel-softmmu', 'mips64-softmmu', 
> > 'mips64el-softmmu']
>
> Are you sure both 32-bit hosts and targets are supported?
>
> I don't have hardware to test. If you are not working with
> 32-bit hardware I'd remove them.
When I add MIPS64 KVM support (Loongson-3 is MIPS64), MIPS32 KVM is
already there. On the kernel side, MIPS32 KVM is supported, but I
don't know whether it can work well.

Huacai

>
> >  else
> >kvm_targets = []
> >  endif
> >

[Bug 1904954] Re: lan9118 bug peeking receive massage size not equal to received message size

2020-11-19 Thread alfred gedeon

** Description changed:

  peeked message size is not equal to read message size
  
  Bug in the code at line:
  https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209
  
  s->tx_status_fifo_head should be s->rx_status_fifo_head
  
+ Could also be a security bug, as the user could allocate a buffer of
+ size peeked data smaller than the actual packet received, which could
+ cause a buffer overflow and its attaks.
+ 
  Thanks,
  
  Alfred

** Description changed:

  peeked message size is not equal to read message size
  
  Bug in the code at line:
  https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209
  
  s->tx_status_fifo_head should be s->rx_status_fifo_head
  
  Could also be a security bug, as the user could allocate a buffer of
  size peeked data smaller than the actual packet received, which could
- cause a buffer overflow and its attaks.
+ cause a buffer overflow.
  
  Thanks,
  
  Alfred

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

Title:
  lan9118 bug peeking receive massage size not equal to received message
  size

Status in QEMU:
  New

Bug description:
  peeked message size is not equal to read message size

  Bug in the code at line:
  https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209

  s->tx_status_fifo_head should be s->rx_status_fifo_head

  Could also be a security bug, as the user could allocate a buffer of
  size peeked data smaller than the actual packet received, which could
  cause a buffer overflow.

  Thanks,

  Alfred

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

[PATCH v2 2/2] display/vmware_vga: Replace fprintf(stderr, "*\n") with error_report()

Replace all fprintf(stderr...) calls in hw/display/vmware_vga.c 
witherror_report().
Remove the "\n" from strings passed to all modified calls, since error_report() 
appends one.

Signed-off-by: Alex Chen 
---
 hw/display/vmware_vga.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index f93bbe15c2..a012e22e69 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -34,6 +34,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qom/object.h"
+#include "qemu/error-report.h"
 
 #undef VERBOSE
 #define HW_RECT_ACCEL
@@ -298,45 +299,45 @@ static inline bool vmsvga_verify_rect(DisplaySurface 
*surface,
   int x, int y, int w, int h)
 {
 if (x < 0) {
-fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
+error_report("%s: x was < 0 (%d)", name, x);
 return false;
 }
 if (x > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
+error_report("%s: x was > %d (%d)", name, SVGA_MAX_WIDTH, x);
 return false;
 }
 if (w < 0) {
-fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
+error_report("%s: w was < 0 (%d)", name, w);
 return false;
 }
 if (w > SVGA_MAX_WIDTH) {
-fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
+error_report("%s: w was > %d (%d)", name, SVGA_MAX_WIDTH, w);
 return false;
 }
 if (x + w > surface_width(surface)) {
-fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
+error_report("%s: width was > %d (x: %d, w: %d)",
 name, surface_width(surface), x, w);
 return false;
 }
 
 if (y < 0) {
-fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
+error_report("%s: y was < 0 (%d)", name, y);
 return false;
 }
 if (y > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
+error_report("%s: y was > %d (%d)", name, SVGA_MAX_HEIGHT, y);
 return false;
 }
 if (h < 0) {
-fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
+error_report("%s: h was < 0 (%d)", name, h);
 return false;
 }
 if (h > SVGA_MAX_HEIGHT) {
-fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
+error_report("%s: h was > %d (%d)", name, SVGA_MAX_HEIGHT, h);
 return false;
 }
 if (y + h > surface_height(surface)) {
-fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
+error_report("%s: update height > %d (y: %d, h: %d)",
 name, surface_height(surface), y, h);
 return false;
 }
@@ -534,7 +535,7 @@ static inline void vmsvga_cursor_define(struct 
vmsvga_state_s *s,
 #endif
 break;
 default:
-fprintf(stderr, "%s: unhandled bpp %u, using fallback cursor\n",
+error_report("%s: unhandled bpp %u, using fallback cursor",
 __func__, c->bpp);
 cursor_put(qc);
 qc = cursor_builtin_left_ptr();
-- 
2.19.1

[PATCH v2 1/2] display/vmware_vga: Fix bad printf format specifiers

We should use printf format specifier "%u" instead of "%d" for
argument of type "unsigned int".

Reported-by: Euler Robot 
Signed-off-by: Alex Chen 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/display/vmware_vga.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index bef0d7d69a..f93bbe15c2 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -534,7 +534,7 @@ static inline void vmsvga_cursor_define(struct 
vmsvga_state_s *s,
 #endif
 break;
 default:
-fprintf(stderr, "%s: unhandled bpp %d, using fallback cursor\n",
+fprintf(stderr, "%s: unhandled bpp %u, using fallback cursor\n",
 __func__, c->bpp);
 cursor_put(qc);
 qc = cursor_builtin_left_ptr();
-- 
2.19.1

[PATCH v2 0/2] Optimized some code for display/vmware_vga

Optimized some code for vmware_vga:
patch1 fixes a bad printf format specifier and
patch2 replaces fprintf(stderr, "*\n") with error_report()

Alex Chen (2):
  display/vmware_vga: Fix bad printf format specifiers
  display/vmware_vga: Replace fprintf(stderr, "*\n") with error_report()

 hw/display/vmware_vga.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

-- 
2.19.1

[Bug 1904954] [NEW] lan9118 bug peeking receive massage size not equal to received message size

2020-11-19 Thread alfred gedeon

Public bug reported:

peeked message size is not equal to read message size

Bug in the code at line:
https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209

s->tx_status_fifo_head should be s->rx_status_fifo_head

Thanks,

Alfred

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: ethernet lan lan9118 netwroking

** Description changed:

- peeked message is not equal to read message
- 
+ peeked message size is not equal to read message size
  
  Bug in the code at line:
  https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209
  
  s->tx_status_fifo_head should be s->rx_status_fifo_head
  
  Thanks,
  
  Alfred

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

Title:
  lan9118 bug peeking receive massage size not equal to received message
  size

Status in QEMU:
  New

Bug description:
  peeked message size is not equal to read message size

  Bug in the code at line:
  https://github.com/qemu/qemu/blob/master/hw/net/lan9118.c#L1209

  s->tx_status_fifo_head should be s->rx_status_fifo_head

  Thanks,

  Alfred

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

Re: [RFC PATCH 2/2] vfio-ccw: Connect the device request notifier

2020-11-19 Thread Halil Pasic

On Tue, 17 Nov 2020 04:26:05 +0100
Eric Farman  wrote:

> Now that the vfio-ccw code has a notifier interface to request that
> a device be unplugged, let's wire that together.

I'm aware of the fact that performing an unplug is what vfio-pci does,
but I was not aware of this before, and I became curious with regards
to how is this going to mesh with migration (in the future).

The sentence 'For this to work, QEMU has to be launched with the same
arguments the two times.' form docs/devel/migration.rst should not
be taken literally, I know, but the VM configuration changing not because
it was changed via a management interface, but because of events that
may not be under the control of whoever is managing the VM does
make thinks harder to reason about.

I suppose, we now basically mandate that whoever manages the VM, either
a) maintains his own model of the VM and listens to the events, to
update it if such a device removal happens, or
b) inspects the VM at some appropriate point in time, to figure out how
the target VM is supposed to be started.

I think libvirt does a).

I also suppose, such a device removal may not happen during device
migration. That is, form the QEMU perspective I  believe taken care
by the BQL.

But I'm in the dark regarding the management software/libivrt view. For
example what happens if we get a req_notification on the target while
pre-copy memory migration? At that point the destination is already
receiving pages, thus it is already constructed.

My questions are not necessarily addressed to you Eric. Maybe the
folks working on vfio migration can help us out. I'm also adding
our libvirt guys.

Regards,
Halil

[Bug 1904486] Re: resource leak in /net/tap.c

2020-11-19 Thread yuanjungong

Hi Alex,

Thanks for offering to help, but I submitted a patch to the maillist
yesterday. Thank you again.

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

Title:
  resource leak in /net/tap.c

Status in QEMU:
  New

Bug description:
  Hi,there might be a resource leak in function net_init_tap in
  /net/tap.c. The version is 5.1.91.

  
   811 fd = monitor_fd_param(monitor_cur(), tap->fd, errp);
   812 if (fd == -1) {
   813 return -1;
   814 }
   815
   816 ret = qemu_try_set_nonblock(fd);
   817 if (ret < 0) {
   818 error_setg_errno(errp, -ret, "%s: Can't use file descriptor 
%d",
   819  name, fd);
   820 return -1;
   821 }
   822
   823 vnet_hdr = tap_probe_vnet_hdr(fd, errp);
   824 if (vnet_hdr < 0) {
   825 close(fd);
   826 return -1;
   827 }
   828
   829 net_init_tap_one(tap, peer, "tap", name, NULL,
   830  script, downscript,
   831  vhostfdname, vnet_hdr, fd, );
   832 if (err) {
   833 error_propagate(errp, err);
   834 return -1;
   835 }

  fd should be closed before return in line 820 and line 834, similar to
  the implementation in line 825.

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

[Bug 1904486] Re: resource leak in /net/tap.c

2020-11-19 Thread yuanjungong

A patch has been submitted at https://lists.nongnu.org/archive/html
/qemu-trivial/2020-11/msg00355.html

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

Title:
  resource leak in /net/tap.c

Status in QEMU:
  New

Bug description:
  Hi,there might be a resource leak in function net_init_tap in
  /net/tap.c. The version is 5.1.91.

  
   811 fd = monitor_fd_param(monitor_cur(), tap->fd, errp);
   812 if (fd == -1) {
   813 return -1;
   814 }
   815
   816 ret = qemu_try_set_nonblock(fd);
   817 if (ret < 0) {
   818 error_setg_errno(errp, -ret, "%s: Can't use file descriptor 
%d",
   819  name, fd);
   820 return -1;
   821 }
   822
   823 vnet_hdr = tap_probe_vnet_hdr(fd, errp);
   824 if (vnet_hdr < 0) {
   825 close(fd);
   826 return -1;
   827 }
   828
   829 net_init_tap_one(tap, peer, "tap", name, NULL,
   830  script, downscript,
   831  vhostfdname, vnet_hdr, fd, );
   832 if (err) {
   833 error_propagate(errp, err);
   834 return -1;
   835 }

  fd should be closed before return in line 820 and line 834, similar to
  the implementation in line 825.

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

[Bug 1904486] Re: resource leak in /net/tap.c

Hi yuanjungong,

If you don't have time to submit a patch, can I submit a patch to fix
it?

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

Title:
  resource leak in /net/tap.c

Status in QEMU:
  New

Bug description:
  Hi,there might be a resource leak in function net_init_tap in
  /net/tap.c. The version is 5.1.91.

  
   811 fd = monitor_fd_param(monitor_cur(), tap->fd, errp);
   812 if (fd == -1) {
   813 return -1;
   814 }
   815
   816 ret = qemu_try_set_nonblock(fd);
   817 if (ret < 0) {
   818 error_setg_errno(errp, -ret, "%s: Can't use file descriptor 
%d",
   819  name, fd);
   820 return -1;
   821 }
   822
   823 vnet_hdr = tap_probe_vnet_hdr(fd, errp);
   824 if (vnet_hdr < 0) {
   825 close(fd);
   826 return -1;
   827 }
   828
   829 net_init_tap_one(tap, peer, "tap", name, NULL,
   830  script, downscript,
   831  vhostfdname, vnet_hdr, fd, );
   832 if (err) {
   833 error_propagate(errp, err);
   834 return -1;
   835 }

  fd should be closed before return in line 820 and line 834, similar to
  the implementation in line 825.

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

Re: [RFC 00/15] support subsets of bitmanip extension

2020-11-19 Thread Frank Chang

On Fri, Nov 20, 2020 at 6:26 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> > This patchset implements RISC-V B-extension latest draft version
> > (2020.10.26) Zbb, Zbs and Zba subset instructions.
>
> With some additional instructions from Zbp, it seems.  Although the
> document
> isn't completely coherent, with various instructions being present in
> multiple
> subsets, and some instructions w/ strike-out.
>
> The B extension requires more than these three, but I suppose turning it on
> with just these 3 subsets during development is ok.
>
>
> r~
>

Yes, some instructions are striked out and moved to another subset during
my implementation.
The B extension spec. is still changing occasionally.

I will send out the next patchset based on your comments.
Thanks for the reviews.

Frank Chang

Re: [PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps

2020-11-19 Thread Bruce Rogers

On Tue, 2020-10-20 at 09:48 +0200, Gerd Hoffmann wrote:
> USB_XHCI does not depend on PCI any more.
> USB_XHCI_SYSBUS must select USB_XHCI not USB.
> 
> Signed-off-by: Gerd Hoffmann 
> ---
>  hw/usb/Kconfig | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index 4dd2ba9630cb..a674ce4c542e 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -32,8 +32,6 @@ config USB_EHCI_SYSBUS
>  
>  config USB_XHCI
>  bool
> -default y if PCI_DEVICES
> -depends on PCI
>  select USB
>  
>  config USB_XHCI_PCI
> @@ -50,8 +48,8 @@ config USB_XHCI_NEC
>  
>  config USB_XHCI_SYSBUS
>  bool
> -default y if USB_XHCI
> -select USB
> +default y
> +select USB_XHCI
>  
>  config USB_MUSB
>  bool

I was reviewing what device changes are happening between v5.1.0 and
v5.2.0 for the QEMU arch's we support and noticed for s390x system
emulation that usb devices have appeared in the info qdm list in the
monitor.

I bisected the change to this patch, now commit 7114f6eac333d99b, which
does a 'default y' without any conditionals. I'm fairly sure that was
not the intent, though I do know what the proper conditionals should
be.

Can you take a look at it?

Thanks,

Bruce

Re: QMP and the 'id' parameter

2020-11-19 Thread John Snow


On 11/11/20 3:27 AM, Markus Armbruster wrote:

John Snow  writes:


On 11/10/20 1:22 AM, Markus Armbruster wrote:

John Snow  writes:


The QMP specification states:


NOTE: Some errors can occur before the Server is able to read the "id"
member, in these cases the "id" member will not be part of the error
response, even if provided by the client.


I am assuming this case ONLY occurs for Parse errors:

{'class': 'GenericError', 'desc': 'JSON parse error, expecting value'}


There are more "desc" possible, actually.

The JSON parser gets fed chunks of input, and calls a callback for every
full JSON value, and on parse error.

QMP's callback is handle_qmp_command().  Parameter @req is the parsed
JSON value, parameter @err is the (parse) error object, and exactly one
of them is non-null.

1. Parse error

If @err, we send an error response for it.  It never has "id".  See
qmp_error_response() caller monitor_qmp_dispatcher_co().  The possible
@err are:

  $ grep error_setg qobject/json-*[ch]
  qobject/json-parser.c:error_setg(>err, "JSON parse error, %s", 
message);

This is a syntax error.

Search for parse_error() to see the possible @message patterns.

  qobject/json-streamer.c:error_setg(, "JSON parse error, stray 
'%s'", input->str);

This is a lexical error.

  qobject/json-streamer.c:error_setg(, "JSON token size limit 
exceeded");
  qobject/json-streamer.c:error_setg(, "JSON token count limit 
exceeded");
  qobject/json-streamer.c:error_setg(, "JSON nesting depth limit 
exceeded");

These are (intentional) parser limits.

2. Successful parse

If @req, it's a successful parse.

If @req is not a JSON object, there is no "id".  qmp_dispatch() reports

  error_setg(, "QMP input must be a JSON object");

If @req is a JSON object, it has "id" exactly when the client supplied
one.  The response mirrors @req's "id".  See qmp_error_response() caller
qmp_dispatch().



That's very helpful, thank you. So in summary, the possibilities are:

1. syntax error (with several descriptions)
2. lexical error (with several descriptions, templated from one message)
3. lexical limitation (with several descriptions)


Nitpick: parser limitation.  Nesting depth is not lexical :)


4. grammatical error ("QMP input must be a JSON object")


We got two layers of syntax: QAPI schema above JSON.  The JSON layer
builds JSON from characters, the QAPI schema layer builds QAPI schema
from JSON.

Errors 1-3 are for the JSON layer.

Error 4 is a QAPI schema syntax error.  The qobject input visitor
reports more of them.  Error 4 is special only because it's the only one
where the JSON cannot have an ID.  All the other QAPI schema syntax
errors can have an ID.


(I know we have declared the error_class passe and we now prefer
"generic_error", but I lack the history lesson on why we do that.


The initial design had "rich" error objects, basically:

* An error QDict with a string "class" member and arbitrary data in a
   QDict "data" member

   To build a QMP error response, add the command's ID (if any) to the
   error QDict, and serialize to JSON.

* A pointer into a table of pairs (JSON template string, human-friendly
   template string).  More on the use of JSON template strings below.

   To build a human-friendly error message, interpolate the members of
   the data QDict into the human-friendly template.

Say you're writing a piece of code.  Among many other things, there are
a few errors to report.  You get to drop what you're doing, and do a
three step dance:

1. Define a QERR_YOUR_NEW_ERROR macro that expands into a JSON template
for the error QDict.  These are all in one place, which is included all
over the place.  Merge conflicts galore.

2. Add a table entry mapping QERR_YOUR_NEW_ERROR to the human-friendly
template.

3. Call the error function, passing QERR_YOUR_NEW_ERROR template and the
arguments.  The arguments must match the template.  This builds the
error QDict from template and the arguments, and looks up the template
in the table.

I strenuously objected to this.  In the end, all I could accomplish was
the inclusion of the human-friendly message in the QMP error response.

I (correctly) predicted two issues:

1. Error classes multiplied.  Not quite like rabbits, but bad enough to
be confusing.  Most of them were undocumented and of absolutely no use
to QMP clients.

2. The three step dance proved bothersome, and people frequently took
the obvious shortcut: instead of defining a new error that fits the
situation, reuse some existing error.  Even when it doesn't quite fit.
Error message quality went into the toilet.

After a few years of this, I went in with an axe, and nobody objected.

Among the stuff I axed were most error classes.  I spared only the ones
that were documented and plausibly useful to some QMP client.


Wouldn't the error_class here be helpful for interpreting the response?
It helps differentiate between 'The RPC call was received'

Re: [PATCH 6/6] configure / meson: Move check for linux/btrfs.h to meson.build

On 11/18/20 9:10 AM, Thomas Huth wrote:
> This check can be done in a much shorter way in meson.build. And while
> we're at it, rename the #define to HAVE_BTRFS_H to match the other
> HAVE_someheader_H symbols that we already have.
> 
> Signed-off-by: Thomas Huth 
> ---
>  configure | 9 -
>  linux-user/syscall.c  | 2 +-
>  linux-user/syscall_defs.h | 2 +-
>  meson.build   | 1 +
>  4 files changed, 3 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 5/6] configure / meson: Move check for sys/signal.h to meson.build

On 11/18/20 9:10 AM, Thomas Huth wrote:
> This check can be done in a much shorter way in meson.build. And while
> we're at it, rename the #define to HAVE_SYS_KCOV_H to match the other
> HAVE_someheader_H symbols that we already have.
> 
> Signed-off-by: Thomas Huth 
> ---
>  configure| 9 -
>  linux-user/ioctls.h  | 2 +-
>  linux-user/syscall.c | 2 +-
>  meson.build  | 1 +
>  4 files changed, 3 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 3/6] configure / meson: Move check for drm.h to meson.build

On 11/18/20 9:10 AM, Thomas Huth wrote:
> This check can be done in a much shorter way in meson.build
> 
> Signed-off-by: Thomas Huth 
> ---
>  configure   | 10 --
>  meson.build |  1 +
>  2 files changed, 1 insertion(+), 10 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 4/6] configure / meson: Move check for sys/signal.h to meson.build

On 11/18/20 9:10 AM, Thomas Huth wrote:
> This check can be done in a much shorter way in meson.build
> 
> Signed-off-by: Thomas Huth 
> ---
>  configure   | 10 --
>  meson.build |  1 +
>  2 files changed, 1 insertion(+), 10 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 2/6] configure / meson: Move check for pty.h to meson.build

On 11/18/20 9:10 AM, Thomas Huth wrote:
> This check can be done in a much shorter way in meson.build
> 
> Signed-off-by: Thomas Huth 
> ---
>  configure   | 9 -
>  meson.build | 1 +
>  2 files changed, 1 insertion(+), 9 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 1/6] configure: Remove the obsolete check for ifaddrs.h

On 11/18/20 9:10 AM, Thomas Huth wrote:
> The code that used HAVE_IFADDRS_H has been removed in commit
> 0a27af918b ("io: use bind() to check for IPv4/6 availability"),
> so we don't need this check in the configure script anymore.
> 
> Signed-off-by: Thomas Huth 
> ---
>  configure | 11 ---
>  1 file changed, 11 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 1/2] file-posix: Use OFD lock only if the filesystem supports the lock

2020-11-19 Thread Masayoshi Mizuma

On Thu, Nov 19, 2020 at 11:44:42AM +0100, Kevin Wolf wrote:
> Am 18.11.2020 um 20:48 hat Masayoshi Mizuma geschrieben:
> > On Wed, Nov 18, 2020 at 02:10:36PM -0500, Masayoshi Mizuma wrote:
> > > On Wed, Nov 18, 2020 at 04:42:47PM +0100, Kevin Wolf wrote:
> > > > Am 06.11.2020 um 05:01 hat Masayoshi Mizuma geschrieben:
> > > > > From: Masayoshi Mizuma 
> > > > > 
> > > > > locking=auto doesn't work if the filesystem doesn't support OFD lock.
> > > > > In that situation, following error happens:
> > > > > 
> > > > >   qemu-system-x86_64: -blockdev 
> > > > > driver=qcow2,node-name=disk,file.driver=file,file.filename=/mnt/guest.qcow2,file.locking=auto:
> > > > >  Failed to lock byte 100
> > > > > 
> > > > > qemu_probe_lock_ops() judges whether qemu can use OFD lock
> > > > > or not with doing fcntl(F_OFD_GETLK) to /dev/null. So the
> > > > > error happens if /dev/null supports OFD lock, but the filesystem
> > > > > doesn't support the lock.
> > > > > 
> > > > > Lock the actual file, not /dev/null, using F_OFD_SETLK and if that
> > > > > fails, then fallback to F_SETLK.
> > > > > 
> > > > > Signed-off-by: Masayoshi Mizuma 
> 
> > > > > -bool qemu_has_ofd_lock(void)
> > > > > -{
> > > > > -qemu_probe_lock_ops();
> > > > >  #ifdef F_OFD_SETLK
> > > > > -return fcntl_op_setlk == F_OFD_SETLK;
> > > > > +static int _qemu_lock_fcntl(int fd, struct flock *fl)
> > > > > +{
> > > > > +int ret;
> > > > > +bool ofd_lock = true;
> > > > > +
> > > > > +do {
> > > > > +if (ofd_lock) {
> > > > > +ret = fcntl(fd, F_OFD_SETLK, fl);
> > > > > +if ((ret == -1) && (errno == EINVAL)) {
> > > > > +ofd_lock = false;
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +if (!ofd_lock) {
> > > > > +/* Fallback to POSIX lock */
> > > > > +ret = fcntl(fd, F_SETLK, fl);
> > > > > +}
> > > > > +} while (ret == -1 && errno == EINTR);
> > > > > +
> > > > > +return ret == -1 ? -errno : 0;
> > > > > +}
> > > > >  #else
> > > > > -return false;
> > > > > -#endif
> > > > > +static int _qemu_lock_fcntl(int fd, struct flock *fl)
> > > > > +{
> > > > > +int ret;
> > > > > +
> > > > > +do {
> > > > > +ret = fcntl(fd, F_SETLK, fl);
> > > > > +} while (ret == -1 && errno == EINTR);
> > > > > +
> > > > > +return ret == -1 ? -errno : 0;
> > > > >  }
> > > > > +#endif
> > > > 
> > > > The logic looks fine to me, at least assuming that EINVAL is really what
> > > > we will consistently get from the kernel if OFD locks are not supported.
> > > > Is this documented anywhere? The fcntl manpage doesn't seem to mention
> > > > this case.
> > 
> > The man page of fcntl(2) says:
> > 
> >EINVAL The value specified in cmd is not recognized by this kernel.
> > 
> > So I think EINVAL is good enough to check whether the filesystem supports
> > OFD locks or not...
> 
> A kernel not knowing the cmd at all is a somewhat different case (and
> certainly a different code path) than a filesystem not supporting it.
> 
> I just had a look at the kernel code, and to me it seems that the
> difference between POSIX locks and OFD locks is handled entirely in
> filesystem independent code. A filesystem driver would in theory have
> ways to distinguish both, but I don't see any driver in the kernel tree
> that actually does this (and there is probably little reason for a
> driver to do so).
> 
> So now I wonder what filesystem you are using? I'm curious what I
> missed.

I'm using a proprietary filesystem, which isn't in the linux kernel.
The filesystem supports posix lock only, doesn't support OFD lock...

Thanks,
Masa

Re: [PATCH 4/4] RFC qemu-binfmt-conf.sh: Add MIPS64 o32 ABI

On 11/19/20 8:17 AM, Philippe Mathieu-Daudé wrote:
> ... but this is wrong as the same header matches MIPS32 o32 ELFs...
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---

Yeah, I don't think you'll be able to include this.


r~

Re: [PATCH 2/4] linux-user/mips64: Support o32 ABI syscalls

On 11/19/20 3:08 PM, Richard Henderson wrote:
> On 11/19/20 8:17 AM, Philippe Mathieu-Daudé wrote:
>> +#if defined(TARGET_ABI_MIPSO32)
>> +#define TARGET_SYSCALL_OFFSET 4000
>> +#include "syscall_o32_nr.h"
> 
> Where does this get built?

Ah, I see, next patch.

Reviewed-by: Richard Henderson 

r~

Re: [PATCH 2/4] linux-user/mips64: Support o32 ABI syscalls

On 11/19/20 8:17 AM, Philippe Mathieu-Daudé wrote:
> +#if defined(TARGET_ABI_MIPSO32)
> +#define TARGET_SYSCALL_OFFSET 4000
> +#include "syscall_o32_nr.h"

Where does this get built?


r~

Re: [PATCH 1/4] linux-user/mips64: Restore setup_frame() for o32 ABI

On 11/19/20 8:17 AM, Philippe Mathieu-Daudé wrote:
> 64-bit MIPS targets lost setup_frame() during the refactor in commit
> 8949bef18b9. Restore it declaring TARGET_ARCH_HAS_SETUP_FRAME, to be
> able to build the o32 ABI target.
> 
> Fixes: 8949bef18b9 ("linux-user: move mips/mips64 signal.c parts to mips 
> directory")
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  linux-user/mips64/target_signal.h | 4 
>  1 file changed, 4 insertions(+)

Reviewed-by: Richard Henderson 

r~

Re: [RFC PATCH v5 20/33] Hexagon (target/hexagon) generator phase 2 - generate header files

2020-11-19 Thread Alessandro Di Federico via

On Thu, 29 Oct 2020 19:08:26 -0500
Taylor Simpson  wrote:

> +from hex_common import *

I'd suggest to avoid `import *`.

See:

python -c 'import this' | sed -n '4p'

-- 
Alessandro Di Federico
rev.ng

Re: [RFC PATCH v5 22/33] Hexagon (target/hexagon) generater phase 4 - decode tree

2020-11-19 Thread Alessandro Di Federico via

On Thu, 29 Oct 2020 19:08:28 -0500
Taylor Simpson  wrote:

> +if __name__ == '__main__':
> +f = io.StringIO()
> +print_tree(f, dectree_normal)
> +print_tree(f, dectree_16bit)
> +if subinsn_groupings:
> +print_tree(f, dectree_subinsn_groupings)
> +for (name, dectree_subinsn) in sorted(dectree_subinsns.items()):
> +print_tree(f, dectree_subinsn)
> +for (name, dectree_ext) in sorted(dectree_extensions.items()):
> +print_tree(f, dectree_ext)
> +print_match_info(f)
> +print_op_info(f)
> +open(sys.argv[1], 'w').write(f.getvalue())

Is there any specific reason why (here and elsewhere) you use
`StringIO` instead of writing to the file directly?

I'd expect something like:

```
if __name__ == '__main__':
with open(sys.argv[1], 'w') as f:
print_tree(f, dectree_normal)
print_tree(f, dectree_16bit)
if subinsn_groupings:
print_tree(f, dectree_subinsn_groupings)
for (name, dectree_subinsn) in sorted(dectree_subinsns.items()):
print_tree(f, dectree_subinsn)
for (name, dectree_ext) in sorted(dectree_extensions.items()):
print_tree(f, dectree_ext)
print_match_info(f)
print_op_info(f)
```

Maybe you're trying to avoid leaving a corrupted file in case of error,
but I guess that's more of a concern for the build system.

Elsewhere, you invoke `.close()`. I'd suggest to use a `with`-statement
there too.

-- 
Alessandro Di Federico
rev.ng

Re: [PATCH v2] target/mips/helper: Also display exception names in user-mode

On 11/19/20 8:05 AM, Philippe Mathieu-Daudé wrote:
> Currently MIPS exceptions are displayed as string in system-mode
> emulation, but as number in user-mode.
> Unify by extracting the current system-mode code as excp_name()
> and use that in user-mode.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Since v1: Fixed failed cherry-pick conflict resolution
> ---
>  target/mips/helper.c | 24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-5.2?] target/arm: fix stage 2 page-walks in 32-bit emulation

On 11/18/20 7:04 AM, Rémi Denis-Courmont wrote:
> From: Rémi Denis-Courmont 
> 
> Using a target unsigned long would limit the Input Address to a LPAE
> page-walk to 32 bits on AArch32 and 64 bits on AArch64. This is okay
> for stage 1 or on AArch64, but it is insufficient for stage 2 on
> AArch32. In that later case, the Input Address can have up to 40 bits.
> 
> Signed-off-by: Rémi Denis-Courmont 
> ---
>  target/arm/helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 

Peter, bug fix for 5.2 or postpone?


r~

python asyncio QMP library (AQMP)

2020-11-19 Thread John Snow

I've been doodling a new Asyncio-based QMP library that might fix some 
of the problems[1][2] with our older, non-async QMP library and provide 
a better basis for a proper distributable python package for people to 
write their own toy scripts to control QEMU.


It's a very early prototype, but I think I have the basics down and 
working now. I figured I would float my extremely early copy of it in 
case interested parties wanted to take a peek. (Or just to formally 
announce that "Hey, I'm working on this!")


https://gitlab.com/jsnow/aqmp/-/tree/devel

The design uses two independent coroutines in the bottom half, a reader 
and a writer. execute() works something like this:


   +-+
 + |execute()| <--+
 | +-+|
 ||
---
 v|
++++---+   +--+---+
|Mailboxes||Event Queue|   |Outbound Queue|
++++--++   +--+---+
 ||   ^
 vv   |
  +--++---+   +---+---+
  | Reader Task/Coroutine |   | Writer Task/Coroutine |
  +---+---+   +---+---+
  |   ^
  v   |
+-+--+  +-+--+
|StreamReader|  |StreamWriter|
++  ++


Execute deposits a message in the outbound queue; the writer issues it 
to the server. Execute goes to sleep waiting for mail in its inbox. The 
reader awakens upon new data available in the stream, the new data is 
placed in the mailbox, which awakens the caller.


The Reader/Writer are each executed by _task_runner, which ensures that 
any error that occurs in the BH will create a disconnection Task that 
will quiesce everything in the bottom half.


Any pending execute actions will be cancelled by the disconnection task; 
and will raise "DisconnectedError" to the caller.


Upon learning of a disconnection event, the client code responsible for 
managing the overall state of the connection can call disconnect(), 
which will raise any exceptions that occurred in the bottom halves that 
caused the unscheduled disconnect. From there, the client can decide to 
reconnect or abort, depending.



classes and things of interest:

aqmp.py - Mailbox: Simple data structure pairing an asyncio.Event with a 
Message. This is done to allow the disconnect task to wake up any 
waiting clients even if no message arrived. Python's asyncio.Queue does 
not offer a cancel primitive, so this approximates it.


aqmp.py - AQMP: The core of the loop logic is written here; the 
public-facing methods are connect(), disconnect(), execute() and 
execute_obj(), with a "make_execute_msg" class method available for 
creating Messages that can be executed later.


message.py - Message: This class comprises a JSON Object as read off the 
wire; putting it in a class like this allows me to write stricter type 
guarantees and offer _serialize/_deserialize class methods.


models.py - This module offers strictly typed and validated 
deserializations of core QMP structures that are not (generally) defined 
by the QAPI schema; i.e., it offers types, de/serialization, and 
validation for things such as "Greeting", "SuccessResponse", 
"AsynchronousEvent", etc.



What's left to do?

- Work on Exception naming and hierarchy,
  more helpful/pretty display of errors.
- Expand the event handling system to be more useful (Maybe?)
- Add QGA support
- Re-add socket creation support (Presently, it relies on the socket 
being created already.)

- Documentation, tests, etc.


Might be nice:

- Look into creating a "Server" variant to be used for testing the 
library, or even other clients.
- Look into synchronous wrappers that offer thread-safe primitives that 
can be used from traditional, synchronous contexts. (preventing "async 
creep".)



Long term goal:

- Get it merged in-tree; replace the legacy QMP module over time.

- Create an auto-generated SDK layer based on our QAPI schema that adds 
a library of type-safe commands, responses, and events that uses this 
QAPI-agnostic QMP library as its core.


- Add my "JobRunner" patches from earlier this year as a full-fledged 
jobs API that is designed to make running long-running tasks very 
trivial from a python console / qmp-shell.



--js


[1] qmp-shell is not capable of displaying events as they arrive 
asynchronously very well.
[2] Our use of non-blocking sockets and socket timeouts in qmp.py relies 
quite a bit on undefined behavior that thankfully has not broken yet.

Re: [RFC 00/15] support subsets of bitmanip extension

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> This patchset implements RISC-V B-extension latest draft version
> (2020.10.26) Zbb, Zbs and Zba subset instructions.

With some additional instructions from Zbp, it seems.  Although the document
isn't completely coherent, with various instructions being present in multiple
subsets, and some instructions w/ strike-out.

The B extension requires more than these three, but I suppose turning it on
with just these 3 subsets during development is ok.

r~

Re: [PATCH v2 27/28] hw/arm/armv7m: Correct typo in QOM object name

2020-11-19 Thread Philippe Mathieu-Daudé

On 11/19/20 10:56 PM, Peter Maydell wrote:
> Correct a typo in the name we give the NVIC object.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm/armv7m.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé

[PATCH v2 25/28] target/arm: Implement M-profile "minimal RAS implementation"

For v8.1M the architecture mandates that CPUs must provide at
least the "minimal RAS implementation" from the Reliability,
Availability and Serviceability extension. This consists of:
 * an ESB instruction which is a NOP
   -- since it is in the HINT space we need only add a comment
 * an RFSR register which will RAZ/WI
 * a RAZ/WI AIRCR.IESB bit
   -- the code which handles writes to AIRCR does not allow setting
  of RES0 bits, so we already treat this as RAZ/WI; add a comment
  noting that this is deliberate
 * minimal implementation of the RAS register block at 0xe0005000
   -- this will be in a subsequent commit
 * setting the ID_PFR0.RAS field to 0b0010
   -- we will do this when we add the Cortex-M55 CPU model

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h  | 14 ++
 target/arm/t32.decode |  4 
 hw/intc/armv7m_nvic.c | 13 +
 3 files changed, 31 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 22c55c81933..7e6c881a7e2 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1827,6 +1827,15 @@ FIELD(ID_MMFR4, LSM, 20, 4)
 FIELD(ID_MMFR4, CCIDX, 24, 4)
 FIELD(ID_MMFR4, EVT, 28, 4)
 
+FIELD(ID_PFR0, STATE0, 0, 4)
+FIELD(ID_PFR0, STATE1, 4, 4)
+FIELD(ID_PFR0, STATE2, 8, 4)
+FIELD(ID_PFR0, STATE3, 12, 4)
+FIELD(ID_PFR0, CSV2, 16, 4)
+FIELD(ID_PFR0, AMU, 20, 4)
+FIELD(ID_PFR0, DIT, 24, 4)
+FIELD(ID_PFR0, RAS, 28, 4)
+
 FIELD(ID_PFR1, PROGMOD, 0, 4)
 FIELD(ID_PFR1, SECURITY, 4, 4)
 FIELD(ID_PFR1, MPROGMOD, 8, 4)
@@ -3573,6 +3582,11 @@ static inline bool isar_feature_aa32_predinv(const 
ARMISARegisters *id)
 return FIELD_EX32(id->id_isar6, ID_ISAR6, SPECRES) != 0;
 }
 
+static inline bool isar_feature_aa32_ras(const ARMISARegisters *id)
+{
+return FIELD_EX32(id->id_pfr0, ID_PFR0, RAS) != 0;
+}
+
 static inline bool isar_feature_aa32_mprofile(const ARMISARegisters *id)
 {
 return FIELD_EX32(id->id_pfr1, ID_PFR1, MPROGMOD) != 0;
diff --git a/target/arm/t32.decode b/target/arm/t32.decode
index f045eb62c84..8b2c487fa7a 100644
--- a/target/arm/t32.decode
+++ b/target/arm/t32.decode
@@ -307,6 +307,10 @@ CLZ   1010 1011    1000    
   @rdm
   # SEV   0011 1010  1000   0100
   # SEVL  0011 1010  1000   0101
 
+  # For M-profile minimal-RAS ESB can be a NOP, which is the
+  # default behaviour since it is in the hint space.
+  # ESB   0011 1010  1000  0001 
+
   # The canonical nop ends in  , but the whole rest
   # of the space is "reserved hint, behaves as nop".
   NOP 0011 1010  1000   
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index fa8aeca82ec..c42b291f881 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1489,6 +1489,12 @@ static uint32_t nvic_readl(NVICState *s, uint32_t 
offset, MemTxAttrs attrs)
 return 0;
 }
 return cpu->env.v7m.sfar;
+case 0xf04: /* RFSR */
+if (!cpu_isar_feature(aa32_ras, cpu)) {
+goto bad_offset;
+}
+/* We provide minimal-RAS only: RFSR is RAZ/WI */
+return 0;
 case 0xf34: /* FPCCR */
 if (!cpu_isar_feature(aa32_vfp_simd, cpu)) {
 return 0;
@@ -1617,6 +1623,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
   R_V7M_AIRCR_PRIGROUP_SHIFT,
   R_V7M_AIRCR_PRIGROUP_LENGTH);
 }
+/* AIRCR.IESB is RAZ/WI because we implement only minimal RAS */
 if (attrs.secure) {
 /* These bits are only writable by secure */
 cpu->env.v7m.aircr = value &
@@ -2037,6 +2044,12 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 }
 break;
 }
+case 0xf04: /* RFSR */
+if (!cpu_isar_feature(aa32_ras, cpu)) {
+goto bad_offset;
+}
+/* We provide minimal-RAS only: RFSR is RAZ/WI */
+break;
 case 0xf34: /* FPCCR */
 if (cpu_isar_feature(aa32_vfp_simd, cpu)) {
 /* Not all bits here are banked. */
-- 
2.20.1

[PATCH v2 23/28] target/arm: Implement CCR_S.TRD behaviour for SG insns

v8.1M introduces a new TRD flag in the CCR register, which enables
checking for stack frame integrity signatures on SG instructions.
Add the code in the SG insn implementation for the new behaviour.

Signed-off-by: Peter Maydell 
---
 target/arm/m_helper.c | 86 +++
 1 file changed, 86 insertions(+)

diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 0bdd3cc10e9..643dcafb83d 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -1999,6 +1999,64 @@ static bool v7m_read_half_insn(ARMCPU *cpu, ARMMMUIdx 
mmu_idx,
 return true;
 }
 
+static bool v7m_read_sg_stack_word(ARMCPU *cpu, ARMMMUIdx mmu_idx,
+   uint32_t addr, uint32_t *spdata)
+{
+/*
+ * Read a word of data from the stack for the SG instruction,
+ * writing the value into *spdata. If the load succeeds, return
+ * true; otherwise pend an appropriate exception and return false.
+ * (We can't use data load helpers here that throw an exception
+ * because of the context we're called in, which is halfway through
+ * arm_v7m_cpu_do_interrupt().)
+ */
+CPUState *cs = CPU(cpu);
+CPUARMState *env = >env;
+MemTxAttrs attrs = {};
+MemTxResult txres;
+target_ulong page_size;
+hwaddr physaddr;
+int prot;
+ARMMMUFaultInfo fi = {};
+ARMCacheAttrs cacheattrs = {};
+uint32_t value;
+
+if (get_phys_addr(env, addr, MMU_DATA_LOAD, mmu_idx, ,
+  , , _size, , )) {
+/* MPU/SAU lookup failed */
+if (fi.type == ARMFault_QEMU_SFault) {
+qemu_log_mask(CPU_LOG_INT,
+  "...SecureFault during stack word read\n");
+env->v7m.sfsr |= R_V7M_SFSR_AUVIOL_MASK | 
R_V7M_SFSR_SFARVALID_MASK;
+env->v7m.sfar = addr;
+armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_SECURE, false);
+} else {
+qemu_log_mask(CPU_LOG_INT,
+  "...MemManageFault during stack word read\n");
+env->v7m.cfsr[M_REG_S] |= R_V7M_CFSR_DACCVIOL_MASK |
+R_V7M_CFSR_MMARVALID_MASK;
+env->v7m.mmfar[M_REG_S] = addr;
+armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_MEM, false);
+}
+return false;
+}
+value = address_space_ldl(arm_addressspace(cs, attrs), physaddr,
+  attrs, );
+if (txres != MEMTX_OK) {
+/* BusFault trying to read the data */
+qemu_log_mask(CPU_LOG_INT,
+  "...BusFault during stack word read\n");
+env->v7m.cfsr[M_REG_NS] |=
+(R_V7M_CFSR_PRECISERR_MASK | R_V7M_CFSR_BFARVALID_MASK);
+env->v7m.bfar = addr;
+armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_BUS, false);
+return false;
+}
+
+*spdata = value;
+return true;
+}
+
 static bool v7m_handle_execute_nsc(ARMCPU *cpu)
 {
 /*
@@ -2055,6 +2113,34 @@ static bool v7m_handle_execute_nsc(ARMCPU *cpu)
  */
 qemu_log_mask(CPU_LOG_INT, "...really an SG instruction at 0x%08" PRIx32
   ", executing it\n", env->regs[15]);
+
+if (cpu_isar_feature(aa32_m_sec_state, cpu) &&
+!arm_v7m_is_handler_mode(env)) {
+/*
+ * v8.1M exception stack frame integrity check. Note that we
+ * must perform the memory access even if CCR_S.TRD is zero
+ * and we aren't going to check what the data loaded is.
+ */
+uint32_t spdata, sp;
+
+/*
+ * We know we are currently NS, so the S stack pointers must be
+ * in other_ss_{psp,msp}, not in regs[13]/other_sp.
+ */
+sp = v7m_using_psp(env) ? env->v7m.other_ss_psp : 
env->v7m.other_ss_msp;
+if (!v7m_read_sg_stack_word(cpu, mmu_idx, sp, )) {
+/* Stack access failed and an exception has been pended */
+return false;
+}
+
+if (env->v7m.ccr[M_REG_S] & R_V7M_CCR_TRD_MASK) {
+if (((spdata & ~1) == 0xfefa125a) ||
+!(env->v7m.control[M_REG_S] & 1)) {
+goto gen_invep;
+}
+}
+}
+
 env->regs[14] &= ~1;
 env->v7m.control[M_REG_S] &= ~R_V7M_CONTROL_SFPA_MASK;
 switch_v7m_security_state(env, true);
-- 
2.20.1

Re: [RFC 14/15] target/riscv: rvb: add/sub with postfix zero-extend

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> +addwu  101 .. 000 . 0111011 @r
> +subwu  0100101 .. 000 . 0111011 @r
> +addu_w 100 .. 000 . 0111011 @r
>  
>  sbsetiw0010100 .. 001 . 0011011 @sh5
>  sbclriw0100100 .. 001 . 0011011 @sh5
> @@ -116,3 +119,7 @@ sroiw  001 .. 101 . 0011011 @sh5
>  roriw  011 .. 101 . 0011011 @sh5
>  greviw 0110100 .. 101 . 0011011 @sh5
>  gorciw 0010100 .. 101 . 0011011 @sh5
> +
> +addiwu . 100 . 0011011 @i
> +
> +slliu_w10 ... 001 . 0011011 @sh


addwu, subwu, addiwu have been removed in the current draft.

> +static bool trans_slliu_w(DisasContext *ctx, arg_slliu_w *a)
> +{
> +TCGv source1;
> +source1 = tcg_temp_new();
> +gen_get_gpr(source1, a->rs1);
> +
> +tcg_gen_ext32u_tl(source1, source1);
> +tcg_gen_shli_tl(source1, source1, a->shamt);
> +gen_set_gpr(a->rd, source1);

if (a->shamt < 32) {
tcg_gen_deposit_z_i64(source1, source1, a->shamt, 32);
} else {
tcg_gen_shli_i64(source1, source1, a->shamt);
}


r~

[PATCH v2 20/28] target/arm: Implement new v8.1M VLLDM and VLSTM encodings

v8.1M adds new encodings of VLLDM and VLSTM (where bit 7 is set).
The only difference is that:
 * the old T1 encodings UNDEF if the implementation implements 32
   Dregs (this is currently architecturally impossible for M-profile)
 * the new T2 encodings have the implementation-defined option to
   read from memory (discarding the data) or write UNKNOWN values to
   memory for the stack slots that would be D16-D31

We choose not to make those accesses, so for us the two
instructions behave identically assuming they don't UNDEF.

Signed-off-by: Peter Maydell 
---
 target/arm/m-nocp.decode   |  2 +-
 target/arm/translate-vfp.c.inc | 25 +
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/target/arm/m-nocp.decode b/target/arm/m-nocp.decode
index ccd62e8739a..6699626d7cb 100644
--- a/target/arm/m-nocp.decode
+++ b/target/arm/m-nocp.decode
@@ -36,7 +36,7 @@
 
 {
   # Special cases which do not take an early NOCP: VLLDM and VLSTM
-  VLLDM_VLSTM  1110 1100 001 l:1 rn:4  1010  
+  VLLDM_VLSTM  1110 1100 001 l:1 rn:4  1010 op:1 000 
   # VSCCLRM (new in v8.1M) is similar:
   VSCCLRM  1110 1100 1.01   1011 imm:7 0   vd=%vd_dp size=3
   VSCCLRM  1110 1100 1.01   1010 imm:8 vd=%vd_sp size=2
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 1c2d31f6f30..c974d5b0e16 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -3814,6 +3814,31 @@ static bool trans_VLLDM_VLSTM(DisasContext *s, 
arg_VLLDM_VLSTM *a)
 !arm_dc_feature(s, ARM_FEATURE_V8)) {
 return false;
 }
+
+if (a->op) {
+/*
+ * T2 encoding ({D0-D31} reglist): v8.1M and up. We choose not
+ * to take the IMPDEF option to make memory accesses to the stack
+ * slots that correspond to the D16-D31 registers (discarding
+ * read data and writing UNKNOWN values), so for us the T2
+ * encoding behaves identically to the T1 encoding.
+ */
+if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
+return false;
+}
+} else {
+/*
+ * T1 encoding ({D0-D15} reglist); undef if we have 32 Dregs.
+ * This is currently architecturally impossible, but we add the
+ * check to stay in line with the pseudocode. Note that we must
+ * emit code for the UNDEF so it takes precedence over the NOCP.
+ */
+if (dc_isar_feature(aa32_simd_r32, s)) {
+unallocated_encoding(s);
+return true;
+}
+}
+
 /*
  * If not secure, UNDEF. We must emit code for this
  * rather than returning false so that this takes
-- 
2.20.1

[PATCH v2 22/28] hw/intc/armv7m_nvic: Support v8.1M CCR.TRD bit

v8.1M introduces a new TRD flag in the CCR register, which enables
checking for stack frame integrity signatures on SG instructions.
This bit is not banked, and is always RAZ/WI to Non-secure code.
Adjust the code for handling CCR reads and writes to handle this.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h  |  2 ++
 hw/intc/armv7m_nvic.c | 26 ++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 47cb5032ce9..22c55c81933 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1611,6 +1611,8 @@ FIELD(V7M_CCR, STKOFHFNMIGN, 10, 1)
 FIELD(V7M_CCR, DC, 16, 1)
 FIELD(V7M_CCR, IC, 17, 1)
 FIELD(V7M_CCR, BP, 18, 1)
+FIELD(V7M_CCR, LOB, 19, 1)
+FIELD(V7M_CCR, TRD, 20, 1)
 
 /* V7M SCR bits */
 FIELD(V7M_SCR, SLEEPONEXIT, 1, 1)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index deb4bd56c95..c901d20ae00 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1095,8 +1095,9 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
 }
 return cpu->env.v7m.scr[attrs.secure];
 case 0xd14: /* Configuration Control.  */
-/* The BFHFNMIGN bit is the only non-banked bit; we
- * keep it in the non-secure copy of the register.
+/*
+ * Non-banked bits: BFHFNMIGN (stored in the NS copy of the register)
+ * and TRD (stored in the S copy of the register)
  */
 val = cpu->env.v7m.ccr[attrs.secure];
 val |= cpu->env.v7m.ccr[M_REG_NS] & R_V7M_CCR_BFHFNMIGN_MASK;
@@ -1645,17 +1646,25 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 cpu->env.v7m.scr[attrs.secure] = value;
 break;
 case 0xd14: /* Configuration Control.  */
+{
+uint32_t mask;
+
 if (!arm_feature(>env, ARM_FEATURE_M_MAIN)) {
 goto bad_offset;
 }
 
 /* Enforce RAZ/WI on reserved and must-RAZ/WI bits */
-value &= (R_V7M_CCR_STKALIGN_MASK |
-  R_V7M_CCR_BFHFNMIGN_MASK |
-  R_V7M_CCR_DIV_0_TRP_MASK |
-  R_V7M_CCR_UNALIGN_TRP_MASK |
-  R_V7M_CCR_USERSETMPEND_MASK |
-  R_V7M_CCR_NONBASETHRDENA_MASK);
+mask = R_V7M_CCR_STKALIGN_MASK |
+R_V7M_CCR_BFHFNMIGN_MASK |
+R_V7M_CCR_DIV_0_TRP_MASK |
+R_V7M_CCR_UNALIGN_TRP_MASK |
+R_V7M_CCR_USERSETMPEND_MASK |
+R_V7M_CCR_NONBASETHRDENA_MASK;
+if (arm_feature(>env, ARM_FEATURE_V8_1M) && attrs.secure) {
+/* TRD is always RAZ/WI from NS */
+mask |= R_V7M_CCR_TRD_MASK;
+}
+value &= mask;
 
 if (arm_feature(>env, ARM_FEATURE_V8)) {
 /* v8M makes NONBASETHRDENA and STKALIGN be RES1 */
@@ -1677,6 +1686,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 
 cpu->env.v7m.ccr[attrs.secure] = value;
 break;
+}
 case 0xd24: /* System Handler Control and State (SHCSR) */
 if (!arm_feature(>env, ARM_FEATURE_V7)) {
 goto bad_offset;
-- 
2.20.1

Re: [PATCH v2 12/28] target/arm: Factor out preserve-fp-state from full_vfp_access_check()

2020-11-19 Thread Philippe Mathieu-Daudé

On 11/19/20 10:56 PM, Peter Maydell wrote:
> Factor out the code which handles M-profile lazy FP state preservation
> from full_vfp_access_check(); accesses to the FPCXT_NS register are
> a special case which need to do just this part (corresponding in the
> pseudocode to the PreserveFPState() function), and not the full
> set of actions matching the pseudocode ExecuteFPCheck() which
> normal FP instructions need to do.
> 
> Signed-off-by: Peter Maydell 
> Reviewed-by: Richard Henderson 
> ---
>  target/arm/translate-vfp.c.inc | 45 --
>  1 file changed, 27 insertions(+), 18 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé

[PATCH v2 21/28] hw/intc/armv7m_nvic: Correct handling of CCR.BFHFNMIGN

The CCR is a register most of whose bits are banked between security
states but where BFHFNMIGN is not, and we keep it in the non-secure
entry of the v7m.ccr[] array.  The logic which tries to handle this
bit fails to implement the "RAZ/WI from Nonsecure if AIRCR.BFHFNMINS
is zero" requirement; correct the omission.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index effc4a784ca..deb4bd56c95 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1100,6 +1100,12 @@ static uint32_t nvic_readl(NVICState *s, uint32_t 
offset, MemTxAttrs attrs)
  */
 val = cpu->env.v7m.ccr[attrs.secure];
 val |= cpu->env.v7m.ccr[M_REG_NS] & R_V7M_CCR_BFHFNMIGN_MASK;
+/* BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0 */
+if (!attrs.secure) {
+if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) {
+val &= ~R_V7M_CCR_BFHFNMIGN_MASK;
+}
+}
 return val;
 case 0xd24: /* System Handler Control and State (SHCSR) */
 if (!arm_feature(>env, ARM_FEATURE_V7)) {
@@ -1662,6 +1668,11 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 (cpu->env.v7m.ccr[M_REG_NS] & ~R_V7M_CCR_BFHFNMIGN_MASK)
 | (value & R_V7M_CCR_BFHFNMIGN_MASK);
 value &= ~R_V7M_CCR_BFHFNMIGN_MASK;
+} else {
+/* BFHFNMIGN is RAZ/WI from NS if AIRCR.BFHFNMINS is 0 */
+if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) {
+value &= ~R_V7M_CCR_BFHFNMIGN_MASK;
+}
 }
 
 cpu->env.v7m.ccr[attrs.secure] = value;
-- 
2.20.1

[PATCH v2 24/28] hw/intc/armv7m_nvic: Fix "return from inactive handler" check

In commit 077d7449100d824a4 we added code to handle the v8M
requirement that returns from NMI or HardFault forcibly deactivate
those exceptions regardless of what interrupt the guest is trying to
deactivate.  Unfortunately this broke the handling of the "illegal
exception return because the returning exception number is not
active" check for those cases.  In the pseudocode this test is done
on the exception the guest asks to return from, but because our
implementation was doing this in armv7m_nvic_complete_irq() after the
new "deactivate NMI/HardFault regardless" code we ended up doing the
test on the VecInfo for that exception instead, which usually meant
failing to raise the illegal exception return fault.

In the case for "configurable exception targeting the opposite
security state" we detected the illegal-return case but went ahead
and deactivated the VecInfo anyway, which is wrong because that is
the VecInfo for the other security state.

Rearrange the code so that we first identify the illegal return
cases, then see if we really need to deactivate NMI or HardFault
instead, and finally do the deactivation.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c | 59 +++
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index c901d20ae00..fa8aeca82ec 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -832,10 +832,40 @@ int armv7m_nvic_complete_irq(void *opaque, int irq, bool 
secure)
 {
 NVICState *s = (NVICState *)opaque;
 VecInfo *vec = NULL;
-int ret;
+int ret = 0;
 
 assert(irq > ARMV7M_EXCP_RESET && irq < s->num_irq);
 
+trace_nvic_complete_irq(irq, secure);
+
+if (secure && exc_is_banked(irq)) {
+vec = >sec_vectors[irq];
+} else {
+vec = >vectors[irq];
+}
+
+/*
+ * Identify illegal exception return cases. We can't immediately
+ * return at this point because we still need to deactivate
+ * (either this exception or NMI/HardFault) first.
+ */
+if (!exc_is_banked(irq) && exc_targets_secure(s, irq) != secure) {
+/*
+ * Return from a configurable exception targeting the opposite
+ * security state from the one we're trying to complete it for.
+ * Clear vec because it's not really the VecInfo for this
+ * (irq, secstate) so we mustn't deactivate it.
+ */
+ret = -1;
+vec = NULL;
+} else if (!vec->active) {
+/* Return from an inactive interrupt */
+ret = -1;
+} else {
+/* Legal return, we will return the RETTOBASE bit value to the caller 
*/
+ret = nvic_rettobase(s);
+}
+
 /*
  * For negative priorities, v8M will forcibly deactivate the appropriate
  * NMI or HardFault regardless of what interrupt we're being asked to
@@ -865,32 +895,7 @@ int armv7m_nvic_complete_irq(void *opaque, int irq, bool 
secure)
 }
 
 if (!vec) {
-if (secure && exc_is_banked(irq)) {
-vec = >sec_vectors[irq];
-} else {
-vec = >vectors[irq];
-}
-}
-
-trace_nvic_complete_irq(irq, secure);
-
-if (!vec->active) {
-/* Tell the caller this was an illegal exception return */
-return -1;
-}
-
-/*
- * If this is a configurable exception and it is currently
- * targeting the opposite security state from the one we're trying
- * to complete it for, this counts as an illegal exception return.
- * We still need to deactivate whatever vector the logic above has
- * selected, though, as it might not be the same as the one for the
- * requested exception number.
- */
-if (!exc_is_banked(irq) && exc_targets_secure(s, irq) != secure) {
-ret = -1;
-} else {
-ret = nvic_rettobase(s);
+return ret;
 }
 
 vec->active = 0;
-- 
2.20.1

[PATCH v2 18/28] target/arm: Implement v8.1M REVIDR register

In v8.1M a REVIDR register is defined, which is at address 0xe00ecfc
and is a read-only IMPDEF register providing implementation specific
minor revision information, like the v8A REVIDR_EL1. Implement this.

Signed-off-by: Peter Maydell 
---
 hw/intc/armv7m_nvic.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index be3bc1f1f45..effc4a784ca 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1025,6 +1025,11 @@ static uint32_t nvic_readl(NVICState *s, uint32_t 
offset, MemTxAttrs attrs)
 }
 return val;
 }
+case 0xcfc:
+if (!arm_feature(>env, ARM_FEATURE_V8_1M)) {
+goto bad_offset;
+}
+return cpu->revidr;
 case 0xd00: /* CPUID Base.  */
 return cpu->midr;
 case 0xd04: /* Interrupt Control State (ICSR) */
-- 
2.20.1

Re: [PATCH v3 1/9] fuzz: Make fork_fuzz.ld compatible with LLVM's LLD

2020-11-19 Thread Daniele Buono


Thanks Alex,
do you think you could also give it a try linking with LLD?

just add --extra-ldflags="-fuse-ld=lld"

I do see some small differences when moving from BFD ro LLD, but they 
should not be of importance. The position of the data.fuzz* is kept.


size -A on qemu-fuzz-i386, LTO DISABLED:

BFD
section  size   addr
[...]
.got10704   29849128
.data 1160800   29859840
__sancov_pcs  3362992   31020640
.data.fuzz_start   210187   34385920
.data.fuzz_ordered 211456   34596352
.bss  9659608   34807808
.comment  225  0
[...]

BFD
section  size   addr
[...]
.got  816   27824632
.got.plt 9992   27825448
.data 1160808   27839536
.data.fuzz_start   210187   29003776
.data.fuzz_ordered 211456   29214208
.data.fuzz_end  0   29425664
.tm_clone_table 0   29425664
__sancov_pcs  3362992   29425664
.bss  9659624   32788672

I tried running the fuzzer and didn't seem to have any issues, but I
haven't tried a test-build with OSS-Fuzz. Is there a info somewhere
on how to do that?

Thanks,
Daniele

On 11/6/2020 9:50 AM, Alexander Bulekov wrote:

On 201105 1718, Daniele Buono wrote:

LLVM's linker, LLD, supports the keyword "INSERT AFTER", starting with
version 11.
However, when multiple sections are defined in the same "INSERT AFTER",
they are added in a reversed order, compared to BFD's LD.

This patch makes fork_fuzz.ld generic enough to work with both linkers.
Each section now has its own "INSERT AFTER" keyword, so proper ordering is
defined between the sections added.



Hi Daniele,
Good to know that LLVM now has support for "INSERT AFTER" :)

I compared the resulting symbols between __FUZZ_COUNTERS_{START,END}
(after linking with BFD) before/after this patch, and they look good. I
also ran a test-build with OSS-Fuzz container and confirmed that the
resulting binary also had proper symbols.

Reviewed-by: Alexander Bulekov 
Tested-by: Alexander Bulekov 

Thanks


Signed-off-by: Daniele Buono 
---
  tests/qtest/fuzz/fork_fuzz.ld | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/fuzz/fork_fuzz.ld b/tests/qtest/fuzz/fork_fuzz.ld
index bfb667ed06..cfb88b7fdb 100644
--- a/tests/qtest/fuzz/fork_fuzz.ld
+++ b/tests/qtest/fuzz/fork_fuzz.ld
@@ -16,6 +16,11 @@ SECTIONS
/* Lowest stack counter */
*(__sancov_lowest_stack);
}
+}
+INSERT AFTER .data;
+
+SECTIONS
+{
.data.fuzz_ordered :
{
/*
@@ -34,6 +39,11 @@ SECTIONS
 */
 *(.bss._ZN6fuzzer3TPCE);
}
+}
+INSERT AFTER .data.fuzz_start;
+
+SECTIONS
+{
.data.fuzz_end : ALIGN(4K)
{
__FUZZ_COUNTERS_END = .;
@@ -43,4 +53,4 @@ SECTIONS
   * Don't overwrite the SECTIONS in the default linker script. Instead insert 
the
   * above into the default script
   */
-INSERT AFTER .data;
+INSERT AFTER .data.fuzz_ordered;
--
2.17.1

[PATCH v2 19/28] target/arm: Implement new v8.1M NOCP check for exception return

In v8.1M a new exception return check is added which may cause a NOCP
UsageFault (see rule R_XLTP): before we clear s0..s15 and the FPSCR
we must check whether access to CP10 from the Security state of the
returning exception is disabled; if it is then we must take a fault.

(Note that for our implementation CPPWR is always RAZ/WI and so can
never cause CP10 accesses to fail.)

The other v8.1M change to this register-clearing code is that if MVE
is implemented VPR must also be cleared, so add a TODO comment to
that effect.

Signed-off-by: Peter Maydell 
---
 target/arm/m_helper.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 9cdc8a64c29..0bdd3cc10e9 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -1515,7 +1515,27 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
 v7m_exception_taken(cpu, excret, true, false);
 return;
 } else {
-/* Clear s0..s15 and FPSCR */
+if (arm_feature(env, ARM_FEATURE_V8_1M)) {
+/* v8.1M adds this NOCP check */
+bool nsacr_pass = exc_secure ||
+extract32(env->v7m.nsacr, 10, 1);
+bool cpacr_pass = v7m_cpacr_pass(env, exc_secure, true);
+if (!nsacr_pass) {
+armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE, 
true);
+env->v7m.cfsr[M_REG_S] |= R_V7M_CFSR_NOCP_MASK;
+qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on 
existing "
+"stackframe: NSACR prevents clearing FPU registers\n");
+v7m_exception_taken(cpu, excret, true, false);
+} else if (!cpacr_pass) {
+armv7m_nvic_set_pending(env->nvic, ARMV7M_EXCP_USAGE,
+exc_secure);
+env->v7m.cfsr[exc_secure] |= R_V7M_CFSR_NOCP_MASK;
+qemu_log_mask(CPU_LOG_INT, "...taking UsageFault on 
existing "
+"stackframe: CPACR prevents clearing FPU registers\n");
+v7m_exception_taken(cpu, excret, true, false);
+}
+}
+/* Clear s0..s15 and FPSCR; TODO also VPR when MVE is implemented 
*/
 int i;
 
 for (i = 0; i < 16; i += 2) {
-- 
2.20.1

[PATCH v2 15/28] hw/intc/armv7m_nvic: Update FPDSCR masking for v8.1M

The FPDSCR register has a similar layout to the FPSCR.  In v8.1M it
gains new fields FZ16 (if half-precision floating point is supported)
and LTPSIZE (always reads as 4).  Update the reset value and the code
that handles writes to this register accordingly.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h  | 5 +
 hw/intc/armv7m_nvic.c | 9 -
 target/arm/cpu.c  | 3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 04f6220b2f7..47cb5032ce9 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1521,14 +1521,19 @@ void vfp_set_fpscr(CPUARMState *env, uint32_t val);
 #define FPCR_IXE(1 << 12)   /* Inexact exception trap enable */
 #define FPCR_IDE(1 << 15)   /* Input Denormal exception trap enable */
 #define FPCR_FZ16   (1 << 19)   /* ARMv8.2+, FP16 flush-to-zero */
+#define FPCR_RMODE_MASK (3 << 22) /* Rounding mode */
 #define FPCR_FZ (1 << 24)   /* Flush-to-zero enable bit */
 #define FPCR_DN (1 << 25)   /* Default NaN enable bit */
+#define FPCR_AHP(1 << 26)   /* Alternative half-precision */
 #define FPCR_QC (1 << 27)   /* Cumulative saturation bit */
 #define FPCR_V  (1 << 28)   /* FP overflow flag */
 #define FPCR_C  (1 << 29)   /* FP carry flag */
 #define FPCR_Z  (1 << 30)   /* FP zero flag */
 #define FPCR_N  (1 << 31)   /* FP negative flag */
 
+#define FPCR_LTPSIZE_SHIFT 16   /* LTPSIZE, M-profile only */
+#define FPCR_LTPSIZE_MASK (7 << FPCR_LTPSIZE_SHIFT)
+
 #define FPCR_NZCV_MASK (FPCR_N | FPCR_Z | FPCR_C | FPCR_V)
 #define FPCR_NZCVQC_MASK (FPCR_NZCV_MASK | FPCR_QC)
 
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 9628ce876e0..be3bc1f1f45 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2068,7 +2068,14 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 break;
 case 0xf3c: /* FPDSCR */
 if (cpu_isar_feature(aa32_vfp_simd, cpu)) {
-value &= 0x07c0;
+uint32_t mask = FPCR_AHP | FPCR_DN | FPCR_FZ | FPCR_RMODE_MASK;
+if (cpu_isar_feature(any_fp16, cpu)) {
+mask |= FPCR_FZ16;
+}
+value &= mask;
+if (cpu_isar_feature(aa32_lob, cpu)) {
+value |= 4 << FPCR_LTPSIZE_SHIFT;
+}
 cpu->env.v7m.fpdscr[attrs.secure] = value;
 }
 break;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 40f3f798b2b..d6188f6566a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -262,6 +262,9 @@ static void arm_cpu_reset(DeviceState *dev)
  * always reset to 4.
  */
 env->v7m.ltpsize = 4;
+/* The LTPSIZE field in FPDSCR is constant and reads as 4. */
+env->v7m.fpdscr[M_REG_NS] = 4 << FPCR_LTPSIZE_SHIFT;
+env->v7m.fpdscr[M_REG_S] = 4 << FPCR_LTPSIZE_SHIFT;
 }
 
 if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
-- 
2.20.1

[PATCH v2 28/28] target/arm: Implement Cortex-M55 model

Now that we have implemented all the features needed by the v8.1M
architecture, we can add the model of the Cortex-M55.  This is the
configuration without MVE support; we'll add MVE later.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu_tcg.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 0013e25412f..98544db2df3 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -401,6 +401,46 @@ static void cortex_m33_initfn(Object *obj)
 cpu->ctr = 0x8000c000;
 }
 
+static void cortex_m55_initfn(Object *obj)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+
+set_feature(>env, ARM_FEATURE_V8);
+set_feature(>env, ARM_FEATURE_V8_1M);
+set_feature(>env, ARM_FEATURE_M);
+set_feature(>env, ARM_FEATURE_M_MAIN);
+set_feature(>env, ARM_FEATURE_M_SECURITY);
+set_feature(>env, ARM_FEATURE_THUMB_DSP);
+cpu->midr = 0x410fd221; /* r0p1 */
+cpu->revidr = 0;
+cpu->pmsav7_dregion = 16;
+cpu->sau_sregion = 8;
+/*
+ * These are the MVFR* values for the FPU, no MVE configuration;
+ * we will update them later when we implement MVE
+ */
+cpu->isar.mvfr0 = 0x10110221;
+cpu->isar.mvfr1 = 0x12100011;
+cpu->isar.mvfr2 = 0x0040;
+cpu->isar.id_pfr0 = 0x2030;
+cpu->isar.id_pfr1 = 0x0230;
+cpu->isar.id_dfr0 = 0x1020;
+cpu->id_afr0 = 0x;
+cpu->isar.id_mmfr0 = 0x00111040;
+cpu->isar.id_mmfr1 = 0x;
+cpu->isar.id_mmfr2 = 0x0100;
+cpu->isar.id_mmfr3 = 0x0011;
+cpu->isar.id_isar0 = 0x01103110;
+cpu->isar.id_isar1 = 0x02212000;
+cpu->isar.id_isar2 = 0x20232232;
+cpu->isar.id_isar3 = 0x0131;
+cpu->isar.id_isar4 = 0x01310132;
+cpu->isar.id_isar5 = 0x;
+cpu->isar.id_isar6 = 0x;
+cpu->clidr = 0x; /* caches not implemented */
+cpu->ctr = 0x8303c003;
+}
+
 static const ARMCPRegInfo cortexr5_cp_reginfo[] = {
 /* Dummy the TCM region regs for the moment */
 { .name = "ATCM", .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 0,
@@ -655,6 +695,8 @@ static const ARMCPUInfo arm_tcg_cpus[] = {
  .class_init = arm_v7m_class_init },
 { .name = "cortex-m33",  .initfn = cortex_m33_initfn,
  .class_init = arm_v7m_class_init },
+{ .name = "cortex-m55",  .initfn = cortex_m55_initfn,
+ .class_init = arm_v7m_class_init },
 { .name = "cortex-r5",   .initfn = cortex_r5_initfn },
 { .name = "cortex-r5f",  .initfn = cortex_r5f_initfn },
 { .name = "ti925t",  .initfn = ti925t_initfn },
-- 
2.20.1

Re: [PATCH v3 0/9] Add support for Control-Flow Integrity

2020-11-19 Thread Daniele Buono


Hi Alex,

Yeah I assumed it was an older version because the errors triggered by
clang11 stop the compilation.

I checked again and for oss-fuzz, you disable failing on warnings.
So again, these patches are not directly connected to CFI and therefore 
could land independently.


On 11/6/2020 9:58 AM, Alexander Bulekov wrote:

I think oss-fuzz is using a bleeding edge version of Clang, so that
might not be a problem.
Here is the oss-fuzz build-log from earlier today:
https://oss-fuzz-build-logs.storage.googleapis.com/log-1747e14f-6b87-43e0-96aa-07ea159e7eb2.txt

...
Step #4: C compiler for the host machine: clang (clang 12.0.0 "clang version 12.0.0 
(https://github.com/llvm/llvm-project.git  
c9f69ee7f94cfefc373c3c6cae08e51b11e6d3c2)")
Step #4: C linker for the host machine: clang ld.bfd 2.26.1
Step #4: Host machine cpu family: x86_64
...


Yeah I assumed it was an older version because the errors triggered by
clang11 stop the compilation.

I checked again and for oss-fuzz, you disable failing on warnings.
So again, these patches are not directly connected to CFI and therefore 
could land independently.

[PATCH v2 26/28] hw/intc/armv7m_nvic: Implement read/write for RAS register block

The RAS feature has a block of memory-mapped registers at offset
0x5000 within the PPB.  For a "minimal RAS" implementation we provide
no error records and so the only registers that exist in the block
are ERRIIDR and ERRDEVID.

The "RAZ/WI for privileged, BusFault for nonprivileged" behaviour
of the "nvic-default" region is actually valid for minimal-RAS,
so the main benefit of providing an explicit implementation of
the register block is more accurate LOG_UNIMP messages, and a
framework for where we could add a real RAS implementation later
if necessary.

Signed-off-by: Peter Maydell 
---
 include/hw/intc/armv7m_nvic.h |  1 +
 hw/intc/armv7m_nvic.c | 56 +++
 2 files changed, 57 insertions(+)

diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h
index 33b6d8810c7..39c71e15936 100644
--- a/include/hw/intc/armv7m_nvic.h
+++ b/include/hw/intc/armv7m_nvic.h
@@ -83,6 +83,7 @@ struct NVICState {
 MemoryRegion sysreg_ns_mem;
 MemoryRegion systickmem;
 MemoryRegion systick_ns_mem;
+MemoryRegion ras_mem;
 MemoryRegion container;
 MemoryRegion defaultmem;
 
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index c42b291f881..5ab77a3530c 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2530,6 +2530,56 @@ static const MemoryRegionOps nvic_systick_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+
+static MemTxResult ras_read(void *opaque, hwaddr addr,
+uint64_t *data, unsigned size,
+MemTxAttrs attrs)
+{
+if (attrs.user) {
+return MEMTX_ERROR;
+}
+
+switch (addr) {
+case 0xe10: /* ERRIIDR */
+/* architect field = Arm; product/variant/revision 0 */
+*data = 0x43b;
+break;
+case 0xfc8: /* ERRDEVID */
+/* Minimal RAS: we implement 0 error record indexes */
+*data = 0;
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "Read RAS register offset 0x%x\n",
+  (uint32_t)addr);
+*data = 0;
+break;
+}
+return MEMTX_OK;
+}
+
+static MemTxResult ras_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size,
+ MemTxAttrs attrs)
+{
+if (attrs.user) {
+return MEMTX_ERROR;
+}
+
+switch (addr) {
+default:
+qemu_log_mask(LOG_UNIMP, "Write to RAS register offset 0x%x\n",
+  (uint32_t)addr);
+break;
+}
+return MEMTX_OK;
+}
+
+static const MemoryRegionOps ras_ops = {
+.read_with_attrs = ras_read,
+.write_with_attrs = ras_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 /*
  * Unassigned portions of the PPB space are RAZ/WI for privileged
  * accesses, and fault for non-privileged accesses.
@@ -2877,6 +2927,12 @@ static void armv7m_nvic_realize(DeviceState *dev, Error 
**errp)
 >systick_ns_mem, 1);
 }
 
+if (cpu_isar_feature(aa32_ras, s->cpu)) {
+memory_region_init_io(>ras_mem, OBJECT(s),
+  _ops, s, "nvic_ras", 0x1000);
+memory_region_add_subregion(>container, 0x5000, >ras_mem);
+}
+
 sysbus_init_mmio(SYS_BUS_DEVICE(dev), >container);
 }
 
-- 
2.20.1

[PATCH v2 14/28] target/arm: Implement FPCXT_NS fp system register

Implement the v8.1M FPCXT_NS floating-point system register.  This is
a little more complicated than FPCXT_S, because it has specific
handling for "current FP state is inactive", and it only wants to do
PreserveFPState(), not the full set of actions done by
ExecuteFPCheck() which vfp_access_check() implements.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-vfp.c.inc | 110 ++---
 1 file changed, 103 insertions(+), 7 deletions(-)

diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index ebc59daf613..1c2d31f6f30 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -647,8 +647,20 @@ typedef enum fp_sysreg_check_result {
 fp_sysreg_check_continue, /* caller should continue generating code */
 } fp_sysreg_check_result;
 
-static fp_sysreg_check_result fp_sysreg_checks(DisasContext *s, int regno)
+/*
+ * Emit code to check common UNDEF cases and handle lazy state preservation
+ * including the special casing for FPCXT_NS. For reads of sysregs, caller
+ * should provide storefn and opaque; for writes to sysregs these can be NULL.
+ * On return, if *insn_end_label is not NULL the caller needs to 
gen_set_label()
+ * it at the end of the other code generated for the insn.
+ */
+static fp_sysreg_check_result fp_sysreg_checks(DisasContext *s, int regno,
+   fp_sysreg_storefn *storefn,
+   void *opaque,
+   TCGLabel **insn_end_label)
 {
+*insn_end_label = NULL;
+
 if (!dc_isar_feature(aa32_fpsp_v2, s)) {
 return fp_sysreg_check_failed;
 }
@@ -663,6 +675,7 @@ static fp_sysreg_check_result fp_sysreg_checks(DisasContext 
*s, int regno)
 }
 break;
 case ARM_VFP_FPCXT_S:
+case ARM_VFP_FPCXT_NS:
 if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
 return false;
 }
@@ -674,8 +687,46 @@ static fp_sysreg_check_result 
fp_sysreg_checks(DisasContext *s, int regno)
 return fp_sysreg_check_failed;
 }
 
-if (!vfp_access_check(s)) {
-return fp_sysreg_check_done;
+/*
+ * FPCXT_NS is a special case: it has specific handling for
+ * "current FP state is inactive", and must do the PreserveFPState()
+ * but not the usual full set of actions done by ExecuteFPCheck().
+ * We don't have a TB flag that matches the fpInactive check, so we
+ * do it at runtime as we don't expect FPCXT_NS accesses to be frequent.
+ * The code emitted here handles the fpInactive special case;
+ * the caller just has to do the codegen for the normal (!fpInactive)
+ * special case, and then set the label at the end.
+ */
+if (regno == ARM_VFP_FPCXT_NS) {
+/* fpInactive = FPCCR_NS.ASPEN == 1 && CONTROL.FPCA == 0 */
+TCGLabel *fp_active_label = gen_new_label();
+TCGv_i32 aspen, fpca;
+aspen = load_cpu_field(v7m.fpccr[M_REG_NS]);
+fpca = load_cpu_field(v7m.control[M_REG_S]);
+tcg_gen_andi_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
+tcg_gen_xori_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
+tcg_gen_andi_i32(fpca, fpca, R_V7M_CONTROL_FPCA_MASK);
+tcg_gen_or_i32(fpca, fpca, aspen);
+tcg_gen_brcondi_i32(TCG_COND_NE, fpca, 0, fp_active_label);
+tcg_temp_free_i32(aspen);
+tcg_temp_free_i32(fpca);
+
+/* fpInactive case: FPCXT_NS reads as FPDSCR_NS, write is NOP */
+if (storefn) {
+TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
+storefn(s, opaque, tmp);
+}
+/* jump to end of insn */
+*insn_end_label = gen_new_label();
+tcg_gen_br(*insn_end_label);
+
+gen_set_label(fp_active_label);
+/* !fpInactive: PreserveFPState() and handle register as normal */
+gen_preserve_fp_state(s);
+} else {
+if (!vfp_access_check(s)) {
+return fp_sysreg_check_done;
+}
 }
 
 return fp_sysreg_check_continue;
@@ -687,8 +738,10 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int 
regno,
 {
 /* Do a write to an M-profile floating point system register */
 TCGv_i32 tmp;
+TCGLabel *insn_end_label;
+bool lookup_tb = false;
 
-switch (fp_sysreg_checks(s, regno)) {
+switch (fp_sysreg_checks(s, regno, NULL, NULL, _end_label)) {
 case fp_sysreg_check_failed:
 return false;
 case fp_sysreg_check_done:
@@ -702,7 +755,7 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int 
regno,
 tmp = loadfn(s, opaque);
 gen_helper_vfp_set_fpscr(cpu_env, tmp);
 tcg_temp_free_i32(tmp);
-gen_lookup_tb(s);
+lookup_tb = true;
 break;
 case ARM_VFP_FPSCR_NZCVQC:
 {
@@ -721,6 +774,7 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int 
regno,
 break;
 }
 case ARM_VFP_FPCXT_S:
+case ARM_VFP_FPCXT_NS:
 {

[PATCH v2 27/28] hw/arm/armv7m: Correct typo in QOM object name

Correct a typo in the name we give the NVIC object.

Signed-off-by: Peter Maydell 
---
 hw/arm/armv7m.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 944f261dd05..8224d4ade9f 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -136,7 +136,7 @@ static void armv7m_instance_init(Object *obj)
 
 memory_region_init(>container, obj, "armv7m-container", UINT64_MAX);
 
-object_initialize_child(obj, "nvnic", >nvic, TYPE_NVIC);
+object_initialize_child(obj, "nvic", >nvic, TYPE_NVIC);
 object_property_add_alias(obj, "num-irq",
   OBJECT(>nvic), "num-irq");
 
-- 
2.20.1

[PATCH v2 17/28] target/arm: In v8.1M, don't set HFSR.FORCED on vector table fetch failures

In v8.1M, vector table fetch failures don't set HFSR.FORCED (see rule
R_LLRP).  (In previous versions of the architecture this was either
required or IMPDEF.)

Signed-off-by: Peter Maydell 
---
 target/arm/m_helper.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index 721b4b4896e..9cdc8a64c29 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -722,11 +722,15 @@ load_fail:
  * The HardFault is Secure if BFHFNMINS is 0 (meaning that all HFs are
  * secure); otherwise it targets the same security state as the
  * underlying exception.
+ * In v8.1M HardFaults from vector table fetch fails don't set FORCED.
  */
 if (!(cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK)) {
 exc_secure = true;
 }
-env->v7m.hfsr |= R_V7M_HFSR_VECTTBL_MASK | R_V7M_HFSR_FORCED_MASK;
+env->v7m.hfsr |= R_V7M_HFSR_VECTTBL_MASK;
+if (!arm_feature(env, ARM_FEATURE_V8_1M)) {
+env->v7m.hfsr |= R_V7M_HFSR_FORCED_MASK;
+}
 armv7m_nvic_set_pending_derived(env->nvic, ARMV7M_EXCP_HARD, exc_secure);
 return false;
 }
-- 
2.20.1

[PATCH v2 12/28] target/arm: Factor out preserve-fp-state from full_vfp_access_check()

Factor out the code which handles M-profile lazy FP state preservation
from full_vfp_access_check(); accesses to the FPCXT_NS register are
a special case which need to do just this part (corresponding in the
pseudocode to the PreserveFPState() function), and not the full
set of actions matching the pseudocode ExecuteFPCheck() which
normal FP instructions need to do.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/translate-vfp.c.inc | 45 --
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 6bc327e9819..9c90c0647bd 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -83,6 +83,32 @@ static inline long vfp_f16_offset(unsigned reg, bool top)
 return offs;
 }
 
+/*
+ * Generate code for M-profile lazy FP state preservation if needed;
+ * this corresponds to the pseudocode PreserveFPState() function.
+ */
+static void gen_preserve_fp_state(DisasContext *s)
+{
+if (s->v7m_lspact) {
+/*
+ * Lazy state saving affects external memory and also the NVIC,
+ * so we must mark it as an IO operation for icount (and cause
+ * this to be the last insn in the TB).
+ */
+if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) {
+s->base.is_jmp = DISAS_UPDATE_EXIT;
+gen_io_start();
+}
+gen_helper_v7m_preserve_fp_state(cpu_env);
+/*
+ * If the preserve_fp_state helper doesn't throw an exception
+ * then it will clear LSPACT; we don't need to repeat this for
+ * any further FP insns in this TB.
+ */
+s->v7m_lspact = false;
+}
+}
+
 /*
  * Check that VFP access is enabled. If it is, do the necessary
  * M-profile lazy-FP handling and then return true.
@@ -113,24 +139,7 @@ static bool full_vfp_access_check(DisasContext *s, bool 
ignore_vfp_enabled)
 /* Handle M-profile lazy FP state mechanics */
 
 /* Trigger lazy-state preservation if necessary */
-if (s->v7m_lspact) {
-/*
- * Lazy state saving affects external memory and also the NVIC,
- * so we must mark it as an IO operation for icount (and cause
- * this to be the last insn in the TB).
- */
-if (tb_cflags(s->base.tb) & CF_USE_ICOUNT) {
-s->base.is_jmp = DISAS_UPDATE_EXIT;
-gen_io_start();
-}
-gen_helper_v7m_preserve_fp_state(cpu_env);
-/*
- * If the preserve_fp_state helper doesn't throw an exception
- * then it will clear LSPACT; we don't need to repeat this for
- * any further FP insns in this TB.
- */
-s->v7m_lspact = false;
-}
+gen_preserve_fp_state(s);
 
 /* Update ownership of FP context: set FPCCR.S to match current state 
*/
 if (s->v8m_fpccr_s_wrong) {
-- 
2.20.1

[PATCH v2 13/28] target/arm: Implement FPCXT_S fp system register

Implement the new-in-v8.1M FPCXT_S floating point system register.
This is for saving and restoring the secure floating point context,
and it reads and writes bits [27:0] from the FPSCR and the
CONTROL.SFPA bit in bit [31].

Signed-off-by: Peter Maydell 
---
 target/arm/translate-vfp.c.inc | 58 ++
 1 file changed, 58 insertions(+)

diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 9c90c0647bd..ebc59daf613 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -662,6 +662,14 @@ static fp_sysreg_check_result 
fp_sysreg_checks(DisasContext *s, int regno)
 return false;
 }
 break;
+case ARM_VFP_FPCXT_S:
+if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
+return false;
+}
+if (!s->v8m_secure) {
+return false;
+}
+break;
 default:
 return fp_sysreg_check_failed;
 }
@@ -712,6 +720,26 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int 
regno,
 tcg_temp_free_i32(tmp);
 break;
 }
+case ARM_VFP_FPCXT_S:
+{
+TCGv_i32 sfpa, control, fpscr;
+/* Set FPSCR[27:0] and CONTROL.SFPA from value */
+tmp = loadfn(s, opaque);
+sfpa = tcg_temp_new_i32();
+tcg_gen_shri_i32(sfpa, tmp, 31);
+control = load_cpu_field(v7m.control[M_REG_S]);
+tcg_gen_deposit_i32(control, control, sfpa,
+R_V7M_CONTROL_SFPA_SHIFT, 1);
+store_cpu_field(control, v7m.control[M_REG_S]);
+fpscr = load_cpu_field(vfp.xregs[ARM_VFP_FPSCR]);
+tcg_gen_andi_i32(fpscr, fpscr, FPCR_NZCV_MASK);
+tcg_gen_andi_i32(tmp, tmp, ~FPCR_NZCV_MASK);
+tcg_gen_or_i32(fpscr, fpscr, tmp);
+store_cpu_field(fpscr, vfp.xregs[ARM_VFP_FPSCR]);
+tcg_temp_free_i32(tmp);
+tcg_temp_free_i32(sfpa);
+break;
+}
 default:
 g_assert_not_reached();
 }
@@ -755,6 +783,36 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int 
regno,
 tcg_gen_andi_i32(tmp, tmp, FPCR_NZCV_MASK);
 storefn(s, opaque, tmp);
 break;
+case ARM_VFP_FPCXT_S:
+{
+TCGv_i32 control, sfpa, fpscr;
+/* Bits [27:0] from FPSCR, bit [31] from CONTROL.SFPA */
+tmp = tcg_temp_new_i32();
+sfpa = tcg_temp_new_i32();
+gen_helper_vfp_get_fpscr(tmp, cpu_env);
+tcg_gen_andi_i32(tmp, tmp, ~FPCR_NZCV_MASK);
+control = load_cpu_field(v7m.control[M_REG_S]);
+tcg_gen_andi_i32(sfpa, control, R_V7M_CONTROL_SFPA_MASK);
+tcg_gen_shli_i32(sfpa, sfpa, 31 - R_V7M_CONTROL_SFPA_SHIFT);
+tcg_gen_or_i32(tmp, tmp, sfpa);
+tcg_temp_free_i32(sfpa);
+/*
+ * Store result before updating FPSCR etc, in case
+ * it is a memory write which causes an exception.
+ */
+storefn(s, opaque, tmp);
+/*
+ * Now we must reset FPSCR from FPDSCR_NS, and clear
+ * CONTROL.SFPA; so we'll end the TB here.
+ */
+tcg_gen_andi_i32(control, control, ~R_V7M_CONTROL_SFPA_MASK);
+store_cpu_field(control, v7m.control[M_REG_S]);
+fpscr = load_cpu_field(v7m.fpdscr[M_REG_NS]);
+gen_helper_vfp_set_fpscr(cpu_env, fpscr);
+tcg_temp_free_i32(fpscr);
+gen_lookup_tb(s);
+break;
+}
 default:
 g_assert_not_reached();
 }
-- 
2.20.1

[PATCH v2 10/28] target/arm: Implement M-profile FPSCR_nzcvqc

v8.1M defines a new FP system register FPSCR_nzcvqc; this behaves
like the existing FPSCR, except that it reads and writes only bits
[31:27] of the FPSCR (the N, Z, C, V and QC flag bits).  (Unlike the
FPSCR, the special case for Rt=15 of writing the CPSR.NZCV is not
permitted.)

Implement the register.  Since we don't yet implement MVE, we handle
the QC bit as RES0, with todo comments for where we will need to add
support later.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h   | 13 +
 target/arm/translate-vfp.c.inc | 27 +++
 2 files changed, 40 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index ad8b80c667d..04f6220b2f7 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1524,6 +1524,13 @@ void vfp_set_fpscr(CPUARMState *env, uint32_t val);
 #define FPCR_FZ (1 << 24)   /* Flush-to-zero enable bit */
 #define FPCR_DN (1 << 25)   /* Default NaN enable bit */
 #define FPCR_QC (1 << 27)   /* Cumulative saturation bit */
+#define FPCR_V  (1 << 28)   /* FP overflow flag */
+#define FPCR_C  (1 << 29)   /* FP carry flag */
+#define FPCR_Z  (1 << 30)   /* FP zero flag */
+#define FPCR_N  (1 << 31)   /* FP negative flag */
+
+#define FPCR_NZCV_MASK (FPCR_N | FPCR_Z | FPCR_C | FPCR_V)
+#define FPCR_NZCVQC_MASK (FPCR_NZCV_MASK | FPCR_QC)
 
 static inline uint32_t vfp_get_fpsr(CPUARMState *env)
 {
@@ -1568,6 +1575,12 @@ enum arm_cpu_mode {
 #define ARM_VFP_FPEXC   8
 #define ARM_VFP_FPINST  9
 #define ARM_VFP_FPINST2 10
+/* These ones are M-profile only */
+#define ARM_VFP_FPSCR_NZCVQC 2
+#define ARM_VFP_VPR 12
+#define ARM_VFP_P0 13
+#define ARM_VFP_FPCXT_NS 14
+#define ARM_VFP_FPCXT_S 15
 
 /* QEMU-internal value meaning "FPSCR, but we care only about NZCV" */
 #define QEMU_VFP_FPSCR_NZCV 0x
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index dc26759ab95..6c4b7db8213 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -648,6 +648,11 @@ static fp_sysreg_check_result 
fp_sysreg_checks(DisasContext *s, int regno)
 case ARM_VFP_FPSCR:
 case QEMU_VFP_FPSCR_NZCV:
 break;
+case ARM_VFP_FPSCR_NZCVQC:
+if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
+return false;
+}
+break;
 default:
 return fp_sysreg_check_failed;
 }
@@ -682,6 +687,22 @@ static bool gen_M_fp_sysreg_write(DisasContext *s, int 
regno,
 tcg_temp_free_i32(tmp);
 gen_lookup_tb(s);
 break;
+case ARM_VFP_FPSCR_NZCVQC:
+{
+TCGv_i32 fpscr;
+tmp = loadfn(s, opaque);
+/*
+ * TODO: when we implement MVE, write the QC bit.
+ * For non-MVE, QC is RES0.
+ */
+tcg_gen_andi_i32(tmp, tmp, FPCR_NZCV_MASK);
+fpscr = load_cpu_field(vfp.xregs[ARM_VFP_FPSCR]);
+tcg_gen_andi_i32(fpscr, fpscr, ~FPCR_NZCV_MASK);
+tcg_gen_or_i32(fpscr, fpscr, tmp);
+store_cpu_field(fpscr, vfp.xregs[ARM_VFP_FPSCR]);
+tcg_temp_free_i32(tmp);
+break;
+}
 default:
 g_assert_not_reached();
 }
@@ -710,6 +731,12 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int 
regno,
 gen_helper_vfp_get_fpscr(tmp, cpu_env);
 storefn(s, opaque, tmp);
 break;
+case ARM_VFP_FPSCR_NZCVQC:
+/*
+ * TODO: MVE has a QC bit, which we probably won't store
+ * in the xregs[] field. For non-MVE, where QC is RES0,
+ * we can just fall through to the FPSCR_NZCV case.
+ */
 case QEMU_VFP_FPSCR_NZCV:
 /*
  * Read just NZCV; this is a special case to avoid the
-- 
2.20.1

[PATCH v2 16/28] target/arm: For v8.1M, always clear R0-R3, R12, APSR, EPSR on exception entry

In v8.0M, on exception entry the registers R0-R3, R12, APSR and EPSR
are zeroed for an exception taken to Non-secure state; for an
exception taken to Secure state they become UNKNOWN, and we chose to
leave them at their previous values.

In v8.1M the behaviour is specified more tightly and these registers
are always zeroed regardless of the security state that the exception
targets (see rule R_KPZV).  Implement this.

Signed-off-by: Peter Maydell 
---
 target/arm/m_helper.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/target/arm/m_helper.c b/target/arm/m_helper.c
index aad01ea0127..721b4b4896e 100644
--- a/target/arm/m_helper.c
+++ b/target/arm/m_helper.c
@@ -897,10 +897,12 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr, 
bool dotailchain,
  * Clear registers if necessary to prevent non-secure exception
  * code being able to see register values from secure code.
  * Where register values become architecturally UNKNOWN we leave
- * them with their previous values.
+ * them with their previous values. v8.1M is tighter than v8.0M
+ * here and always zeroes the caller-saved registers regardless
+ * of the security state the exception is targeting.
  */
 if (arm_feature(env, ARM_FEATURE_M_SECURITY)) {
-if (!targets_secure) {
+if (!targets_secure || arm_feature(env, ARM_FEATURE_V8_1M)) {
 /*
  * Always clear the caller-saved registers (they have been
  * pushed to the stack earlier in v7m_push_stack()).
@@ -909,10 +911,16 @@ static void v7m_exception_taken(ARMCPU *cpu, uint32_t lr, 
bool dotailchain,
  * v7m_push_callee_stack()).
  */
 int i;
+/*
+ * r4..r11 are callee-saves, zero only if background
+ * state was Secure (EXCRET.S == 1) and exception
+ * targets Non-secure state
+ */
+bool zero_callee_saves = !targets_secure &&
+(lr & R_V7M_EXCRET_S_MASK);
 
 for (i = 0; i < 13; i++) {
-/* r4..r11 are callee-saves, zero only if EXCRET.S == 1 */
-if (i < 4 || i > 11 || (lr & R_V7M_EXCRET_S_MASK)) {
+if (i < 4 || i > 11 || zero_callee_saves) {
 env->regs[i] = 0;
 }
 }
-- 
2.20.1

[PATCH v2 04/28] target/arm: Implement VSCCLRM insn

Implement the v8.1M VSCCLRM insn, which zeros floating point
registers if there is an active floating point context.
This requires support in write_neon_element32() for the MO_32
element size, so add it.

Because we want to use arm_gen_condlabel(), we need to move
the definition of that function up in translate.c so it is
before the #include of translate-vfp.c.inc.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/cpu.h   |  9 
 target/arm/m-nocp.decode   |  8 +++-
 target/arm/translate.c | 21 +
 target/arm/translate-vfp.c.inc | 84 ++
 4 files changed, 111 insertions(+), 11 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e5514c82862..11400a9d248 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3555,6 +3555,15 @@ static inline bool isar_feature_aa32_mprofile(const 
ARMISARegisters *id)
 return FIELD_EX32(id->id_pfr1, ID_PFR1, MPROGMOD) != 0;
 }
 
+static inline bool isar_feature_aa32_m_sec_state(const ARMISARegisters *id)
+{
+/*
+ * Return true if M-profile state handling insns
+ * (VSCCLRM, CLRM, FPCTX access insns) are implemented
+ */
+return FIELD_EX32(id->id_pfr1, ID_PFR1, SECURITY) >= 3;
+}
+
 static inline bool isar_feature_aa32_fp16_arith(const ARMISARegisters *id)
 {
 /* Sadly this is encoded differently for A-profile and M-profile */
diff --git a/target/arm/m-nocp.decode b/target/arm/m-nocp.decode
index 28c8ac6b94c..ccd62e8739a 100644
--- a/target/arm/m-nocp.decode
+++ b/target/arm/m-nocp.decode
@@ -29,13 +29,17 @@
 # If the coprocessor is not present or disabled then we will generate
 # the NOCP exception; otherwise we let the insn through to the main decode.
 
+%vd_dp  22:1 12:4
+%vd_sp  12:4 22:1
+
  cp
 
 {
   # Special cases which do not take an early NOCP: VLLDM and VLSTM
   VLLDM_VLSTM  1110 1100 001 l:1 rn:4  1010  
-  # TODO: VSCCLRM (new in v8.1M) is similar:
-  #VSCCLRM  1110 1100 1-01   1011  ---0
+  # VSCCLRM (new in v8.1M) is similar:
+  VSCCLRM  1110 1100 1.01   1011 imm:7 0   vd=%vd_dp size=3
+  VSCCLRM  1110 1100 1.01   1010 imm:8 vd=%vd_sp size=2
 
   NOCP 111- 1110    cp:4   
   NOCP 111- 110-    cp:4   
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 6d04ca3a8a0..9f2b6018a21 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -100,6 +100,15 @@ void arm_translate_init(void)
 a64_translate_init();
 }
 
+/* Generate a label used for skipping this instruction */
+static void arm_gen_condlabel(DisasContext *s)
+{
+if (!s->condjmp) {
+s->condlabel = gen_new_label();
+s->condjmp = 1;
+}
+}
+
 /* Flags for the disas_set_da_iss info argument:
  * lower bits hold the Rt register number, higher bits are flags.
  */
@@ -1221,6 +1230,9 @@ static void write_neon_element64(TCGv_i64 src, int reg, 
int ele, MemOp memop)
 long off = neon_element_offset(reg, ele, memop);
 
 switch (memop) {
+case MO_32:
+tcg_gen_st32_i64(src, cpu_env, off);
+break;
 case MO_64:
 tcg_gen_st_i64(src, cpu_env, off);
 break;
@@ -5156,15 +5168,6 @@ static void gen_srs(DisasContext *s,
 s->base.is_jmp = DISAS_UPDATE_EXIT;
 }
 
-/* Generate a label used for skipping this instruction */
-static void arm_gen_condlabel(DisasContext *s)
-{
-if (!s->condjmp) {
-s->condlabel = gen_new_label();
-s->condjmp = 1;
-}
-}
-
 /* Skip this instruction if the ARM condition is false */
 static void arm_skip_unless(DisasContext *s, uint32_t cond)
 {
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 96948f5a2d3..2a67ed0f6e2 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -3406,6 +3406,90 @@ static bool trans_VLLDM_VLSTM(DisasContext *s, 
arg_VLLDM_VLSTM *a)
 return true;
 }
 
+static bool trans_VSCCLRM(DisasContext *s, arg_VSCCLRM *a)
+{
+int btmreg, topreg;
+TCGv_i64 zero;
+TCGv_i32 aspen, sfpa;
+
+if (!dc_isar_feature(aa32_m_sec_state, s)) {
+/* Before v8.1M, fall through in decode to NOCP check */
+return false;
+}
+
+/* Explicitly UNDEF because this takes precedence over NOCP */
+if (!arm_dc_feature(s, ARM_FEATURE_M_MAIN) || !s->v8m_secure) {
+unallocated_encoding(s);
+return true;
+}
+
+if (!dc_isar_feature(aa32_vfp_simd, s)) {
+/* NOP if we have neither FP nor MVE */
+return true;
+}
+
+/*
+ * If FPCCR.ASPEN != 0 && CONTROL_S.SFPA == 0 then there is no
+ * active floating point context so we must NOP (without doing
+ * any lazy state preservation or the NOCP check).
+ */
+aspen = load_cpu_field(v7m.fpccr[M_REG_S]);
+sfpa = load_cpu_field(v7m.control[M_REG_S]);
+tcg_gen_andi_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
+tcg_gen_xori_i32(aspen,

[PATCH v2 09/28] target/arm: Implement VLDR/VSTR system register

Implement the new-in-v8.1M VLDR/VSTR variants which directly
read or write FP system registers to memory.

Signed-off-by: Peter Maydell 
---
 target/arm/vfp.decode  | 14 ++
 target/arm/translate-vfp.c.inc | 89 ++
 2 files changed, 103 insertions(+)

diff --git a/target/arm/vfp.decode b/target/arm/vfp.decode
index 1300ba045dd..6f7f28f9a46 100644
--- a/target/arm/vfp.decode
+++ b/target/arm/vfp.decode
@@ -84,6 +84,20 @@ VLDR_VSTR_hp  1101 u:1 .0 l:1 rn:4  1001 imm:8  
vd=%vd_sp
 VLDR_VSTR_sp  1101 u:1 .0 l:1 rn:4  1010 imm:8  vd=%vd_sp
 VLDR_VSTR_dp  1101 u:1 .0 l:1 rn:4  1011 imm:8  vd=%vd_dp
 
+# M-profile VLDR/VSTR to sysreg
+%vldr_sysreg 22:1 13:3
+%imm7_0x4 0:7 !function=times_4
+
+_sysreg rn reg imm a w p
+@vldr_sysreg  ... . a:1 . . . rn:4 ... . ... .. ... \
+ reg=%vldr_sysreg imm=%imm7_0x4 _sysreg
+
+# P=0 W=0 is SEE "Related encodings", so split into two patterns
+VLDR_sysreg   110 1 . . w:1 1  ... 0 111 11 ... @vldr_sysreg p=1
+VLDR_sysreg   110 0 . . 1   1  ... 0 111 11 ... @vldr_sysreg p=0 
w=1
+VSTR_sysreg   110 1 . . w:1 0  ... 0 111 11 ... @vldr_sysreg p=1
+VSTR_sysreg   110 0 . . 1   0  ... 0 111 11 ... @vldr_sysreg p=0 
w=1
+
 # We split the load/store multiple up into two patterns to avoid
 # overlap with other insns in the "Advanced SIMD load/store and 64-bit move"
 # grouping:
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 2d201ad0888..dc26759ab95 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -912,6 +912,95 @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS 
*a)
 return true;
 }
 
+static void fp_sysreg_to_memory(DisasContext *s, void *opaque, TCGv_i32 value)
+{
+arg_vldr_sysreg *a = opaque;
+uint32_t offset = a->imm;
+TCGv_i32 addr;
+
+if (!a->a) {
+offset = - offset;
+}
+
+addr = load_reg(s, a->rn);
+if (a->p) {
+tcg_gen_addi_i32(addr, addr, offset);
+}
+
+if (s->v8m_stackcheck && a->rn == 13 && a->w) {
+gen_helper_v8m_stackcheck(cpu_env, addr);
+}
+
+gen_aa32_st32(s, value, addr, get_mem_index(s));
+tcg_temp_free_i32(value);
+
+if (a->w) {
+/* writeback */
+if (!a->p) {
+tcg_gen_addi_i32(addr, addr, offset);
+}
+store_reg(s, a->rn, addr);
+} else {
+tcg_temp_free_i32(addr);
+}
+}
+
+static TCGv_i32 memory_to_fp_sysreg(DisasContext *s, void *opaque)
+{
+arg_vldr_sysreg *a = opaque;
+uint32_t offset = a->imm;
+TCGv_i32 addr;
+TCGv_i32 value = tcg_temp_new_i32();
+
+if (!a->a) {
+offset = - offset;
+}
+
+addr = load_reg(s, a->rn);
+if (a->p) {
+tcg_gen_addi_i32(addr, addr, offset);
+}
+
+if (s->v8m_stackcheck && a->rn == 13 && a->w) {
+gen_helper_v8m_stackcheck(cpu_env, addr);
+}
+
+gen_aa32_ld32u(s, value, addr, get_mem_index(s));
+
+if (a->w) {
+/* writeback */
+if (!a->p) {
+tcg_gen_addi_i32(addr, addr, offset);
+}
+store_reg(s, a->rn, addr);
+} else {
+tcg_temp_free_i32(addr);
+}
+return value;
+}
+
+static bool trans_VLDR_sysreg(DisasContext *s, arg_vldr_sysreg *a)
+{
+if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
+return false;
+}
+if (a->rn == 15) {
+return false;
+}
+return gen_M_fp_sysreg_write(s, a->reg, memory_to_fp_sysreg, a);
+}
+
+static bool trans_VSTR_sysreg(DisasContext *s, arg_vldr_sysreg *a)
+{
+if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
+return false;
+}
+if (a->rn == 15) {
+return false;
+}
+return gen_M_fp_sysreg_read(s, a->reg, fp_sysreg_to_memory, a);
+}
+
 static bool trans_VMOV_half(DisasContext *s, arg_VMOV_single *a)
 {
 TCGv_i32 tmp;
-- 
2.20.1

[PATCH v2 11/28] target/arm: Use new FPCR_NZCV_MASK constant

We defined a constant name for the mask of NZCV bits in the FPCR/FPSCR
in the previous commit; use it in a couple of places in existing code,
where we're masking out everything except NZCV for the "load to Rt=15
sets CPSR.NZCV" special case.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/translate-vfp.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 6c4b7db8213..6bc327e9819 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -743,7 +743,7 @@ static bool gen_M_fp_sysreg_read(DisasContext *s, int regno,
  * helper call for the "VMRS to CPSR.NZCV" insn.
  */
 tmp = load_cpu_field(vfp.xregs[ARM_VFP_FPSCR]);
-tcg_gen_andi_i32(tmp, tmp, 0xf000);
+tcg_gen_andi_i32(tmp, tmp, FPCR_NZCV_MASK);
 storefn(s, opaque, tmp);
 break;
 default:
@@ -884,7 +884,7 @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS 
*a)
 case ARM_VFP_FPSCR:
 if (a->rt == 15) {
 tmp = load_cpu_field(vfp.xregs[ARM_VFP_FPSCR]);
-tcg_gen_andi_i32(tmp, tmp, 0xf000);
+tcg_gen_andi_i32(tmp, tmp, FPCR_NZCV_MASK);
 } else {
 tmp = tcg_temp_new_i32();
 gen_helper_vfp_get_fpscr(tmp, cpu_env);
-- 
2.20.1

[PATCH v2 06/28] target/arm: Enforce M-profile VMRS/VMSR register restrictions

For M-profile before v8.1M, the only valid register for VMSR/VMRS is
the FPSCR.  We have a comment that states this, but the actual logic
to forbid accesses for any other register value is missing, so we
would end up with A-profile style behaviour.  Add the missing check.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/translate-vfp.c.inc | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 2a67ed0f6e2..e100182a32c 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -622,7 +622,10 @@ static bool trans_VMSR_VMRS(DisasContext *s, arg_VMSR_VMRS 
*a)
  * Accesses to R15 are UNPREDICTABLE; we choose to undef.
  * (FPSCR -> r15 is a special case which writes to the PSR flags.)
  */
-if (a->rt == 15 && (!a->l || a->reg != ARM_VFP_FPSCR)) {
+if (a->reg != ARM_VFP_FPSCR) {
+return false;
+}
+if (a->rt == 15 && !a->l) {
 return false;
 }
 }
-- 
2.20.1

[PATCH v2 05/28] target/arm: Implement CLRM instruction

In v8.1M the new CLRM instruction allows zeroing an arbitrary set of
the general-purpose registers and APSR.  Implement this.

The encoding is a subset of the LDMIA T2 encoding, using what would
be Rn=0b (which UNDEFs for LDMIA).

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/t32.decode  |  6 +-
 target/arm/translate.c | 38 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/target/arm/t32.decode b/target/arm/t32.decode
index cfcc71bfb0a..f045eb62c84 100644
--- a/target/arm/t32.decode
+++ b/target/arm/t32.decode
@@ -609,7 +609,11 @@ UXTAB 1010 0101    10..    
   @rrr_rot
 
 STM_t32  1110 1000 10.0   @ldstm i=1 b=0
 STM_t32  1110 1001 00.0   @ldstm i=0 b=1
-LDM_t32  1110 1000 10.1   @ldstm i=1 b=0
+{
+  # Rn=15 UNDEFs for LDM; M-profile CLRM uses that encoding
+  CLRM   1110 1000 1001  list:16
+  LDM_t321110 1000 10.1   @ldstm i=1 b=0
+}
 LDM_t32  1110 1001 00.1   @ldstm i=0 b=1
 
  !extern rn w pu
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 9f2b6018a21..47a1a5739c8 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -7968,6 +7968,44 @@ static bool trans_LDM_t16(DisasContext *s, 
arg_ldst_block *a)
 return do_ldm(s, a, 1);
 }
 
+static bool trans_CLRM(DisasContext *s, arg_CLRM *a)
+{
+int i;
+TCGv_i32 zero;
+
+if (!dc_isar_feature(aa32_m_sec_state, s)) {
+return false;
+}
+
+if (extract32(a->list, 13, 1)) {
+return false;
+}
+
+if (!a->list) {
+/* UNPREDICTABLE; we choose to UNDEF */
+return false;
+}
+
+zero = tcg_const_i32(0);
+for (i = 0; i < 15; i++) {
+if (extract32(a->list, i, 1)) {
+/* Clear R[i] */
+tcg_gen_mov_i32(cpu_R[i], zero);
+}
+}
+if (extract32(a->list, 15, 1)) {
+/*
+ * Clear APSR (by calling the MSR helper with the same argument
+ * as for "MSR APSR_nzcvqg, Rn": mask = 0b1100, SYSM=0)
+ */
+TCGv_i32 maskreg = tcg_const_i32(0xc << 8);
+gen_helper_v7m_msr(cpu_env, maskreg, zero);
+tcg_temp_free_i32(maskreg);
+}
+tcg_temp_free_i32(zero);
+return true;
+}
+
 /*
  * Branch, branch with link
  */
-- 
2.20.1

[PATCH v2 08/28] target/arm: Move general-use constant expanders up in translate.c

The constant-expander functions like negate, plus_2, etc, are
generally useful; move them up in translate.c so we can use them in
the VFP/Neon decoders as well as in the A32/T32/T16 decoders.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/translate.c | 46 +++---
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 47a1a5739c8..f5acd32e76a 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -109,6 +109,30 @@ static void arm_gen_condlabel(DisasContext *s)
 }
 }
 
+/*
+ * Constant expanders for the decoders.
+ */
+
+static int negate(DisasContext *s, int x)
+{
+return -x;
+}
+
+static int plus_2(DisasContext *s, int x)
+{
+return x + 2;
+}
+
+static int times_2(DisasContext *s, int x)
+{
+return x * 2;
+}
+
+static int times_4(DisasContext *s, int x)
+{
+return x * 4;
+}
+
 /* Flags for the disas_set_da_iss info argument:
  * lower bits hold the Rt register number, higher bits are flags.
  */
@@ -5177,29 +5201,9 @@ static void arm_skip_unless(DisasContext *s, uint32_t 
cond)
 
 
 /*
- * Constant expanders for the decoders.
+ * Constant expanders used by T16/T32 decode
  */
 
-static int negate(DisasContext *s, int x)
-{
-return -x;
-}
-
-static int plus_2(DisasContext *s, int x)
-{
-return x + 2;
-}
-
-static int times_2(DisasContext *s, int x)
-{
-return x * 2;
-}
-
-static int times_4(DisasContext *s, int x)
-{
-return x * 4;
-}
-
 /* Return only the rotation part of T32ExpandImm.  */
 static int t32_expandimm_rot(DisasContext *s, int x)
 {
-- 
2.20.1

[PATCH v2 03/28] target/arm: Don't clobber ID_PFR1.Security on M-profile cores

In arm_cpu_realizefn() we check whether the board code disabled EL3
via the has_el3 CPU object property, which we create if the CPU
starts with the ARM_FEATURE_EL3 feature bit.  If it is disabled, then
we turn off ARM_FEATURE_EL3 and also zero out the relevant fields in
the ID_PFR1 and ID_AA64PFR0 registers.

This codepath was incorrectly being taken for M-profile CPUs, which
do not have an EL3 and don't set ARM_FEATURE_EL3, but which may have
the M-profile Security extension and so should have non-zero values
in the ID_PFR1.Security field.

Restrict the handling of the feature flag to A/R-profile cores.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 07492e9f9a4..40f3f798b2b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1674,7 +1674,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 }
 
-if (!cpu->has_el3) {
+if (!arm_feature(env, ARM_FEATURE_M) && !cpu->has_el3) {
 /* If the has_el3 CPU property is disabled then we need to disable the
  * feature.
  */
-- 
2.20.1

[PATCH v2 07/28] target/arm: Refactor M-profile VMSR/VMRS handling

Currently M-profile borrows the A-profile code for VMSR and VMRS
(access to the FP system registers), because all it needs to support
is the FPSCR.  In v8.1M things become significantly more complicated
in two ways:

 * there are several new FP system registers; some have side effects
   on read, and one (FPCXT_NS) needs to avoid the usual
   vfp_access_check() and the "only if FPU implemented" check

 * all sysregs are now accessible both by VMRS/VMSR (which
   reads/writes a general purpose register) and also by VLDR/VSTR
   (which reads/writes them directly to memory)

Refactor the structure of how we handle VMSR/VMRS to cope with this:

 * keep the M-profile code entirely separate from the A-profile code

 * abstract out the "read or write the general purpose register" part
   of the code into a loadfn or storefn function pointer, so we can
   reuse it for VLDR/VSTR.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h   |   3 +
 target/arm/translate-vfp.c.inc | 181 ++---
 2 files changed, 170 insertions(+), 14 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 11400a9d248..ad8b80c667d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1569,6 +1569,9 @@ enum arm_cpu_mode {
 #define ARM_VFP_FPINST  9
 #define ARM_VFP_FPINST2 10
 
+/* QEMU-internal value meaning "FPSCR, but we care only about NZCV" */
+#define QEMU_VFP_FPSCR_NZCV 0x
+
 /* iwMMXt coprocessor control registers.  */
 #define ARM_IWMMXT_wCID  0
 #define ARM_IWMMXT_wCon  1
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index e100182a32c..2d201ad0888 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -607,27 +607,180 @@ static bool trans_VDUP(DisasContext *s, arg_VDUP *a)
 return true;
 }
 
+/*
+ * M-profile provides two different sets of instructions that can
+ * access floating point system registers: VMSR/VMRS (which move
+ * to/from a general purpose register) and VLDR/VSTR sysreg (which
+ * move directly to/from memory). In some cases there are also side
+ * effects which must happen after any write to memory (which could
+ * cause an exception). So we implement the common logic for the
+ * sysreg access in gen_M_fp_sysreg_write() and gen_M_fp_sysreg_read(),
+ * which take pointers to callback functions which will perform the
+ * actual "read/write general purpose register" and "read/write
+ * memory" operations.
+ */
+
+/*
+ * Emit code to store the sysreg to its final destination; frees the
+ * TCG temp 'value' it is passed.
+ */
+typedef void fp_sysreg_storefn(DisasContext *s, void *opaque, TCGv_i32 value);
+/*
+ * Emit code to load the value to be copied to the sysreg; returns
+ * a new TCG temporary
+ */
+typedef TCGv_i32 fp_sysreg_loadfn(DisasContext *s, void *opaque);
+
+/* Common decode/access checks for fp sysreg read/write */
+typedef enum fp_sysreg_check_result {
+fp_sysreg_check_failed, /* caller should return false */
+fp_sysreg_check_done, /* caller should return true */
+fp_sysreg_check_continue, /* caller should continue generating code */
+} fp_sysreg_check_result;
+
+static fp_sysreg_check_result fp_sysreg_checks(DisasContext *s, int regno)
+{
+if (!dc_isar_feature(aa32_fpsp_v2, s)) {
+return fp_sysreg_check_failed;
+}
+
+switch (regno) {
+case ARM_VFP_FPSCR:
+case QEMU_VFP_FPSCR_NZCV:
+break;
+default:
+return fp_sysreg_check_failed;
+}
+
+if (!vfp_access_check(s)) {
+return fp_sysreg_check_done;
+}
+
+return fp_sysreg_check_continue;
+}
+
+static bool gen_M_fp_sysreg_write(DisasContext *s, int regno,
+ fp_sysreg_loadfn *loadfn,
+ void *opaque)
+{
+/* Do a write to an M-profile floating point system register */
+TCGv_i32 tmp;
+
+switch (fp_sysreg_checks(s, regno)) {
+case fp_sysreg_check_failed:
+return false;
+case fp_sysreg_check_done:
+return true;
+case fp_sysreg_check_continue:
+break;
+}
+
+switch (regno) {
+case ARM_VFP_FPSCR:
+tmp = loadfn(s, opaque);
+gen_helper_vfp_set_fpscr(cpu_env, tmp);
+tcg_temp_free_i32(tmp);
+gen_lookup_tb(s);
+break;
+default:
+g_assert_not_reached();
+}
+return true;
+}
+
+static bool gen_M_fp_sysreg_read(DisasContext *s, int regno,
+fp_sysreg_storefn *storefn,
+void *opaque)
+{
+/* Do a read from an M-profile floating point system register */
+TCGv_i32 tmp;
+
+switch (fp_sysreg_checks(s, regno)) {
+case fp_sysreg_check_failed:
+return false;
+case fp_sysreg_check_done:
+return true;
+case fp_sysreg_check_continue:
+break;
+}
+
+switch (regno) {
+case ARM_VFP_FPSCR:
+tmp = tcg_temp_new_i32();
+gen_helper_vfp_get_fpscr(tmp, cpu_env);
+storefn(s,

[PATCH v2 02/28] target/arm: Implement v8.1M PXN extension

In v8.1M the PXN architecture extension adds a new PXN bit to the
MPU_RLAR registers, which forbids execution of code in the region
from a privileged mode.

This is another feature which is just in the generic "in v8.1M" set
and has no ID register field indicating its presence.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 target/arm/helper.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 11b0803df72..abc470d9f17 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -11754,6 +11754,11 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t 
address,
 } else {
 uint32_t ap = extract32(env->pmsav8.rbar[secure][matchregion], 1, 2);
 uint32_t xn = extract32(env->pmsav8.rbar[secure][matchregion], 0, 1);
+bool pxn = false;
+
+if (arm_feature(env, ARM_FEATURE_V8_1M)) {
+pxn = extract32(env->pmsav8.rlar[secure][matchregion], 4, 1);
+}
 
 if (m_is_system_region(env, address)) {
 /* System space is always execute never */
@@ -11761,7 +11766,7 @@ bool pmsav8_mpu_lookup(CPUARMState *env, uint32_t 
address,
 }
 
 *prot = simple_ap_to_rw_prot(env, mmu_idx, ap);
-if (*prot && !xn) {
+if (*prot && !xn && !(pxn && !is_user)) {
 *prot |= PAGE_EXEC;
 }
 /* We don't need to look the attribute up in the MAIR0/MAIR1
-- 
2.20.1

[PATCH v2 00/28] target/arm: Implement v8.1M and Cortex-M55

This is a v2 because it's a respin of "target/arm: More v8.1M
features".  The bad news is it's nearly doubled in length.  The good
news is that this is because the new patches on the end are enough to
implement all the remaining missing v8.1M specifics to the point
where we can provide a Cortex-M55 CPU.  (There is as yet no board
model that uses the Cortex-M55, though; that's next on my todo list.)

As before, the series is a mix of bugfixes and new features:

In the bugfix category:
 * RAZWI (or BusFault for unprivileged accesses) the whole of the
   system PPB address range, not just the SCS
 * Don't clobber ID_PFR1.Security on M-profile
 * Don't allow VMRS/VMSR to fp sysregs that don't exist on M-profile
 * CCR.BFHFNMIGN should RAZ/WI for Nonsecure if AIRCR.BFHFNMINS == 0
 * The change in commit 077d7449100d824a4 to handle "bad returns from
   NMI/HardFault forcibly deactivate those exceptions" wasn't quite
   right; we would deactivate those exceptions but generally not
   trigger the illegal exception return that we ought to
 * fix a typo in the name we give the NVIC object when we create it

In the features category:
 * v8.1M PXN extension
 * VSCCLRM and CLRM insns
 * VLDR/VSTR (sysreg) insns
 * new M-profile fp sysregs: FPSCR_nzcvqc, FPCXT_S, FPCXT_NS
 * update FPDSCR masking to allow new-in-v8.1M bits in that register
 * v8.1M has a new REVIDR register
 * v8.1M does not set HFSR.FORCED on vector table fetch failure
 * v8.1M always clears R0-R3, R12, APSR, EPSR on exception entry
   (the behaviour is tightened up compared to v8.0M)
 * v8.1M has a new check on exception return which might trigger
   a NOCP UsageFault
 * v8.1M has new VLLDM and VLSTM encodings
 * v8.1M's new CCR.TRD bit that enables an extra integrity check
   when executing an SG instruction
 * v8.1M "minimal RAS" (the architecturally minimum permitted
   do-nothing implementation, essentially)
  
Already reviewed: patches 1-6, 8, 11, 12.

thanks
-- PMM

Peter Maydell (28):
  hw/intc/armv7m_nvic: Make all of system PPB range be RAZWI/BusFault
  target/arm: Implement v8.1M PXN extension
  target/arm: Don't clobber ID_PFR1.Security on M-profile cores
  target/arm: Implement VSCCLRM insn
  target/arm: Implement CLRM instruction
  target/arm: Enforce M-profile VMRS/VMSR register restrictions
  target/arm: Refactor M-profile VMSR/VMRS handling
  target/arm: Move general-use constant expanders up in translate.c
  target/arm: Implement VLDR/VSTR system register
  target/arm: Implement M-profile FPSCR_nzcvqc
  target/arm: Use new FPCR_NZCV_MASK constant
  target/arm: Factor out preserve-fp-state from full_vfp_access_check()
  target/arm: Implement FPCXT_S fp system register
  target/arm: Implement FPCXT_NS fp system register
  hw/intc/armv7m_nvic: Update FPDSCR masking for v8.1M
  target/arm: For v8.1M, always clear R0-R3, R12, APSR, EPSR on
exception entry
  target/arm: In v8.1M, don't set HFSR.FORCED on vector table fetch
failures
  target/arm: Implement v8.1M REVIDR register
  target/arm: Implement new v8.1M NOCP check for exception return
  target/arm: Implement new v8.1M VLLDM and VLSTM encodings
  hw/intc/armv7m_nvic: Correct handling of CCR.BFHFNMIGN
  hw/intc/armv7m_nvic: Support v8.1M CCR.TRD bit
  target/arm: Implement CCR_S.TRD behaviour for SG insns
  hw/intc/armv7m_nvic: Fix "return from inactive handler" check
  target/arm: Implement M-profile "minimal RAS implementation"
  hw/intc/armv7m_nvic: Implement read/write for RAS register block
  hw/arm/armv7m: Correct typo in QOM object name
  target/arm: Implement Cortex-M55 model

 include/hw/intc/armv7m_nvic.h  |   2 +
 target/arm/cpu.h   |  46 +++
 target/arm/m-nocp.decode   |  10 +-
 target/arm/t32.decode  |  10 +-
 target/arm/vfp.decode  |  14 +
 hw/arm/armv7m.c|   4 +-
 hw/intc/armv7m_nvic.c  | 257 +++---
 target/arm/cpu.c   |   5 +-
 target/arm/cpu_tcg.c   |  42 +++
 target/arm/helper.c|   7 +-
 target/arm/m_helper.c  | 130 ++-
 target/arm/translate.c | 105 --
 target/arm/translate-vfp.c.inc | 604 +++--
 13 files changed, 1115 insertions(+), 121 deletions(-)

-- 
2.20.1

[PATCH v2 01/28] hw/intc/armv7m_nvic: Make all of system PPB range be RAZWI/BusFault

For M-profile CPUs, the range from 0xe000 to 0xe00f is the
Private Peripheral Bus range, which includes all of the memory mapped
devices and registers that are part of the CPU itself, including the
NVIC, systick timer, and debug and trace components like the Data
Watchpoint and Trace unit (DWT).  Within this large region, the range
0xe000e000 to 0xe000efff is the System Control Space (NVIC, system
registers, systick) and 0xe002e000 to 0exe002efff is its Non-secure
alias.

The architecture is clear that within the SCS unimplemented registers
should be RES0 for privileged accesses and generate BusFault for
unprivileged accesses, and we currently implement this.

It is less clear about how to handle accesses to unimplemented
regions of the wider PPB.  Unprivileged accesses should definitely
cause BusFaults (R_DQQS), but the behaviour of privileged accesses is
not given as a general rule.  However, the register definitions of
individual registers for components like the DWT all state that they
are RES0 if the relevant component is not implemented, so the
simplest way to provide that is to provide RAZ/WI for the whole range
for privileged accesses.  (The v7M Arm ARM does say that reserved
registers should be UNK/SBZP.)

Expand the container MemoryRegion that the NVIC exposes so that
it covers the whole PPB space. This means:
 * moving the address that the ARMV7M device maps it to down by
   0xe000 bytes
 * moving the off and the offsets within the container of all the
   subregions forward by 0xe000 bytes
 * adding a new default MemoryRegion that covers the whole container
   at a lower priority than anything else and which provides the
   RAZWI/BusFault behaviour

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
---
 include/hw/intc/armv7m_nvic.h |  1 +
 hw/arm/armv7m.c   |  2 +-
 hw/intc/armv7m_nvic.c | 78 ++-
 3 files changed, 69 insertions(+), 12 deletions(-)

diff --git a/include/hw/intc/armv7m_nvic.h b/include/hw/intc/armv7m_nvic.h
index bb087b23c35..33b6d8810c7 100644
--- a/include/hw/intc/armv7m_nvic.h
+++ b/include/hw/intc/armv7m_nvic.h
@@ -84,6 +84,7 @@ struct NVICState {
 MemoryRegion systickmem;
 MemoryRegion systick_ns_mem;
 MemoryRegion container;
+MemoryRegion defaultmem;
 
 uint32_t num_irq;
 qemu_irq excpout;
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 8113b29f1fd..944f261dd05 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -225,7 +225,7 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
 sysbus_connect_irq(sbd, 0,
qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ));
 
-memory_region_add_subregion(>container, 0xe000e000,
+memory_region_add_subregion(>container, 0xe000,
 sysbus_mmio_get_region(sbd, 0));
 
 for (i = 0; i < ARRAY_SIZE(s->bitband); i++) {
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 42b1ad59e65..9628ce876e0 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2479,6 +2479,43 @@ static const MemoryRegionOps nvic_systick_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+/*
+ * Unassigned portions of the PPB space are RAZ/WI for privileged
+ * accesses, and fault for non-privileged accesses.
+ */
+static MemTxResult ppb_default_read(void *opaque, hwaddr addr,
+uint64_t *data, unsigned size,
+MemTxAttrs attrs)
+{
+qemu_log_mask(LOG_UNIMP, "Read of unassigned area of PPB: offset 0x%x\n",
+  (uint32_t)addr);
+if (attrs.user) {
+return MEMTX_ERROR;
+}
+*data = 0;
+return MEMTX_OK;
+}
+
+static MemTxResult ppb_default_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned size,
+ MemTxAttrs attrs)
+{
+qemu_log_mask(LOG_UNIMP, "Write of unassigned area of PPB: offset 0x%x\n",
+  (uint32_t)addr);
+if (attrs.user) {
+return MEMTX_ERROR;
+}
+return MEMTX_OK;
+}
+
+static const MemoryRegionOps ppb_default_ops = {
+.read_with_attrs = ppb_default_read,
+.write_with_attrs = ppb_default_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid.min_access_size = 1,
+.valid.max_access_size = 8,
+};
+
 static int nvic_post_load(void *opaque, int version_id)
 {
 NVICState *s = opaque;
@@ -2675,7 +2712,6 @@ static void nvic_systick_trigger(void *opaque, int n, int 
level)
 static void armv7m_nvic_realize(DeviceState *dev, Error **errp)
 {
 NVICState *s = NVIC(dev);
-int regionlen;
 
 /* The armv7m container object will have set our CPU pointer */
 if (!s->cpu || !arm_feature(>cpu->env, ARM_FEATURE_M)) {
@@ -2718,7 +2754,20 @@ static void armv7m_nvic_realize(DeviceState *dev, Error 
**errp)
   M_REG_S));
 }
 
-/* The NVIC and System Control Space (SCS)

Re: [RFC 13/15] target/riscv: rvb: address calculation

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> From: Kito Cheng 
> 
> Signed-off-by: Kito Cheng 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn32-64.decode   |  3 +++
>  target/riscv/insn32.decode  |  3 +++
>  target/riscv/insn_trans/trans_rvb.c.inc | 23 +
>  target/riscv/translate.c| 33 +
>  4 files changed, 62 insertions(+)

Reviewed-by: Richard Henderson 

r~

Re: [RFC 12/15] target/riscv: rvb: generalized or-combine

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> +static target_ulong do_gorc(target_ulong rs1,
> +target_ulong rs2,
> +const target_ulong masks[])

Similar comments to grev.  I'll also say that the masks array should *not* be
local to do_grev, but placed at file level so that it can be shared between the
two functions.

r~

Re: [RFC 11/15] target/riscv: rvb: generalized reverse

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> +static target_ulong do_grev(target_ulong rs1,
> +target_ulong rs2,
> +const target_ulong masks[])
> +{

I think the masks should be placed here, and not passed in.
What you should pass in is "int bits".

> +target_ulong x = rs1;
> +int shift = 1;
> +int i = 0;
> +
> +while (shift < TARGET_LONG_BITS) {
> +if (rs2 & shift) {
> +x = do_swap(x, masks[i], shift);
> +}
> +shift <<= 1;
> +++i;

Cleaner as for loop:

for (i = 0, shift = 1; shift < bits; i++, shift <<= 1)

> +static const target_ulong masks[] = {
> +#ifdef TARGET_RISCV32
> +0x, 0x, 0x0f0f0f0f,
> +0x00ff00ff, 0x,
> +#else
> +0x, 0x,
> +0x0f0f0f0f0f0f0f0f, 0x00ff00ff00ff00ff,
> +0x, 0x,
> +#endif

You don't need to replicate every entry.

dup_const(0x55, MO_8),
dup_const(0x33, MO_8),
dup_const(0x0f, MO_8),
dup_const(0xff, MO_16),
dup_const(0x, MO_32),
#ifdef TARGET_RISCV64
UINT32_MAX
#endif

> +target_ulong HELPER(grevw)(target_ulong rs1, target_ulong rs2)
> +{
> +static const target_ulong masks[] = {
> +0x, 0x, 0x0f0f0f0f,
> +0x00ff00ff, 0x,
> +};
> +
> +return do_grev(rs1, rs2, masks);

This one is broken because do_grev iterated to TARGET_LONG_BITS == 64, and the
masks array is too small.

Fixed by passing in 32 as bits parameter to do_grev, as above.

> +static bool trans_grevi(DisasContext *ctx, arg_grevi *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +
> +if (a->shamt >= TARGET_LONG_BITS) {
> +return false;
> +}
> +
> +return gen_grevi(ctx, a);
> +}

While this is ok for an initial implementation, it is worth noticing the shamt
for rev8 as a special case for tcg_gen_bswap_tl.

Otherwise, this needs the same gen_shift treatment.

r~

[Bug 1579645] Re: [XEN/KVM] pch audio doesn't work on both Windows and linux guest with soundhw="ac97"

The QEMU project is currently considering to move its bug tracking to another 
system. For this we need to know which bugs are still valid and which could be 
closed already. Thus we are setting older bugs to "Incomplete" now.
If you still think this bug report here is valid, then please switch the state 
back to "New" within the next 60 days, otherwise this report will be marked as 
"Expired". Or mark it as "Fix Released" if the problem has been solved with a 
newer version of QEMU already. Thank you and sorry for the inconvenience.

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

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

Title:
   [XEN/KVM] pch audio doesn't work on both Windows and linux guest with
  soundhw="ac97"

Status in QEMU:
  Incomplete

Bug description:
  Environment:

  when try to boot a guest by qemu with parameter "-soundhw ac97", it showed 
like below:
  "audio: Could no init “oss” audio driver.",
  then login the guest and try run audio, no sound output.
  Reproduce:
  1. kvm: qemu-system-x86_64 -enable-kvm -m 2048 -smp 4 -net nic,model=rtl8139 
-net tap,script=/etc/kvm/qemu-ifup -soundhw ac97 -hda [target.img]
 xen: add the audio device in guest configure file by soundhw="ac97", xl 
create $guest-configure
  2. it will show "audio: Could no init “oss” audio driver".
  3. login in guest, it can detect audio device, but actually it is not working.

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

Re: [PATCH 2/2] pc-bios: s390x: Give precedence to reset PSW

2020-11-19 Thread Eric Farman





On 11/19/20 3:20 PM, Thomas Huth wrote:

On 19/11/2020 17.57, Eric Farman wrote:

Let's look at the Reset PSW first instead of the contents of memory.
It might be leftover from an earlier system boot when processing
a chreipl.

Signed-off-by: Eric Farman 
---
  pc-bios/s390-ccw/jump2ipl.c | 20 ++--
  1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/pc-bios/s390-ccw/jump2ipl.c b/pc-bios/s390-ccw/jump2ipl.c
index fbae45b03c..67b4afb6a0 100644
--- a/pc-bios/s390-ccw/jump2ipl.c
+++ b/pc-bios/s390-ccw/jump2ipl.c
@@ -72,16 +72,6 @@ void jump_to_IPL_code(uint64_t address)
  
  void jump_to_low_kernel(void)

  {
-/*
- * If it looks like a Linux binary, i.e. there is the "S390EP" magic from
- * arch/s390/kernel/head.S here, then let's jump to the well-known Linux
- * kernel start address (when jumping to the PSW-at-zero address instead,
- * the kernel startup code fails when we booted from a network device).
- */
-if (!memcmp((char *)0x10008, "S390EP", 6)) {
-jump_to_IPL_code(KERN_IMAGE_START);
-}
-
  /* Trying to get PSW at zero address */
  if (*((uint64_t *)0) & RESET_PSW_MASK) {
  /*
@@ -92,6 +82,16 @@ void jump_to_low_kernel(void)
  jump_to_IPL_code(0);
  }
  
+/*

+ * If it looks like a Linux binary, i.e. there is the "S390EP" magic from
+ * arch/s390/kernel/head.S here, then let's jump to the well-known Linux
+ * kernel start address (when jumping to the PSW-at-zero address instead,
+ * the kernel startup code fails when we booted from a network device).
+ */
+if (!memcmp((char *)0x10008, "S390EP", 6)) {
+jump_to_IPL_code(KERN_IMAGE_START);
+}


That feels a little bit dangerous ... I assume the order has been that way
for a reason, e.g. I think we had to jump to KERN_IMAGE_START for some older
versions of the Linux kernel since the startup code that was referenced by
the PSW at address zero was not working in KVM...


Makes sense.  It does seem like a precarious piece of code.



What do you think about this alternate idea instead: Clear the memory at
location 0x10008 at the very beginning of the main() function (or maybe in
boot_setup())? 


This seems to work too (I put it in boot_setup(), prior to call to 
store_iplb()).


Then we can be sure that there is no stale S390EP magic

dangling around anymore once we've loaded the new kernel...

  Thomas

Re: [RFC 08/15] target/riscv: rvb: single-bit instructions

On 11/19/20 12:35 PM, Richard Henderson wrote:
> On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
>> +static bool trans_sbset(DisasContext *ctx, arg_sbset *a)
>> +{
>> +REQUIRE_EXT(ctx, RVB);
>> +return gen_arith(ctx, a, _sbset);
>> +}
>> +
>> +static bool trans_sbseti(DisasContext *ctx, arg_sbseti *a)
>> +{
>> +REQUIRE_EXT(ctx, RVB);
>> +return gen_arith_shamt_tl(ctx, a, _sbset);
>> +}
>> +
>> +static bool trans_sbclr(DisasContext *ctx, arg_sbclr *a)
>> +{
>> +REQUIRE_EXT(ctx, RVB);
>> +return gen_arith(ctx, a, _sbclr);
>> +}
> 
> Coming back to my re-use of code thing, these should use gen_shift.  That
> handles the truncate of source2 to the shift amount.
> 
>> +static bool trans_sbclri(DisasContext *ctx, arg_sbclri *a)
>> +{
>> +REQUIRE_EXT(ctx, RVB);
>> +return gen_arith_shamt_tl(ctx, a, _sbclr);
>> +}
>> +
>> +static bool trans_sbinv(DisasContext *ctx, arg_sbinv *a)
>> +{
>> +REQUIRE_EXT(ctx, RVB);
>> +return gen_arith(ctx, a, _sbinv);
>> +}
>> +
>> +static bool trans_sbinvi(DisasContext *ctx, arg_sbinvi *a)
>> +{
>> +REQUIRE_EXT(ctx, RVB);
>> +return gen_arith_shamt_tl(ctx, a, _sbinv);
>> +}
> 
> I think there ought to be a gen_shifti for these.

Hmm.  I just realized that gen_shifti would have a generator callback with a
constant argument, a-la tcg_gen_shli_tl.

I don't know if it's worth duplicating gen_sbclr et al for a constant argument.
 And the sloi/sroi insns besides.  Perhaps a gen_shifti_var helper instead?

Let me know what you think, but at the moment we're left with an incoherent set
of helpers that seem split along lines that are less than ideal.


r~

Re: [RFC 10/15] target/riscv: rvb: rotate (left/right)

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> +static bool trans_rori(DisasContext *ctx, arg_rori *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +
> +if (a->shamt >= TARGET_LONG_BITS) {
> +return false;
> +}
> +
> +return gen_arith_shamt_tl(ctx, a, _gen_rotr_tl);
> +}

We most definitely want to use tcg_gen_rotri_tl here, as we have special
expansions of constant shifts for hosts without rotate (e.g. riscv64g ;-).

Otherwise, this patch should be included in the shift cleanup discussed 
upthread.

r~

[Bug 1518969] Re: Instance of QEMU doesn't unplug virtio scsi disk after device_del and drive_del commands

The QEMU project is currently considering to move its bug tracking to another 
system. For this we need to know which bugs are still valid and which could be 
closed already. Thus we are setting older bugs to "Incomplete" now.
If you still think this bug report here is valid, then please switch the state 
back to "New" within the next 60 days, otherwise this report will be marked as 
"Expired". Or mark it as "Fix Released" if the problem has been solved with a 
newer version of QEMU already. Thank you and sorry for the inconvenience.

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

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

Title:
  Instance of QEMU doesn't unplug virtio scsi disk after device_del and
  drive_del commands

Status in QEMU:
  Incomplete

Bug description:
  device_del and drive_del commands don't cause virtio disk detaching

  Steps to reproduce:
  1. Run instance

  2. Attach virtio scsi disk

  3. Reboot instance

  4. Immediately after reboot detach disk with QEMU commands:
  device_del
  drive_del

  Expected result:
  Disk should be detached anyway

  Actual:
  Domain info contains attached disk even after waiting long period of time(5 
min in my case).

  Additional info:
  QEMU process:
  root 29010 42.6  1.9 562036 61272 ?Rl   03:42   0:01 
/usr/bin/qemu-system-x86_64 -name instance-0069 -S -machine 
pc-i440fx-trusty,accel=tcg,usb=off -m 64 -realtime mlock=off -smp 
1,sockets=1,cores=1,threads=1 -uuid d2418536-2547-4740-96b5-0d4f1d74b9f3 
-smbios type=1,manufacturer=OpenStack Foundation,product=OpenStack 
Nova,version=13.0.0,serial=1fd8f69a-909b-4ed1-aae9-4fc9318fc622,uuid=d2418536-2547-4740-96b5-0d4f1d74b9f3,family=Virtual
 Machine -no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/instance-0069.monitor,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-boot strict=on -kernel 
/opt/stack/data/nova/instances/d2418536-2547-4740-96b5-0d4f1d74b9f3/kernel 
-initrd 
/opt/stack/data/nova/instances/d2418536-2547-4740-96b5-0d4f1d74b9f3/ramdisk 
-append root=/dev/vda console=tty0 console=ttyS0 no_timer_check -device 
piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive 
file=/opt/stack/data/nova/instances/d2418536-2547-4740-96b5-0d4f1d74b9f3/disk,if=none,id=drive-virtio-disk0,format=qcow2,cache=none
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -drive 
file=/opt/stack/data/nova/instances/d2418536-2547-4740-96b5-0d4f1d74b9f3/disk.config,if=none,id=drive-ide0-1-1,readonly=on,format=raw,cache=none
 -device ide-cd,bus=ide.1,unit=1,drive=drive-ide0-1-1,id=ide0-1-1 -netdev 
tap,fd=18,id=hostnet0 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=fa:16:3e:1a:10:3d,bus=pci.0,addr=0x3 
-chardev 
file,id=charserial0,path=/opt/stack/data/nova/instances/d2418536-2547-4740-96b5-0d4f1d74b9f3/console.log
 -device isa-serial,chardev=charserial0,id=serial0 -chardev pty,id=charserial1 
-device isa-serial,chardev=charserial1,id=serial1 -vnc 127.0.0.1:1 -k en-us 
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

  QEMU version:
  qemu-system-x86_64 --version
  QEMU emulator version 2.0.0 (Debian 2.0.0+dfsg-2ubuntu1.19), Copyright (c) 
2003-2008 Fabrice Bellard

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

[Bug 1391942] Re: Unnecessary events option of the trace argument with UST backend

The QEMU project is currently considering to move its bug tracking to another 
system. For this we need to know which bugs are still valid and which could be 
closed already. Thus we are setting older bugs to "Incomplete" now.
If you still think this bug report here is valid, then please switch the state 
back to "New" within the next 60 days, otherwise this report will be marked as 
"Expired". Or mark it as "Fix Released" if the problem has been solved with a 
newer version of QEMU already. Thank you and sorry for the inconvenience.

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

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

Title:
  Unnecessary events option of the trace argument with UST backend

Status in QEMU:
  Incomplete

Bug description:
  When running configure with the --enable-trace-backends=ust option and 
compiling. 
  The user should not have to specify a the "events" and "file" options because 
they are not used with that tracing framework.

  Right now, in order the use this option the need to specify a dummy
  events file.

  This fails:
  $> qemu-system-x86_64 -hda debian_wheezy_amd64_standard.qcow2 -trace -m 512
  qemu-system-x86_64: -trace -m: Invalid parameter '-m'

  This works:
  $> qemu-system-x86_64 -hda debian_wheezy_amd64_standard.qcow2 -trace 
events=dummy-events.txt -m 512
  VNC server running on `127.0.0.1:5900'

  I am using version:
  $> qemu-system-x86_64 --version
  QEMU emulator version 2.1.90, Copyright (c) 2003-2008 Fabrice Bellard

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

[Bug 1576347] Re: Only one NVMe device is usable in Windows (10) guest

The QEMU project is currently considering to move its bug tracking to another 
system. For this we need to know which bugs are still valid and which could be 
closed already. Thus we are setting older bugs to "Incomplete" now.
If you still think this bug report here is valid, then please switch the state 
back to "New" within the next 60 days, otherwise this report will be marked as 
"Expired". Or mark it as "Fix Released" if the problem has been solved with a 
newer version of QEMU already. Thank you and sorry for the inconvenience.

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

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

Title:
  Only one NVMe device is usable in Windows (10) guest

Status in QEMU:
  Incomplete

Bug description:
  Full command: qemu-system-x86_64 -enable-kvm -cpu host -smp cores=4 -m
  4G -net bridge -net nic -full-screen -drive
  file=ovmf_x64.bin,format=raw,if=pflash -drive
  file=disks/win16_ide.img,format=raw,cache=none,aio=native -drive
  file=disks/one.img,if=none,format=qcow2,id=one -drive
  file=disks/two.img,if=none,format=qcow2,id=two -device
  nvme,drive=one,serial=E86C3CFC43518D6F -device
  nvme,drive=two,serial=2BDAC262CF831698

  QEMU version: 2.5.0

  Kernel: 4.5.1 (Arch Linux)

  When there are two NVMe devices specified, only the second one will be
  usable in Windows. The following error is shown under "Device status"
  of the failed NVMe controller in Device Manager:

  "This device cannot start. (Code 10)

  The I/O device is configured incorrectly or the configuration
  parameters to the driver are incorrect."

  The only thing seems suspicious to me is that the nvme emulation in
  qemu does not have WWN/EUI-64 set for the devices, though I have no
  idea at all whether that is mandatory:

  "C:\Windows\system32>sg_vpd -i PD1
  Device Identification VPD page:
Addressed logical unit:
  designator type: SCSI name string,  code set: UTF-8
SCSI name string:
8086QEMU NVMe Ctrl  00012BDAC262CF831698

  C:\Windows\system32>sg_vpd -p sn PD1
  Unit serial number VPD page:
Unit serial number: ___."

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

[Bug 1579306] Re: usb-uas does not work in Windows (10) guest

The QEMU project is currently considering to move its bug tracking to another 
system. For this we need to know which bugs are still valid and which could be 
closed already. Thus we are setting older bugs to "Incomplete" now.
If you still think this bug report here is valid, then please switch the state 
back to "New" within the next 60 days, otherwise this report will be marked as 
"Expired". Or mark it as "Fix Released" if the problem has been solved with a 
newer version of QEMU already. Thank you and sorry for the inconvenience.

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

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

Title:
  usb-uas does not work in Windows (10) guest

Status in QEMU:
  Incomplete

Bug description:
  When I attach a "-device usb-uas" to a VM with Windows 10 10586, the
  device fail to start with the following error in the guest:

  "The device cannot start. (Code 10)

  {Operation Failed}
  The requested operation was unsuccessful"

  If the host controller is nec-usb-xhci, there will be two of the
  following error on the terminal of the host as well:

  "qemu-system-x86_64: usb_uas_handle_control: unhandled control
  request"

  If it's usb-ehci, ich9-usb-ehci1 or ich9-usb-echi2, this will not show
  up on the host side, but the device stil fails with the same error on
  the guest side.

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

[Bug 1390520] Re: virtual machine fails to start with connected audio cd

Sorry, since nobody seems to have capacity to work on this, it's
unlikely that this will ever be implemented in QEMU. Thus I'm closing
this as WontFix for now.

** Changed in: qemu
   Status: New => Won't Fix

** Changed in: libvirt (Ubuntu)
   Status: Confirmed => Invalid

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

Title:
  virtual machine fails to start with connected audio cd

Status in QEMU:
  Won't Fix
Status in libvirt package in Ubuntu:
  Invalid
Status in libvirt source package in Trusty:
  Won't Fix

Bug description:
  when connecting a data cd with a virtual machine (IDE CDROM 1), the virtual 
machine starts up and the data cd is accessable (for example to install 
software package or drivers),
  but connecting an audio cd the following error appears:

  
---
  cannot read header '/dev/sr0': Input/output error

  Traceback (most recent call last):
File "/usr/share/virt-manager/virtManager/details.py", line 2530, in 
_change_config_helper
  func(*args)
File "/usr/share/virt-manager/virtManager/domain.py", line 850, in 
hotplug_storage_media
  self.attach_device(devobj)
File "/usr/share/virt-manager/virtManager/domain.py", line 798, in 
attach_device
  self._backend.attachDevice(devxml)
File "/usr/lib/python2.7/dist-packages/libvirt.py", line 493, in 
attachDevice
  if ret == -1: raise libvirtError ('virDomainAttachDevice() failed', 
dom=self)
  libvirtError: cannot read header '/dev/sr0': Input/output error
  


  Description:Ubuntu 14.04.1 LTS
  Release:14.04

  qemu:
Installiert:   2.0.0+dfsg-2ubuntu1.6
Installationskandidat: 2.0.0+dfsg-2ubuntu1.6

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

[Bug 1381846] Re: Data sent to parallel port in guest is lost if host buffer fills up

The QEMU project is currently considering to move its bug tracking to another 
system. For this we need to know which bugs are still valid and which could be 
closed already. Thus we are setting older bugs to "Incomplete" now.
If you still think this bug report here is valid, then please switch the state 
back to "New" within the next 60 days, otherwise this report will be marked as 
"Expired". Or mark it as "Fix Released" if the problem has been solved with a 
newer version of QEMU already. Thank you and sorry for the inconvenience.

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

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

Title:
  Data sent to parallel port in guest is lost if host buffer fills up

Status in QEMU:
  Incomplete

Bug description:
  It appears that qemu will blindly write characters out to the chardev
  and drop them on the floor if a write fails with EAGAIN, without
  initiating flow control (via BUSY and ACK) back to the guest. If the
  host buffer is too small, or is talking to a hardware device that is
  too slow, data will be lost.

  I notice this problem when I run a DOS program with this on the qemu command 
line:
  -parallel /dev/usb/lp0

  I can work around this problem by buffering via a pipe, but this looks
  like a general problem. Is there a way to wire up the readiness of the
  output chardev to the parallel port ACK and BUSY lines, and signal an
  ISA interrupt? I don't know the code well enough to tell.

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

Re: [RFC 09/15] target/riscv: rvb: shift ones

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> +static bool trans_slo(DisasContext *ctx, arg_slo *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith(ctx, a, _slo);
> +}
> +
> +static bool trans_sloi(DisasContext *ctx, arg_sloi *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +
> +if (a->shamt >= TARGET_LONG_BITS) {
> +return false;
> +}
> +
> +return gen_arith_shamt_tl(ctx, a, _slo);
> +}
> +
> +static bool trans_sro(DisasContext *ctx, arg_sro *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith(ctx, a, _sro);
> +}
> +
> +static bool trans_sroi(DisasContext *ctx, arg_sroi *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +
> +if (a->shamt >= TARGET_LONG_BITS) {
> +return false;
> +}
> +
> +return gen_arith_shamt_tl(ctx, a, _sro);
> +}

Use the same gen_shift family of functions discussed vs patch 8.


r~

Re: [RFC PATCH 06/15] docs: add some documentation for the guest-loader

On Thu, 19 Nov 2020 at 20:30, Alistair Francis  wrote:
>
> On Thu, Nov 5, 2020 at 9:58 AM Alex Bennée  wrote:
> > +Currently the only supported machines which use FDT data to boot are
> > +the ARM and RiscV `virt` machines.
>
> RISC-V.

If we're going to be picky, also "Arm" :-)

-- PMM

[Bug 1377163] Re: Does not add usb-host devices as they are hotplugged

The QEMU project is currently considering to move its bug tracking to another 
system. For this we need to know which bugs are still valid and which could be 
closed already. Thus we are setting older bugs to "Incomplete" now.
If you still think this bug report here is valid, then please switch the state 
back to "New" within the next 60 days, otherwise this report will be marked as 
"Expired". Or mark it as "Fix Released" if the problem has been solved with a 
newer version of QEMU already. Thank you and sorry for the inconvenience.

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

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

Title:
  Does not add usb-host devices as they are hotplugged

Status in QEMU:
  Incomplete

Bug description:
  A commandline such as

  qemu-kvm -device usb-ehci,id=USBCtrl -device host-
  usb,bus=USBCtrl.0,hostbus=3

  should automatically add all devices on the given bus (here: 3) not
  only initially, but also when new devices appear on that bus while
  Qemu runs. Currently, all devices on the bus are added initially, but
  new devices which are added to the (host) usb while Qemu runs have to
  be added manually.

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

Re: [PATCH v2 3/8] qnum: QNumValue type for QNum value literals

2020-11-19 Thread Eduardo Habkost

On Thu, Nov 19, 2020 at 01:21:58PM -0500, Eduardo Habkost wrote:
> On Thu, Nov 19, 2020 at 11:24:52AM +0100, Markus Armbruster wrote:
[...]
> > >> > > +return qnum_from_value((QNumValue) QNUM_VAL_INT(value));
> > 
> > No space between between (type) and its operand, please.
> > 
> > Could we lift the cast into the macro somehow?
> 
> I think we can.  I had thought the cast in the macro would break
> usage as static variable initializers.  I was wrong.

Actually, including the cast in the macro breaks QLIT_QDICT
initializers (which use (QLitDictEntry[]) compound literals), and
I don't know why.

Compound literals in initializers of static variables is a GCC
extension.  I don't understand why it doesn't work inside array
compound literals, though.

Any language lawyers around?

This works:

  typedef struct QLit {
  int x, y;
  } QLit;
  
  typedef struct Entry {
  int key;
  QLit value;
  } Entry;
  
  Entry e = { .key = 0, .value = (QLit) { 1,   2 } };

This works:

  Entry *es1 = (Entry[]) {
  { .key = 0, .value = { 1,   2 } },
  };

But this doesn't:

  Entry *es2 = (Entry[]) {
  { .key = 0, .value = (QLit) { 1,   2 } },
  };

dict.c:16:24: error: initializer element is not constant
   16 | Entry *es2 = (Entry[]) {
  |^
dict.c:16:24: note: (near initialization for ‘es2’)

(gcc (GCC) 10.2.1 20201005 (Red Hat 10.2.1-5))

-- 
Eduardo

Re: [RFC 08/15] target/riscv: rvb: single-bit instructions

On 11/18/20 12:29 AM, frank.ch...@sifive.com wrote:
> +static bool trans_sbset(DisasContext *ctx, arg_sbset *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith(ctx, a, _sbset);
> +}
> +
> +static bool trans_sbseti(DisasContext *ctx, arg_sbseti *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith_shamt_tl(ctx, a, _sbset);
> +}
> +
> +static bool trans_sbclr(DisasContext *ctx, arg_sbclr *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith(ctx, a, _sbclr);
> +}

Coming back to my re-use of code thing, these should use gen_shift.  That
handles the truncate of source2 to the shift amount.

> +static bool trans_sbclri(DisasContext *ctx, arg_sbclri *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith_shamt_tl(ctx, a, _sbclr);
> +}
> +
> +static bool trans_sbinv(DisasContext *ctx, arg_sbinv *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith(ctx, a, _sbinv);
> +}
> +
> +static bool trans_sbinvi(DisasContext *ctx, arg_sbinvi *a)
> +{
> +REQUIRE_EXT(ctx, RVB);
> +return gen_arith_shamt_tl(ctx, a, _sbinv);
> +}

I think there ought to be a gen_shifti for these.

Similarly gen_shiftiw and gen_shiftw, which would truncate and sign-extend the
result.  Existing code in trans_rvi.c.inc should be converted to use the new
functions.


r~

Re: [PATCH v1 6/9] softmmu/physmem: Don't use atomic operations in ram_block_discard_(disable|require)

2020-11-19 Thread Peter Xu

On Thu, Nov 19, 2020 at 04:39:15PM +0100, David Hildenbrand wrote:
>  int ram_block_discard_disable(bool state)
>  {
> -int old;
> +int ret = 0;
>  
> +ram_block_discard_disable_mutex_lock();
>  if (!state) {
> -qatomic_dec(_block_discard_disabled);
> -return 0;
> +ram_block_discard_disablers--;
> +} else if (!ram_block_discard_requirers) {
> +ram_block_discard_disablers++;
> +} else {
> +ret = -EBUSY;
>  }

I would make things even easier by:

  if (ram_block_discard_is_required()) {
return -EBUSY;
  }

  if (state) {
ram_block_discard_disablers++;
  } else {
ram_block_discard_disablers--;
  }

But I think it's kind of nitpicking. :)

Reviewed-by: Peter Xu 

Thanks for writing this patch.

-- 
Peter Xu

Re: iotest 030 still occasionally intermittently failing

2020-11-19 Thread Vladimir Sementsov-Ogievskiy


19.11.2020 22:31, Vladimir Sementsov-Ogievskiy wrote:

19.11.2020 22:30, Vladimir Sementsov-Ogievskiy wrote:

19.11.2020 19:11, Vladimir Sementsov-Ogievskiy wrote:

16.11.2020 20:59, Peter Maydell wrote:

On Mon, 16 Nov 2020 at 17:34, Alberto Garcia  wrote:

Do you know if there is a core dump or stack trace available ?


Nope, sorry. What you get is what the 'vm-build-netbsd' etc targets
produce, so if you want more diagnostics on failures you have to
arrange for the test harness to produce them...

thanks
-- PMM



Hi!

After some iterations I've reproduced on SIGABRT:

#0  0x7feb701bae35 in raise () at /lib64/libc.so.6
#1  0x7feb701a5895 in abort () at /lib64/libc.so.6
#2  0x7feb701a5769 in _nl_load_domain.cold () at /lib64/libc.so.6
#3  0x7feb701b3566 in annobin_assert.c_end () at /lib64/libc.so.6
#4  0x55a93374f7d3 in bdrv_replace_child (child=0x55a9363a3a00, new_bs=0x0) 
at ../block.c:2648
#5  0x55a93374fd5a in bdrv_detach_child (child=0x55a9363a3a00) at 
../block.c:2777
#6  0x55a93374fd9c in bdrv_root_unref_child (child=0x55a9363a3a00) at 
../block.c:2789
#7  0x55a933722e8b in block_job_remove_all_bdrv (job=0x55a935f4f4b0) at 
../blockjob.c:191
#8  0x55a933722bb2 in block_job_free (job=0x55a935f4f4b0) at 
../blockjob.c:88
#9  0x55a9337755fa in job_unref (job=0x55a935f4f4b0) at ../job.c:380
#10 0x55a9337767a6 in job_exit (opaque=0x55a935f4f4b0) at ../job.c:894
#11 0x55a93386037e in aio_bh_call (bh=0x55a9352e16b0) at ../util/async.c:136
#12 0x55a933860488 in aio_bh_poll (ctx=0x55a9351366f0) at 
../util/async.c:164
#13 0x55a93383151e in aio_dispatch (ctx=0x55a9351366f0) at 
../util/aio-posix.c:381
#14 0x55a9338608b9 in aio_ctx_dispatch (source=0x55a9351366f0, 
callback=0x0, user_data=0x0)
 at ../util/async.c:306
#15 0x7feb71349ecd in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#16 0x55a933840300 in glib_pollfds_poll () at ../util/main-loop.c:221
#17 0x55a93384037a in os_host_main_loop_wait (timeout=0) at 
../util/main-loop.c:244
#18 0x55a933840482 in main_loop_wait (nonblocking=0) at 
../util/main-loop.c:520
#19 0x55a933603979 in qemu_main_loop () at ../softmmu/vl.c:1678
#20 0x55a933190046 in main (argc=20, argv=0x7ffd48c31138, 
envp=0x7ffd48c311e0)

(gdb) fr 4
#4  0x55a93374f7d3 in bdrv_replace_child (child=0x55a9363a3a00, new_bs=0x0) 
at ../block.c:2648
2648    assert(tighten_restrictions == false);
(gdb) list
2643    int ret;
2644
2645    bdrv_get_cumulative_perm(old_bs, , _perm);
2646    ret = bdrv_check_perm(old_bs, NULL, perm, shared_perm, NULL,
2647  _restrictions, NULL);
2648    assert(tighten_restrictions == false);
2649    if (ret < 0) {
2650    /* We only tried to loosen restrictions, so errors are not 
fatal */
2651    bdrv_abort_perm_update(old_bs);
2652    } else {
(gdb) p tighten_restrictions
$1 = true




I've modified code a bit, to crash when we actually want to set 
tighten_restrictions to true, and get the following bt:
#0  0x7f6dbb49ee35 in raise () at /lib64/libc.so.6
#1  0x7f6dbb489895 in abort () at /lib64/libc.so.6
#2  0x55b9174104d7 in bdrv_check_perm
 (bs=0x55b918f09720, q=0x0, cumulative_perms=1, cumulative_shared_perms=21, 
ignore_children=0x55b918a57b20 = {...}, tighten_restrictions=0x55b917b044f8 
, errp=0x0) at ../block.c:2009
#3  0x55b917410ec0 in bdrv_check_update_perm
 (bs=0x55b918f09720, q=0x0, new_used_perm=1, new_shared_perm=21, 
ignore_children=0x55b918a57b20 = {...}, tighten_restrictions=0x55b917b044f8 
, errp=0x0) at ../block.c:2280
#4  0x55b917410f38 in bdrv_child_check_perm
 (c=0x55b91921fcf0, q=0x0, perm=1, shared=21, ignore_children=0x55b918a57b20 = 
{...}, tighten_restrictions=0x55b917b044f8 , errp=0x0) at 
../block.c:2294
#5  0x55b91741078c in bdrv_check_perm
 (bs=0x55b918defd90, q=0x0, cumulative_perms=1, cumulative_shared_perms=21, 
ignore_children=0x0, tighten_restrictions=0x55b917b044f8 
, errp=0x0) at ../block.c:2076
#6  0x55b91741194e in bdrv_replace_child (child=0x55b919cf6200, new_bs=0x0) 
at ../block.c:2666
#7  0x55b917411f1d in bdrv_detach_child (child=0x55b919cf6200) at 
../block.c:2798
#8  0x55b917411f5f in bdrv_root_unref_child (child=0x55b919cf6200) at 
../block.c:2810
#9  0x55b9173e4d88 in block_job_remove_all_bdrv (job=0x55b918f06a60) at 
../blockjob.c:191
#10 0x55b9173e4aaf in block_job_free (job=0x55b918f06a60) at 
../blockjob.c:88
#11 0x55b917437aca in job_unref (job=0x55b918f06a60) at ../job.c:380
#12 0x55b917438c76 in job_exit (opaque=0x55b918f06a60) at ../job.c:894
#13 0x55b917522a57 in aio_bh_call (bh=0x55b919a2b3b0) at ../util/async.c:136
#14 0x55b917522b61 in aio_bh_poll (ctx=0x55b918a866f0) at 
../util/async.c:164
#15 0x55b9174f3bf7 in aio_dispatch (ctx=0x55b918a866f0) at 
../util/aio-posix.c:381
#16 0x55b917522f92 in aio_ctx_dispatch

[Bug 1815721] Re: RISC-V PLIC enable interrupt for multicore

2020-11-19 Thread Alistair Francis

I'm going to close this bug as it seems like the issue that RTOS Pharos
raised is not an issue.

@Teodori Serge please open a new issue if you have a bug. Make sure to
include as much detail as possible and steps to reproduce it.

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

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

Title:
  RISC-V PLIC enable interrupt for multicore

Status in QEMU:
  Invalid

Bug description:
  Hello all,

  There is a bug in Qemu related to the enabling of external interrupts
  for multicores (Virt machine).

  After correcting Qemu as described in #1815078
  (https://bugs.launchpad.net/qemu/+bug/1815078), when we try to enable
  interrupts for core 1 at address 0x0C00_2080 we don't seem to be able
  to trigger an external interrupt  (e.g. UART0).

  This works perfectly for core 0, but fore core 1 it does not work at
  all. I assume that given bug #1815078 does not enable any external
  interrupt then this feature has not been tested. I tried to look at
  the qemu source code but with no luck so far.

  I guess the problem is related to function parse_hart_config (in
  sfive_plic.c) that initializes incorrectly the
  plic->addr_config[addrid].hartid, which is later on read in
  sifive_plic_update. But this is a guess.

  Best regards,
  Pharos team

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

[Bug 1788665] Re: Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM using "Spectre" protection

2020-11-19 Thread Heiko Sieger

I've switched hosts so I would have to run a series of tests to compare.

There are a number of new variables:

1. Windows 10 release (I'm now on Windows 2004)
2. My host OS is now Manjaro
3. CPU is now AMD Ryzen 3900X (before it was Intel 3930k)
4. Kernel is 5.8.18-1-MANJARO
5. qemu 5.1.0
6. libvirt 6.5.0
7. New VM configuration using virt-manager/libvirt with EPYC-IBPB model instead
of host-passthrough, instead of a qemu bash script to launch the VM.

Time permitting, I plan to replace Manjaro for a Ubuntu 20.04 based
distro. But this will not happen in the very near future.

In the meantime I will do some a/b tests with spectre protection under
Windows enabled/disabled to see if it is still an issue.

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

Title:
Low 2D graphics performance with Windows 10 (1803) VGA passthrough VM
using "Spectre" protection

Status in QEMU:
Incomplete

Bug description:
Windows 10 (1803) VM using VGA passthrough via qemu script.

After upgrading Windows 10 Pro VM to version 1803, or possibly after
applying the March/April security updates from Microsoft, the VM would
show low 2D graphics performance (sluggishness in 2D applications and
low Passmark results).

Turning off Spectre vulnerability protection in Windows remedies the
issue.

Expected behavior:
qemu/kvm hypervisor to expose firmware capabilities of host to guest OS - see
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/CVE-2017-5715-and-hyper-v-vms

Background:

Starting in March or April Microsoft began to push driver updates in
their updates / security updates. See https://support.microsoft.com
/en-us/help/4073757/protect-your-windows-devices-against-spectre-
meltdown

One update concerns the Intel microcode - see
https://support.microsoft.com/en-us/help/4100347. It is activated by
default within Windows.

Once the updates are applied within the Windows guest, 2D graphics
performance drops significantly. Other performance benchmarks are not
affected.

A bare metal Windows installation does not display a performance loss
after the update. See https://heiko-sieger.info/low-2d-graphics-
benchmark-with-windows-10-1803-kvm-vm/

Similar reports can be found here:

https://www.reddit.com/r/VFIO/comments/97unx4/passmark_lousy_2d_graphics_performance_on_windows/

Hardware:

6 core Intel Core i7-3930K (-MT-MCP-)

Host OS:
Linux Mint 19/Ubuntu 18.04
Kernel: 4.15.0-32-generic x86_64
Qemu: QEMU emulator version 2.11.1
Intel microcode (host): 0x714
dmesg | grep microcode
[0.00] microcode: microcode updated early to revision 0x714, date =
2018-05-08
[2.810683] microcode: sig=0x206d7, pf=0x4, revision=0x714
[2.813340] microcode: Microcode Update Driver: v2.2.

Note: I manually updated the Intel microcode on the host from 0x713 to
0x714. However, both microcode versions produce the same result in the
Windows guest.

Guest OS:
Windows 10 Pro 64 bit, release 1803

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

[Bug 1885350] Re: RISCV dynamic rounding mode is not behaving correctly

2020-11-19 Thread Alistair Francis

As commented on the patch submission, this should already be handled:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg718331.html

Can you attach the test case that is failing?

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

Title:
  RISCV dynamic rounding mode is not behaving correctly

Status in QEMU:
  New

Bug description:
  Hello,

  I’ve gone through the RISC-V code in latest QEMU release
  (qemu-5.0.0-rc2) and when checking the Floating point encodings I
  found the rounding mode is only updated if the opcode field “rm” is
  changed “ctx->frm == rm”. But according to RISC-V Volume I:
  Unprivileged ISA, there’s a dynamic mode when rm=7 where the rounding
  mode is set with frm value.

  So for the same rm value (=7) and when changing frm value seeking
  different rounding modes, and according to the below code, the
  rounding mode won’t be updated. Please correct me if I got this
  implementation wrong.

  static void gen_set_rm(DisasContext *ctx, int rm)
  {
  TCGv_i32 t0;
  if (ctx->frm == rm) {
  return;
  }
  ctx->frm = rm;
  t0 = tcg_const_i32(rm);
  gen_helper_set_rounding_mode(cpu_env, t0);
  tcg_temp_free_i32(t0);
  }

  
  My testcase:
  I set statically the rm field in the instruction to 7 and before this 
execution I changed the value of frm field in fcsr register. For the 1st time 
it worked (according to the code above, the rm is updated so the round mode 
will also be updated). But when changing fcsr register an re-execute the 
instruction, there's no difference and the rounding mode is the same like the 
previous frm value.

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

Re: [PATCH 2/2] pc-bios: s390x: Give precedence to reset PSW