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

[osstest service owner]

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

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  f86d0a1ff200264aaf80b65d7d200a3ba19c7845
baseline version:
 xen  54073350bad16b6045522df40a90be79d970aa0e

Last test of basis   175131  2022-12-09 22:01:53 Z0 days
Testing same since   175133  2022-12-10 01:03:23 Z0 days1 attempts


People who touched revisions under test:
  Bertrand Marquis 
  Daniel P. Smith 
  Julien Grall 
  Michal Orzel 
  Stefano Stabellini 
  Stefano Stabellini 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   54073350ba..f86d0a1ff2  f86d0a1ff200264aaf80b65d7d200a3ba19c7845 -> smoke

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-multivcpu 14 guest-start fail in 175106 pass in 175128
 test-armhf-armhf-xl-rtds 14 guest-startfail pass in 175106
 test-armhf-armhf-libvirt 13 debian-fixup   fail pass in 175106
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 20 
guest-start/debianhvm.repeat fail pass in 175106
 test-armhf-armhf-xl-vhd  13 guest-startfail pass in 175106

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeat fail in 175106 REGR. vs. 
174962

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

[xen-unstable test] 175126: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt   7 xen-install  fail REGR. vs. 175104

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

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

RE: [PATCH 1/3] xen/arm: Add memory overlap check for bootinfo.reserved_mem

[Henry Wang]

Hi both,

I was lurking around to see how the discussion would go. Thanks for the
discussions/inputs in this thread :) 

> -Original Message-
> From: Stefano Stabellini 
> Subject: Re: [PATCH 1/3] xen/arm: Add memory overlap check for
> bootinfo.reserved_mem
> > > It cannot be a single static inline function because the bootinfo
> > > arguments are of three different types, it just happens that all three
> > > have a "start" and "size" struct member so it works great with a macro,
> > > but doesn't for a function.
> >
> > It is not clear to me what are the three types you are referring to. Looking
> > at the definition of bootinfo is:
> >
> > struct bootinfo {
> > struct meminfo mem;
> > /* The reserved regions are only used when booting using Device-Tree */
> > struct meminfo reserved_mem;
> > struct bootmodules modules;
> > struct bootcmdlines cmdlines;
> > #ifdef CONFIG_ACPI
> > struct meminfo acpi;
> > #endif
> > bool static_heap;
> > };
> >
> > cmdlines is something uninteresting here. So we have two types:
> >   - bootmodules for modules
> >   - meminfo used by reserved_mem, mem and acpi

Exactly, we need to check the given input physical address range is
not overlapping with any of the banks in bootmodules and meminfo used by
reserved_mem & acpi.

> >
> > Looking in details the code, now I understand why you suggested the
> macro.
> > This is far better than the checking what the array type (not very 
> > scalable).
> 
> Thank you :-)

+1, I also thought this would be quite painful to extend in the future (once we
add a new member in bootinfo, for example what Penny did in [1], we need to
extend the overlap check as well), but I didn't think of using macro so thank 
you
Stefano :)

> 
> 
> > Personally, I think trying to share the code between the two types is a bit
> > odd. The logic is the same today, but I envision to merge reserved_mem,
> mem
> > and acpi in a single array (it would look like the E820) as this would make
> > easier to find the caching attributes per regions when mapping the RAM. So
> > sharing the code would not be possible.
> >
> > That said, if you really want to share the code between the two types. Then
> I
> > would prefer one of the following option:
> >1) Provide a callback that is used to fetch the information from the 
> > array
> >2) Provide a common structure that could be used by the function.
> >
> > This would match other generic function like sort & co.
> 
> I think option 2) would be the best but I couldn't think of a simple way
> to do it (without using a union and I thought a union would not make
> things nicer in this case).
> 
> Rather than option 1), I think I would rather have 2 different functions
> to check struct bootmodules and struct meminfo, or the macro.

I personally don't have specific taste here. I think the option
is good one as long as we can (1) share most part of the code (2) make the
code easy to extend in the future. So as long as you two reach
a consensus here I will change to the agreed method in v2.

[1] 
https://lore.kernel.org/xen-devel/20221115025235.1378931-2-penny.zh...@arm.com/

Kind regards,
Henry

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

[osstest service owner]

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

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  54073350bad16b6045522df40a90be79d970aa0e
baseline version:
 xen  8d30b9e32c462fdfab4207bfcd31ed63749c4f28

Last test of basis   175130  2022-12-09 19:03:41 Z0 days
Testing same since   175131  2022-12-09 22:01:53 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 

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
   8d30b9e32c..54073350ba  54073350bad16b6045522df40a90be79d970aa0e -> smoke

[linux-linus test] 175116: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-examine  8 reboot   fail REGR. vs. 173462
 test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 173462
 test-arm64-arm64-xl-seattle   8 xen-boot fail REGR. vs. 173462
 test-arm64-arm64-xl-credit1   8 xen-boot fail REGR. vs. 173462
 test-arm64-arm64-xl-vhd   8 xen-boot fail REGR. vs. 173462
 test-armhf-armhf-xl-multivcpu  8 xen-bootfail REGR. vs. 173462
 test-armhf-armhf-libvirt-qcow2  8 xen-boot   fail REGR. vs. 173462
 test-armhf-armhf-xl-arndale   8 xen-boot fail REGR. vs. 173462
 test-armhf-armhf-examine  8 reboot   fail REGR. vs. 173462
 test-armhf-armhf-xl   8 xen-boot fail REGR. vs. 173462
 test-arm64-arm64-xl-xsm   8 xen-boot fail REGR. vs. 173462
 test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 173462
 test-arm64-arm64-libvirt-xsm  8 xen-boot fail REGR. vs. 173462
 test-arm64-arm64-libvirt-raw  8 xen-boot fail REGR. vs. 173462
 test-armhf-armhf-xl-credit2   8 xen-boot fail REGR. vs. 173462
 test-armhf-armhf-xl-vhd   8 xen-boot fail REGR. vs. 173462
 test-armhf-armhf-libvirt  8 xen-boot fail REGR. vs. 173462
 test-armhf-armhf-libvirt-raw  8 xen-boot fail REGR. vs. 173462
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 173462

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds  8 xen-boot fail REGR. vs. 173462

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

version targeted for testing:
 linux0d1409e4ff08aa4a9a254d3f723410db32aa7552
baseline version:
 linux9d84bb40bcb30a7fa16f33baa967aeb9953dda78

Last test of basis   173462  2022-10-07 18:41:45 Z   63 days
Failing since173470  2022-10-08 06:21:34 Z   62 days  124 attempts
Testing same since   175116  2022-12-09 05:37:10 Z0 days1 attempts


1991 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-coresched-amd64-xlpass
 test-arm64-arm64-xl  fail
 test-armhf-armhf-xl  fail
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 

Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in arch_domain_create()

[Julien Grall]

Hi Andrew,

On 09/12/2022 22:18, Andrew Cooper wrote:

On 09/12/2022 18:51, Julien Grall wrote:
That said, moving the creation side of things doesn't change the
teardown requirements.  When I get time to respin the fault_ttl series,
Gitlab CI will be able to demonstrate the bug I keep on telling you is
still there, the fix for which is taking the patch I already wrote for
you.


Is it just a matter of rebasing the series? If so, I am happy to give a 
try to see what comes up...



There is no correct way to move the final loop out of
complete_domain_destroy(), even if in the general case you can make it
more preemptible by moving the allocation later.


Hmmm... I don't think the call to p2m_teardown() in 
complete_domain_destroy() will be necessary once we move the P2M 
allocation out of arch_domain_create().


This is because there will be no use of the P2M after 
domain_relinquish_resources().


Cheers,

--
Julien Grall

Re: MISRA C Rule 20.7 disambiguation

[Stefano Stabellini]

On Fri, 9 Dec 2022, Jan Beulich wrote:
> On 09.12.2022 01:45, Stefano Stabellini wrote:
> > This patch is to start a discussion in regard to rule 20.7 and its
> > interpretation. During the last MISRA C call we discussed that "our"> 
> > interpretation of the rule means that the following two cases don't need
> > extra parenthesis:
> > 
> > #define M(a, b) func(a, b)
> > #define M(a, b) (a) = b
> 
> I'm puzzled by the latter.

Sorry that was my mistake, I misread one of the examples.


> Iirc there was discussion on whether the LHS of an assignment needs
> parentheses, but I don't think there was any question about the RHS
> wanting them, irrespective of the facts that only comma expressions
> have lower precedence than assignment ones and that evaluation goes
> right to left anyway.
> 
> One aspect speaking for parentheses even on the LHS is that an expression
> (rather than an lvalue) passed as macro argument then uniformly becomes
> invalid, i.e. not just
> 
>   M(x + y, z)
> 
> would be rejected by the compiler, but also
> 
>   M(x = y, z)

So actually it should be:

#define M(a, b) (a) = (b)


> > Let's take another example:
> > 
> > #define xzalloc_flex_struct(type, field, nr) \
> > ((type *)_xzalloc(offsetof(type, field[nr]), __alignof__(type)))
> > 
> > "type" is the same as last time. There are 2 other interesting macro
> > parameters here: nr and field.
> > 
> > nr could result in an expression, but I don't think it needs
> > parenthesis because it is between []? However, we know we have a clear
> > exception for the "," operator. We don't have a clear exception for the
> > [] operator. Do we need (nr)?
> 
> The question of whether parentheses are needed clearly need to be based
> on whether without parentheses anything that looks sensible on the surface
> can be mistaken for other than what was meant. I think [] simply are
> another form of parenthesization, even if these are commonly called square
> bracket (not parentheses). For this to be mistaken, a macro argument would
> need to be passed which first has a ] and then a [. This clearly doesn't
> look sensible even when just very briefly looking at it. Plus the same
> issue would exist with parentheses: You could also undermine the proper
> use of parentheses in the macro by passing a macro argument which first
> has ) and then (. IOW - adding parentheses here adds no value, and hence
> is merely clutter.
> 
> > field could result in an expression, so I think it needs parenthesis.
> 
> No, field (and intentionally named that way) is a struct member indicator.
> Neither p->(field) nor s.(field) are syntactically valid. There simply
> cannot be parentheses here, so the same conclusion as near the top applies.
>
> > Just to be clear, I'll list all the possible options below.
> > 
> > a) no changes needed, xzalloc_flex_struct is good as is
> 
> This is it, and not surprisingly: This construct was introduced not that
> long ago, when we already paid close attention to parenthesization needs.


Let's see if we can configure a MISRA C checking tool with the behavior
we would like, i.e. xzalloc_flex_struct not a violation. (I am using
xzalloc_flex_struct as an example.)

cppcheck marks xzalloc_flex_struct as violation but it is not surprising
as we know cppcheck is a bit rudimental. It might be better to disable
scanning for 20.7 in cppcheck.

Eclair marks it as a violation too. Eclair thinks "nr" needs
parenthesis. Roberto, we have already discussed how the comma operator
"," being the lower precedence doesn't require extra parenthesis.
Roberto, what's your take on the [] square brakets?

[PATCH v2] xen/arm: efi-boot misra rule 4.1 fix

[Stefano Stabellini]

We have 3 violations of MISRA C Rule 4.1 ("Octal and hexadecimal escape
sequences shall be terminated") in xen/arch/arm/efi/efi-boot.h. Fix them
and take the opportunity to declare them as static const __initconst and
improve the style.

Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- add static const __initconst
- add blank newline for style
---
 xen/arch/arm/efi/efi-boot.h | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 43a836c3a7..223db0c4da 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -542,7 +542,9 @@ static void __init efi_arch_handle_module(const struct file 
*file,
 
 if ( file ==  )
 {
-char ramdisk_compat[] = "multiboot,ramdisk\0multiboot,module";
+static const char __initconst ramdisk_compat[] = "multiboot,ramdisk\0"
+ "multiboot,module";
+
 node = fdt_add_subnode(fdt, chosen, "ramdisk");
 if ( node < 0 )
 blexit(L"Unable to add ramdisk FDT node.");
@@ -555,7 +557,9 @@ static void __init efi_arch_handle_module(const struct file 
*file,
 }
 else if ( file ==  )
 {
-char xsm_compat[] = "xen,xsm-policy\0multiboot,module";
+static const char __initconst xsm_compat[] = "xen,xsm-policy\0"
+ "multiboot,module";
+
 node = fdt_add_subnode(fdt, chosen, "xsm");
 if ( node < 0 )
 blexit(L"Unable to add xsm FDT node.");
@@ -568,7 +572,9 @@ static void __init efi_arch_handle_module(const struct file 
*file,
 }
 else if ( file ==  )
 {
-char kernel_compat[] = "multiboot,kernel\0multiboot,module";
+static const char __initconst kernel_compat[] = "multiboot,kernel\0"
+"multiboot,module";
+
 node = fdt_add_subnode(fdt, chosen, "kernel");
 if ( node < 0 )
 blexit(L"Unable to add dom0 FDT node.");
-- 
2.25.1

Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in arch_domain_create()

[Andrew Cooper]

On 09/12/2022 18:51, Julien Grall wrote:
> Hi Henry,
>
> On 08/12/2022 03:06, Henry Wang wrote:
>> I am trying to work on the follow-up improvements about the Arm P2M
>> code,
>> and while trying to address the comment below, I noticed there was an
>> unfinished
>> discussion between me and Julien which I would like to continue and here
>> opinions from all of you (if possible).
>>
>>> -Original Message-
>>> From: Julien Grall 
>>> Subject: Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2
>>> mapping in
>>> arch_domain_create()
> I also noticed that relinquish_p2m_mapping() is not called. This
> should
> be fine for us because arch_domain_create() should never create a
> mapping that requires p2m_put_l3_page() to be called.
>>
>> For the background of the discussion, this was about the failure path of
>> arch_domain_create(), where we only call p2m_final_teardown() which does
>> not call relinquish_p2m_mapping()...
> So all this mess with the P2M is necessary because we are mapping the
> GICv2 CPU interface in arch_domain_create(). I think we should
> consider to defer the mapping to later.
>
> Other than it makes the code simpler, it also means we could also the
> P2M root on the same numa node the domain is going to run (those
> information are passed later on).
>
> There is a couple of options:
>  1. Introduce a new hypercall to finish the initialization of a domain
> (e.g. allocating the P2M and map the GICv2 CPU interface). This option
> was briefly discussed with Jan (see [2])./
>  2. Allocate the P2M when populate the P2M pool and defer the GICv2
> CPU interface mapping until the first access (similar to how with deal
> with MMIO access for ACPI).
>
> I find the second option is neater but it has the drawback that it
> requires on more trap to the hypervisor and we can't report any
> mapping failure (which should not happen if the P2M was correctly
> sized). So I am leaning towards option 2.
>
> Any opinions?

A DOMCTL_creation_finished hypercall (name subject to improvement) is
mandatory for encrypted VM support in x86 (there's crypto needed at this
point to complete the measurement of the guest and create an attestation
report), so we are going to gain something to this effect one way or
another.

Such a hypercall also removes the giant bodge of using
domain_unpause_by_systemcontroller() for this purpose.

But it seems like ARM already has arch_domain_creation_finished() so you
can do 1) independently of adding a new hypercall.  x86 uses that hook
for hooking up the magic APICv sinc page into the p2m, so you're already
in good(?) company with a GIC bodge.


As to where the logic *should* be, it should be done in the hypercall(s)
where the toolstack is describing the guests phymap to Xen.  The fact
that these don't exist is yet another problem needing to be worked on.


That said, moving the creation side of things doesn't change the
teardown requirements.  When I get time to respin the fault_ttl series,
Gitlab CI will be able to demonstrate the bug I keep on telling you is
still there, the fix for which is taking the patch I already wrote for
you.  There is no correct way to move the final loop out of
complete_domain_destroy(), even if in the general case you can make it
more preemptible by moving the allocation later.

~Andrew

Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in arch_domain_create()

[Stefano Stabellini]

On Fri, 9 Dec 2022, Julien Grall wrote:
> Hi Henry,
> 
> On 08/12/2022 03:06, Henry Wang wrote:
> > I am trying to work on the follow-up improvements about the Arm P2M code,
> > and while trying to address the comment below, I noticed there was an
> > unfinished
> > discussion between me and Julien which I would like to continue and here
> > opinions from all of you (if possible).
> > 
> > > -Original Message-
> > > From: Julien Grall 
> > > Subject: Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in
> > > arch_domain_create()
> > > > > I also noticed that relinquish_p2m_mapping() is not called. This
> > > > > should
> > > > > be fine for us because arch_domain_create() should never create a
> > > > > mapping that requires p2m_put_l3_page() to be called.
> > 
> > For the background of the discussion, this was about the failure path of
> > arch_domain_create(), where we only call p2m_final_teardown() which does
> > not call relinquish_p2m_mapping()...
> So all this mess with the P2M is necessary because we are mapping the GICv2
> CPU interface in arch_domain_create(). I think we should consider to defer the
> mapping to later.
> 
> Other than it makes the code simpler, it also means we could also the P2M root
> on the same numa node the domain is going to run (those information are passed
> later on).
> 
> There is a couple of options:
>  1. Introduce a new hypercall to finish the initialization of a domain (e.g.
> allocating the P2M and map the GICv2 CPU interface). This option was briefly
> discussed with Jan (see [2])./
>  2. Allocate the P2M when populate the P2M pool and defer the GICv2 CPU
> interface mapping until the first access (similar to how with deal with MMIO
> access for ACPI).
> 
> I find the second option is neater but it has the drawback that it requires on
> more trap to the hypervisor and we can't report any mapping failure (which
> should not happen if the P2M was correctly sized). So I am leaning towards
> option 2.
> 
> Any opinions?

Option 1 is not great due to the extra hypercall. But I worry that
option 2 might make things harder for safety because the
mapping/initialization becomes "dynamic". I don't know if this is a
valid concern.

I would love to hear Bertrand's thoughts about it. Putting him in To:

Re: [PATCH 5/5] x86/tboot: actually wipe contexts

[Andrew Cooper]

On 06/12/2022 13:57, Jan Beulich wrote:
> Especially with our use of __builtin_memset() to implement memset() the
> compiler is free to eliminate instances when it can prove that the
> affected object is dead. Introduce a small helper function accompanying
> the memset() with a construct forcing the compiler to retain the
> clearing of (stack) memory.
>
> Fixes: c021c95498d9 ("x86: Replace our own specialised versions of memset and 
> memcpy with")
> Signed-off-by: Jan Beulich 

Thanks.  I'd noticed this before wanted to do something about it.

Acked-by: Andrew Cooper 

Re: [PATCH 4/5] x86/tboot: correct IOMMU (VT-d) interaction

[Andrew Cooper]

On 06/12/2022 13:56, Jan Beulich wrote:
> First of all using is_idle_domain() on the subject domain in the body of
> for_each_domain() is pointless. Replace that conditional by one checking
> that a domain actually has IOMMU support enabled for it, and that we're
> actually on a VT-d system (both are largely cosmetic / documentary with
> how things work elsewhere, but still).
>
> Reported-by: Andrew Cooper 
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

Re: [PATCH 3/5] x86/mm: PGC_shadowed_pt is used by shadow code only

[Andrew Cooper]

On 06/12/2022 13:54, Jan Beulich wrote:
> By defining the constant to zero when !SHADOW_PAGING we give compilers
> the chance to eliminate a little more dead code elsewhere in the tree.
> Plus, as a minor benefit, the general reference count can be one bit
> wider. (To simplify things, have PGC_shadowed_pt change places with
> PGC_extra.)
>
> Signed-off-by: Jan Beulich 

Acked-by: Andrew Cooper 

Coverity is going to complain some more.  I've still not figured out a
way to get it to ignore "AND with something that's
compile-time-conditionally-0", but we have this pattern elsewhere.

~Andrew

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

[osstest service owner]

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

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  8d30b9e32c462fdfab4207bfcd31ed63749c4f28
baseline version:
 xen  d7669c101427c1504517418e832fb760ae89e6bc

Last test of basis   175102  2022-12-08 22:02:07 Z0 days
Testing same since   175130  2022-12-09 19:03:41 Z0 days1 attempts


People who touched revisions under test:
  Michal Orzel 

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
   d7669c1014..8d30b9e32c  8d30b9e32c462fdfab4207bfcd31ed63749c4f28 -> smoke

[PATCH v2-ish] x86/boot: Relocate Xen using memcpy() directly

[Andrew Cooper]

We can relocate Xen by reading out of the virtual mapping that we're executing
on, and write directly into the directmap.  In fact, this removes one
dependency on Xen being "at 0" (the XEN_IMG_OFFSET passed as src) for
relocation to occur.

This removes all the temporary pagetable handling under the covers of
move_memory(), and results in a forward copy rather than a chunked backwards
copy (caused by move_memory() always constructing src and dst in a way to
trigger memmove() to copy backwards).

With the penultimate caller of move_memory() dropped, clean up the API.  Drop
the keep boolean, folding in 0 from the final caller, and drop the return
address which has been unused since c/s 0b76ce20de85 ("x86/setup: don't
relocate the VGA hole.") in 2007.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 

v2-ish:
 * Split out previous series.  This was the "easy to shuffle" work that still
   gets a win.  Everything else I'm going to rework differently, so will have
   to be deferred for now.
---
 xen/arch/x86/setup.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 6bb5bc7c84be..4102aae76dde 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -425,8 +425,8 @@ void *__init bootstrap_map(const module_t *mod)
 return ret;
 }
 
-static void *__init move_memory(
-uint64_t dst, uint64_t src, unsigned int size, bool keep)
+static void __init move_memory(
+uint64_t dst, uint64_t src, unsigned int size)
 {
 unsigned int blksz = BOOTSTRAP_MAP_LIMIT - BOOTSTRAP_MAP_BASE;
 unsigned int mask = (1L << L2_PAGETABLE_SHIFT) - 1;
@@ -463,13 +463,8 @@ static void *__init move_memory(
 src += sz;
 size -= sz;
 
-if ( keep )
-return size ? NULL : d + doffs;
-
 bootstrap_map(NULL);
 }
-
-return NULL;
 }
 
 #undef BOOTSTRAP_MAP_LIMIT
@@ -1277,7 +1272,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  * data until after we have switched to the relocated pagetables!
  */
 barrier();
-move_memory(e, XEN_IMG_OFFSET, _end - _start, 1);
+memcpy(__va(__pa(_start)), _start, _end - _start);
 
 /* Walk idle_pg_table, relocating non-leaf entries. */
 pl4e = __va(__pa(idle_pg_table));
@@ -1334,8 +1329,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
"1" (__va(__pa(cpu0_stack))), "2" (STACK_SIZE / 8)
 : "memory" );
 
-bootstrap_map(NULL);
-
 printk("New Xen image base address: %#lx\n", xen_phys_start);
 }
 
@@ -1361,7 +1354,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 {
 move_memory(end - size + headroom,
 (uint64_t)mod[j].mod_start << PAGE_SHIFT,
-mod[j].mod_end, 0);
+mod[j].mod_end);
 mod[j].mod_start = (end - size) >> PAGE_SHIFT;
 mod[j].mod_end += headroom;
 mod[j].reserved = 1;
-- 
2.11.0

Re: [PATCH 1/3] x86/boot: Drop pte_update_limit from physical relocation logic

[Andrew Cooper]

On 10/12/2021 07:17, Jan Beulich wrote:
> On 09.12.2021 18:34, Andrew Cooper wrote:
>> On 07/12/2021 11:37, Jan Beulich wrote:
>>> On 07.12.2021 11:53, Andrew Cooper wrote:
 --- a/xen/arch/x86/setup.c
 +++ b/xen/arch/x86/setup.c
 @@ -1230,7 +1230,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
  l3_pgentry_t *pl3e;
  l2_pgentry_t *pl2e;
  int i, j, k;
 -unsigned long pte_update_limit;
  
  /* Select relocation address. */
  xen_phys_start = end - reloc_size;
 @@ -1238,14 +1237,6 @@ void __init noreturn __start_xen(unsigned long 
 mbi_p)
  bootsym(trampoline_xen_phys_start) = xen_phys_start;
  
  /*
 - * No PTEs pointing above this address are candidates for 
 relocation.
 - * Due to possibility of partial overlap of the end of source 
 image
 - * and the beginning of region for destination image some 
 PTEs may
 - * point to addresses in range [e, e + XEN_IMG_OFFSET).
 - */
 -pte_update_limit = PFN_DOWN(e);
>>> ... considering the comment here: Isn't it 0d31d1680868 ("x86/setup: do
>>> not relocate Xen over current Xen image placement") where need for this
>>> disappeared? Afaict the non-overlap of source and destination is the
>>> crucial factor here, yet your description doesn't mention this aspect at
>>> all. I'd therefore like to ask for an adjustment there.
>> I don't consider that commit relevant.
>>
>> There is no circumstance ever where you can relocate Xen with
>> actually-overlapping ranges, because one way or another, you'd end up
>> copying non-pagetable data over the live pagetables.
> That was fragile, yes. I think I (vaguely!) recall a discussion I had
> with Keir about this, with him pointing out that the logic builds upon
> all necessary mappings being held in the TLB. If you strictly think
> that's not worthwhile to consider or mention, then so be it.

Ok, I'll tweak the commit message.  But having come back to this after
more than a year away, I think it's worth pointing out that the details
in 0d31d1680868 demonstrate that the "partial overlap" logic was indeed
buggy.

In a VM, a vcpu migration at just the wrong moment will empty the TLB
under your feet.  So will a whole slew of micro-architectural
conditions, or a poorly timed SMI. 

Whatever people have tried to call it in the past, it was broken plain
and simple.

~Andrew

Re: [PATCH 5/5] x86/tboot: actually wipe contexts

[Jason Andryuk]

On Tue, Dec 6, 2022 at 8:57 AM Jan Beulich  wrote:
>
> Especially with our use of __builtin_memset() to implement memset() the
> compiler is free to eliminate instances when it can prove that the
> affected object is dead. Introduce a small helper function accompanying
> the memset() with a construct forcing the compiler to retain the
> clearing of (stack) memory.
>
> Fixes: c021c95498d9 ("x86: Replace our own specialised versions of memset and 
> memcpy with")
> Signed-off-by: Jan Beulich 

Reviewed-by: Jason Andryuk 

Re: [PATCH 4/5] x86/tboot: correct IOMMU (VT-d) interaction

[Jason Andryuk]

On Tue, Dec 6, 2022 at 8:57 AM Jan Beulich  wrote:
>
> First of all using is_idle_domain() on the subject domain in the body of
> for_each_domain() is pointless. Replace that conditional by one checking
> that a domain actually has IOMMU support enabled for it, and that we're
> actually on a VT-d system (both are largely cosmetic / documentary with
> how things work elsewhere, but still).
>
> Reported-by: Andrew Cooper 
> Signed-off-by: Jan Beulich 

Reviewed-by: Jason Andryuk 

Re: [PATCH 1/5] x86/tboot: drop failed attempt to hash shadow page tables

[Jason Andryuk]

On Tue, Dec 6, 2022 at 8:58 AM Jan Beulich  wrote:
>
> On 06.12.2022 14:53, Jan Beulich wrote:
> > While plausible to do what was intended based on the name of the flag
> > (PGC_page_table), that name was misleading and is going to be changed.
> > It marks page tables pages _having_ a shadow, not shadows of page table
> > pages. The attempt also didn't cover the HAP case at all, and it
> > constituted a potentially very long loop doing nothing when
> > !SHADOW_PAGING. Instead leave a comment of what actually wants doing
> > there (which then also may need to account for e.g. the risk of A/D bits
> > becoming set behind our backs).
> >
> > Signed-off-by: Jan Beulich 

Reviewed-by: Jason Andryuk 

Re: [PATCH linux-next] x86/xen/time: prefer tsc as clocksource when it is invariant

[Boris Ostrovsky]

On 12/8/22 11:36 AM, Krister Johansen wrote:

+   /*
+* As Dom0 is never moved, no penalty on using TSC there.
+*
+* If the guest has invariant tsc, then set xen_clocksource rating
+* below that of the tsc so that the system prefers tsc instead.  This
+* check excludes PV domains, because PV is unable to guarantee that the
+* guest's cpuid call has been intercepted by the hypervisor.
+*/
+   if (xen_initial_domain()) {
xen_clocksource.rating = 275;
+   } else if ((xen_hvm_domain() || xen_pvh_domain()) &&
+   boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
+   boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
+   !check_tsc_unstable()) {
+   xen_clocksource.rating = 299;
+   }



What if RDTSC is intercepted?


-boris

Re: [XEN v1] xen/Arm: Probe the entry point address of an uImage correctly

[Julien Grall]

Hi Ayan,

On 09/12/2022 19:10, Ayan Kumar Halder wrote:
zImage and Image are image protocols, uImage is not. It is just a 
legacy u-boot header (no requirements
\wrt booting,memory,registers, etc.). It can be added on top of 
anything (even vmlinux or a text file).
So I guess this is why Xen states that it supports zImage/Image and 
does not mention uImage.
A header is one thing, the boot requirements are another. Supporting 
uImage is ok but if we specify

that it must be a u-boot header added on top of zImage/Image.


Let me first confine to Arm32 only.

zephyr.bin has to be loaded at a fixed address with which it has been 
built.


So, we could either use zImage header (where 'start_address' can be used 
to specify the load address).


Or uImage (where -a  is used to specify the load address).

Adding uImage header on top of zImage does not make sense.

Now for Arm64,  we do need to parse the zImage header

#ifdef CONFIG_ARM_64
     if ( info->type == DOMAIN_64BIT )
     {
     return info->mem.bank[0].start + info->zimage.text_offset;
     }
#endif

Again, adding uImage header on top of zImage header does not make sense 
as well.


Also, I believe zImage boot requirements are specific for linux kernel.


Correct. But then this is what Xen tries to adhere to when preparing the 
guest. So...



zephyr or any other RTOS may not follow the same boot requirements.


... if Zephyr or any other RTOS have different requirements, then we may 
need to modify Xen.


Can you describe the expectation of Zephyr for the following components:
  - State of the memory/cache:
* Should the image be cleaned to PoC or more?
* What about the area not part of the binary (e.g. BSS)?
* What about the rest of the memory
  - State of the co-processor registers:
* Can we call the kernel with I-cache enabled?
* ...
  - State of the general purpose registers:
* For instance, Linux expects a pointer to the device-tree in r0

Cheers,

--
Julien Grall

Re: [XEN v1] xen/Arm: Probe the entry point address of an uImage correctly

[Ayan Kumar Halder]

Hi Michal,

On 09/12/2022 08:53, Michal Orzel wrote:

On 08/12/2022 19:42, Ayan Kumar Halder wrote:

On 08/12/2022 16:53, Julien Grall wrote:

Hi,

Hi,

On 08/12/2022 15:24, Michal Orzel wrote:

On 08/12/2022 14:51, Julien Grall wrote:

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


Hi,

Title extra NIT: I have seen it multiple time and so far refrain to say
it. Please use 'arm' rather than 'Arm'. This is for consistency in the
way we name the subsystem in the title.

On 08/12/2022 12:49, Ayan Kumar Halder wrote:

Currently, kernel_uimage_probe() does not set info->zimage.start. As a
result, it contains the default value (ie 0). This causes,
kernel_zimage_place() to treat the binary (contained within uImage) as
position independent executable. Thus, it loads it at an incorrect
address.

The correct approach would be to read "uimage.ep" and set
info->zimage.start. This will ensure that the binary is loaded at the
correct address.

In non-statically allocated setup, a user doesn't know where the memory
for dom0/domU will be allocated.

So I think this was correct to ignore the address. In fact, I am worry
that...


Signed-off-by: Ayan Kumar Halder 
---

I uncovered this issue while loading Zephyr as a dom0less domU with
Xen on
R52 FVP. Zephyr builds with static device tree. Thus, the load
address is
always fixed.

    xen/arch/arm/kernel.c | 2 ++
    1 file changed, 2 insertions(+)

diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 2556a45c38..e4e8c67669 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -222,6 +222,8 @@ static int __init kernel_uimage_probe(struct
kernel_info *info,
    if ( len > size - sizeof(uimage) )
    return -EINVAL;

+    info->zimage.start = be32_to_cpu(uimage.ep);

... this will now ended up to break anyone that may have set an address
but didn't care where it should be loaded.

I also understand your use case but now, we have contradictory
approaches. I am not entirely sure how we can solve it. We may have to
break those users (Cc some folks that may use it). But we should figure
out what is the alternative for them.

If we decide to break those users, then this should be documented in
the
commit message and in docs/misc/arm/booting.txt (which interestingly
didn't mention uImage).


So the first issue with Zephyr is that it does not support zImage
protocol for arm32.
Volodymyr added support only for Image header for arm64 Zephyr.
I guess this is why Ayan, willing to boot it on Xen (arm32), decided
to add u-boot header.

If that's the only reason, then I would rather prefer if we go with
zImage for a few reasons:
  - The zImage protocol has at least some documentation (not perfect)
of the expected state of the memory/registers when jumping to the image.
  - AFAICT libxl is not (yet) supporting uImage. So this means zephyr
cannot be loaded on older Xen releases (not great).

I am exploring for a similar option as Volodymyr ie support zimage
protocol for arm32.

But for that I need some public documentation that explains the zimage
header format for arm32.

Refer xen/arch/arm/kernel.c

#define ZIMAGE32_MAGIC_OFFSET 0x24
#define ZIMAGE32_START_OFFSET 0x28
#define ZIMAGE32_END_OFFSET   0x2c
#define ZIMAGE32_HEADER_LEN   0x30

#define ZIMAGE32_MAGIC 0x016f2818

Is this documented anywhere ?

I had a look at
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm/booting.rst
, but there is nothing that explains the header format.


Note this doesn't mean we should not fix Xen for uImage.


Now, there is also a question about supporting arm64 uImage kernels.
In Xen kernel_zimage_place,
we do:
#ifdef CONFIG_ARM_64
  if ( info->type == DOMAIN_64BIT )
  return info->mem.bank[0].start + info->zimage.text_offset;
#endif

So if we modify the uImage behavior for arm32, we will break
consistency with arm64
(we would take uImage entry point address into account for arm32 but
not for arm64).
At the moment at least they are in sync.

That's a good point. It would be best if the behavior is consistent.

Currently, kernel_zimage_place() is called for both uImage and zImage.

Will it be sane if we write a different function for uImage ?

Something like this ...

static paddr_t __init kernel_uimage_place(struct kernel_info *info)

{

      /* Read and return uImage header's load address */

      return be32_to_cpu(uimage.load);

}

This will be consistent across arm32 and arm64


All of these does not make a lot of sense because we are allocating memory for 
a domain
before probing the kernel image. This means that the load/entry address for a 
kernel
must be within the bank allocated for a domain. So the kernel already needs to 
know
that it is running e.g. as a Xen domU, and add corresponding u-boot header to 
load
us at e.g. GUEST_RAM0_BASE. Otherwise Xen will fail trying to copy the kernel 
into domain's
memory. Whereas 

Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in arch_domain_create()

[Julien Grall]

Hi Henry,

On 08/12/2022 03:06, Henry Wang wrote:

I am trying to work on the follow-up improvements about the Arm P2M code,
and while trying to address the comment below, I noticed there was an unfinished
discussion between me and Julien which I would like to continue and here
opinions from all of you (if possible).


-Original Message-
From: Julien Grall 
Subject: Re: [PATCH v2] xen/arm: p2m: Populate pages for GICv2 mapping in
arch_domain_create()

I also noticed that relinquish_p2m_mapping() is not called. This should
be fine for us because arch_domain_create() should never create a
mapping that requires p2m_put_l3_page() to be called.


For the background of the discussion, this was about the failure path of
arch_domain_create(), where we only call p2m_final_teardown() which does
not call relinquish_p2m_mapping()...
So all this mess with the P2M is necessary because we are mapping the 
GICv2 CPU interface in arch_domain_create(). I think we should consider 
to defer the mapping to later.


Other than it makes the code simpler, it also means we could also the 
P2M root on the same numa node the domain is going to run (those 
information are passed later on).


There is a couple of options:
 1. Introduce a new hypercall to finish the initialization of a domain 
(e.g. allocating the P2M and map the GICv2 CPU interface). This option 
was briefly discussed with Jan (see [2])./
 2. Allocate the P2M when populate the P2M pool and defer the GICv2 CPU 
interface mapping until the first access (similar to how with deal with 
MMIO access for ACPI).


I find the second option is neater but it has the drawback that it 
requires on more trap to the hypervisor and we can't report any mapping 
failure (which should not happen if the P2M was correctly sized). So I 
am leaning towards option 2.


Any opinions?

Cheers,

[1] 
https://lore.kernel.org/xen-devel/6c07cdfc-888a-45bb-2077-528a809a6...@suse.com/


--
Julien Grall

Linux 6.0.8 generates L1TF-vulnerable PTE if Xen's PAT is modified

[Demi Marie Obenour]

If Xen is patched to use the same PAT Linux does, it appears to break
L1TF mitigations in PV Linux 6.0.8.  Linux 5.15.81 works fine.  The
symptom is that Linux fails to boot, with Xen complaining about an
L1TF-vulnerable PTE with shadow paging disabled.

Details are at https://github.com/QubesOS/qubes-issues/issues/7935.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab


signature.asc
Description: PGP signature

Re: [PATCH] xen/arm: efi-boot misra rule 4.1 fix

[Julien Grall]

Hi,

On 09/12/2022 09:04, Jan Beulich wrote:

On 09.12.2022 01:41, Stefano Stabellini wrote:

We have 3 violations of MISRA C Rule 4.1 ("Octal and hexadecimal escape
sequences shall be terminated") in xen/arch/arm/efi/efi-boot.h. Fix
them.

Signed-off-by: Stefano Stabellini 


While I certainly agree, I wonder if you don't want to correct style
(missing blank line after every one of these declarations) as well as
data placement (all three should imo be static __initconst) at the
same time.


+1. And use 'const' as well :).

Cheers,

--
Julien Grall

Re: [PATCH v2] xen/arm: Do not route NS phys timer IRQ to Xen

[Julien Grall]

Hi Michal,

On 24/11/2022 18:57, Julien Grall wrote:

On 28/10/2022 14:49, Michal Orzel wrote:

At the moment, we route NS phys timer IRQ to Xen even though it does not
make use of this timer. Xen uses hypervisor timer for itself and the
physical timer is fully emulated, hence there is nothing that can trigger
such IRQ. This means that requesting/releasing IRQ ends up as a deadcode
as it has no impact on the functional behavior, whereas the code within
a handler ends up being unreachable. This is a left over from the early
days when the CNTHP IRQ was buggy on the HW model used for testing and we
had to use the CNTP instead.

Remove the calls to {request/release}_irq for this timer as well as the
code within the handler. Since timer_interrupt handler is now only used
by the CNTHP, refactor it as follows:
  - rename it to htimer_interrupt to reflect its purpose,
  - remove the IRQ affiliation test,
  - invert the condition to avoid indented code and use unlikely,
  - improve readability by adding new lines \btw code and comments.

Keep the calls to zero the CNTP_CTL_EL0 register for sanity and also
remove the corresponding perf counter definition.

Signed-off-by: Michal Orzel 


Reviewed-by: Julien Grall 


Committed.

Cheers,

--
Julien Grall

[libvirt test] 175113: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

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

version targeted for testing:
 libvirt  8a5a4e6dbd935e985f0bc16865f176f69f368265
baseline version:
 libvirt  4b29d5c4f5c4463458c46fa4de22766251b36c43

Last test of basis   175086  2022-12-08 04:20:29 Z1 days
Testing same since   175113  2022-12-09 04:18:58 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  liqiang 
  Michal Privoznik 

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   fail
 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

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

[Anthony PERARD]

On Tue, Dec 06, 2022 at 10:18:15PM -0800, Vikram Garhwal wrote:
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index 35182ca196..868364c58d 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -630,6 +630,12 @@ const struct cmd_spec cmd_table[] = {
>"Issue a qemu monitor command to the device model of a domain",
>" ",
>  },
> +{ "dt_overlay",

Command with multiple words are using '-' instead of '_', could you
rename the command to "dt-overlay"?

> +  _dt_overlay, 0, 1,
> +  "Add/Remove a device tree overlay",
> +  "add/remove <.dtbo>"
> +  "-h print this help\n"
> +},
>  };
>  
>  const int cmdtable_len = ARRAY_SIZE(cmd_table);
> diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
> index 5518c78dc6..b5f04e2741 100644
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -1265,6 +1265,54 @@ int main_create(int argc, char **argv)
>  return 0;
>  }
>  
> +int main_dt_overlay(int argc, char **argv)
> +{
> +const char *overlay_ops = NULL;
> +const char *overlay_config_file = NULL;
> +void *overlay_dtb = NULL;
> +int rc;
> +uint8_t op;
> +int overlay_dtb_size = 0;
> +const int overlay_add_op = 1;
> +const int overlay_remove_op = 2;
> +
> +if (argc < 2) {
> +help("dt_overlay");
> +return EXIT_FAILURE;
> +}
> +
> +overlay_ops = argv[1];
> +overlay_config_file = argv[2];
> +
> +if (strcmp(overlay_ops, "add") == 0)
> +op = overlay_add_op;
> +else if (strcmp(overlay_ops, "remove") == 0)
> +op = overlay_remove_op;
> +else {
> +fprintf(stderr, "Invalid dt overlay operation\n");
> +return ERROR_FAIL;

ERROR_FAIL isn't really a value to be used when exiting the programme,
it's value is -3. It's from libxl API.

Instead, could you return EXIT_FAILURE?

> +}
> +
> +if (overlay_config_file) {
> +rc = libxl_read_file_contents(ctx, overlay_config_file,
> +  _dtb, _dtb_size);
> +
> +if (rc) {
> +fprintf(stderr, "failed to read the overlay device tree file 
> %s\n",
> +overlay_config_file);
> +free(overlay_dtb);
> +return ERROR_FAIL;
> +}
> +} else {
> +fprintf(stderr, "overlay dtbo file not provided\n");
> +return ERROR_FAIL;
> +}
> +
> +rc = libxl_dt_overlay(ctx, overlay_dtb, overlay_dtb_size, op);

Value returned by libxl_*() are going to be negative when there's an
error, so not something to be return by main(). Could you check if
there's an error and return EXIT_FAILURE instead?

> +free(overlay_dtb);
> +return rc;
> +}

Thanks,

-- 
Anthony PERARD

Re: [XEN][RFC PATCH v4 15/16] tools/libs/light: Implement new libxl functions for device tree overlay ops

[Anthony PERARD]

On Tue, Dec 06, 2022 at 10:18:14PM -0800, Vikram Garhwal wrote:
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index 2321a648a5..82fc7ce6b9 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -245,6 +245,11 @@
>   */
>  #define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1
>  
> +/**
> + * This means Device Tree Overlay is supported.
> + */
> +#define LIBXL_HAVE_DT_OVERLAY 1
> +
>  /*
>   * libxl_domain_build_info has device_model_user to specify the user to
>   * run the device model with. See docs/misc/qemu-deprivilege.txt.
> @@ -2448,6 +2453,9 @@ libxl_device_pci *libxl_device_pci_list(libxl_ctx *ctx, 
> uint32_t domid,
>  int *num);
>  void libxl_device_pci_list_free(libxl_device_pci* list, int num);
>  
> +int libxl_dt_overlay(libxl_ctx *ctx, void *overlay,
> + uint32_t overlay_size, uint8_t overlay_op);
> +

Could you guard both the LIBXL_HAVE_* macro and this prototype with "#if
arm"? Since the dt_overlay operation to libxl built on arm.

>  /*
>   * Turns the current process into a backend device service daemon
>   * for a driver domain.
> diff --git a/tools/libs/light/Makefile b/tools/libs/light/Makefile
> index 374be1cfab..2fde58246e 100644
> --- a/tools/libs/light/Makefile
> +++ b/tools/libs/light/Makefile
> @@ -111,6 +111,9 @@ OBJS-y += _libxl_types.o
>  OBJS-y += libxl_flask.o
>  OBJS-y += _libxl_types_internal.o
>  
> +# Device tree overlay is enabled only for ARM architecture.
> +OBJS-$(CONFIG_ARM) += libxl_dt_overlay.o
> +
>  ifeq ($(CONFIG_LIBNL),y)
>  CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
>  endif
> diff --git a/tools/libs/light/libxl_dt_overlay.c 
> b/tools/libs/light/libxl_dt_overlay.c
> new file mode 100644
> index 00..38cab880a0
> --- /dev/null
> +++ b/tools/libs/light/libxl_dt_overlay.c
> +#include "libxl_osdeps.h" /* must come before any other headers */
> +#include "libxl_internal.h"
> +#include 
> +#include 
> +#include 

Don't you need just xenctrl.h and not xenguest.h? (They both already are
libxl_internal.h so I'm not sure if xenguest.h is needed., but
xc_dt_overlay() is in xenctrl.h)


Thanks,

-- 
Anthony PERARD

Re: Xen Security Advisory 424 v1 (CVE-2022-42328,CVE-2022-42329) - Guests can trigger deadlock in Linux netback driver

[Ross Lagerwall]

On Thu, Dec 8, 2022 at 4:13 PM Juergen Gross  wrote:
>
> On 08.12.22 16:59, Pratyush Yadav wrote:
> >
> > Hi,
> >
> > I noticed one interesting thing about this patch but I'm not familiar
> > enough with the driver to say for sure what the right thing is.
> >
> > On Tue, Dec 06 2022, Xen.org security team wrote:
> >
> > [...]
> >>
> >>  From cfdf8fd81845734b6152b4617746c1127ec52228 Mon Sep 17 00:00:00 2001
> >> From: Juergen Gross 
> >> Date: Tue, 6 Dec 2022 08:54:24 +0100
> >> Subject: [PATCH] xen/netback: don't call kfree_skb() with interrupts 
> >> disabled
> >>
> >> It is not allowed to call kfree_skb() from hardware interrupt
> >> context or with interrupts being disabled. So remove kfree_skb()
> >> from the spin_lock_irqsave() section and use the already existing
> >> "drop" label in xenvif_start_xmit() for dropping the SKB. At the
> >> same time replace the dev_kfree_skb() call there with a call of
> >> dev_kfree_skb_any(), as xenvif_start_xmit() can be called with
> >> disabled interrupts.
> >>
> >> This is XSA-424 / CVE-2022-42328 / CVE-2022-42329.
> >>
> >> Fixes: be81992f9086 ("xen/netback: don't queue unlimited number of 
> >> packages")
> >> Reported-by: Yang Yingliang 
> >> Signed-off-by: Juergen Gross 
> >> Reviewed-by: Jan Beulich 
> >> ---
> >>   drivers/net/xen-netback/common.h| 2 +-
> >>   drivers/net/xen-netback/interface.c | 6 --
> >>   drivers/net/xen-netback/rx.c| 8 +---
> >>   3 files changed, 10 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/net/xen-netback/common.h 
> >> b/drivers/net/xen-netback/common.h
> >> index 1545cbee77a4..3dbfc8a6924e 100644
> >> --- a/drivers/net/xen-netback/common.h
> >> +++ b/drivers/net/xen-netback/common.h
> >> @@ -386,7 +386,7 @@ int xenvif_dealloc_kthread(void *data);
> >>   irqreturn_t xenvif_ctrl_irq_fn(int irq, void *data);
> >>
> >>   bool xenvif_have_rx_work(struct xenvif_queue *queue, bool test_kthread);
> >> -void xenvif_rx_queue_tail(struct xenvif_queue *queue, struct sk_buff 
> >> *skb);
> >> +bool xenvif_rx_queue_tail(struct xenvif_queue *queue, struct sk_buff 
> >> *skb);
> >>
> >>   void xenvif_carrier_on(struct xenvif *vif);
> >>
> >> diff --git a/drivers/net/xen-netback/interface.c 
> >> b/drivers/net/xen-netback/interface.c
> >> index 650fa180220f..f3f2c07423a6 100644
> >> --- a/drivers/net/xen-netback/interface.c
> >> +++ b/drivers/net/xen-netback/interface.c
> >> @@ -254,14 +254,16 @@ xenvif_start_xmit(struct sk_buff *skb, struct 
> >> net_device *dev)
> >>  if (vif->hash.alg == XEN_NETIF_CTRL_HASH_ALGORITHM_NONE)
> >>  skb_clear_hash(skb);
> >>
> >> -xenvif_rx_queue_tail(queue, skb);
> >> +if (!xenvif_rx_queue_tail(queue, skb))
> >> +goto drop;
> >> +
> >>  xenvif_kick_thread(queue);
> >>
> >>  return NETDEV_TX_OK;
> >>
> >>drop:
> >>  vif->dev->stats.tx_dropped++;
> >
> > Now tx_dropped is incremented on packet drop...
> >
> >> -dev_kfree_skb(skb);
> >> +dev_kfree_skb_any(skb);
> >>  return NETDEV_TX_OK;
> >>   }
> >>
> >> diff --git a/drivers/net/xen-netback/rx.c b/drivers/net/xen-netback/rx.c
> >> index 932762177110..0ba754ebc5ba 100644
> >> --- a/drivers/net/xen-netback/rx.c
> >> +++ b/drivers/net/xen-netback/rx.c
> >> @@ -82,9 +82,10 @@ static bool xenvif_rx_ring_slots_available(struct 
> >> xenvif_queue *queue)
> >>  return false;
> >>   }
> >>
> >> -void xenvif_rx_queue_tail(struct xenvif_queue *queue, struct sk_buff *skb)
> >> +bool xenvif_rx_queue_tail(struct xenvif_queue *queue, struct sk_buff *skb)
> >>   {
> >>  unsigned long flags;
> >> +bool ret = true;
> >>
> >>  spin_lock_irqsave(>rx_queue.lock, flags);
> >>
> >> @@ -92,8 +93,7 @@ void xenvif_rx_queue_tail(struct xenvif_queue *queue, 
> >> struct sk_buff *skb)
> >>  struct net_device *dev = queue->vif->dev;
> >>
> >>  netif_tx_stop_queue(netdev_get_tx_queue(dev, queue->id));
> >> -kfree_skb(skb);
> >> -queue->vif->dev->stats.rx_dropped++;
> >
> > ... but earlier rx_dropped was incremented.
> >
> > Which one is actually correct? This line was added by be81992f9086b
> > ("xen/netback: don't queue unlimited number of packages"), which was the
> > fix for XSA-392. I think incrementing tx_dropped is the right thing to
> > do, as was done before XSA-392 but it would be nice if someone else
> > takes a look at this as well.
>
> Yes, I think the XSA-392 patch was wrong in this regard.
>

Netback calls this rx (to-guest) traffic so rx_dropped seems better. On the
other hand, the networking stack thinks of this as tx since the packet is going
from the networking stack to the NIC driver...

Regardless, it is currently inconsistent since to-guest traffic increments
tx_dropped if it is dropped because the rx queue len is too long but it
increments rx_dropped if those same packets are dropped when they expire in the
rx queue.

I also see that the tx path (from-guest) doesn't increment any dropped counters
when it drops 

Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

On Fri, Dec 09, 2022 at 02:54:15PM +, Henry Wang wrote:
> Hi,
> 
> > -Original Message-
> > From: Demi Marie Obenour 
> > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > 
> > On Fri, Dec 09, 2022 at 07:37:53AM +, Henry Wang wrote:
> > > Hi Julien,
> > >
> > > > -Original Message-
> > > > From: Julien Grall 
> > > > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > > >
> > > > Hi,
> > > >
> > > > >>> Signed-off-by: Demi Marie Obenour 
> > > > >>
> > > > >> Acked-by: Jan Beulich 
> > > > >>
> > > > >>> Should this be included in 4.17?  It is a bug fix for a feature new 
> > > > >>> to
> > > > >>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The 
> > > > >>> code
> > > > >>> is identical to v2, but I have improved the commit message.
> > > > >>
> > > > >> It may be too late now, looking at the state of the tree. Henry, 
> > > > >> Julien?
> > > > >
> > > > > Like I said in v2, I don't object the change if you would like to 
> > > > > include
> > this
> > > > patch
> > > > > to 4.17, so if you are sure this patch is safe and want to commit it, 
> > > > > feel
> > free
> > > > to add:
> > > > >
> > > > > Release-acked-by: Henry Wang 
> > > > >
> > > > > Since we also need to commit:
> > > > > "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my
> > side
> > > > > I am no problem. Julien might have different opinion though, if Julien
> > > > object
> > > > > the change I would like to respect his opinion and leave this patch
> > > > uncommitted.
> > > >
> > > > I have committed it after SUPPORT.md. So if for some reasons we are
> > seen
> > > > any issues with Osstest, then I can tag the tree without this patch
> > >
> > > This is a great solution :)
> > >
> > > > (that said, I would rather prefer if we have staging-4.17 == 
> > > > stable-4.17).
> > >
> > > Looks like now staging-4.17 == stable-4.17 now, with this patch pushed.
> > > So we are ready to tag.
> > 
> > And it turns out that I botched the initial patch, sorry.  (I forgot to
> > handle the multiboot2 case.)
> > 
> > I understand if it is too late for stable-4.17, but it ought to make
> > stable 4.17.1 as it was simply omitted from the initial patch series.
> 
> I don't think this patch will make it today so I would suggest we still follow
> what Julien planned yesterday. Also I think this is also consistent with the
> release management guideline.

That’s okay.  Qubes can take this as an out of tree patch until it is
merged.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab


signature.asc
Description: PGP signature

[linux-5.4 test] 175106: regressions - FAIL

[osstest service owner]

flight 175106 linux-5.4 real [real]
flight 175127 linux-5.4 real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/175106/
http://logs.test-lab.xenproject.org/osstest/logs/175127/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-multivcpu 18 guest-start/debian.repeat fail in 175092 
REGR. vs. 174962

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-rtds 14 guest-start  fail in 175092 pass in 175106
 test-armhf-armhf-xl 18 guest-start/debian.repeat fail in 175092 pass in 175106
 test-armhf-armhf-xl-vhd  13 guest-start  fail in 175092 pass in 175106
 test-armhf-armhf-xl-credit2  14 guest-start  fail in 175092 pass in 175106
 test-armhf-armhf-xl-multivcpu 14 guest-start   fail pass in 175092

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds18 guest-start/debian.repeat fail REGR. vs. 174962

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

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

[Anthony PERARD]

On Tue, Dec 06, 2022 at 10:18:13PM -0800, Vikram Garhwal wrote:
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index 0c8b4c3aa7..a71bc0bb1d 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -2633,6 +2633,9 @@ int xc_livepatch_replace(xc_interface *xch, char *name, 
> uint32_t timeout, uint32
>  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
>   xen_pfn_t start_pfn, xen_pfn_t nr_pfns);
>  
> +int xc_dt_overlay(xc_interface *xch, void *overlay_fdt,
> +  uint32_t overlay_fdt_size, uint8_t overlay_op);
> +

Since xc_dt_overlay.c is only built on CONFIG_ARM=y, could you enclose
that prototype in "#if defined(__arm__) || defined(__aarch64__)"?

>  /* Compat shims */
>  #include "xenctrl_compat.h"
>  
> diff --git a/tools/libs/ctrl/Makefile.common b/tools/libs/ctrl/Makefile.common
> index 0a09c28fd3..247afbe5f9 100644
> --- a/tools/libs/ctrl/Makefile.common
> +++ b/tools/libs/ctrl/Makefile.common
> @@ -24,6 +24,7 @@ OBJS-y   += xc_hcall_buf.o
>  OBJS-y   += xc_foreign_memory.o
>  OBJS-y   += xc_kexec.o
>  OBJS-y   += xc_resource.o
> +OBJS-$(CONFIG_ARM)  += xc_dt_overlay.o
>  OBJS-$(CONFIG_X86) += xc_psr.o
>  OBJS-$(CONFIG_X86) += xc_pagetab.o
>  OBJS-$(CONFIG_Linux) += xc_linux.o
> diff --git a/tools/libs/ctrl/xc_dt_overlay.c b/tools/libs/ctrl/xc_dt_overlay.c
> new file mode 100644
> index 00..325c9ef4c0
> --- /dev/null
> +++ b/tools/libs/ctrl/xc_dt_overlay.c
> +#include "xc_bitops.h"
> +#include "xc_private.h"
> +#include 
> +#include 

Why all these headers? "xc_private.h" seems sufficient.
There isn't any bitmap operation, so xc_bitops.h seems unused.
The device tree isn't manipulated so libfdt.h seems unused.
hvm_op.h seems to be for HVMOP, not SYSCTL hypercall, xc_private.h might
already include the right header to do SYSCTL.

Thanks,

-- 
Anthony PERARD

[ovmf test] 175124: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 1ef86f12014c19e7bd6b2f008e868c61b5c71878
baseline version:
 ovmf 127e2c531556b968a51e8e2191d6e4580281856a

Last test of basis   175119  2022-12-09 07:10:49 Z0 days
Testing same since   175124  2022-12-09 14:10:43 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 

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



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/osstest/ovmf.git
   127e2c5315..1ef86f1201  1ef86f12014c19e7bd6b2f008e868c61b5c71878 -> 
xen-tested-master

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 12 debian-hvm-install fail 
pass in 175125-retest
 test-armhf-armhf-xl-credit2 18 guest-start/debian.repeat fail pass in 
175125-retest

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

RE: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Henry Wang]

Hi,

> -Original Message-
> From: Demi Marie Obenour 
> Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> 
> On Fri, Dec 09, 2022 at 07:37:53AM +, Henry Wang wrote:
> > Hi Julien,
> >
> > > -Original Message-
> > > From: Julien Grall 
> > > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > >
> > > Hi,
> > >
> > > >>> Signed-off-by: Demi Marie Obenour 
> > > >>
> > > >> Acked-by: Jan Beulich 
> > > >>
> > > >>> Should this be included in 4.17?  It is a bug fix for a feature new to
> > > >>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The 
> > > >>> code
> > > >>> is identical to v2, but I have improved the commit message.
> > > >>
> > > >> It may be too late now, looking at the state of the tree. Henry, 
> > > >> Julien?
> > > >
> > > > Like I said in v2, I don't object the change if you would like to 
> > > > include
> this
> > > patch
> > > > to 4.17, so if you are sure this patch is safe and want to commit it, 
> > > > feel
> free
> > > to add:
> > > >
> > > > Release-acked-by: Henry Wang 
> > > >
> > > > Since we also need to commit:
> > > > "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my
> side
> > > > I am no problem. Julien might have different opinion though, if Julien
> > > object
> > > > the change I would like to respect his opinion and leave this patch
> > > uncommitted.
> > >
> > > I have committed it after SUPPORT.md. So if for some reasons we are
> seen
> > > any issues with Osstest, then I can tag the tree without this patch
> >
> > This is a great solution :)
> >
> > > (that said, I would rather prefer if we have staging-4.17 == stable-4.17).
> >
> > Looks like now staging-4.17 == stable-4.17 now, with this patch pushed.
> > So we are ready to tag.
> 
> And it turns out that I botched the initial patch, sorry.  (I forgot to
> handle the multiboot2 case.)
> 
> I understand if it is too late for stable-4.17, but it ought to make
> stable 4.17.1 as it was simply omitted from the initial patch series.

I don't think this patch will make it today so I would suggest we still follow
what Julien planned yesterday. Also I think this is also consistent with the
release management guideline.

Kind regards,
Henry


> --
> Sincerely,
> Demi Marie Obenour (she/her/hers)
> Invisible Things Lab

Re: [PATCH V7 3/3] docs: Add documentation for generic virtio devices

[Anthony PERARD]

On Wed, Dec 07, 2022 at 12:50:44PM +0530, Viresh Kumar wrote:
> +=item B
> +
> +Specifies the Virtio devices to be provided to the guest.
> +
> +Each B is a comma-separated list of C
> +settings from the following list:

We should probably say something about allowing a comma in the VALUE for
"type" when the value starts with "virtio," as otherwise it might be
confusing.

> +
> +=over 4
> +
> +=item B
> +
> +Specifies the backend domain name or id, defaults to dom0.
> +
> +=item B
> +
> +Specifies the compatible string for the specific Virtio device. The same 
> will be
> +written in the Device Tree compatible property of the Virtio device. For
> +example, "type=virtio,device22" for the I2C device, whose device-tree 
> binding is
> +present here:
> +
> +L
> +
> +=item B
> +
> +Specifies the transport mechanism for the Virtio device, like "mmio" or 
> "pci".

"pci" doesn't exist, only "mmio" is allowed here at the moment. We can
change the man page later when new options become available.


Thanks,

-- 
Anthony PERARD

[PATCH v2] Input: xen-kbdfront - drop keys to shrink modalias

[Jason Andryuk]

xen kbdfront registers itself as being able to deliver *any* key since
it doesn't know what keys the backend may produce.

Unfortunately, the generated modalias gets too large and uevent creation
fails with -ENOMEM.

This can lead to gdm not using the keyboard since there is no seat
associated [1] and the debian installer crashing [2].

Trim the ranges of key capabilities by removing some BTN_* ranges.
While doing this, some neighboring undefined ranges are removed to trim
it further.

An upper limit of KEY_KBD_LCD_MENU5 is still too large.  Use an upper
limit of KEY_BRIGHTNESS_MENU.

This removes:
BTN_DPAD_UP(0x220)..BTN_DPAD_RIGHT(0x223)
Empty space 0x224..0x229

Empty space 0x28a..0x28f
KEY_MACRO1(0x290)..KEY_MACRO30(0x2ad)
KEY_MACRO_RECORD_START  0x2b0
KEY_MACRO_RECORD_STOP   0x2b1
KEY_MACRO_PRESET_CYCLE  0x2b2
KEY_MACRO_PRESET1(0x2b3)..KEY_MACRO_PRESET3(0xb5)
Empty space 0x2b6..0x2b7
KEY_KBD_LCD_MENU1(0x2b8)..KEY_KBD_LCD_MENU5(0x2bc)
Empty space 0x2bd..0x2bf
BTN_TRIGGER_HAPPY(0x2c0)..BTN_TRIGGER_HAPPY40(0x2e7)
Empty space 0x2e8..0x2ff

The modalias shrinks from 2082 to 1550 bytes.

A chunk of keys need to be removed to allow the keyboard to be used.
This may break some functionality, but the hope is these macro keys are
uncommon and don't affect any users.

[1] https://github.com/systemd/systemd/issues/22944
[2] https://lore.kernel.org/xen-devel/87o8dw52jc@vps.thesusis.net/T/

Cc: Phillip Susi 
Cc: sta...@vger.kernel.org
Signed-off-by: Jason Andryuk 
---
 drivers/input/misc/xen-kbdfront.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

v2:
Remove more keys: v1 didn't remove enough and modalias was still broken.

diff --git a/drivers/input/misc/xen-kbdfront.c 
b/drivers/input/misc/xen-kbdfront.c
index 8d8ebdc2039b..4ecb579df748 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -256,7 +256,14 @@ static int xenkbd_probe(struct xenbus_device *dev,
__set_bit(EV_KEY, kbd->evbit);
for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
__set_bit(i, kbd->keybit);
-   for (i = KEY_OK; i < KEY_MAX; i++)
+   /* In theory we want to go KEY_OK..KEY_MAX, but that grows the
+* modalias line too long.  There is a gap of buttons from
+* BTN_DPAD_UP..BTN_DPAD_RIGHT and KEY_ALS_TOGGLE is the next
+* defined. Then continue up to KEY_BRIGHTNESS_MENU as an upper
+* limit. */
+   for (i = KEY_OK; i < BTN_DPAD_UP; i++)
+   __set_bit(i, kbd->keybit);
+   for (i = KEY_ALS_TOGGLE; i <= KEY_BRIGHTNESS_MENU; i++)
__set_bit(i, kbd->keybit);
 
ret = input_register_device(kbd);
-- 
2.38.1

Re: [PATCH V7 1/3] libxl: Add support for generic virtio device

[Anthony PERARD]

On Fri, Dec 09, 2022 at 05:43:54AM +0530, Viresh Kumar wrote:
> On 08-12-22, 18:06, Anthony PERARD wrote:
> > Nit: Something like:
> > const char check[] = "virtio,device";
> > const size_t checkl = sizeof(check) - 1;
> > ... strncmp(tmp, check, checkl)...
> > (or just strncmp(tmp, check, sizeof(check)-1))
> > would avoid issue with both string "virtio,device" potentially been
> > different.
> 
> I think that is a generic problem with all the strings I am using. What about
> this diff over current patch ?
> 
> diff --git a/tools/libs/light/libxl_internal.h 
> b/tools/libs/light/libxl_internal.h
> index cdd155d925c1..a062fca0e2bb 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -166,6 +166,11 @@
>  /* Convert pfn to physical address space. */
>  #define pfn_to_paddr(x) ((uint64_t)(x) << XC_PAGE_SHIFT)
>  
> +/* Virtio device types */
> +#define VIRTIO_DEVICE_TYPE_GENERIC   "virtio,device"
> +#define VIRTIO_DEVICE_TYPE_GPIO  "virtio,device22"
> +#define VIRTIO_DEVICE_TYPE_I2C   "virtio,device29"

That a good idea.

>  /* logging */
>  _hidden void libxl__logv(libxl_ctx *ctx, xentoollog_level msglevel, int 
> errnoval,
>   const char *file /* may be 0 */, int line /* ignored if !file 
> */,
> diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c
> index 64cec989c674..183d9c906e27 100644
> --- a/tools/libs/light/libxl_virtio.c
> +++ b/tools/libs/light/libxl_virtio.c
> @@ -62,7 +62,7 @@ static int libxl__virtio_from_xenstore(libxl__gc *gc, const 
> char *libxl_path,
> libxl_device_virtio *virtio)
>  {
>  const char *be_path, *tmp = NULL;
> -int rc;
> +int rc, len = sizeof(VIRTIO_DEVICE_TYPE_GENERIC) - 1;

That `len` variable is initialized quite far away from where it's used,
so ...
>  
>  virtio->devid = devid;
>  
> @@ -112,8 +110,8 @@ static int libxl__virtio_from_xenstore(libxl__gc *gc, 
> const char *libxl_path,
>  if (rc) goto out;
>  
>  if (tmp) {

... you could declare `len` here instead.

> -if (!strncmp(tmp, "virtio,device", strlen("virtio,device"))) {
> -virtio->type = strdup(tmp);
> +if (!strncmp(tmp, VIRTIO_DEVICE_TYPE_GENERIC, len)) {
> +virtio->type = libxl__strdup(NOGC, tmp);
>  } else {
>  return ERROR_INVAL;
>  }
> 


Otherwise, those change looks fine.
Thanks,

-- 
Anthony PERARD

Re: [PATCH V6 2/3] xl: Add support to parse generic virtio device

[Anthony PERARD]

Sorry, I've replied to the wrong version, but those comment apply to V7.

Cheers,

-- 
Anthony PERARD

Re: [PATCH V6 2/3] xl: Add support to parse generic virtio device

[Anthony PERARD]

On Tue, Nov 08, 2022 at 04:53:59PM +0530, Viresh Kumar wrote:
> diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
> index 7bf26bdcd831..b188104299b1 100644
> --- a/tools/ocaml/libs/xl/genwrap.py
> +++ b/tools/ocaml/libs/xl/genwrap.py
> @@ -36,6 +36,7 @@ DEVICE_LIST =  [ ("list",   ["ctx", "domid", "t 
> list"]),
>  functions = { # ( name , [type1,type2,] )
>  "device_vfb": DEVICE_FUNCTIONS,
>  "device_vkb": DEVICE_FUNCTIONS,
> +"device_virtio": DEVICE_FUNCTIONS,
>  "device_disk":DEVICE_FUNCTIONS + DEVICE_LIST +
>[ ("insert", ["ctx", "t", "domid", 
> "?async:'a", "unit", "unit"]),
>  ("of_vdev",["ctx", "domid", "string", "t"]),
> diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c 
> b/tools/ocaml/libs/xl/xenlight_stubs.c
> index 45b8af61c74a..8e54f95da7c7 100644
> --- a/tools/ocaml/libs/xl/xenlight_stubs.c
> +++ b/tools/ocaml/libs/xl/xenlight_stubs.c
> @@ -707,6 +707,7 @@ DEVICE_ADDREMOVE(disk)
>  DEVICE_ADDREMOVE(nic)
>  DEVICE_ADDREMOVE(vfb)
>  DEVICE_ADDREMOVE(vkb)
> +DEVICE_ADDREMOVE(virtio)
>  DEVICE_ADDREMOVE(pci)
>  _DEVICE_ADDREMOVE(disk, cdrom, insert)

I don't think these ocaml changes are necessary, because they don't
build. I'm guessing those adds the ability to hotplug devices which
virtio device don't have, so function for that are missing.

> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 1b5381cef033..c6f35c069d2a 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2309,8 +2390,10 @@ void parse_config_data(const char *config_source,
>  
>  d_config->num_vfbs = 0;
>  d_config->num_vkbs = 0;
> +d_config->num_virtios = 0;
>  d_config->vfbs = NULL;
>  d_config->vkbs = NULL;
> +d_config->virtios = NULL;

These look a bit out of place, I think it would be fine to set
num_virtios and virtios just before calling parse_virtio_list(), as
array are usually initialised just before parsing the associated config
option in parse_config_data().

>  if (!xlu_cfg_get_list (config, "vfb", , 0, 0)) {
>  while ((buf = xlu_cfg_get_listitem (cvfbs, d_config->num_vfbs)) != 
> NULL) {
> @@ -2752,6 +2835,7 @@ void parse_config_data(const char *config_source,
>  }
>  
>  parse_vkb_list(config, d_config);
> +parse_virtio_list(config, d_config);
>  
>  xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat",
>  _info->xend_suspend_evtchn_compat, 0);

Thanks,

-- 
Anthony PERARD

Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT

[Demi Marie Obenour]

On Fri, Dec 09, 2022 at 07:37:53AM +, Henry Wang wrote:
> Hi Julien,
> 
> > -Original Message-
> > From: Julien Grall 
> > Subject: Re: [PATCH v3] Use EfiACPIReclaimMemory for ESRT
> > 
> > Hi,
> > 
> > >>> Signed-off-by: Demi Marie Obenour 
> > >>
> > >> Acked-by: Jan Beulich 
> > >>
> > >>> Should this be included in 4.17?  It is a bug fix for a feature new to
> > >>> 4.17, so I suspect yes, but it is ultimately up to Henry Wang.  The code
> > >>> is identical to v2, but I have improved the commit message.
> > >>
> > >> It may be too late now, looking at the state of the tree. Henry, Julien?
> > >
> > > Like I said in v2, I don't object the change if you would like to include 
> > > this
> > patch
> > > to 4.17, so if you are sure this patch is safe and want to commit it, 
> > > feel free
> > to add:
> > >
> > > Release-acked-by: Henry Wang 
> > >
> > > Since we also need to commit:
> > > "[PATCH for-4.17] SUPPORT.md: Define support lifetime" so from my side
> > > I am no problem. Julien might have different opinion though, if Julien
> > object
> > > the change I would like to respect his opinion and leave this patch
> > uncommitted.
> > 
> > I have committed it after SUPPORT.md. So if for some reasons we are seen
> > any issues with Osstest, then I can tag the tree without this patch
> 
> This is a great solution :)
> 
> > (that said, I would rather prefer if we have staging-4.17 == stable-4.17).
> 
> Looks like now staging-4.17 == stable-4.17 now, with this patch pushed.
> So we are ready to tag.

And it turns out that I botched the initial patch, sorry.  (I forgot to
handle the multiboot2 case.)

I understand if it is too late for stable-4.17, but it ought to make
stable 4.17.1 as it was simply omitted from the initial patch series.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab


signature.asc
Description: PGP signature

Re: [PATCH v8 3/3] ASoC: SOF: Fix deadlock when shutdown a frozen userspace

[Kai Vehmanen]

Hi,

On Thu, 1 Dec 2022, Ricardo Ribalda wrote:

> On Thu, 1 Dec 2022 at 14:22, 'Oliver Neukum' via Chromeos Kdump 
>  wrote:
> >
> > On 01.12.22 14:03, Ricardo Ribalda wrote:
> > > This patchset does not modify this behaviour. It simply fixes the
> > > stall for kexec().
> > >
> > > The  patch that introduced the stall:
> > > 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine drivers
> > > in .shutdown")
> >
> > That patch is problematic. I would go as far as saying that
> > it needs to be reverted.
> 
> It fixes a real issue. We have not had any complaints until we tried
> to kexec in the platform.
> I wont recommend reverting it until we have an alternative implementation.
> 
> kexec is far less common than suspend/reboot.

I've posted an alternative to ALSA list that reverts the problematic
patch and fixes the problem (the patch was originally addressing)
in a different way:

https://mailman.alsa-project.org/pipermail/alsa-devel/2022-December/209776.html

No changes outside sound/soc/ are needed with this approach.

Br, Kai

[ovmf test] 175119: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 127e2c531556b968a51e8e2191d6e4580281856a
baseline version:
 ovmf 54d81d06fc165fcb8eb832acd6a7cf644b029549

Last test of basis   175101  2022-12-08 21:43:51 Z0 days
Testing same since   175119  2022-12-09 07:10:49 Z0 days1 attempts


People who touched revisions under test:
  Ryan Afranji 

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
   54d81d06fc..127e2c5315  127e2c531556b968a51e8e2191d6e4580281856a -> 
xen-tested-master

Re: [PATCH] x86/SVM: restrict hardware SSBD update upon guest VIRT_SPEC_CTRL write

[Andrew Cooper]

On 09/12/2022 09:59, Roger Pau Monné wrote:
> On Thu, Dec 08, 2022 at 12:24:54PM +0100, Jan Beulich wrote:
>> core_set_legacy_ssbd() counts the number of times SSBD is being enabled
>> via LS_CFG on a core. This assumes that calls there only occur if the
>> state actually changes. While svm_ctxt_switch_{to,from}() conform to
>> this, guest_wrmsr() doesn't: It also calls the function when the bit
>> doesn't actually change. Extend the conditional there accordingly.
>>
>> Fixes: b2030e6730a2 ("amd/virt_ssbd: set SSBD at vCPU context switch")
>> Reported-by: Andrew Cooper 
>> Signed-off-by: Jan Beulich 
>> ---
>> This is the less intrusive but more fragile variant of a fix. The
>> alternative would be to have core_set_legacy_ssbd() record per-thread
>> state, such that the necessary checking can be done there.
> Hm, yes, it's going to take a bit more of memory to keep track of
> this.

It shouldn't.  Turn the count field into a bitmap of thread_ids.

The interface to this functionality should be WRMSR-like, and should
support repeated writes of the same value.  Anything else is a timebomb
which has already gone off once...

I'll have a play with this while looking into the repro I've got.

~Andrew

Re: [PATCH 1/3] acpi/processor: fix evaluating _PDC method when running as Xen dom0

[Roger Pau Monné]

On Fri, Dec 02, 2022 at 06:06:26PM +0100, Rafael J. Wysocki wrote:
> On Fri, Dec 2, 2022 at 5:37 PM Roger Pau Monné  wrote:
> >
> > On Fri, Dec 02, 2022 at 08:17:56AM -0800, Dave Hansen wrote:
> > > On 12/2/22 04:24, Roger Pau Monné wrote:
> > > > On the implementation side, is the proposed approach acceptable?
> > > > Mostly asking because it adds Xen conditionals to otherwise generic
> > > > ACPI code.
> > >
> > > That's a good Rafael question.
> 
> Sorry for joining late, but first off _PDC has been deprecated since
> ACPI 3.0 (2004) and it is not even present in ACPI 6.5 any more.
> 
> It follows from your description that _PDC is still used in the field,
> though, after 18 years of deprecation.  Who uses it, if I may know?

I saw this issue on a Sapphire Rapids SDP from Intel, but I would
think there are other platforms affected.

> > > But, how do other places in the ACPI code handle things like this?
> >
> > Hm, I don't know of other places in the Xen case, the only resource
> > in ACPI AML tables managed by Xen are Processor objects/devices AFAIK.
> > The rest of devices are fully managed by the dom0 guest.
> >
> > I think such special handling is very specific to Xen, but maybe I'm
> > wrong and there are similar existing cases in ACPI code already.
> >
> > We could add some kind of hook (iow: a function pointer in some struct
> > that could be filled on a implementation basis?) but I didn't want
> > overengineering this if adding a conditional was deemed OK.
> 
> What _PDC capabilities specifically do you need to pass to the
> firmware for things to work correctly?

I'm not sure what capabilities do I need to pass explicitly to _PSD,
but the call to _PDC as done by Linux currently changes the reported
_PSD Coordination Field (vs not doing the call).

Thanks, Roger.

Re: [PATCH] x86/SVM: restrict hardware SSBD update upon guest VIRT_SPEC_CTRL write

[Jan Beulich]

On 09.12.2022 10:59, Roger Pau Monné wrote:
> On Thu, Dec 08, 2022 at 12:24:54PM +0100, Jan Beulich wrote:
>> --- a/xen/arch/x86/msr.c
>> +++ b/xen/arch/x86/msr.c
>> @@ -699,12 +699,16 @@ int guest_wrmsr(struct vcpu *v, uint32_t
>>  }
>>  else
> 
> I think you could turn this into an `else if` and check if the new
> value and the current one differ on the SSBD bit?

I'd prefer not to: Keeping it as I have it will likely reduce code churn
if a 2nd bit wants supporting in that MSR.

> Provided it fixes the issue:
> 
> Acked-by: Roger Pau Monné 

Thanks, but I'm a little puzzled by the constraint: Imo even if this
doesn't address the observed issue, it still fixes one aspect of wrong
behavior here. The sole difference then would be that the Reported-by:
would go away.

Jan

Re: [PATCH] x86/SVM: restrict hardware SSBD update upon guest VIRT_SPEC_CTRL write

[Roger Pau Monné]

On Thu, Dec 08, 2022 at 12:24:54PM +0100, Jan Beulich wrote:
> core_set_legacy_ssbd() counts the number of times SSBD is being enabled
> via LS_CFG on a core. This assumes that calls there only occur if the
> state actually changes. While svm_ctxt_switch_{to,from}() conform to
> this, guest_wrmsr() doesn't: It also calls the function when the bit
> doesn't actually change. Extend the conditional there accordingly.
> 
> Fixes: b2030e6730a2 ("amd/virt_ssbd: set SSBD at vCPU context switch")
> Reported-by: Andrew Cooper 
> Signed-off-by: Jan Beulich 
> ---
> This is the less intrusive but more fragile variant of a fix. The
> alternative would be to have core_set_legacy_ssbd() record per-thread
> state, such that the necessary checking can be done there.

Hm, yes, it's going to take a bit more of memory to keep track of
this.

> This wants properly testing on affected hardware. From Andrew's
> description it's also not clear whether this really is addressing that
> problem, or yet another one in this same area.
> 
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -699,12 +699,16 @@ int guest_wrmsr(struct vcpu *v, uint32_t
>  }
>  else

I think you could turn this into an `else if` and check if the new
value and the current one differ on the SSBD bit?

Provided it fixes the issue:

Acked-by: Roger Pau Monné 

Thanks, Roger.

[xen-4.16-testing test] 175099: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  4ad5975d4e35635f03d2cb9e86292c0daeabd75f
baseline version:
 xen  

Re: [PATCH] xen/arm: efi-boot misra rule 4.1 fix

[Luca Fancellu]

> On 9 Dec 2022, at 00:41, Stefano Stabellini  wrote:
> 
> We have 3 violations of MISRA C Rule 4.1 ("Octal and hexadecimal escape
> sequences shall be terminated") in xen/arch/arm/efi/efi-boot.h. Fix
> them.
> 
> Signed-off-by: Stefano Stabellini 

Hi Stefano,

I’ve checked also with coverity and it solves the violation.

Reviewed-by: Luca Fancellu 

Re: [PATCH] drivers/xen/hypervisor: Expose Xen SIF flags to userspace

[Juergen Gross]

On 02.12.22 19:22, Per Bilse wrote:

/proc/xen is a legacy pseudo filesystem which predates Xen support
getting merged into Linux.  It has largely been replaced with more
normal locations for data (/sys/hypervisor/ for info, /dev/xen/ for
user devices).  We want to compile xenfs support out of the dom0 kernel.

There is one item which only exists in /proc/xen, namely
/proc/xen/capabilities with "control_d" being the signal of "you're in
the control domain".  This ultimately comes from the SIF flags provided
at VM start.

This patch exposes all SIF flags in /sys/hypervisor/start_flags/ as
boolean files, one for each bit, returning '1' if set, '0' otherwise.
Two known flags, 'privileged' and 'initdomain', are explicitly named,
and all remaining flags can be accessed via generically named files,
as suggested by Andrew Cooper.

This patch replaces previous suggestion for SIF flags exposure in its
entirety.

Signed-off-by: Per Bilse 
---
  Documentation/ABI/stable/sysfs-hypervisor-xen |  8 +++
  drivers/xen/sys-hypervisor.c  | 70 +--
  2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-hypervisor-xen 
b/Documentation/ABI/stable/sysfs-hypervisor-xen
index 748593c64568..f52f98548184 100644
--- a/Documentation/ABI/stable/sysfs-hypervisor-xen
+++ b/Documentation/ABI/stable/sysfs-hypervisor-xen
@@ -120,3 +120,11 @@ Contact:   xen-devel@lists.xenproject.org
  Description:  If running under Xen:
The Xen version is in the format .
This is the  part of it.
+
+What:  /sys/hypervisor/start_flags/*
+Date:  December 2022
+KernelVersion: 6.1.0
+Contact:   xen-devel@lists.xenproject.org
+Description:   If running under Xen:
+   All bits in Xen's start-flags are represented as
+   boolean files, returning '1' if set, '0' otherwise.


I think at least the files which want to be used by e.g. systemd
("initdomain" as replacement for the "control_d" string in capabilities,
but I think "privileged" as well) should be explicitly added to this
description, as those are meant to be used as a stable ABI.


diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
index fcb0792f090e..e15d3ff2c56f 100644
--- a/drivers/xen/sys-hypervisor.c
+++ b/drivers/xen/sys-hypervisor.c
@@ -31,7 +31,10 @@ struct hyp_sysfs_attr {
struct attribute attr;
ssize_t (*show)(struct hyp_sysfs_attr *, char *);
ssize_t (*store)(struct hyp_sysfs_attr *, const char *, size_t);
-   void *hyp_attr_data;
+   union {
+   void *hyp_attr_data;
+   unsigned long hyp_attr_value;
+   };
  };
  
  static ssize_t type_show(struct hyp_sysfs_attr *attr, char *buffer)

@@ -399,6 +402,61 @@ static int __init xen_sysfs_properties_init(void)
return sysfs_create_group(hypervisor_kobj, _properties_group);
  }
  
+#define FLAG_UNAME "unknown"

+#define FLAG_UNAME_FMT FLAG_UNAME "%02u"
+#define FLAG_UNAME_MAX sizeof(FLAG_UNAME "XX")
+#define FLAG_COUNT (sizeof(xen_start_flags) * BITS_PER_BYTE)
+static_assert(sizeof(xen_start_flags)
+   <= sizeof_field(struct hyp_sysfs_attr, hyp_attr_value));
+
+static ssize_t flag_show(struct hyp_sysfs_attr *attr, char *buffer)
+{
+   char *p = buffer;
+
+   *p++ = '0' + ((xen_start_flags & attr->hyp_attr_value) != 0);
+   *p++ = '\n';
+   return p - buffer;
+}
+
+/*
+* Add new, known flags here.  No other changes are required, but
+* note that each known flag wastes one entry in flag_unames[].
+* The code/complexity machinations to avoid this isn't worth it
+* for a few entries, but keep it in mind.
+*/
+static struct hyp_sysfs_attr flag_attrs[FLAG_COUNT] = {
+   [ilog2(SIF_PRIVILEGED)] = {
+   .attr = { .name = "privileged", .mode = 0444 },
+   .show = flag_show,
+   .hyp_attr_value = SIF_PRIVILEGED
+   },


What about:

#define FLAG_NODE(bit, node)\
[ilog2(bit)] = {\
.attr = { .name = #node, .mode = 0444 },\
.show = flag_show,  \
.hyp_attr_value = bit   \
}

FLAG_NODE(SIF_PRIVILEGED, privileged),
FLAG_NODE(SIF_INITDOMAIN, initdomain)


+   [ilog2(SIF_INITDOMAIN)] = {
+   .attr = { .name = "initdomain", .mode = 0444 },
+   .show = flag_show,
+   .hyp_attr_value = SIF_INITDOMAIN
+   }


In order to avoid a consumer having to look into the sources for any other
set flag, maybe add names for currently defined flags, too? Or just skip
the other flags and add a single additional node ("flags"?) with the whole
value of xen_start_flags either in hex or binary form?

Please note that this is a suggestion only, I'm not insisting on it.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Re: [PATCH] xen/arm: efi-boot misra rule 4.1 fix

[Jan Beulich]

On 09.12.2022 01:41, Stefano Stabellini wrote:
> We have 3 violations of MISRA C Rule 4.1 ("Octal and hexadecimal escape
> sequences shall be terminated") in xen/arch/arm/efi/efi-boot.h. Fix
> them.
> 
> Signed-off-by: Stefano Stabellini 

While I certainly agree, I wonder if you don't want to correct style
(missing blank line after every one of these declarations) as well as
data placement (all three should imo be static __initconst) at the
same time.

Jan

> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -542,7 +542,7 @@ static void __init efi_arch_handle_module(const struct 
> file *file,
>  
>  if ( file ==  )
>  {
> -char ramdisk_compat[] = "multiboot,ramdisk\0multiboot,module";
> +char ramdisk_compat[] = "multiboot,ramdisk\0" "multiboot,module";
>  node = fdt_add_subnode(fdt, chosen, "ramdisk");
>  if ( node < 0 )
>  blexit(L"Unable to add ramdisk FDT node.");
> @@ -555,7 +555,7 @@ static void __init efi_arch_handle_module(const struct 
> file *file,
>  }
>  else if ( file ==  )
>  {
> -char xsm_compat[] = "xen,xsm-policy\0multiboot,module";
> +char xsm_compat[] = "xen,xsm-policy\0" "multiboot,module";
>  node = fdt_add_subnode(fdt, chosen, "xsm");
>  if ( node < 0 )
>  blexit(L"Unable to add xsm FDT node.");
> @@ -568,7 +568,7 @@ static void __init efi_arch_handle_module(const struct 
> file *file,
>  }
>  else if ( file ==  )
>  {
> -char kernel_compat[] = "multiboot,kernel\0multiboot,module";
> +char kernel_compat[] = "multiboot,kernel\0" "multiboot,module";
>  node = fdt_add_subnode(fdt, chosen, "kernel");
>  if ( node < 0 )
>  blexit(L"Unable to add dom0 FDT node.");
> 

Re: MISRA C Rule 20.7 disambiguation

[Jan Beulich]

On 09.12.2022 01:45, Stefano Stabellini wrote:
> This patch is to start a discussion in regard to rule 20.7 and its
> interpretation. During the last MISRA C call we discussed that "our"> 
> interpretation of the rule means that the following two cases don't need
> extra parenthesis:
> 
> #define M(a, b) func(a, b)
> #define M(a, b) (a) = b

I'm puzzled by the latter. Iirc there was discussion on whether the LHS
of an assignment needs parentheses, but I don't think there was any
question about the RHS wanting them, irrespective of the facts that only
comma expressions have lower precedence than assignment ones and that
evaluation goes right to left anyway.

One aspect speaking for parentheses even on the LHS is that an expression
(rather than an lvalue) passed as macro argument then uniformly becomes
invalid, i.e. not just

M(x + y, z)

would be rejected by the compiler, but also

M(x = y, z)

.

> Moreover, MISRA C states that parenthesis should be added when the
> expansion of a MACRO parameters would result in an *expression*.
> 
> Expression is the important word. Looking at this *compliant* example
> from the manual:
> 
> #define GET_MEMBER( S, M ) ( S ).M
> 
> It is compliant because S results in an expression so it needs
> parenthesis, while M does not, so it doesn't need parenthesis.
> 
> My understanding is the following:
> - is it possible to pass an expression as a parameter of the MACRO?
> - if yes -> need parenthesis
> - if no  -> doesn't need parenthesis
> 
> 
> As an example, cppcheck reports the following (from xmalloc.h) as
> violation:
> 
> #define xmalloc_array(_type, _num) \
> ((_type *)_xmalloc_array(sizeof(_type), __alignof__(_type), _num))
> 
> I think this is a false positive. We have already enstablished that the
> "," operator doesn't require parenthesis, so "_num" is not the problem.
> And there is no way that adding parenthesis to "type" would allow an
> expression to be passed as the type argument.

"Allow" (here and elsewhere) is probably not a good word. You can pass
_anything_ to a macro. The question is whether the macro expansion
would yield something sensible. And another question is whether with
parentheses added the result actually still compiles when the macro is
used as intended. The 2nd aspect is relevant here - you cannot add
parentheses like this: ((_type)*) - this isn't a well formed cast
anymore, and the compiler will complain. _If_ this is what cppcheck is
complaining about, then this imo is a pretty clear bug in the tool.

> Let's take another example:
> 
> #define xzalloc_flex_struct(type, field, nr) \
> ((type *)_xzalloc(offsetof(type, field[nr]), __alignof__(type)))
> 
> "type" is the same as last time. There are 2 other interesting macro
> parameters here: nr and field.
> 
> nr could result in an expression, but I don't think it needs
> parenthesis because it is between []? However, we know we have a clear
> exception for the "," operator. We don't have a clear exception for the
> [] operator. Do we need (nr)?

The question of whether parentheses are needed clearly need to be based
on whether without parentheses anything that looks sensible on the surface
can be mistaken for other than what was meant. I think [] simply are
another form of parenthesization, even if these are commonly called square
bracket (not parentheses). For this to be mistaken, a macro argument would
need to be passed which first has a ] and then a [. This clearly doesn't
look sensible even when just very briefly looking at it. Plus the same
issue would exist with parentheses: You could also undermine the proper
use of parentheses in the macro by passing a macro argument which first
has ) and then (. IOW - adding parentheses here adds no value, and hence
is merely clutter.

> field could result in an expression, so I think it needs parenthesis.

No, field (and intentionally named that way) is a struct member indicator.
Neither p->(field) nor s.(field) are syntactically valid. There simply
cannot be parentheses here, so the same conclusion as near the top applies.

> Just to be clear, I'll list all the possible options below.
> 
> a) no changes needed, xzalloc_flex_struct is good as is

This is it, and not surprisingly: This construct was introduced not that
long ago, when we already paid close attention to parenthesization needs.

Jan

> b) only "field" needs parenthesis
> c) only "nr" needs parenthesis
> d) both "field" and "nr" need parenthesis
> 
> Option d) would look like this:
> 
> #define xzalloc_flex_struct(type, field, nr) \
> ((type *)_xzalloc(offsetof(type, (field)[(nr)]), __alignof__(type)))
> 
> What do you guys think?
> 
> Cheers,
> 
> Stefano

Re: [XEN v1] xen/Arm: Probe the entry point address of an uImage correctly

[Michal Orzel]

On 08/12/2022 19:42, Ayan Kumar Halder wrote:
> 
> On 08/12/2022 16:53, Julien Grall wrote:
>> Hi,
> Hi,
>>
>> On 08/12/2022 15:24, Michal Orzel wrote:
>>> On 08/12/2022 14:51, Julien Grall wrote:
 Caution: This message originated from an External Source. Use proper 
 caution when opening attachments, clicking links, or responding.


 Hi,

 Title extra NIT: I have seen it multiple time and so far refrain to say
 it. Please use 'arm' rather than 'Arm'. This is for consistency in the
 way we name the subsystem in the title.

 On 08/12/2022 12:49, Ayan Kumar Halder wrote:
> Currently, kernel_uimage_probe() does not set info->zimage.start. As a
> result, it contains the default value (ie 0). This causes,
> kernel_zimage_place() to treat the binary (contained within uImage) as
> position independent executable. Thus, it loads it at an incorrect 
> address.
>
> The correct approach would be to read "uimage.ep" and set
> info->zimage.start. This will ensure that the binary is loaded at the
> correct address.

 In non-statically allocated setup, a user doesn't know where the memory
 for dom0/domU will be allocated.

 So I think this was correct to ignore the address. In fact, I am worry
 that...

>
> Signed-off-by: Ayan Kumar Halder 
> ---
>
> I uncovered this issue while loading Zephyr as a dom0less domU with 
> Xen on
> R52 FVP. Zephyr builds with static device tree. Thus, the load 
> address is
> always fixed.
>
>    xen/arch/arm/kernel.c | 2 ++
>    1 file changed, 2 insertions(+)
>
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 2556a45c38..e4e8c67669 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -222,6 +222,8 @@ static int __init kernel_uimage_probe(struct 
> kernel_info *info,
>    if ( len > size - sizeof(uimage) )
>    return -EINVAL;
>
> +    info->zimage.start = be32_to_cpu(uimage.ep);
 ... this will now ended up to break anyone that may have set an address
 but didn't care where it should be loaded.

 I also understand your use case but now, we have contradictory
 approaches. I am not entirely sure how we can solve it. We may have to
 break those users (Cc some folks that may use it). But we should figure
 out what is the alternative for them.

 If we decide to break those users, then this should be documented in 
 the
 commit message and in docs/misc/arm/booting.txt (which interestingly
 didn't mention uImage).

>>> So the first issue with Zephyr is that it does not support zImage 
>>> protocol for arm32.
>>> Volodymyr added support only for Image header for arm64 Zephyr.
>>> I guess this is why Ayan, willing to boot it on Xen (arm32), decided 
>>> to add u-boot header.
>>
>> If that's the only reason, then I would rather prefer if we go with 
>> zImage for a few reasons:
>>  - The zImage protocol has at least some documentation (not perfect) 
>> of the expected state of the memory/registers when jumping to the image.
>>  - AFAICT libxl is not (yet) supporting uImage. So this means zephyr 
>> cannot be loaded on older Xen releases (not great).
> 
> I am exploring for a similar option as Volodymyr ie support zimage 
> protocol for arm32.
> 
> But for that I need some public documentation that explains the zimage 
> header format for arm32.
> 
> Refer xen/arch/arm/kernel.c
> 
> #define ZIMAGE32_MAGIC_OFFSET 0x24
> #define ZIMAGE32_START_OFFSET 0x28
> #define ZIMAGE32_END_OFFSET   0x2c
> #define ZIMAGE32_HEADER_LEN   0x30
> 
> #define ZIMAGE32_MAGIC 0x016f2818
> 
> Is this documented anywhere ?
> 
> I had a look at 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm/booting.rst
>  
> , but there is nothing that explains the header format.
> 
>>
>> Note this doesn't mean we should not fix Xen for uImage.
>>
>>> Now, there is also a question about supporting arm64 uImage kernels. 
>>> In Xen kernel_zimage_place,
>>> we do:
>>> #ifdef CONFIG_ARM_64
>>>  if ( info->type == DOMAIN_64BIT )
>>>  return info->mem.bank[0].start + info->zimage.text_offset;
>>> #endif
>>>
>>> So if we modify the uImage behavior for arm32, we will break 
>>> consistency with arm64
>>> (we would take uImage entry point address into account for arm32 but 
>>> not for arm64).
>>> At the moment at least they are in sync.
>>
>> That's a good point. It would be best if the behavior is consistent.
> 
> Currently, kernel_zimage_place() is called for both uImage and zImage.
> 
> Will it be sane if we write a different function for uImage ?
> 
> Something like this ...
> 
> static paddr_t __init kernel_uimage_place(struct kernel_info *info)
> 
> {
> 
>      /* Read and return uImage header's load address */
> 
>      return be32_to_cpu(uimage.load);
> 
> }
> 
> This will be