[linux-linus test] 180427: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

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

version targeted for testing:
 linux0cfd8703e7da687924371e9bc77a025bdeba9637
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   10 days
Failing since180281  2023-04-17 06:24:36 Z9 days   17 attempts
Testing same since   180427  2023-04-26 08:52:34 Z0 days1 attempts


1122 people touched revisions under test,
not listing them all

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

[xen-4.15-testing test] 180426: tolerable FAIL - PUSHED

[osstest service owner]

flight 180426 xen-4.15-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180426/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  87cb0fd8757542893336aa2ffce3947451adf144
baseline version:
 xen  7963cdbf91d8a8d2f8338171adab3807b20f658a

Last test of basis   180218  2023-04-12 08:06:45 Z   14 days
Testing same since   180426  2023-04-26 07:39:28 Z0 days1 attempts


People who touched revisions under test:
  Michal Orzel 

jobs:
 build-amd64-xsm

Re: [PATCH v2] ns16550: enable memory decoding on MMIO-based PCI console card

[Marek Marczykowski-Górecki]

On Wed, Apr 26, 2023 at 10:24:28AM +0200, Jan Beulich wrote:
> On 26.04.2023 09:48, Roger Pau Monné wrote:
> > On Tue, Apr 25, 2023 at 04:39:02PM +0200, Marek Marczykowski-Górecki wrote:
> >> --- a/xen/drivers/char/ns16550.c
> >> +++ b/xen/drivers/char/ns16550.c
> >> @@ -272,7 +272,15 @@ static int cf_check ns16550_getc(struct serial_port 
> >> *port, char *pc)
> >>  static void pci_serial_early_init(struct ns16550 *uart)
> >>  {
> >>  #ifdef NS16550_PCI
> >> -if ( !uart->ps_bdf_enable || uart->io_base >= 0x1 )
> >> +if ( uart->bar )
> >> +{
> >> +pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
> >> +  uart->ps_bdf[2]),
> >> + PCI_COMMAND, PCI_COMMAND_MEMORY);
> > 
> > Don't you want to read the current command register first and just
> > or PCI_COMMAND_MEMORY?
> > 
> > I see that for IO decoding we already do it this way, but I'm not sure
> > whether it could cause issues down the road by unintentionally
> > disabling command flags.
> 
> Quite some time ago I asked myself the same question when seeing that
> code, but I concluded that perhaps none of the bits are really sensible
> to be set for a device as simple as a serial one. I'm actually thinking
> that for such a device to be used during early boot, it might even be
> helpful if bits like PARITY or SERR get cleared. (Of course if any of
> that was really the intention of the change introducing that code, it
> should have come with a code comment.)

I have mirrored the approach used for IO ports, with similar thinking,
and I read the above as you agree. Does it mean this patch is okay,
or you request some change here?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab


signature.asc
Description: PGP signature

[xen-4.14-testing test] 180425: tolerable trouble: fail/pass/starved - PUSHED

[osstest service owner]

flight 180425 xen-4.14-testing real [real]
flight 180437 xen-4.14-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180425/
http://logs.test-lab.xenproject.org/osstest/logs/180437/

Failures :-/ but no regressions.

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

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

version targeted for testing:
 xen  98ec8ad2eeb96eb9d4b7f9bfd1ef3a994c63af17
baseline version:
 xen  622675cdbc5f249bddfe970054f43a867d3ebed0

Last test of basis   180217  2023-04-12 08:06:29 Z   14 days
Testing same since   180425  2023-04-26 07:39:28 Z0 days1 attempts


People who touched revisions under test:
  Michal Orzel 

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

[ovmf test] 180436: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf edacc551e6586258ab046dd852f65d674e3e2af0
baseline version:
 ovmf ede0bd1496405f72147308b9570efba0234349b2

Last test of basis   180429  2023-04-26 09:41:52 Z0 days
Testing same since   180436  2023-04-26 17:42:39 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Jiewen Yao 
  Michael Roth 
  Roth, Michael via groups.io 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   ede0bd1496..edacc551e6  edacc551e6586258ab046dd852f65d674e3e2af0 -> 
xen-tested-master

Gitlab curiosity: Was [PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}()

[Andrew Cooper]

On 26/04/2023 3:59 pm, Alejandro Vallejo wrote:
> xc_domain_getinfo() returns the list of domains with domid >= first_domid.
> It does so by repeatedly invoking XEN_DOMCTL_getdomaininfo, which leads to
> unintuitive behaviour (asking for domid=1 might succeed returning domid=2).
> Furthermore, N hypercalls are required whereas the equivalent functionality
> can be achieved using with XEN_SYSCTL_getdomaininfo.
>
> Ideally, we want a DOMCTL interface that operates over a single precisely
> specified domain and a SYSCTL interface that can be used for bulk queries.
>
> All callers of xc_domain_getinfo() that are better off using SYSCTL are
> migrated to use that instead. That includes callers performing domain
> discovery and those requesting info for more than 1 domain per hypercall.
>
> A new xc_domain_getinfo_single() is introduced with stricter semantics than
> xc_domain_getinfo() (failing if domid isn't found) to migrate the rest to.
>
> With no callers left the xc_dominfo_t structure and the xc_domain_getinfo()
> call itself can be cleanly removed, and the DOMCTL interface simplified to
> only use its fastpath.
>
> With the DOMCTL ammended, the new xc_domain_getinfo_single() drops its
> stricter check, becoming a simple wrapper to invoke the hypercall itself.
>
> Alejandro Vallejo (7):
>   tools: Make some callers of xc_domain_getinfo use
> xc_domain_getinfolist
>   tools: Create xc_domain_getinfo_single()
>   tools: Refactor the console/io.c to avoid using xc_domain_getinfo()
>   tools: Make init-xenstore-domain use xc_domain_getinfolist()
>   tools: Modify single-domid callers of xc_domain_getinfolist
>   tools: Use new xc function for some xc_domain_getinfo calls
>   domctl: Modify getdomaininfo to fail if domid is not found

The patchew run found one single failure,

https://gitlab.com/xen-project/patchew/xen/-/jobs/4183881202

This part looks reasonably fatal:

 * Starting local ... *   Executing "/etc/local.d/xen.start" ...Starting
/usr/local/sbin/xenstored...
/etc/xen/scripts/launch-xenstore: line 90: echo: write error: Invalid
argument
Setting domain 0 name, domid and JSON config...
Done setting up Dom0
Starting xenconsoled...

except it was only the part trying to set the OOM score after starting
xenstored, and the only way that plausibly fails is if the pidfile was
bad.  And then the other print messages are clearly out of order.

I've rerun the pipeline a second time,
https://gitlab.com/xen-project/patchew/xen/-/pipelines/850230144, to see
if gitlab thinks it is a reliable or unreliable failure.


But, there's a plenty of other stuff in this log which is concerning. 
Stefano, Michal:

 * Starting networking ...awk: /etc/network/interfaces: No such file or
directory
 * ERROR: networking failed to start

The domains ought to have a interfaces file with "auto eth0", or even
nothing.  Alpine clearly isn't expecting the absence of the file at
all.  The fact the ping test passes usually means that this error isn't
as fatal as it makes out.

Next,

 * Executing: /lib/rc/sh/openrc-run.sh /lib/rc/sh/openrc-run.sh
/etc/init.d/modloop start
 * Mounting modloop  ... [ !! ]
 * ERROR: modloop failed to start

Not sure what modloop is, but this doesn't look healthy.

Next,

 * Loading modules ... *   Processing /etc/modules
modprobe: can't change directory to '/lib/modules': No such file or
directory

This probably just wants an empty dir in the filesystem.

I could go on, but I wont.  One thing that we do need however is
/var/log/xen/* pulled into the artefacts of the job, because if there
really is a real xenstored problem hiding in this series, there's no way
to debug it with the current artefacts collected.

~Andrew

Re: [PATCH] CI: Remove all use of /bin/false as a ROM

[Stefano Stabellini]

On Wed, 26 Apr 2023, Andrew Cooper wrote:
> As the recent work to get PCI Passthrough testing working shows, putting
> `/bin/false` as a ROM into guest context doesn't work so well.
> 
> For all ROM paths where we're skipping the build, use a slightly-plausible but
> likely non-existent path instead.
> 
> Signed-off-by: Andrew Cooper 

Assuming you (or patchew) tested it:

Acked-by: Stefano Stabellini 


> ---
> CC: Anthony PERARD 
> CC: Stefano Stabellini 
> CC: Michal Orzel 
> CC: Doug Goldstein 
> CC: Marek Marczykowski-Górecki 
> ---
>  automation/scripts/build | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/automation/scripts/build b/automation/scripts/build
> index d830cff7b7c7..197d085f3e07 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -67,9 +67,9 @@ else
>  
>  if [[ "${cc_is_clang}" == "y" ]]; then
>  # SeaBIOS cannot be built with clang
> -cfgargs+=("--with-system-seabios=/usr/share/seabios/bios.bin")
> +cfgargs+=("--with-system-seabios=/usr/share/no-seabios.bin")
>  # iPXE cannot be built with clang
> -cfgargs+=("--with-system-ipxe=/usr/lib/ipxe/ipxe.pxe")
> +cfgargs+=("--with-system-ipxe=/usr/share/no-ipxe.pxe")
>  # newlib cannot be built with clang so we cannot build stubdoms
>  cfgargs+=("--disable-stubdom")
>  fi
> @@ -87,7 +87,7 @@ else
>  
>  # SeaBIOS requires GCC 4.6 or later
>  if [[ "${cc_is_gcc}" == "y" && "${cc_ver}" -lt 0x040600 ]]; then
> -cfgargs+=("--with-system-seabios=/bin/false")
> +cfgargs+=("--with-system-seabios=/usr/share/no-seabios.bin")
>  fi
>  
>  ./configure "${cfgargs[@]}"
> -- 
> 2.30.2
> 

Re: [PATCH v3 2/2] acpi: Add TPM2 interface definition.

[Jason Andryuk]

On Tue, Apr 25, 2023 at 1:48 PM Jennifer Herbert
 wrote:
>
> This patch introduces an optional TPM 2 interface definition to the ACPI 
> table,
> which is to be used as part of a vTPM 2 implementation.
>
> Signed-off-by: Jennifer Herbert 
> ---
...
> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
> index f39a8e584f..51272530fe 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -1009,6 +1009,15 @@ void hvmloader_acpi_build_tables(struct acpi_config 
> *config,
>  config->table_flags |= ACPI_HAS_TPM;
>  config->tis_hdr = (uint16_t *)ACPI_TIS_HDR_ADDRESS;
>  break;
> +
> +case 2:
> +config->table_flags |= ACPI_HAS_TPM;
> +config->crb_id = (uint16_t *)TPM_CRB_INTF_ID;
> +
> +mem_hole_populate_ram(TPM_LOG_AREA_ADDRESS >> PAGE_SHIFT,
> +  TPM_LOG_SIZE >> PAGE_SHIFT);
> +memset((void *)TPM_LOG_AREA_ADDRESS, 0, TPM_LOG_SIZE);

TPM_LOG_AREA_ADDRESS is reserved in the e820 table since it is the
high memory range after the ACPI data, correct?

Reviewed-by: Jason Andryuk 

Thanks,
Jason

Re: [PATCH v3 1/2] acpi: Make TPM version configurable.

[Jason Andryuk]

Hi, Jennifer,

On Tue, Apr 25, 2023 at 1:48 PM Jennifer Herbert
 wrote:
>
> This patch makes the TPM version, for which the ACPI libary probes, 
> configurable.
> If acpi_config.tpm_verison is set to 1, it indicates that 1.2 (TCPA) should 
> be probed.
> I have also added to hvmloader an option to allow setting this new config, 
> which can
> be triggered by setting the platform/tpm_version xenstore key.
>
> Signed-off-by: Jennifer Herbert 
...
> --- a/tools/libacpi/build.c
> +++ b/tools/libacpi/build.c
> @@ -409,38 +409,47 @@ static int construct_secondary_tables(struct acpi_ctxt 
> *ctxt,
...
> +switch ( config->tpm_version )
>  {
> -tcpa->lasa = ctxt->mem_ops.v2p(ctxt, lasa);
> -tcpa->laml = ACPI_2_0_TCPA_LAML_SIZE;
> -memset(lasa, 0, tcpa->laml);
> -set_checksum(tcpa,
> - offsetof(struct acpi_header, checksum),
> - tcpa->header.length);
> +case 0: /* Assume legacy code wanted tpm 1.2 */

This shouldn't be reached, since tpm_version == 0 won't have
ACPI_HAS_TPM set.  Still, do you want to make it a break or drop the
case to avoid falling through to the TPM1.2 code?

Looks good though.

Reviewed-by: Jason Andryuk 

Thanks,
Jason

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

[osstest service owner]

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

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  dde20f7dc182fdfeeb6c55648979326bb982ca8c
baseline version:
 xen  18a36b4a9b088875486cfe33a2d4a8ae7eb4ab47

Last test of basis   180420  2023-04-25 23:02:14 Z0 days
Testing same since   180435  2023-04-26 17:01:58 Z0 days1 attempts


People who touched revisions under test:
  Jason Andryuk 
  Olaf Hering 

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
   18a36b4a9b..dde20f7dc1  dde20f7dc182fdfeeb6c55648979326bb982ca8c -> smoke

[libvirt test] 180424: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180403
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180403
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180403
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 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-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-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

version targeted for testing:
 libvirt  74b86146ef8a4ae484d74f104b465bfc8cc73512
baseline version:
 libvirt  c4bc4d3b82fbe22e03c986ca896090f481df5c10

Last test of basis   180403  2023-04-25 04:20:19 Z1 days
Testing same since   180424  2023-04-26 04:18:59 Z0 days1 attempts


People who touched revisions under test:
  Fedora Weblate Translation 
  Göran Uddeborg 
  Jiri Denemark 
  Ján Tomko 
  Martin Kletzander 
  Michal Privoznik 
  Pavel Borecki 
  Weblate 
  김인수 

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



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

Logs, config files, etc

[xen-4.17-testing test] 180422: tolerable FAIL - PUSHED

[osstest service owner]

flight 180422 xen-4.17-testing real [real]
flight 180434 xen-4.17-testing real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180422/
http://logs.test-lab.xenproject.org/osstest/logs/180434/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-raw 17 guest-start/debian.repeat fail pass in 
180434-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180406
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180406
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180406
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180406
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180406
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180406
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180406
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180406
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180406
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180406
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180406
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180406
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-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-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-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  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 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-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:
 xen  8b5be1fe938f52b5d3682dee7702fd51c8cfb61b
baseline version:
 xen  208dd44299193347d4ececdc1c8f864f6d9a0b9b

Last test of basis   180406  2023-04-25 

[ovmf test] 180429: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf ede0bd1496405f72147308b9570efba0234349b2
baseline version:
 ovmf 5a349b96b171e85744024904b0c8453d06d2fb45

Last test of basis   180423  2023-04-26 03:40:45 Z0 days
Testing same since   180429  2023-04-26 09:41:52 Z0 days1 attempts


People who touched revisions under test:
  Dun Tan 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   5a349b96b1..ede0bd1496  ede0bd1496405f72147308b9570efba0234349b2 -> 
xen-tested-master

[PATCH] xen/blkfront: Only check REQ_FUA for writes

[Ross Lagerwall]

The existing code silently converts read operations with the
REQ_FUA bit set into write-barrier operations. This results in data
loss as the backend scribbles zeroes over the data instead of returning
it.

While the REQ_FUA bit doesn't make sense on a read operation, at least
one well-known out-of-tree kernel module does set it and since it
results in data loss, let's be safe here and only look at REQ_FUA for
writes.

Signed-off-by: Ross Lagerwall 
---
 drivers/block/xen-blkfront.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 23ed258b57f0..c1890c8a9f6e 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -780,7 +780,8 @@ static int blkif_queue_rw_req(struct request *req, struct 
blkfront_ring_info *ri
ring_req->u.rw.handle = info->handle;
ring_req->operation = rq_data_dir(req) ?
BLKIF_OP_WRITE : BLKIF_OP_READ;
-   if (req_op(req) == REQ_OP_FLUSH || req->cmd_flags & REQ_FUA) {
+   if (req_op(req) == REQ_OP_FLUSH ||
+   (req_op(req) == REQ_OP_WRITE && (req->cmd_flags & 
REQ_FUA))) {
/*
 * Ideally we can do an unordered flush-to-disk.
 * In case the backend onlysupports barriers, use that.
-- 
2.31.1

[PATCH v9 4/8] hw: replace most qemu_bh_new calls with qemu_bh_new_guarded

[Alexander Bulekov]

This protects devices from bh->mmio reentrancy issues.

Thanks: Thomas Huth  for diagnosing OS X test failure.
Reviewed-by: Darren Kenny 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Paul Durrant 
Signed-off-by: Alexander Bulekov 
Reviewed-by: Thomas Huth 
---
 hw/9pfs/xen-9p-backend.c| 5 -
 hw/block/dataplane/virtio-blk.c | 3 ++-
 hw/block/dataplane/xen-block.c  | 5 +++--
 hw/char/virtio-serial-bus.c | 3 ++-
 hw/display/qxl.c| 9 ++---
 hw/display/virtio-gpu.c | 6 --
 hw/ide/ahci.c   | 3 ++-
 hw/ide/ahci_internal.h  | 1 +
 hw/ide/core.c   | 4 +++-
 hw/misc/imx_rngc.c  | 6 --
 hw/misc/macio/mac_dbdma.c   | 2 +-
 hw/net/virtio-net.c | 3 ++-
 hw/nvme/ctrl.c  | 6 --
 hw/scsi/mptsas.c| 3 ++-
 hw/scsi/scsi-bus.c  | 3 ++-
 hw/scsi/vmw_pvscsi.c| 3 ++-
 hw/usb/dev-uas.c| 3 ++-
 hw/usb/hcd-dwc2.c   | 3 ++-
 hw/usb/hcd-ehci.c   | 3 ++-
 hw/usb/hcd-uhci.c   | 2 +-
 hw/usb/host-libusb.c| 6 --
 hw/usb/redirect.c   | 6 --
 hw/usb/xen-usb.c| 3 ++-
 hw/virtio/virtio-balloon.c  | 5 +++--
 hw/virtio/virtio-crypto.c   | 3 ++-
 25 files changed, 66 insertions(+), 33 deletions(-)

diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
index 74f3a05f88..0e266c552b 100644
--- a/hw/9pfs/xen-9p-backend.c
+++ b/hw/9pfs/xen-9p-backend.c
@@ -61,6 +61,7 @@ typedef struct Xen9pfsDev {
 
 int num_rings;
 Xen9pfsRing *rings;
+MemReentrancyGuard mem_reentrancy_guard;
 } Xen9pfsDev;
 
 static void xen_9pfs_disconnect(struct XenLegacyDevice *xendev);
@@ -443,7 +444,9 @@ static int xen_9pfs_connect(struct XenLegacyDevice *xendev)
 xen_9pdev->rings[i].ring.out = xen_9pdev->rings[i].data +
XEN_FLEX_RING_SIZE(ring_order);
 
-xen_9pdev->rings[i].bh = qemu_bh_new(xen_9pfs_bh, 
&xen_9pdev->rings[i]);
+xen_9pdev->rings[i].bh = qemu_bh_new_guarded(xen_9pfs_bh,
+ &xen_9pdev->rings[i],
+ 
&xen_9pdev->mem_reentrancy_guard);
 xen_9pdev->rings[i].out_cons = 0;
 xen_9pdev->rings[i].out_size = 0;
 xen_9pdev->rings[i].inprogress = false;
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index b28d81737e..a6202997ee 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -127,7 +127,8 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, 
VirtIOBlkConf *conf,
 } else {
 s->ctx = qemu_get_aio_context();
 }
-s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
+s->bh = aio_bh_new_guarded(s->ctx, notify_guest_bh, s,
+   &DEVICE(vdev)->mem_reentrancy_guard);
 s->batch_notify_vqs = bitmap_new(conf->num_queues);
 
 *dataplane = s;
diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
index 734da42ea7..d8bc39d359 100644
--- a/hw/block/dataplane/xen-block.c
+++ b/hw/block/dataplane/xen-block.c
@@ -633,8 +633,9 @@ XenBlockDataPlane *xen_block_dataplane_create(XenDevice 
*xendev,
 } else {
 dataplane->ctx = qemu_get_aio_context();
 }
-dataplane->bh = aio_bh_new(dataplane->ctx, xen_block_dataplane_bh,
-   dataplane);
+dataplane->bh = aio_bh_new_guarded(dataplane->ctx, xen_block_dataplane_bh,
+   dataplane,
+   &DEVICE(xendev)->mem_reentrancy_guard);
 
 return dataplane;
 }
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 7d4601cb5d..dd619f0731 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -985,7 +985,8 @@ static void virtser_port_device_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
-port->bh = qemu_bh_new(flush_queued_data_bh, port);
+port->bh = qemu_bh_new_guarded(flush_queued_data_bh, port,
+   &dev->mem_reentrancy_guard);
 port->elem = NULL;
 }
 
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 80ce1e9a93..f1c0eb7dfc 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2201,11 +2201,14 @@ static void qxl_realize_common(PCIQXLDevice *qxl, Error 
**errp)
 
 qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
 
-qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
+qxl->update_irq = qemu_bh_new_guarded(qxl_update_irq_bh, qxl,
+  &DEVICE(qxl)->mem_reentrancy_guard);
 qxl_reset_state(qxl);
 
-qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
-qxl->ssd.cursor_bh = qemu_bh_new(qemu_spice_cursor_refresh_bh, &qxl->ssd);
+qxl->update_area_bh = qemu_bh_new_gu

Re: [PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist()

[Juergen Gross]

On 26.04.23 16:59, Alejandro Vallejo wrote:

It currently relies on xc_domain_getinfo() returning the next available
domain past "first_domid", which is a feature that will disappear in a
future patch.

Furthermore and while at it, make it so the hypercall tries to fetch information
about more than one domain per hypercall so we can (hopefully) get away with a
single hypercall in a typical system.

Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Juergen Gross 
---
  tools/helpers/init-xenstore-domain.c | 14 +-
  1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/helpers/init-xenstore-domain.c 
b/tools/helpers/init-xenstore-domain.c
index 0950ba7dc5..5f40901d31 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -21,6 +21,7 @@
  #define LAPIC_BASE_ADDRESS  0xfee0UL
  #define MB(x)   ((uint64_t)x << 20)
  #define GB(x)   ((uint64_t)x << 30)
+#define ARRAY_SIZE(x)   (sizeof(x) / sizeof((x)[0]))


Please include  instead of defining ARRAY_SIZE().

With that changed:

Reviewed-by: Juergen Gross 


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

[PATCH 6/7] tools: Use new xc function for some xc_domain_getinfo calls

[Alejandro Vallejo]

Move calls that require a information about a single precisely identified
domain to the new xc_domain_getinfo_single().

Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Tim Deegan 
Cc: George Dunlap 
Cc: Juergen Gross 
---
 tools/console/client/main.c |  7 +++
 tools/debugger/kdd/kdd-xen.c|  6 --
 tools/include/xenctrl.h |  7 +++
 tools/libs/ctrl/xc_domain.c |  5 ++---
 tools/libs/ctrl/xc_pagetab.c|  7 +++
 tools/libs/ctrl/xc_private.c|  7 +++
 tools/libs/ctrl/xc_private.h|  6 +++---
 tools/libs/guest/xg_core.c  | 21 +++
 tools/libs/guest/xg_core.h  |  6 +++---
 tools/libs/guest/xg_core_arm.c  | 10 -
 tools/libs/guest/xg_core_x86.c  | 18 
 tools/libs/guest/xg_cpuid_x86.c | 28 +
 tools/libs/guest/xg_dom_boot.c  | 12 ++-
 tools/libs/guest/xg_domain.c|  6 +++---
 tools/libs/guest/xg_offline_page.c  | 10 -
 tools/libs/guest/xg_private.h   |  1 +
 tools/libs/guest/xg_resume.c| 17 +++
 tools/libs/guest/xg_sr_common.h |  2 +-
 tools/libs/guest/xg_sr_restore.c| 14 +
 tools/libs/guest/xg_sr_restore_x86_pv.c |  2 +-
 tools/libs/guest/xg_sr_save.c   | 26 +--
 tools/libs/guest/xg_sr_save_x86_pv.c|  6 +++---
 tools/libs/light/libxl_sched.c  | 16 +++---
 tools/libs/light/libxl_x86_acpi.c   |  4 ++--
 tools/misc/xen-hvmcrash.c   |  6 +++---
 tools/misc/xen-lowmemd.c|  6 +++---
 tools/misc/xen-mfndump.c| 22 ---
 tools/misc/xen-vmtrace.c|  6 +++---
 tools/vchan/vchan-socket-proxy.c|  6 +++---
 tools/xenstore/xenstored_domain.c   | 15 +++--
 tools/xentrace/xenctx.c |  8 +++
 31 files changed, 146 insertions(+), 167 deletions(-)

diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index 1a6fa162f7..6775006488 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -408,17 +408,16 @@ int main(int argc, char **argv)
if (dom_path == NULL)
err(errno, "xs_get_domain_path()");
if (type == CONSOLE_INVAL) {
-   xc_dominfo_t xcinfo;
+   xc_domaininfo_t xcinfo;
xc_interface *xc_handle = xc_interface_open(0,0,0);
if (xc_handle == NULL)
err(errno, "Could not open xc interface");
-   if ( (xc_domain_getinfo(xc_handle, domid, 1, &xcinfo) != 1) ||
-(xcinfo.domid != domid) ) {
+   if (xc_domain_getinfo_single(xc_handle, domid, &xcinfo) < 0) {
xc_interface_close(xc_handle);
err(errno, "Failed to get domain information");
}
/* default to pv console for pv guests and serial for hvm 
guests */
-   if (xcinfo.hvm)
+   if (xcinfo.flags & XEN_DOMINF_hvm_guest)
type = CONSOLE_SERIAL;
else
type = CONSOLE_PV;
diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
index e78c9311c4..b1a96bf4e2 100644
--- a/tools/debugger/kdd/kdd-xen.c
+++ b/tools/debugger/kdd/kdd-xen.c
@@ -570,7 +570,7 @@ kdd_guest *kdd_guest_init(char *arg, FILE *log, int 
verbosity)
 kdd_guest *g = NULL;
 xc_interface *xch = NULL;
 uint32_t domid;
-xc_dominfo_t info;
+xc_domaininfo_t info;
 
 g = calloc(1, sizeof (kdd_guest));
 if (!g) 
@@ -590,7 +590,9 @@ kdd_guest *kdd_guest_init(char *arg, FILE *log, int 
verbosity)
 g->domid = domid;
 
 /* Check that the domain exists and is HVM */
-if (xc_domain_getinfo(xch, domid, 1, &info) != 1 || !info.hvm)
+if (xc_domain_getinfo_single(xch, domid, &info) < 0)
+goto err;
+if (!(info.flags & XEN_DOMINF_hvm_guest))
 goto err;
 
 snprintf(g->id, (sizeof g->id) - 1, 
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 73b07955c6..755759f0fe 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -473,6 +473,13 @@ static inline unsigned int dominfo_shutdown_reason(const 
xc_domaininfo_t *info)
 return (info->flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask;
 }
 
+static inline bool dominfo_shutdown_with(xc_domaininfo_t *info, unsigned int 
expected_reason)
+{
+/* The reason doesn't make sense unless the domain is actually shutdown */
+return (info->flags & XEN_DOMINF_shutdown) &&
+(dominfo_shutdown_reason(info) == expected_reason);
+}
+
 typedef union 
 {
 #if defined(__i386__) || defined(__x86_64__)
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 3ff91023bf..5c37dbe200 100644
--- a/

[PATCH 4/7] tools: Make init-xenstore-domain use xc_domain_getinfolist()

[Alejandro Vallejo]

It currently relies on xc_domain_getinfo() returning the next available
domain past "first_domid", which is a feature that will disappear in a
future patch.

Furthermore and while at it, make it so the hypercall tries to fetch information
about more than one domain per hypercall so we can (hopefully) get away with a
single hypercall in a typical system.

Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Juergen Gross 
---
 tools/helpers/init-xenstore-domain.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/helpers/init-xenstore-domain.c 
b/tools/helpers/init-xenstore-domain.c
index 0950ba7dc5..5f40901d31 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -21,6 +21,7 @@
 #define LAPIC_BASE_ADDRESS  0xfee0UL
 #define MB(x)   ((uint64_t)x << 20)
 #define GB(x)   ((uint64_t)x << 30)
+#define ARRAY_SIZE(x)   (sizeof(x) / sizeof((x)[0]))
 
 static uint32_t domid = ~0;
 static char *kernel;
@@ -322,16 +323,19 @@ err:
 
 static int check_domain(xc_interface *xch)
 {
-xc_dominfo_t info;
+xc_domaininfo_t info[8];
 uint32_t dom;
 int ret;
 
 dom = 1;
-while ( (ret = xc_domain_getinfo(xch, dom, 1, &info)) == 1 )
+while ( (ret = xc_domain_getinfolist(xch, dom, ARRAY_SIZE(info), info)) > 
0 )
 {
-if ( info.xenstore )
-return 1;
-dom = info.domid + 1;
+for ( size_t i = 0; i < ret; i++ )
+{
+if ( info[i].flags & XEN_DOMINF_xs_domain )
+return 1;
+}
+dom = info[ret - 1].domain + 1;
 }
 if ( ret < 0 && errno != ESRCH )
 {
-- 
2.34.1

[PATCH 7/7] domctl: Modify getdomaininfo to fail if domid is not found

[Alejandro Vallejo]

It previously mimicked the getdomaininfo sysctl semantics by returning
the first domid higher than the requested domid that does exist. This
unintuitive behaviour causes quite a few mistakes and makes the call
needlessly slow in its error path.

This patch removes the fallback search, returning -ESRCH if the requested
domain doesn't exist. Domain discovery can still be done through the sysctl
interface as that performs a linear search on the list of domains.

With this modification the xc_domain_getinfo() function is deprecated and
removed to make sure it's not mistakenly used expecting the old behaviour.
The new xc wrapper is xc_domain_getinfo_single().

All previous callers of xc_domain_getinfo() have been updated to use
xc_domain_getinfo_single() or xc_domain_getinfolist() instead. This also
means xc_dominfo_t is no longer used by anything and can be purged.

Resolves: xen-project/xen#105
Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Juergen Gross 
---
 tools/include/xenctrl.h | 43 ---
 tools/libs/ctrl/xc_domain.c | 70 -
 xen/common/domctl.c | 32 ++---
 3 files changed, 2 insertions(+), 143 deletions(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 755759f0fe..7ed22eff0d 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -444,28 +444,6 @@ typedef struct xc_core_header {
  * DOMAIN MANAGEMENT FUNCTIONS
  */
 
-typedef struct xc_dominfo {
-uint32_t  domid;
-uint32_t  ssidref;
-unsigned int  dying:1, crashed:1, shutdown:1,
-  paused:1, blocked:1, running:1,
-  hvm:1, debugged:1, xenstore:1, hap:1;
-unsigned int  shutdown_reason; /* only meaningful if shutdown==1 */
-unsigned long nr_pages; /* current number, not maximum */
-unsigned long nr_outstanding_pages;
-unsigned long nr_shared_pages;
-unsigned long nr_paged_pages;
-unsigned long shared_info_frame;
-uint64_t  cpu_time;
-unsigned long max_memkb;
-unsigned int  nr_online_vcpus;
-unsigned int  max_vcpu_id;
-xen_domain_handle_t handle;
-unsigned int  cpupool;
-uint8_t   gpaddr_bits;
-struct xen_arch_domainconfig arch_config;
-} xc_dominfo_t;
-
 typedef xen_domctl_getdomaininfo_t xc_domaininfo_t;
 
 static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info)
@@ -720,27 +698,6 @@ int xc_domain_getinfo_single(xc_interface *xch,
  uint32_t domid,
  xc_domaininfo_t *info);
 
-/**
- * This function will return information about one or more domains. It is
- * designed to iterate over the list of domains. If a single domain is
- * requested, this function will return the next domain in the list - if
- * one exists. It is, therefore, important in this case to make sure the
- * domain requested was the one returned.
- *
- * @parm xch a handle to an open hypervisor interface
- * @parm first_domid the first domain to enumerate information from.  Domains
- *   are currently enumerate in order of creation.
- * @parm max_doms the number of elements in info
- * @parm info an array of max_doms size that will contain the information for
- *the enumerated domains.
- * @return the number of domains enumerated or -1 on error
- */
-int xc_domain_getinfo(xc_interface *xch,
-  uint32_t first_domid,
-  unsigned int max_doms,
-  xc_dominfo_t *info);
-
-
 /**
  * This function will set the execution context for the specified vcpu.
  *
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 5c37dbe200..581e7529e5 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -358,82 +358,12 @@ int xc_domain_getinfo_single(xc_interface *xch,
 if (rc < 0)
 return rc;
 
-if (domctl.u.getdomaininfo.domain != domid)
-return -ESRCH;
-
 if (info)
 *info = domctl.u.getdomaininfo;
 
 return rc;
 }
 
-int xc_domain_getinfo(xc_interface *xch,
-  uint32_t first_domid,
-  unsigned int max_doms,
-  xc_dominfo_t *info)
-{
-unsigned int nr_doms;
-uint32_t next_domid = first_domid;
-DECLARE_DOMCTL;
-int rc = 0;
-
-memset(info, 0, max_doms*sizeof(xc_dominfo_t));
-
-for ( nr_doms = 0; nr_doms < max_doms; nr_doms++ )
-{
-domctl.cmd = XEN_DOMCTL_getdomaininfo;
-domctl.domain = next_domid;
-if ( (rc = do_domctl(xch, &domctl)) < 0 )
-break;
-info->domid  = domctl.domain;
-
-info->dying= !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_dying);
-info->shutdown = !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_shutdown);
-info->paused   = !!(domctl.u.getdomaininfo

[PATCH 0/7] Rationalize usage of xc_domain_getinfo{,list}()

[Alejandro Vallejo]

xc_domain_getinfo() returns the list of domains with domid >= first_domid.
It does so by repeatedly invoking XEN_DOMCTL_getdomaininfo, which leads to
unintuitive behaviour (asking for domid=1 might succeed returning domid=2).
Furthermore, N hypercalls are required whereas the equivalent functionality
can be achieved using with XEN_SYSCTL_getdomaininfo.

Ideally, we want a DOMCTL interface that operates over a single precisely
specified domain and a SYSCTL interface that can be used for bulk queries.

All callers of xc_domain_getinfo() that are better off using SYSCTL are
migrated to use that instead. That includes callers performing domain
discovery and those requesting info for more than 1 domain per hypercall.

A new xc_domain_getinfo_single() is introduced with stricter semantics than
xc_domain_getinfo() (failing if domid isn't found) to migrate the rest to.

With no callers left the xc_dominfo_t structure and the xc_domain_getinfo()
call itself can be cleanly removed, and the DOMCTL interface simplified to
only use its fastpath.

With the DOMCTL ammended, the new xc_domain_getinfo_single() drops its
stricter check, becoming a simple wrapper to invoke the hypercall itself.

Alejandro Vallejo (7):
  tools: Make some callers of xc_domain_getinfo use
xc_domain_getinfolist
  tools: Create xc_domain_getinfo_single()
  tools: Refactor the console/io.c to avoid using xc_domain_getinfo()
  tools: Make init-xenstore-domain use xc_domain_getinfolist()
  tools: Modify single-domid callers of xc_domain_getinfolist
  tools: Use new xc function for some xc_domain_getinfo calls
  domctl: Modify getdomaininfo to fail if domid is not found

 tools/console/client/main.c |  7 +--
 tools/console/daemon/io.c   | 31 +-
 tools/debugger/kdd/kdd-xen.c|  6 +-
 tools/helpers/init-xenstore-domain.c| 14 +++--
 tools/include/xenctrl.h | 63 
 tools/libs/ctrl/xc_domain.c | 79 +
 tools/libs/ctrl/xc_pagetab.c|  7 +--
 tools/libs/ctrl/xc_private.c|  7 +--
 tools/libs/ctrl/xc_private.h|  6 +-
 tools/libs/guest/xg_core.c  | 21 +++
 tools/libs/guest/xg_core.h  |  6 +-
 tools/libs/guest/xg_core_arm.c  | 10 ++--
 tools/libs/guest/xg_core_x86.c  | 18 +++---
 tools/libs/guest/xg_cpuid_x86.c | 28 +
 tools/libs/guest/xg_dom_boot.c  | 12 +---
 tools/libs/guest/xg_domain.c|  6 +-
 tools/libs/guest/xg_offline_page.c  | 10 ++--
 tools/libs/guest/xg_private.h   |  1 +
 tools/libs/guest/xg_resume.c| 17 +++---
 tools/libs/guest/xg_sr_common.h |  2 +-
 tools/libs/guest/xg_sr_restore.c| 14 ++---
 tools/libs/guest/xg_sr_restore_x86_pv.c |  2 +-
 tools/libs/guest/xg_sr_save.c   | 26 
 tools/libs/guest/xg_sr_save_x86_pv.c|  6 +-
 tools/libs/light/libxl_dom.c| 15 ++---
 tools/libs/light/libxl_dom_suspend.c|  7 +--
 tools/libs/light/libxl_domain.c | 12 ++--
 tools/libs/light/libxl_mem.c|  4 +-
 tools/libs/light/libxl_sched.c  | 28 -
 tools/libs/light/libxl_x86_acpi.c   |  4 +-
 tools/misc/xen-hvmcrash.c   |  6 +-
 tools/misc/xen-lowmemd.c|  6 +-
 tools/misc/xen-mfndump.c| 22 +++
 tools/misc/xen-vmtrace.c|  6 +-
 tools/python/xen/lowlevel/xc/xc.c   | 29 -
 tools/vchan/vchan-socket-proxy.c|  6 +-
 tools/xenmon/xenbaked.c |  6 +-
 tools/xenpaging/xenpaging.c | 14 ++---
 tools/xenstore/xenstored_domain.c   | 15 +++--
 tools/xentrace/xenctx.c |  8 +--
 xen/common/domctl.c | 32 +-
 41 files changed, 245 insertions(+), 374 deletions(-)

-- 
2.34.1

[PATCH 1/7] tools: Make some callers of xc_domain_getinfo use xc_domain_getinfolist

[Alejandro Vallejo]

xc_domain_getinfo() is slow and prone to races because N hypercalls are
needed to find information about N domains. xc_domain_getinfolist() finds
the same information in a single hypercall as long as a big enough buffer
is provided. Plus, xc_domain_getinfo() is disappearing on a future patch
so migrate the callers interested in more than 1 domain to the the *list()
version.

Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Juergen Gross 
---
 tools/include/xenctrl.h   |  5 +
 tools/python/xen/lowlevel/xc/xc.c | 29 +++--
 tools/xenmon/xenbaked.c   |  6 +++---
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 05967ecc92..90b33aa3a7 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -468,6 +468,11 @@ typedef struct xc_dominfo {
 
 typedef xen_domctl_getdomaininfo_t xc_domaininfo_t;
 
+static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info)
+{
+return (info->flags >> XEN_DOMINF_shutdownshift) & XEN_DOMINF_shutdownmask;
+}
+
 typedef union 
 {
 #if defined(__i386__) || defined(__x86_64__)
diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index 35901c2d63..38212e8091 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -342,7 +342,7 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
 uint32_t first_dom = 0;
 int max_doms = 1024, nr_doms, i;
 size_t j;
-xc_dominfo_t *info;
+xc_domaininfo_t *info;
 
 static char *kwd_list[] = { "first_dom", "max_doms", NULL };
 
@@ -350,11 +350,11 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
   &first_dom, &max_doms) )
 return NULL;
 
-info = calloc(max_doms, sizeof(xc_dominfo_t));
+info = calloc(max_doms, sizeof(*info));
 if (info == NULL)
 return PyErr_NoMemory();
 
-nr_doms = xc_domain_getinfo(self->xc_handle, first_dom, max_doms, info);
+nr_doms = xc_domain_getinfolist(self->xc_handle, first_dom, max_doms, 
info);
 
 if (nr_doms < 0)
 {
@@ -368,21 +368,22 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
 info_dict = Py_BuildValue(
 "{s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i,s:i"
 ",s:L,s:L,s:L,s:i,s:i,s:i}",
-"domid",   (int)info[i].domid,
+"domid",   (int)info[i].domain,
 "online_vcpus",info[i].nr_online_vcpus,
 "max_vcpu_id", info[i].max_vcpu_id,
-"hvm", info[i].hvm,
-"dying",   info[i].dying,
-"crashed", info[i].crashed,
-"shutdown",info[i].shutdown,
-"paused",  info[i].paused,
-"blocked", info[i].blocked,
-"running", info[i].running,
-"mem_kb",  (long long)info[i].nr_pages*(XC_PAGE_SIZE/1024),
+"hvm", !!(info[i].flags & XEN_DOMINF_hvm_guest),
+"dying",   !!(info[i].flags & XEN_DOMINF_dying),
+"crashed", (info[i].flags & XEN_DOMINF_shutdown) &&
+ (dominfo_shutdown_reason(&info[i]) == 
SHUTDOWN_crash),
+"shutdown",!!(info[i].flags & XEN_DOMINF_shutdown),
+"paused",  !!(info[i].flags & XEN_DOMINF_paused),
+"blocked", !!(info[i].flags & XEN_DOMINF_blocked),
+"running", !!(info[i].flags & XEN_DOMINF_running),
+"mem_kb",  (long 
long)info[i].tot_pages*(XC_PAGE_SIZE/1024),
 "cpu_time",(long long)info[i].cpu_time,
-"maxmem_kb",   (long long)info[i].max_memkb,
+"maxmem_kb",   (long long)(info[i].max_pages << (XC_PAGE_SHIFT 
- 10)),
 "ssidref", (int)info[i].ssidref,
-"shutdown_reason", info[i].shutdown_reason,
+"shutdown_reason", dominfo_shutdown_reason(&info[i]),
 "cpupool", (int)info[i].cpupool);
 pyhandle = PyList_New(sizeof(xen_domain_handle_t));
 if ( (pyhandle == NULL) || (info_dict == NULL) )
diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c
index 4dddbd20e2..8632b10ea4 100644
--- a/tools/xenmon/xenbaked.c
+++ b/tools/xenmon/xenbaked.c
@@ -775,7 +775,7 @@ static void global_init_domain(int domid, int idx)
 static int indexof(int domid)
 {
 int idx;
-xc_dominfo_t dominfo[NDOMAINS];
+xc_domaininfo_t dominfo[NDOMAINS];
 xc_interface *xc_handle;
 int ndomains;
   
@@ -797,7 +797,7 @@ static int indexof(int domid)
 
 // call domaininfo hypercall to try and garbage collect unused entries
 xc_handle = xc_interface_open(0,0,0);
-ndomains = xc_domain_getinfo(xc_handle, 0, NDOMAINS, dominfo);
+ndomains = xc_domain_getinfolist(xc_handle, 0, NDOMAINS, dominfo);

[PATCH 3/7] tools: Refactor the console/io.c to avoid using xc_domain_getinfo()

[Alejandro Vallejo]

It has 2 avoidable occurences

* Check whether a domain is valid, which can be done faster with
xc_domain_getinfo_single()
* Domain discovery, which can be done much faster with the sysctl
interface through xc_domain_getinfolist().

Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Anthony PERARD 
---
 tools/console/daemon/io.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 6bfe96715b..1fc56f6643 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -405,13 +405,7 @@ static void buffer_advance(struct buffer *buffer, size_t 
len)
 
 static bool domain_is_valid(int domid)
 {
-   bool ret;
-   xc_dominfo_t info;
-
-   ret = (xc_domain_getinfo(xc, domid, 1, &info) == 1 &&
-  info.domid == domid);
-   
-   return ret;
+   return xc_domain_getinfo_single(xc, domid, NULL) == 0;
 }
 
 static int create_hv_log(void)
@@ -959,26 +953,35 @@ static void shutdown_domain(struct domain *d)
 
 static unsigned enum_pass = 0;
 
+/**
+ * Memory set aside to query the state of every
+ * domain in the hypervisor in a single hypercall.
+ */
+static xc_domaininfo_t domaininfo[DOMID_FIRST_RESERVED - 1];
+
 static void enum_domains(void)
 {
-   int domid = 1;
-   xc_dominfo_t dominfo;
+   int ret;
struct domain *dom;
 
enum_pass++;
 
-   while (xc_domain_getinfo(xc, domid, 1, &dominfo) == 1) {
-   dom = lookup_domain(dominfo.domid);
-   if (dominfo.dying) {
+   /* Fetch info on every valid domain except for dom0 */
+   ret = xc_domain_getinfolist(xc, 1, DOMID_FIRST_RESERVED - 1, 
domaininfo);
+   if (ret < 0)
+   return;
+
+   for (size_t i = 0; i < ret; i++) {
+   dom = lookup_domain(domaininfo[i].domain);
+   if (domaininfo[i].flags & XEN_DOMINF_dying) {
if (dom)
shutdown_domain(dom);
} else {
if (dom == NULL)
-   dom = create_domain(dominfo.domid);
+   dom = create_domain(domaininfo[i].domain);
}
if (dom)
dom->last_seen = enum_pass;
-   domid = dominfo.domid + 1;
}
 }
 
-- 
2.34.1

[PATCH 2/7] tools: Create xc_domain_getinfo_single()

[Alejandro Vallejo]

It's a stricter version of xc_domain_getinfo() where the returned domid
always matches the requested domid or the error code shows an error instead.
A few patches ahead usages of xc_domain_getinfo() are removed until only
xc_domain_getinfo_single() and xc_domain_getinfolist() remain.

Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Juergen Gross 
---
 tools/include/xenctrl.h | 16 
 tools/libs/ctrl/xc_domain.c | 22 ++
 2 files changed, 38 insertions(+)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 90b33aa3a7..73b07955c6 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -696,6 +696,22 @@ int xc_vcpu_getaffinity(xc_interface *xch,
 int xc_domain_get_guest_width(xc_interface *xch, uint32_t domid,
   unsigned int *guest_width);
 
+/**
+ * This function will return information about a single domain. It looks
+ * up the domain by the provided domid and succeeds if the domain exists
+ * and is accesible by the current domain, or fails otherwise. A buffer
+ * may optionally passed on the `info` parameter in order to retrieve
+ * information about the domain. The buffer is ignored if NULL is
+ * passed instead.
+ *
+ * @parm xch a handle to an open hypervisor interface
+ * @parm domid domid to lookup
+ * @parm info Optional domain information buffer (may be NULL)
+ * @return 0 on success, otherwise the call failed and info is undefined
+ */
+int xc_domain_getinfo_single(xc_interface *xch,
+ uint32_t domid,
+ xc_domaininfo_t *info);
 
 /**
  * This function will return information about one or more domains. It is
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index e939d07157..3ff91023bf 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -345,6 +345,28 @@ int xc_dom_vuart_init(xc_interface *xch,
 return rc;
 }
 
+int xc_domain_getinfo_single(xc_interface *xch,
+ uint32_t domid,
+ xc_domaininfo_t *info)
+{
+struct xen_domctl domctl = {
+.cmd = XEN_DOMCTL_getdomaininfo,
+.domain = domid,
+};
+
+int rc = do_domctl(xch, &domctl);
+if (rc < 0)
+return rc;
+
+if (domctl.u.getdomaininfo.domain != domid)
+return -ESRCH;
+
+if (info)
+*info = domctl.u.getdomaininfo;
+
+return rc;
+}
+
 int xc_domain_getinfo(xc_interface *xch,
   uint32_t first_domid,
   unsigned int max_doms,
-- 
2.34.1

[PATCH 5/7] tools: Modify single-domid callers of xc_domain_getinfolist

[Alejandro Vallejo]

xc_domain_getinfolist() internally relies on a sysctl that performs
a linear search for the domids. Many callers of xc_domain_getinfolist()
who require information about a precise domid are much better off calling
xc_domain_getinfo_single() instead, that will use the getdomaininfo domctl
instead and ensure the returned domid matches the requested one. The domtctl
will find the domid faster too, because that uses hashed lists.

Signed-off-by: Alejandro Vallejo 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Juergen Gross 
---
 tools/libs/light/libxl_dom.c | 15 +--
 tools/libs/light/libxl_dom_suspend.c |  7 +--
 tools/libs/light/libxl_domain.c  | 12 
 tools/libs/light/libxl_mem.c |  4 ++--
 tools/libs/light/libxl_sched.c   | 12 
 tools/xenpaging/xenpaging.c  | 14 +++---
 6 files changed, 23 insertions(+), 41 deletions(-)

diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index 25fb716084..482e04b38c 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -32,8 +32,8 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t 
domid)
 xc_domaininfo_t info;
 int ret;
 
-ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
-if (ret != 1 || info.domain != domid) {
+ret = xc_domain_getinfo_single(ctx->xch, domid, &info);
+if (ret < 0) {
 LOG(ERROR, "unable to get domain type for domid=%"PRIu32, domid);
 return LIBXL_DOMAIN_TYPE_INVALID;
 }
@@ -70,15 +70,10 @@ int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
 xc_domaininfo_t info;
 int ret;
 
-ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
-if (ret != 1)
+ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
+if (ret < 0)
 {
-LOGE(ERROR, "getinfolist failed %d", ret);
-return ERROR_FAIL;
-}
-if (info.domain != domid)
-{
-LOGE(ERROR, "got info for dom%d, wanted dom%d\n", info.domain, domid);
+LOGE(ERROR, "getinfo_single failed %d", ret);
 return ERROR_FAIL;
 }
 return info.cpupool;
diff --git a/tools/libs/light/libxl_dom_suspend.c 
b/tools/libs/light/libxl_dom_suspend.c
index 4fa22bb739..6091a5f3f6 100644
--- a/tools/libs/light/libxl_dom_suspend.c
+++ b/tools/libs/light/libxl_dom_suspend.c
@@ -332,13 +332,8 @@ static void suspend_common_wait_guest_check(libxl__egc 
*egc,
 /* Convenience aliases */
 const uint32_t domid = dsps->domid;
 
-ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
+ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
 if (ret < 0) {
-LOGED(ERROR, domid, "unable to check for status of guest");
-goto err;
-}
-
-if (!(ret == 1 && info.domain == domid)) {
 LOGED(ERROR, domid, "guest we were suspending has been destroyed");
 goto err;
 }
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 7f0986c185..33ac8e9ce8 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -349,16 +349,12 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo 
*info_r,
 int ret;
 GC_INIT(ctx);
 
-ret = xc_domain_getinfolist(ctx->xch, domid, 1, &xcinfo);
+ret = xc_domain_getinfo_single(ctx->xch, domid, &xcinfo);
 if (ret<0) {
-LOGED(ERROR, domid, "Getting domain info list");
+LOGED(ERROR, domid, "Getting domain info single");
 GC_FREE;
 return ERROR_FAIL;
 }
-if (ret==0 || xcinfo.domain != domid) {
-GC_FREE;
-return ERROR_DOMAIN_NOTFOUND;
-}
 
 if (info_r)
 libxl__xcinfo2xlinfo(ctx, &xcinfo, info_r);
@@ -1663,8 +1659,8 @@ libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t 
domid,
 xc_vcpuinfo_t vcpuinfo;
 unsigned int nr_vcpus;
 
-if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
-LOGED(ERROR, domid, "Getting infolist");
+if (xc_domain_getinfo_single(ctx->xch, domid, &domaininfo) < 0) {
+LOGED(ERROR, domid, "Getting info single");
 GC_FREE;
 return NULL;
 }
diff --git a/tools/libs/light/libxl_mem.c b/tools/libs/light/libxl_mem.c
index 92ec09f4cf..44e554adba 100644
--- a/tools/libs/light/libxl_mem.c
+++ b/tools/libs/light/libxl_mem.c
@@ -323,8 +323,8 @@ retry_transaction:
 libxl__xs_printf(gc, t, GCSPRINTF("%s/memory/target", dompath),
  "%"PRIu64, new_target_memkb);
 
-r = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
-if (r != 1 || info.domain != domid) {
+r = xc_domain_getinfo_single(ctx->xch, domid, &info);
+if (r < 0) {
 abort_transaction = 1;
 rc = ERROR_FAIL;
 goto out;
diff --git a/tools/libs/light/libxl_sched.c b/tools/libs/light/libxl_sched.c
index 7c53dc60e6..19da7c49ea 100644
--- a/tools/libs/light/libxl_sched.c
+++ b/tools/libs/light/libxl_sched.c
@@ -219,13 +219,11 @@ static int sched_credit_domain_set(l

[PATCH] x86/msi: dynamically map pages for MSI-X tables

[Ruben Hakobyan]

Xen reserves a constant number of pages that can be used for mapping
MSI-X tables. This limit is defined by FIX_MSIX_MAX_PAGES in fixmap.h.

Reserving a fixed number of pages could result in an -ENOMEM if a
device requests a new page when the fixmap limit is exhausted and will
necessitate manually adjusting the limit before compilation.

To avoid the issues with the current fixmap implementation, we modify
the MSI-X page mapping logic to instead dynamically map new pages when
they are needed by making use of ioremap().

Signed-off-by: Ruben Hakobyan 
---
 xen/arch/x86/include/asm/fixmap.h |  2 -
 xen/arch/x86/include/asm/msi.h|  5 +--
 xen/arch/x86/msi.c| 69 ---
 3 files changed, 19 insertions(+), 57 deletions(-)

diff --git a/xen/arch/x86/include/asm/fixmap.h 
b/xen/arch/x86/include/asm/fixmap.h
index 516ec3fa6c..139c3e2dcc 100644
--- a/xen/arch/x86/include/asm/fixmap.h
+++ b/xen/arch/x86/include/asm/fixmap.h
@@ -61,8 +61,6 @@ enum fixed_addresses {
 FIX_ACPI_END = FIX_ACPI_BEGIN + NUM_FIXMAP_ACPI_PAGES - 1,
 FIX_HPET_BASE,
 FIX_TBOOT_SHARED_BASE,
-FIX_MSIX_IO_RESERV_BASE,
-FIX_MSIX_IO_RESERV_END = FIX_MSIX_IO_RESERV_BASE + FIX_MSIX_MAX_PAGES -1,
 FIX_TBOOT_MAP_ADDRESS,
 FIX_APEI_RANGE_BASE,
 FIX_APEI_RANGE_END = FIX_APEI_RANGE_BASE + FIX_APEI_RANGE_MAX -1,
diff --git a/xen/arch/x86/include/asm/msi.h b/xen/arch/x86/include/asm/msi.h
index a53ade95c9..16c80c9883 100644
--- a/xen/arch/x86/include/asm/msi.h
+++ b/xen/arch/x86/include/asm/msi.h
@@ -55,9 +55,6 @@
 #define MSI_ADDR_DEST_ID_MASK  0x00ff000
 #define  MSI_ADDR_DEST_ID(dest)(((dest) << 
MSI_ADDR_DEST_ID_SHIFT) & MSI_ADDR_DEST_ID_MASK)
 
-/* MAX fixed pages reserved for mapping MSIX tables. */
-#define FIX_MSIX_MAX_PAGES  512
-
 struct msi_info {
 pci_sbdf_t sbdf;
 int irq;
@@ -213,7 +210,7 @@ struct arch_msix {
 unsigned long first, last;
 } table, pba;
 int table_refcnt[MAX_MSIX_TABLE_PAGES];
-int table_idx[MAX_MSIX_TABLE_PAGES];
+void __iomem *table_va[MAX_MSIX_TABLE_PAGES];
 spinlock_t table_lock;
 bool host_maskall, guest_maskall;
 domid_t warned;
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index d0bf63df1d..8128274c07 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -39,75 +38,44 @@ boolean_param("msi", use_msi);
 
 static void __pci_disable_msix(struct msi_desc *);
 
-/* bitmap indicate which fixed map is free */
-static DEFINE_SPINLOCK(msix_fixmap_lock);
-static DECLARE_BITMAP(msix_fixmap_pages, FIX_MSIX_MAX_PAGES);
-
-static int msix_fixmap_alloc(void)
-{
-int i, rc = -ENOMEM;
-
-spin_lock(&msix_fixmap_lock);
-for ( i = 0; i < FIX_MSIX_MAX_PAGES; i++ )
-if ( !test_bit(i, &msix_fixmap_pages) )
-break;
-if ( i == FIX_MSIX_MAX_PAGES )
-goto out;
-rc = FIX_MSIX_IO_RESERV_BASE + i;
-set_bit(i, &msix_fixmap_pages);
-
- out:
-spin_unlock(&msix_fixmap_lock);
-return rc;
-}
-
-static void msix_fixmap_free(int idx)
-{
-spin_lock(&msix_fixmap_lock);
-if ( idx >= FIX_MSIX_IO_RESERV_BASE )
-clear_bit(idx - FIX_MSIX_IO_RESERV_BASE, &msix_fixmap_pages);
-spin_unlock(&msix_fixmap_lock);
-}
-
-static int msix_get_fixmap(struct arch_msix *msix, u64 table_paddr,
+static void __iomem *msix_map_table(struct arch_msix *msix, u64 table_paddr,
u64 entry_paddr)
 {
 long nr_page;
-int idx;
+void __iomem *va = NULL;
 
 nr_page = (entry_paddr >> PAGE_SHIFT) - (table_paddr >> PAGE_SHIFT);
 
 if ( nr_page < 0 || nr_page >= MAX_MSIX_TABLE_PAGES )
-return -EINVAL;
+return NULL;
 
 spin_lock(&msix->table_lock);
 if ( msix->table_refcnt[nr_page]++ == 0 )
 {
-idx = msix_fixmap_alloc();
-if ( idx < 0 )
+va = ioremap(entry_paddr, PAGE_SIZE);
+if ( va == NULL )
 {
 msix->table_refcnt[nr_page]--;
 goto out;
 }
-set_fixmap_nocache(idx, entry_paddr);
-msix->table_idx[nr_page] = idx;
+msix->table_va[nr_page] = va;
 }
 else
-idx = msix->table_idx[nr_page];
+va = msix->table_va[nr_page];
 
  out:
 spin_unlock(&msix->table_lock);
-return idx;
+return va;
 }
 
-static void msix_put_fixmap(struct arch_msix *msix, int idx)
+static void msix_unmap_table(struct arch_msix *msix, void __iomem *va)
 {
 int i;
 
 spin_lock(&msix->table_lock);
 for ( i = 0; i < MAX_MSIX_TABLE_PAGES; i++ )
 {
-if ( msix->table_idx[i] == idx )
+if ( msix->table_va[i] == va )
 break;
 }
 if ( i == MAX_MSIX_TABLE_PAGES )
@@ -115,9 +83,8 @@ static void msix_put_fixmap(struct arch_msix *msix, int idx)
 
 if ( --msix->table_refcnt[i] == 0 )
 {
-clear_fixmap(idx);
-msix_fixmap_free(id

[PATCH] CI: Remove all use of /bin/false as a ROM

[Andrew Cooper]

As the recent work to get PCI Passthrough testing working shows, putting
`/bin/false` as a ROM into guest context doesn't work so well.

For all ROM paths where we're skipping the build, use a slightly-plausible but
likely non-existent path instead.

Signed-off-by: Andrew Cooper 
---
CC: Anthony PERARD 
CC: Stefano Stabellini 
CC: Michal Orzel 
CC: Doug Goldstein 
CC: Marek Marczykowski-Górecki 
---
 automation/scripts/build | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/automation/scripts/build b/automation/scripts/build
index d830cff7b7c7..197d085f3e07 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -67,9 +67,9 @@ else
 
 if [[ "${cc_is_clang}" == "y" ]]; then
 # SeaBIOS cannot be built with clang
-cfgargs+=("--with-system-seabios=/usr/share/seabios/bios.bin")
+cfgargs+=("--with-system-seabios=/usr/share/no-seabios.bin")
 # iPXE cannot be built with clang
-cfgargs+=("--with-system-ipxe=/usr/lib/ipxe/ipxe.pxe")
+cfgargs+=("--with-system-ipxe=/usr/share/no-ipxe.pxe")
 # newlib cannot be built with clang so we cannot build stubdoms
 cfgargs+=("--disable-stubdom")
 fi
@@ -87,7 +87,7 @@ else
 
 # SeaBIOS requires GCC 4.6 or later
 if [[ "${cc_is_gcc}" == "y" && "${cc_ver}" -lt 0x040600 ]]; then
-cfgargs+=("--with-system-seabios=/bin/false")
+cfgargs+=("--with-system-seabios=/usr/share/no-seabios.bin")
 fi
 
 ./configure "${cfgargs[@]}"
-- 
2.30.2

Re: [PATCH] xen/misra: xen-analysis.py: fix return error on PhaseExceptions

[Luca Fancellu]

> On 26 Apr 2023, at 15:08, Andrew Cooper  wrote:
> 
> On 26/04/2023 11:46 am, Luca Fancellu wrote:
>> Currently the script return code is 0 even if an exception is
>> found, because the return code is written only if the exception
>> object has the errorcode member.
>> 
>> Fix the issue returning the errorcode member in case it exists,
>> otherwise use a generic value different from 0.
>> 
>> Fixes: 02b26c02c7c4 ("xen/scripts: add cppcheck tool to the xen-analysis.py 
>> script")
>> Signed-off-by: Luca Fancellu 
>> ---
>> xen/scripts/xen-analysis.py | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/xen/scripts/xen-analysis.py b/xen/scripts/xen-analysis.py
>> index 8e50c27cd898..7185c5a06d2c 100755
>> --- a/xen/scripts/xen-analysis.py
>> +++ b/xen/scripts/xen-analysis.py
>> @@ -26,8 +26,7 @@ def main(argv):
>> cppcheck_analysis.generate_cppcheck_report()
>> except PhaseExceptions as e:
>> print("ERROR: {}".format(e))
>> -if hasattr(e, "errorcode"):
>> -ret_code = e.errorcode
>> +ret_code = e.errorcode if hasattr(e, "errorcode") else 1
> 
> ret_code = getattr(e, "errorcode", 1)
> 
> is rather more succinct, and pythonic.

Yes it looks better, I’ll update the patch

> 
> ~Andrew

Re: [PATCH] xen/misra: xen-analysis.py: fix return error on PhaseExceptions

[Andrew Cooper]

On 26/04/2023 11:46 am, Luca Fancellu wrote:
> Currently the script return code is 0 even if an exception is
> found, because the return code is written only if the exception
> object has the errorcode member.
>
> Fix the issue returning the errorcode member in case it exists,
> otherwise use a generic value different from 0.
>
> Fixes: 02b26c02c7c4 ("xen/scripts: add cppcheck tool to the xen-analysis.py 
> script")
> Signed-off-by: Luca Fancellu 
> ---
>  xen/scripts/xen-analysis.py | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/xen/scripts/xen-analysis.py b/xen/scripts/xen-analysis.py
> index 8e50c27cd898..7185c5a06d2c 100755
> --- a/xen/scripts/xen-analysis.py
> +++ b/xen/scripts/xen-analysis.py
> @@ -26,8 +26,7 @@ def main(argv):
>  cppcheck_analysis.generate_cppcheck_report()
>  except PhaseExceptions as e:
>  print("ERROR: {}".format(e))
> -if hasattr(e, "errorcode"):
> -ret_code = e.errorcode
> +ret_code = e.errorcode if hasattr(e, "errorcode") else 1

ret_code = getattr(e, "errorcode", 1)

is rather more succinct, and pythonic.

~Andrew

[xen-unstable test] 180416: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 180401

Tests which are failing intermittently (not blocking):
 test-amd64-i386-freebsd10-amd64  7 xen-install  fail pass in 180430-retest

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail like 
180381
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeatfail  like 180381
 test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install   fail like 180391
 test-amd64-i386-pair 11 xen-install/dst_host fail  like 180401
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180401
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180401
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180401
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180401
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180401
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180401
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180401
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180401
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180401
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180401
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180401
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-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-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   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-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-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  15 migrate-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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt 15 migrate-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-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 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 

Re: [RFC PATCH] xen/sched/null: avoid crash after failed domU creation

[Stewart Hildebrand]

On 4/25/23 03:42, Jan Beulich wrote:
> On 25.04.2023 08:36, Juergen Gross wrote:
>> On 24.04.23 23:00, Stewart Hildebrand wrote:
>>> When creating a domU, but the creation fails, we may end up in a state
>>> where a vcpu has not yet been added to the null scheduler, but
>>> unit_deassign() is invoked.
>>
>> This is not really true. The vcpu has been added, but it was offline at
>> that time. This resulted in null_unit_insert() returning early and not
>> calling unit_assign().
>>
>> Later the vcpu was onlined during XEN_DOMCTL_setvcpucontext handling,
>> resulting in null_unit_remove() calling unit_deassign().

Makes sense. I'll reword the message in the next revision.

>>> In this case, when running a debug build of
>>> Xen, we will hit an ASSERT and crash Xen:
>>>
>>> (XEN) 
>>> (XEN) Panic on CPU 0:
>>> (XEN) Assertion 'npc->unit == unit' failed at common/sched/null.c:379
>>> (XEN) 
>>>
>>> To work around this, remove the ASSERT and introduce a check for the
>>> case where npc->unit is NULL and simply return false from
>>> unit_deassign().
>>
>> I think the correct fix would be to call unit_deassign() from
>> null_unit_remove() only, if npc->unit isn't NULL. Dario might have a
>> different opinion, though. :-)

Yes, this seems cleaner to me, thanks for the suggestion. I did a quick test, 
and this approach works to avoid the crash too. I'll wait a few days in case 
anyone else wants to chime in, and if there aren't any more comments I'll send 
out a new patch following this suggestion.

> Furthermore, even if the proposed solution was (roughly) followed, ...
> 
>>> --- a/xen/common/sched/null.c
>>> +++ b/xen/common/sched/null.c
>>> @@ -376,7 +376,14 @@ static bool unit_deassign(struct null_private *prv, 
>>> const struct sched_unit *uni
>>>   struct null_pcpu *npc = get_sched_res(cpu)->sched_priv;
>>>
>>>   ASSERT(list_empty(&null_unit(unit)->waitq_elem));
>>> -ASSERT(npc->unit == unit);
>>> +
>>> +if ( !npc->unit )
>>> +{
>>> +dprintk(XENLOG_G_INFO, "%d <-- NULL (%pdv%d)\n", cpu, unit->domain,
>>> +unit->unit_id);
>>> +return false;
>>> +}
>>> +
> 
> ... shouldn't the assertion be kept, with the new if() inserted ahead of
> it? Plus the log message probably better wouldn't print a unit ID like a
> vCPU one, but instead use e.g. %pdu%u?

Sure, although, with Juergen's suggested fix in null_unit_remove(), I think we 
could simply drop this snippet and leave unit_deassign() unmodified.

Your suggested print format is an improvement, but perhaps it would be better 
suited for a separate patch since there are several more instances throughout 
null.c that would also want to be changed.

Example with %pdv%d:
# xl create ...
(XEN) common/sched/null.c:355: 3 <-- d1v0
# xl destroy ...
(XEN) common/sched/null.c:385: 3 <-- NULL (d1v0)

Example with %pdu%u:
# xl create ...
(XEN) common/sched/null.c:355: 3 <-- d1u0
# xl destroy ...
(XEN) common/sched/null.c:385: 3 <-- NULL (d1u0)

Re: [PATCH] Fix install.sh for systemd

[Jason Andryuk]

On Wed, Apr 26, 2023 at 6:40 AM Olaf Hering  wrote:
>
> Wed, 26 Apr 2023 11:07:17 +0200 Jan Beulich :
>
> > On 26.04.2023 10:47, Olaf Hering wrote:
> > > XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from 
> > > make install.
> > ... this suggests to me that you really mean the change doesn't go far
> > enough, but that's then different from nack-ing a change. Can you please
> > clarify this for me (and maybe also for Jason, depending on how he has
> > read your replies)?
>
> I think the change should look like this, the runtime directories have to be 
> created at runtime.

Thanks, Olaf.  Yes, I think your approach is better.  Will you submit
it as a formal patch?  I'm happy to test it.

> --- a/tools/hotplug/Linux/init.d/xendriverdomain.in
> +++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
> @@ -49,6 +49,7 @@ fi
>
>  do_start () {
> echo Starting xl devd...
> +   mkdir -m700 -p ${XEN_RUN_DIR}

This one should be "@XEN_RUN_DIR@"?

Thanks,
Jason

Re: [PATCH] libxl: device_backend_callback() print rc on error

[Jason Andryuk]

On Wed, Apr 26, 2023 at 4:39 AM Jan Beulich  wrote:
>
> On 25.04.2023 21:46, Jason Andryuk wrote:
> > Print the rc when an error is found in device_backend_callback() so the
> > user can have some idea of why things went wrong.
> >
> > Signed-off-by: Jason Andryuk 
> > ---
> >  tools/libs/light/libxl_device.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
>
> While patches which are part of a series should be sent as replies to the
> cover letter, may I ask that you do not send individual patches as replies
> to other (unrelated) patches (or, in general, really as replies to anything,
> i.e. also not as replies to e.g. an earlier discussion)?

Certainly.  Sorry about that.  I formatted the patches individually,
but sent them with a single git send-email command.  Looks like I
should have added --no-thread to have them sent individually.

Regards,
Jason

[PATCH 2/2] VMX/cpu-policy: disable RDTSCP and INVPCID insns as needed

[Jan Beulich]

When either feature is available in hardware, but disabled for a guest,
the respective insn would better cause #UD if attempted to be used.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -785,6 +785,30 @@ static void cf_check vmx_cpuid_policy_ch
 vmx_vmcs_enter(v);
 vmx_update_exception_bitmap(v);
 
+if ( cp->extd.rdtscp )
+{
+v->arch.hvm.vmx.secondary_exec_control |= SECONDARY_EXEC_ENABLE_RDTSCP;
+vmx_update_secondary_exec_control(v);
+}
+else if ( v->arch.hvm.vmx.secondary_exec_control &
+  SECONDARY_EXEC_ENABLE_RDTSCP )
+{
+v->arch.hvm.vmx.secondary_exec_control &= 
~SECONDARY_EXEC_ENABLE_RDTSCP;
+vmx_update_secondary_exec_control(v);
+}
+
+if ( cp->feat.invpcid )
+{
+v->arch.hvm.vmx.secondary_exec_control |= 
SECONDARY_EXEC_ENABLE_INVPCID;
+vmx_update_secondary_exec_control(v);
+}
+else if ( v->arch.hvm.vmx.secondary_exec_control &
+  SECONDARY_EXEC_ENABLE_INVPCID )
+{
+v->arch.hvm.vmx.secondary_exec_control &= 
~SECONDARY_EXEC_ENABLE_INVPCID;
+vmx_update_secondary_exec_control(v);
+}
+
 /*
  * We can safely pass MSR_SPEC_CTRL through to the guest, even if STIBP
  * isn't enumerated in hardware, as SPEC_CTRL_STIBP is ignored.

[PATCH 1/2] VMX/cpu-policy: check availability of RDTSCP and INVPCID

[Jan Beulich]

Both have separate enable bits, which are optional. While on real
hardware we can perhaps expect these VMX controls to be available if
(and only if) the base CPU feature is available, when running
virtualized ourselves this may not be the case.

Signed-off-by: Jan Beulich 
---
Afaics we don't ourselves expose the 1-setting of the two enables. (We
also don't constrain guests to set only bits we report as available to
set; there's a respective TODO comment in set_vvmcs_virtual_safe().)

--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -594,6 +594,12 @@ static void __init calculate_hvm_max_pol
  */
 if ( cpu_has_vmx )
 {
+if ( !cpu_has_vmx_rdtscp )
+__clear_bit(X86_FEATURE_RDTSCP, fs);
+
+if ( !cpu_has_vmx_invpcid )
+__clear_bit(X86_FEATURE_INVPCID, fs);
+
 if ( !cpu_has_vmx_mpx )
 __clear_bit(X86_FEATURE_MPX, fs);
 
--- a/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
+++ b/xen/arch/x86/include/asm/hvm/vmx/vmcs.h
@@ -299,6 +299,8 @@ extern u64 vmx_ept_vpid_cap;
 (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_EPT)
 #define cpu_has_vmx_dt_exiting \
 (vmx_secondary_exec_control & SECONDARY_EXEC_DESCRIPTOR_TABLE_EXITING)
+#define cpu_has_vmx_rdtscp \
+(vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_RDTSCP)
 #define cpu_has_vmx_vpid \
 (vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VPID)
 #define cpu_has_monitor_trap_flag \
@@ -314,6 +316,8 @@ extern u64 vmx_ept_vpid_cap;
  SECONDARY_EXEC_UNRESTRICTED_GUEST)
 #define cpu_has_vmx_ple \
 (vmx_secondary_exec_control & SECONDARY_EXEC_PAUSE_LOOP_EXITING)
+#define cpu_has_vmx_invpcid \
+(vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_INVPCID)
 #define cpu_has_vmx_apic_reg_virt \
 (vmx_secondary_exec_control & SECONDARY_EXEC_APIC_REGISTER_VIRT)
 #define cpu_has_vmx_virtual_intr_delivery \

[PATCH 0/2] VMX/cpu-policy: RDTSCP and INVPCID handling

[Jan Beulich]

While putting in place more of the still missing MSRLIST code, I've
noticed two anomalies here.

1: check availability of RDTSCP and INVPCID
2: disable RDTSCP and INVPCID insns as needed

Jan

Re: [PATCH v1] stubdom: fix errors in newlib:makedoc

[Samuel Thibault]

Olaf Hering, le mer. 26 avril 2023 10:52:39 +, a ecrit:
> rpm post-build-checks found a few code bugs in newlib, and marks them as
> errors. Add another newlib patch and apply it during stubdom build.
> 
> [  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c: In function 
> 'lookup_word':
> [  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147:10: warning: 
> implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
> [  227s]   if (strcmp(ptr->word, word) == 0) return ptr;
> [  227s]   ^
> 
> [  460s] I: Program is using implicit definitions of special functions.
> [  460s]these functions need to use their correct prototypes to allow
> [  460s]the lightweight buffer overflow checking to work.
> [  460s]  - Implicit memory/string functions need #include .
> [  460s]  - Implicit *printf functions need #include .
> [  460s]  - Implicit *printf functions need #include .
> [  460s]  - Implicit *read* functions need #include .
> [  460s]  - Implicit *recv* functions need #include .
> [  460s] E: xen implicit-fortify-decl 
> ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147
> 
> Signed-off-by: Olaf Hering 

Reviewed-by: Samuel Thibault 

Thanks!

> ---
> 
> Depends on newlib-cygmon-gmon.patch
> 
>  stubdom/Makefile |  1 +
>  stubdom/newlib-makedoc.patch | 35 +++
>  2 files changed, 36 insertions(+)
>  create mode 100644 stubdom/newlib-makedoc.patch
> 
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index cddbbe2da0..a21e1c3fa3 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -96,6 +96,7 @@ newlib-$(NEWLIB_VERSION): newlib-$(NEWLIB_VERSION).tar.gz
>   patch -d $@ -p1 < newlib-stdint-size_max-fix-from-1.17.0.patch
>   patch -d $@ -p1 < newlib-disable-texinfo.patch
>   patch -d $@ -p1 < newlib-cygmon-gmon.patch
> + patch -d $@ -p1 < newlib-makedoc.patch
>   find $@ -type f | xargs perl -i.bak \
>   -pe 's/\b_(tzname|daylight|timezone)\b/$$1/g'
>   touch $@
> diff --git a/stubdom/newlib-makedoc.patch b/stubdom/newlib-makedoc.patch
> new file mode 100644
> index 00..90678f1b63
> --- /dev/null
> +++ b/stubdom/newlib-makedoc.patch
> @@ -0,0 +1,35 @@
> +stubdom: fix errors in newlib
> +
> +rpm post-build-checks found a few code bugs in newlib, and marks them as
> +errors. Add another newlib patch and apply it during stubdom build.
> +
> +[  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c: In function 
> 'lookup_word':
> +[  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147:10: warning: 
> implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
> +[  227s]   if (strcmp(ptr->word, word) == 0) return ptr;
> +[  227s]   ^
> +
> +[  460s] I: Program is using implicit definitions of special functions.
> +[  460s]these functions need to use their correct prototypes to allow
> +[  460s]the lightweight buffer overflow checking to work.
> +[  460s]  - Implicit memory/string functions need #include .
> +[  460s]  - Implicit *printf functions need #include .
> +[  460s]  - Implicit *printf functions need #include .
> +[  460s]  - Implicit *read* functions need #include .
> +[  460s]  - Implicit *recv* functions need #include .
> +[  460s] E: xen implicit-fortify-decl 
> ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147
> +---
> + newlib/doc/makedoc.c |1 +
> + 1 file changed, 1 insertion(+)
> +
> +Index: newlib-1.16.0/newlib/doc/makedoc.c
> +===
> +--- newlib-1.16.0.orig/newlib/doc/makedoc.c
>  newlib-1.16.0/newlib/doc/makedoc.c
> +@@ -38,6 +38,7 @@ There is  no
> + #include "ansidecl.h"
> + #include 
> + #include 
> ++#include 
> + #include 
> + 
> + #define DEF_SIZE 5000
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

Re: [PATCH v1] stubdom: fix errors in newlib:cygmon-gmon.c

[Samuel Thibault]

Olaf Hering, le mer. 26 avril 2023 10:51:56 +, a ecrit:
> rpm post-build-checks found a few code bugs in newlib, and marks them as
> errors. Add another newlib patch and apply it during stubdom build.
> 
> I: A function uses a 'return;' statement, but has actually a value
>to return, like an integer ('return 42;') or similar.
> W: xen voidreturn ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:117, 
> 125, 146, 157, 330
> 
> I: Program is using implicit definitions of special functions.
>these functions need to use their correct prototypes to allow
>the lightweight buffer overflow checking to work.
>  - Implicit memory/string functions need #include .
>  - Implicit *printf functions need #include .
>  - Implicit *printf functions need #include .
>  - Implicit *read* functions need #include .
>  - Implicit *recv* functions need #include .
> E: xen implicit-fortify-decl 
> ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:119
> 
> I: Program returns random data in a function
> E: xen no-return-in-nonvoid-function 
> ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:362
> 
> Signed-off-by: Olaf Hering 

Reviewed-by: Samuel Thibault 

Thanks!

> ---
>  stubdom/Makefile |  1 +
>  stubdom/newlib-cygmon-gmon.patch | 60 
>  2 files changed, 61 insertions(+)
>  create mode 100644 stubdom/newlib-cygmon-gmon.patch
> 
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index b312f710cd..cddbbe2da0 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -95,6 +95,7 @@ newlib-$(NEWLIB_VERSION): newlib-$(NEWLIB_VERSION).tar.gz
>   patch -d $@ -p0 < newlib-chk.patch
>   patch -d $@ -p1 < newlib-stdint-size_max-fix-from-1.17.0.patch
>   patch -d $@ -p1 < newlib-disable-texinfo.patch
> + patch -d $@ -p1 < newlib-cygmon-gmon.patch
>   find $@ -type f | xargs perl -i.bak \
>   -pe 's/\b_(tzname|daylight|timezone)\b/$$1/g'
>   touch $@
> diff --git a/stubdom/newlib-cygmon-gmon.patch 
> b/stubdom/newlib-cygmon-gmon.patch
> new file mode 100644
> index 00..b2dfbfafe2
> --- /dev/null
> +++ b/stubdom/newlib-cygmon-gmon.patch
> @@ -0,0 +1,60 @@
> +
> +I: A function uses a 'return;' statement, but has actually a value
> +   to return, like an integer ('return 42;') or similar.
> +W: xen voidreturn ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:117, 
> 125, 146, 157, 330
> +
> +I: Program is using implicit definitions of special functions.
> +   these functions need to use their correct prototypes to allow
> +   the lightweight buffer overflow checking to work.
> + - Implicit memory/string functions need #include .
> + - Implicit *printf functions need #include .
> + - Implicit *printf functions need #include .
> + - Implicit *read* functions need #include .
> + - Implicit *recv* functions need #include .
> +E: xen implicit-fortify-decl 
> ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:119
> +
> +I: Program returns random data in a function
> +E: xen no-return-in-nonvoid-function 
> ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:362
> +
> +---
> + libgloss/i386/cygmon-gmon.c |6 +-
> + 1 file changed, 5 insertions(+), 1 deletion(-)
> +
> +Index: newlib-1.16.0/libgloss/i386/cygmon-gmon.c
> +===
> +--- newlib-1.16.0.orig/libgloss/i386/cygmon-gmon.c
>  newlib-1.16.0/libgloss/i386/cygmon-gmon.c
> +@@ -61,6 +61,8 @@
> + static char sccsid[] = "@(#)gmon.c  5.3 (Berkeley) 5/22/91";
> + #endif /* not lint */
> + 
> ++#include 
> ++#include 
> + #define DEBUG
> + #ifdef DEBUG
> + #include 
> +@@ -89,7 +91,7 @@ static int s_scale;
> + 
> + extern int errno;
> + 
> +-int
> ++void
> + monstartup(lowpc, highpc)
> +  char   *lowpc;
> +  char   *highpc;
> +@@ -199,6 +201,7 @@ _mcleanup()
> + 
> + static char already_setup = 0;
> + 
> ++void
> + _mcount()
> + {
> +   register char *selfpc;
> +@@ -341,6 +344,7 @@ overflow:
> +  *  profiling is what mcount checks to see if
> +  *  all the data structures are ready.
> +  */
> ++void
> + moncontrol(mode)
> + int mode;
> + {
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[PATCH v1] stubdom: fix errors in newlib:makedoc

[Olaf Hering]

rpm post-build-checks found a few code bugs in newlib, and marks them as
errors. Add another newlib patch and apply it during stubdom build.

[  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c: In function 
'lookup_word':
[  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147:10: warning: 
implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
[  227s]   if (strcmp(ptr->word, word) == 0) return ptr;
[  227s]   ^

[  460s] I: Program is using implicit definitions of special functions.
[  460s]these functions need to use their correct prototypes to allow
[  460s]the lightweight buffer overflow checking to work.
[  460s]  - Implicit memory/string functions need #include .
[  460s]  - Implicit *printf functions need #include .
[  460s]  - Implicit *printf functions need #include .
[  460s]  - Implicit *read* functions need #include .
[  460s]  - Implicit *recv* functions need #include .
[  460s] E: xen implicit-fortify-decl 
../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147

Signed-off-by: Olaf Hering 
---

Depends on newlib-cygmon-gmon.patch

 stubdom/Makefile |  1 +
 stubdom/newlib-makedoc.patch | 35 +++
 2 files changed, 36 insertions(+)
 create mode 100644 stubdom/newlib-makedoc.patch

diff --git a/stubdom/Makefile b/stubdom/Makefile
index cddbbe2da0..a21e1c3fa3 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -96,6 +96,7 @@ newlib-$(NEWLIB_VERSION): newlib-$(NEWLIB_VERSION).tar.gz
patch -d $@ -p1 < newlib-stdint-size_max-fix-from-1.17.0.patch
patch -d $@ -p1 < newlib-disable-texinfo.patch
patch -d $@ -p1 < newlib-cygmon-gmon.patch
+   patch -d $@ -p1 < newlib-makedoc.patch
find $@ -type f | xargs perl -i.bak \
-pe 's/\b_(tzname|daylight|timezone)\b/$$1/g'
touch $@
diff --git a/stubdom/newlib-makedoc.patch b/stubdom/newlib-makedoc.patch
new file mode 100644
index 00..90678f1b63
--- /dev/null
+++ b/stubdom/newlib-makedoc.patch
@@ -0,0 +1,35 @@
+stubdom: fix errors in newlib
+
+rpm post-build-checks found a few code bugs in newlib, and marks them as
+errors. Add another newlib patch and apply it during stubdom build.
+
+[  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c: In function 
'lookup_word':
+[  227s] ../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147:10: warning: 
implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
+[  227s]   if (strcmp(ptr->word, word) == 0) return ptr;
+[  227s]   ^
+
+[  460s] I: Program is using implicit definitions of special functions.
+[  460s]these functions need to use their correct prototypes to allow
+[  460s]the lightweight buffer overflow checking to work.
+[  460s]  - Implicit memory/string functions need #include .
+[  460s]  - Implicit *printf functions need #include .
+[  460s]  - Implicit *printf functions need #include .
+[  460s]  - Implicit *read* functions need #include .
+[  460s]  - Implicit *recv* functions need #include .
+[  460s] E: xen implicit-fortify-decl 
../../../../newlib-1.16.0/newlib/doc/makedoc.c:1147
+---
+ newlib/doc/makedoc.c |1 +
+ 1 file changed, 1 insertion(+)
+
+Index: newlib-1.16.0/newlib/doc/makedoc.c
+===
+--- newlib-1.16.0.orig/newlib/doc/makedoc.c
 newlib-1.16.0/newlib/doc/makedoc.c
+@@ -38,6 +38,7 @@ There is  no
+ #include "ansidecl.h"
+ #include 
+ #include 
++#include 
+ #include 
+ 
+ #define DEF_SIZE 5000

[PATCH v1] stubdom: fix errors in newlib:cygmon-gmon.c

[Olaf Hering]

rpm post-build-checks found a few code bugs in newlib, and marks them as
errors. Add another newlib patch and apply it during stubdom build.

I: A function uses a 'return;' statement, but has actually a value
   to return, like an integer ('return 42;') or similar.
W: xen voidreturn ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:117, 
125, 146, 157, 330

I: Program is using implicit definitions of special functions.
   these functions need to use their correct prototypes to allow
   the lightweight buffer overflow checking to work.
 - Implicit memory/string functions need #include .
 - Implicit *printf functions need #include .
 - Implicit *printf functions need #include .
 - Implicit *read* functions need #include .
 - Implicit *recv* functions need #include .
E: xen implicit-fortify-decl 
../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:119

I: Program returns random data in a function
E: xen no-return-in-nonvoid-function 
../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:362

Signed-off-by: Olaf Hering 
---
 stubdom/Makefile |  1 +
 stubdom/newlib-cygmon-gmon.patch | 60 
 2 files changed, 61 insertions(+)
 create mode 100644 stubdom/newlib-cygmon-gmon.patch

diff --git a/stubdom/Makefile b/stubdom/Makefile
index b312f710cd..cddbbe2da0 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -95,6 +95,7 @@ newlib-$(NEWLIB_VERSION): newlib-$(NEWLIB_VERSION).tar.gz
patch -d $@ -p0 < newlib-chk.patch
patch -d $@ -p1 < newlib-stdint-size_max-fix-from-1.17.0.patch
patch -d $@ -p1 < newlib-disable-texinfo.patch
+   patch -d $@ -p1 < newlib-cygmon-gmon.patch
find $@ -type f | xargs perl -i.bak \
-pe 's/\b_(tzname|daylight|timezone)\b/$$1/g'
touch $@
diff --git a/stubdom/newlib-cygmon-gmon.patch b/stubdom/newlib-cygmon-gmon.patch
new file mode 100644
index 00..b2dfbfafe2
--- /dev/null
+++ b/stubdom/newlib-cygmon-gmon.patch
@@ -0,0 +1,60 @@
+
+I: A function uses a 'return;' statement, but has actually a value
+   to return, like an integer ('return 42;') or similar.
+W: xen voidreturn ../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:117, 
125, 146, 157, 330
+
+I: Program is using implicit definitions of special functions.
+   these functions need to use their correct prototypes to allow
+   the lightweight buffer overflow checking to work.
+ - Implicit memory/string functions need #include .
+ - Implicit *printf functions need #include .
+ - Implicit *printf functions need #include .
+ - Implicit *read* functions need #include .
+ - Implicit *recv* functions need #include .
+E: xen implicit-fortify-decl 
../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:119
+
+I: Program returns random data in a function
+E: xen no-return-in-nonvoid-function 
../../../../newlib-1.16.0/libgloss/i386/cygmon-gmon.c:362
+
+---
+ libgloss/i386/cygmon-gmon.c |6 +-
+ 1 file changed, 5 insertions(+), 1 deletion(-)
+
+Index: newlib-1.16.0/libgloss/i386/cygmon-gmon.c
+===
+--- newlib-1.16.0.orig/libgloss/i386/cygmon-gmon.c
 newlib-1.16.0/libgloss/i386/cygmon-gmon.c
+@@ -61,6 +61,8 @@
+ static char sccsid[] = "@(#)gmon.c5.3 (Berkeley) 5/22/91";
+ #endif /* not lint */
+ 
++#include 
++#include 
+ #define DEBUG
+ #ifdef DEBUG
+ #include 
+@@ -89,7 +91,7 @@ static int   s_scale;
+ 
+ extern int errno;
+ 
+-int
++void
+ monstartup(lowpc, highpc)
+  char *lowpc;
+  char *highpc;
+@@ -199,6 +201,7 @@ _mcleanup()
+ 
+ static char already_setup = 0;
+ 
++void
+ _mcount()
+ {
+   register char   *selfpc;
+@@ -341,6 +344,7 @@ overflow:
+  *profiling is what mcount checks to see if
+  *all the data structures are ready.
+  */
++void
+ moncontrol(mode)
+ int mode;
+ {

[PATCH] xen/misra: xen-analysis.py: fix return error on PhaseExceptions

[Luca Fancellu]

Currently the script return code is 0 even if an exception is
found, because the return code is written only if the exception
object has the errorcode member.

Fix the issue returning the errorcode member in case it exists,
otherwise use a generic value different from 0.

Fixes: 02b26c02c7c4 ("xen/scripts: add cppcheck tool to the xen-analysis.py 
script")
Signed-off-by: Luca Fancellu 
---
 xen/scripts/xen-analysis.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/scripts/xen-analysis.py b/xen/scripts/xen-analysis.py
index 8e50c27cd898..7185c5a06d2c 100755
--- a/xen/scripts/xen-analysis.py
+++ b/xen/scripts/xen-analysis.py
@@ -26,8 +26,7 @@ def main(argv):
 cppcheck_analysis.generate_cppcheck_report()
 except PhaseExceptions as e:
 print("ERROR: {}".format(e))
-if hasattr(e, "errorcode"):
-ret_code = e.errorcode
+ret_code = e.errorcode if hasattr(e, "errorcode") else 1
 finally:
 if settings.step_clean_analysis:
 cppcheck_analysis.clean_analysis_artifacts()
-- 
2.34.1

Re: [PATCH] Fix install.sh for systemd

[Olaf Hering]

Wed, 26 Apr 2023 11:07:17 +0200 Jan Beulich :

> On 26.04.2023 10:47, Olaf Hering wrote:
> > XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from 
> > make install.  
> ... this suggests to me that you really mean the change doesn't go far
> enough, but that's then different from nack-ing a change. Can you please
> clarify this for me (and maybe also for Jason, depending on how he has
> read your replies)?

I think the change should look like this, the runtime directories have to be 
created at runtime.

 tools/Makefile |2 --
 tools/hotplug/FreeBSD/rc.d/xencommons.in   |1 +
 tools/hotplug/FreeBSD/rc.d/xendriverdomain.in  |1 +
 tools/hotplug/Linux/init.d/xendriverdomain.in  |1 +
 tools/hotplug/Linux/systemd/xenconsoled.service.in |2 +-
 tools/hotplug/NetBSD/rc.d/xendriverdomain.in   |2 +-

--- a/tools/Makefile
+++ b/tools/Makefile
@@ -58,9 +58,7 @@ build all: subdirs-all
 install:
$(INSTALL_DIR) -m 700 $(DESTDIR)$(XEN_DUMP_DIR)
$(INSTALL_DIR) $(DESTDIR)$(XEN_LOG_DIR)
-   $(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_DIR)
$(INSTALL_DIR) $(DESTDIR)$(XEN_LIB_DIR)
-   $(INSTALL_DIR) $(DESTDIR)$(XEN_RUN_STORED)
$(INSTALL_DIR) $(DESTDIR)$(PKG_INSTALLDIR)
$(MAKE) subdirs-install
 
--- a/tools/hotplug/FreeBSD/rc.d/xencommons.in
+++ b/tools/hotplug/FreeBSD/rc.d/xencommons.in
@@ -34,6 +34,7 @@ xen_startcmd()
local time=0
local timeout=30
 
+   mkdir -p "@XEN_RUN_DIR@"
xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${XENSTORED})
if test -z "$xenstored_pid"; then
printf "Starting xenservices: xenstored, xenconsoled."
--- a/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
+++ b/tools/hotplug/FreeBSD/rc.d/xendriverdomain.in
@@ -27,6 +27,7 @@ xendriverdomain_start()
 {
printf "Starting xenservices: xl devd."
 
+   mkdir -p "@XEN_RUN_DIR@"
PATH="${bindir}:${sbindir}:$PATH" ${sbindir}/xl devd --pidfile 
${XLDEVD_PIDFILE} ${XLDEVD_ARGS}
 
printf "\n"
--- a/tools/hotplug/Linux/init.d/xendriverdomain.in
+++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
@@ -49,6 +49,7 @@ fi
 
 do_start () {
echo Starting xl devd...
+   mkdir -m700 -p ${XEN_RUN_DIR}
${sbindir}/xl devd --pidfile=$XLDEVD_PIDFILE $XLDEVD_ARGS
 }
 do_stop () {
--- a/tools/hotplug/Linux/systemd/xenconsoled.service.in
+++ b/tools/hotplug/Linux/systemd/xenconsoled.service.in
@@ -11,7 +11,7 @@ Environment=XENCONSOLED_TRACE=none
 Environment=XENCONSOLED_LOG_DIR=@XEN_LOG_DIR@/console
 EnvironmentFile=-@CONFIG_DIR@/@CONFIG_LEAF_DIR@/xencommons
 ExecStartPre=/bin/grep -q control_d /proc/xen/capabilities
-ExecStartPre=/bin/mkdir -p ${XENCONSOLED_LOG_DIR}
+ExecStartPre=/bin/mkdir -p ${XENCONSOLED_LOG_DIR} @XEN_RUN_DIR@
 ExecStart=@sbindir@/xenconsoled -i --log=${XENCONSOLED_TRACE} 
--log-dir=${XENCONSOLED_LOG_DIR} $XENCONSOLED_ARGS
 
 [Install]
--- a/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
+++ b/tools/hotplug/NetBSD/rc.d/xendriverdomain.in
@@ -23,7 +23,7 @@ XLDEVD_PIDFILE="@XEN_RUN_DIR@/xldevd.pid"
 
 xendriverdomain_precmd()
 {
-   :
+   mkdir -p "@XEN_RUN_DIR@"
 }
 
 xendriverdomain_startcmd()


Olaf


pgpCFVikVGxYQ.pgp
Description: Digitale Signatur von OpenPGP

[ovmf test] 180423: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 5a349b96b171e85744024904b0c8453d06d2fb45
baseline version:
 ovmf 18f463edbaa911b6e2c32a3e783bf6c2c9997512

Last test of basis   180409  2023-04-25 11:10:48 Z0 days
Testing same since   180423  2023-04-26 03:40:45 Z0 days1 attempts


People who touched revisions under test:
  Igor Kulchytskyy 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   18f463edba..5a349b96b1  5a349b96b171e85744024904b0c8453d06d2fb45 -> 
xen-tested-master

Re: [PATCH] Fix install.sh for systemd

[Jan Beulich]

On 26.04.2023 10:47, Olaf Hering wrote:
> Wed, 26 Apr 2023 10:28:38 +0200 Jan Beulich :
> 
>> Is this to be translated to Reviewed-by: ?
> 
> It was a Nack, in case such thing exists, and if it has any meaning if sent 
> by me.

Now I'm confused: Your reply didn't read like a nack at all, at least to
me. Furthermore ...

> There are a few places already which do a mkdir -p XEN_RUN_DIR prior usage.
> But a few are missing.
> XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from make 
> install.

... this suggests to me that you really mean the change doesn't go far
enough, but that's then different from nack-ing a change. Can you please
clarify this for me (and maybe also for Jason, depending on how he has
read your replies)?

Jan

RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Henry Wang]

Hi Jan

> -Original Message-
> From: Henry Wang
> Subject: RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> tree NUMA distance map
> 
> Hi Jan,
> 
> > -Original Message-
> > From: Jan Beulich 
> > Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> > tree NUMA distance map
> >
> > > Great points! Thanks for pointing the 8-bit truncation out. You are 
> > > correct.
> > > Somehow my impression of numa_set_distance()'s first two arguments
> are
> > > already "unsigned int" so I missed this part...Sorry.
> > >
> > > In that case, I think I will add a check between "from, to" and
> > MAX_NUMNODES
> > > as soon as the values of "from" and "to" are populated by dt_next_cell().
> > > Hopefully this will address your concern.
> >
> > While this would address by concern, I don't see why you want to repeat
> > the checking that numa_set_distance() already does.
> 
> Correct, I think I would better to move the check in numa_set_distance() to
> the caller fdt_parse_numa_distance_map_v1() as I believe if the truncation
> really happens it is too late to check in numa_set_distance().

On second thought, maybe even remove the same check in __node_distance()
if we do the check in the caller, as I believe they will suffer the same 
problem...

Kind regards,
Henry

> 
> Kind regards,
> Henry
> 
> >
> > Jan

Re: [PATCH] Fix install.sh for systemd

[Olaf Hering]

Wed, 26 Apr 2023 10:28:38 +0200 Jan Beulich :

> Is this to be translated to Reviewed-by: ?

It was a Nack, in case such thing exists, and if it has any meaning if sent by 
me.

There are a few places already which do a mkdir -p XEN_RUN_DIR prior usage.
But a few are missing.
XEN_RUN_DIR and most likely also XEN_RUN_STORED have to be removed from make 
install.


Olaf


pgpsyUKXed8zB.pgp
Description: Digitale Signatur von OpenPGP

[linux-linus test] 180418: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

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

version targeted for testing:
 linux736b378b29d89c8c3567fa4b2e948be5568aebb8
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z9 days
Failing since180281  2023-04-17 06:24:36 Z9 days   16 attempts
Testing same since   180418  2023-04-25 20:19:00 Z0 days1 attempts


638 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopsfail
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-amd64-coresched-amd64-xlpass
 test-arm64-arm64-xl  blocked 
 test-armhf-armhf-xl  fail
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm pass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm blocked 
 test-

Re: [PATCH] libxl: device_backend_callback() print rc on error

[Jan Beulich]

On 25.04.2023 21:46, Jason Andryuk wrote:
> Print the rc when an error is found in device_backend_callback() so the
> user can have some idea of why things went wrong.
> 
> Signed-off-by: Jason Andryuk 
> ---
>  tools/libs/light/libxl_device.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

While patches which are part of a series should be sent as replies to the
cover letter, may I ask that you do not send individual patches as replies
to other (unrelated) patches (or, in general, really as replies to anything,
i.e. also not as replies to e.g. an earlier discussion)?

Thanks, Jan

Re: [PATCH v2 RESEND] xen: Fix SEGV on domain disconnect

[Tim Smith]

On Mon, Apr 24, 2023 at 2:51 PM Paul Durrant  wrote:
>
> So if you drop the ring drain then this patch should still stop the
> SEGVs, right?
>

I think that's worth a few test runs. I recall some coredumps in that
condition when I was investigating early on, but I don't have them in
my collection so maybe I'm misremembering.

Tim

RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Henry Wang]

Hi Jan,

> -Original Message-
> From: Jan Beulich 
> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> tree NUMA distance map
> 
> > Great points! Thanks for pointing the 8-bit truncation out. You are correct.
> > Somehow my impression of numa_set_distance()'s first two arguments are
> > already "unsigned int" so I missed this part...Sorry.
> >
> > In that case, I think I will add a check between "from, to" and
> MAX_NUMNODES
> > as soon as the values of "from" and "to" are populated by dt_next_cell().
> > Hopefully this will address your concern.
> 
> While this would address by concern, I don't see why you want to repeat
> the checking that numa_set_distance() already does.

Correct, I think I would better to move the check in numa_set_distance() to
the caller fdt_parse_numa_distance_map_v1() as I believe if the truncation
really happens it is too late to check in numa_set_distance().

Kind regards,
Henry

> 
> Jan

Re: [PATCH] Fix install.sh for systemd

[Jan Beulich]

On 26.04.2023 09:15, Olaf Hering wrote:
> Tue, 25 Apr 2023 15:46:20 -0400 Jason Andryuk :
> 
>> Skip populating /var/run/xen when systemd is being used.
> 
> It was wrong to do it like that from day one.
> Such directory has to be populated on demand at runtime.

Is this to be translated to Reviewed-by: ?

Jan

Re: [PATCH v2] ns16550: enable memory decoding on MMIO-based PCI console card

[Jan Beulich]

On 26.04.2023 09:48, Roger Pau Monné wrote:
> On Tue, Apr 25, 2023 at 04:39:02PM +0200, Marek Marczykowski-Górecki wrote:
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -272,7 +272,15 @@ static int cf_check ns16550_getc(struct serial_port 
>> *port, char *pc)
>>  static void pci_serial_early_init(struct ns16550 *uart)
>>  {
>>  #ifdef NS16550_PCI
>> -if ( !uart->ps_bdf_enable || uart->io_base >= 0x1 )
>> +if ( uart->bar )
>> +{
>> +pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
>> +  uart->ps_bdf[2]),
>> + PCI_COMMAND, PCI_COMMAND_MEMORY);
> 
> Don't you want to read the current command register first and just
> or PCI_COMMAND_MEMORY?
> 
> I see that for IO decoding we already do it this way, but I'm not sure
> whether it could cause issues down the road by unintentionally
> disabling command flags.

Quite some time ago I asked myself the same question when seeing that
code, but I concluded that perhaps none of the bits are really sensible
to be set for a device as simple as a serial one. I'm actually thinking
that for such a device to be used during early boot, it might even be
helpful if bits like PARITY or SERR get cleared. (Of course if any of
that was really the intention of the change introducing that code, it
should have come with a code comment.)

Jan

Re: [patch 22/37] arm64: smp: Switch to hotplug core state synchronization

[Thomas Gleixner]

On Wed, Apr 26 2023 at 08:59, Mark Rutland wrote:
> On Tue, Apr 25, 2023 at 09:51:12PM +0200, Thomas Gleixner wrote:
>> If not then it's just yet another way of DoS which is an "acceptable"
>> attack as it only affects availability but not confidentiality.
>
> Sure.
>
> My thinking is that this is an attack against the *integrity* of the guest
> (since the vCPU that gets unpasued may write to memory), and so it's
> potentially more than just a DoS.
>
> I only mention this because I'd like to account for that on arm64, and if 
> other
> architectures also wanted to handle that it might make sense to have some
> common infrastructure to track whether CPUs are potentially still within the
> kernel.

Fair enough.

Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Jan Beulich]

On 26.04.2023 09:36, Henry Wang wrote:
>> -Original Message-
>> From: Jan Beulich 
>>
>> On 26.04.2023 08:29, Henry Wang wrote:
 -Original Message-
 From: Jan Beulich 

> +distance = dt_next_cell(1, &matrix);

 Upon second thought I checked what dt_next_cell() returns: You're silently
 truncating here and then ...

> +/* Bi-directions are not set, set both */
> +numa_set_distance(from, to, distance);
> +numa_set_distance(to, from, distance);

 ... here again. Is that really the intention?
>>>
>>> By hunting down the historical discussions I found that using dt_next_cell()
>> is
>>> what Julien suggested 2 years ago in the RFC series [1]. Given the 
>>> truncation
>>> here is for node id (from/to) and distance which I am pretty sure will not
>>> exceed 32-bit range, I think the silent truncation is safe.
>>
>> That discussion is orthogonal; the previously used dt_read_number() is no
>> different in the regard I'm referring to.
>>
>>> However I understand your point here, the silent truncation is not ideal, so
>>> I wonder if you have any suggestions to improve, do you think I should
>> change
>>> these variables to u64 or maybe I need to do the explicit type cast or any
>>> better suggestions from you? Thanks!
>>
>> So one thing I overlooked is that by passing 1 as the first argument, you
>> only request a 32-bit value. Hence there's no (silent) truncation then on
>> the dt_next_cell() uses. But the numa_set_distance() calls still truncate
>> to 8 bits. Adding explicit type casts won't help at all - truncation will
>> remain as silent as it was before. However, numa_set_distance()'s first
>> two arguments could easily become "unsigned int", resulting in its node
>> related bounds checking to take care of all truncation issues. The
>> "distance" parameter already is unsigned int, and is already being bounds
>> checked against NUMA_NO_DISTANCE.
> 
> Great points! Thanks for pointing the 8-bit truncation out. You are correct.
> Somehow my impression of numa_set_distance()'s first two arguments are
> already "unsigned int" so I missed this part...Sorry.
> 
> In that case, I think I will add a check between "from, to" and MAX_NUMNODES
> as soon as the values of "from" and "to" are populated by dt_next_cell().
> Hopefully this will address your concern.

While this would address by concern, I don't see why you want to repeat
the checking that numa_set_distance() already does.

Jan

Re: [patch 22/37] arm64: smp: Switch to hotplug core state synchronization

[Mark Rutland]

On Tue, Apr 25, 2023 at 09:51:12PM +0200, Thomas Gleixner wrote:
> On Mon, Apr 17 2023 at 16:50, Mark Rutland wrote:
> > As a tangent/aside, we might need to improve that for confidential compute
> > architectures, and we might want to generically track cpus which might 
> > still be
> > using kernel text/data. On arm64 we ensure that via our cpu_kill() callback
> > (which'll use PSCI CPU_AFFINITY_INFO), but I'm not sure if TDX and/or 
> > SEV-SNP
> > have a similar mechanism.
> >
> > Otherwise, a malicious hypervisor can pause a vCPU just before it leaves the
> > kernel (e.g. immediately after the arch_cpuhp_cleanup_dead_cpu() call), wait
> > for a kexec (or resuse of stack memroy), and unpause the vCPU to cause 
> > things
> > to blow up.
> 
> There are a gazillion ways for a malicious hypervisor to blow up a
> 'squint enough to be confident' guest.
> 
> The real question is whether it can utilize such a blow up to extract
> confidential information from the guest.
>
> If not then it's just yet another way of DoS which is an "acceptable"
> attack as it only affects availability but not confidentiality.

Sure.

My thinking is that this is an attack against the *integrity* of the guest
(since the vCPU that gets unpasued may write to memory), and so it's
potentially more than just a DoS.

I only mention this because I'd like to account for that on arm64, and if other
architectures also wanted to handle that it might make sense to have some
common infrastructure to track whether CPUs are potentially still within the
kernel.

Thanks,
Mark.

Re: [PATCH v2] ns16550: enable memory decoding on MMIO-based PCI console card

[Roger Pau Monné]

On Tue, Apr 25, 2023 at 04:39:02PM +0200, Marek Marczykowski-Górecki wrote:
> pci_serial_early_init() enables PCI_COMMAND_IO for IO-based UART
> devices, add setting PCI_COMMAND_MEMORY for MMIO-based UART devices too.
> Note the MMIO-based devices in practice need a "pci" sub-option,
> otherwise a few parameters are not initialized (including bar_idx,
> reg_shift, reg_width etc). The "pci" is not supposed to be used with
> explicit BDF, so do not key setting PCI_COMMAND_MEMORY on explicit BDF
> being set. Contrary to the IO-based UART, pci_serial_early_init() will
> not attempt to set BAR0 address, even if user provided io_base manually
> - in most cases, those are with an offest and the current cmdline syntax
> doesn't allow expressing it. Due to this, enable PCI_COMMAND_MEMORY only
> if uart->bar is already populated. In similar spirit, this patch does
> not support setting BAR0 of the bridge.

FWIW (not that should be done here) but I think we also want to
disable memory decoding in pci_uart_config() while sizing the BAR.

> Signed-off-by: Marek Marczykowski-Górecki 
> ---
> This fixes the issue I was talking about on #xendevel. Thanks Roger for
> the hint.
> 
> Changes in v2:
>  - check if uart->bar instead of uart->io_base
>  - move it ahead of !uart->ps_bdf_enable return
>  - expand commit message.
> ---
>  xen/drivers/char/ns16550.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 1b21eb93c45f..34231dcb23ea 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -272,7 +272,15 @@ static int cf_check ns16550_getc(struct serial_port 
> *port, char *pc)
>  static void pci_serial_early_init(struct ns16550 *uart)
>  {
>  #ifdef NS16550_PCI
> -if ( !uart->ps_bdf_enable || uart->io_base >= 0x1 )
> +if ( uart->bar )
> +{
> +pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
> +  uart->ps_bdf[2]),
> + PCI_COMMAND, PCI_COMMAND_MEMORY);

Don't you want to read the current command register first and just
or PCI_COMMAND_MEMORY?

I see that for IO decoding we already do it this way, but I'm not sure
whether it could cause issues down the road by unintentionally
disabling command flags.

Thanks, Roger.

RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Henry Wang]

Hi Jan,

> -Original Message-
> From: Jan Beulich 
> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> tree NUMA distance map
> 
> On 26.04.2023 08:29, Henry Wang wrote:
> >> -Original Message-
> >> From: Jan Beulich 
> >>
> >>> +distance = dt_next_cell(1, &matrix);
> >>
> >> Upon second thought I checked what dt_next_cell() returns: You're silently
> >> truncating here and then ...
> >>
> >>> +/* Bi-directions are not set, set both */
> >>> +numa_set_distance(from, to, distance);
> >>> +numa_set_distance(to, from, distance);
> >>
> >> ... here again. Is that really the intention?
> >
> > By hunting down the historical discussions I found that using dt_next_cell()
> is
> > what Julien suggested 2 years ago in the RFC series [1]. Given the 
> > truncation
> > here is for node id (from/to) and distance which I am pretty sure will not
> > exceed 32-bit range, I think the silent truncation is safe.
> 
> That discussion is orthogonal; the previously used dt_read_number() is no
> different in the regard I'm referring to.
> 
> > However I understand your point here, the silent truncation is not ideal, so
> > I wonder if you have any suggestions to improve, do you think I should
> change
> > these variables to u64 or maybe I need to do the explicit type cast or any
> > better suggestions from you? Thanks!
> 
> So one thing I overlooked is that by passing 1 as the first argument, you
> only request a 32-bit value. Hence there's no (silent) truncation then on
> the dt_next_cell() uses. But the numa_set_distance() calls still truncate
> to 8 bits. Adding explicit type casts won't help at all - truncation will
> remain as silent as it was before. However, numa_set_distance()'s first
> two arguments could easily become "unsigned int", resulting in its node
> related bounds checking to take care of all truncation issues. The
> "distance" parameter already is unsigned int, and is already being bounds
> checked against NUMA_NO_DISTANCE.

Great points! Thanks for pointing the 8-bit truncation out. You are correct.
Somehow my impression of numa_set_distance()'s first two arguments are
already "unsigned int" so I missed this part...Sorry.

In that case, I think I will add a check between "from, to" and MAX_NUMNODES
as soon as the values of "from" and "to" are populated by dt_next_cell().
Hopefully this will address your concern.

Kind regards,
Henry

> 
> Jan
> 
> > [1] https://lists.xenproject.org/archives/html/xen-devel/2021-
> 08/msg01175.html
> >
> > Kind regards,
> > Henry

Re: [PATCH v4 00/13] tools/xenstore: rework internal accounting

[Juergen Gross]

On 05.04.23 09:03, Juergen Gross wrote:

This series reworks the Xenstore internal accounting to use a uniform
generic framework. It is adding some additional useful diagnostic
information, like accounting trace and max. per-domain and global quota
values seen.

Changes in V2:
- added patch 1 (leftover from previous series)
- rebase

Changes in V3:
- addressed comments

Changes in V4:
- fixed patch 3


Another thought for this series and followup one:

Do we want to keep current coding style in tools/xenstore (basically
Linux kernel style), or do we want to switch to Xen style instead?

If a switch to Xen style is preferred (I do prefer that switch), I'd
like to suggest that I do a rework of this series and the followup one
to use the Xen style for new or moved functions.

A more radical approach would be to do a large style switch series
after the two series, but I'm hesitant as this would affect backports
rather badly.


Juergen



OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] Fix install.sh for systemd

[Olaf Hering]

Tue, 25 Apr 2023 15:46:20 -0400 Jason Andryuk :

> Skip populating /var/run/xen when systemd is being used.

It was wrong to do it like that from day one.
Such directory has to be populated on demand at runtime.


Olaf


pgpScH1_9_YpA.pgp
Description: Digitale Signatur von OpenPGP

Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Jan Beulich]

On 26.04.2023 09:08, Henry Wang wrote:
> Hi Jan,
> 
>> -Original Message-
>> From: Jan Beulich 
>> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
>> tree NUMA distance map
>>
>> On 26.04.2023 07:33, Henry Wang wrote:
 -Original Message-
 From: Jan Beulich 

> +/* Get opposite way distance */
> +opposite = __node_distance(to, from);
> +/* The default value in node_distance_map is
>> NUMA_NO_DISTANCE
 */
> +if ( opposite == NUMA_NO_DISTANCE )

 And the matrix you're reading from can't hold NUMA_NO_DISTANCE
>> entries?
 I ask because you don't check this above; you only check against
 NUMA_LOCAL_DISTANCE.
>>>
>>> My understanding for the purpose of this part of code is to check if the
>> opposite
>>> way distance has already been set, so we need to compare the opposite
>> way
>>> distance with the default value NUMA_NO_DISTANCE here.
>>>
>>> Back to your question, I can see your point of the question. However I don't
>> think
>>> NUMA_NO_DISTANCE is a valid value to describe the node distance in the
>> device
>>> tree. This is because I hunted down the previous discussions and found [2]
>> about
>>> we should try to keep consistent between the value used in device tree and
>> ACPI
>>> tables. From the ACPI spec, 0xFF, i.e. NUMA_NO_DISTANCE means
>> unreachable.
>>> I think this is also the reason why NUMA_NO_DISTANCE can be used as the
>> default
>>> value of the distance map, otherwise we won't have any value to use.
>>
>> The [2] link you provided discusses NUMA_LOCAL_DISTANCE.
> 
> I inferred the discussion as "we should try to keep consistent between the 
> value
> used in device tree and ACPI tables". Maybe my inference is wrong.
> 
>> Looking at
>> Linux'es Documentation/devicetree/numa.txt, there's no mention of an
>> upper bound on the distance values. It only says that on the diagonal
>> entries should be 10 (i.e. matching ACPI, without really saying so).
> 
> I agree that the NUMA device tree binding is a little bit vague. So I cannot
> say the case that you provided is not valid. I would like to ask Arm 
> maintainers
> (putting them into To:) opinion on this as I think I am not the one to decide 
> the
> expected behavior on Arm.
> 
> Bertrand/Julien/Stefano: Would you please kindly share your opinion on which
> value should be used as the default value of the node distance map? Do you
> think reusing the "unreachable" distance, i.e. 0xFF, as the default node 
> distance
> is acceptable here? Thanks!

My suggestion would be to, rather than rejecting values >= 0xff, to saturate
at 0xfe, while keeping 0xff for NUMA_NO_DISTANCE (and overall keeping things
consistent with ACPI).

Jan

RE: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Henry Wang]

Hi Jan,

> -Original Message-
> From: Jan Beulich 
> Subject: Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device
> tree NUMA distance map
> 
> On 26.04.2023 07:33, Henry Wang wrote:
> >> -Original Message-
> >> From: Jan Beulich 
> >>
> >>> +/* Get opposite way distance */
> >>> +opposite = __node_distance(to, from);
> >>> +/* The default value in node_distance_map is
> NUMA_NO_DISTANCE
> >> */
> >>> +if ( opposite == NUMA_NO_DISTANCE )
> >>
> >> And the matrix you're reading from can't hold NUMA_NO_DISTANCE
> entries?
> >> I ask because you don't check this above; you only check against
> >> NUMA_LOCAL_DISTANCE.
> >
> > My understanding for the purpose of this part of code is to check if the
> opposite
> > way distance has already been set, so we need to compare the opposite
> way
> > distance with the default value NUMA_NO_DISTANCE here.
> >
> > Back to your question, I can see your point of the question. However I don't
> think
> > NUMA_NO_DISTANCE is a valid value to describe the node distance in the
> device
> > tree. This is because I hunted down the previous discussions and found [2]
> about
> > we should try to keep consistent between the value used in device tree and
> ACPI
> > tables. From the ACPI spec, 0xFF, i.e. NUMA_NO_DISTANCE means
> unreachable.
> > I think this is also the reason why NUMA_NO_DISTANCE can be used as the
> default
> > value of the distance map, otherwise we won't have any value to use.
> 
> The [2] link you provided discusses NUMA_LOCAL_DISTANCE.

I inferred the discussion as "we should try to keep consistent between the value
used in device tree and ACPI tables". Maybe my inference is wrong.

> Looking at
> Linux'es Documentation/devicetree/numa.txt, there's no mention of an
> upper bound on the distance values. It only says that on the diagonal
> entries should be 10 (i.e. matching ACPI, without really saying so).

I agree that the NUMA device tree binding is a little bit vague. So I cannot
say the case that you provided is not valid. I would like to ask Arm maintainers
(putting them into To:) opinion on this as I think I am not the one to decide 
the
expected behavior on Arm.

Bertrand/Julien/Stefano: Would you please kindly share your opinion on which
value should be used as the default value of the node distance map? Do you
think reusing the "unreachable" distance, i.e. 0xFF, as the default node 
distance
is acceptable here? Thanks!

Kind regards,
Henry

> 
> Jan

Re: [PATCH v4 04/13] tools/xenstore: add framework to commit accounting data on success only

[Juergen Gross]

On 25.04.23 19:52, Julien Grall wrote:

Hi Juergen,

On 05/04/2023 08:03, Juergen Gross wrote:

Instead of modifying accounting data and undo those modifications in
case of an error during further processing, add a framework for
collecting the needed changes and commit them only when the whole
operation has succeeded.

This scheme can reuse large parts of the per transaction accounting.
The changed_domain handling can be reused, but the array size of the
accounting data should be possible to be different for both use cases.

Signed-off-by: Juergen Gross 
---
V3:
- call acc_commit() earlier (Julien Grall)
- add assert() to acc_commit()
- use fixed sized acc array in struct changed_domain (Julien Grall)
---
  tools/xenstore/xenstored_core.c   |  9 --
  tools/xenstore/xenstored_core.h   |  3 ++
  tools/xenstore/xenstored_domain.c | 53 ++-
  tools/xenstore/xenstored_domain.h |  5 ++-
  4 files changed, 66 insertions(+), 4 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 3ca68681e3..84335f5f3d 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1023,6 +1023,9 @@ static void send_error(struct connection *conn, int error)
  break;
  }
  }
+
+    acc_drop(conn);
+
  send_reply(conn, XS_ERROR, xsd_errors[i].errstring,
    strlen(xsd_errors[i].errstring) + 1);
  }
@@ -1034,6 +1037,9 @@ void send_reply(struct connection *conn, enum 
xsd_sockmsg_type type,

  assert(type != XS_WATCH_EVENT);
+    conn->in = NULL;


AFAIU, you are setting conn->in to NULL in order to please..


+    acc_commit(conn);


... this call. However in case of an error like...


+
  if ( len > XENSTORE_PAYLOAD_MAX ) { >   send_error(conn, E2BIG);


... here, send_reply() will be called again. But the error will not be set 
because conn->in is NULL.


So I think you want to restore conn->in rewrite acc_commit(). The ordering would 
also deserve an explanation in a comment.


Just to make sure I understand you correctly (I have some difficulties
parsing "So I think you want to restore conn->in rewrite acc_commit()."
completely):

You are suggesting to move setting conn->in to NULL to acc_commit() and
to restore it before returning? I'm fine with that.




  return;
@@ -1059,8 +1065,6 @@ void send_reply(struct connection *conn, enum 
xsd_sockmsg_type type,

  }
  }
-    conn->in = NULL;
-
  /* Update relevant header fields and fill in the message body. */
  bdata->hdr.msg.type = type;
  bdata->hdr.msg.len = len;
@@ -2195,6 +2199,7 @@ struct connection *new_connection(const struct 
interface_funcs *funcs)

  new->is_stalled = false;
  new->transaction_started = 0;
  INIT_LIST_HEAD(&new->out_list);
+    INIT_LIST_HEAD(&new->acc_list);
  INIT_LIST_HEAD(&new->ref_list);
  INIT_LIST_HEAD(&new->watches);
  INIT_LIST_HEAD(&new->transaction_list);
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index c59b06551f..1f811f38cb 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -139,6 +139,9 @@ struct connection
  struct list_head out_list;
  uint64_t timeout_msec;
+    /* Not yet committed accounting data (valid if in != NULL). */
+    struct list_head acc_list;
+
  /* Referenced requests no longer pending. */
  struct list_head ref_list;
diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c

index 30fb9acec6..144cbafb73 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -91,6 +91,8 @@ struct domain
  bool wrl_delay_logged;
  };
+#define ACC_CHD_N (ACC_TR_N < ACC_REQ_N ? ACC_REQ_N : ACC_TR_N)


Both ACC_TR_N and ACC_REQ_N are fixed. Can you explain why we need this magic?

Related, wouldn't it be better to define it in the enum?


I can do that, of course. I just didn't want to make the enum even more
complex. :-)

But with a comment this should be okay IMO.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH v4 09/17] xen/arm: introduce a helper to parse device tree NUMA distance map

[Jan Beulich]

On 26.04.2023 08:29, Henry Wang wrote:
>> -Original Message-
>> From: Jan Beulich 
>>
>>> +distance = dt_next_cell(1, &matrix);
>>
>> Upon second thought I checked what dt_next_cell() returns: You're silently
>> truncating here and then ...
>>
>>> +/* Bi-directions are not set, set both */
>>> +numa_set_distance(from, to, distance);
>>> +numa_set_distance(to, from, distance);
>>
>> ... here again. Is that really the intention?
> 
> By hunting down the historical discussions I found that using dt_next_cell() 
> is
> what Julien suggested 2 years ago in the RFC series [1]. Given the truncation
> here is for node id (from/to) and distance which I am pretty sure will not
> exceed 32-bit range, I think the silent truncation is safe.

That discussion is orthogonal; the previously used dt_read_number() is no
different in the regard I'm referring to.

> However I understand your point here, the silent truncation is not ideal, so
> I wonder if you have any suggestions to improve, do you think I should change
> these variables to u64 or maybe I need to do the explicit type cast or any
> better suggestions from you? Thanks!

So one thing I overlooked is that by passing 1 as the first argument, you
only request a 32-bit value. Hence there's no (silent) truncation then on
the dt_next_cell() uses. But the numa_set_distance() calls still truncate
to 8 bits. Adding explicit type casts won't help at all - truncation will
remain as silent as it was before. However, numa_set_distance()'s first
two arguments could easily become "unsigned int", resulting in its node
related bounds checking to take care of all truncation issues. The
"distance" parameter already is unsigned int, and is already being bounds
checked against NUMA_NO_DISTANCE.

Jan

> [1] https://lists.xenproject.org/archives/html/xen-devel/2021-08/msg01175.html
> 
> Kind regards,
> Henry