[linux-5.4 test] 180690: tolerable FAIL - PUSHED

2023-05-17 Thread osstest service owner
flight 180690 linux-5.4 real [real]
flight 180697 linux-5.4 real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180690/
http://logs.test-lab.xenproject.org/osstest/logs/180697/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-xsm20 guest-localmigrate/x10 fail pass in 180697-retest
 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start  fail pass in 180697-retest
 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail pass in 180697-retest
 test-armhf-armhf-xl-multivcpu 19 guest-start.2  fail pass in 180697-retest

Tests which did not succeed, but are not blocking:
 test-amd64-i386-libvirt-raw   7 xen-install  fail  like 180443
 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeatfail  like 180443
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180461
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180461
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180461
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180461
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180461
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180461
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180461
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180461
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180461
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180461
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180461
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180461
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never 

[ovmf test] 180695: all pass - PUSHED

2023-05-17 Thread osstest service owner
flight 180695 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180695/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 0abfb0be6cf78a8e962383e85cec57851ddae5bc
baseline version:
 ovmf cafb4f3f36e2101cab2ed6db3ea246a5a3e4708e

Last test of basis   180675  2023-05-15 21:40:50 Z2 days
Testing same since   180695  2023-05-18 00:10:44 Z0 days1 attempts


People who touched revisions under test:
  Andrei Warkentin 
  Gerd Hoffmann 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   cafb4f3f36..0abfb0be6c  0abfb0be6cf78a8e962383e85cec57851ddae5bc -> 
xen-tested-master



[xen-unstable-smoke test] 180694: tolerable all pass - PUSHED

2023-05-17 Thread osstest service owner
flight 180694 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180694/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  753d903a6f2d1e68d98487d36449b5739c28d65a
baseline version:
 xen  42abf5b9c53eb1b1a902002fcda68708234152c3

Last test of basis   180685  2023-05-16 20:03:38 Z1 days
Testing same since   180694  2023-05-18 00:03:29 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Luca Fancellu 
  Olaf Hering 
  Stefano Stabellini 
  Stefano Stabellini 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   42abf5b9c5..753d903a6f  753d903a6f2d1e68d98487d36449b5739c28d65a -> smoke



Re: [PATCH] xen: Fix host pci for stubdom

2023-05-17 Thread Jason Andryuk
On Mon, May 15, 2023 at 11:04 AM Anthony PERARD
 wrote:
>
> On Sun, Mar 19, 2023 at 08:05:54PM -0400, Jason Andryuk wrote:
> > diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
> > index 8c6e9a1716..51a72b432d 100644
> > --- a/hw/xen/xen-host-pci-device.c
> > +++ b/hw/xen/xen-host-pci-device.c
> > @@ -33,13 +34,101 @@
> >  #define IORESOURCE_PREFETCH 0x1000  /* No side effects */
> >  #define IORESOURCE_MEM_64   0x0010
> >
> > +/*
> > + * Non-passthrough (dom0) accesses are local PCI devices and use the given 
> > BDF
> > + * Passthough (stubdom) accesses are through PV frontend PCI device.  Those
> > + * either have a BDF identical to the backend's BFD 
> > (xen-backend.passthrough=1)
> > + * or a local virtual BDF (xen-backend.passthrough=0)
> > + *
> > + * We are always given the backend's BDF and need to lookup the appropriate
> > + * local BDF for sysfs access.
> > + */
> > +static void xen_host_pci_fill_local_addr(XenHostPCIDevice *d, Error **errp)
> > +{
> > +unsigned int num_devs, len, i;
> > +unsigned int domain, bus, dev, func;
> > +char *be_path;
> > +char path[80];
> > +char *msg;
> > +
> > +be_path = qemu_xen_xs_read(xenstore, 0, "device/pci/0/backend", );
> > +if (!be_path) {
> > +/*
> > + * be_path doesn't exist, so we are dealing with a local
> > + * (non-passthough) device.
> > + */
> > +d->local_domain = d->domain;
> > +d->local_bus = d->bus;
> > +d->local_dev = d->dev;
> > +d->local_func = d->func;
> > +
> > +return;
> > +}
> > +
> > +snprintf(path, sizeof(path), "%s/num_devs", be_path);
>
> Is 80 bytes for `path` enough?
> What if the path is truncated due to the limit?
>
>
> There's xs_node_scanf() which might be useful. It does the error
> handling and call scanf(). But I'm not sure if it can be used here, in
> this file.

Thanks for the suggestion - I'll take a look.  Your other comments
sound good, too.

Also, for the next version, I plan to change the From: to Marek. I was
thinking of doing it earlier, but failed to do so when it was time to
send out the patch.  Most of the code is Marek's from his Qubes
stubdom repo.  My modifications were to make it work with non-stubdom
as well.

Thanks,
Jason



PVH Dom0 related UART failure

2023-05-17 Thread Stefano Stabellini
Hi all,

I have run into another PVH Dom0 issue. I am trying to enable a PVH Dom0
test with the brand new gitlab-ci runner offered by Qubes. It is an AMD
Zen3 system and we already have a few successful tests with it, see
automation/gitlab-ci/test.yaml.

We managed to narrow down the issue to a console problem. We are
currently using console=com1 com1=115200,8n1,pci,msi as Xen command line
options, it works with PV Dom0 and it is using a PCI UART card.

In the case of Dom0 PVH:
- it works without console=com1
- it works with console=com1 and with the patch appended below
- it doesn't work otherwise and crashes with this error:
https://matrix-client.matrix.org/_matrix/media/r0/download/invisiblethingslab.com/uzcmldIqHptFZuxqsJtviLZK

What is the right way to fix it?

Keep in mind that I don't have access to the system except via gitlab-ci
pipelines. Marek (CCed) might have more info on the system and the PCI
UART he is using in case it's needed.

Many thanks for any help you can provide.

Cheers,

Stefano


diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 212a9c49ae..57623bc091 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -429,17 +428,6 @@ static void __init cf_check ns16550_init_postirq(struct 
serial_port *port)
 #ifdef NS16550_PCI
 if ( uart->bar || uart->ps_bdf_enable )
 {
-if ( uart->param && uart->param->mmio &&
- rangeset_add_range(mmio_ro_ranges, PFN_DOWN(uart->io_base),
-PFN_UP(uart->io_base + uart->io_size) - 1) )
-printk(XENLOG_INFO "Error while adding MMIO range of device to 
mmio_ro_ranges\n");
-
-if ( pci_ro_device(0, uart->ps_bdf[0],
-   PCI_DEVFN(uart->ps_bdf[1], uart->ps_bdf[2])) )
-printk(XENLOG_INFO "Could not mark config space of %02x:%02x.%u 
read-only.\n",
-   uart->ps_bdf[0], uart->ps_bdf[1],
-   uart->ps_bdf[2]);
-
 if ( uart->msi )
 {
 struct msi_info msi = {



[xen-unstable test] 180689: tolerable FAIL - PUSHED

2023-05-17 Thread osstest service owner
flight 180689 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180689/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180682
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180682
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180682
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180682
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180682
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180682
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180682
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180682
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180682
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180682
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180682
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180682
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  42abf5b9c53eb1b1a902002fcda68708234152c3
baseline version:
 xen  8f9c8274a4e3e860bd777269cb2c91971e9fa69e

Last test of basis   180682  2023-05-16 15:10:00 Z1 days
Testing same since   180689  2023-05-17 06:37:20 Z0 days1 attempts


People who touched revisions under test:
  Alejandro Vallejo 
  Anthony PERARD 
  Jason Andryuk 

[PATCH 0/6] block: add blk_io_plug_call() API

2023-05-17 Thread Stefan Hajnoczi
The existing blk_io_plug() API is not block layer multi-queue friendly because
the plug state is per-BlockDriverState.

Change blk_io_plug()'s implementation so it is thread-local. This is done by
introducing the blk_io_plug_call() function that block drivers use to batch
calls while plugged. It is relatively easy to convert block drivers from
.bdrv_co_io_plug() to blk_io_plug_call().

Random read 4KB performance with virtio-blk on a host NVMe block device:

iodepth   iops   change vs today
145612   -4%
287967   +2%
4   129872   +0%
8   171096   -3%
16  194508   -4%
32  208947   -1%
64  217647   +0%
128 229629   +0%

The results are within the noise for these benchmarks. This is to be expected
because the plugging behavior for a single thread hasn't changed in this patch
series, only that the state is thread-local now.

The following graph compares several approaches:
https://vmsplice.net/~stefan/blk_io_plug-thread-local.png
- v7.2.0: before most of the multi-queue block layer changes landed.
- with-blk_io_plug: today's post-8.0.0 QEMU.
- blk_io_plug-thread-local: this patch series.
- no-blk_io_plug: what happens when we simply remove plugging?
- call-after-dispatch: what if we integrate plugging into the event loop? I
  decided against this approach in the end because it's more likely to
  introduce performance regressions since I/O submission is deferred until the
  end of the event loop iteration.

Aside from the no-blk_io_plug case, which bottlenecks much earlier than the
others, we see that all plugging approaches are more or less equivalent in this
benchmark. It is also clear that QEMU 8.0.0 has lower performance than 7.2.0.

The Ansible playbook, fio results, and a Jupyter notebook are available here:
https://github.com/stefanha/qemu-perf/tree/remove-blk_io_plug

Stefan Hajnoczi (6):
  block: add blk_io_plug_call() API
  block/nvme: convert to blk_io_plug_call() API
  block/blkio: convert to blk_io_plug_call() API
  block/io_uring: convert to blk_io_plug_call() API
  block/linux-aio: convert to blk_io_plug_call() API
  block: remove bdrv_co_io_plug() API

 MAINTAINERS   |   1 +
 include/block/block-io.h  |   3 -
 include/block/block_int-common.h  |  11 ---
 include/block/raw-aio.h   |  14 ---
 include/sysemu/block-backend-io.h |  13 +--
 block/blkio.c |  40 
 block/block-backend.c |  22 -
 block/file-posix.c|  38 ---
 block/io.c|  37 ---
 block/io_uring.c  |  45 -
 block/linux-aio.c |  41 +++-
 block/nvme.c  |  44 +++--
 block/plug.c  | 159 ++
 hw/block/dataplane/xen-block.c|   8 +-
 hw/block/virtio-blk.c |   4 +-
 hw/scsi/virtio-scsi.c |   6 +-
 block/meson.build |   1 +
 block/trace-events|   5 +-
 18 files changed, 236 insertions(+), 256 deletions(-)
 create mode 100644 block/plug.c

-- 
2.40.1




[PATCH 4/6] block/io_uring: convert to blk_io_plug_call() API

2023-05-17 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
---
 include/block/raw-aio.h |  7 ---
 block/file-posix.c  | 10 -
 block/io_uring.c| 45 -
 block/trace-events  |  5 ++---
 4 files changed, 19 insertions(+), 48 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index 0fe85ade77..da60ca13ef 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -81,13 +81,6 @@ int coroutine_fn luring_co_submit(BlockDriverState *bs, int 
fd, uint64_t offset,
   QEMUIOVector *qiov, int type);
 void luring_detach_aio_context(LuringState *s, AioContext *old_context);
 void luring_attach_aio_context(LuringState *s, AioContext *new_context);
-
-/*
- * luring_io_plug/unplug work in the thread's current AioContext, therefore the
- * caller must ensure that they are paired in the same IOThread.
- */
-void luring_io_plug(void);
-void luring_io_unplug(void);
 #endif
 
 #ifdef _WIN32
diff --git a/block/file-posix.c b/block/file-posix.c
index 0ab158efba..7baa8491dd 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2558,11 +2558,6 @@ static void coroutine_fn raw_co_io_plug(BlockDriverState 
*bs)
 laio_io_plug();
 }
 #endif
-#ifdef CONFIG_LINUX_IO_URING
-if (s->use_linux_io_uring) {
-luring_io_plug();
-}
-#endif
 }
 
 static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
@@ -2573,11 +2568,6 @@ static void coroutine_fn 
raw_co_io_unplug(BlockDriverState *bs)
 laio_io_unplug(s->aio_max_batch);
 }
 #endif
-#ifdef CONFIG_LINUX_IO_URING
-if (s->use_linux_io_uring) {
-luring_io_unplug();
-}
-#endif
 }
 
 static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
diff --git a/block/io_uring.c b/block/io_uring.c
index 82cab6a5bd..9a45e5fb8b 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -16,6 +16,7 @@
 #include "block/raw-aio.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
+#include "sysemu/block-backend.h"
 #include "trace.h"
 
 /* Only used for assertions.  */
@@ -41,7 +42,6 @@ typedef struct LuringAIOCB {
 } LuringAIOCB;
 
 typedef struct LuringQueue {
-int plugged;
 unsigned int in_queue;
 unsigned int in_flight;
 bool blocked;
@@ -267,7 +267,7 @@ static void 
luring_process_completions_and_submit(LuringState *s)
 {
 luring_process_completions(s);
 
-if (!s->io_q.plugged && s->io_q.in_queue > 0) {
+if (s->io_q.in_queue > 0) {
 ioq_submit(s);
 }
 }
@@ -301,29 +301,17 @@ static void qemu_luring_poll_ready(void *opaque)
 static void ioq_init(LuringQueue *io_q)
 {
 QSIMPLEQ_INIT(_q->submit_queue);
-io_q->plugged = 0;
 io_q->in_queue = 0;
 io_q->in_flight = 0;
 io_q->blocked = false;
 }
 
-void luring_io_plug(void)
+static void luring_unplug_fn(void *opaque)
 {
-AioContext *ctx = qemu_get_current_aio_context();
-LuringState *s = aio_get_linux_io_uring(ctx);
-trace_luring_io_plug(s);
-s->io_q.plugged++;
-}
-
-void luring_io_unplug(void)
-{
-AioContext *ctx = qemu_get_current_aio_context();
-LuringState *s = aio_get_linux_io_uring(ctx);
-assert(s->io_q.plugged);
-trace_luring_io_unplug(s, s->io_q.blocked, s->io_q.plugged,
-   s->io_q.in_queue, s->io_q.in_flight);
-if (--s->io_q.plugged == 0 &&
-!s->io_q.blocked && s->io_q.in_queue > 0) {
+LuringState *s = opaque;
+trace_luring_unplug_fn(s, s->io_q.blocked, s->io_q.in_queue,
+   s->io_q.in_flight);
+if (!s->io_q.blocked && s->io_q.in_queue > 0) {
 ioq_submit(s);
 }
 }
@@ -337,7 +325,6 @@ void luring_io_unplug(void)
  * @type: type of request
  *
  * Fetches sqes from ring, adds to pending queue and preps them
- *
  */
 static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s,
 uint64_t offset, int type)
@@ -370,14 +357,16 @@ static int luring_do_submit(int fd, LuringAIOCB 
*luringcb, LuringState *s,
 
 QSIMPLEQ_INSERT_TAIL(>io_q.submit_queue, luringcb, next);
 s->io_q.in_queue++;
-trace_luring_do_submit(s, s->io_q.blocked, s->io_q.plugged,
-   s->io_q.in_queue, s->io_q.in_flight);
-if (!s->io_q.blocked &&
-(!s->io_q.plugged ||
- s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES)) {
-ret = ioq_submit(s);
-trace_luring_do_submit_done(s, ret);
-return ret;
+trace_luring_do_submit(s, s->io_q.blocked, s->io_q.in_queue,
+   s->io_q.in_flight);
+if (!s->io_q.blocked) {
+if (s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES) {
+ret = ioq_submit(s);
+trace_luring_do_submit_done(s, ret);
+return ret;
+}
+
+blk_io_plug_call(luring_unplug_fn, 

[PATCH 5/6] block/linux-aio: convert to blk_io_plug_call() API

2023-05-17 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
---
 include/block/raw-aio.h |  7 ---
 block/file-posix.c  | 28 
 block/linux-aio.c   | 41 +++--
 3 files changed, 11 insertions(+), 65 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index da60ca13ef..0f63c2800c 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -62,13 +62,6 @@ int coroutine_fn laio_co_submit(int fd, uint64_t offset, 
QEMUIOVector *qiov,
 
 void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context);
 void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context);
-
-/*
- * laio_io_plug/unplug work in the thread's current AioContext, therefore the
- * caller must ensure that they are paired in the same IOThread.
- */
-void laio_io_plug(void);
-void laio_io_unplug(uint64_t dev_max_batch);
 #endif
 /* io_uring.c - Linux io_uring implementation */
 #ifdef CONFIG_LINUX_IO_URING
diff --git a/block/file-posix.c b/block/file-posix.c
index 7baa8491dd..ac1ed54811 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2550,26 +2550,6 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState 
*bs, int64_t offset,
 return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE);
 }
 
-static void coroutine_fn raw_co_io_plug(BlockDriverState *bs)
-{
-BDRVRawState __attribute__((unused)) *s = bs->opaque;
-#ifdef CONFIG_LINUX_AIO
-if (s->use_linux_aio) {
-laio_io_plug();
-}
-#endif
-}
-
-static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs)
-{
-BDRVRawState __attribute__((unused)) *s = bs->opaque;
-#ifdef CONFIG_LINUX_AIO
-if (s->use_linux_aio) {
-laio_io_unplug(s->aio_max_batch);
-}
-#endif
-}
-
 static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs)
 {
 BDRVRawState *s = bs->opaque;
@@ -3914,8 +3894,6 @@ BlockDriver bdrv_file = {
 .bdrv_co_copy_range_from = raw_co_copy_range_from,
 .bdrv_co_copy_range_to  = raw_co_copy_range_to,
 .bdrv_refresh_limits = raw_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
@@ -4286,8 +4264,6 @@ static BlockDriver bdrv_host_device = {
 .bdrv_co_copy_range_from = raw_co_copy_range_from,
 .bdrv_co_copy_range_to  = raw_co_copy_range_to,
 .bdrv_refresh_limits = raw_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
@@ -4424,8 +4400,6 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_co_pwritev= raw_co_pwritev,
 .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
 .bdrv_refresh_limits= cdrom_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
@@ -4552,8 +4526,6 @@ static BlockDriver bdrv_host_cdrom = {
 .bdrv_co_pwritev= raw_co_pwritev,
 .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
 .bdrv_refresh_limits= cdrom_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
 .bdrv_attach_aio_context = raw_aio_attach_aio_context,
 
 .bdrv_co_truncate   = raw_co_truncate,
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 442c86209b..5021aed68f 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -15,6 +15,7 @@
 #include "qemu/event_notifier.h"
 #include "qemu/coroutine.h"
 #include "qapi/error.h"
+#include "sysemu/block-backend.h"
 
 /* Only used for assertions.  */
 #include "qemu/coroutine_int.h"
@@ -46,7 +47,6 @@ struct qemu_laiocb {
 };
 
 typedef struct {
-int plugged;
 unsigned int in_queue;
 unsigned int in_flight;
 bool blocked;
@@ -236,7 +236,7 @@ static void 
qemu_laio_process_completions_and_submit(LinuxAioState *s)
 {
 qemu_laio_process_completions(s);
 
-if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(>io_q.pending)) {
+if (!QSIMPLEQ_EMPTY(>io_q.pending)) {
 ioq_submit(s);
 }
 }
@@ -277,7 +277,6 @@ static void qemu_laio_poll_ready(EventNotifier *opaque)
 static void ioq_init(LaioQueue *io_q)
 {
 QSIMPLEQ_INIT(_q->pending);
-io_q->plugged = 0;
 io_q->in_queue = 0;
 io_q->in_flight = 0;
 io_q->blocked = false;
@@ -354,31 +353,11 @@ static uint64_t laio_max_batch(LinuxAioState *s, uint64_t 
dev_max_batch)
 return max_batch;
 }
 
-void laio_io_plug(void)
+static void laio_unplug_fn(void *opaque)
 {
-AioContext *ctx = 

[PATCH 6/6] block: remove bdrv_co_io_plug() API

2023-05-17 Thread Stefan Hajnoczi
No block driver implements .bdrv_co_io_plug() anymore. Get rid of the
function pointers.

Signed-off-by: Stefan Hajnoczi 
---
 include/block/block-io.h |  3 ---
 include/block/block_int-common.h | 11 --
 block/io.c   | 37 
 3 files changed, 51 deletions(-)

diff --git a/include/block/block-io.h b/include/block/block-io.h
index a27e471a87..43af816d75 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -259,9 +259,6 @@ void coroutine_fn bdrv_co_leave(BlockDriverState *bs, 
AioContext *old_ctx);
 
 AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c);
 
-void coroutine_fn GRAPH_RDLOCK bdrv_co_io_plug(BlockDriverState *bs);
-void coroutine_fn GRAPH_RDLOCK bdrv_co_io_unplug(BlockDriverState *bs);
-
 bool coroutine_fn GRAPH_RDLOCK
 bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name,
uint32_t granularity, Error **errp);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index dbec0e3bb4..fa369d83dd 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -753,11 +753,6 @@ struct BlockDriver {
 void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_debug_event)(
 BlockDriverState *bs, BlkdebugEvent event);
 
-/* io queue for linux-aio */
-void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_plug)(BlockDriverState 
*bs);
-void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_unplug)(
-BlockDriverState *bs);
-
 /**
  * bdrv_drain_begin is called if implemented in the beginning of a
  * drain operation to drain and stop any internal sources of requests in
@@ -1227,12 +1222,6 @@ struct BlockDriverState {
 unsigned int in_flight;
 unsigned int serialising_in_flight;
 
-/*
- * counter for nested bdrv_io_plug.
- * Accessed with atomic ops.
- */
-unsigned io_plugged;
-
 /* do we need to tell the quest if we have a volatile write cache? */
 int enable_write_cache;
 
diff --git a/block/io.c b/block/io.c
index 4d54fda593..56b0c1ce6c 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3219,43 +3219,6 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t 
size)
 return mem;
 }
 
-void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs)
-{
-BdrvChild *child;
-IO_CODE();
-assert_bdrv_graph_readable();
-
-QLIST_FOREACH(child, >children, next) {
-bdrv_co_io_plug(child->bs);
-}
-
-if (qatomic_fetch_inc(>io_plugged) == 0) {
-BlockDriver *drv = bs->drv;
-if (drv && drv->bdrv_co_io_plug) {
-drv->bdrv_co_io_plug(bs);
-}
-}
-}
-
-void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs)
-{
-BdrvChild *child;
-IO_CODE();
-assert_bdrv_graph_readable();
-
-assert(bs->io_plugged);
-if (qatomic_fetch_dec(>io_plugged) == 1) {
-BlockDriver *drv = bs->drv;
-if (drv && drv->bdrv_co_io_unplug) {
-drv->bdrv_co_io_unplug(bs);
-}
-}
-
-QLIST_FOREACH(child, >children, next) {
-bdrv_co_io_unplug(child->bs);
-}
-}
-
 /* Helper that undoes bdrv_register_buf() when it fails partway through */
 static void GRAPH_RDLOCK
 bdrv_register_buf_rollback(BlockDriverState *bs, void *host, size_t size,
-- 
2.40.1




[PATCH 1/6] block: add blk_io_plug_call() API

2023-05-17 Thread Stefan Hajnoczi
Introduce a new API for thread-local blk_io_plug() that does not
traverse the block graph. The goal is to make blk_io_plug() multi-queue
friendly.

Instead of having block drivers track whether or not we're in a plugged
section, provide an API that allows them to defer a function call until
we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is
called multiple times with the same fn/opaque pair, then fn() is only
called once at the end of the function - resulting in batching.

This patch introduces the API and changes blk_io_plug()/blk_io_unplug().
blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument
because the plug state is now thread-local.

Later patches convert block drivers to blk_io_plug_call() and then we
can finally remove .bdrv_co_io_plug() once all block drivers have been
converted.

Signed-off-by: Stefan Hajnoczi 
---
 MAINTAINERS   |   1 +
 include/sysemu/block-backend-io.h |  13 +--
 block/block-backend.c |  22 -
 block/plug.c  | 159 ++
 hw/block/dataplane/xen-block.c|   8 +-
 hw/block/virtio-blk.c |   4 +-
 hw/scsi/virtio-scsi.c |   6 +-
 block/meson.build |   1 +
 8 files changed, 173 insertions(+), 41 deletions(-)
 create mode 100644 block/plug.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 50585117a0..574202295c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2644,6 +2644,7 @@ F: util/aio-*.c
 F: util/aio-*.h
 F: util/fdmon-*.c
 F: block/io.c
+F: block/plug.c
 F: migration/block*
 F: include/block/aio.h
 F: include/block/aio-wait.h
diff --git a/include/sysemu/block-backend-io.h 
b/include/sysemu/block-backend-io.h
index d62a7ee773..be4dcef59d 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -100,16 +100,9 @@ void blk_iostatus_set_err(BlockBackend *blk, int error);
 int blk_get_max_iov(BlockBackend *blk);
 int blk_get_max_hw_iov(BlockBackend *blk);
 
-/*
- * blk_io_plug/unplug are thread-local operations. This means that multiple
- * IOThreads can simultaneously call plug/unplug, but the caller must ensure
- * that each unplug() is called in the same IOThread of the matching plug().
- */
-void coroutine_fn blk_co_io_plug(BlockBackend *blk);
-void co_wrapper blk_io_plug(BlockBackend *blk);
-
-void coroutine_fn blk_co_io_unplug(BlockBackend *blk);
-void co_wrapper blk_io_unplug(BlockBackend *blk);
+void blk_io_plug(void);
+void blk_io_unplug(void);
+void blk_io_plug_call(void (*fn)(void *), void *opaque);
 
 AioContext *blk_get_aio_context(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index ca537cd0ad..1f1d226ba6 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2568,28 +2568,6 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, 
Notifier *notify)
 notifier_list_add(>insert_bs_notifiers, notify);
 }
 
-void coroutine_fn blk_co_io_plug(BlockBackend *blk)
-{
-BlockDriverState *bs = blk_bs(blk);
-IO_CODE();
-GRAPH_RDLOCK_GUARD();
-
-if (bs) {
-bdrv_co_io_plug(bs);
-}
-}
-
-void coroutine_fn blk_co_io_unplug(BlockBackend *blk)
-{
-BlockDriverState *bs = blk_bs(blk);
-IO_CODE();
-GRAPH_RDLOCK_GUARD();
-
-if (bs) {
-bdrv_co_io_unplug(bs);
-}
-}
-
 BlockAcctStats *blk_get_stats(BlockBackend *blk)
 {
 IO_CODE();
diff --git a/block/plug.c b/block/plug.c
new file mode 100644
index 00..6738a568ba
--- /dev/null
+++ b/block/plug.c
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Block I/O plugging
+ *
+ * Copyright Red Hat.
+ *
+ * This API defers a function call within a blk_io_plug()/blk_io_unplug()
+ * section, allowing multiple calls to batch up. This is a performance
+ * optimization that is used in the block layer to submit several I/O requests
+ * at once instead of individually:
+ *
+ *   blk_io_plug(); <-- start of plugged region
+ *   ...
+ *   blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call
+ *   blk_io_plug_call(my_func, my_obj); <-- another
+ *   blk_io_plug_call(my_func, my_obj); <-- another
+ *   ...
+ *   blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called once
+ *
+ * This code is actually generic and not tied to the block layer. If another
+ * subsystem needs this functionality, it could be renamed.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/coroutine-tls.h"
+#include "qemu/notify.h"
+#include "qemu/thread.h"
+#include "sysemu/block-backend.h"
+
+/* A function call that has been deferred until unplug() */
+typedef struct {
+void (*fn)(void *);
+void *opaque;
+} UnplugFn;
+
+/* Per-thread state */
+typedef struct {
+unsigned count;   /* how many times has plug() been called? */
+GArray *unplug_fns;   /* functions to call at unplug time */
+} Plug;
+
+/* Use get_ptr_plug() to fetch this thread-local value */

[PATCH 3/6] block/blkio: convert to blk_io_plug_call() API

2023-05-17 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
---
 block/blkio.c | 40 +---
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/block/blkio.c b/block/blkio.c
index 0cdc99a729..f2a1dc1fb2 100644
--- a/block/blkio.c
+++ b/block/blkio.c
@@ -325,16 +325,28 @@ static void blkio_detach_aio_context(BlockDriverState *bs)
false, NULL, NULL, NULL, NULL, NULL);
 }
 
-/* Call with s->blkio_lock held to submit I/O after enqueuing a new request */
-static void blkio_submit_io(BlockDriverState *bs)
+/*
+ * Called by blk_io_unplug() or immediately if not plugged. Called without
+ * blkio_lock.
+ */
+static void blkio_unplug_fn(BlockDriverState *bs)
 {
-if (qatomic_read(>io_plugged) == 0) {
-BDRVBlkioState *s = bs->opaque;
+BDRVBlkioState *s = bs->opaque;
 
+WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_do_io(s->blkioq, NULL, 0, 0, NULL);
 }
 }
 
+/*
+ * Schedule I/O submission after enqueuing a new request. Called without
+ * blkio_lock.
+ */
+static void blkio_submit_io(BlockDriverState *bs)
+{
+blk_io_plug_call(blkio_unplug_fn, bs);
+}
+
 static int coroutine_fn
 blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes)
 {
@@ -345,9 +357,9 @@ blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, 
int64_t bytes)
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_discard(s->blkioq, offset, bytes, , 0);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 return cod.ret;
 }
@@ -378,9 +390,9 @@ blkio_co_preadv(BlockDriverState *bs, int64_t offset, 
int64_t bytes,
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_readv(s->blkioq, offset, iov, iovcnt, , 0);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 
 if (use_bounce_buffer) {
@@ -423,9 +435,9 @@ static int coroutine_fn blkio_co_pwritev(BlockDriverState 
*bs, int64_t offset,
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_writev(s->blkioq, offset, iov, iovcnt, , blkio_flags);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 
 if (use_bounce_buffer) {
@@ -444,9 +456,9 @@ static int coroutine_fn blkio_co_flush(BlockDriverState *bs)
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_flush(s->blkioq, , 0);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 return cod.ret;
 }
@@ -472,22 +484,13 @@ static int coroutine_fn 
blkio_co_pwrite_zeroes(BlockDriverState *bs,
 
 WITH_QEMU_LOCK_GUARD(>blkio_lock) {
 blkioq_write_zeroes(s->blkioq, offset, bytes, , blkio_flags);
-blkio_submit_io(bs);
 }
 
+blkio_submit_io(bs);
 qemu_coroutine_yield();
 return cod.ret;
 }
 
-static void coroutine_fn blkio_co_io_unplug(BlockDriverState *bs)
-{
-BDRVBlkioState *s = bs->opaque;
-
-WITH_QEMU_LOCK_GUARD(>blkio_lock) {
-blkio_submit_io(bs);
-}
-}
-
 typedef enum {
 BMRR_OK,
 BMRR_SKIP,
@@ -1009,7 +1012,6 @@ static void blkio_refresh_limits(BlockDriverState *bs, 
Error **errp)
 .bdrv_co_pwritev = blkio_co_pwritev, \
 .bdrv_co_flush_to_disk   = blkio_co_flush, \
 .bdrv_co_pwrite_zeroes   = blkio_co_pwrite_zeroes, \
-.bdrv_co_io_unplug   = blkio_co_io_unplug, \
 .bdrv_refresh_limits = blkio_refresh_limits, \
 .bdrv_register_buf   = blkio_register_buf, \
 .bdrv_unregister_buf = blkio_unregister_buf, \
-- 
2.40.1




[PATCH 2/6] block/nvme: convert to blk_io_plug_call() API

2023-05-17 Thread Stefan Hajnoczi
Stop using the .bdrv_co_io_plug() API because it is not multi-queue
block layer friendly. Use the new blk_io_plug_call() API to batch I/O
submission instead.

Signed-off-by: Stefan Hajnoczi 
---
 block/nvme.c | 44 
 1 file changed, 12 insertions(+), 32 deletions(-)

diff --git a/block/nvme.c b/block/nvme.c
index 5b744c2bda..100b38b592 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -25,6 +25,7 @@
 #include "qemu/vfio-helpers.h"
 #include "block/block-io.h"
 #include "block/block_int.h"
+#include "sysemu/block-backend.h"
 #include "sysemu/replay.h"
 #include "trace.h"
 
@@ -119,7 +120,6 @@ struct BDRVNVMeState {
 int blkshift;
 
 uint64_t max_transfer;
-bool plugged;
 
 bool supports_write_zeroes;
 bool supports_discard;
@@ -282,7 +282,7 @@ static void nvme_kick(NVMeQueuePair *q)
 {
 BDRVNVMeState *s = q->s;
 
-if (s->plugged || !q->need_kick) {
+if (!q->need_kick) {
 return;
 }
 trace_nvme_kick(s, q->index);
@@ -387,10 +387,6 @@ static bool nvme_process_completion(NVMeQueuePair *q)
 NvmeCqe *c;
 
 trace_nvme_process_completion(s, q->index, q->inflight);
-if (s->plugged) {
-trace_nvme_process_completion_queue_plugged(s, q->index);
-return false;
-}
 
 /*
  * Support re-entrancy when a request cb() function invokes aio_poll().
@@ -480,6 +476,15 @@ static void nvme_trace_command(const NvmeCmd *cmd)
 }
 }
 
+static void nvme_unplug_fn(void *opaque)
+{
+NVMeQueuePair *q = opaque;
+
+QEMU_LOCK_GUARD(>lock);
+nvme_kick(q);
+nvme_process_completion(q);
+}
+
 static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
 NvmeCmd *cmd, BlockCompletionFunc cb,
 void *opaque)
@@ -496,8 +501,7 @@ static void nvme_submit_command(NVMeQueuePair *q, 
NVMeRequest *req,
q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));
 q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
 q->need_kick++;
-nvme_kick(q);
-nvme_process_completion(q);
+blk_io_plug_call(nvme_unplug_fn, q);
 qemu_mutex_unlock(>lock);
 }
 
@@ -1567,27 +1571,6 @@ static void nvme_attach_aio_context(BlockDriverState *bs,
 }
 }
 
-static void coroutine_fn nvme_co_io_plug(BlockDriverState *bs)
-{
-BDRVNVMeState *s = bs->opaque;
-assert(!s->plugged);
-s->plugged = true;
-}
-
-static void coroutine_fn nvme_co_io_unplug(BlockDriverState *bs)
-{
-BDRVNVMeState *s = bs->opaque;
-assert(s->plugged);
-s->plugged = false;
-for (unsigned i = INDEX_IO(0); i < s->queue_count; i++) {
-NVMeQueuePair *q = s->queues[i];
-qemu_mutex_lock(>lock);
-nvme_kick(q);
-nvme_process_completion(q);
-qemu_mutex_unlock(>lock);
-}
-}
-
 static bool nvme_register_buf(BlockDriverState *bs, void *host, size_t size,
   Error **errp)
 {
@@ -1664,9 +1647,6 @@ static BlockDriver bdrv_nvme = {
 .bdrv_detach_aio_context  = nvme_detach_aio_context,
 .bdrv_attach_aio_context  = nvme_attach_aio_context,
 
-.bdrv_co_io_plug  = nvme_co_io_plug,
-.bdrv_co_io_unplug= nvme_co_io_unplug,
-
 .bdrv_register_buf= nvme_register_buf,
 .bdrv_unregister_buf  = nvme_unregister_buf,
 };
-- 
2.40.1




Re: [PATCH 2/2] xen/x86/pvh: copy ACPI tables to Dom0 instead of mapping

2023-05-17 Thread Stefano Stabellini
On Wed, 17 May 2023, Roger Pau Monné wrote:
> On Tue, May 16, 2023 at 04:34:09PM -0700, Stefano Stabellini wrote:
> > On Tue, 16 May 2023, Roger Pau Monné wrote:
> > > On Mon, May 15, 2023 at 05:11:25PM -0700, Stefano Stabellini wrote:
> > > > On Mon, 15 May 2023, Roger Pau Monné wrote:
> > > > > On Fri, May 12, 2023 at 06:17:20PM -0700, Stefano Stabellini wrote:
> > > > > > From: Stefano Stabellini 
> > > > > > 
> > > > > > Mapping the ACPI tables to Dom0 PVH 1:1 leads to memory corruptions 
> > > > > > of
> > > > > > the tables in the guest. Instead, copy the tables to Dom0.
> > > > > > 
> > > > > > This is a workaround.
> > > > > > 
> > > > > > Signed-off-by: Stefano Stabellini 
> > > > > > ---
> > > > > > As mentioned in the cover letter, this is a RFC workaround as I 
> > > > > > don't
> > > > > > know the cause of the underlying problem. I do know that this patch
> > > > > > solves what would be otherwise a hang at boot when Dom0 PVH 
> > > > > > attempts to
> > > > > > parse ACPI tables.
> > > > > 
> > > > > I'm unsure how safe this is for native systems, as it's possible for
> > > > > firmware to modify the data in the tables, so copying them would
> > > > > break that functionality.
> > > > > 
> > > > > I think we need to get to the root cause that triggers this behavior
> > > > > on QEMU.  Is it the table checksum that fail, or something else?  Is
> > > > > there an error from Linux you could reference?
> > > > 
> > > > I agree with you but so far I haven't managed to find a way to the root
> > > > of the issue. Here is what I know. These are the logs of a successful
> > > > boot using this patch:
> > > > 
> > > > [   10.437488] ACPI: Early table checksum verification disabled
> > > > [   10.439345] ACPI: RSDP 0x4005F955 24 (v02 BOCHS )
> > > > [   10.441033] ACPI: RSDT 0x4005F979 34 (v01 BOCHS  
> > > > BXPCRSDT 0001 BXPC 0001)
> > > > [   10.444045] ACPI: APIC 0x40060F76 8A (v01 BOCHS  
> > > > BXPCAPIC 0001 BXPC 0001)
> > > > [   10.445984] ACPI: FACP 0x4005FA65 74 (v01 BOCHS  
> > > > BXPCFACP 0001 BXPC 0001)
> > > > [   10.447170] ACPI BIOS Warning (bug): Incorrect checksum in table 
> > > > [FACP] - 0x67, should be 0x30 (20220331/tbprint-174)
> > > > [   10.449522] ACPI: DSDT 0x4005FB19 00145D (v01 BOCHS  
> > > > BXPCDSDT 0001 BXPC 0001)
> > > > [   10.451258] ACPI: FACS 0x4005FAD9 40
> > > > [   10.452245] ACPI: Reserving APIC table memory at [mem 
> > > > 0x40060f76-0x40060fff]
> > > > [   10.452389] ACPI: Reserving FACP table memory at [mem 
> > > > 0x4005fa65-0x4005fad8]
> > > > [   10.452497] ACPI: Reserving DSDT table memory at [mem 
> > > > 0x4005fb19-0x40060f75]
> > > > [   10.452602] ACPI: Reserving FACS table memory at [mem 
> > > > 0x4005fad9-0x4005fb18]
> > > > 
> > > > 
> > > > And these are the logs of the same boot (unsuccessful) without this
> > > > patch:
> > > > 
> > > > [   10.516015] ACPI: Early table checksum verification disabled
> > > > [   10.517732] ACPI: RSDP 0x40060F1E 24 (v02 BOCHS )
> > > > [   10.519535] ACPI: RSDT 0x40060F42 34 (v01 BOCHS  
> > > > BXPCRSDT 0001 BXPC 0001)
> > > > [   10.522523] ACPI: APIC 0x40060F76 8A (v01 BOCHS  
> > > > BXPCAPIC 0001 BXPC 0001)
> > > > [   10.527453] ACPI:  0x7FFE149D  (v255 �� 
> > > >    )
> > > > [   10.528362] ACPI: Reserving APIC table memory at [mem 
> > > > 0x40060f76-0x40060fff]
> > > > [   10.528491] ACPI: Reserving  table memory at [mem 
> > > > 0x7ffe149d-0x17ffe149b]
> > > > 
> > > > It is clearly a memory corruption around FACS but I couldn't find the
> > > > reason for it. The mapping code looks correct. I hope you can suggest a
> > > > way to narrow down the problem. If I could, I would suggest to apply
> > > > this patch just for the QEMU PVH tests but we don't have the
> > > > infrastructure for that in gitlab-ci as there is a single Xen build for
> > > > all tests.
> > > 
> > > Would be helpful to see the memory map provided to Linux, just in case
> > > we messed up and there's some overlap.
> > 
> > Everything looks correct. Here are some more logs:
> > 
> > (XEN) Xen-e820 RAM map:
> > (XEN)  [, 0009fbff] (usable)
> > (XEN)  [0009fc00, 0009] (reserved)
> > (XEN)  [000f, 000f] (reserved)
> > (XEN)  [0010, 7ffd] (usable)
> > (XEN)  [7ffe, 7fff] (reserved)
> > (XEN)  [fffc, ] (reserved)
> > (XEN)  [00fd, 00ff] (reserved)
> > (XEN) Microcode loading not available
> > (XEN) New Xen image base address: 0x7f60
> > (XEN) System RAM: 2047MB (2096636kB)
> > (XEN) ACPI: RSDP 000F58D0, 0014 (r0 BOCHS )
> > (XEN) ACPI: RSDT 7FFE1B21, 0034 (r1 BOCHS  BXPC1 BXPC1)
> > (XEN) ACPI: FACP 7FFE19CD, 0074 (r1 BOCHS  BXPC   

Re: [PATCH] xen/pt: fix igd passthrough for pc machine with xen accelerator

2023-05-17 Thread Stefano Stabellini
On Wed, 17 May 2023, Michael Tokarev wrote:
> 17.05.2023 12:47, Chuck Zmudzinski wrote:
> > On 5/17/2023 2:39 AM, Michael Tokarev wrote:
> > > 08.02.2023 05:03, Chuck Zmudzinski wrote:...
> > > > Fixes: 998250e97661 ("xen, gfx passthrough: register host bridge
> > > > specific to passthrough")
> > > > Signed-off-by: Chuck Zmudzinski 
> > > 
> > > Has this change been forgotten?  Is it not needed anymore?
> > 
> > Short answer:
> > 
> > After 4f67543b ("xen/pt: reserve PCI slot 2 for Intel igd-passthru ") was
> > applied, I was inclined to think this change is not needed anymore, but
> > it would not hurt to add this change also, and now I think it might be
> > more correct to also add this change.
> ...
> 
> Well, there were two machines with broken IDG passthrough in xen, now
> there's one machine with broken IDG passthrough. Let's fix them all :)
> Note this patch is tagged -stable as well.
> 
> > If you want to add this change also, let's make sure recent changes to the
> > xen header files do not require the patch to be rebased before committing
> > it.
> 
> It doesn't require rebasing, it looks like, - just built 8.0 and current
> master
> qemu with it applied.  I haven't tried the actual IDG passthrough, though.
> 
> It just neeeds to be picked up the usual way as all other qemu changes goes
> in.

Hi Michal,

I am OK with this patch and acked it. However, I think it also needs an
ack from one if the i386 maintainers, Michal T or Marcel.



Re: [PATCH] xen: xen_debug_interrupt prototype to global header

2023-05-17 Thread Stefano Stabellini
On Wed, 17 May 2023, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> The xen_debug_interrupt() function is only called on x86, which has a
> prototype in an architecture specific header, but the definition also
> exists on others, where the lack of a prototype causes a W=1 warning:
> 
> drivers/xen/events/events_2l.c:264:13: error: no previous prototype for 
> 'xen_debug_interrupt' [-Werror=missing-prototypes]
> 
> Move the prototype into a global header instead to avoid this warning.
> 
> Signed-off-by: Arnd Bergmann 

Reviewed-by: Stefano Stabellini 


> ---
>  arch/x86/xen/xen-ops.h | 2 --
>  include/xen/events.h   | 3 +++
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
> index 84a35ff1e0c9..0f71ee3fe86b 100644
> --- a/arch/x86/xen/xen-ops.h
> +++ b/arch/x86/xen/xen-ops.h
> @@ -72,8 +72,6 @@ void xen_restore_time_memory_area(void);
>  void xen_init_time_ops(void);
>  void xen_hvm_init_time_ops(void);
>  
> -irqreturn_t xen_debug_interrupt(int irq, void *dev_id);
> -
>  bool xen_vcpu_stolen(int vcpu);
>  
>  void xen_vcpu_setup(int cpu);
> diff --git a/include/xen/events.h b/include/xen/events.h
> index 44c2855c76d1..ac1281c5ead6 100644
> --- a/include/xen/events.h
> +++ b/include/xen/events.h
> @@ -138,4 +138,7 @@ int xen_test_irq_shared(int irq);
>  
>  /* initialize Xen IRQ subsystem */
>  void xen_init_IRQ(void);
> +
> +irqreturn_t xen_debug_interrupt(int irq, void *dev_id);
> +
>  #endif   /* _XEN_EVENTS_H */
> -- 
> 2.39.2
> 



Re: [PATCH 2/2] xen/misra: diff-report.py: add report patching feature

2023-05-17 Thread Stefano Stabellini
On Wed, 17 May 2023, Luca Fancellu wrote:
> > On 17 May 2023, at 02:33, Stefano Stabellini  wrote:
> > 
> > On Thu, 4 May 2023, Luca Fancellu wrote:
> >> Add a feature to the diff-report.py script that improves the comparison
> >> between two analysis report, one from a baseline codebase and the other
> >> from the changes applied to the baseline.
> >> 
> >> The comparison between reports of different codebase is an issue because
> >> entries in the baseline could have been moved in position due to addition
> >> or deletion of unrelated lines or can disappear because of deletion of
> >> the interested line, making the comparison between two revisions of the
> >> code harder.
> >> 
> >> Having a baseline report, a report of the codebase with the changes
> >> called "new report" and a git diff format file that describes the
> >> changes happened to the code from the baseline, this feature can
> >> understand which entries from the baseline report are deleted or shifted
> >> in position due to changes to unrelated lines and can modify them as
> >> they will appear in the "new report".
> >> 
> >> Having the "patched baseline" and the "new report", now it's simple
> >> to make the diff between them and print only the entry that are new.
> >> 
> >> Signed-off-by: Luca Fancellu 
> > 
> > This is an amazing work!! Thanks Luca!
> > 
> > I am having issues trying the new patch feature. After applying this
> > patch I get:
> > 
> > sstabellini@ubuntu-linux-20-04-desktop:/local/repos/xen-upstream/xen$ 
> > ./scripts/diff-report.py
> > Traceback (most recent call last):
> >  File "./scripts/diff-report.py", line 5, in 
> >from xen_analysis.diff_tool.debug import Debug
> >  File 
> > "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/debug.py", 
> > line 4, in 
> >from .report import Report
> >  File 
> > "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/report.py", 
> > line 4, in 
> >from .unified_format_parser import UnifiedFormatParser, ChangeSet
> >  File 
> > "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
> >  line 56, in 
> >class UnifiedFormatParser:
> >  File 
> > "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
> >  line 57, in UnifiedFormatParser
> >def __init__(self, args: str | list) -> None:
> > TypeError: unsupported operand type(s) for |: 'type' and 'type'
> > 
> > Also got a similar error elsewhere:
> > 
> > sstabellini@ubuntu-linux-20-04-desktop:/local/repos/xen-upstream/xen$ 
> > ./scripts/diff-report.py --patch ~/p/1 -b /tmp/1 -r /tmp/1
> > Traceback (most recent call last):
> >  File "./scripts/diff-report.py", line 127, in 
> >main(sys.argv[1:])
> >  File "./scripts/diff-report.py", line 102, in main
> >diffs = UnifiedFormatParser(diff_source)
> >  File 
> > "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
> >  line 79, in __init__
> >self.__parse()
> >  File 
> > "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
> >  line 94, in __parse
> >def parse_diff_header(line: str) -> ChangeSet | None:
> > TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'
> > 
> > My Python is 2.7.18
> > 
> > 
> > Am I understanding correctly that one should run the scan for the
> > baseline (saving the result somewhere), then apply the patch, run the
> > scan again. Finally, one should call diff-report.py passing -b
> > baseline-report -r new-report --patch the-patch-applied?
> 
> Hi Stefano,
> 
> Yes indeed, that procedure is correct, I think the error you are seeing comes 
> from the python version,
> I am using python 3, version 3.10.6.
> 
> The error seems to come from python annotations, I’m surprised you didn’t hit 
> it when testing the first patch,
> did you use python2 for that?
> 
> Is it a problem if I developed the tool having in mind its usage with python3?

Hi Luca,

It is not a problem per se if the script requires python3 but then we
should check for the python version at the beginning of the script to
fail explictly with a nice error message if python < 3.

I am fine if you want to proceed that way, but if the only issue are the
annotations, I suggest it might be easier to remove them and then you
also get the benefit of python2 compatibility. I'll leave the choice to
you.

Either way, if you are OK with it, I think you should add a new entry to
the MAINTAINERS file to cover the xen analysis scripts if you are OK
with it:

xen/scripts/xen_analysis
xen/scripts/xen-analysis.py
xen/scripts/diff-report.py

[libvirt test] 180688: tolerable all pass - PUSHED

2023-05-17 Thread osstest service owner
flight 180688 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180688/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180642
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180642
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180642
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  b10bc8f7ab6f9986ccc54ba04fc5b3bad7576be6
baseline version:
 libvirt  4a681995bc9f0ba5df779c392b7bebf3470a3f9a

Last test of basis   180642  2023-05-13 04:21:42 Z4 days
Testing same since   180688  2023-05-17 04:20:21 Z0 days1 attempts


People who touched revisions under test:
  Michal Privoznik 
  Peter Krempa 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-armhf-armhf-libvirt-qcow2   pass
 test-arm64-arm64-libvirt-raw pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-i386-libvirt-raw  pass
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest 

Re: [PATCH v2 1/2] x86/mm: add API for marking only part of a MMIO page read only

2023-05-17 Thread Jason Andryuk
On Fri, May 5, 2023 at 5:26 PM Marek Marczykowski-Górecki
 wrote:
>
> In some cases, only few registers on a page needs to be write-protected.

Maybe "In some cases, only part of a page needs to be write-protected"?

> Examples include USB3 console (64 bytes worth of registers) or MSI-X's
> PBA table (which doesn't need to span the whole table either), although
> in the latter case the spec forbids placing other registers on the same
> page. Current API allows only marking whole pages pages read-only,
> which sometimes may cover other registers that guest may need to
> write into.
>
> Currently, when a guest tries to write to an MMIO page on the
> mmio_ro_ranges, it's either immediately crashed on EPT violation - if
> that's HVM, or if PV, it gets #PF. In case of Linux PV, if access was
> from userspace (like, /dev/mem), it will try to fixup by updating page
> tables (that Xen again will force to read-only) and will hit that #PF
> again (looping endlessly). Both behaviors are undesirable if guest could
> actually be allowed the write.
>
> Introduce an API that allows marking part of a page read-only. Since
> sub-page permissions are not a thing in page tables (they are in EPT,
> but not granular enough), do this via emulation (or simply page fault
> handler for PV) that handles writes that are supposed to be allowed.
> The new subpage_mmio_ro_add() takes a start physical address and the
> region size in bytes. Both start address and the size need to be 8-byte
> aligned, as a practical simplification (allows using smaller bitmask,
> and a smaller granularity isn't really necessary right now).
> It will internally add relevant pages to mmio_ro_ranges, but if either
> start or end address is not page-aligned, it additionally adds that page
> to a list for sub-page R/O handling. The list holds a bitmask which
> dwords are supposed to be read-only and an address where page is mapped
> for write emulation - this mapping is done only on the first access. A
> plain list is used instead of more efficient structure, because there
> isn't supposed to be many pages needing this precise r/o control.
>
> The mechanism this API is plugged in is slightly different for PV and
> HVM. For both paths, it's plugged into mmio_ro_emulated_write(). For PV,
> it's already called for #PF on read-only MMIO page. For HVM however, EPT
> violation on p2m_mmio_direct page results in a direct domain_crash().
> To reach mmio_ro_emulated_write(), change how write violations for
> p2m_mmio_direct are handled - specifically, check if they relate to such
> partially protected page via subpage_mmio_write_accept() and if so, call
> hvm_emulate_one_mmio() for them too. This decodes what guest is trying
> write and finally calls mmio_ro_emulated_write(). Note that hitting EPT
> write violation for p2m_mmio_direct page can only happen if the page was
> on mmio_ro_ranges (see ept_p2m_type_to_flags()), so there is no need for
> checking that again.
> Both of those paths need an MFN to which guest tried to write (to check
> which part of the page is supposed to be read-only, and where
> the page is mapped for writes). This information currently isn't
> available directly in mmio_ro_emulated_write(), but in both cases it is
> already resolved somewhere higher in the call tree. Pass it down to
> mmio_ro_emulated_write() via new mmio_ro_emulate_ctxt.mfn field.
>
> Signed-off-by: Marek Marczykowski-Górecki 

Reviewed-by: Jason Andryuk 

> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -522,9 +522,24 @@ extern struct rangeset *mmio_ro_ranges;
>  void memguard_guard_stack(void *p);
>  void memguard_unguard_stack(void *p);
>
> +/*
> + * Add more precise r/o marking for a MMIO page. Bytes range specified here
> + * will still be R/O, but the rest of the page (nor marked as R/O via another

s/nor/not/

> + * call) will have writes passed through.
> + * The start address and the size must be aligned to SUBPAGE_MMIO_RO_ALIGN.
> + *

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c

> +/*
> + * We don't know the write seize at this point yet, so it could 
> be

s/seize/size/

> + * an unalligned write, but accept it here anyway and deal with 
> it

s/unalligned/unaligned/

> + * later.
> + */

Thanks,
Jason



Re: [PATCH v1] automation: allow to rerun build script

2023-05-17 Thread Stefano Stabellini
On Wed, 17 May 2023, Olaf Hering wrote:
> Calling build twice in the same environment will fail because the
> directory 'binaries' was already created before. Use mkdir -p to ignore
> an existing directory and move on to the actual build.
> 
> Signed-off-by: Olaf Hering 

Acked-by: Stefano Stabellini 


> ---
>  automation/scripts/build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 197d085f3e..9085cba352 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -36,7 +36,7 @@ fi
>  cp xen/.config xen-config
>  
>  # Directory for the artefacts to be dumped into
> -mkdir binaries
> +mkdir -p binaries
>  
>  if [[ "${CPPCHECK}" == "y" ]] && [[ "${HYPERVISOR_ONLY}" == "y" ]]; then
>  # Cppcheck analysis invokes Xen-only build.
> 



Re: [PATCH] x86: do away with HAVE_AS_NEGATIVE_TRUE

2023-05-17 Thread Andrew Cooper
On 17/05/2023 3:22 pm, Jan Beulich wrote:
> There's no real need for the associated probing - we can easily convert
> to a uniform value without knowing the specific behavior (note also that
> the respective comments weren't fully correct and have gone stale). All
> we (need to) depend upon is unary ! producing 0 or 1 (and never -1).
>
> For all present purposes yielding a value with all bits set is more
> useful.
>
> No difference in generated code.
>
> Signed-off-by: Jan Beulich 
> ---
> Unlike in C, there's also binary ! in assembly expressions, and even
> binary !!. But those don't get in the way here.

I had been wanting to do this for a while, but IMO a clearer expression
is to take ((x) & 1) to discard the sign.

It doesn't change any of the logic to use +1 (I don't think), and it's
definitely the more common way for the programmer to think.

~Andrew



Re: [PATCH RFC] xen: Enable -Wwrite-strings

2023-05-17 Thread Andrew Cooper
On 17/05/2023 11:34 am, Jan Beulich wrote:
> On 16.05.2023 22:34, Andrew Cooper wrote:
>> Following on from the MISRA discussions.
>>
>> On x86, most are trivial.  The two slightly suspect cases are __hvm_copy()
>> where constness is dependent on flags,
> But do we ever pass string literals into there? I certainly would
> like to avoid the explicit casts to get rid of the const there.

The thing which trips it up is the constness of the cmdline param in the
construct_dom0() calltree.  It may have been tied up in the constness
from cmdline_cook() - I wasn't paying that much attention.

Irrespective, from a conceptual point of view, we ought to be able to
use the copy_to_* helpers from a const source.

>> and kextra in __start_xen() which only
>> compiles because of laundering the pointer through strstr().
> The sole string literal there looks to be the empty string in
> cmdline_cook(), which could be easily replaced, I think:
>
> static char * __init cmdline_cook(char *p, const char *loader_name)
> {
> static char __initdata empty[] = "";
>
> p = p ? : empty;
>
> Yet of course only if we were unhappy with the strstr() side effect.

It's quite possible we can do something better here.  This logic looks
unnecessarily complicated and fragile.

>
>> The one case which I can't figure out how to fix is EFI:
>>
>>   In file included from arch/x86/efi/boot.c:700:
>>   arch/x86/efi/efi-boot.h: In function ‘efi_arch_handle_cmdline’:
>>   arch/x86/efi/efi-boot.h:327:16: error: assignment discards ‘const’ 
>> qualifier from pointer target type [-Werror=discarded-qualifiers]
>> 327 | name.s = "xen";
>> |^
>>   cc1: all warnings being treated as errors
>>
>> Why do we have something that looks like this ?
>>
>>   union string {
>>   CHAR16 *w;
>>   char *s;
>>   const char *cs;
>>   };
> Because that was the least clutter (at respective use sites) that I
> could think of at the time. Looks like you could simply assign to
> name.cs, now that we have that field (iirc it wasn't there originally).
> Of course that's then only papering over the issue.

Well yes.  If it's only this one, we could use the same initconst trick
and delete the cs field, but I suspect the fields existence means it
would cause problems elsewhere.

>
>> --- a/xen/include/acpi/actypes.h
>> +++ b/xen/include/acpi/actypes.h
>> @@ -281,7 +281,7 @@ typedef acpi_native_uint acpi_size;
>>   */
>>  typedef u32 acpi_status;/* All ACPI Exceptions */
>>  typedef u32 acpi_name;  /* 4-byte ACPI name */
>> -typedef char *acpi_string;  /* Null terminated ASCII string */
>> +typedef const char *acpi_string;/* Null terminated ASCII string */
>>  typedef void *acpi_handle;  /* Actually a ptr to a NS Node */
> For all present uses that we have this change looks okay, but changing
> this header leaves me a little uneasy. At the same time I have no
> better suggestion.

I was honestly tempted to purge this typedef with prejudice.  Hiding
indirection like this is nothing but an obfuscation technique.

~Andrew



Re: [PATCH 4/4] x86/spec-ctrl: Remove opencoded MSR_ARCH_CAPS check

2023-05-17 Thread Andrew Cooper
On 17/05/2023 3:47 pm, Jan Beulich wrote:
> On 16.05.2023 16:53, Andrew Cooper wrote:
>> @@ -401,6 +400,8 @@ static void __init print_details(enum ind_thunk thunk, 
>> uint64_t caps)
>>  cpuid_count(7, 2, , , , &_7d2);
>>  if ( boot_cpu_data.extended_cpuid_level >= 0x8008 )
>>  cpuid(0x8008, , , , );
>> +if ( cpu_has_arch_caps )
>> +rdmsrl(MSR_ARCH_CAPABILITIES, caps);
> Why do you read the MSR again? I would have expected this to come out
> of raw_cpu_policy now (and incrementally the CPUID pieces as well,
> later on).

Consistency with the surrounding logic.

Also because the raw and host policies don't get sorted until much later
in boot.

> Apart from this, with all the uses further down gone, perhaps there's
> not even a need for the raw value, if you used the bitfields in the
> printk(). Which in turn raises the question whether the #define-s in
> msr-index.h are of much use then anymore.

One of the next phases of work is synthesizing these in the host policy
for CPUs which didn't receive microcode updates (for whatever reason).

There is a valid discussion for whether we ought to render the raw or
host info here (currently we do raw), but I'm not adjusting that in this
patch.

~Andrew



Re: [PATCH v7 2/5] xen/riscv: introduce setup_initial_pages

2023-05-17 Thread Oleksii
On Tue, 2023-05-16 at 18:02 +0200, Jan Beulich wrote:
> On 11.05.2023 19:09, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/page.h
> > @@ -0,0 +1,58 @@
> > +#ifndef _ASM_RISCV_PAGE_H
> > +#define _ASM_RISCV_PAGE_H
> > +
> > +#include 
> > +#include 
> > +
> > +#define VPN_MASK    ((unsigned
> > long)(PAGETABLE_ENTRIES - 1))
> > +
> > +#define XEN_PT_LEVEL_ORDER(lvl) ((lvl) * PAGETABLE_ORDER)
> > +#define XEN_PT_LEVEL_SHIFT(lvl) (XEN_PT_LEVEL_ORDER(lvl) +
> > PAGE_SHIFT)
> > +#define XEN_PT_LEVEL_SIZE(lvl)  (_AT(paddr_t, 1) <<
> > XEN_PT_LEVEL_SHIFT(lvl))
> > +#define XEN_PT_LEVEL_MAP_MASK(lvl)  (~(XEN_PT_LEVEL_SIZE(lvl) -
> > 1))
> > +#define XEN_PT_LEVEL_MASK(lvl)  (VPN_MASK <<
> > XEN_PT_LEVEL_SHIFT(lvl))
> > +
> > +#define PTE_VALID   BIT(0, UL)
> > +#define PTE_READABLE    BIT(1, UL)
> > +#define PTE_WRITABLE    BIT(2, UL)
> > +#define PTE_EXECUTABLE  BIT(3, UL)
> > +#define PTE_USER    BIT(4, UL)
> > +#define PTE_GLOBAL  BIT(5, UL)
> > +#define PTE_ACCESSED    BIT(6, UL)
> > +#define PTE_DIRTY   BIT(7, UL)
> > +#define PTE_RSW (BIT(8, UL) | BIT(9, UL))
> > +
> > +#define PTE_LEAF_DEFAULT    (PTE_VALID | PTE_READABLE |
> > PTE_WRITABLE)
> > +#define PTE_TABLE   (PTE_VALID)
> > +
> > +/* Calculate the offsets into the pagetables for a given VA */
> > +#define pt_linear_offset(lvl, va)   ((va) >>
> > XEN_PT_LEVEL_SHIFT(lvl))
> > +
> > +#define pt_index(lvl, va) (pt_linear_offset(lvl, (va)) & VPN_MASK)
> 
> Nit: Please be consistent with parentheses. Here va doesn't need any,
> but if you added / kept them, then lvl should also gain them.
Sure. I'll fix that.

> 
> > +/* Page Table entry */
> > +typedef struct {
> > +#ifdef CONFIG_RISCV_64
> > +    uint64_t pte;
> > +#else
> > +    uint32_t pte;
> > +#endif
> > +} pte_t;
> > +
> > +static inline pte_t paddr_to_pte(paddr_t paddr,
> > + unsigned int permissions)
> > +{
> > +    return (pte_t) { .pte = (paddr >> PAGE_SHIFT) << PTE_PPN_SHIFT
> > | permissions };
> 
> Please parenthesize the << against the |. I have also previously
> recommended to avoid open-coding of things like PFN_DOWN() (or
> paddr_to_pfn(), if you like that better) or ...
I'll change it to paddr_to_pfn. It sounds more clear than PFN_DOWN()
in this context.
Thanks for reminder.
> 
> > +}
> > +
> > +static inline paddr_t pte_to_paddr(pte_t pte)
> > +{
> > +    return ((paddr_t)pte.pte >> PTE_PPN_SHIFT) << PAGE_SHIFT;
> 
> ... or pfn_to_paddr() (which here would avoid the misplaced cast).
pfn_to_paddr() would be better here so I'll update it.

> 
> > --- a/xen/arch/riscv/include/asm/processor.h
> > +++ b/xen/arch/riscv/include/asm/processor.h
> > @@ -69,6 +69,11 @@ static inline void die(void)
> >  wfi();
> >  }
> >  
> > +static inline void sfence_vma(void)
> > +{
> > +    __asm__ __volatile__ ("sfence.vma" ::: "memory");
> > +}
> 
> Hmm, in switch_stack_and_jump() you use "asm volatile()" (no
> underscores). This is another thing which would nice if it was
> consistent (possibly among headers as one group, and .c files as
> another - there may be reasons why one wants the underscore
> variants in headers, but the "easier" ones in .c files).
I will remove "__" to be consistent here and in future.
> 
> Also nit: Style (missing blanks inside the parentheses).
THanks. Missed that.
> 
> > +static void __init setup_initial_mapping(struct mmu_desc
> > *mmu_desc,
> > + unsigned long map_start,
> > + unsigned long map_end,
> > + unsigned long pa_start)
> > +{
> > +    unsigned int index;
> > +    pte_t *pgtbl;
> > +    unsigned long page_addr;
> > +
> > +    if ( (unsigned long)_start % XEN_PT_LEVEL_SIZE(0) )
> > +    {
> > +    early_printk("(XEN) Xen should be loaded at 4k
> > boundary\n");
> > +    die();
> > +    }
> > +
> > +    if ( map_start & ~XEN_PT_LEVEL_MAP_MASK(0) ||
> > + pa_start & ~XEN_PT_LEVEL_MAP_MASK(0) )
> 
> Nit: Please parenthesize the two & against the ||.
Sure. THanks.
> 
> > +    {
> > +    early_printk("(XEN) map and pa start addresses should be
> > aligned\n");
> > +    /* panic(), BUG() or ASSERT() aren't ready now. */
> > +    die();
> > +    }
> > +
> > +    for ( page_addr = map_start;
> > +  page_addr < map_end;
> > +  page_addr += XEN_PT_LEVEL_SIZE(0) )
> > +    {
> > +    pgtbl = mmu_desc->pgtbl_base;
> > +
> > +    switch ( mmu_desc->num_levels )
> > +    {
> > +    case 4: /* Level 3 */
> > +    HANDLE_PGTBL(3);
> > +    case 3: /* Level 2 */
> > +    HANDLE_PGTBL(2);
> > +    case 2: /* Level 1 */
> > +    HANDLE_PGTBL(1);
> > +    case 1: /* Level 0 */
> > +    {
> > +    

[linux-linus test] 180687: regressions - FAIL

2023-05-17 Thread osstest service owner
flight 180687 linux-linus real [real]
flight 180692 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180687/
http://logs.test-lab.xenproject.org/osstest/logs/180692/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linuxf1fcbaa18b28dec10281551dfe6ed3a3ed80e3d6
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   30 days
Failing since180281  2023-04-17 06:24:36 Z   30 days   56 attempts
Testing same since   180664  2023-05-14 20:12:01 Z2 days6 attempts


2389 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 

Re: [PATCH v7 1/5] xen/riscv: add VM space layout

2023-05-17 Thread Oleksii
On Tue, 2023-05-16 at 17:42 +0200, Jan Beulich wrote:
> On 11.05.2023 19:09, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/include/asm/config.h
> > +++ b/xen/arch/riscv/include/asm/config.h
> > @@ -4,6 +4,42 @@
> >  #include 
> >  #include 
> >  
> > +/*
> > + * RISC-V64 Layout:
> > + *
> > + * #ifdef SV39
> 
> I did point you at x86'es similar #ifdef. Unlike here, there we use a
> symbol which actually has a meaning, allowing to spot this comment in
> e.g. grep output when looking for uses of that symbol. Hence here
> e.g.
> 
> #ifdef RV_STAGE1_MODE == SATP_MODE_SV39
> 
> ? (I would also recommend to use the same style as x86 does, such
> that
> the #ifdef and #endif look like normal directives [e.g. again in grep
> output], leaving aside that they're inside a comment.)
It would be better. Thanks.
> 
> > + * From the riscv-privileged doc:
> > + *   When mapping between narrower and wider addresses,
> > + *   RISC-V zero-extends a narrower physical address to a wider
> > size.
> > + *   The mapping between 64-bit virtual addresses and the 39-bit
> > usable
> > + *   address space of Sv39 is not based on zero-extension but
> > instead
> > + *   follows an entrenched convention that allows an OS to use one
> > or
> > + *   a few of the most-significant bits of a full-size (64-bit)
> > virtual
> > + *   address to quickly distinguish user and supervisor address
> > regions.
> > + *
> > + * It means that:
> > + *   top VA bits are simply ignored for the purpose of translating
> > to PA.
> > + *
> > + *
> > ===
> > =
> > + *    Start addr    |   End addr    |  Size  | Slot  
> > |area description
> > + *
> > ===
> > =
> > + * C080 |   |1016 MB | L2 511 |
> > Unused
> > + * C060 |  C080 |  2 MB  | L2 511 |
> > Fixmap
> > + * C020 |  C060 |  4 MB  | L2 511 |
> > FDT
> > + * C000 |  C020 |  2 MB  | L2 511 |
> > Xen
> > + * ...  |  1 GB  | L2 510 |
> > Unused
> > + * 0032 |  007f4000 | 309 GB | L2 200-509 |
> > Direct map
> 
> The upper bound here is 007f8000 afaict, 
It should be 007f8000. 007f4000 is start address of 509
slot.

> which then also makes
> the earlier gap 1Gb in size.
do you mean that it is better to write start and end address (
007f8000 - 7FC000 ) of L2 510 slot explicitly?

~ Oleksii



Re: [PATCH] MAINTAINERS: add more xenstore files

2023-05-17 Thread Jan Beulich
On 28.04.2023 15:27, Juergen Gross wrote:
> Xenstore consists of more files than just the tools/xenstore directory.
> 
> Add them to the XENSTORE block.
> 
> Suggested-by: Andrew Cooper 
> Signed-off-by: Juergen Gross 
> ---
>  MAINTAINERS | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0e5eba2312..f2f1881b32 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -653,6 +653,11 @@ M:   Wei Liu 
>  M:   Juergen Gross 
>  R:   Julien Grall 
>  S:   Supported
> +F:   tools/helpers/init-xenstore-domain.c
> +F:   tools/include/xenstore-compat/
> +F:   tools/include/xenstore.h
> +F:   tools/include/xenstore_lib.h
> +F:   tools/libs/store/
>  F:   tools/xenstore/

I wonder if at the same time xenstore specific include files shouldn't
have been purged from LIBS.

Jan



Re: [PATCH 0/3] officializing xenstore control/feature-balloon entry

2023-05-17 Thread Jan Beulich
On 10.05.2023 16:20, Yann Dirson wrote:
> The main topic of this patch series is the ~/control/feature-balloon
> entry used by XAPI, prompted by the report of xe-guest-utilities on
> FreeBSD not being able to report the feature when using just libxl on
> the host.
> 
> First patch is a bit off-topic, but included here because it fixes the
> text from which this feature description was adapted from.
> 
> Yann Dirson (3):
>   docs: fix complex-and-wrong xenstore-path wording
>   docs: document ~/control/feature-balloon
>   libxl: create ~/control/feature-balloon
> 
>  docs/misc/xenstore-paths.pandoc | 16 ++--
>  tools/libs/light/libxl_create.c |  3 +++
>  2 files changed, 13 insertions(+), 6 deletions(-)

You may want to re-send this series with maintainers properly Cc-ed.
For the docs changes I wouldn't go by get_maintainer.pl, but rather
Cc the xenstore maintainers. (I guess this file ought to be added to
that section, on top of what was recently added there.)

Jan



Re: [PATCH] docs: fix xenstore-paths doc structure

2023-05-17 Thread Jan Beulich
On 09.05.2023 12:25, Yann Dirson wrote:
> We currently have "Per Domain Paths" as an empty section, whereas it
> looks like "General Paths" was not indended to include all the
> following sections.
> 
> Signed-off-by: Yann Dirson 

Reviewed-by: Jan Beulich 

Cc-ing the xenstore folks for an ack.

Jan

> --- a/docs/misc/xenstore-paths.pandoc
> +++ b/docs/misc/xenstore-paths.pandoc
> @@ -129,7 +129,7 @@ create writable subdirectories as necessary.
>  
>  ## Per Domain Paths
>  
> -## General Paths
> +### General Paths
>  
>   ~/vm = PATH []
>  




Re: [PATCH 4/4] x86/spec-ctrl: Remove opencoded MSR_ARCH_CAPS check

2023-05-17 Thread Jan Beulich
On 16.05.2023 16:53, Andrew Cooper wrote:
> @@ -401,6 +400,8 @@ static void __init print_details(enum ind_thunk thunk, 
> uint64_t caps)
>  cpuid_count(7, 2, , , , &_7d2);
>  if ( boot_cpu_data.extended_cpuid_level >= 0x8008 )
>  cpuid(0x8008, , , , );
> +if ( cpu_has_arch_caps )
> +rdmsrl(MSR_ARCH_CAPABILITIES, caps);

Why do you read the MSR again? I would have expected this to come out
of raw_cpu_policy now (and incrementally the CPUID pieces as well,
later on).

Apart from this, with all the uses further down gone, perhaps there's
not even a need for the raw value, if you used the bitfields in the
printk(). Which in turn raises the question whether the #define-s in
msr-index.h are of much use then anymore.

Jan



Re: [PATCH 2/4] x86/vtx: Remove opencoded MSR_ARCH_CAPS check

2023-05-17 Thread Jan Beulich
On 17.05.2023 16:36, Jan Beulich wrote:
> On 16.05.2023 16:53, Andrew Cooper wrote:
>> MSR_ARCH_CAPS data is now included in featureset information.
>>
>> Signed-off-by: Andrew Cooper 
> 
> Acked-by: Jan Beulich 

Oops - this was really meant to be
Reviewed-by: Jan Beulich 

Jan

> albeit, like in one of the patches in the earlier series, ...
> 
>> --- a/xen/arch/x86/include/asm/cpufeature.h
>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>> @@ -183,6 +183,9 @@ static inline bool boot_cpu_has(unsigned int feat)
>>  #define cpu_has_avx_vnni_int8   boot_cpu_has(X86_FEATURE_AVX_VNNI_INT8)
>>  #define cpu_has_avx_ne_convert  boot_cpu_has(X86_FEATURE_AVX_NE_CONVERT)
>>  
>> +/* MSR_ARCH_CAPS 10A */
>> +#define cpu_has_if_pschange_mc_no 
>> boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
> 
> ... I'm not convinced that having the (unadorned) MSR index here
> is really helpful. In particular to people not knowing the indexes
> by heart (possibly most except you) the number may actually look
> odd / misplaced there.
> 
> Jan
> 




Re: [PATCH 3/4] x86/tsx: Remove opencoded MSR_ARCH_CAPS check

2023-05-17 Thread Jan Beulich
On 16.05.2023 16:53, Andrew Cooper wrote:
> The current cpu_has_tsx_ctrl tristate is serving double pupose; to signal the
> first pass through tsx_init(), and the availability of MSR_TSX_CTRL.
> 
> Drop the variable, replacing it with a once boolean, and altering
> cpu_has_tsx_ctrl to come out of the feature information.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 




Re: [PATCH 2/4] x86/vtx: Remove opencoded MSR_ARCH_CAPS check

2023-05-17 Thread Jan Beulich
On 16.05.2023 16:53, Andrew Cooper wrote:
> MSR_ARCH_CAPS data is now included in featureset information.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Jan Beulich 
albeit, like in one of the patches in the earlier series, ...

> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -183,6 +183,9 @@ static inline bool boot_cpu_has(unsigned int feat)
>  #define cpu_has_avx_vnni_int8   boot_cpu_has(X86_FEATURE_AVX_VNNI_INT8)
>  #define cpu_has_avx_ne_convert  boot_cpu_has(X86_FEATURE_AVX_NE_CONVERT)
>  
> +/* MSR_ARCH_CAPS 10A */
> +#define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)

... I'm not convinced that having the (unadorned) MSR index here
is really helpful. In particular to people not knowing the indexes
by heart (possibly most except you) the number may actually look
odd / misplaced there.

Jan



[PATCH] x86: do away with HAVE_AS_NEGATIVE_TRUE

2023-05-17 Thread Jan Beulich
There's no real need for the associated probing - we can easily convert
to a uniform value without knowing the specific behavior (note also that
the respective comments weren't fully correct and have gone stale). All
we (need to) depend upon is unary ! producing 0 or 1 (and never -1).

For all present purposes yielding a value with all bits set is more
useful.

No difference in generated code.

Signed-off-by: Jan Beulich 
---
Unlike in C, there's also binary ! in assembly expressions, and even
binary !!. But those don't get in the way here.

--- a/xen/arch/x86/arch.mk
+++ b/xen/arch/x86/arch.mk
@@ -26,10 +26,6 @@ $(call as-option-add,CFLAGS,CC,"invpcid
 $(call as-option-add,CFLAGS,CC,"movdiri %rax$(comma)(%rax)",-DHAVE_AS_MOVDIR)
 $(call as-option-add,CFLAGS,CC,"enqcmd (%rax)$(comma)%rax",-DHAVE_AS_ENQCMD)
 
-# GAS's idea of true is -1.  Clang's idea is 1
-$(call as-option-add,CFLAGS,CC,\
-".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE)
-
 # Check to see whether the assmbler supports the .nop directive.
 $(call as-option-add,CFLAGS,CC,\
 ".L1: .L2: .nops (.L2 - .L1)$(comma)9",-DHAVE_AS_NOPS_DIRECTIVE)
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -35,19 +35,19 @@ extern void alternative_branches(void);
 #define alt_repl_e(num)".LXEN%=_repl_e"#num
 #define alt_repl_len(num)  "(" alt_repl_e(num) " - " alt_repl_s(num) ")"
 
-/* GAS's idea of true is -1, while Clang's idea is 1. */
-#ifdef HAVE_AS_NEGATIVE_TRUE
-# define AS_TRUE "-"
-#else
-# define AS_TRUE ""
-#endif
+/*
+ * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea was
+ * consistently 1 up to 6.x (it matches GAS's now).  Transform it to uniformly
+ * -1 (aka ~0).
+ */
+#define AS_TRUE "-!!"
 
-#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & -("AS_TRUE"(("a") < 
("b")"
+#define as_max(a, b) "(("a") ^ ((("a") ^ ("b")) & "AS_TRUE"(("a") < ("b""
 
 #define OLDINSTR(oldinstr, padding)  \
 ".LXEN%=_orig_s:\n\t" oldinstr "\n .LXEN%=_orig_e:\n\t"  \
 ".LXEN%=_diff = " padding "\n\t" \
-"mknops ("AS_TRUE"(.LXEN%=_diff > 0) * .LXEN%=_diff)\n\t"\
+"mknops ("AS_TRUE"(.LXEN%=_diff > 0) & .LXEN%=_diff)\n\t"\
 ".LXEN%=_orig_p:\n\t"
 
 #define OLDINSTR_1(oldinstr, n1) \
--- a/xen/arch/x86/include/asm/alternative-asm.h
+++ b/xen/arch/x86/include/asm/alternative-asm.h
@@ -29,17 +29,17 @@
 #endif
 .endm
 
-/* GAS's idea of true is -1, while Clang's idea is 1. */
-#ifdef HAVE_AS_NEGATIVE_TRUE
-# define as_true(x) (-(x))
-#else
-# define as_true(x) (x)
-#endif
+/*
+ * GAS's idea of true is sometimes 1 and sometimes -1, while Clang's idea was
+ * consistently 1 up to 6.x (it matches GAS's now).  Transform it to uniformly
+ * -1 (aka ~0).
+ */
+#define as_true(x) (-!!(x))
 
 #define decl_orig(insn, padding)  \
  .L\@_orig_s: insn; .L\@_orig_e:  \
  .L\@_diff = padding; \
- mknops (as_true(.L\@_diff > 0) * .L\@_diff); \
+ mknops (as_true(.L\@_diff > 0) & .L\@_diff); \
  .L\@_orig_p:
 
 #define orig_len   (.L\@_orig_e   - .L\@_orig_s)
@@ -49,7 +49,7 @@
 #define decl_repl(insn, nr) .L\@_repl_s\()nr: insn; .L\@_repl_e\()nr:
 #define repl_len(nr)   (.L\@_repl_e\()nr  - .L\@_repl_s\()nr)
 
-#define as_max(a, b)   ((a) ^ (((a) ^ (b)) & -as_true((a) < (b
+#define as_max(a, b)   ((a) ^ (((a) ^ (b)) & as_true((a) < (b
 
 .macro ALTERNATIVE oldinstr, newinstr, feature
 decl_orig(\oldinstr, repl_len(1) - orig_len)



[PATCH] libelf: make L1_MFN_VALID note known

2023-05-17 Thread Jan Beulich
We still don't use it (in the tool stack), and its values (plural) also
aren't fetched correctly, but it is odd to continue to see the
hypervisor log "ELF: note: unknown (0xd)" when loading a Linux Dom0.

Signed-off-by: Jan Beulich 

--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -117,6 +117,7 @@ elf_errorstatus elf_xen_parse_note(struc
 [XEN_ELFNOTE_FEATURES] = { "FEATURES", 1},
 [XEN_ELFNOTE_SUPPORTED_FEATURES] = { "SUPPORTED_FEATURES", 0},
 [XEN_ELFNOTE_BSD_SYMTAB] = { "BSD_SYMTAB", 1},
+[XEN_ELFNOTE_L1_MFN_VALID] = { "L1_MFN_VALID", false },
 [XEN_ELFNOTE_SUSPEND_CANCEL] = { "SUSPEND_CANCEL", 0 },
 [XEN_ELFNOTE_MOD_START_PFN] = { "MOD_START_PFN", 0 },
 [XEN_ELFNOTE_PHYS32_ENTRY] = { "PHYS32_ENTRY", 0 },



Re: [PATCH 1/4] x86/cpufeature: Rework {boot_,}cpu_has()

2023-05-17 Thread Jan Beulich
On 16.05.2023 16:53, Andrew Cooper wrote:
> --- a/xen/arch/x86/include/asm/cpufeature.h
> +++ b/xen/arch/x86/include/asm/cpufeature.h
> @@ -7,6 +7,7 @@
>  #define __ASM_I386_CPUFEATURE_H
>  
>  #include 
> +#include 
>  #include 

This isn't needed up here, and ...

> @@ -17,7 +18,6 @@
>  #define X86_FEATURE_ALWAYS  X86_FEATURE_LM
>  
>  #ifndef __ASSEMBLY__
> -#include 

... putting it here would (a) eliminate a header dependency for
assembly sources including this file (perhaps indirectly) and (b)
eliminate the risk of a build breakage if something was added to
that header which isn't valid assembly.

Preferably with the adjustment
Reviewed-by: Jan Beulich 

Jan



[PATCH] xen: xen_debug_interrupt prototype to global header

2023-05-17 Thread Arnd Bergmann
From: Arnd Bergmann 

The xen_debug_interrupt() function is only called on x86, which has a
prototype in an architecture specific header, but the definition also
exists on others, where the lack of a prototype causes a W=1 warning:

drivers/xen/events/events_2l.c:264:13: error: no previous prototype for 
'xen_debug_interrupt' [-Werror=missing-prototypes]

Move the prototype into a global header instead to avoid this warning.

Signed-off-by: Arnd Bergmann 
---
 arch/x86/xen/xen-ops.h | 2 --
 include/xen/events.h   | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 84a35ff1e0c9..0f71ee3fe86b 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -72,8 +72,6 @@ void xen_restore_time_memory_area(void);
 void xen_init_time_ops(void);
 void xen_hvm_init_time_ops(void);
 
-irqreturn_t xen_debug_interrupt(int irq, void *dev_id);
-
 bool xen_vcpu_stolen(int vcpu);
 
 void xen_vcpu_setup(int cpu);
diff --git a/include/xen/events.h b/include/xen/events.h
index 44c2855c76d1..ac1281c5ead6 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -138,4 +138,7 @@ int xen_test_irq_shared(int irq);
 
 /* initialize Xen IRQ subsystem */
 void xen_init_IRQ(void);
+
+irqreturn_t xen_debug_interrupt(int irq, void *dev_id);
+
 #endif /* _XEN_EVENTS_H */
-- 
2.39.2




Re: [PATCH] xen/pt: fix igd passthrough for pc machine with xen accelerator

2023-05-17 Thread Michael Tokarev

17.05.2023 12:47, Chuck Zmudzinski wrote:

On 5/17/2023 2:39 AM, Michael Tokarev wrote:

08.02.2023 05:03, Chuck Zmudzinski wrote:...

Fixes: 998250e97661 ("xen, gfx passthrough: register host bridge specific to 
passthrough")
Signed-off-by: Chuck Zmudzinski 


Has this change been forgotten?  Is it not needed anymore?


Short answer:

After 4f67543b ("xen/pt: reserve PCI slot 2 for Intel igd-passthru ") was
applied, I was inclined to think this change is not needed anymore, but
it would not hurt to add this change also, and now I think it might be
more correct to also add this change.

...

Well, there were two machines with broken IDG passthrough in xen, now
there's one machine with broken IDG passthrough. Let's fix them all :)
Note this patch is tagged -stable as well.


If you want to add this change also, let's make sure recent changes to the
xen header files do not require the patch to be rebased before committing
it.


It doesn't require rebasing, it looks like, - just built 8.0 and current master
qemu with it applied.  I haven't tried the actual IDG passthrough, though.

It just neeeds to be picked up the usual way as all other qemu changes goes in.

Thanks,

/mjt



Re: [PATCH 1/2] xen/misra: add diff-report.py tool

2023-05-17 Thread Luca Fancellu



> On 17 May 2023, at 02:26, Stefano Stabellini  wrote:
> 
> On Thu, 4 May 2023, Luca Fancellu wrote:
>> Add a new tool, diff-report.py that can be used to make diff between
>> reports generated by xen-analysis.py tool.
>> Currently this tool supports the Xen cppcheck text report format in
>> its operations.
>> 
>> The tool prints every finding that is in the report passed with -r
>> (check report) which is not in the report passed with -b (baseline).
>> 
>> Signed-off-by: Luca Fancellu 
> 
> Acked-by: Stefano Stabellini 
> Tested-by: Stefano Stabellini 

Thank you Stefano for taking the time to review and test it, I will push
the new version of the serie with the stale functions removed and I will
add your A-by and T-by.

Cheers,
Luca





[qemu-mainline test] 180686: tolerable FAIL - PUSHED

2023-05-17 Thread osstest service owner
flight 180686 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180686/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180673
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180673
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180673
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180673
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180673
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180673
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180673
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180673
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass

version targeted for testing:
 qemuuf9d58e0ca53b3f470b84725a7b5e47fcf446a2ea
baseline version:
 qemuu18b6727083acceac5d76ea0b8cb6f5cdef6858a7

Last test of basis   180673  2023-05-15 17:38:45 Z1 days
Failing since180679  2023-05-16 05:45:31 Z1 days2 attempts
Testing same since   180686  2023-05-16 20:07:22 Z0 days1 attempts


People who touched revisions under test:
  Andrei Gudkov 
  Ani Sinha 
  Christian Schoenebeck 
  Claudio Imbrenda 
  Daniel P. Berrangé 
  Ilya Leoshkevich 
  Jason Andryuk 
  Jonathan Cameron 
  Juan Quintela 
  Kevin Wolf 
  Laurent Vivier 
  Lizhi Yang 
  Mateusz Krawczuk 
  Peter Foley 
  Philippe Mathieu-Daudé 
  Richard Henderson 

Re: [PATCH RFC] xen: Enable -Wwrite-strings

2023-05-17 Thread Jan Beulich
On 16.05.2023 22:34, Andrew Cooper wrote:
> Following on from the MISRA discussions.
> 
> On x86, most are trivial.  The two slightly suspect cases are __hvm_copy()
> where constness is dependent on flags,

But do we ever pass string literals into there? I certainly would
like to avoid the explicit casts to get rid of the const there.

> and kextra in __start_xen() which only
> compiles because of laundering the pointer through strstr().

The sole string literal there looks to be the empty string in
cmdline_cook(), which could be easily replaced, I think:

static char * __init cmdline_cook(char *p, const char *loader_name)
{
static char __initdata empty[] = "";

p = p ? : empty;

Yet of course only if we were unhappy with the strstr() side effect.

> The one case which I can't figure out how to fix is EFI:
> 
>   In file included from arch/x86/efi/boot.c:700:
>   arch/x86/efi/efi-boot.h: In function ‘efi_arch_handle_cmdline’:
>   arch/x86/efi/efi-boot.h:327:16: error: assignment discards ‘const’ 
> qualifier from pointer target type [-Werror=discarded-qualifiers]
> 327 | name.s = "xen";
> |^
>   cc1: all warnings being treated as errors
> 
> Why do we have something that looks like this ?
> 
>   union string {
>   CHAR16 *w;
>   char *s;
>   const char *cs;
>   };

Because that was the least clutter (at respective use sites) that I
could think of at the time. Looks like you could simply assign to
name.cs, now that we have that field (iirc it wasn't there originally).
Of course that's then only papering over the issue.

> --- a/xen/include/acpi/actypes.h
> +++ b/xen/include/acpi/actypes.h
> @@ -281,7 +281,7 @@ typedef acpi_native_uint acpi_size;
>   */
>  typedef u32 acpi_status; /* All ACPI Exceptions */
>  typedef u32 acpi_name;   /* 4-byte ACPI name */
> -typedef char *acpi_string;   /* Null terminated ASCII string */
> +typedef const char *acpi_string; /* Null terminated ASCII string */
>  typedef void *acpi_handle;   /* Actually a ptr to a NS Node */

For all present uses that we have this change looks okay, but changing
this header leaves me a little uneasy. At the same time I have no
better suggestion.

Jan



Re: [PATCH 2/2] xen/misra: diff-report.py: add report patching feature

2023-05-17 Thread Luca Fancellu


> On 17 May 2023, at 02:33, Stefano Stabellini  wrote:
> 
> On Thu, 4 May 2023, Luca Fancellu wrote:
>> Add a feature to the diff-report.py script that improves the comparison
>> between two analysis report, one from a baseline codebase and the other
>> from the changes applied to the baseline.
>> 
>> The comparison between reports of different codebase is an issue because
>> entries in the baseline could have been moved in position due to addition
>> or deletion of unrelated lines or can disappear because of deletion of
>> the interested line, making the comparison between two revisions of the
>> code harder.
>> 
>> Having a baseline report, a report of the codebase with the changes
>> called "new report" and a git diff format file that describes the
>> changes happened to the code from the baseline, this feature can
>> understand which entries from the baseline report are deleted or shifted
>> in position due to changes to unrelated lines and can modify them as
>> they will appear in the "new report".
>> 
>> Having the "patched baseline" and the "new report", now it's simple
>> to make the diff between them and print only the entry that are new.
>> 
>> Signed-off-by: Luca Fancellu 
> 
> This is an amazing work!! Thanks Luca!
> 
> I am having issues trying the new patch feature. After applying this
> patch I get:
> 
> sstabellini@ubuntu-linux-20-04-desktop:/local/repos/xen-upstream/xen$ 
> ./scripts/diff-report.py
> Traceback (most recent call last):
>  File "./scripts/diff-report.py", line 5, in 
>from xen_analysis.diff_tool.debug import Debug
>  File 
> "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/debug.py", line 
> 4, in 
>from .report import Report
>  File 
> "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/report.py", 
> line 4, in 
>from .unified_format_parser import UnifiedFormatParser, ChangeSet
>  File 
> "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
>  line 56, in 
>class UnifiedFormatParser:
>  File 
> "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
>  line 57, in UnifiedFormatParser
>def __init__(self, args: str | list) -> None:
> TypeError: unsupported operand type(s) for |: 'type' and 'type'
> 
> Also got a similar error elsewhere:
> 
> sstabellini@ubuntu-linux-20-04-desktop:/local/repos/xen-upstream/xen$ 
> ./scripts/diff-report.py --patch ~/p/1 -b /tmp/1 -r /tmp/1
> Traceback (most recent call last):
>  File "./scripts/diff-report.py", line 127, in 
>main(sys.argv[1:])
>  File "./scripts/diff-report.py", line 102, in main
>diffs = UnifiedFormatParser(diff_source)
>  File 
> "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
>  line 79, in __init__
>self.__parse()
>  File 
> "/local/repos/xen-upstream/xen/scripts/xen_analysis/diff_tool/unified_format_parser.py",
>  line 94, in __parse
>def parse_diff_header(line: str) -> ChangeSet | None:
> TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'
> 
> My Python is 2.7.18
> 
> 
> Am I understanding correctly that one should run the scan for the
> baseline (saving the result somewhere), then apply the patch, run the
> scan again. Finally, one should call diff-report.py passing -b
> baseline-report -r new-report --patch the-patch-applied?

Hi Stefano,

Yes indeed, that procedure is correct, I think the error you are seeing comes 
from the python version,
I am using python 3, version 3.10.6.

The error seems to come from python annotations, I’m surprised you didn’t hit 
it when testing the first patch,
did you use python2 for that?

Is it a problem if I developed the tool having in mind its usage with python3?

Cheers,
Luca



Re: [PATCH] xen/pt: fix igd passthrough for pc machine with xen accelerator

2023-05-17 Thread Chuck Zmudzinski
On 5/17/2023 2:39 AM, Michael Tokarev wrote:
> 08.02.2023 05:03, Chuck Zmudzinski wrote:
> > Commit 998250e97661 ("xen, gfx passthrough: register host bridge specific
> > to passthrough") uses the igd-passthrough-i440FX pci host device with
> > the xenfv machine type and igd-passthru=on, but using it for the pc
> > machine type, xen accelerator, and igd-passtru=on was omitted from that
> > commit.
> > 
> > The igd-passthru-i440FX pci host device is also needed for guests
> > configured with the pc machine type, the xen accelerator, and
> > igd-passthru=on. Specifically, tests show that not using the igd-specific
> > pci host device with the Intel igd passed through to the guest results
> > in slower startup performance and reduced resolution of the display
> > during startup. This patch fixes this issue.
> > 
> > To simplify the logic that is needed to support both the --enable-xen
> > and the --disable-xen configure options, introduce the boolean symbol
> > pc_xen_igd_gfx_pt_enabled() whose value is set appropriately in the
> > sysemu/xen.h header file as the test to determine whether or not
> > to use the igd-passthrough-i440FX pci host device instead of the
> > normal i440FX pci host device.
> > 
> > Fixes: 998250e97661 ("xen, gfx passthrough: register host bridge specific 
> > to passthrough")
> > Signed-off-by: Chuck Zmudzinski 
>
> Has this change been forgotten?  Is it not needed anymore?

Short answer:

After 4f67543b ("xen/pt: reserve PCI slot 2 for Intel igd-passthru ") was
applied, I was inclined to think this change is not needed anymore, but
it would not hurt to add this change also, and now I think it might be
more correct to also add this change.

Longer explanation:

I strongly desired that at least one of the patches I proposed to improve
support for Intel IGD passthrough with xen be committed. Since
4f67543b ("xen/pt: reserve PCI slot 2 for Intel igd-passthru ") that fixed
Intel IGD passthrough for the xenfv machine type has been committed,
I reasoned that there is not such a great need to also fix Intel IGD
passthrough for the pc machine type with xen so I did not push hard for
this patch to also be applied.

My requirement was that either the xenfv machine be fixed or the pc
machine be fixed. I did not think it was necessary to fix them both, and
4f67543b ("xen/pt: reserve PCI slot 2 for Intel igd-passthru ") fixed the
xenfv machine. But this patch provides the additional fix for the pc machine,
a fix that is distinct from the fix that has already been committed for the
xenfv machine, and it probably should also be applied so pc and xenfv
machines will work equally well with Intel IGD passthrough.

In other words, it is good to fix at least one of the two broken machines 
configured
for Intel IGD passthrough and xen, it is better to fix them both. We already 
fixed
one of them with 4f67543b ("xen/pt: reserve PCI slot 2 for Intel igd-passthru 
"),
this patch would fix the other one.

If you want to add this change also, let's make sure recent changes to the
xen header files do not require the patch to be rebased before committing
it.

Chuck



Re: [PATCH 6/6] x86/boot: Expose MSR_ARCH_CAPS data in guest max policies

2023-05-17 Thread Jan Beulich
On 16.05.2023 21:31, Andrew Cooper wrote:
> On 16/05/2023 3:53 pm, Jan Beulich wrote:
>> On 16.05.2023 16:16, Andrew Cooper wrote:
>>> On 16/05/2023 3:06 pm, Jan Beulich wrote:
 On 16.05.2023 15:51, Andrew Cooper wrote:
> On 16/05/2023 2:06 pm, Jan Beulich wrote:
>> On 15.05.2023 16:42, Andrew Cooper wrote:
>> Further is even just non-default exposure of all the various bits okay
>> to other than Dom0? IOW is there indeed no further adjustment necessary
>> to guest_rdmsr()?
 With your reply further down also sufficiently clarifying things for
 me (in particular pointing the one oversight of mine), the question
 above is the sole part remaining before I'd be okay giving my R-b here.
>>> Oh sorry.  Yes, it is sufficient.  Because VMs (other than dom0) don't
>>> get the ARCH_CAPS CPUID bit, reads of MSR_ARCH_CAPS will #GP.
>>>
>>> Right now, you can set cpuid = "host:arch-caps" in an xl.cfg file.  If
>>> you do this, you get to keep both pieces, as you'll end up advertising
>>> the MSR but with a value of 0 because of the note in patch 4.  libxl
>>> still only understand the xend CPUID format and can't express any MSR
>>> data at all.
>> Hmm, so the CPUID bit being max only results in all the ARCH_CAPS bits
>> getting turned off in the default policy. That is, to enable anything
>> you need to not only enable the CPUID bit, but also the ARCH_CAPS bits
>> you want enabled (with no presents means to do so).
> 
> Correct.
> 
>> I guess that's no
>> different from other max-only features with dependents, but I wonder
>> whether that's good behavior.
> 
> It's not really something you get a choice over.
> 
> Default is always less than max, so however you choose to express these
> concepts, when you're opting-in you're always having to put information
> back in which was previously stripped out.

But my point is towards the amount of data you need to specify manually.
I would find it quite helpful if default-on sub-features became available
automatically once the top-level feature was turned on. I guess so far
we don't have many such cases, but here you add a whole bunch.

>> Wouldn't it make more sense for the
>> individual bits' exposure qualifiers to become meaningful one to
>> qualifying feature is enabled? I.e. here this would then mean that
>> some ARCH_CAPS bits may become available, while others may require
>> explicit turning on (assuming they weren't all 'A').
> 
> I'm afraid I don't follow.  You could make some bits of MSR_ARCH_CAPS
> itself 'a' vs 'A', and that would have the expected effect (for any VM
> where arch_caps was visible).

Visible by default, you mean. Whereas I'm considering the case where
the CPUID bit is default-off, and turning it on for a guest doesn't at
the same time turn on all the 'A' bits in ARCH_CAPS (which hardware
offers, or which we synthesize).

Something similar could be seen / utilized for AMX, where in my
pending series I set all the bits to 'a', requiring every individual
bit to be turned on along with turning on AMX-TILE. Yet it would be
more user friendly if only the top-level bit needed enabling manually,
with available sub-features then becoming available "automatically".

> The thing which is 99% of the complexity with MSR_ARCH_CAPS is getting
> RSBA/RRSBA right.  The moment we advertise MSR_ARCH_CAPS to guests,
> RSBA/RRSBA must be set appropriately for migrate or we're creating a
> security vulnerability in the guest.
> 
> If you're wondering about the block disable, that's because MSRs and
> CPUID are different.  With CPUID, we have
> x86_cpu_policy_clear_out_of_range_leaves() which uses the various
> max_leaf.  e.g. a feat.max_leaf=0 is what causes all of subleaf 1 and 2
> to be zeroed in a policy.
> 
> 
>> But irrespective of that (which is kind of orthogonal) my question was
>> rather with already considering the point in time when the CPUID bit
>> would become 'A'. IOW I was wondering whether at that point having all
>> the individual bits be 'A' is actually going to be correct.
> 
> I've chosen all 'A' for these bits because that is what I expect to be
> correct in due course.  They're all the simple "you're not vulnerable to
> $X" bits, plus eIBRS which in practice is just a qualifying statement on
> IBRS (already fully supported in guests).

Right, upon checking again I agree.

Jan

> The rest of MSR_ARCH_CAPS is pretty much a dumping ground for all of the
> controls we can't give to guests under any circumstance.  (FB_CLEAR_CTRL
> might be an exception - allegedly we might want to give it to guests
> which have passthrough and trust their userspace, but I'm unconvinced of
> this argument and going to insist on concrete numbers from anyone
> wanting to try and implement this usecase.)
> 
> But there certainly could be a feature in there in the future where we
> leave it at 'a' for a while...  It's just feature bitmap data in a
> non-CPUID form factor.
> 
> ~Andrew




Re: [PATCH 1/2] xen/x86/pvh: use preset XSDT header for XSDT generation

2023-05-17 Thread Roger Pau Monné
On Tue, May 16, 2023 at 03:11:49PM -0700, Stefano Stabellini wrote:
> On Tue, 16 May 2023, Roger Pau Monné wrote:
> > On Tue, May 16, 2023 at 10:10:07AM +0200, Roger Pau Monné wrote:
> > > On Mon, May 15, 2023 at 05:16:27PM -0700, Stefano Stabellini wrote:
> > > > On Mon, 15 May 2023, Jan Beulich wrote:
> > > > > On 15.05.2023 10:48, Roger Pau Monné wrote:
> > > > > > On Fri, May 12, 2023 at 06:17:19PM -0700, Stefano Stabellini wrote:
> > > > > >> From: Stefano Stabellini 
> > > > > >>
> > > > > >> Xen always generates a XSDT table even if the firmware provided a 
> > > > > >> RSDT
> > > > > >> table. Instead of copying the XSDT header from the firmware table 
> > > > > >> (that
> > > > > >> might be missing), generate the XSDT header from a preset.
> > > > > >>
> > > > > >> Signed-off-by: Stefano Stabellini 
> > > > > >> ---
> > > > > >>  xen/arch/x86/hvm/dom0_build.c | 32 
> > > > > >> +---
> > > > > >>  1 file changed, 9 insertions(+), 23 deletions(-)
> > > > > >>
> > > > > >> diff --git a/xen/arch/x86/hvm/dom0_build.c 
> > > > > >> b/xen/arch/x86/hvm/dom0_build.c
> > > > > >> index 307edc6a8c..5fde769863 100644
> > > > > >> --- a/xen/arch/x86/hvm/dom0_build.c
> > > > > >> +++ b/xen/arch/x86/hvm/dom0_build.c
> > > > > >> @@ -963,13 +963,18 @@ static int __init pvh_setup_acpi_xsdt(struct 
> > > > > >> domain *d, paddr_t madt_addr,
> > > > > >>paddr_t *addr)
> > > > > >>  {
> > > > > >>  struct acpi_table_xsdt *xsdt;
> > > > > >> -struct acpi_table_header *table;
> > > > > >> -struct acpi_table_rsdp *rsdp;
> > > > > >>  const struct acpi_table_desc *tables = 
> > > > > >> acpi_gbl_root_table_list.tables;
> > > > > >>  unsigned long size = sizeof(*xsdt);
> > > > > >>  unsigned int i, j, num_tables = 0;
> > > > > >> -paddr_t xsdt_paddr;
> > > > > >>  int rc;
> > > > > >> +struct acpi_table_header header = {
> > > > > >> +.signature= "XSDT",
> > > > > >> +.length   = sizeof(struct acpi_table_header),
> > > > > >> +.revision = 0x1,
> > > > > >> +.oem_id   = "Xen",
> > > > > >> +.oem_table_id = "HVM",
> > > > > > 
> > > > > > I think this is wrong, as according to the spec the OEM Table ID 
> > > > > > must
> > > > > > match the OEM Table ID in the FADT.
> > > > > > 
> > > > > > We likely want to copy the OEM ID and OEM Table ID from the RSDP, 
> > > > > > and
> > > > > > possibly also the other OEM related fields.
> > > > > > 
> > > > > > Alternatively we might want to copy and use the RSDT on systems that
> > > > > > lack an XSDT, or even just copy the header from the RSDT into Xen's
> > > > > > crafted XSDT, since the format of the RSDP and the XSDT headers are
> > > > > > exactly the same (the difference is in the size of the description
> > > > > > headers that come after).
> > > > > 
> > > > > I guess I'd prefer that last variant.
> > > > 
> > > > I tried this approach (together with the second patch for necessity) and
> > > > it worked.
> > > > 
> > > > diff --git a/xen/arch/x86/hvm/dom0_build.c 
> > > > b/xen/arch/x86/hvm/dom0_build.c
> > > > index fd2cbf68bc..11d6d1bc23 100644
> > > > --- a/xen/arch/x86/hvm/dom0_build.c
> > > > +++ b/xen/arch/x86/hvm/dom0_build.c
> > > > @@ -967,6 +967,10 @@ static int __init pvh_setup_acpi_xsdt(struct 
> > > > domain *d, paddr_t madt_addr,
> > > >  goto out;
> > > >  }
> > > >  xsdt_paddr = rsdp->xsdt_physical_address;
> > > > +if ( !xsdt_paddr )
> > > > +{
> > > > +xsdt_paddr = rsdp->rsdt_physical_address;
> > > > +}
> > > >  acpi_os_unmap_memory(rsdp, sizeof(*rsdp));
> > > >  table = acpi_os_map_memory(xsdt_paddr, sizeof(*table));
> > > >  if ( !table )
> > > 
> > > To be slightly more consistent, could you use:
> > > 
> > > /*
> > >  * Note the header is the same for both RSDT and XSDT, so it's fine to
> > >  * copy the native RSDT header to the Xen crafted XSDT if no native
> > >  * XSDT is available.
> > >  */
> > > if (rsdp->revision > 1 && rsdp->xsdt_physical_address)
> > > sdt_paddr = rsdp->xsdt_physical_address;
> > > else
> > > sdt_paddr = rsdp->rsdt_physical_address;
> > > 
> > > It was an oversight of mine to not check for the RSDP revision, as
> > > RSDP < 2 will never have an XSDT.  Also add:
> > > 
> > > Fixes: 1d74282c455f ('x86: setup PVHv2 Dom0 ACPI tables')
> > 
> > Just realized this will require some more work so that the guest
> > (dom0) provided RSDP is at least revision 2.  You will need to adjust
> > the field and recalculate the checksum if needed.
> 
> But we are always providing RSDP version 2 in pvh_setup_acpi, right?

Yes, as said in the reply to Jan, just ignore this.

Thanks, Roger.



Re: [PATCH 2/2] xen/x86/pvh: copy ACPI tables to Dom0 instead of mapping

2023-05-17 Thread Roger Pau Monné
On Tue, May 16, 2023 at 04:34:09PM -0700, Stefano Stabellini wrote:
> On Tue, 16 May 2023, Roger Pau Monné wrote:
> > On Mon, May 15, 2023 at 05:11:25PM -0700, Stefano Stabellini wrote:
> > > On Mon, 15 May 2023, Roger Pau Monné wrote:
> > > > On Fri, May 12, 2023 at 06:17:20PM -0700, Stefano Stabellini wrote:
> > > > > From: Stefano Stabellini 
> > > > > 
> > > > > Mapping the ACPI tables to Dom0 PVH 1:1 leads to memory corruptions of
> > > > > the tables in the guest. Instead, copy the tables to Dom0.
> > > > > 
> > > > > This is a workaround.
> > > > > 
> > > > > Signed-off-by: Stefano Stabellini 
> > > > > ---
> > > > > As mentioned in the cover letter, this is a RFC workaround as I don't
> > > > > know the cause of the underlying problem. I do know that this patch
> > > > > solves what would be otherwise a hang at boot when Dom0 PVH attempts 
> > > > > to
> > > > > parse ACPI tables.
> > > > 
> > > > I'm unsure how safe this is for native systems, as it's possible for
> > > > firmware to modify the data in the tables, so copying them would
> > > > break that functionality.
> > > > 
> > > > I think we need to get to the root cause that triggers this behavior
> > > > on QEMU.  Is it the table checksum that fail, or something else?  Is
> > > > there an error from Linux you could reference?
> > > 
> > > I agree with you but so far I haven't managed to find a way to the root
> > > of the issue. Here is what I know. These are the logs of a successful
> > > boot using this patch:
> > > 
> > > [   10.437488] ACPI: Early table checksum verification disabled
> > > [   10.439345] ACPI: RSDP 0x4005F955 24 (v02 BOCHS )
> > > [   10.441033] ACPI: RSDT 0x4005F979 34 (v01 BOCHS  BXPCRSDT 
> > > 0001 BXPC 0001)
> > > [   10.444045] ACPI: APIC 0x40060F76 8A (v01 BOCHS  BXPCAPIC 
> > > 0001 BXPC 0001)
> > > [   10.445984] ACPI: FACP 0x4005FA65 74 (v01 BOCHS  BXPCFACP 
> > > 0001 BXPC 0001)
> > > [   10.447170] ACPI BIOS Warning (bug): Incorrect checksum in table 
> > > [FACP] - 0x67, should be 0x30 (20220331/tbprint-174)
> > > [   10.449522] ACPI: DSDT 0x4005FB19 00145D (v01 BOCHS  BXPCDSDT 
> > > 0001 BXPC 0001)
> > > [   10.451258] ACPI: FACS 0x4005FAD9 40
> > > [   10.452245] ACPI: Reserving APIC table memory at [mem 
> > > 0x40060f76-0x40060fff]
> > > [   10.452389] ACPI: Reserving FACP table memory at [mem 
> > > 0x4005fa65-0x4005fad8]
> > > [   10.452497] ACPI: Reserving DSDT table memory at [mem 
> > > 0x4005fb19-0x40060f75]
> > > [   10.452602] ACPI: Reserving FACS table memory at [mem 
> > > 0x4005fad9-0x4005fb18]
> > > 
> > > 
> > > And these are the logs of the same boot (unsuccessful) without this
> > > patch:
> > > 
> > > [   10.516015] ACPI: Early table checksum verification disabled
> > > [   10.517732] ACPI: RSDP 0x40060F1E 24 (v02 BOCHS )
> > > [   10.519535] ACPI: RSDT 0x40060F42 34 (v01 BOCHS  BXPCRSDT 
> > > 0001 BXPC 0001)
> > > [   10.522523] ACPI: APIC 0x40060F76 8A (v01 BOCHS  BXPCAPIC 
> > > 0001 BXPC 0001)
> > > [   10.527453] ACPI:  0x7FFE149D  (v255 �� 
> > >    )
> > > [   10.528362] ACPI: Reserving APIC table memory at [mem 
> > > 0x40060f76-0x40060fff]
> > > [   10.528491] ACPI: Reserving  table memory at [mem 
> > > 0x7ffe149d-0x17ffe149b]
> > > 
> > > It is clearly a memory corruption around FACS but I couldn't find the
> > > reason for it. The mapping code looks correct. I hope you can suggest a
> > > way to narrow down the problem. If I could, I would suggest to apply
> > > this patch just for the QEMU PVH tests but we don't have the
> > > infrastructure for that in gitlab-ci as there is a single Xen build for
> > > all tests.
> > 
> > Would be helpful to see the memory map provided to Linux, just in case
> > we messed up and there's some overlap.
> 
> Everything looks correct. Here are some more logs:
> 
> (XEN) Xen-e820 RAM map:
> (XEN)  [, 0009fbff] (usable)
> (XEN)  [0009fc00, 0009] (reserved)
> (XEN)  [000f, 000f] (reserved)
> (XEN)  [0010, 7ffd] (usable)
> (XEN)  [7ffe, 7fff] (reserved)
> (XEN)  [fffc, ] (reserved)
> (XEN)  [00fd, 00ff] (reserved)
> (XEN) Microcode loading not available
> (XEN) New Xen image base address: 0x7f60
> (XEN) System RAM: 2047MB (2096636kB)
> (XEN) ACPI: RSDP 000F58D0, 0014 (r0 BOCHS )
> (XEN) ACPI: RSDT 7FFE1B21, 0034 (r1 BOCHS  BXPC1 BXPC1)
> (XEN) ACPI: FACP 7FFE19CD, 0074 (r1 BOCHS  BXPC1 BXPC1)
> (XEN) ACPI: DSDT 7FFE0040, 198D (r1 BOCHS  BXPC1 BXPC1)
> (XEN) ACPI: FACS 7FFE, 0040
> (XEN) ACPI: APIC 7FFE1A41, 0080 (r1 BOCHS  BXPC1 BXPC1)
> (XEN) ACPI: HPET 7FFE1AC1, 0038 (r1 BOCHS  BXPC   

[xen-4.17-testing test] 180683: tolerable trouble: fail/pass/starved - PUSHED

2023-05-17 Thread osstest service owner
flight 180683 xen-4.17-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180683/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180446
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180446
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180446
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180446
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180446
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180446
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180446
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180446
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180446
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180446
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180446
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180446
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-xsm   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-thunderx  3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-vhd   3 hosts-allocate   starved  n/a
 test-arm64-arm64-xl-credit1   3 hosts-allocate   starved  n/a

version targeted for testing:
 xen  b773c48e368d9cf1ea29b259fe4ae434b8bb42da
baseline version:
 xen  0880df6f5f905bffc86dd181a8af64f16cc62110

Last test of basis   180446  2023-04-27 13:08:27 Z   19 days
Testing same since   180683  2023-05-16 15:38:22 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Roger Pau Monné 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64-xtf  pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386

Re: [PATCH] xen/pt: fix igd passthrough for pc machine with xen accelerator

2023-05-17 Thread Michael Tokarev

08.02.2023 05:03, Chuck Zmudzinski wrote:

Commit 998250e97661 ("xen, gfx passthrough: register host bridge specific
to passthrough") uses the igd-passthrough-i440FX pci host device with
the xenfv machine type and igd-passthru=on, but using it for the pc
machine type, xen accelerator, and igd-passtru=on was omitted from that
commit.

The igd-passthru-i440FX pci host device is also needed for guests
configured with the pc machine type, the xen accelerator, and
igd-passthru=on. Specifically, tests show that not using the igd-specific
pci host device with the Intel igd passed through to the guest results
in slower startup performance and reduced resolution of the display
during startup. This patch fixes this issue.

To simplify the logic that is needed to support both the --enable-xen
and the --disable-xen configure options, introduce the boolean symbol
pc_xen_igd_gfx_pt_enabled() whose value is set appropriately in the
sysemu/xen.h header file as the test to determine whether or not
to use the igd-passthrough-i440FX pci host device instead of the
normal i440FX pci host device.

Fixes: 998250e97661 ("xen, gfx passthrough: register host bridge specific to 
passthrough")
Signed-off-by: Chuck Zmudzinski 


Has this change been forgotten?  Is it not needed anymore?

Thanks,

/mjt



[xen-unstable test] 180682: tolerable FAIL - PUSHED

2023-05-17 Thread osstest service owner
flight 180682 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180682/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180671
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180671
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180671
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180671
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180671
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180671
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180671
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180671
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180671
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180671
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180671
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180671
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  8f9c8274a4e3e860bd777269cb2c91971e9fa69e
baseline version:
 xen  b8be19ce432a2edd69c0673768a0beeec77f795a

Last test of basis   180671  2023-05-15 13:38:21 Z1 days
Failing since180677  2023-05-15 23:07:02 Z1 days2 attempts
Testing same since   180682  2023-05-16 15:10:00 Z0 days1 attempts


People who touched 

fix qemu to build with gcc13

2023-05-17 Thread Olaf Hering
Hello,

please backport d66ba6dc1cce914673bd8a89fca30a7715ea70d1 to
qemu-xen.git to allow building it with gcc13.

Thanks,
Olaf


pgpN4YcYzn_NI.pgp
Description: Digitale Signatur von OpenPGP