Re: [PATCH] xen/usb: harden xen_hcd against malicious backends

[Juergen Gross]

On 15.03.22 18:41, Greg Kroah-Hartman wrote:

On Fri, Mar 11, 2022 at 11:35:09AM +0100, Juergen Gross wrote:

Make sure a malicious backend can't cause any harm other than wrong
I/O data.

Missing are verification of the request id in a response, sanitizing
the reported actual I/O length, and protection against interrupt storms
from the backend.

Signed-off-by: Juergen Gross 
---
  drivers/usb/host/xen-hcd.c | 57 --
  1 file changed, 43 insertions(+), 14 deletions(-)


Fails to apply to my tree:

checking file drivers/usb/host/xen-hcd.c
Hunk #2 succeeded at 720 (offset -1 lines).
Hunk #3 succeeded at 807 (offset -3 lines).
Hunk #4 succeeded at 934 (offset -5 lines).
Hunk #5 FAILED at 986.
Hunk #6 succeeded at 1003 with fuzz 1 (offset -10 lines).
Hunk #7 succeeded at 1048 (offset -10 lines).
Hunk #8 succeeded at 1072 (offset -10 lines).
Hunk #9 succeeded at 1161 (offset -10 lines).
Hunk #10 succeeded at 1516 (offset -10 lines).
1 out of 10 hunks FAILED

Any hints?


Rebase your tree to v5.17-rc8? It is missing the recent security
patches which modified drivers/usb/host/xen-hcd.c.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: An error due to installation that require binutils package

[John Simpson via Gcc-help]

Hi again!
Review the essential case please:


https://onedrive.live.com/download?cid=7525DC7DC7D0C530=7525DC7DC7D0C530%21107=APAIWBtplog6Z2E





File password: E4345

Hello,

Kindly ask you to have a look at this bug.
Thank you for your replies.

On Mon, Mar 29, 2021 at 7:07 PM George Dunlap 
wrote:

> John,
>
> Thanks for your report.  Can you post your bug report
>  ?
>
> The bug is in the compilation of QEMU, which is an external project; so
> it’s possible that we’ll end up having to raise this with that community as
> well.
>
> Thanks,
>  -George Dunlap
>
> > On Mar 28, 2021, at 2:26 PM, John Simpson <> wrote:
> >
> > Hello,
> >
> > Just forwarding this message to you. Can you give some thoughs about
> this? Thanks a lot.
> >
> >
> > -- Forwarded message -
> > From: Alan Modra <>
> > Date: Sun, Mar 28, 2021 at 2:21 PM
> > Subject: Re: An error due to installation that require binutils package.
> > To: John Simpson <>
> > Cc: <>
> >
> >
> > On Sun, Mar 28, 2021 at 12:55:23PM +0300, John Simpson via Binutils
> wrote:
> > >   BUILD   pc-bios/optionrom/kvmvapic.img
> > > ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
> >
> > -no-pie is a gcc option.  Neither -no-pie nor --no-pie is a valid ld
> > option.  The fault lies with whatever passed -no-pie to ld.
> >
> > --
> > Alan Modra
> > Australia Development Lab, IBM
> >
> >
> >
> > -- Forwarded message -
> > From: Andreas Schwab <>
> > Date: Sun, Mar 28, 2021 at 2:17 PM
> > Subject: Re: An error due to installation that require binutils package.
> > To: John Simpson via Binutils <>
> > Cc: John Simpson <>
> >
> >
> > Please report that to the xen project.  ld -no-pie doesn't have a useful
> > meaning.  It used to mean the same as ld -n -o-pie, which sets "-pie" as
> > the output file name.
> >
> > Andreas.
> >
> > --
> > Andreas Schwab, 
> > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> > "And now for something completely different."
> >
> >
> >
> > -- Forwarded message -
> > From: John Simpson <>
> > Date: Sun, Mar 28, 2021 at 12:55 PM
> > Subject: An error due to installation that require binutils package.
> > To: <>
> >
> >
> > Hello,
> >
> > Recently I got a following error due to installation xen on
> 5.11.6-1-MANJARO kernel:
> >
> >   GEN target/riscv/trace.c
> >   GEN target/s390x/trace.c
> >   GEN target/sparc/trace.c
> >   GEN util/trace.c
> >   GEN config-all-devices.mak
> > make[1]: Entering directory
> '/home/username/xen/src/xen-4.14.1/tools/qemu-xen/slirp'
> > make[1]: Nothing to be done for 'all'.
> > make[1]: Leaving directory
> '/home/username/xen/src/xen-4.14.1/tools/qemu-xen/slirp'
> >   BUILD   pc-bios/optionrom/multiboot.img
> >   BUILD   pc-bios/optionrom/linuxboot.img
> >   BUILD   pc-bios/optionrom/linuxboot_dma.img
> >   BUILD   pc-bios/optionrom/kvmvapic.img
> > ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
> > make[1]: *** [Makefile:53: multiboot.img] Error 1
> > make[1]: *** Waiting for unfinished jobs
> > ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
> > make[1]: *** [Makefile:53: linuxboot_dma.img] Error 1
> >   BUILD   pc-bios/optionrom/pvh.img
> > ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
> > make[1]: *** [Makefile:53: linuxboot.img] Error 1
> > ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
> > make[1]: *** [Makefile:53: kvmvapic.img] Error 1
> > ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
> > make[1]: *** [Makefile:50: pvh.img] Error 1
> > make: *** [Makefile:581: pc-bios/optionrom/all] Error 2
> > make: Leaving directory
> '/home/username/xen/src/xen-4.14.1/tools/qemu-xen-build'
> > make[3]: *** [Makefile:218: subdir-all-qemu-xen-dir] Error 2
> > make[3]: Leaving directory '/home/username/xen/src/xen-4.14.1/tools'
> > make[2]: ***
> [/home/username/xen/src/xen-4.14.1/tools/../tools/Rules.mk:235:
> subdirs-install] Error 2
> > make[2]: Leaving directory '/home/username/xen/src/xen-4.14.1/tools'
> > make[1]: *** [Makefile:72: install] Error 2
> > make[1]: Leaving directory '/home/username/xen/src/xen-4.14.1/tools'
> > make: *** [Makefile:134: install-tools] Error 2
> > ==> ERROR: A failure occurred in build().
> > Aborting...
> >
> > Currently I have fresh binutils 2.36.1-2 and it seems to me that the
> issue is related to this part of code:
> >
> > github.com/bminor/binutils-gdb/blob/master/ld/lexsup.c#L451
> >
> > It seems to me that this could impact far more users than just me.
> >
>
>

[linux-linus test] 168620: tolerable FAIL - PUSHED

[osstest service owner]

flight 168620 linux-linus real [real]
flight 168628 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/168620/
http://logs.test-lab.xenproject.org/osstest/logs/168628/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-freebsd11-amd64 13 guest-start fail pass in 168628-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 168575
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 168575
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168575
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168575
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 168575
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168575
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 168575
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 168575
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-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-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-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-arm64-arm64-xl-credit1  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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-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-armhf-armhf-libvirt 15 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-armhf-armhf-xl-credit1  15 migrate-support-checkfail   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-credit1  16 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-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 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-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass

version targeted for testing:
 linux56e337f2cf1326323844927a04e9dbce9a244835
baseline version:
 linux09688c0166e76ce2fb85e86b9d99be8b0084cdf9

Last test of basis   168575  2022-03-14 03:07:00 Z2 days
Testing same since   168620  2022-03-15 18:10:19 Z0 days1 attempts


People who touched revisions under test:
  Bartosz Golaszewski 
  Eli Cohen 
  Jason Wang 
  Linus Torvalds 
  Michael S. Tsirkin 

jobs:
 build-amd64-xsm  

[ovmf test] 168627: regressions - FAIL

[osstest service owner]

flight 168627 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168627/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf c8ea48bdf95532f9a3a4c39a154c09988566901f
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   15 days  147 attempts
Testing same since   168617  2022-03-15 15:41:41 Z0 days6 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jagadeesh Ujja 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sami Mujawar 
  Sean Rhodes 
  Sebastien Boeuf 
  Sunny Wang 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 653 lines long.)

[ovmf test] 168625: regressions - FAIL

[osstest service owner]

flight 168625 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168625/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf c8ea48bdf95532f9a3a4c39a154c09988566901f
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   15 days  146 attempts
Testing same since   168617  2022-03-15 15:41:41 Z0 days5 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jagadeesh Ujja 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sami Mujawar 
  Sean Rhodes 
  Sebastien Boeuf 
  Sunny Wang 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 653 lines long.)

Re: [XEN PATCH] tools/libs/light/libxl_pci.c: explicitly grant access to Intel IGD opregion

[Chuck Zmudzinski]

On 3/15/22 7:38 AM, Jan Beulich wrote:

On 14.03.2022 04:41, Chuck Zmudzinski wrote:


@@ -610,6 +612,45 @@ out:
  return ret;
  }
  
+static uint32_t sysfs_dev_get_igd_opregion(libxl__gc *gc,

+   libxl_device_pci *pci)
+{
+char *pci_device_config_path =
+GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/config",
+  pci->domain, pci->bus, pci->dev, pci->func);
+size_t read_items;
+uint32_t igd_opregion;
+uint32_t error = 0x;

I think this constant wants to gain a #define, to be able to correlate
the use sites. I'm also not sure of the value - in principle the
register can hold this value, but of course then it won't be 3 pages.



I have one more comment to add here. I am not intending
to define igd_opregion as a data structure 3 pages (12k)
long, much less as a pointer to such a structure. However,
it would be nice to have access to the actual data structure
in libxl, because we could use it to validate its contents.
I looked in the code for the i915 Linux kernel module, and
the IGD opregion does have a signature that we could check
if we have access to it. That would mitigate my concerns
expressed in my first version of the patch about a false
positive when identifying an Intel IGD. Hvmloader should
probably also do this check before it maps the Intel IGD
into guest memory if that is possible. However, I expect
that it is not a memory that libxl has access to. It is
probably a structure in kernel space, but it might be
possible for libxl to ask the hypervisor for access to it.
Perhaps the libxl maintainers can shed some light on that
possibility. If this is possible, I will include such a check for
the validity of the contents in the IGD in version 2 of the
patch.

Regards,

Chuck

Re: [PATCH 1/2] xen/grant-table: remove gnttab_*transfer*() functions

[Boris Ostrovsky]

On 3/15/22 2:10 AM, Juergen Gross wrote:

On 15.03.22 00:37, Boris Ostrovsky wrote:


On 3/11/22 5:34 AM, Juergen Gross wrote:

All grant table operations related to the "transfer" functionality
are unused currently. There have been users in the old days of the
"Xen-o-Linux" kernel, but those didn't make it upstream.

So remove the "transfer" related functions.



Do we need to assert somewhere that transfer flags are not set?


This would be an orthogonal change, right? My patch is just removing
never called functions.



I was thinking of having this done as part of code removal (maybe as a separate 
patch).



In any case I believe checking those flags is the job of the hypervisor.
If an operation is illegal due to a transfer flag being set, it needs to
be rejected at hypervisor level.


True.

Reviewed-by: Boris Ostrovsky 

Re: [PATCH 12/15] swiotlb: provide swiotlb_init variants that remap the buffer

[Boris Ostrovsky]

On 3/15/22 2:36 AM, Christoph Hellwig wrote:


@@ -271,12 +273,23 @@ void __init swiotlb_init(bool addressing_limit, unsigned 
int flags)
 * allow to pick a location everywhere for hypervisors with guest
 * memory encryption.
 */
+retry:
+   bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
if (flags & SWIOTLB_ANY)
tlb = memblock_alloc(bytes, PAGE_SIZE);
else
tlb = memblock_alloc_low(bytes, PAGE_SIZE);
if (!tlb)
goto fail;
+   if (remap && remap(tlb, nslabs) < 0) {
+   memblock_free(tlb, PAGE_ALIGN(bytes));
+
+   if (nslabs <= IO_TLB_MIN_SLABS)
+   panic("%s: Failed to remap %zu bytes\n",
+ __func__, bytes);
+   nslabs = max(1024UL, ALIGN(nslabs >> 1, IO_TLB_SEGSIZE));



I spoke with Konrad (who wrote the original patch --- 
f4b2f07b2ed9b469ead87e06fc2fc3d12663a725) and apparently the reason for 2MB was 
to optimize for Xen's slab allocator, it had nothing to do with 
IO_TLB_MIN_SLABS. Since this is now common code we should not expose 
Xen-specific optimizations here and smaller values will still work so 
IO_TLB_MIN_SLABS is fine.

I think this should be mentioned in the commit message though, probably best in 
the next patch where you switch to this code.

As far as the hunk above, I don't think we need the max() here: with 
IO_TLB_MIN_SLABS being 512 we may get stuck in an infinite loop. Something like

nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE);
if (nslabs <= IO_TLB_MIN_SLABS)
panic()

should be sufficient.



+   goto retry;
+   }
if (swiotlb_init_with_tbl(tlb, default_nslabs, flags))
goto fail_free_mem;
return;
@@ -287,12 +300,18 @@ void __init swiotlb_init(bool addressing_limit, unsigned 
int flags)
pr_warn("Cannot allocate buffer");
  }
  
+void __init swiotlb_init(bool addressing_limit, unsigned int flags)

+{
+   return swiotlb_init_remap(addressing_limit, flags, NULL);
+}
+
  /*
   * Systems with larger DMA zones (those that don't support ISA) can
   * initialize the swiotlb later using the slab allocator if needed.
   * This should be just like above, but with some error catching.
   */
-int swiotlb_init_late(size_t size, gfp_t gfp_mask)
+int swiotlb_init_late(size_t size, gfp_t gfp_mask,
+   int (*remap)(void *tlb, unsigned long nslabs))
  {
unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_SEGSIZE);
unsigned long bytes;
@@ -303,6 +322,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask)
if (swiotlb_force_disable)
return 0;
  
+retry:

order = get_order(nslabs << IO_TLB_SHIFT);
nslabs = SLABS_PER_PAGE << order;
bytes = nslabs << IO_TLB_SHIFT;
@@ -317,6 +337,16 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask)
  
  	if (!vstart)

return -ENOMEM;
+   if (remap)
+   rc = remap(vstart, nslabs);
+   if (rc) {
+   free_pages((unsigned long)vstart, order);
+
+   if (IO_TLB_MIN_SLABS <= 1024)
+   return rc;
+   nslabs = max(1024UL, ALIGN(nslabs >> 1, IO_TLB_SEGSIZE));



Same here. (The 'if' check above is wrong anyway).

Patches 13 and 14 look good.


-boris




+   goto retry;
+   }
  
  	if (order != get_order(bytes)) {

pr_warn("only able to allocate %ld MB\n",

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

[osstest service owner]

flight 168615 xen-unstable real [real]
flight 168624 xen-unstable real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/168615/
http://logs.test-lab.xenproject.org/osstest/logs/168624/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 168624-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 168604
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168604
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168604
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 168604
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 168604
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 168604
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 168604
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 168604
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 168604
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 168604
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168604
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 168604
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  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-credit2  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-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   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-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-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-raw 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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 

[ovmf test] 168623: regressions - FAIL

[osstest service owner]

flight 168623 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168623/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf c8ea48bdf95532f9a3a4c39a154c09988566901f
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  145 attempts
Testing same since   168617  2022-03-15 15:41:41 Z0 days4 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jagadeesh Ujja 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sami Mujawar 
  Sean Rhodes 
  Sebastien Boeuf 
  Sunny Wang 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 653 lines long.)

[PATCH] tools/ctrl: add xc_domain_hvm_getcontext_size

[Tamas K Lengyel]

Current users of xc_domain_hvm_getcontext_partial rely on knowing ahead of time
the size of the requested HVM context. This isn't exactly prudent as the size
may change across Xen versions, requiring callers to re-compile in lockstep
with the Xen headers. This isn't an issue for in-tree tools and libraries,
but out-of-tree tools that supposed to work across all Xen versions, like
LibVMI, would fail to work after a size change to any of the HVM contexts.

Add xc_domain_hvm_getcontext_size so that external callers can at least be
backwards compatible with various HVM context sizes by maintaining their
own internal definitions of them and not relying on the current Xen header. It
is preferred to know the exact size Xen is going to return instead of guessing
by associating the HVM context sizes with Xen releases. The underlying domctl
has supported returning the context size to begin with, so with this change
we are just exposing that functionality to the users of libxc.

Signed-off-by: Tamas K Lengyel 
---
 tools/include/xenctrl.h | 12 
 tools/libs/ctrl/xc_domain.c | 21 +
 2 files changed, 33 insertions(+)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 95bd5eca67..ee5095a892 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -787,6 +787,18 @@ int xc_domain_hvm_getcontext_partial(xc_interface *xch,
  uint16_t instance,
  void *ctxt_buf,
  uint32_t size);
+/**
+ * This function returns the size of the specified context of a hvm domain
+ * @parm xch a handle to an open hypervisor interface
+ * @parm domid the domain to get information from
+ * @parm typecode which type of elemnt required
+ * @parm instance which instance of the type
+ * @return size on success, 0 on failure
+ */
+unsigned long xc_domain_hvm_getcontext_size(xc_interface *xch,
+uint32_t domid,
+uint16_t typecode,
+uint16_t instance);
 
 /**
  * This function will set the context for hvm domain
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index ef62f66009..f52d08 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -510,6 +510,27 @@ int xc_domain_hvm_getcontext_partial(xc_interface *xch,
 return ret ? -1 : 0;
 }
 
+unsigned long xc_domain_hvm_getcontext_size(xc_interface *xch,
+uint32_t domid,
+uint16_t typecode,
+uint16_t instance)
+{
+int ret;
+void *p = NULL;
+DECLARE_DOMCTL;
+DECLARE_HYPERCALL_BOUNCE(p, 0, XC_HYPERCALL_BUFFER_BOUNCE_OUT);
+
+domctl.cmd = XEN_DOMCTL_gethvmcontext_partial;
+domctl.domain = domid;
+domctl.u.hvmcontext_partial.type = typecode;
+domctl.u.hvmcontext_partial.instance = instance;
+set_xen_guest_handle(domctl.u.hvmcontext_partial.buffer, p);
+
+ret = do_domctl(xch, );
+
+return ret ? 0 : domctl.u.hvmcontext_partial.bufsz;
+}
+
 /* set info to hvm guest for restore */
 int xc_domain_hvm_setcontext(xc_interface *xch,
  uint32_t domid,
-- 
2.32.0

[ovmf test] 168622: regressions - FAIL

[osstest service owner]

flight 168622 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168622/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf c8ea48bdf95532f9a3a4c39a154c09988566901f
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  144 attempts
Testing same since   168617  2022-03-15 15:41:41 Z0 days3 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jagadeesh Ujja 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sami Mujawar 
  Sean Rhodes 
  Sebastien Boeuf 
  Sunny Wang 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 653 lines long.)

[ovmf test] 168618: regressions - FAIL

[osstest service owner]

flight 168618 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168618/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf c8ea48bdf95532f9a3a4c39a154c09988566901f
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  143 attempts
Testing same since   168617  2022-03-15 15:41:41 Z0 days2 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jagadeesh Ujja 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sami Mujawar 
  Sean Rhodes 
  Sebastien Boeuf 
  Sunny Wang 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 653 lines long.)

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

[osstest service owner]

flight 168612 qemu-mainline real [real]
flight 168619 qemu-mainline real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/168612/
http://logs.test-lab.xenproject.org/osstest/logs/168619/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 18 guest-localmigrate/x10 fail 
pass in 168619-retest
 test-arm64-arm64-libvirt-raw 13 guest-start fail pass in 168619-retest

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-raw 14 migrate-support-check fail in 168619 never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-check fail in 168619 never 
pass
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168601
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168601
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 168601
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 168601
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 168601
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 168601
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 168601
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168601
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-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-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   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-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  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-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  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-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 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

version targeted for testing:
 qemuu6f4fe14b46f0a161f94e3f6e98690ac38184b0be
baseline version:
 qemuu 

Re: [PATCH 36/36] doc, arm: add usage documentation for cache coloring support

[Julien Grall]

Hi Marco,

On 04/03/2022 17:47, Marco Solieri wrote:

From: Luca Miccio 

Add basic documentation that shows how cache coloring support can be
used in Xen. It introduces the basic concepts behind cache coloring,
defines the cache selection format, and explains how to assign colors to
the supported domains: Dom0, DomUs and Xen itself. Known issues are
also reported.

Signed-off-by: Luca Miccio 
Signed-off-by: Marco Solieri 
---
  docs/misc/arm/cache_coloring.rst | 191 +++
  1 file changed, 191 insertions(+)
  create mode 100644 docs/misc/arm/cache_coloring.rst

diff --git a/docs/misc/arm/cache_coloring.rst b/docs/misc/arm/cache_coloring.rst
new file mode 100644
index 00..082afb1b6c
--- /dev/null
+++ b/docs/misc/arm/cache_coloring.rst
@@ -0,0 +1,191 @@
+Xen coloring support user's guide
+=
+
+The cache coloring support in Xen allows to reserve last level cache partition
AFAICT, the code is assuming that the Last level cache is L3. However, 
this documentation looks generic enough that someone could think it can 
be used on any platforms.



+for Dom0, DomUs and Xen itself. Currently only ARM64 is supported.


What is missing to support arm32?


+
+In order to enable and use it, few steps are needed.
+
+- Enable coloring in XEN configuration file.
+
+CONFIG_COLORING=y
+
+- Enable/disable debug information (optional).
+
+CONFIG_COLORING_DEBUG=y/n


This option doesn't seem to exist.


+
+Before digging into configuration instructions, configurers should first


I would write "system integrator" rather than "configurers".


+understand the basics of cache coloring.


I read this as a suggestion to read external documentation. Do you have 
good pointer?



+
+Background
+**
+
+Cache hierarchy of a modern multi-core CPU typically has first levels dedicated
+to each core (hence using multiple cache units), while the last level is shared
+among all of them. Such configuration implies that memory operations on one
+core (e.g. running a DomU) are able to generate interference on another core
+(e.g .hosting another DomU). Cache coloring allows eliminating this
+mutual interference, and thus guaranteeing higher and more predictable
+performances for memory accesses.
+The key concept underlying cache coloring is a fragmentation of the memory
+space into a set of sub-spaces called colors that are mapped to disjoint cache
+partitions. Technically, the whole memory space is first divided into a number
+of subsequent regions. Then each region is in turn divided into a number of
+subsequent sub-colors. The generic i-th color is then obtained by all the
+i-th sub-colors in each region.
+
+.. raw:: html
+
+
+Region jRegion j+1
+.   
+. . .
+.   .
+_ _ ___ _ _ _ _
+| | | | | | |
+| c_0 | c_1 | | c_n | c_0 | c_1 |
+   _ _ _|_|_|_ _ _|_|_|_|_ _ _
+:   :
+:   :... ... .
+:color 0
+:... ... .
+:
+  . . ..:
+
+
+There are two pragmatic lesson to be learnt.
+
+1. If one wants to avoid cache interference between two domains, different
+   colors needs to be used for their memory.
+
+2. Color assignment must privilege contiguity in the partitioning. E.g.,
+   assigning colors (0,1) to domain I  and (2,3) to domain  J is better than
+   assigning colors (0,2) to I and (1,3) to J.
+
+
+Color(s) selection format
+**
+
+Regardless of the domain that has to be colored (Dom0, DomUs and Xen),


Xen is not really a domain. How about 'memory pool'?


+the color selection can be expressed using the same syntax.  In particular,


Here you are saying the syntax is the same for everyone. But below, you 
provide a new syntax for dom0less domUs.



+the latter is expressed as a comma-separated list of hyphen-separated intervals
+of color numbers, as in `0-4,5-8,10-15`.  Ranges are always represented using
+strings. Note that no spaces are allowed.
+
+The number of available colors depends on the LLC layout of the specific
+platform and determines the maximum allowed value.  This number can be either
+calculated [#f1]_ or read from the output given by the hypervisor during boot,
+if DEBUG logging is enabled.


I think it would be good to print the number of colors even in non-debug 
build.



+
+Examples:
+
++-+---+
+|**Configuration**|**Actual selection**   |

Re: [PATCH early-RFC 4/5] xen/arm: mm: Rework switch_ttbr()

[Julien Grall]

Hi Stefano,

On 14/03/2022 23:48, Stefano Stabellini wrote:

- we save the current mapping
- update it with the Xen 1:1
- switch_ttbr
- remove Xen 1:1
- restore mapping

It should work, right? Basically, a mapping conflict shouldn't be an
issue given that the mapping has only to live long enough to call
switch_ttbr_id.


Today switch_ttbr() is called before we initialized most of the memory layout.
So clashing with the VMAP and frametable is not a problem.

However, the identity mapping may also clash with the region used to map Xen.
That said, technically, we are not able to handle Xen when its start address
is in region 2MB + 4K to 4MB (Xen is loaded at a 4KB aligned address).

The trouble is some features (e.g. UBSAN, GCOV) can generate Xen image over
2MB. IOW, the range where Xen cannot be loaded will increase.

This is an issue because AFAIK, there is no away to tell GRUB "You can't load
Xen at this region". But even if there were one, I feel this restriction is
sort of random.

I already wrote a patch to get rid of the restriction. The code is not too bad
(we only need an extra indirection). But I haven't sent it yet because it is
less critical with the re-shuffling of the memory layout.


Interesting! I am curious: how did you manage to do it?


When the identity mapping is clashing with Xen runtime address, I am 
creating a temporary mapping for Xen at a different fixed address.


Once the MMU is turned on, we can jump to the temporary mapping. After 
that we are safe to remove the identity mapping and create the runtime 
Xen mapping. The last step is to jump on the runtime mapping and then 
remove the temporary mapping.




For now and for this series the current approach and the 512GB limit are
fine. My replies here are brainstorming to see if there are potential
alternatives in the future in case the need arises.


On Arm64, we have 256TB worth of virtual address. So I think we can 
easily spare 512GB for the foreseeable :).


If we are at the point where we can't space 512GB, then we would need to 
have a more dynamic layout as I plan on arm32.


Xen would still be mapped at a specific virtual address so we don't need 
to update the relocations. But we could decide at runtime the position 
of other large mappings (e.g. vmap, domheap).


Probably the safest way is to link Xen at a very high address. It is 
quite unlikely that Xen will be loaded at such high address.


If it is, we could exceptionally relocate Xen (with this series it 
should be safer to do). That said, I would like to avoid relocating Xen 
until we see a use for that.




I can see that a clash with Xen mapping could be problematic and the
chances of that happening are low but non-zero. We could make sure that
ImageBuilder always picks safe addresses and that would help but
wouldn't remove the issue if someone is not using ImageBuilder.


AFAIU, ImageBuilder is here to cater U-boot users. I am not too worry 
about those setups because a user can pick any address they want. So as 
long as Xen print an error during the clash (already the case), the user 
can easily update their scripts.


This is more a problem for UEFI/GRUB where, AFAICT, we can't control 
where Xen will be loaded.


Cheers,

--
Julien Grall

Re: [XEN PATCH] tools/libs/light/libxl_pci.c: explicitly grant access to Intel IGD opregion

[Chuck Zmudzinski]

On 3/15/22 7:38 AM, Jan Beulich wrote:

On 14.03.2022 04:41, Chuck Zmudzinski wrote:

Fixes: abfb006f1ff4 (tools/libxl: explicitly grant access to needed I/O-memory 
ranges)
Fixes: 0561e1f01e87 (xen/common: do not implicitly permit access to mapped I/O 
memory)
Backport: 4.12+

Just fyi: This is fine to have as a tag, but it wouldn't be backported
farther than to 4.15.


That's entirely reasonable. I understand this is a bug fix, not a
security issue.


Apart from this largely some style issues (see below), but please
realize that I'm not a libxl maintainer and hence I may not have good
enough knowledge of, in particular, potential unwritten conventions.


I will take your comments into consideration regarding style before
writing the next version of the patch, and carefully check libxl's
coding style file.



@@ -610,6 +612,45 @@ out:
  return ret;
  }
  
+static uint32_t sysfs_dev_get_igd_opregion(libxl__gc *gc,

+   libxl_device_pci *pci)
+{
+char *pci_device_config_path =
+GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/config",
+  pci->domain, pci->bus, pci->dev, pci->func);
+size_t read_items;
+uint32_t igd_opregion;
+uint32_t error = 0x;

I think this constant wants to gain a #define, to be able to correlate
the use sites. I'm also not sure of the value - in principle the
register can hold this value, but of course then it won't be 3 pages.


What we are storing as the return value is the starting address,
not the size, of the opregion, and that is a 32-bit value. If we
cannot read it, we return 0x instead to indicate the error
that we were unable to read the starting address of the opregion
from the config attribute in sysfs. The 32-bit value we are looking for
is read at offset 0xfc from the start of the config attribute of the
Intel IGD in sysfs. The offset 0xfc is defined by PCI_INTEL_OPREGION
both here and in hvmloader (and also in Qemu). The data that is
read at this offset from the start of the config attribute of the Intel
IGD in sysfs is the 32-bit address of the start of the opregion.

Maybe the error check further down should be to see whether adding 2
to the value would overflow in 32 bits? (In that case a #define may
not be needed anymore, as there wouldn't be multiple instances of the
constant in the code.)


That would work also. Please not that I chose that value for an error
value consistent with the way the current function sysfs_dev_get_vendor
does it. While that function does not assign the variable 'error' to
its return value for an error, which in that case is 0x because
that function returns uint16_t instead of uint32_t,
I chose to explicitly assign the error variable to that value to help make
the code more readable. Your  comment that this could be a #define
instead is good. I also think we should use a #define for the error return
value of the sysfs_dev_get_vendor function Something like:

#define ERROR_16    0x
#define ERROR_32    0x

might be appropriate. But that would be touching code unrelated to
this bug fix. I think again the libxl maintainers should weigh in about
what to do here. They might let me take this opportunity to update
and improve the style of the patched file in other functions in the
file not related to this bug fix but I am not inclined to do that without
an explicit request from them to do so. So I am not sure yet what I will
do in the next version of the patch, but I will address your concerns here
and try to explain my reasoning for the changes in the changelog for
version 2 of the patch.



+
+FILE *f = fopen(pci_device_config_path, "r");
+if (!f) {

While libxl has some special style rules, I think it still wants a
blank line between declaration(s) and statement(s), just like we
expect elsewhere. Effectively you want to simply move the blank line
you have one line down.


I think I followed the same style here as the existing sysfs_dev_get_xxx
functions. I will double check that and use the same style the other
functions use unless they clearly violate the rules, and note that I
deviated from the style of the existing functions to conform to current
coding style and suggest a subsequent patch to update the style of
the other functions.



@@ -2531,6 +2572,37 @@ int libxl__grant_vga_iomem_permission(libxl__gc *gc, 
const uint32_t domid,
domid, vga_iomem_start, (vga_iomem_start + 0x20 - 1));
  return ret;
  }
+
+/* If this is an Intel IGD, allow access to the IGD opregion */
+if (!libxl__is_igd_vga_passthru(gc, d_config)) return 0;

Despite the provision for "return" or alike to go on the same line
as an error code check, I don't think this is okay here. It would be
if, as iirc generally expected in libxl, you latched the function
return value into a local variable named "rc" (I think).


I will double check how the function being patched handles the return
value. I don't 

Re: [PATCH v2 2/6] xen/sched: create public function for cpupools creation

[Luca Fancellu]

> On 15 Mar 2022, at 17:45, Andrew Cooper  wrote:
> 
> On 10/03/2022 17:10, Luca Fancellu wrote:
>> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
>> index 10ea969c7af9..47fc856e0fe0 100644
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -1145,6 +1145,22 @@ int cpupool_move_domain(struct domain *d, struct 
>> cpupool *c);
>> int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op);
>> unsigned int cpupool_get_id(const struct domain *d);
>> const cpumask_t *cpupool_valid_cpus(const struct cpupool *pool);
>> +
>> +/*
>> + * cpupool_create_pool - Creates a cpupool
>> + * @pool_id: id of the pool to be created
>> + * @sched_id: id of the scheduler to be used for the pool
>> + *
>> + * Creates a cpupool with pool_id id.
>> + * The sched_id parameter identifies the scheduler to be used, if it is
>> + * negative, the default scheduler of Xen will be used.
>> + *
>> + * returns:
>> + * pointer to the struct cpupool just created, on success
>> + * NULL, on cpupool creation error
> 
> What makes you say this?  Your new function will fall over a NULL
> pointer before it returns one...

You are right, it’s a leftover from the v1, I will change it and review the 
code that uses it.

Cheers,
Luca

> 
> ~Andrew

Re: [PATCH v2 2/6] xen/sched: create public function for cpupools creation

[Andrew Cooper]

On 10/03/2022 17:10, Luca Fancellu wrote:
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 10ea969c7af9..47fc856e0fe0 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -1145,6 +1145,22 @@ int cpupool_move_domain(struct domain *d, struct 
> cpupool *c);
>  int cpupool_do_sysctl(struct xen_sysctl_cpupool_op *op);
>  unsigned int cpupool_get_id(const struct domain *d);
>  const cpumask_t *cpupool_valid_cpus(const struct cpupool *pool);
> +
> +/*
> + * cpupool_create_pool - Creates a cpupool
> + * @pool_id: id of the pool to be created
> + * @sched_id: id of the scheduler to be used for the pool
> + *
> + * Creates a cpupool with pool_id id.
> + * The sched_id parameter identifies the scheduler to be used, if it is
> + * negative, the default scheduler of Xen will be used.
> + *
> + * returns:
> + * pointer to the struct cpupool just created, on success
> + * NULL, on cpupool creation error

What makes you say this?  Your new function will fall over a NULL
pointer before it returns one...

~Andrew

Re: [PATCH] xen/usb: harden xen_hcd against malicious backends

[Greg Kroah-Hartman]

On Fri, Mar 11, 2022 at 11:35:09AM +0100, Juergen Gross wrote:
> Make sure a malicious backend can't cause any harm other than wrong
> I/O data.
> 
> Missing are verification of the request id in a response, sanitizing
> the reported actual I/O length, and protection against interrupt storms
> from the backend.
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/usb/host/xen-hcd.c | 57 --
>  1 file changed, 43 insertions(+), 14 deletions(-)

Fails to apply to my tree:

checking file drivers/usb/host/xen-hcd.c
Hunk #2 succeeded at 720 (offset -1 lines).
Hunk #3 succeeded at 807 (offset -3 lines).
Hunk #4 succeeded at 934 (offset -5 lines).
Hunk #5 FAILED at 986.
Hunk #6 succeeded at 1003 with fuzz 1 (offset -10 lines).
Hunk #7 succeeded at 1048 (offset -10 lines).
Hunk #8 succeeded at 1072 (offset -10 lines).
Hunk #9 succeeded at 1161 (offset -10 lines).
Hunk #10 succeeded at 1516 (offset -10 lines).
1 out of 10 hunks FAILED

Any hints?

thanks,

greg k-h

[XEN PATCH] tools/libs/light/libxl_pci.c: explicitly grant access to Intel IGD opregion

[Chuck Zmudzinski]

On 3/15/2022 7:38 AM, Jan Beulich wrote:

On 14.03.2022 04:41, Chuck Zmudzinski wrote:
Fixes: abfb006f1ff4 (tools/libxl: explicitly grant access to needed 
I/O-memory ranges)
Fixes: 0561e1f01e87 (xen/common: do not implicitly permit access to 
mapped I/O memory)

Backport: 4.12+

Just fyi: This is fine to have as a tag, but it wouldn't be backported
farther than to 4.15.


That's entirely reasonable. I understand this is a bug fix, not a
security issue.


Apart from this largely some style issues (see below), but please
realize that I'm not a libxl maintainer and hence I may not have good
enough knowledge of, in particular, potential unwritten conventions.


I will take your comments into consideration regarding style before
writing the next version of the patch, and carefully check libxl's
coding style file.



@@ -610,6 +612,45 @@ out: return ret; } +static uint32_t 
sysfs_dev_get_igd_opregion(libxl__gc *gc, + libxl_device_pci *pci) +{ 
+ char *pci_device_config_path = + 
GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/config", + pci->domain, pci->bus, 
pci->dev, pci->func); + size_t read_items; + uint32_t igd_opregion; + 
uint32_t error = 0x;

I think this constant wants to gain a #define, to be able to correlate
the use sites. I'm also not sure of the value - in principle the
register can hold this value, but of course then it won't be 3 pages.


What we are storing as the return value is the starting address,
not the size, of the opregion, and that is a 32-bit value. If we
cannot read it, we return 0x instead to indicate the error
that we were unable to read the starting address of the opregion
from the config attribute in sysfs. The 32-bit value we are looking for
is read at offset 0xfc from the start of the config attribute of the
Intel IGD in sysfs. The offset 0xfc is defined by PCI_INTEL_OPREGION
both here and in hvmloader (and also in Qemu). The data that is
read at this offset from the start of the config attribute of the Intel
IGD in sysfs is the 32-bit address of the start of the opregion.


Maybe the error check further down should be to see whether adding 2
to the value would overflow in 32 bits? (In that case a #define may
not be needed anymore, as there wouldn't be multiple instances of the
constant in the code.)


That would work also. Please not that I chose that value for an error
value consistent with the way the current function sysfs_dev_get_vendor
does it. While that function does not assign the variable 'error' to
its return value for an error, which in that case is 0x because
that function returns uint16_t instead of uint32_t,
I chose to explicitly assign the error variable to that value to help make
the code more readable. Your  comment that this could be a #define
instead is good. I also think we should use a #define for the error return
value of the sysfs_dev_get_vendor function Something like:

#define ERROR_16    0x
#define ERROR_32    0x

might be appropriate. But that would be touching code unrelated to
this bug fix. I think again the libxl maintainers should weigh in about
what to do here. They might let me take this opportunity to update
and improve the style of the patched file in other functions in the
file not related to this bug fix but I am not inclined to do that without
an explicit request from them to do so. So I am not sure yet what I will
do in the next version of the patch, but I will address your concerns here
and try to explain my reasoning for the changes in the changelog for
version 2 of the patch.


+
+ FILE *f = fopen(pci_device_config_path, "r");
+ if (!f) {

While libxl has some special style rules, I think it still wants a
blank line between declaration(s) and statement(s), just like we
expect elsewhere. Effectively you want to simply move the blank line
you have one line down. I can double check in libxlt's coding style file.


I think I followed the same style here as the existing sysfs_dev_get_xxx
functions. I will double check that and use the same style the other
functions use unless they clearly violate the rules, and note that I
deviated from the style of the existing functions to conform to current
coding style and suggest a subsequent patch to update the style of
the other functions.

@@ -2531,6 +2572,37 @@ int 
libxl__grant_vga_iomem_permission(libxl__gc *gc, const uint32_t domid,

domid, vga_iomem_start, (vga_iomem_start + 0x20 - 1));
return ret;
}
+
+ /* If this is an Intel IGD, allow access to the IGD opregion */
+ if (!libxl__is_igd_vga_passthru(gc, d_config)) return 0;

Despite the provision for "return" or alike to go on the same line
as an error code check, I don't think this is okay here. It would be
if, as iirc generally expected in libxl, you latched the function
return value into a local variable named "rc" (I think).


I will double check how the function being patched handles the return
value. I don't even remember if it has a local variable named rc for a 
return

value. IIRC it was either ret or 0. I 

[PATCH] x86/cet: Remove writeable mapping of the BSPs shadow stack

[Andrew Cooper]

An unintended consequence of the BSP using cpu0_stack[] is that writeable
mappings to the BSPs shadow stacks are retained in the bss.  This renders
CET-SS almost useless, as an attacker can update both return addresses and the
ret will not fault.

We specifically don't want the shatter the superpage mapping .data/.bss, so
the only way to fix this is to not have the BSP stack in the main Xen image.

Break cpu_alloc_stack() out of cpu_smpboot_alloc(), and dynamically allocate
the BSP stack as early as reasonable in __start_xen().  As a consequence,
there is no need to delay the BSP's memguard_guard_stack() call.

Copy the top of cpu info block just before switching to use the new stack.
Fix a latent bug by setting %rsp to info->guest_cpu_user_regs rather than
->es; this would be buggy if reinit_bsp_stack() called schedule() (which
rewrites the GPR block) directly, but luckily it doesn't.

Finally, move cpu0_stack[] into .init, so it can be reclaimed after boot.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Marek Marczykowski-Górecki 
---
 xen/arch/x86/include/asm/smp.h |  2 ++
 xen/arch/x86/setup.c   | 20 +---
 xen/arch/x86/smpboot.c | 26 +++---
 xen/arch/x86/xen.lds.S |  4 ++--
 4 files changed, 36 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h
index 1747772d232e..41a3b6a0dadf 100644
--- a/xen/arch/x86/include/asm/smp.h
+++ b/xen/arch/x86/include/asm/smp.h
@@ -85,6 +85,8 @@ extern cpumask_t **socket_cpumask;
 extern unsigned int disabled_cpus;
 extern bool unaccounted_cpus;
 
+void *cpu_alloc_stack(unsigned int cpu);
+
 #endif /* !__ASSEMBLY__ */
 
 #endif
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 22a9885dee5c..1f816ce05a07 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -148,7 +148,7 @@ cpumask_t __read_mostly cpu_present_map;
 
 unsigned long __read_mostly xen_phys_start;
 
-char __section(".bss.stack_aligned") __aligned(STACK_SIZE)
+char __section("init.bss.stack_aligned") __aligned(STACK_SIZE)
 cpu0_stack[STACK_SIZE];
 
 /* Used by the BSP/AP paths to find the higher half stack mapping to use. */
@@ -712,7 +712,6 @@ static void __init noreturn reinit_bsp_stack(void)
 percpu_traps_init();
 
 stack_base[0] = stack;
-memguard_guard_stack(stack);
 
 rc = setup_cpu_root_pgt(0);
 if ( rc )
@@ -886,6 +885,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 {
 char *memmap_type = NULL;
 char *cmdline, *kextra, *loader;
+void *bsp_stack;
+struct cpu_info *info = get_cpu_info(), *bsp_info;
 unsigned int initrdidx, num_parked = 0;
 multiboot_info_t *mbi;
 module_t *mod;
@@ -918,7 +919,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 /* Full exception support from here on in. */
 
 rdmsrl(MSR_EFER, this_cpu(efer));
-asm volatile ( "mov %%cr4,%0" : "=r" (get_cpu_info()->cr4) );
+asm volatile ( "mov %%cr4,%0" : "=r" (info->cr4) );
 
 /* Enable NMIs.  Our loader (e.g. Tboot) may have left them disabled. */
 enable_nmis();
@@ -1703,6 +1704,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  */
 vm_init();
 
+bsp_stack = cpu_alloc_stack(0);
+if ( !bsp_stack )
+panic("No memory for BSP stack\n");
+
 console_init_ring();
 vesa_init();
 
@@ -1974,17 +1979,18 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
 if ( bsp_delay_spec_ctrl )
 {
-struct cpu_info *info = get_cpu_info();
-
 info->spec_ctrl_flags &= ~SCF_use_shadow;
 barrier();
 wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
 info->last_spec_ctrl = default_xen_spec_ctrl;
 }
 
-/* Jump to the 1:1 virtual mappings of cpu0_stack. */
+/* Copy the cpu info block, and move onto the BSP stack. */
+bsp_info = get_cpu_info_from_stack((unsigned long)bsp_stack);
+*bsp_info = *info;
+
 asm volatile ("mov %[stk], %%rsp; jmp %c[fn]" ::
-  [stk] "g" (__va(__pa(get_stack_bottom(,
+  [stk] "g" (_info->guest_cpu_user_regs),
   [fn] "i" (reinit_bsp_stack) : "memory");
 unreachable();
 }
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 709704d71ada..b46fd9ab18e4 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1023,6 +1023,23 @@ static void cpu_smpboot_free(unsigned int cpu, bool 
remove)
 }
 }
 
+void *cpu_alloc_stack(unsigned int cpu)
+{
+nodeid_t node = cpu_to_node(cpu);
+unsigned int memflags = 0;
+void *stack;
+
+if ( node != NUMA_NO_NODE )
+memflags = MEMF_node(node);
+
+stack = alloc_xenheap_pages(STACK_ORDER, memflags);
+
+if ( stack )
+memguard_guard_stack(stack);
+
+return stack;
+}
+
 static int cpu_smpboot_alloc(unsigned int cpu)
 {
 struct cpu_info *info;
@@ -1035,15 +1052,10 @@ static int cpu_smpboot_alloc(unsigned 

[ovmf test] 168617: regressions - FAIL

[osstest service owner]

flight 168617 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168617/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf c8ea48bdf95532f9a3a4c39a154c09988566901f
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  142 attempts
Testing same since   168617  2022-03-15 15:41:41 Z0 days1 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jagadeesh Ujja 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sami Mujawar 
  Sean Rhodes 
  Sebastien Boeuf 
  Sunny Wang 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 653 lines long.)

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Alex Bennée]

Markus Armbruster  writes:

> Philippe Mathieu-Daudé  writes:
>
>> On 15/3/22 14:59, Markus Armbruster wrote:
>>> Alex Bennée  writes:
>>> 
 Markus Armbruster  writes:

> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
> for two reasons.  One, it catches multiplication overflowing size_t.
> Two, it returns T * rather than void *, which lets the compiler catch
> more type errors.
>
 
> diff --git a/semihosting/config.c b/semihosting/config.c
> index 137171b717..6d48ec9566 100644
> --- a/semihosting/config.c
> +++ b/semihosting/config.c
> @@ -98,7 +98,7 @@ static int add_semihosting_arg(void *opaque,
>   if (strcmp(name, "arg") == 0) {
>   s->argc++;
>   /* one extra element as g_strjoinv() expects NULL-terminated 
> array */
> -s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
> +s->argv = g_renew(void *, s->argv, s->argc + 1);

 This did indeed break CI because s->argv is an array of *char:

 ../semihosting/config.c:101:17: error: assignment to ‘const char **’ from 
 incompatible pointer type ‘void **’ [-Werror=incompatible-pointer-types]
101 | s->argv = g_renew(void *, s->argv, s->argc + 1);
| ^
 cc1: all warnings being treated as errors

 So it did the job of type checking but failed to build ;-)
>>>
>>> You found a hole in my compile testing, thanks!
>>>
>>> I got confused about the configuration of my build trees.  Catching such
>>> mistakes is what CI is for :)
>>
>> FYI Alex fixed this here:
>> https://lore.kernel.org/qemu-devel/20220315121251.2280317-8-alex.ben...@linaro.org/
>>
>> So your series could go on top (modulo the Coverity change).
>
> I dropped this hunk in v2.
>
> Whether my v2 or Alex's series goes in first doesn't matter.

That's great. Thanks for finding the ugliness in the first place ;-)

-- 
Alex Bennée

Re: [PATCH 3/3] livepatch: correctly handle altinstruction sections

[Roger Pau Monné]

On Thu, Mar 10, 2022 at 04:08:34PM +0100, Roger Pau Monne wrote:
> The current handling of altinstructions sections by the livepatch
> tools is incorrect, as on Xen those sections are part of .init and
> thus discarded after load. Correctly handle them by just ignoring, as
> it's done with other .init related sections.

I think my logic here is wrong.

While looking at something else I realized that livepatch does support
'.altinstructions', as it needs to be able to patch the contents of
the payload that is being loaded - hence ignoring any '.altintr*'
section at patch generation is not OK.

I have to withdraw this patch, will likely repost with the other
sections that do need to be ignored.

Thanks, Roger.

[ovmf test] 168616: regressions - FAIL

[osstest service owner]

flight 168616 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168616/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf a13dfc769bd7097d8d9ffe3e029a2c1d062d712b
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  141 attempts
Testing same since   168579  2022-03-14 08:41:36 Z1 days   12 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sean Rhodes 
  Sebastien Boeuf 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 629 lines long.)

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

[osstest service owner]

flight 168613 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168613/

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  c7a80bc50ac768b4eecaad85b77ae45790c93c73
baseline version:
 xen  07aebcd55fd2f7997e9fe50a6c849c8a12ec2e68

Last test of basis   168607  2022-03-15 03:02:22 Z0 days
Testing same since   168613  2022-03-15 12:01:42 Z0 days1 attempts


People who touched revisions under test:
  Jan Beulich 
  Julien Grall 
  Marek Marczykowski-Górecki 

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
   07aebcd55f..c7a80bc50a  c7a80bc50ac768b4eecaad85b77ae45790c93c73 -> smoke

Re: [PATCH v3 2/4] x86/APIC: calibrate against platform timer when possible

[Roger Pau Monné]

On Tue, Mar 15, 2022 at 11:39:29AM +0100, Jan Beulich wrote:
> On 15.03.2022 10:12, Roger Pau Monné wrote:
> > On Mon, Mar 14, 2022 at 05:19:37PM +0100, Jan Beulich wrote:
> >> One thing seems quite clear though: Doing any of this with interrupts
> >> enabled increases the chances for the read pairs to not properly
> >> correlate, due to an interrupt happening in the middle. This alone is
> >> a reason for me to want to keep IRQs off here.
> > 
> > Right, TSC calibration is also done with interrupts disabled, so it
> > does seem correct to do the same here for APIC.
> > 
> > Maybe it would be cleaner to hide the specific PIT logic in
> > calibrate_apic_timer() so that we could remove get_8254_timer_count()
> > and wait_8254_wraparound() from apic.c and apic.c doesn't have any PIT
> > specific code anymore?
> 
> Yes, that's certainly a further cleanup step to take (saying this
> without actually having tried, so there may be obstacles).

OK, I think you are planning to post a new version of this to avoid
open-coding apic_read() in read_tmcct()?

TBH the PIT calibration done in calibrate_APIC_clock seems fairly
bogus, as it's possible the counter wraps around more than once
between calls when running virtualized. Maybe reprogramming channel 2
would be better, as then at least wrap around would be detected
(albeit it's unclear how much delta we would have between the counter
reaching 0 and Xen realizing).

Thanks, Roger.

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Philippe Mathieu-Daudé  writes:

> On 15/3/22 14:59, Markus Armbruster wrote:
>> Alex Bennée  writes:
>> 
>>> Markus Armbruster  writes:
>>>
 g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
 for two reasons.  One, it catches multiplication overflowing size_t.
 Two, it returns T * rather than void *, which lets the compiler catch
 more type errors.

>>> 
 diff --git a/semihosting/config.c b/semihosting/config.c
 index 137171b717..6d48ec9566 100644
 --- a/semihosting/config.c
 +++ b/semihosting/config.c
 @@ -98,7 +98,7 @@ static int add_semihosting_arg(void *opaque,
   if (strcmp(name, "arg") == 0) {
   s->argc++;
   /* one extra element as g_strjoinv() expects NULL-terminated 
 array */
 -s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
 +s->argv = g_renew(void *, s->argv, s->argc + 1);
>>>
>>> This did indeed break CI because s->argv is an array of *char:
>>>
>>> ../semihosting/config.c:101:17: error: assignment to ‘const char **’ from 
>>> incompatible pointer type ‘void **’ [-Werror=incompatible-pointer-types]
>>>101 | s->argv = g_renew(void *, s->argv, s->argc + 1);
>>>| ^
>>> cc1: all warnings being treated as errors
>>>
>>> So it did the job of type checking but failed to build ;-)
>>
>> You found a hole in my compile testing, thanks!
>>
>> I got confused about the configuration of my build trees.  Catching such
>> mistakes is what CI is for :)
>
> FYI Alex fixed this here:
> https://lore.kernel.org/qemu-devel/20220315121251.2280317-8-alex.ben...@linaro.org/
>
> So your series could go on top (modulo the Coverity change).

I dropped this hunk in v2.

Whether my v2 or Alex's series goes in first doesn't matter.

[PATCH v2 3/3] Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
for two reasons.  One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.

This commit only touches allocations with size arguments of the form
sizeof(T).

Patch created mechanically with:

$ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
 --macro-file scripts/cocci-macro-file.h FILES...

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Alex Bennée 
Acked-by: Dr. David Alan Gilbert 
---
 include/qemu/timer.h |  2 +-
 accel/kvm/kvm-all.c  |  6 ++--
 accel/tcg/tcg-accel-ops-mttcg.c  |  2 +-
 accel/tcg/tcg-accel-ops-rr.c |  4 +--
 audio/audio.c|  4 +--
 audio/audio_legacy.c |  6 ++--
 audio/dsoundaudio.c  |  2 +-
 audio/jackaudio.c|  6 ++--
 audio/paaudio.c  |  4 +--
 backends/cryptodev.c |  2 +-
 contrib/vhost-user-gpu/vhost-user-gpu.c  |  2 +-
 cpus-common.c|  4 +--
 dump/dump.c  |  2 +-
 hw/acpi/hmat.c   |  2 +-
 hw/audio/intel-hda.c |  2 +-
 hw/char/parallel.c   |  2 +-
 hw/char/riscv_htif.c |  2 +-
 hw/char/virtio-serial-bus.c  |  6 ++--
 hw/core/irq.c|  2 +-
 hw/core/reset.c  |  2 +-
 hw/display/pxa2xx_lcd.c  |  2 +-
 hw/display/tc6393xb.c|  2 +-
 hw/display/virtio-gpu.c  |  4 +--
 hw/display/xenfb.c   |  4 +--
 hw/dma/rc4030.c  |  4 +--
 hw/i2c/core.c|  4 +--
 hw/i2c/i2c_mux_pca954x.c |  2 +-
 hw/i386/amd_iommu.c  |  4 +--
 hw/i386/intel_iommu.c|  2 +-
 hw/i386/xen/xen-hvm.c| 10 +++---
 hw/i386/xen/xen-mapcache.c   | 14 
 hw/input/lasips2.c   |  2 +-
 hw/input/pckbd.c |  2 +-
 hw/input/ps2.c   |  4 +--
 hw/input/pxa2xx_keypad.c |  2 +-
 hw/input/tsc2005.c   |  3 +-
 hw/intc/riscv_aclint.c   |  6 ++--
 hw/intc/xics.c   |  2 +-
 hw/m68k/virt.c   |  2 +-
 hw/mips/mipssim.c|  2 +-
 hw/misc/applesmc.c   |  2 +-
 hw/misc/imx6_src.c   |  2 +-
 hw/misc/ivshmem.c|  4 +--
 hw/net/virtio-net.c  |  4 +--
 hw/nvme/ns.c |  2 +-
 hw/pci-host/pnv_phb3.c   |  2 +-
 hw/pci-host/pnv_phb4.c   |  2 +-
 hw/pci/pcie_sriov.c  |  2 +-
 hw/ppc/e500.c|  2 +-
 hw/ppc/ppc.c |  8 ++---
 hw/ppc/ppc405_boards.c   |  4 +--
 hw/ppc/ppc405_uc.c   | 18 +-
 hw/ppc/ppc4xx_devs.c |  2 +-
 hw/ppc/ppc_booke.c   |  4 +--
 hw/ppc/spapr.c   |  2 +-
 hw/ppc/spapr_events.c|  2 +-
 hw/ppc/spapr_hcall.c |  2 +-
 hw/ppc/spapr_numa.c  |  3 +-
 hw/rdma/vmw/pvrdma_dev_ring.c|  2 +-
 hw/rdma/vmw/pvrdma_qp_ops.c  |  6 ++--
 hw/sh4/r2d.c |  4 +--
 hw/sh4/sh7750.c  |  2 +-
 hw/sparc/leon3.c |  2 +-
 hw/sparc64/sparc64.c |  4 +--
 hw/timer/arm_timer.c |  2 +-
 hw/timer/slavio_timer.c  |  2 +-
 hw/vfio/pci.c|  4 +--
 hw/vfio/platform.c   |  4 +--
 hw/virtio/virtio-crypto.c|  2 +-
 hw/virtio/virtio-iommu.c |  2 +-
 hw/virtio/virtio.c   |  5 ++-
 hw/xtensa/xtfpga.c   |  2 +-
 linux-user/syscall.c |  2 +-
 migration/dirtyrate.c|  4 +--
 migration/multifd-zlib.c |  4 +--
 migration/ram.c  |  2 +-
 monitor/misc.c   |  2 +-
 monitor/qmp-cmds.c   |  2 +-
 qga/commands-win32.c |  8 ++---
 qga/commands.c   |  2 +-
 qom/qom-qmp-cmds.c   |  2 +-
 replay/replay-char.c |  4 +--
 replay/replay-events.c   | 10 +++---
 softmmu/bootdevice.c |  4 +--
 softmmu/dma-helpers.c|  4 +--
 softmmu/memory_mapping.c 

[PATCH v2 0/3] Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
for two reasons.  One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.

This series only touches allocations with size arguments of the form
sizeof(T).  It's mechanical, except for a tiny fix in PATCH 2.

PATCH 1 adds the Coccinelle script.

PATCH 2 cleans up the virtio-9p subsystem, and fixes a harmless typing
error uncovered by the cleanup.

PATCH 3 cleans up everything else.  I started to split it up, but
splitting is a lot of decisions, and I just can't see the value.

For instance, MAINTAINERS tells me to split for subsystem "virtio",
patching

hw/char/virtio-serial-bus.c
hw/display/virtio-gpu.c
hw/net/virtio-net.c
hw/virtio/virtio-crypto.c
hw/virtio/virtio-iommu.c
hw/virtio/virtio.c

But it also tells me to split for subsystem "Character devices",
patching

hw/char/parallel.c   |  2 +-
hw/char/riscv_htif.c |  2 +-
hw/char/virtio-serial-bus.c  |  6 +-

and for subsystem "Network devices", patching

hw/net/virtio-net.c

and for subsystem "virtio-gpu", patching

hw/display/virtio-gpu.c

I guess I'd go with "virtio".  Six files down, 103 to go.  Thanks, but
no thanks.

Since the transformation is local to a function call, dropping is
completely safe.  We can deal with conflicts by dropping conflicting
hunks, with "git-pull -s recursive -X ours".  Or drop entire files
with conflicts.

If you want me to split off certain parts, please tell me exactly what
you want split off, and I'll gladly do the splitting.  I don't mind
the splitting part, I do mind the *thinking* part.

I backed out two changes made by the Coccinelle script:
scripts/coverity-scan/model.c, because that's special, and
semihosting/config.c, because it has a typing error similar to the one
fixed in PATCH 2, and Alex already posted a patch for it.

v2:
* PATCH 3: Change to scripts/coverity-scan/model.c dropped [Eric]
* PATCH 3: Change to semihosting/config.c dropped [Alex]
* Commit messages tweaked

Markus Armbruster (3):
  scripts/coccinelle: New use-g_new-etc.cocci
  9pfs: Use g_new() & friends where that makes obvious sense
  Use g_new() & friends where that makes obvious sense

 scripts/coccinelle/use-g_new-etc.cocci   | 75 
 include/qemu/timer.h |  2 +-
 accel/kvm/kvm-all.c  |  6 +-
 accel/tcg/tcg-accel-ops-mttcg.c  |  2 +-
 accel/tcg/tcg-accel-ops-rr.c |  4 +-
 audio/audio.c|  4 +-
 audio/audio_legacy.c |  6 +-
 audio/dsoundaudio.c  |  2 +-
 audio/jackaudio.c|  6 +-
 audio/paaudio.c  |  4 +-
 backends/cryptodev.c |  2 +-
 contrib/vhost-user-gpu/vhost-user-gpu.c  |  2 +-
 cpus-common.c|  4 +-
 dump/dump.c  |  2 +-
 hw/9pfs/9p-proxy.c   |  2 +-
 hw/9pfs/9p-synth.c   |  4 +-
 hw/9pfs/9p.c |  8 +--
 hw/9pfs/codir.c  |  6 +-
 hw/acpi/hmat.c   |  2 +-
 hw/audio/intel-hda.c |  2 +-
 hw/char/parallel.c   |  2 +-
 hw/char/riscv_htif.c |  2 +-
 hw/char/virtio-serial-bus.c  |  6 +-
 hw/core/irq.c|  2 +-
 hw/core/reset.c  |  2 +-
 hw/display/pxa2xx_lcd.c  |  2 +-
 hw/display/tc6393xb.c|  2 +-
 hw/display/virtio-gpu.c  |  4 +-
 hw/display/xenfb.c   |  4 +-
 hw/dma/rc4030.c  |  4 +-
 hw/i2c/core.c|  4 +-
 hw/i2c/i2c_mux_pca954x.c |  2 +-
 hw/i386/amd_iommu.c  |  4 +-
 hw/i386/intel_iommu.c|  2 +-
 hw/i386/xen/xen-hvm.c| 10 ++--
 hw/i386/xen/xen-mapcache.c   | 14 ++---
 hw/input/lasips2.c   |  2 +-
 hw/input/pckbd.c |  2 +-
 hw/input/ps2.c   |  4 +-
 hw/input/pxa2xx_keypad.c |  2 +-
 hw/input/tsc2005.c   |  3 +-
 hw/intc/riscv_aclint.c   |  6 +-
 hw/intc/xics.c   |  2 +-
 hw/m68k/virt.c   |  2 +-
 hw/mips/mipssim.c|  2 +-
 hw/misc/applesmc.c   |  2 +-
 hw/misc/imx6_src.c   |  2 +-
 hw/misc/ivshmem.c|  4 +-
 hw/net/virtio-net.c  |  4 +-
 hw/nvme/ns.c |  2 +-
 hw/pci-host/pnv_phb3.c   |  2 +-
 hw/pci-host/pnv_phb4.c   |  2 +-
 hw/pci/pcie_sriov.c

[PATCH v2 2/3] 9pfs: Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
for two reasons.  One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.

This commit only touches allocations with size arguments of the form
sizeof(T).

Initial patch created mechanically with:

$ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
 --macro-file scripts/cocci-macro-file.h FILES...

This uncovers a typing error:

../hw/9pfs/9p.c: In function ‘qid_path_fullmap’:
../hw/9pfs/9p.c:855:13: error: assignment to ‘QpfEntry *’ from incompatible 
pointer type ‘QppEntry *’ [-Werror=incompatible-pointer-types]
  855 | val = g_new0(QppEntry, 1);
  | ^

Harmless, because QppEntry is larger than QpfEntry.  Manually fixed to
allocate a QpfEntry instead.

Cc: Greg Kurz 
Cc: Christian Schoenebeck 
Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Christian Schoenebeck 
Reviewed-by: Alex Bennée 
Reviewed-by: Greg Kurz 
---
 hw/9pfs/9p-proxy.c   | 2 +-
 hw/9pfs/9p-synth.c   | 4 ++--
 hw/9pfs/9p.c | 8 
 hw/9pfs/codir.c  | 6 +++---
 tests/qtest/virtio-9p-test.c | 4 ++--
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 8b4b5cf7dc..4c5e0fc217 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -1187,7 +1187,7 @@ static int proxy_parse_opts(QemuOpts *opts, FsDriverEntry 
*fs, Error **errp)
 
 static int proxy_init(FsContext *ctx, Error **errp)
 {
-V9fsProxy *proxy = g_malloc(sizeof(V9fsProxy));
+V9fsProxy *proxy = g_new(V9fsProxy, 1);
 int sock_id;
 
 if (ctx->export_flags & V9FS_PROXY_SOCK_NAME) {
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index b3080e415b..d99d263985 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -49,7 +49,7 @@ static V9fsSynthNode *v9fs_add_dir_node(V9fsSynthNode 
*parent, int mode,
 
 /* Add directory type and remove write bits */
 mode = ((mode & 0777) | S_IFDIR) & ~(S_IWUSR | S_IWGRP | S_IWOTH);
-node = g_malloc0(sizeof(V9fsSynthNode));
+node = g_new0(V9fsSynthNode, 1);
 if (attr) {
 /* We are adding .. or . entries */
 node->attr = attr;
@@ -128,7 +128,7 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int 
mode,
 }
 /* Add file type and remove write bits */
 mode = ((mode & 0777) | S_IFREG);
-node = g_malloc0(sizeof(V9fsSynthNode));
+node = g_new0(V9fsSynthNode, 1);
 node->attr = >actual_attr;
 node->attr->inode  = synth_node_count++;
 node->attr->nlink  = 1;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index a6d6b3f835..8e9d4aea73 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -324,7 +324,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
 return NULL;
 }
 }
-f = g_malloc0(sizeof(V9fsFidState));
+f = g_new0(V9fsFidState, 1);
 f->fid = fid;
 f->fid_type = P9_FID_NONE;
 f->ref = 1;
@@ -804,7 +804,7 @@ static int qid_inode_prefix_hash_bits(V9fsPDU *pdu, dev_t 
dev)
 
 val = qht_lookup(>s->qpd_table, , hash);
 if (!val) {
-val = g_malloc0(sizeof(QpdEntry));
+val = g_new0(QpdEntry, 1);
 *val = lookup;
 affix = affixForIndex(pdu->s->qp_affix_next);
 val->prefix_bits = affix.bits;
@@ -852,7 +852,7 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct stat 
*stbuf,
 return -ENFILE;
 }
 
-val = g_malloc0(sizeof(QppEntry));
+val = g_new0(QpfEntry, 1);
 *val = lookup;
 
 /* new unique inode and device combo */
@@ -928,7 +928,7 @@ static int qid_path_suffixmap(V9fsPDU *pdu, const struct 
stat *stbuf,
 return -ENFILE;
 }
 
-val = g_malloc0(sizeof(QppEntry));
+val = g_new0(QppEntry, 1);
 *val = lookup;
 
 /* new unique inode affix and device combo */
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index 75148bc985..93ba44fb75 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -141,9 +141,9 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp,
 
 /* append next node to result chain */
 if (!e) {
-*entries = e = g_malloc0(sizeof(V9fsDirEnt));
+*entries = e = g_new0(V9fsDirEnt, 1);
 } else {
-e = e->next = g_malloc0(sizeof(V9fsDirEnt));
+e = e->next = g_new0(V9fsDirEnt, 1);
 }
 e->dent = qemu_dirent_dup(dent);
 
@@ -163,7 +163,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp,
 break;
 }
 
-e->st = g_malloc0(sizeof(struct stat));
+e->st = g_new0(struct stat, 1);
 memcpy(e->st, , sizeof(struct stat));
 }
 
diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
index 01ca076afe..e28c71bd8f 100644
--- 

[PATCH v2 1/3] scripts/coccinelle: New use-g_new-etc.cocci

[Markus Armbruster]

This is the semantic patch from commit b45c03f585 "arm: Use g_new() &
friends where that makes obvious sense".

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Reviewed-by: Alex Bennée 
---
 scripts/coccinelle/use-g_new-etc.cocci | 75 ++
 1 file changed, 75 insertions(+)
 create mode 100644 scripts/coccinelle/use-g_new-etc.cocci

diff --git a/scripts/coccinelle/use-g_new-etc.cocci 
b/scripts/coccinelle/use-g_new-etc.cocci
new file mode 100644
index 00..e2280e93b3
--- /dev/null
+++ b/scripts/coccinelle/use-g_new-etc.cocci
@@ -0,0 +1,75 @@
+// Use g_new() & friends where that makes obvious sense
+@@
+type T;
+@@
+-g_malloc(sizeof(T))
++g_new(T, 1)
+@@
+type T;
+@@
+-g_try_malloc(sizeof(T))
++g_try_new(T, 1)
+@@
+type T;
+@@
+-g_malloc0(sizeof(T))
++g_new0(T, 1)
+@@
+type T;
+@@
+-g_try_malloc0(sizeof(T))
++g_try_new0(T, 1)
+@@
+type T;
+expression n;
+@@
+-g_malloc(sizeof(T) * (n))
++g_new(T, n)
+@@
+type T;
+expression n;
+@@
+-g_try_malloc(sizeof(T) * (n))
++g_try_new(T, n)
+@@
+type T;
+expression n;
+@@
+-g_malloc0(sizeof(T) * (n))
++g_new0(T, n)
+@@
+type T;
+expression n;
+@@
+-g_try_malloc0(sizeof(T) * (n))
++g_try_new0(T, n)
+@@
+type T;
+expression p, n;
+@@
+-g_realloc(p, sizeof(T) * (n))
++g_renew(T, p, n)
+@@
+type T;
+expression p, n;
+@@
+-g_try_realloc(p, sizeof(T) * (n))
++g_try_renew(T, p, n)
+@@
+type T;
+expression n;
+@@
+-(T *)g_new(T, n)
++g_new(T, n)
+@@
+type T;
+expression n;
+@@
+-(T *)g_new0(T, n)
++g_new0(T, n)
+@@
+type T;
+expression p, n;
+@@
+-(T *)g_renew(T, p, n)
++g_renew(T, p, n)
-- 
2.35.1

[libvirt test] 168608: regressions - FAIL

[osstest service owner]

flight 168608 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168608/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 151777
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 151777

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a

version targeted for testing:
 libvirt  2804fa912fa160e38e4ce7f537b6f6e1dcd5ee9d
baseline version:
 libvirt  2c846fa6bcc11929c9fb857a22430fb9945654ad

Last test of basis   151777  2020-07-10 04:19:19 Z  613 days
Failing since151818  2020-07-11 04:18:52 Z  612 days  594 attempts
Testing same since   168522  2022-03-12 04:18:54 Z3 days4 attempts


People who touched revisions under test:
Adolfo Jayme Barrientos 
  Aleksandr Alekseev 
  Aleksei Zakharov 
  Andika Triwidada 
  Andrea Bolognani 
  Ani Sinha 
  Balázs Meskó 
  Barrett Schonefeld 
  Bastian Germann 
  Bastien Orivel 
  BiaoXiang Ye 
  Bihong Yu 
  Binfeng Wu 
  Bjoern Walk 
  Boris Fiuczynski 
  Brad Laue 
  Brian Turek 
  Bruno Haible 
  Chris Mayo 
  Christian Borntraeger 
  Christian Ehrhardt 
  Christian Kirbach 
  Christian Schoenebeck 
  Christophe Fergeau 
  Cole Robinson 
  Collin Walling 
  Cornelia Huck 
  Cédric Bosdonnat 
  Côme Borsoi 
  Daniel Henrique Barboza 
  Daniel Letai 
  Daniel P. Berrange 
  Daniel P. Berrangé 
  Didik Supriadi 
  dinglimin 
  Divya Garg 
  Dmitrii Shcherbakov 
  Dmytro Linkin 
  Eiichi Tsukata 
  Emilio Herrera 
  Eric Farman 
  Erik Skultety 
  Fabian Affolter 
  Fabian Freyer 
  Fabiano Fidêncio 
  Fangge Jin 
  Farhan Ali 
  Fedora Weblate Translation 
  Franck Ridel 
  Gavi Teitz 
  gongwei 
  Guoyi Tu
  Göran Uddeborg 
  Halil Pasic 
  Han Han 
  Hao Wang 
  Haonan Wang 
  Hela Basa 
  Helmut Grohne 
  Hiroki Narukawa 
  Hyman Huang(黄勇) 
  Ian Wienand 
  Ioanna Alifieraki 
  Ivan Teterevkov 
  Jakob Meng 
  Jamie Strandboge 
  Jamie Strandboge 
  Jan Kuparinen 
  jason lee 
  Jean-Baptiste Holcroft 
  Jia Zhou 
  Jianan Gao 
  Jim Fehlig 
  Jin Yan 
  Jing Qi 
  Jinsheng Zhang 
  Jiri Denemark 
  Joachim Falk 
  John Ferlan 
  Jonathan Watt 
  Jonathon Jongsma 
  Julio Faracco 
  Justin Gatzen 
  Ján Tomko 
  Kashyap Chamarthy 
  Kevin Locke 
  Kim InSoo 
  Koichi Murase 
  Kristina Hanicova 
  Laine Stump 
  Laszlo Ersek 
  Lee Yarwood 
  Lei Yang 
  Liao Pingfang 
  Lin Ma 
  Lin Ma 
  Lin Ma 
  Liu Yiding 
  Lubomir Rintel 
  Luke Yue 
  Luyao Zhong 
  Marc Hartmayer 
  Marc-André Lureau 
  Marek Marczykowski-Górecki 
  Markus Schade 
  Martin Kletzander 
  Martin Pitt 
  Masayoshi Mizuma 
  Matej Cepl 
  Matt Coleman 
  Matt Coleman 
  Mauro Matteo Cascella 
  Meina Li 
  Michal Privoznik 
  Michał Smyk 
  Milo Casagrande 
  Moshe Levi 
  Muha Aliss 
  Nathan 
  Neal Gompa 
  Nick Chevsky 
  Nick Shyrokovskiy 
  Nickys Music Group 
  Nico Pache 
  Nicolas Lécureuil 
  Nicolas Lécureuil 
  Nikolay Shirokovskiy 
  Olaf Hering 
  Olesya Gerasimenko 
  Or Ozeri 
  Orion Poplawski 
  Pany 
  Patrick Magauran 
  Paulo de Rezende Pinatti 
  Pavel Hrdina 
  Peng Liang 
  Peter Krempa 
  Pino Toscano 
  Pino Toscano 
  Piotr Drąg 
  Prathamesh Chavan 
  Praveen K Paladugu 
  Richard W.M. Jones 
  Ricky Tigg 
  Robin Lee 
  Rohit Kumar 
  Roman Bogorodskiy 
  Roman Bolshakov 
  Ryan Gahagan 
  Ryan Schmidt 
  Sam Hartman 
  Scott Shambarger 
  Sebastian Mitterle 
  SeongHyun Jo 
  

[PATCH v2 3/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

[Roger Pau Monne]

Expose VIRT_SSBD to guests if the hardware supports setting SSBD in
the LS_CFG MSR (a.k.a. non-architectural way). Different AMD CPU
families use different bits in LS_CFG, so exposing VIRT_SPEC_CTRL.SSBD
allows for an unified way of exposing SSBD support to guests on AMD
hardware that's compatible migration wise, regardless of what
underlying mechanism is used to set SSBD.

Note that on AMD Family 17h (Zen 1) the value of SSBD in LS_CFG is
shared between threads on the same core, so there's extra logic in
order to synchronize the value and have SSBD set as long as one of the
threads in the core requires it to be set. Such logic also requires
extra storage for each thread state, which is allocated at
initialization time.

Do the context switching of the SSBD selection in LS_CFG between
hypervisor and guest in the same handler that's already used to switch
the value of VIRT_SPEC_CTRL in the hardware when supported.

Suggested-by: Andrew Cooper 
Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Report legacy SSBD support using a global variable.
 - Use ro_after_init for ssbd_max_cores.
 - Handle boot_cpu_data.x86_num_siblings < 1.
 - Add comment regarding _irqsave usage in amd_set_legacy_ssbd.
---
 xen/arch/x86/cpu/amd.c | 116 -
 xen/arch/x86/cpuid.c   |  10 +++
 xen/arch/x86/hvm/svm/svm.c |  12 +++-
 xen/arch/x86/include/asm/amd.h |   4 ++
 xen/arch/x86/msr.c |  22 ---
 xen/arch/x86/spec_ctrl.c   |   4 +-
 6 files changed, 141 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 4999f8be2b..63d466b1df 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -48,6 +48,7 @@ boolean_param("allow_unsafe", opt_allow_unsafe);
 
 /* Signal whether the ACPI C1E quirk is required. */
 bool __read_mostly amd_acpi_c1e_quirk;
+bool __ro_after_init amd_legacy_ssbd;
 
 static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
 unsigned int *hi)
@@ -685,23 +686,10 @@ void amd_init_lfence(struct cpuinfo_x86 *c)
  * Refer to the AMD Speculative Store Bypass whitepaper:
  * 
https://developer.amd.com/wp-content/resources/124441_AMD64_SpeculativeStoreBypassDisable_Whitepaper_final.pdf
  */
-void amd_init_ssbd(const struct cpuinfo_x86 *c)
+static bool set_legacy_ssbd(const struct cpuinfo_x86 *c, bool enable)
 {
int bit = -1;
 
-   if (cpu_has_ssb_no)
-   return;
-
-   if (cpu_has_amd_ssbd) {
-   /* Handled by common MSR_SPEC_CTRL logic */
-   return;
-   }
-
-   if (cpu_has_virt_ssbd) {
-   wrmsrl(MSR_VIRT_SPEC_CTRL, opt_ssbd ? SPEC_CTRL_SSBD : 0);
-   return;
-   }
-
switch (c->x86) {
case 0x15: bit = 54; break;
case 0x16: bit = 33; break;
@@ -715,20 +703,114 @@ void amd_init_ssbd(const struct cpuinfo_x86 *c)
if (rdmsr_safe(MSR_AMD64_LS_CFG, val) ||
({
val &= ~mask;
-   if (opt_ssbd)
+   if (enable)
val |= mask;
false;
}) ||
wrmsr_safe(MSR_AMD64_LS_CFG, val) ||
({
rdmsrl(MSR_AMD64_LS_CFG, val);
-   (val & mask) != (opt_ssbd * mask);
+   (val & mask) != (enable * mask);
}))
bit = -1;
}
 
-   if (bit < 0)
+   return bit >= 0;
+}
+
+void amd_init_ssbd(const struct cpuinfo_x86 *c)
+{
+   if (cpu_has_ssb_no)
+   return;
+
+   if (cpu_has_amd_ssbd) {
+   /* Handled by common MSR_SPEC_CTRL logic */
+   return;
+   }
+
+   if (cpu_has_virt_ssbd) {
+   wrmsrl(MSR_VIRT_SPEC_CTRL, opt_ssbd ? SPEC_CTRL_SSBD : 0);
+   return;
+   }
+
+   if (!set_legacy_ssbd(c, opt_ssbd))
+   {
printk_once(XENLOG_ERR "No SSBD controls available\n");
+   if (amd_legacy_ssbd)
+   panic("CPU feature mismatch: no legacy SSBD\n");
+   }
+   else if ( c == _cpu_data )
+   amd_legacy_ssbd = true;
+}
+
+static struct ssbd_core {
+spinlock_t lock;
+unsigned int count;
+} *ssbd_core;
+static unsigned int __ro_after_init ssbd_max_cores;
+
+bool __init amd_setup_legacy_ssbd(void)
+{
+   unsigned int i;
+
+   if (boot_cpu_data.x86 != 0x17 || boot_cpu_data.x86_num_siblings <= 1)
+   return true;
+
+   /*
+* One could be forgiven for thinking that c->x86_max_cores is the
+* correct value to use here.
+*
+* However, that value is derived from the current configuration, and
+* c->cpu_core_id is sparse on all but the top end CPUs.  Derive
+* max_cpus from ApicIdCoreIdSize which will cover 

[PATCH v2 2/3] amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests

[Roger Pau Monne]

Allow HVM guests untrapped access to MSR_VIRT_SPEC_CTRL if the
hardware has support for it. This requires adding logic in the
vm{entry,exit} paths for SVM in order to context switch between the
hypervisor value and the guest one. The added handlers for context
switch will also be used for the legacy SSBD support.

Introduce a new synthetic feature leaf (X86_FEATURE_VIRT_SC_MSR_HVM)
to signal whether VIRT_SPEC_CTRL needs to be handled on guest
vm{entry,exit}.

Note the change in the handling of VIRT_SSBD in the featureset
description. The change from 's' to 'S' is due to the fact that now if
VIRT_SSBD is exposed by the hardware it can be passed through to HVM
guests.

Suggested-by: Andrew Cooper 
Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Introduce virt_spec_ctrl vCPU field.
 - Context switch VIRT_SPEC_CTRL on vmentry/vmexit separately from
   SPEC_CTRL.
---
 xen/arch/x86/cpuid.c| 11 ++
 xen/arch/x86/hvm/svm/entry.S|  6 
 xen/arch/x86/hvm/svm/svm.c  | 39 +
 xen/arch/x86/include/asm/cpufeatures.h  |  1 +
 xen/arch/x86/include/asm/msr.h  | 10 ++
 xen/arch/x86/spec_ctrl.c|  9 -
 xen/include/public/arch-x86/cpufeatureset.h |  2 +-
 7 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 4ca77ea870..91e53284fc 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -534,6 +534,10 @@ static void __init calculate_hvm_max_policy(void)
  raw_cpuid_policy.basic.sep )
 __set_bit(X86_FEATURE_SEP, hvm_featureset);
 
+if ( !boot_cpu_has(X86_FEATURE_VIRT_SC_MSR_HVM) )
+/* Clear VIRT_SSBD if VIRT_SPEC_CTRL is not exposed to guests. */
+__clear_bit(X86_FEATURE_VIRT_SSBD, hvm_featureset);
+
 /*
  * If Xen isn't virtualising MSR_SPEC_CTRL for HVM guests (functional
  * availability, or admin choice), hide the feature.
@@ -590,6 +594,13 @@ static void __init calculate_hvm_def_policy(void)
 guest_common_feature_adjustments(hvm_featureset);
 guest_common_default_feature_adjustments(hvm_featureset);
 
+/*
+ * AMD_SSBD if preferred over VIRT_SSBD, so don't expose the later by
+ * default if the former is available.
+ */
+if ( boot_cpu_has(X86_FEATURE_AMD_SSBD) )
+__clear_bit(X86_FEATURE_VIRT_SSBD, hvm_featureset);
+
 sanitise_featureset(hvm_featureset);
 cpuid_featureset_to_policy(hvm_featureset, p);
 recalculate_xstate(p);
diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index 4ae55a2ef6..e2c104bb1e 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -57,6 +57,9 @@ __UNLIKELY_END(nsvm_hap)
 
 clgi
 
+ALTERNATIVE "", STR(call vmentry_virt_spec_ctrl), \
+X86_FEATURE_VIRT_SC_MSR_HVM
+
 /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
 /* SPEC_CTRL_EXIT_TO_SVM   Req: b=curr %rsp=regs/cpuinfo, Clob: 
acd */
 .macro svm_vmentry_spec_ctrl
@@ -114,6 +117,9 @@ __UNLIKELY_END(nsvm_hap)
 ALTERNATIVE "", svm_vmexit_spec_ctrl, X86_FEATURE_SC_MSR_HVM
 /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
+ALTERNATIVE "", STR(call vmexit_virt_spec_ctrl), \
+X86_FEATURE_VIRT_SC_MSR_HVM
+
 stgi
 GLOBAL(svm_stgi_label)
 mov  %rsp,%rdi
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 64a45045da..73a0af599b 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -610,6 +611,14 @@ static void cf_check svm_cpuid_policy_changed(struct vcpu 
*v)
 svm_intercept_msr(v, MSR_SPEC_CTRL,
   cp->extd.ibrs ? MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW);
 
+/*
+ * Give access to MSR_VIRT_SPEC_CTRL if the guest has been told about it
+ * and the hardware implements it.
+ */
+svm_intercept_msr(v, MSR_VIRT_SPEC_CTRL,
+  cp->extd.virt_ssbd && cpu_has_virt_ssbd ?
+  MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW);
+
 /* Give access to MSR_PRED_CMD if the guest has been told about it. */
 svm_intercept_msr(v, MSR_PRED_CMD,
   cp->extd.ibpb ? MSR_INTERCEPT_NONE : MSR_INTERCEPT_RW);
@@ -3105,6 +3114,36 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
 vmcb_set_vintr(vmcb, intr);
 }
 
+/* Called with GIF=0. */
+void vmexit_virt_spec_ctrl(void)
+{
+unsigned int val = opt_ssbd ? SPEC_CTRL_SSBD : 0;
+
+if ( cpu_has_virt_ssbd )
+{
+unsigned int lo, hi;
+
+/*
+ * Need to read from the hardware because VIRT_SPEC_CTRL is not context
+ * switched by the hardware, and we allow the guest untrapped access to
+ * the register.
+ */
+rdmsr(MSR_VIRT_SPEC_CTRL, 

[PATCH v2 1/3] amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL

[Roger Pau Monne]

Use the logic to set shadow SPEC_CTRL values in order to implement
support for VIRT_SPEC_CTRL (signaled by VIRT_SSBD CPUID flag) for HVM
guests. This includes using the spec_ctrl vCPU MSR variable to store
the guest set value of VIRT_SPEC_CTRL.SSBD, which will be OR'ed with
any SPEC_CTRL values being set by the guest.

On hardware having SPEC_CTRL VIRT_SPEC_CTRL will not be offered by
default to guests. VIRT_SPEC_CTRL will only be part of the max CPUID
policy so it can be enabled for compatibility purposes.

Note that '!' is used in order to tag the VIRT_SSBD feature as
specially handled. It's possible for the feature to be available to
guests on hardware that doesn't support it natively, for example when
implemented as done by this patch on top of SPEC_CTRL.SSBD (AMD_SSBD).

Suggested-by: Andrew Cooper 
Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Only expose VIRT_SSBD if AMD_SSBD is available on the host.
 - Revert change to msr-sc= command line option documentation.
 - Only set or clear the SSBD bit of spec_ctrl.
---
 xen/arch/x86/cpuid.c|  7 +++
 xen/arch/x86/hvm/hvm.c  |  1 +
 xen/arch/x86/include/asm/msr.h  |  4 
 xen/arch/x86/msr.c  | 21 +
 xen/arch/x86/spec_ctrl.c|  3 ++-
 xen/include/public/arch-x86/cpufeatureset.h |  2 +-
 6 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index bb554b06a7..4ca77ea870 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -543,6 +543,13 @@ static void __init calculate_hvm_max_policy(void)
 __clear_bit(X86_FEATURE_IBRSB, hvm_featureset);
 __clear_bit(X86_FEATURE_IBRS, hvm_featureset);
 }
+else if ( boot_cpu_has(X86_FEATURE_AMD_SSBD) )
+/*
+ * If SPEC_CTRL.SSBD is available VIRT_SPEC_CTRL.SSBD can be exposed
+ * and implemented using the former. Expose in the max policy only as
+ * the preference is for guests to use SPEC_CTRL.SSBD if available.
+ */
+__set_bit(X86_FEATURE_VIRT_SSBD, hvm_featureset);
 
 /*
  * With VT-x, some features are only supported by Xen if dedicated
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 709a4191ef..595858f2a7 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1334,6 +1334,7 @@ static const uint32_t msrs_to_send[] = {
 MSR_INTEL_MISC_FEATURES_ENABLES,
 MSR_IA32_BNDCFGS,
 MSR_IA32_XSS,
+MSR_VIRT_SPEC_CTRL,
 MSR_AMD64_DR0_ADDRESS_MASK,
 MSR_AMD64_DR1_ADDRESS_MASK,
 MSR_AMD64_DR2_ADDRESS_MASK,
diff --git a/xen/arch/x86/include/asm/msr.h b/xen/arch/x86/include/asm/msr.h
index ce4fe51afe..ab6fbb5051 100644
--- a/xen/arch/x86/include/asm/msr.h
+++ b/xen/arch/x86/include/asm/msr.h
@@ -291,6 +291,7 @@ struct vcpu_msrs
 {
 /*
  * 0x0048 - MSR_SPEC_CTRL
+ * 0xc001011f - MSR_VIRT_SPEC_CTRL (if X86_FEATURE_AMD_SSBD)
  *
  * For PV guests, this holds the guest kernel value.  It is accessed on
  * every entry/exit path.
@@ -306,6 +307,9 @@ struct vcpu_msrs
  * We must clear/restore Xen's value before/after VMRUN to avoid unduly
  * influencing the guest.  In order to support "behind the guest's back"
  * protections, we load this value (commonly 0) before VMRUN.
+ *
+ * Once of such "behind the guest's back" usages is setting SPEC_CTRL.SSBD
+ * if the guest sets VIRT_SPEC_CTRL.SSBD.
  */
 struct {
 uint32_t raw;
diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index 01a15857b7..b212acf29d 100644
--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -381,6 +381,13 @@ int guest_rdmsr(struct vcpu *v, uint32_t msr, uint64_t 
*val)
? K8_HWCR_TSC_FREQ_SEL : 0;
 break;
 
+case MSR_VIRT_SPEC_CTRL:
+if ( !cp->extd.virt_ssbd )
+goto gp_fault;
+
+*val = msrs->spec_ctrl.raw & SPEC_CTRL_SSBD;
+break;
+
 case MSR_AMD64_DE_CFG:
 if ( !(cp->x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) )
 goto gp_fault;
@@ -666,6 +673,20 @@ int guest_wrmsr(struct vcpu *v, uint32_t msr, uint64_t val)
 wrmsr_tsc_aux(val);
 break;
 
+case MSR_VIRT_SPEC_CTRL:
+if ( !cp->extd.virt_ssbd )
+goto gp_fault;
+
+/*
+ * Only supports SSBD bit, the rest are ignored. Only modify the SSBD
+ * bit in case other bits are set.
+ */
+if ( val & SPEC_CTRL_SSBD )
+msrs->spec_ctrl.raw |= SPEC_CTRL_SSBD;
+else
+msrs->spec_ctrl.raw &= ~SPEC_CTRL_SSBD;
+break;
+
 case MSR_AMD64_DE_CFG:
 /*
  * OpenBSD 6.7 will panic if writing to DE_CFG triggers a #GP:
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 1408e4c7ab..f338bfe292 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -402,12 +402,13 @@ static void __init 

[PATCH v2 0/3] amd/msr: implement MSR_VIRT_SPEC_CTRL for HVM guests

[Roger Pau Monne]

Hello,

The following series implements support for MSR_VIRT_SPEC_CTRL
(VIRT_SSBD) on different AMD CPU families.

Note that the support is added backwards, starting with the newer CPUs
that support MSR_SPEC_CTRL and moving to the older ones either using
MSR_VIRT_SPEC_CTRL or the SSBD bit in LS_CFG.

Xen is still free to use it's own SSBD setting, as the selection is
context switched on vm{entry,exit}.

On Zen2 and later, SPEC_CTRL.SSBD exists and should be used in
preference to VIRT_SPEC_CTRL.SSBD.  However, for migration
compatibility, Xen offers VIRT_SSBD to guests (in the max CPUID policy,
not default) implemented in terms of SPEC_CTRL.SSBD.

On Fam15h thru Zen1, Xen exposes VIRT_SSBD to guests by default to
abstract away the model and/or hypervisor specific differences in
MSR_LS_CFG/MSR_VIRT_SPEC_CTRL.

Note that if the hardware itself does offer VIRT_SSBD (ie: very likely
when running virtualized on < Zen2 hardware) and not AMD_SSBD Xen will
allow untrapped access to MSR_VIRT_SPEC_CTRL for HVM guests.

So the implementation of VIRT_SSBD exposed to HVM guests will use one of
the following underlying mechanisms, in the preference order listed
below:

 * SPEC_CTRL.SSBD. (patch 1)
 * VIRT_SPEC_CTRL.SSBD (untrapped). (patch 2).
 * Non-architectural way using LS_CFG. (patch 3)

Thanks, Roger.

Roger Pau Monne (3):
  amd/msr: implement VIRT_SPEC_CTRL for HVM guests on top of SPEC_CTRL
  amd/msr: allow passthrough of VIRT_SPEC_CTRL for HVM guests
  amd/msr: implement VIRT_SPEC_CTRL for HVM guests using legacy SSBD

 xen/arch/x86/cpu/amd.c  | 116 +---
 xen/arch/x86/cpuid.c|  28 +
 xen/arch/x86/hvm/hvm.c  |   1 +
 xen/arch/x86/hvm/svm/entry.S|   6 +
 xen/arch/x86/hvm/svm/svm.c  |  49 +
 xen/arch/x86/include/asm/amd.h  |   4 +
 xen/arch/x86/include/asm/cpufeatures.h  |   1 +
 xen/arch/x86/include/asm/msr.h  |  14 +++
 xen/arch/x86/msr.c  |  27 +
 xen/arch/x86/spec_ctrl.c|  12 +-
 xen/include/public/arch-x86/cpufeatureset.h |   2 +-
 11 files changed, 241 insertions(+), 19 deletions(-)

-- 
2.34.1

Re: [PATCH v3 2/5] xen: make evtchn_alloc_unbound public

[Daniel P. Smith]

On 1/28/22 16:33, Stefano Stabellini wrote:
> From: Luca Miccio 
> 
> The xenstore event channel will be allocated for dom0less domains. It is
> necessary to have access to the evtchn_alloc_unbound function to do
> that, so make evtchn_alloc_unbound public.
> 
> Add a skip_xsm parameter to allow disabling the XSM check in
> evtchn_alloc_unbound (xsm_evtchn_unbound wouldn't work for a call
> originated from Xen before running any domains.)
> 
> Signed-off-by: Luca Miccio 
> Signed-off-by: Stefano Stabellini 
> CC: Julien Grall 
> CC: Volodymyr Babchuk 
> CC: Bertrand Marquis 
> CC: Andrew Cooper 
> CC: George Dunlap 
> CC: Jan Beulich 
> CC: Wei Liu 
> ---
> Changes v3:
> - expose evtchn_alloc_unbound, assing a skip_xsm parameter
> ---
>  xen/common/event_channel.c | 13 -
>  xen/include/xen/event.h|  3 +++
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index da88ad141a..be57d00a15 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -284,7 +284,7 @@ void evtchn_free(struct domain *d, struct evtchn *chn)
>  xsm_evtchn_close_post(chn);
>  }
>  
> -static int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
> +int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc, bool skip_xsm)
>  {
>  struct evtchn *chn;
>  struct domain *d;
> @@ -301,9 +301,12 @@ static int evtchn_alloc_unbound(evtchn_alloc_unbound_t 
> *alloc)
>  ERROR_EXIT_DOM(port, d);
>  chn = evtchn_from_port(d, port);
>  
> -rc = xsm_evtchn_unbound(XSM_TARGET, d, chn, alloc->remote_dom);
> -if ( rc )
> -goto out;
> +if ( !skip_xsm )
> +{
> +rc = xsm_evtchn_unbound(XSM_TARGET, d, chn, alloc->remote_dom);
> +if ( rc )
> +goto out;
> +}

Please do not subvert the security framework because it causes an
inconvenience. As Jan recommended, work within the XSM check to allow
your access so that we may ensure it is done safely. If you need any
help making modifications to XSM, please do not hesitate to reach out as
I will gladly help.

>  evtchn_write_lock(chn);
>  
> @@ -1195,7 +1198,7 @@ long do_event_channel_op(int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  struct evtchn_alloc_unbound alloc_unbound;
>  if ( copy_from_guest(_unbound, arg, 1) != 0 )
>  return -EFAULT;
> -rc = evtchn_alloc_unbound(_unbound);
> +rc = evtchn_alloc_unbound(_unbound, false);
>  if ( !rc && __copy_to_guest(arg, _unbound, 1) )
>  rc = -EFAULT; /* Cleaning up here would be a mess! */
>  break;
> diff --git a/xen/include/xen/event.h b/xen/include/xen/event.h
> index 21c95e14fd..0a2cdedf7d 100644
> --- a/xen/include/xen/event.h
> +++ b/xen/include/xen/event.h
> @@ -68,6 +68,9 @@ int evtchn_close(struct domain *d1, int port1, bool guest);
>  /* Free an event channel. */
>  void evtchn_free(struct domain *d, struct evtchn *chn);
>  
> +/* Create a new event channel port */
> +int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc, bool skip_xsm);
> +
>  /* Allocate a specific event channel port. */
>  int evtchn_allocate_port(struct domain *d, unsigned int port);
>  

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Philippe Mathieu-Daudé]

On 15/3/22 14:59, Markus Armbruster wrote:

Alex Bennée  writes:


Markus Armbruster  writes:


g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
for two reasons.  One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.




diff --git a/semihosting/config.c b/semihosting/config.c
index 137171b717..6d48ec9566 100644
--- a/semihosting/config.c
+++ b/semihosting/config.c
@@ -98,7 +98,7 @@ static int add_semihosting_arg(void *opaque,
  if (strcmp(name, "arg") == 0) {
  s->argc++;
  /* one extra element as g_strjoinv() expects NULL-terminated array */
-s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
+s->argv = g_renew(void *, s->argv, s->argc + 1);


This did indeed break CI because s->argv is an array of *char:

../semihosting/config.c:101:17: error: assignment to ‘const char **’ from 
incompatible pointer type ‘void **’ [-Werror=incompatible-pointer-types]
   101 | s->argv = g_renew(void *, s->argv, s->argc + 1);
   | ^
cc1: all warnings being treated as errors

So it did the job of type checking but failed to build ;-)


You found a hole in my compile testing, thanks!

I got confused about the configuration of my build trees.  Catching such
mistakes is what CI is for :)


FYI Alex fixed this here:
https://lore.kernel.org/qemu-devel/20220315121251.2280317-8-alex.ben...@linaro.org/

So your series could go on top (modulo the Coverity change).

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Eric Blake  writes:

> On Mon, Mar 14, 2022 at 05:01:08PM +0100, Markus Armbruster wrote:
>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>> for two reasons.  One, it catches multiplication overflowing size_t.
>> Two, it returns T * rather than void *, which lets the compiler catch
>> more type errors.
>> 
>> This commit only touches allocations with size arguments of the form
>> sizeof(T).
>> 
>> Patch created mechanically with:
>> 
>> $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
>>   --macro-file scripts/cocci-macro-file.h FILES...
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>
> I agree that this is mechanical, but...
>
> 
>>  qga/commands-win32.c |  8 ++---
>>  qga/commands.c   |  2 +-
>>  qom/qom-qmp-cmds.c   |  2 +-
>>  replay/replay-char.c |  4 +--
>>  replay/replay-events.c   | 10 +++---
>>  scripts/coverity-scan/model.c|  2 +-
>
> ...are we sure we want to touch this particular file?

Good catch!

>> diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
>> index 9d4fba53d9..30bea672e1 100644
>> --- a/scripts/coverity-scan/model.c
>> +++ b/scripts/coverity-scan/model.c
>> @@ -356,7 +356,7 @@ int g_poll (GPollFD *fds, unsigned nfds, int timeout)
>>  typedef struct _GIOChannel GIOChannel;
>>  GIOChannel *g_io_channel_unix_new(int fd)
>>  {
>> -GIOChannel *c = g_malloc0(sizeof(GIOChannel));
>> +GIOChannel *c = g_new0(GIOChannel, 1);
>>  __coverity_escape__(fd);
>>  return c;
>>  }
>
> Our model has a definition of g_malloc0(), but I'm not sure whether
> Coverity picks up the macro g_new0() in the same manner.

I believe it does, by parsing the macro definition from the header.

Regardless, I'd prefer to keep model.c self-contained.  I'll drop this
hunk.

Thanks!

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Alex Bennée  writes:

> Markus Armbruster  writes:
>
>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>> for two reasons.  One, it catches multiplication overflowing size_t.
>> Two, it returns T * rather than void *, which lets the compiler catch
>> more type errors.
>>
> 
>> diff --git a/semihosting/config.c b/semihosting/config.c
>> index 137171b717..6d48ec9566 100644
>> --- a/semihosting/config.c
>> +++ b/semihosting/config.c
>> @@ -98,7 +98,7 @@ static int add_semihosting_arg(void *opaque,
>>  if (strcmp(name, "arg") == 0) {
>>  s->argc++;
>>  /* one extra element as g_strjoinv() expects NULL-terminated array 
>> */
>> -s->argv = g_realloc(s->argv, (s->argc + 1) * sizeof(void *));
>> +s->argv = g_renew(void *, s->argv, s->argc + 1);
>
> This did indeed break CI because s->argv is an array of *char:
>
> ../semihosting/config.c:101:17: error: assignment to ‘const char **’ from 
> incompatible pointer type ‘void **’ [-Werror=incompatible-pointer-types]
>   101 | s->argv = g_renew(void *, s->argv, s->argc + 1);
>   | ^
> cc1: all warnings being treated as errors
>
> So it did the job of type checking but failed to build ;-)

You found a hole in my compile testing, thanks!

I got confused about the configuration of my build trees.  Catching such
mistakes is what CI is for :)

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Markus Armbruster]

Alex Bennée  writes:

> Markus Armbruster  writes:
>
>> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
>> for two reasons.  One, it catches multiplication overflowing size_t.
>> Two, it returns T * rather than void *, which lets the compiler catch
>> more type errors.
>>
>> This commit only touches allocations with size arguments of the form
>> sizeof(T).
>>
>> Patch created mechanically with:
>>
>> $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
>>   --macro-file scripts/cocci-macro-file.h FILES...
>>
>> Signed-off-by: Markus Armbruster 

[...]

>> --- a/hw/pci/pcie_sriov.c
>> +++ b/hw/pci/pcie_sriov.c
>> @@ -177,7 +177,7 @@ static void register_vfs(PCIDevice *dev)
>>  assert(sriov_cap > 0);
>>  num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
>>  
>> -dev->exp.sriov_pf.vf = g_malloc(sizeof(PCIDevice *) * num_vfs);
>> +dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
>>  assert(dev->exp.sriov_pf.vf);
>
> So what I find confusing about the conversion of sizeof(foo *) is that
> while the internal sizeof in g_new is unaffected I think the casting
> ends up as
>
>  foo **

Yes.  g_malloc(...) returns void *.  g_new(T, ...) returns T *.

> but I guess the compiler would have complained so maybe I don't
> understand the magic enough.

It doesn't complain here because dev->exp.sriov_pf.vf is of type
PCIDevice **:

struct PCIESriovPF {
uint16_t num_vfs;   /* Number of virtual functions created */
uint8_t vf_bar_type[PCI_NUM_REGIONS];   /* Store type for each VF bar */
const char *vfname; /* Reference to the device type used for the VFs */
PCIDevice **vf; /* Pointer to an array of num_vfs VF devices */
};

It does complain when the types don't match, as shown in PATCH 2.

> 
>> index 42130667a7..598e6adc5e 100644
>> --- a/hw/rdma/vmw/pvrdma_dev_ring.c
>> +++ b/hw/rdma/vmw/pvrdma_dev_ring.c
>> @@ -41,7 +41,7 @@ int pvrdma_ring_init(PvrdmaRing *ring, const char *name, 
>> PCIDevice *dev,
>>  qatomic_set(>ring_state->cons_head, 0);
>>  */
>>  ring->npages = npages;
>> -ring->pages = g_malloc0(npages * sizeof(void *));
>> +ring->pages = g_new0(void *, npages);
>
> At least here ring->pages agrees about void ** being the result.
>
> 
>
> So other than my queries about the sizeof(foo *) which I'd like someone
> to assuage me of my concerns it looks like the script has done a
> thorough mechanical job as all good scripts should ;-)
>
> Reviewed-by: Alex Bennée 

Thanks!

[xen-unstable test] 168604: tolerable FAIL

[osstest service owner]

flight 168604 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168604/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 168585
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168585
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168585
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 168585
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 168585
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 168585
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 168585
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 168585
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 168585
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 168585
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168585
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 168585
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  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-credit2  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-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   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-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-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-raw 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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  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  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass

version targeted for testing:
 xen  dedb0aa42c6d1ee31805dfc61630db2c41117330
baseline version:
 xen  

[ovmf test] 168614: regressions - FAIL

[osstest service owner]

flight 168614 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168614/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf a13dfc769bd7097d8d9ffe3e029a2c1d062d712b
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  140 attempts
Testing same since   168579  2022-03-14 08:41:36 Z1 days   11 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sean Rhodes 
  Sebastien Boeuf 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 629 lines long.)

[ovmf test] 168610: regressions - FAIL

[osstest service owner]

flight 168610 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168610/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf a13dfc769bd7097d8d9ffe3e029a2c1d062d712b
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   15 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  139 attempts
Testing same since   168579  2022-03-14 08:41:36 Z1 days   10 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sean Rhodes 
  Sebastien Boeuf 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 629 lines long.)

Re: [XEN][RFC PATCH v3 14/14] tools/xl: Add new xl command overlay for device tree overlay support

[Luca Fancellu]

> On 8 Mar 2022, at 19:47, Vikram Garhwal  wrote:
> 
> Signed-off-by: Vikram Garhwal 
> ---
> tools/xl/xl.h   |  4 
> tools/xl/xl_cmdtable.c  |  6 ++
> tools/xl/xl_vmcontrol.c | 45 +
> 3 files changed, 55 insertions(+)
> 
> diff --git a/tools/xl/xl.h b/tools/xl/xl.h
> index c5c4bedbdd..604fd5bb94 100644
> --- a/tools/xl/xl.h
> +++ b/tools/xl/xl.h
> @@ -97,6 +97,9 @@ struct save_file_header {
> 
> #define SAVEFILE_BYTEORDER_VALUE ((uint32_t)0x01020304UL)
> 
> +#define XL_DT_OVERLAY_ADD   1
> +#define XL_DT_OVERLAY_REMOVE2

Maybe you can get rid of them and ...

> +
> void save_domain_core_begin(uint32_t domid,
> int preserve_domid,
> const char *override_config_file,
> @@ -139,6 +142,7 @@ int main_shutdown(int argc, char **argv);
> int main_reboot(int argc, char **argv);
> int main_list(int argc, char **argv);
> int main_vm_list(int argc, char **argv);
> +int main_dt_overlay(int argc, char **argv);
> int main_create(int argc, char **argv);
> int main_config_update(int argc, char **argv);
> int main_button_press(int argc, char **argv);
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index 661323d488..5812d19db8 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -20,6 +20,12 @@
> #include "xl.h"
> 
> const struct cmd_spec cmd_table[] = {
> +{ "overlay",
> +  _dt_overlay, 1, 1,
> +  "Add/Remove a device tree overlay",
> +  "add/remove <.dtbo>"
> +  "-h print this help\n"
> +},
> { "create",
>   _create, 1, 1,
>   "Create a domain from config file ",
> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 435155a033..76b969dc33 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -1262,6 +1262,51 @@ int main_create(int argc, char **argv)
> return 0;
> }
> 
> +int main_dt_overlay(int argc, char **argv)
> +{
> +const char *overlay_ops = argv[1];
> +const char *overlay_config_file = argv[2];
> +void *overlay_dtb = NULL;
> +int rc;
> +uint8_t op;
> +int overlay_dtb_size = 0;
> +
> +if (overlay_ops == NULL) {
> +fprintf(stderr, "No overlay operation mode provided\n");
> +return ERROR_FAIL;
> +}
> +
> +if (strcmp(overlay_ops, "add") == 0)
> +op = XL_DT_OVERLAY_ADD;
> +else if (strcmp(overlay_ops, "remove") == 0)
> +op = XL_DT_OVERLAY_REMOVE;

Use them there, maybe defining const int  = 

Since these values are only used there

[PATCH] x86: re-inline indirect CALL/JMP

[Jan Beulich]

Using the same thunk for heterogeneous calls (i.e. ones to functions
with entirely different parameter types) is a problem: Speculation may
e.g. result in use as pointers for arguments which are are actually
integers. This is a result of target prediction information being tied
to the address of the RET instruction in each thunk (of which we've got
only 15, and of which in turn a fair share of the call sites use the
single one where %rax is holding the target address).

Except when actually using the full retpoline, arrange for alternatives
patching to put the JMP and LFENCE forms of the thunk back inline.

Signed-off-by: Jan Beulich 
---
I'm sure I didn't get the reasoning right / complete; I'd appreciate
clear enough indication of what needs adding/changing.

Some of the changes to may not strictly be necessary:
- INDIRECT_BRANCH: We don't have any uses left in assembly code.
- GEN_INDIRECT_THUNK: Continuing to patch the thunks when they're not
  used is merely pointless. The change, however, is more in anticipation
  that X86_FEATURE_IND_THUNK_{LFENCE,JMP} may not be fully suitable
  names anymore when the code gets inlined (at least I probably wouldn't
  call such "thunk" anymore).

While perhaps not a big problem, I'd like to point out that this more
than doubles the size of the .altinstr_replacement section (with an
accordingly large increase of .altinstructions), unless we were to make
use of "x86/alternatives: allow replacement code snippets to be merged"
(which of course affects only .altinstr_replacement).

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -195,7 +195,7 @@ t2 = $(call as-insn,$(CC) -I$(BASEDIR)/a
 # https://bugs.llvm.org/show_bug.cgi?id=36110
 t3 = $(call as-insn,$(CC),".macro FOO;.endm"$(close); asm volatile 
$(open)".macro FOO;.endm",-no-integrated-as)
 
-CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3))
+CLANG_FLAGS += $(call or,$(t1),$(t2),$(t3),-DCLANG_INTEGRATED_AS)
 endif
 
 CLANG_FLAGS += -Werror=unknown-warning-option
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -240,7 +240,7 @@ $(obj)/efi/buildid.o $(obj)/efi/relocs-d
 .PHONY: include
 include: $(BASEDIR)/arch/x86/include/asm/asm-macros.h
 
-$(obj)/asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -P
+$(obj)/asm-macros.i: CFLAGS-y += -D__ASSEMBLY__ -D__ASM_MACROS__ -P
 
 $(BASEDIR)/arch/x86/include/asm/asm-macros.h: $(obj)/asm-macros.i 
$(src)/Makefile
$(call filechk,asm-macros.h)
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -244,11 +244,19 @@ static void init_or_livepatch _apply_alt
 
 a->priv = 1;
 
-/* Nothing useful to do? */
-if ( toolchain_nops_are_ideal || a->pad_len <= 1 )
+/* No padding at all? */
+if ( !a->pad_len )
 continue;
 
-add_nops(buf, a->pad_len);
+/* For a JMP to an indirect thunk, replace NOP by INT3. */
+if ( IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_BRANCH) &&
+ a->cpuid == X86_FEATURE_IND_THUNK_JMP && orig[0] == 0xe9 )
+memset(buf, 0xcc, a->pad_len);
+/* Nothing useful to do? */
+else if ( toolchain_nops_are_ideal || a->pad_len <= 1 )
+continue;
+else
+add_nops(buf, a->pad_len);
 text_poke(orig + a->orig_len, buf, a->pad_len);
 continue;
 }
@@ -330,7 +338,12 @@ static void init_or_livepatch _apply_alt
 
 a->priv = 1;
 
-add_nops(buf + a->repl_len, total_len - a->repl_len);
+/* When re-inlining an indirect JMP, pad it by INT3, not NOPs. */
+if ( IS_ENABLED(CONFIG_SPECULATIVE_HARDEN_BRANCH) &&
+ a->cpuid == X86_FEATURE_IND_THUNK_JMP && orig[0] == 0xe9 )
+memset(buf + a->repl_len, 0xcc, total_len - a->repl_len);
+else
+add_nops(buf + a->repl_len, total_len - a->repl_len);
 text_poke(orig, buf, total_len);
 }
 
--- a/xen/arch/x86/asm-macros.c
+++ b/xen/arch/x86/asm-macros.c
@@ -1,2 +1,3 @@
 #include 
 #include 
+#include 
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -98,12 +98,42 @@ search_exception_table(const struct cpu_
  regs->rsp > (unsigned long)regs &&
  regs->rsp < (unsigned long)get_cpu_info() )
 {
-unsigned long retptr = *(unsigned long *)regs->rsp;
+unsigned long retaddr = *(unsigned long *)regs->rsp;
+unsigned long retptr = 0;
+unsigned int pad = 0;
 
-region = find_text_region(retptr);
-retptr = region && region->ex
- ? search_one_extable(region->ex, region->ex_end - 1, retptr)
- : 0;
+region = find_text_region(retaddr);
+while ( region && region->ex )
+{
+retptr = search_one_extable(region->ex, region->ex_end - 1,
+retaddr + pad);
+if ( !retptr )
+{
+/*
+ * Indirect call thunk patching can lead to 

Re: [XEN PATCH] tools/libs/light/libxl_pci.c: explicitly grant access to Intel IGD opregion

[Jan Beulich]

On 14.03.2022 04:41, Chuck Zmudzinski wrote:
> Fixes: abfb006f1ff4 (tools/libxl: explicitly grant access to needed 
> I/O-memory ranges)
> Fixes: 0561e1f01e87 (xen/common: do not implicitly permit access to mapped 
> I/O memory)
> Backport: 4.12+

Just fyi: This is fine to have as a tag, but it wouldn't be backported
farther than to 4.15.

Apart from this largely some style issues (see below), but please
realize that I'm not a libxl maintainer and hence I may not have good
enough knowledge of, in particular, potential unwritten conventions.

> @@ -610,6 +612,45 @@ out:
>  return ret;
>  }
>  
> +static uint32_t sysfs_dev_get_igd_opregion(libxl__gc *gc,
> +   libxl_device_pci *pci)
> +{
> +char *pci_device_config_path =
> +GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/config",
> +  pci->domain, pci->bus, pci->dev, pci->func);
> +size_t read_items;
> +uint32_t igd_opregion;
> +uint32_t error = 0x;

I think this constant wants to gain a #define, to be able to correlate
the use sites. I'm also not sure of the value - in principle the
register can hold this value, but of course then it won't be 3 pages.
Maybe the error check further down should be to see whether adding 2
to the value would overflow in 32 bits? (In that case a #define may
not be needed anymore, as there wouldn't be multiple instances of the
constant in the code.)

> +
> +FILE *f = fopen(pci_device_config_path, "r");
> +if (!f) {

While libxl has some special style rules, I think it still wants a
blank line between declaration(s) and statement(s), just like we
expect elsewhere. Effectively you want to simply move the blank line
you have one line down.

> @@ -2531,6 +2572,37 @@ int libxl__grant_vga_iomem_permission(libxl__gc *gc, 
> const uint32_t domid,
>domid, vga_iomem_start, (vga_iomem_start + 0x20 - 1));
>  return ret;
>  }
> +
> +/* If this is an Intel IGD, allow access to the IGD opregion */
> +if (!libxl__is_igd_vga_passthru(gc, d_config)) return 0;

Despite the provision for "return" or alike to go on the same line
as an error code check, I don't think this is okay here. It would be
if, as iirc generally expected in libxl, you latched the function
return value into a local variable named "rc" (I think).

> +uint32_t igd_opregion = sysfs_dev_get_igd_opregion(gc, pci);
> +uint32_t error = 0x;

Please don't mix declarations and statements. I also don't think
"error" is really necessary as a local variable, but with the change
suggested above it might disappear anyway.

> +if (igd_opregion == error) break;

Like above I'm not sure this is okay to all live on one line. I also
think it would be nice if you used "return 0" or "break" consistently.
Of course a related question is whether failure here should actually
be reported to the caller.

> +vga_iomem_start = ( (uint64_t) igd_opregion ) >> XC_PAGE_SHIFT;

There's no need for a cast here, as you're right-shifting. Also
(just fyi) there would have been three to many spaces here. I'm
additionally not certain whether re-using a variable for a purpose
not matching its name is deemed acceptable by libxl maintainers.

> +ret = xc_domain_iomem_permission(CTX->xch, stubdom_domid,
> + vga_iomem_start,
> + IGD_OPREGION_PAGES, 1);
> +if (ret < 0) {
> +LOGED(ERROR, domid,
> +  "failed to give stubdom%d access to iomem range "
> +  "%"PRIx64"-%"PRIx64" for IGD passthru",
> +  stubdom_domid, vga_iomem_start, (vga_iomem_start +
> +IGD_OPREGION_PAGES - 1));
> +return ret;
> +}

I have to admit that I find it odd that this is done unconditionally,
but I notice the same is done in pre-existing code. I would have
expected this to happen only when there actually is a device model
stub domain.

Jan

> +ret = xc_domain_iomem_permission(CTX->xch, domid,
> + vga_iomem_start,
> + IGD_OPREGION_PAGES, 1);
> +if (ret < 0) {
> +LOGED(ERROR, domid,
> +  "failed to give dom%d access to iomem range "
> +  "%"PRIx64"-%"PRIx64" for IGD passthru",
> +  domid, vga_iomem_start, (vga_iomem_start +
> +   IGD_OPREGION_PAGES - 1));
> +return ret;
> +}
>  break;
>  }
>  

Re: [XEN][RFC PATCH v3 13/14] tools/libs/light: Implement new libxl functions for device tree overlay ops

[Luca Fancellu]

> On 8 Mar 2022, at 19:47, Vikram Garhwal  wrote:
> 
> Signed-off-by: Vikram Garhwal 

I don’t know if an empty commit message is ok here, will leave to the maintainer
the choice, for me the code looks ok

Reviewed-by: Luca Fancellu 

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Dr. David Alan Gilbert]

* Markus Armbruster (arm...@redhat.com) wrote:
> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
> for two reasons.  One, it catches multiplication overflowing size_t.
> Two, it returns T * rather than void *, which lets the compiler catch
> more type errors.
> 
> This commit only touches allocations with size arguments of the form
> sizeof(T).
> 
> Patch created mechanically with:
> 
> $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
>--macro-file scripts/cocci-macro-file.h FILES...
> 
> Signed-off-by: Markus Armbruster 

Just a small patch then...

> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index d65e744af9..aace12a787 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -91,7 +91,7 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
>  {
>  int i;
>  int64_t dirty_rate = DirtyStat.dirty_rate;
> -struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
> +struct DirtyRateInfo *info = g_new0(DirtyRateInfo, 1);
>  DirtyRateVcpuList *head = NULL, **tail = 
>  
>  info->status = CalculatingState;
> @@ -112,7 +112,7 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
>  info->sample_pages = 0;
>  info->has_vcpu_dirty_rate = true;
>  for (i = 0; i < DirtyStat.dirty_ring.nvcpu; i++) {
> -DirtyRateVcpu *rate = g_malloc0(sizeof(DirtyRateVcpu));
> +DirtyRateVcpu *rate = g_new0(DirtyRateVcpu, 1);
>  rate->id = DirtyStat.dirty_ring.rates[i].id;
>  rate->dirty_rate = DirtyStat.dirty_ring.rates[i].dirty_rate;
>  QAPI_LIST_APPEND(tail, rate);
> diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> index aba1c88a0c..3a7ae44485 100644
> --- a/migration/multifd-zlib.c
> +++ b/migration/multifd-zlib.c
> @@ -43,7 +43,7 @@ struct zlib_data {
>   */
>  static int zlib_send_setup(MultiFDSendParams *p, Error **errp)
>  {
> -struct zlib_data *z = g_malloc0(sizeof(struct zlib_data));
> +struct zlib_data *z = g_new0(struct zlib_data, 1);
>  z_stream *zs = >zs;
>  
>  zs->zalloc = Z_NULL;
> @@ -164,7 +164,7 @@ static int zlib_send_prepare(MultiFDSendParams *p, Error 
> **errp)
>   */
>  static int zlib_recv_setup(MultiFDRecvParams *p, Error **errp)
>  {
> -struct zlib_data *z = g_malloc0(sizeof(struct zlib_data));
> +struct zlib_data *z = g_new0(struct zlib_data, 1);
>  z_stream *zs = >zs;
>  
>  p->data = z;
> diff --git a/migration/ram.c b/migration/ram.c
> index 170e522a1f..3532f64ecb 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2059,7 +2059,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
> start, ram_addr_t len)
>  }
>  
>  struct RAMSrcPageRequest *new_entry =
> -g_malloc0(sizeof(struct RAMSrcPageRequest));
> +g_new0(struct RAMSrcPageRequest, 1);
>  new_entry->rb = ramblock;
>  new_entry->offset = start;
>  new_entry->len = len;

For migration:
Acked-by: Dr. David Alan Gilbert 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [XEN][RFC PATCH v3 12/14] tools/libs/ctrl: Implement new xc interfaces for dt overlay

[Luca Fancellu]

> 
> diff --git a/tools/libs/ctrl/Makefile b/tools/libs/ctrl/Makefile
> index ef7362327f..848a8737c7 100644
> --- a/tools/libs/ctrl/Makefile
> +++ b/tools/libs/ctrl/Makefile
> @@ -3,6 +3,7 @@ include $(XEN_ROOT)/tools/Rules.mk
> 
> SRCS-y   += xc_altp2m.c
> SRCS-y   += xc_cpupool.c
> +SRCS-y   += xc_overlay.c

I think these entries are in alphabetical order

> SRCS-y   += xc_domain.c
> SRCS-y   += xc_evtchn.c
> SRCS-y   += xc_gnttab.c
> diff --git a/tools/libs/ctrl/xc_overlay.c b/tools/libs/ctrl/xc_overlay.c
> new file mode 100644
> index 00..8fe780d75a
> --- /dev/null
> +++ b/tools/libs/ctrl/xc_overlay.c
> @@ -0,0 +1,51 @@
> +/*
> + *

This blank line can be removed 

> + * Overlay control functions.
> + * Copyright (C) 2021 Xilinx Inc.
> + * Author Vikram Garhwal 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation;
> + * version 2.1 of the License.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; If not, see 
> .
> + */
> +
> +#include "xc_bitops.h"
> +#include "xc_private.h"
> +#include 
> +#include 
> +
> +int xc_dt_overlay(xc_interface *xch, void *overlay_fdt, int overlay_fdt_size,
> +  uint8_t overlay_op)
> +{
> +int err;
> +DECLARE_SYSCTL;
> +
> +DECLARE_HYPERCALL_BOUNCE(overlay_fdt, overlay_fdt_size,
> +XC_HYPERCALL_BUFFER_BOUNCE_IN);

XC_HYPERCALL_BUFFER_BOUNCE_IN can stay at the same level of overlay_fdt

Re: [XEN][RFC PATCH v3 11/14] xen/arm: Implement device tree node addition functionalities

[Luca Fancellu]

> 
> +static int dt_overlay_add_node(struct dt_device_node *device_node,
> +  const char *parent_node_path)

Const should be indented at the same level of struct

> +/*
> + * Adds device tree nodes under target node.
> + * We use dt_host_new to unflatten the updated device_tree_flattened. This is
> + * done to avoid the removal of device_tree generation, iomem regions 
> mapping to
> + * hardware domain done by handle_node().
> + */
> +static long handle_add_overlay_nodes(void *overlay_fdt,
> + uint32_t overlay_fdt_size)
> +{
> +int rc = 0;
> +struct dt_device_node *overlay_node;
> +char **nodes_full_path = NULL;
> +int **nodes_irq = NULL;
> +int *node_num_irq = NULL;
> +void *fdt = NULL;
> +struct dt_device_node *dt_host_new = NULL;
> +struct domain *d = hardware_domain;
> +struct overlay_track *tr = NULL;
> +unsigned int naddr;
> +unsigned int num_irq;
> +unsigned int i, j, k;
> +unsigned int num_overlay_nodes;

All unsigned int can stay in the same line

> +u64 addr, size;
> +
> +fdt = xmalloc_bytes(fdt_totalsize(device_tree_flattened));
> +if ( fdt == NULL )
> +return -ENOMEM;
> +
> +num_overlay_nodes = overlay_node_count(overlay_fdt);
> +if ( num_overlay_nodes == 0 )
> +{
> +xfree(fdt);
> +return -ENOMEM;
> +}
> +
> +spin_lock(_lock);
> +
> +memcpy(fdt, device_tree_flattened, fdt_totalsize(device_tree_flattened));
> +
> +rc = check_overlay_fdt(overlay_fdt, overlay_fdt_size);
> +if ( rc )
> +{
> +xfree(fdt);
> +return rc;
> +}
> +
> +/*
> + * overlay_get_nodes_info is called to get the node information from 
> dtbo.
> + * This is done before fdt_overlay_apply() because the overlay apply will
> + * erase the magic of overlay_fdt.
> + */
> +rc = overlay_get_nodes_info(overlay_fdt, _full_path,
> +num_overlay_nodes);
> +if ( rc )
> +{
> +printk(XENLOG_ERR "Getting nodes information failed with error %d\n",
> +   rc);
> +goto err;
> +}
> +
> +nodes_irq = xmalloc_bytes(num_overlay_nodes * sizeof(int *));

You can use xzalloc_bytes and remove the memset below here and...

> +
> +if ( nodes_irq == NULL )
> +{
> +rc = -ENOMEM;
> +goto err;
> +}
> +memset(nodes_irq, 0x0, num_overlay_nodes * sizeof(int *));
> +
> +node_num_irq = xmalloc_bytes(num_overlay_nodes * sizeof(int));

Here

> +if ( node_num_irq == NULL )
> +{
> +rc = -ENOMEM;
> +goto err;
> +}
> +memset(node_num_irq, 0x0, num_overlay_nodes * sizeof(int));
> +
> +rc = fdt_overlay_apply(fdt, overlay_fdt);
> +if ( rc )
> +{
> +printk(XENLOG_ERR "Adding overlay node failed with error %d\n", rc);
> +goto err;
> +}
> +
> +   […]
> +err:
> +spin_unlock(_lock);
> +
> +xfree(dt_host_new);
> +xfree(fdt);
> +
> +if ( nodes_full_path != NULL )
> +{
> +for ( i = 0; i < num_overlay_nodes && nodes_full_path[i] != NULL; 
> i++ )
> +{
> +xfree(nodes_full_path[i]);
> +}
> +xfree(nodes_full_path);
> +}
> +
> +if ( nodes_irq != NULL )
> +{
> +for ( i = 0; i < num_overlay_nodes && nodes_irq[i] != NULL; i++ )
> +{
> +xfree(nodes_irq[i]);
> +}
> +xfree(nodes_irq);
> +}

I see you use this operation quite a bit in this module, perhaps you can create 
a function to
do that

Re: [PATCH v3 2/4] x86/APIC: calibrate against platform timer when possible

[Jan Beulich]

On 15.03.2022 10:12, Roger Pau Monné wrote:
> On Mon, Mar 14, 2022 at 05:19:37PM +0100, Jan Beulich wrote:
>> One thing seems quite clear though: Doing any of this with interrupts
>> enabled increases the chances for the read pairs to not properly
>> correlate, due to an interrupt happening in the middle. This alone is
>> a reason for me to want to keep IRQs off here.
> 
> Right, TSC calibration is also done with interrupts disabled, so it
> does seem correct to do the same here for APIC.
> 
> Maybe it would be cleaner to hide the specific PIT logic in
> calibrate_apic_timer() so that we could remove get_8254_timer_count()
> and wait_8254_wraparound() from apic.c and apic.c doesn't have any PIT
> specific code anymore?

Yes, that's certainly a further cleanup step to take (saying this
without actually having tried, so there may be obstacles).

Jan

> I think using channel 2 like it's used for the TSC calibration won't
> be possible at this point, since it will skew read_pit_count() users?
> In any case if we disable interrupts those will already be skewed
> because the timer won't be rearmed until interrupts are enabled.
> 
> Thanks, Roger.
> 

Re: [XEN][RFC PATCH v3 10/14] xen/arm: Implement device tree node removal functionalities

[Luca Fancellu]

> 
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index dc8d3a13f5..2eb5734f8e 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -54,6 +54,7 @@ obj-y += wait.o
> obj-bin-y += warning.init.o
> obj-$(CONFIG_XENOPROF) += xenoprof.o
> obj-y += xmalloc_tlsf.o
> +obj-$(CONFIG_OVERLAY_DTB) += dt_overlay.o

I think the entries are in alphabetical order, this should be added after += 
domain.o

> +/*
> + * overlay_get_nodes_info will get the all node's full name with path. This 
> is
> + * useful when checking node for duplication i.e. dtbo tries to add nodes 
> which
> + * already exists in device tree.
> + */
> +static int overlay_get_nodes_info(const void *fdto, char ***nodes_full_path,
> +  unsigned int num_overlay_nodes)
> +{
> +int fragment;
> +unsigned int node_num = 0;
> +
> +*nodes_full_path = xmalloc_bytes(num_overlay_nodes * sizeof(char *));

Here you can use xzalloc_bytes and...

> +
> +if ( *nodes_full_path == NULL )
> +return -ENOMEM;
> +memset(*nodes_full_path, 0x0, num_overlay_nodes * sizeof(char *));

Get rid of this memset

> +
> +/* Remove nodes from dt_host. */
> +static int remove_nodes(char **full_dt_node_path, int **nodes_irq,
> +int *node_num_irq, unsigned int num_nodes)
> +{
> +struct domain *d = hardware_domain;
> +int rc = 0;
> +struct dt_device_node *overlay_node;
> +unsigned int naddr;
> +unsigned int i, j, nirq;
> +u64 addr, size;
> +domid_t domid = 0;
> +
> +for ( j = 0; j < num_nodes; j++ )
> +{
> +dt_dprintk("Finding node %s in the dt_host\n", full_dt_node_path[j]);
> +
> +overlay_node = dt_find_node_by_path(full_dt_node_path[j]);
> +
> +if ( overlay_node == NULL )
> +{
> +printk(XENLOG_ERR "Device %s is not present in the tree. 
> Removing nodes failed\n",
> +   full_dt_node_path[j]);
> +return -EINVAL;
> +}
> +
> +domid = dt_device_used_by(overlay_node);
> +
> +dt_dprintk("Checking if node %s is used by any domain\n",
> +   full_dt_node_path[j]);
> +
> +/* Remove the node iff it's assigned to domain 0 or domain io. */
> +if ( domid != 0 && domid != DOMID_IO )
> +{
> +printk(XENLOG_ERR "Device %s as it is being used by domain %d. 
> Removing nodes failed\n",
> +   full_dt_node_path[j], domid);
> +return -EINVAL;
> +}
> +
> +dt_dprintk("Removing node: %s\n", full_dt_node_path[j]);
> +
> +nirq = node_num_irq[j];
> +
> +/* Remove IRQ permission */
> +for ( i = 0; i < nirq; i++ )
> +{
> +rc = nodes_irq[j][i];
> +/*
> + * TODO: We don't handle shared IRQs for now. So, it is assumed 
> that
> + * the IRQs was not shared with another domain.
> + */
> +rc = irq_deny_access(d, rc);
> +if ( rc )
> +{
> +printk(XENLOG_ERR "unable to revoke access for irq %u for 
> %s\n",
> +   i, dt_node_full_name(overlay_node));
> +return rc;
> +}
> +}
> +
> +rc = iommu_remove_dt_device(overlay_node);
> +if ( rc != 0 && rc != -ENXIO )
> +return rc;
> +
> +naddr = dt_number_of_address(overlay_node);
> +
> +/* Remove mmio access. */
> +for ( i = 0; i < naddr; i++ )
> +{
> +rc = dt_device_get_address(overlay_node, i, , );
> +if ( rc )
> +{
> +printk(XENLOG_ERR "Unable to retrieve address %u for %s\n",
> +   i, dt_node_full_name(overlay_node));
> +return rc;
> +}
> +
> +rc = iomem_deny_access(d, paddr_to_pfn(addr),
> +   paddr_to_pfn(PAGE_ALIGN(addr + size - 
> 1)));
> +if ( rc )
> +{
> +printk(XENLOG_ERR "Unable to remove dom%d access to"
> +" 0x%"PRIx64" - 0x%"PRIx64"\n",
> +d->domain_id,
> +addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);

NIT: here in each line under XENLOG_ERR, there is an extra space, these lines
Could be aligned to XENLOG_ERR, just for code style purpose.

> +return rc;
> +}
> +}
> +
> +rc = dt_overlay_remove_node(overlay_node);
> +if ( rc )
> +return rc;
> +}
> +
> +return rc;
> +}
> +
> 
> +long dt_sysctl(struct xen_sysctl *op)
> +{
> +long ret = 0;
> +void *overlay_fdt;
> +char **nodes_full_path = NULL;
> +unsigned int num_overlay_nodes = 0;
> +
> +if ( op->u.dt_overlay.overlay_fdt_size <= 0 )
> +return -EINVAL;
> +
> +overlay_fdt = xmalloc_bytes(op->u.dt_overlay.overlay_fdt_size);
> +
> +if ( overlay_fdt == NULL )
> +return 

Re: [PATCH 1/2] ns16550: reject IRQ above nr_irqs

[Roger Pau Monné]

On Fri, Mar 11, 2022 at 05:32:45PM +0100, Marek Marczykowski-Górecki wrote:
> On Fri, Mar 11, 2022 at 04:43:22PM +0100, Roger Pau Monné wrote:
> > Sorry, maybe this wasn't clear. My suggestion was not to just do this
> > fix and call it done, but rather to add this check for sanity and then
> > figure out how to properly handle this specific device.
> 
> Yes, I agree. Having it properly configured is preferred. Linux manages
> to do that, but I'm not sure how exactly. But ...

I think it might get the interrupt from ACPI data, which is likely out
of scope for Xen. Can you take a look at ACPI data from the box and
see whether the interrupt is reported there? (search for a _CRS method
belonging to the LPSS device)

Sadly the LPSS spec doesn't contain any help regarding the usage of
0xff in the Interrupt Line register. Out of curiosity, can you print
what's in the Interrupt Pin register? (PCI_INTERRUPT_PIN)

Thanks, Roger.

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

[osstest service owner]

flight 168601 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168601/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 168586
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 168586
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 168586
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 168586
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 168586
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 168586
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 168586
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 168586
 test-arm64-arm64-xl-seattle  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  16 saverestore-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-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   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-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  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-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   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-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  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-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-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-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:
 qemuu352998df1c53b366413690d95b35f76d0721ebed
baseline version:
 qemuu15df33ceb73cb6bb3c6736cf4d2cff51129ed4b4

Last test of basis   168586  2022-03-14 13:37:01 Z0 days
Testing same since   168601  2022-03-15 01:37:08 Z0 days1 attempts


People who touched revisions under test:
  Patrick Venture 
  Peter Maydell 
  Philippe 

Re: [PATCH v3 2/4] x86/APIC: calibrate against platform timer when possible

[Roger Pau Monné]

On Mon, Mar 14, 2022 at 05:19:37PM +0100, Jan Beulich wrote:
> On 11.03.2022 14:45, Roger Pau Monné wrote:
> > On Mon, Feb 14, 2022 at 10:25:11AM +0100, Jan Beulich wrote:
> >> Use the original calibration against PIT only when the platform timer
> >> is PIT. This implicitly excludes the "xen_guest" case from using the PIT
> >> logic (init_pit() fails there, and as of 5e73b2594c54 ["x86/time: minor
> >> adjustments to init_pit()"] using_pit also isn't being set too early
> >> anymore), so the respective hack there can be dropped at the same time.
> >> This also reduces calibration time from 100ms to 50ms, albeit this step
> >> is being skipped as of 0731a56c7c72 ("x86/APIC: no need for timer
> >> calibration when using TDT") anyway.
> >>
> >> While re-indenting the PIT logic in calibrate_APIC_clock(), besides
> >> adjusting style also switch around the 2nd TSC/TMCCT read pair, to match
> >> the order of the 1st one, yielding more consistent deltas.
> >>
> >> Signed-off-by: Jan Beulich 
> >> ---
> >> Open-coding apic_read() in read_tmcct() isn't overly nice, but I wanted
> >> to avoid x2apic_enabled being evaluated twice in close succession. (The
> >> barrier is there just in case only anyway: While this RDMSR isn't
> >> serializing, I'm unaware of any statement whether it can also be
> >> executed speculatively, like RDTSC can.) An option might be to move the
> >> function to apic.c such that it would also be used by
> >> calibrate_APIC_clock().
> > 
> > I think that would make sense. Or else it's kind of orthogonal that we
> > use a barrier in calibrate_apic_timer but not in calibrate_APIC_clock.
> 
> But there is a barrier there, via rdtsc_ordered(). Thinking about
> this again, I'm not not even sure I'd like to use the helper in
> calibrate_APIC_clock(), as there's no need to have two barriers
> there.
> 
> But I guess I'll move the function in any event, so it at least
> feels less like a layering violation. But I still would want to
> avoid calling apic_read(), i.e. the function would remain as is
> (albeit perhaps renamed as becoming non-static).
> 
> > But maybe we can get rid of the open-coded PIT calibration in
> > calibrate_APIC_clock? (see below)
> > 
> >> --- a/xen/arch/x86/time.c
> >> +++ b/xen/arch/x86/time.c
> >> @@ -26,6 +26,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -1004,6 +1005,78 @@ static u64 __init init_platform_timer(vo
> >>  return rc;
> >>  }
> >>  
> >> +static uint32_t __init read_tmcct(void)
> >> +{
> >> +if ( x2apic_enabled )
> >> +{
> >> +alternative("lfence", "mfence", X86_FEATURE_MFENCE_RDTSC);
> >> +return apic_rdmsr(APIC_TMCCT);
> >> +}
> >> +
> >> +return apic_mem_read(APIC_TMCCT);
> >> +}
> >> +
> >> +static uint64_t __init read_pt_and_tmcct(uint32_t *tmcct)
> >> +{
> >> +uint32_t tmcct_prev = *tmcct = read_tmcct(), tmcct_min = ~0;
> >> +uint64_t best = best;
> >> +unsigned int i;
> >> +
> >> +for ( i = 0; ; ++i )
> >> +{
> >> +uint64_t pt = plt_src.read_counter();
> >> +uint32_t tmcct_cur = read_tmcct();
> >> +uint32_t tmcct_delta = tmcct_prev - tmcct_cur;
> >> +
> >> +if ( tmcct_delta < tmcct_min )
> >> +{
> >> +tmcct_min = tmcct_delta;
> >> +*tmcct = tmcct_cur;
> >> +best = pt;
> >> +}
> >> +else if ( i > 2 )
> >> +break;
> >> +
> >> +tmcct_prev = tmcct_cur;
> >> +}
> >> +
> >> +return best;
> >> +}
> >> +
> >> +uint64_t __init calibrate_apic_timer(void)
> >> +{
> >> +uint32_t start, end;
> >> +uint64_t count = read_pt_and_tmcct(), elapsed;
> >> +uint64_t target = CALIBRATE_VALUE(plt_src.frequency), actual;
> >> +uint64_t mask = (uint64_t)~0 >> (64 - plt_src.counter_bits);
> >> +
> >> +/*
> >> + * PIT cannot be used here as it requires the timer interrupt to 
> >> maintain
> >> + * its 32-bit software counter, yet here we run with IRQs disabled.
> >> + */
> > 
> > The reasoning in calibrate_APIC_clock to have interrupts disabled
> > doesn't apply anymore I would think (interrupts are already enabled
> > when we get there),
> 
> setup_boot_APIC_clock() disables IRQs before calling
> calibrate_APIC_clock(). Whether the reasoning still applies is hard
> to tell - I at least cannot claim I fully understand the concern.

Me neither, I'm not sure what will explicitly need the first
interrupt, and why further interrupts won't be fine.

Also interrupts are already enabled before calling
calibrate_APIC_clock() (as it's setup_boot_APIC_clock() that disables
them), so this whole thing about getting the first interrupt seems
very bogus and plain wrong.

> > and hence it seems to me that calibrate_APIC_clock
> > could be called with interrupts enabled and we could remove the
> > open-coded usage of the PIT in calibrate_APIC_clock.
> 
> I won't exclude this might be possible, but it would mean 

Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time

[Juergen Gross]

On 15.03.22 09:40, Luca Fancellu wrote:



Hmm, this will fail the ASSERT(spin_is_locked(_lock)) in
__cpupool_find_by_id().

I think you need to use cpupool_get_by_id() and cpupool_put() by making them
globally visible (move their prototypes to sched.h).


Hi Juergen,

Yes I was thinking more to a __init wrapper that takes the lock and calls 
__cpupool_find_by_id,
this to avoid exporting cpupool_put outside since we would be the only user.
But I would like your opinion on that.


I'd be fine with that.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense

[Eric Blake]

On Mon, Mar 14, 2022 at 05:01:08PM +0100, Markus Armbruster wrote:
> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
> for two reasons.  One, it catches multiplication overflowing size_t.
> Two, it returns T * rather than void *, which lets the compiler catch
> more type errors.
> 
> This commit only touches allocations with size arguments of the form
> sizeof(T).
> 
> Patch created mechanically with:
> 
> $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
>--macro-file scripts/cocci-macro-file.h FILES...
> 
> Signed-off-by: Markus Armbruster 
> ---

I agree that this is mechanical, but...


>  qga/commands-win32.c |  8 ++---
>  qga/commands.c   |  2 +-
>  qom/qom-qmp-cmds.c   |  2 +-
>  replay/replay-char.c |  4 +--
>  replay/replay-events.c   | 10 +++---
>  scripts/coverity-scan/model.c|  2 +-

...are we sure we want to touch this particular file?

> diff --git a/scripts/coverity-scan/model.c b/scripts/coverity-scan/model.c
> index 9d4fba53d9..30bea672e1 100644
> --- a/scripts/coverity-scan/model.c
> +++ b/scripts/coverity-scan/model.c
> @@ -356,7 +356,7 @@ int g_poll (GPollFD *fds, unsigned nfds, int timeout)
>  typedef struct _GIOChannel GIOChannel;
>  GIOChannel *g_io_channel_unix_new(int fd)
>  {
> -GIOChannel *c = g_malloc0(sizeof(GIOChannel));
> +GIOChannel *c = g_new0(GIOChannel, 1);
>  __coverity_escape__(fd);
>  return c;
>  }

Our model has a definition of g_malloc0(), but I'm not sure whether
Coverity picks up the macro g_new0() in the same manner.

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

Re: [PATCH 2/2] ns16550: Add support for Intel LPSS UART

[Jan Beulich]

On 10.03.2022 15:34, Marek Marczykowski-Górecki wrote:
> This adds support for serial console as found in a laptop with TGL-LP
> (StarBook MkV). Since the device is on the bus 0, it needs to be enabled
> via "com1=...,amt", not just "...,pci".
> 
> Device specification is in Intel docs 631119-007 and 631120-001.
> 
> Signed-off-by: Marek Marczykowski-Górecki 

Reviewed-by: Jan Beulich 

> ---
> This adds only a single device (UART#2) to the table - the only one I
> have present, but the specification includes other device ids too. Should I
> add them too? I don't have a way to test that, though.

Personally I would have added the other ones as well, likely even going
further and including those from the other 500 Series variant as well,
and maybe yet further including e.g. 600 Series IDs too. But if you
want to restrict this to what you can test, that's certainly fine.

Jan

Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time

[Luca Fancellu]

> Hmm, this will fail the ASSERT(spin_is_locked(_lock)) in
> __cpupool_find_by_id().
> 
> I think you need to use cpupool_get_by_id() and cpupool_put() by making them
> globally visible (move their prototypes to sched.h).

Hi Juergen,

Yes I was thinking more to a __init wrapper that takes the lock and calls 
__cpupool_find_by_id,
this to avoid exporting cpupool_put outside since we would be the only user.
But I would like your opinion on that.

Cheers,
Luca

> 
> 
> Juergen
> 

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

[osstest service owner]

flight 168607 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168607/

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  07aebcd55fd2f7997e9fe50a6c849c8a12ec2e68
baseline version:
 xen  dedb0aa42c6d1ee31805dfc61630db2c41117330

Last test of basis   168580  2022-03-14 10:01:40 Z0 days
Failing since168589  2022-03-14 19:00:27 Z0 days3 attempts
Testing same since   168607  2022-03-15 03:02:22 Z0 days1 attempts


People who touched revisions under test:
  Ayan Kumar Halder 
  Ayan Kumar Halder 
  Julien Grall 

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
   dedb0aa42c..07aebcd55f  07aebcd55fd2f7997e9fe50a6c849c8a12ec2e68 -> smoke

Re: [PATCH 2/3] 9pfs: Use g_new() & friends where that makes obvious sense

[Greg Kurz]

On Mon, 14 Mar 2022 17:01:07 +0100
Markus Armbruster  wrote:

> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
> for two reasons.  One, it catches multiplication overflowing size_t.
> Two, it returns T * rather than void *, which lets the compiler catch
> more type errors.
> 
> This commit only touches allocations with size arguments of the form
> sizeof(T).
> 
> Patch created mechanically with:
> 
> $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
>--macro-file scripts/cocci-macro-file.h FILES...
> 
> Except this uncovers a typing error:
> 
> ../hw/9pfs/9p.c:855:13: warning: incompatible pointer types assigning to 
> 'QpfEntry *' from 'QppEntry *' [-Wincompatible-pointer-types]
>   val = g_new0(QppEntry, 1);
>   ^ ~~~
> 1 warning generated.
> 
> Harmless, because QppEntry is larger than QpfEntry.  Fix to allocate a
> QpfEntry instead.
> 
> Cc: Greg Kurz 
> Cc: Christian Schoenebeck 
> Signed-off-by: Markus Armbruster 
> ---

Reviewed-by: Greg Kurz 

>  hw/9pfs/9p-proxy.c   | 2 +-
>  hw/9pfs/9p-synth.c   | 4 ++--
>  hw/9pfs/9p.c | 8 
>  hw/9pfs/codir.c  | 6 +++---
>  tests/qtest/virtio-9p-test.c | 4 ++--
>  5 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
> index 8b4b5cf7dc..4c5e0fc217 100644
> --- a/hw/9pfs/9p-proxy.c
> +++ b/hw/9pfs/9p-proxy.c
> @@ -1187,7 +1187,7 @@ static int proxy_parse_opts(QemuOpts *opts, 
> FsDriverEntry *fs, Error **errp)
>  
>  static int proxy_init(FsContext *ctx, Error **errp)
>  {
> -V9fsProxy *proxy = g_malloc(sizeof(V9fsProxy));
> +V9fsProxy *proxy = g_new(V9fsProxy, 1);
>  int sock_id;
>  
>  if (ctx->export_flags & V9FS_PROXY_SOCK_NAME) {
> diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> index b3080e415b..d99d263985 100644
> --- a/hw/9pfs/9p-synth.c
> +++ b/hw/9pfs/9p-synth.c
> @@ -49,7 +49,7 @@ static V9fsSynthNode *v9fs_add_dir_node(V9fsSynthNode 
> *parent, int mode,
>  
>  /* Add directory type and remove write bits */
>  mode = ((mode & 0777) | S_IFDIR) & ~(S_IWUSR | S_IWGRP | S_IWOTH);
> -node = g_malloc0(sizeof(V9fsSynthNode));
> +node = g_new0(V9fsSynthNode, 1);
>  if (attr) {
>  /* We are adding .. or . entries */
>  node->attr = attr;
> @@ -128,7 +128,7 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int 
> mode,
>  }
>  /* Add file type and remove write bits */
>  mode = ((mode & 0777) | S_IFREG);
> -node = g_malloc0(sizeof(V9fsSynthNode));
> +node = g_new0(V9fsSynthNode, 1);
>  node->attr = >actual_attr;
>  node->attr->inode  = synth_node_count++;
>  node->attr->nlink  = 1;
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index a6d6b3f835..8e9d4aea73 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -324,7 +324,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
>  return NULL;
>  }
>  }
> -f = g_malloc0(sizeof(V9fsFidState));
> +f = g_new0(V9fsFidState, 1);
>  f->fid = fid;
>  f->fid_type = P9_FID_NONE;
>  f->ref = 1;
> @@ -804,7 +804,7 @@ static int qid_inode_prefix_hash_bits(V9fsPDU *pdu, dev_t 
> dev)
>  
>  val = qht_lookup(>s->qpd_table, , hash);
>  if (!val) {
> -val = g_malloc0(sizeof(QpdEntry));
> +val = g_new0(QpdEntry, 1);
>  *val = lookup;
>  affix = affixForIndex(pdu->s->qp_affix_next);
>  val->prefix_bits = affix.bits;
> @@ -852,7 +852,7 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct 
> stat *stbuf,
>  return -ENFILE;
>  }
>  
> -val = g_malloc0(sizeof(QppEntry));
> +val = g_new0(QpfEntry, 1);
>  *val = lookup;
>  
>  /* new unique inode and device combo */
> @@ -928,7 +928,7 @@ static int qid_path_suffixmap(V9fsPDU *pdu, const struct 
> stat *stbuf,
>  return -ENFILE;
>  }
>  
> -val = g_malloc0(sizeof(QppEntry));
> +val = g_new0(QppEntry, 1);
>  *val = lookup;
>  
>  /* new unique inode affix and device combo */
> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
> index 75148bc985..93ba44fb75 100644
> --- a/hw/9pfs/codir.c
> +++ b/hw/9pfs/codir.c
> @@ -141,9 +141,9 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState 
> *fidp,
>  
>  /* append next node to result chain */
>  if (!e) {
> -*entries = e = g_malloc0(sizeof(V9fsDirEnt));
> +*entries = e = g_new0(V9fsDirEnt, 1);
>  } else {
> -e = e->next = g_malloc0(sizeof(V9fsDirEnt));
> +e = e->next = g_new0(V9fsDirEnt, 1);
>  }
>  e->dent = qemu_dirent_dup(dent);
>  
> @@ -163,7 +163,7 @@ static int do_readdir_many(V9fsPDU *pdu, V9fsFidState 
> *fidp,
>  break;
>  }
>  
> -e->st = g_malloc0(sizeof(struct stat));
> +e->st = 

Re: Some feature requests for guest consoles

[Jan Beulich]

On 14.03.2022 17:36, Andy Smith wrote:
> - Ability to remap the "magic sysrq" key combination which is
>   ctrl-o, and possibly disable it while leaving "xl sysrq" and
>   /proc/sysrq-trigger in the guest generally working.
> 
>   Reason: guest administrators are often inexperienced with the
>   details of Xen. ctrl-o is a bad choice because it's actually the
>   "save buffer" shortcut in the popular editor nano. On more than
>   one occasion I have had guest administrators be editing a file
>   with nano on their console, they go to save it with ctrl-o which
>   appears to do nothing (because Xen is waiting for the sysrq
>   command that follows), so they do ctrl-o again which is taken as
>   being command 'o' - immediate power off! I have had an emergency
>   support ticket about this because "my guest randomly crashed while
>   I was editing a file".
> 
>   I would therefore like to remap "magic sysrq" to something more
>   obscure, or failing that disable it in guests as we/they will use
>   "xl sysrq" instead.

Without meaning to turn down the request, I'd like to point out that
- as of Linux commit 368c1e3249af (over 13 years ago) pressing ^O
  twice does not have the described effect, but actually means an
  individual ^O to be sent to the application,
- independent of that commit ^O followed by another ^O would not
  trigger the 'o' sysrq handler, but do nothing; said sysrq handler
  would be triggered when ^O is followed by O or o (without Ctrl),
- this sysrq triggering model isn't specific to Xen (and hence
  sending the request here may not reach the necessary audience) -
  besides being implemented in code common to all hvc drivers, it
  is additionally handled by some non-hvc tty drivers as well.

Jan

Re: [XEN v10 2/4] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler

[Jan Beulich]

On 10.03.2022 18:44, Ayan Kumar Halder wrote:
> When an instruction is trapped in Xen due to translation fault, Xen
> checks if the ISS is invalid (for data abort) or it is an instruction
> abort. If so, Xen tries to resolve the translation fault using p2m page
> tables. In case of data abort, Xen will try to map the mmio region to
> the guest (ie tries to emulate the mmio region).
> 
> If the ISS is not valid and it is a data abort, then Xen tries to
> decode the instruction. In case of ioreq, Xen  saves the decoding state,
> rn and imm9 to vcpu_io. Whenever the vcpu handles the ioreq successfully,
> it will read the decoding state to determine if the instruction decoded
> was a ldr/str post indexing (ie INSTR_LDR_STR_POSTINDEXING). If so, it
> uses these details to post increment rn.
> 
> In case of mmio handler, if the mmio operation was successful, then Xen
> retrives the decoding state, rn and imm9. For state ==
> INSTR_LDR_STR_POSTINDEXING, Xen will update rn.
> 
> If there is an error encountered while decoding/executing the instruction,
> Xen will forward the abort to the guest.
> 
> Also, the logic to infer the type of instruction has been moved from
> try_handle_mmio() to try_decode_instruction() which is called before.
> try_handle_mmio() is solely responsible for handling the mmio operation.
> 
> Signed-off-by: Ayan Kumar Halder 

In addition to the boot crash on 32-bit Arm there's also an early
build failure on x86:

In file included from arch/x86/x86_64/asm-offsets.c:11:
./include/xen/sched.h:164:26: error: field 'info' has incomplete type
 struct arch_vcpu_io  info;
  ^~~~

Note how on Arm you have ...

> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -281,6 +281,10 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
>  /* vPCI is not available on Arm */
>  #define has_vpci(d)({ (void)(d); false; })
>  
> +struct arch_vcpu_io {
> +struct instr_details dabt_instr; /* when the instruction is decoded */
> +};

... this in asm/domain.h, and thus visible to xen/sched.h, while on x86
you have ...

> --- a/xen/arch/x86/include/asm/ioreq.h
> +++ b/xen/arch/x86/include/asm/ioreq.h
> @@ -26,6 +26,9 @@
>  #include 
>  #endif
>  
> +struct arch_vcpu_io {
> +};

... this in a header which xen/sched.h doesn't end up including. I would
like to ask that you arrange locally for at least an x86 build test
whenever you touch code which might also affect x86.

Jan

Re: [PATCH 13/15] swiotlb: merge swiotlb-xen initialization into swiotlb

[Christoph Hellwig]

On Mon, Mar 14, 2022 at 07:07:44PM -0400, Boris Ostrovsky wrote:
>> +swiotlb_init_remap(true, x86_swiotlb_flags, xen_swiotlb_fixup);
>
>
> I think we need to have SWIOTLB_ANY set in x86_swiotlb_flags here.

Yes.

> Notice that we don't do remap() after final update to nslabs. We should.

Indeed.  I've pushed the fixed patches to the git tree and attached
the patches 12, 13 and 14 in case that is easier:
>From 6d72b98620281984ae778659cedeb369e82af8d8 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Mon, 14 Mar 2022 08:02:57 +0100
Subject: swiotlb: provide swiotlb_init variants that remap the buffer

To shared more code between swiotlb and xen-swiotlb, offer a
swiotlb_init_remap interface and add a remap callback to
swiotlb_init_late that will allow Xen to remap the buffer the
buffer without duplicating much of the logic.

Signed-off-by: Christoph Hellwig 
---
 arch/x86/pci/sta2x11-fixup.c |  2 +-
 include/linux/swiotlb.h  |  5 -
 kernel/dma/swiotlb.c | 36 +---
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
index c7e6faf59a861..7368afc039987 100644
--- a/arch/x86/pci/sta2x11-fixup.c
+++ b/arch/x86/pci/sta2x11-fixup.c
@@ -57,7 +57,7 @@ static void sta2x11_new_instance(struct pci_dev *pdev)
 		int size = STA2X11_SWIOTLB_SIZE;
 		/* First instance: register your own swiotlb area */
 		dev_info(>dev, "Using SWIOTLB (size %i)\n", size);
-		if (swiotlb_init_late(size, GFP_DMA))
+		if (swiotlb_init_late(size, GFP_DMA, NULL))
 			dev_emerg(>dev, "init swiotlb failed\n");
 	}
 	list_add(>list, _instance_list);
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ee655f2e4d28b..7b50c82f84ce9 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -36,8 +36,11 @@ struct scatterlist;
 
 int swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, unsigned int flags);
 unsigned long swiotlb_size_or_default(void);
+void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
+	int (*remap)(void *tlb, unsigned long nslabs));
+int swiotlb_init_late(size_t size, gfp_t gfp_mask,
+	int (*remap)(void *tlb, unsigned long nslabs));
 extern int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs);
-int swiotlb_init_late(size_t size, gfp_t gfp_mask);
 extern void __init swiotlb_update_mem_attributes(void);
 
 phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 79641c446d284..c37fd3d1c97f7 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -256,9 +256,11 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs,
  * Statically reserve bounce buffer space and initialize bounce buffer data
  * structures for the software IO TLB used to implement the DMA API.
  */
-void __init swiotlb_init(bool addressing_limit, unsigned int flags)
+void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
+		int (*remap)(void *tlb, unsigned long nslabs))
 {
-	size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
+	unsigned long nslabs = default_nslabs;
+	size_t bytes;
 	void *tlb;
 
 	if (!addressing_limit && !swiotlb_force_bounce)
@@ -271,12 +273,23 @@ void __init swiotlb_init(bool addressing_limit, unsigned int flags)
 	 * allow to pick a location everywhere for hypervisors with guest
 	 * memory encryption.
 	 */
+retry:
+	bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
 	if (flags & SWIOTLB_ANY)
 		tlb = memblock_alloc(bytes, PAGE_SIZE);
 	else
 		tlb = memblock_alloc_low(bytes, PAGE_SIZE);
 	if (!tlb)
 		goto fail;
+	if (remap && remap(tlb, nslabs) < 0) {
+		memblock_free(tlb, PAGE_ALIGN(bytes));
+
+		if (nslabs <= IO_TLB_MIN_SLABS)
+			panic("%s: Failed to remap %zu bytes\n",
+			  __func__, bytes);
+		nslabs = max(1024UL, ALIGN(nslabs >> 1, IO_TLB_SEGSIZE));
+		goto retry;
+	}
 	if (swiotlb_init_with_tbl(tlb, default_nslabs, flags))
 		goto fail_free_mem;
 	return;
@@ -287,12 +300,18 @@ void __init swiotlb_init(bool addressing_limit, unsigned int flags)
 	pr_warn("Cannot allocate buffer");
 }
 
+void __init swiotlb_init(bool addressing_limit, unsigned int flags)
+{
+	return swiotlb_init_remap(addressing_limit, flags, NULL);
+}
+
 /*
  * Systems with larger DMA zones (those that don't support ISA) can
  * initialize the swiotlb later using the slab allocator if needed.
  * This should be just like above, but with some error catching.
  */
-int swiotlb_init_late(size_t size, gfp_t gfp_mask)
+int swiotlb_init_late(size_t size, gfp_t gfp_mask,
+		int (*remap)(void *tlb, unsigned long nslabs))
 {
 	unsigned long nslabs = ALIGN(size >> IO_TLB_SHIFT, IO_TLB_SEGSIZE);
 	unsigned long bytes;
@@ -303,6 +322,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask)
 	if (swiotlb_force_disable)
 		return 0;
 
+retry:
 	order = get_order(nslabs << IO_TLB_SHIFT);
 	nslabs = SLABS_PER_PAGE << order;
 	bytes = nslabs << IO_TLB_SHIFT;
@@ -323,6 

Re: [PATCH 12/15] swiotlb: provide swiotlb_init variants that remap the buffer

[Christoph Hellwig]

On Mon, Mar 14, 2022 at 06:39:21PM -0400, Boris Ostrovsky wrote:
> This is IO_TLB_MIN_SLABS, isn't it? (Xen code didn't say so but that's what 
> it meant to say I believe)

Yes, that makes much more sense.  I've switched the patch to use
IO_TLB_MIN_SLABS and drop the 2MB comment in both places.

Can I get a review with that fixed up?

---
>From 153085bf3e6e69d676bef0fb96395a86fb8122f5 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig 
Date: Mon, 14 Mar 2022 08:02:57 +0100
Subject: swiotlb: provide swiotlb_init variants that remap the buffer

To shared more code between swiotlb and xen-swiotlb, offer a
swiotlb_init_remap interface and add a remap callback to
swiotlb_init_late that will allow Xen to remap the buffer the
buffer without duplicating much of the logic.

Signed-off-by: Christoph Hellwig 
---
 arch/x86/pci/sta2x11-fixup.c |  2 +-
 include/linux/swiotlb.h  |  5 -
 kernel/dma/swiotlb.c | 36 +---
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/arch/x86/pci/sta2x11-fixup.c b/arch/x86/pci/sta2x11-fixup.c
index c7e6faf59a861..7368afc039987 100644
--- a/arch/x86/pci/sta2x11-fixup.c
+++ b/arch/x86/pci/sta2x11-fixup.c
@@ -57,7 +57,7 @@ static void sta2x11_new_instance(struct pci_dev *pdev)
int size = STA2X11_SWIOTLB_SIZE;
/* First instance: register your own swiotlb area */
dev_info(>dev, "Using SWIOTLB (size %i)\n", size);
-   if (swiotlb_init_late(size, GFP_DMA))
+   if (swiotlb_init_late(size, GFP_DMA, NULL))
dev_emerg(>dev, "init swiotlb failed\n");
}
list_add(>list, _instance_list);
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index ee655f2e4d28b..7b50c82f84ce9 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -36,8 +36,11 @@ struct scatterlist;
 
 int swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, unsigned int flags);
 unsigned long swiotlb_size_or_default(void);
+void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
+   int (*remap)(void *tlb, unsigned long nslabs));
+int swiotlb_init_late(size_t size, gfp_t gfp_mask,
+   int (*remap)(void *tlb, unsigned long nslabs));
 extern int swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs);
-int swiotlb_init_late(size_t size, gfp_t gfp_mask);
 extern void __init swiotlb_update_mem_attributes(void);
 
 phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t phys,
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 79641c446d284..b3d4f24fb5f5e 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -256,9 +256,11 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long 
nslabs,
  * Statically reserve bounce buffer space and initialize bounce buffer data
  * structures for the software IO TLB used to implement the DMA API.
  */
-void __init swiotlb_init(bool addressing_limit, unsigned int flags)
+void __init swiotlb_init_remap(bool addressing_limit, unsigned int flags,
+   int (*remap)(void *tlb, unsigned long nslabs))
 {
-   size_t bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
+   unsigned long nslabs = default_nslabs;
+   size_t bytes;
void *tlb;
 
if (!addressing_limit && !swiotlb_force_bounce)
@@ -271,12 +273,23 @@ void __init swiotlb_init(bool addressing_limit, unsigned 
int flags)
 * allow to pick a location everywhere for hypervisors with guest
 * memory encryption.
 */
+retry:
+   bytes = PAGE_ALIGN(default_nslabs << IO_TLB_SHIFT);
if (flags & SWIOTLB_ANY)
tlb = memblock_alloc(bytes, PAGE_SIZE);
else
tlb = memblock_alloc_low(bytes, PAGE_SIZE);
if (!tlb)
goto fail;
+   if (remap && remap(tlb, nslabs) < 0) {
+   memblock_free(tlb, PAGE_ALIGN(bytes));
+
+   if (nslabs <= IO_TLB_MIN_SLABS)
+   panic("%s: Failed to remap %zu bytes\n",
+ __func__, bytes);
+   nslabs = max(1024UL, ALIGN(nslabs >> 1, IO_TLB_SEGSIZE));
+   goto retry;
+   }
if (swiotlb_init_with_tbl(tlb, default_nslabs, flags))
goto fail_free_mem;
return;
@@ -287,12 +300,18 @@ void __init swiotlb_init(bool addressing_limit, unsigned 
int flags)
pr_warn("Cannot allocate buffer");
 }
 
+void __init swiotlb_init(bool addressing_limit, unsigned int flags)
+{
+   return swiotlb_init_remap(addressing_limit, flags, NULL);
+}
+
 /*
  * Systems with larger DMA zones (those that don't support ISA) can
  * initialize the swiotlb later using the slab allocator if needed.
  * This should be just like above, but with some error catching.
  */
-int swiotlb_init_late(size_t size, gfp_t gfp_mask)
+int swiotlb_init_late(size_t size, gfp_t gfp_mask,
+   int (*remap)(void *tlb, unsigned long nslabs))
 {
unsigned 

[ovmf test] 168609: regressions - FAIL

[osstest service owner]

flight 168609 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/168609/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-xsm   6 xen-buildfail REGR. vs. 168254
 build-amd64   6 xen-buildfail REGR. vs. 168254
 build-i386-xsm6 xen-buildfail REGR. vs. 168254
 build-i3866 xen-buildfail REGR. vs. 168254

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a

version targeted for testing:
 ovmf a13dfc769bd7097d8d9ffe3e029a2c1d062d712b
baseline version:
 ovmf b1b89f9009f2390652e0061bd7b24fc40732bc70

Last test of basis   168254  2022-02-28 10:41:46 Z   14 days
Failing since168258  2022-03-01 01:55:31 Z   14 days  138 attempts
Testing same since   168579  2022-03-14 08:41:36 Z0 days9 attempts


People who touched revisions under test:
  Abner Chang 
  Bandaru, Purna Chandra Rao 
  Gerd Hoffmann 
  Guo Dong 
  Guomin Jiang 
  Hua Ma 
  Jason 
  Jason Lou 
  Ken Lautner 
  Kenneth Lautner 
  Kuo, Ted 
  Li, Zhihao 
  Lou, Yun 
  Ma, Hua 
  Matt DeVillier 
  Michael Kubacki 
  Patrick Rudolph 
  Purna Chandra Rao Bandaru 
  Sean Rhodes 
  Sebastien Boeuf 
  Ted Kuo 
  Wenyi Xie 
  wenyi,xie via groups.io 
  Xiaolu.Jiang 
  Zhihao Li 

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



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


Not pushing.

(No revision log; it would be 629 lines long.)

Re: [PATCH 1/2] xen/grant-table: remove gnttab_*transfer*() functions

[Juergen Gross]

On 15.03.22 00:37, Boris Ostrovsky wrote:


On 3/11/22 5:34 AM, Juergen Gross wrote:

All grant table operations related to the "transfer" functionality
are unused currently. There have been users in the old days of the
"Xen-o-Linux" kernel, but those didn't make it upstream.

So remove the "transfer" related functions.



Do we need to assert somewhere that transfer flags are not set?


This would be an orthogonal change, right? My patch is just removing
never called functions.

In any case I believe checking those flags is the job of the hypervisor.
If an operation is illegal due to a transfer flag being set, it needs to
be rejected at hypervisor level.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v2 4/6] xen/cpupool: Create different cpupools at boot time

[Juergen Gross]

On 12.03.22 01:10, Stefano Stabellini wrote:

On Fri, 11 Mar 2022, Luca Fancellu wrote:

On Thu, 10 Mar 2022, Luca Fancellu wrote:

Introduce a way to create different cpupools at boot time, this is
particularly useful on ARM big.LITTLE system where there might be the
need to have different cpupools for each type of core, but also
systems using NUMA can have different cpu pools for each node.

The feature on arm relies on a specification of the cpupools from the
device tree to build pools and assign cpus to them.

Documentation is created to explain the feature.

Signed-off-by: Luca Fancellu 
---
Changes in v2:
- Move feature to common code (Juergen)
- Try to decouple dtb parse and cpupool creation to allow
  more way to specify cpupools (for example command line)
- Created standalone dt node for the scheduler so it can
  be used in future work to set scheduler specific
  parameters
- Use only auto generated ids for cpupools
---
docs/misc/arm/device-tree/cpupools.txt | 156 ++
xen/common/Kconfig |   8 +
xen/common/Makefile|   1 +
xen/common/boot_cpupools.c | 212 +
xen/common/sched/cpupool.c |   6 +-
xen/include/xen/sched.h|  19 +++
6 files changed, 401 insertions(+), 1 deletion(-)
create mode 100644 docs/misc/arm/device-tree/cpupools.txt
create mode 100644 xen/common/boot_cpupools.c

diff --git a/docs/misc/arm/device-tree/cpupools.txt 
b/docs/misc/arm/device-tree/cpupools.txt
new file mode 100644
index ..d5a82ed0d45a
--- /dev/null
+++ b/docs/misc/arm/device-tree/cpupools.txt
@@ -0,0 +1,156 @@
+Boot time cpupools
+==
+
+When BOOT_TIME_CPUPOOLS is enabled in the Xen configuration, it is possible to
+create cpupools during boot phase by specifying them in the device tree.
+
+Cpupools specification nodes shall be direct childs of /chosen node.
+Each cpupool node contains the following properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,cpupool".
+
+- cpupool-cpus (mandatory)
+
+Must be a list of device tree phandle to nodes describing cpus (e.g. having
+device_type = "cpu"), it can't be empty.
+
+- cpupool-sched (optional)
+
+Must be a device tree phandle to a node having "xen,scheduler" compatible
+(description below), it has no effect when the cpupool refers to the 
cpupool
+number zero, in that case the default Xen scheduler is selected 
(sched=<...>
+boot argument).


This is *a lot* better.

The device tree part is nice. I have only one question left on it: why
do we need a separate scheduler node? Could the "cpupool-sched" property
be a simple string with the scheduler name?

E.g.:

cpupool_a {
compatible = "xen,cpupool";
cpupool-cpus = <_1 _2>;
};
cpupool_b {
compatible = "xen,cpupool";
cpupool-cpus = <_1 _2>;
cpupool-sched = "null";
};


To me, it doesn't look like these new "scheduler specification nodes"
bring any benefits. I would just get rid of them.


 From a comment of Juergen on the second patch I thought someone sees the need 
to
have a way to set scheduling parameters:

“you are allowing to use another scheduler,
but what if someone wants to set non-standard scheduling parameters
(e.g. another time slice)?”

So I thought I could introduce a scheduler specification node that could in the 
future be
extended and used to set scheduling parameter.

If it is something that is not needed, I will get rid of it.


I think you should get rid of it because it doesn't help. For instance,
if two cpupools want to use the same scheduler but with different
parameters we would end up with two different scheduler nodes.

Instead, in the future we could have one or more sched-params properties
as needed in the cpupools node to specify scheduler parameters.

  

+A scheduler specification node is a device tree node that contains the 
following
+properties:
+
+- compatible (mandatory)
+
+Must always include the compatiblity string: "xen,scheduler".
+
+- sched-name (mandatory)
+
+Must be a string having the name of a Xen scheduler, check the sched=<...>
+boot argument for allowed values.
+
+
+Constraints
+===
+
+If no cpupools are specified, all cpus will be assigned to one cpupool
+implicitly created (Pool-0).
+
+If cpupools node are specified, but not every cpu brought up by Xen is 
assigned,
+all the not assigned cpu will be assigned to an additional cpupool.
+
+If a cpu is assigned to a cpupool, but it's not brought up correctly, Xen will
+stop.
+
+
+Examples
+
+
+A system having two types of core, the following device tree specification will
+instruct Xen to have two cpupools:
+
+- The cpupool with id 0 will have 4 cpus assigned.
+- The cpupool with id 1 will have 2 cpus assigned.
+
+The following example can work only if hmp-unsafe=1 is passed to Xen boot
+arguments, otherwise not all cores will be brought up