Re: Xens handling of MCE

[Jan Beulich]

On 06.09.2023 23:38, Elliott Mitchell wrote:
> On Thu, Aug 31, 2023 at 07:52:05PM +, Development wrote:
>>
>> However, in 2009-02, "cegger" wrote MCA/MCE_in_Xen, a proposal for 
>> having xen start checking the information
>> Xen started accessing the EDAC information (now called "MCE") at some 
>> point after that, which blocks the linux kernel in dom0 from accessing it.
>> (I also found what appears to be related sides from a presentation from 
>> 2012 at: 
>> https://lkml.iu.edu/hypermail/linux/kernel/1206.3/01304/xen_vMCE_design_%28v0_2%29.pdf
>>  )
>>
> 
> I hadn't seen that before.  Clearly shows someone who had no idea what
> they were doing designed it.  The author was thinking "virtualize 
> everything!", whereas MCE is a perfect situation for paravirtualization.
> Let Dom0 process MCE events (which allows use of Linux's more up to date
> MCE drivers), then let Dom0 notify Xen if action is needed (a page was
> corrupted, tell the effected domain).
> 
> There was a recent proposal to simply import Linux's rather more recent
> MCE/EDAC source.  This hasn't happened yet.  For people using Xen this
> has been a very concerning issue for some time.

I'm unaware of such a proposal; do you have a reference? EDAC drivers
typically are vendor- or even chipset-specific aiui. At least the latter
wouldn't make them a good fit to import into Xen. Along of what you say
earlier, they instead want to become Xen-aware (to deal with address
translation as necessary). That'll also have better chances of things
staying up-to-date.

Jan

Re: [XEN PATCH] x86/ACPI: Ignore entries with invalid APIC IDs when parsing MADT

[Jan Beulich]

On 06.09.2023 22:49, Stefano Stabellini wrote:
> On Fri, 1 Sep 2023, Jan Beulich wrote:
>> On 07.08.2023 11:38, Simon Gaiser wrote:
>>> It seems some firmwares put dummy entries in the ACPI MADT table for non
>>> existing processors. On my NUC11TNHi5 those have the invalid APIC ID
>>> 0xff. Linux already has code to handle those cases both in
>>> acpi_parse_lapic [1] as well as in acpi_parse_x2apic [2]. So add the
>>> same check to Xen.
>>>
>>> Note that on some older (2nd gen Core i) laptop of mine I also saw dummy
>>> entries with a valid APIC ID. Linux would still ignore those because
>>> they have !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE. But in Xen
>>> this check is only active for madt_revision >= 5. But since this version
>>> check seems to be intentionally I leave that alone.
>>>
>>> Link: 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3bf1dbe64b62a2058dd1944c00990df203e8e7a
>>>  # [1]
>>> Link: 
>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=10daf10ab154e31237a8c07242be3063fb6a9bf4
>>>  # [2]
>>> Signed-off-by: Simon Gaiser 
>>
>> This patch was committed with unaddressed review comments. The normal action
>> in such a case would be to revert, expecting a v2 to arrive. One alternative
>> here would be a timely incremental patch submission. Another alternative,
>> considering in particular Thomas's most recent reply, would be to properly
>> downgrade CPU hotplug support in SUPPORT.md (with a corresponding entry in
>> CHANGELOG.md).
> 
> I am in favor of downgrading physical CPU hotplug support in
> SUPPORT.md.
> 
> I noticed that there is no entry for physical CPU hotplug support in
> SUPPORT.md today. Should we assume that it is not supported already as
> it is not listed as supported?

Hmm, I see

## Host hardware support

### Physical CPU Hotplug

Status, x86: Supported

pretty close to the top of the file.

> Specifically, would it be a good idea to add a sentence at the top of
> the file saying that anything not explicitly listed is not supported?

Iirc that was the plan to do for 4.18, but then we need to be sure that
things don't unintentionally become unsupported. I've no clear idea how
this plan was meant to be carried out, though.

Jan

Re: [PATCH v2] docs/misra: add 14.3

[Jan Beulich]

On 07.09.2023 03:21, Stefano Stabellini wrote:
> On Thu, 31 Aug 2023, Jan Beulich wrote:
>> On 31.08.2023 03:53, Stefano Stabellini wrote:
>>> @@ -332,6 +333,17 @@ maintainers if you want to suggest a change.
>>>   - A loop counter shall not have essentially floating type
>>>   -
>>>  
>>> +   * - `Rule 14.3 
>>> `_
>>> + - Required
>>> + - Controlling expressions shall not be invariant
>>> + - Due to the extensive usage of IS_ENABLED, sizeof compile-time
>>> +   checks, and other constructs that are detected as errors by MISRA
>>> +   C scanners, managing the configuration of a MISRA C scanner for
>>> +   this rule would be unmanageable. Thus, this rule is adopted with
>>> +   a project-wide deviation on if and ?: statements.
>>
>> As before - switch(sizeof(...)) and alike also wants deviating, imo.
> 
> I added the switch, but without the words "and alike" because it is not
> clear in this context what it would allow.

Well, what I mean here are expressions involving sizeof() (or e.g also
offsetof()).

Jan

Re: [XEN PATCH] xen/types: address Rule 10.1 for macro BITS_TO_LONGS

[Jan Beulich]

On 07.09.2023 03:33, Stefano Stabellini wrote:
> On Wed, 6 Sep 2023, Jan Beulich wrote:
>> On 06.09.2023 17:57, Nicola Vetrini wrote:
>>> On 05/09/2023 10:33, Jan Beulich wrote:
 On 05.09.2023 10:20, Nicola Vetrini wrote:
> On 05/09/2023 09:46, Jan Beulich wrote:
>> On 05.09.2023 09:31, Nicola Vetrini wrote:
>>> Given its use in the declaration
>>> 'DECLARE_BITMAP(features, IOMMU_FEAT_count)' the argument
>>> 'bits' has essential type 'enum iommu_feature', which is not
>>> allowed by the Rule as an operand to the addition operator.
>>> Given that its value can be represented by a signed integer,
>>> the explicit cast resolves the violation.
>>
>> Wait - why would this lead to a change to BITS_TO_LONGS()? And if 
>> that
>> was to be changed, why plain int? I don't think negative input makes
>> sense there, and in principle I'd expect values beyond 4 billion to
>> also be permissible (even if likely no such use will ever appear in a
>> DECLARE_BITMAP(), but elsewhere it may make sense). Even going to
>> "unsigned long" may be too limiting ...
>>
>
> You have a point. I can think of doing it like this:
> DECLARE_BITMAP(features, (int)IOMMU_FEAT_count)
> 
> I think this is a good solution for this case (even more so if we can't
> find a better implementation of BITS_TO_LONGS)
> 
> 
> on the grounds that the enum constant is representable in an int, and 
> it
> does not seem likely
> to get much bigger.
> Having an unsigned cast requires making the whole expression
> essentially unsigned, otherwise Rule 10.4 is violated because
> BITS_PER_LONG is
> essentially signed. This can be done, but it depends on how
> BITS_TO_LONGS will be/is used.

 It'll need looking closely, yes, but I expect that actually wants to be 
 an
 unsigned constant. I wouldn't be surprised if some use of 
 DECLARE_BITMAP()
 appeared (or already existed) where the 2nd argument involves sizeof() 
 in
 some way.

>>>
>>> I think there's one with ARRAY_SIZE. In my opinion this can be resolved 
>>> as follows:
>>>
>>> #define BYTES_PER_LONG (1U << LONG_BYTEORDER) // the essential type gets 
>>> from signed to unsigned
>>>
>>> #define BITS_TO_LONGS(bits) \
>>>  (((unsigned long long)(bits)+BITS_PER_LONG-1U)/BITS_PER_LONG) // 
>>> same here
>>
>> Except, as said before, I consider any kind of cast on "bits" latently
>> problematic.
> 
> Can't we just do this (same but without the cast):
> 
> #define BYTES_PER_LONG (1U << LONG_BYTEORDER)
> #define BITS_TO_LONGS(bits) \
>  (((bits)+BITS_PER_LONG-1U)/BITS_PER_LONG)

Right, that's what I was expecting we'd use (with blanks suitably added of
course).

Jan

> Then we just need to make sure to pass an unsigned to BITS_TO_LONGS. In
> the case above we would do:
> 
> DECLARE_BITMAP(features, (unsigned int)IOMMU_FEAT_count)
> 

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

[osstest service owner]

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

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  b2dd946ece74e2b6e0601f28caef72f4f9950102
baseline version:
 xen  d8c3ff585b9fd10200b05aca77139de2789d0517

Last test of basis   182674  2023-09-06 14:00:33 Z0 days
Testing same since   182709  2023-09-07 02:00:31 Z0 days1 attempts


People who touched revisions under test:
  Jens Wiklander 
  Michal Orzel 
  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
   d8c3ff585b..b2dd946ece  b2dd946ece74e2b6e0601f28caef72f4f9950102 -> smoke

[xen-4.15-testing test] 182658: tolerable trouble: fail/pass/starved - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-migrupgrade 10 xen-install/src_host fail in 182633 pass in 
182658
 test-amd64-i386-migrupgrade 11 xen-install/dst_host fail in 182633 pass in 
182658
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail in 
182633 pass in 182658
 test-amd64-amd64-xl-credit1  20 guest-localmigrate/x10 fail pass in 182633

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

[libvirt test] 182642: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 libvirt  abecd6633e2b5c191080b6838c4c7658af3fddd8
baseline version:
 libvirt  e95b81c2fd62d700f697349964b8b2f63e44c64b

Last test of basis   182629  2023-09-05 04:18:53 Z1 days
Testing same since   182642  2023-09-06 04:20:16 Z0 days1 attempts


People who touched revisions under test:
  Dmitry Frolov 
  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   pass
 test-armhf-armhf-libvirt-qcow2   pass
 test-arm64-arm64-libvirt-raw pass
 test-armhf-armhf-libvirt-raw pass
 test-amd64-i386-libvirt-raw  pass
 test-amd64-amd64-libvirt-vhd pass



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

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

Explanation of these reports, and of osstest

Re: [XEN PATCH] xen/types: address Rule 10.1 for macro BITS_TO_LONGS

[Stefano Stabellini]

On Wed, 6 Sep 2023, Jan Beulich wrote:
> On 06.09.2023 17:57, Nicola Vetrini wrote:
> > On 05/09/2023 10:33, Jan Beulich wrote:
> >> On 05.09.2023 10:20, Nicola Vetrini wrote:
> >>> On 05/09/2023 09:46, Jan Beulich wrote:
>  On 05.09.2023 09:31, Nicola Vetrini wrote:
> > Given its use in the declaration
> > 'DECLARE_BITMAP(features, IOMMU_FEAT_count)' the argument
> > 'bits' has essential type 'enum iommu_feature', which is not
> > allowed by the Rule as an operand to the addition operator.
> > Given that its value can be represented by a signed integer,
> > the explicit cast resolves the violation.
> 
>  Wait - why would this lead to a change to BITS_TO_LONGS()? And if 
>  that
>  was to be changed, why plain int? I don't think negative input makes
>  sense there, and in principle I'd expect values beyond 4 billion to
>  also be permissible (even if likely no such use will ever appear in a
>  DECLARE_BITMAP(), but elsewhere it may make sense). Even going to
>  "unsigned long" may be too limiting ...
> 
> >>>
> >>> You have a point. I can think of doing it like this:
> >>> DECLARE_BITMAP(features, (int)IOMMU_FEAT_count)

I think this is a good solution for this case (even more so if we can't
find a better implementation of BITS_TO_LONGS)


> >>> on the grounds that the enum constant is representable in an int, and 
> >>> it
> >>> does not seem likely
> >>> to get much bigger.
> >>> Having an unsigned cast requires making the whole expression
> >>> essentially unsigned, otherwise Rule 10.4 is violated because
> >>> BITS_PER_LONG is
> >>> essentially signed. This can be done, but it depends on how
> >>> BITS_TO_LONGS will be/is used.
> >>
> >> It'll need looking closely, yes, but I expect that actually wants to be 
> >> an
> >> unsigned constant. I wouldn't be surprised if some use of 
> >> DECLARE_BITMAP()
> >> appeared (or already existed) where the 2nd argument involves sizeof() 
> >> in
> >> some way.
> >>
> > 
> > I think there's one with ARRAY_SIZE. In my opinion this can be resolved 
> > as follows:
> > 
> > #define BYTES_PER_LONG (1U << LONG_BYTEORDER) // the essential type gets 
> > from signed to unsigned
> > 
> > #define BITS_TO_LONGS(bits) \
> >  (((unsigned long long)(bits)+BITS_PER_LONG-1U)/BITS_PER_LONG) // 
> > same here
> 
> Except, as said before, I consider any kind of cast on "bits" latently
> problematic.

Can't we just do this (same but without the cast):

#define BYTES_PER_LONG (1U << LONG_BYTEORDER)
#define BITS_TO_LONGS(bits) \
 (((bits)+BITS_PER_LONG-1U)/BITS_PER_LONG)

Then we just need to make sure to pass an unsigned to BITS_TO_LONGS. In
the case above we would do:

DECLARE_BITMAP(features, (unsigned int)IOMMU_FEAT_count)

[PATCH v2] docs/misra: add 14.3

[Stefano Stabellini]

From: Stefano Stabellini 

Add 14.3, with project-wide deviations.

Also take the opportunity to clarify that parameters of function pointer
types are expected to have names (Rule 8.2).

Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- add switch to the Rule 14.3 exceptions
- add "and alike" for while(0) and while(1)
---
 docs/misra/rules.rst | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 34916e266a..3d8d1633a4 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -234,7 +234,8 @@ maintainers if you want to suggest a change.
* - `Rule 8.2 
`_
  - Required
  - Function types shall be in prototype form with named parameters
- -
+ - Clarification: both function and function pointers types shall
+   have named parameters.
 
* - `Rule 8.3 
`_
  - Required
@@ -385,6 +386,17 @@ maintainers if you want to suggest a change.
  - A loop counter shall not have essentially floating type
  -
 
+   * - `Rule 14.3 
`_
+ - Required
+ - Controlling expressions shall not be invariant
+ - Due to the extensive usage of IS_ENABLED, sizeof compile-time
+   checks, and other constructs that are detected as errors by MISRA
+   C scanners, managing the configuration of a MISRA C scanner for
+   this rule would be unmanageable. Thus, this rule is adopted with
+   a project-wide deviation on if ?: and switch statements.
+
+   while(0) and while(1) and alike are allowed.
+
* - `Rule 16.7 
`_
  - Required
  - A switch-expression shall not have essentially Boolean type
-- 
2.25.1

Re: [PATCH v2] docs/misra: add 14.3

[Stefano Stabellini]

On Thu, 31 Aug 2023, Jan Beulich wrote:
> On 31.08.2023 03:53, Stefano Stabellini wrote:
> > @@ -332,6 +333,17 @@ maintainers if you want to suggest a change.
> >   - A loop counter shall not have essentially floating type
> >   -
> >  
> > +   * - `Rule 14.3 
> > `_
> > + - Required
> > + - Controlling expressions shall not be invariant
> > + - Due to the extensive usage of IS_ENABLED, sizeof compile-time
> > +   checks, and other constructs that are detected as errors by MISRA
> > +   C scanners, managing the configuration of a MISRA C scanner for
> > +   this rule would be unmanageable. Thus, this rule is adopted with
> > +   a project-wide deviation on if and ?: statements.
> 
> As before - switch(sizeof(...)) and alike also wants deviating, imo.

I added the switch, but without the words "and alike" because it is not
clear in this context what it would allow.


> > +   while(0) and while(1) are allowed.
> 
> ... and alike ... (to also cover e.g. while(true)).

Here the words "and alike" are clearer so I added them

Re: [XEN PATCH v2] xen/arm: ioreq: add header for 'handle_ioserv' and 'try_fwd_ioserv'

[Stefano Stabellini]

On Mon, 4 Sep 2023, Nicola Vetrini wrote:
> The functions referenced by this patch should have had a compatible
> declaration visible prior to their definition. This is achieved by
> including the arch-specific header in 'xen/arch/arm/ioreq.c'
> 
> Fixes: cb9953d2f2bc ("arm/ioreq: Introduce arch specific bits for IOREQ/DM 
> features")
> Signed-off-by: Nicola Vetrini 

Acked-by: Stefano Stabellini 


> ---
> Changes in v2:
> - Avoid including  in  to allow new architectures
>   (e.g. ppc and riscv) not to provide one more stub header,
>   as pointed out by Jan Beulich.
> ---
>  xen/arch/arm/ioreq.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/arch/arm/ioreq.c b/xen/arch/arm/ioreq.c
> index 55854571898d..3bed0a14c050 100644
> --- a/xen/arch/arm/ioreq.c
> +++ b/xen/arch/arm/ioreq.c
> @@ -9,6 +9,7 @@
>  #include 
> 
>  #include 
> +#include 
> 
>  #include 
> 
> --
> 2.34.1
> 

Re: [XEN PATCH 12/13] xen: address violations of MISRA C:2012 Directive 4.10

[Stefano Stabellini]

On Wed, 6 Sep 2023, Jan Beulich wrote:
> On 06.09.2023 00:27, Stefano Stabellini wrote:
> > On Thu, 31 Aug 2023, Simone Ballarin wrote:
> >> On 29/08/23 00:51, Stefano Stabellini wrote:
> >>> On Mon, 28 Aug 2023, Simone Ballarin wrote:
>  Move or amended inclusion guards to address violations of
>  MISRA C:2012 Directive 4.10 ("Precautions shall be taken in order
>  to prevent the contents of a header file being included more than
>  once").
> 
>  Inclusion guards must appear at the beginning of the headers
>  (comments are permitted anywhere) and the #if directive cannot
>  be used for other checks.
> 
>  Mechanical change.
> 
>  Signed-off-by: Simone Ballarin 
>  ---
>    xen/include/xen/err.h   | 4 +++-
>    xen/include/xen/pci_ids.h   | 5 +
>    xen/include/xen/softirq.h   | 4 +++-
>    xen/include/xen/unaligned.h | 7 ---
>    xen/include/xen/vmap.h  | 4 +++-
>    5 files changed, 18 insertions(+), 6 deletions(-)
> 
>  diff --git a/xen/include/xen/err.h b/xen/include/xen/err.h
>  index 2f29b57d28..a6323d82d7 100644
>  --- a/xen/include/xen/err.h
>  +++ b/xen/include/xen/err.h
>  @@ -1,5 +1,6 @@
>  -#if !defined(__XEN_ERR_H__) && !defined(__ASSEMBLY__)
>  +#if !defined(__XEN_ERR_H__)
>    #define __XEN_ERR_H__
>  +#if !defined(__ASSEMBLY__)
> >>>
> >>> The original pattern was also guarding the header file sufficiently,
> >>> protecting it from double-inclusion. In fact, it is posing stricter
> >>> restrictions than usual (not laxer). This change is unnecessary?
> >>
> >> The MISRA directive asks to use one of the two following forms:
> >>
> >> 
> >> #if !defined ( identifier )
> >> #define identifier
> >> /* Contents of file */
> >> #endif
> >> 
> >>
> >> 
> >> #ifndef identifier
> >> #define identifier
> >> /* Contents of file */
> >> #endif
> >> 
> >>
> >> I do not see any reason for deviating, but if you ask that, I can do it.
> > 
> > Let's follow MISRA's form.
> 
> This is what I strongly dislike: They could be less restrictive on the
> exact patterns permitted without impacting the goal intended to be
> reached. But it's all as simple as possible, not as flexible as possible.
> 
> In any event, if a transformation like what can still be seen in context
> is to be made, then please #ifdef / #ifndef instead of defined(...)
> whenever possible.

In all fairness I dislike this too. However the rule is clear that to
make it easier to implement MISRA C checkers MISRA only supports 2
specific patterns. And I can see they have a point there in making it
easier to automatically check for correctness.

So I would go ahead with the change.

Re: [XEN PATCH v2 2/2] automation: execute SAF translation before the analysis with ECLAIR

[Stefano Stabellini]

On Wed, 6 Sep 2023, Nicola Vetrini wrote:
> This allows local MISRA deviation comments to be translated into
> the format recognized by ECLAIR.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Stefano Stabellini 

> ---
>  automation/eclair_analysis/prepare.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/automation/eclair_analysis/prepare.sh 
> b/automation/eclair_analysis/prepare.sh
> index 275a1a3f517c..0cac5eba00ae 100755
> --- a/automation/eclair_analysis/prepare.sh
> +++ b/automation/eclair_analysis/prepare.sh
> @@ -38,5 +38,8 @@ fi
>  cd xen
>  cp "${CONFIG_FILE}" .config
>  make clean
> +find . -type f -name "*.safparse" -print -delete
>  make -f ${script_dir}/Makefile.prepare prepare
> +# Translate the /* SAF-n-safe */ comments into ECLAIR CBTs
> +scripts/xen-analysis.py --run-eclair --no-build --no-clean
>  )
> -- 
> 2.34.1
> 

Re: [XEN PATCH v2 1/2] xen: apply deviation for Rule 8.4 (asm-only definitions)

[Stefano Stabellini]

On Wed, 6 Sep 2023, Nicola Vetrini wrote:
> As stated in 'docs/misra/rules.rst' the functions that are used only by
> asm modules do not need to conform to MISRA C:2012 Rule 8.4.
> The deviations are carried out with a SAF comment.
> 
> Signed-off-by: Nicola Vetrini 

This is better

Reviewed-by: Stefano Stabellini 

> ---
> Changes in v2:
> - Removed deviations on variables
> ---
>  docs/misra/safe.json| 8 
>  xen/arch/arm/cpuerrata.c| 1 +
>  xen/arch/arm/setup.c| 1 +
>  xen/arch/arm/smpboot.c  | 1 +
>  xen/arch/arm/traps.c| 7 +++
>  xen/arch/x86/boot/cmdline.c | 1 +
>  xen/arch/x86/boot/reloc.c   | 1 +
>  xen/arch/x86/extable.c  | 4 ++--
>  xen/arch/x86/setup.c| 1 +
>  xen/arch/x86/traps.c| 9 +
>  xen/common/efi/boot.c   | 5 +++--
>  11 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/misra/safe.json b/docs/misra/safe.json
> index e3c8a1d8eb36..39c5c056c7d4 100644
> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -12,6 +12,14 @@
>  },
>  {
>  "id": "SAF-1-safe",
> +"analyser": {
> +"eclair": "MC3R1.R8.4"
> +},
> +"name": "Rule 8.4: asm-only definition",
> +"text": "Functions and variables used only by asm modules do not 
> need to have a visible declaration prior to their definition."
> +},
> +{
> +"id": "SAF-2-safe",
>  "analyser": {},
>  "name": "Sentinel",
>  "text": "Next ID to be used"
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index fcf32892a7ef..9137958fb682 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -370,6 +370,7 @@ custom_param("spec-ctrl", parse_spec_ctrl);
>  
>  /* Arm64 only for now as for Arm32 the workaround is currently handled in C. 
> */
>  #ifdef CONFIG_ARM_64
> +/* SAF-1-safe */
>  void __init arm_enable_wa2_handling(const struct alt_instr *alt,
>  const uint32_t *origptr,
>  uint32_t *updptr, int nr_inst)
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 44ccea03ca14..db748839d383 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -1077,6 +1077,7 @@ static bool __init is_dom0less_mode(void)
>  size_t __read_mostly dcache_line_bytes;
>  
>  /* C entry point for boot CPU */
> +/* SAF-1-safe */
>  void __init start_xen(unsigned long boot_phys_offset,
>unsigned long fdt_paddr)
>  {
> diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
> index e107b86b7b44..6efd17eb3500 100644
> --- a/xen/arch/arm/smpboot.c
> +++ b/xen/arch/arm/smpboot.c
> @@ -302,6 +302,7 @@ smp_prepare_cpus(void)
>  }
>  
>  /* Boot the current CPU */
> +/* SAF-1-safe */
>  void start_secondary(void)
>  {
>  unsigned int cpuid = init_data.cpuid;
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 46c9a4031b30..ce89f16404d9 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -161,6 +161,7 @@ void init_traps(void)
>  isb();
>  }
>  
> +/* SAF-1-safe */
>  void __div0(void)
>  {
>  printk("Division by zero in hypervisor.\n");
> @@ -1954,6 +1955,7 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
>   * Actions that needs to be done after entering the hypervisor from the
>   * guest and before the interrupts are unmasked.
>   */
> +/* SAF-1-safe */
>  void enter_hypervisor_from_guest_preirq(void)
>  {
>  struct vcpu *v = current;
> @@ -1968,6 +1970,7 @@ void enter_hypervisor_from_guest_preirq(void)
>   * guest and before we handle any request. Depending on the exception trap,
>   * this may be called with interrupts unmasked.
>   */
> +/* SAF-1-safe */
>  void enter_hypervisor_from_guest(void)
>  {
>  struct vcpu *v = current;
> @@ -1996,6 +1999,7 @@ void enter_hypervisor_from_guest(void)
>  vgic_sync_from_lrs(v);
>  }
>  
> +/* SAF-1-safe */
>  void do_trap_guest_sync(struct cpu_user_regs *regs)
>  {
>  const union hsr hsr = { .bits = regs->hsr };
> @@ -2191,11 +2195,13 @@ void do_trap_guest_serror(struct cpu_user_regs *regs)
>  __do_trap_serror(regs, true);
>  }
>  
> +/* SAF-1-safe */
>  void do_trap_irq(struct cpu_user_regs *regs)
>  {
>  gic_interrupt(regs, 0);
>  }
>  
> +/* SAF-1-safe */
>  void do_trap_fiq(struct cpu_user_regs *regs)
>  {
>  gic_interrupt(regs, 1);
> @@ -2269,6 +2275,7 @@ static bool check_for_vcpu_work(void)
>   *
>   * The function will return with IRQ masked.
>   */
> +/* SAF-1-safe */
>  void leave_hypervisor_to_guest(void)
>  {
>  local_irq_disable();
> diff --git a/xen/arch/x86/boot/cmdline.c b/xen/arch/x86/boot/cmdline.c
> index 74997703b31e..f9eee756aaed 100644
> --- a/xen/arch/x86/boot/cmdline.c
> +++ b/xen/arch/x86/boot/cmdline.c
> @@ -340,6 +340,7 @@ static void vga_parse(const char *cmdline, 
> early_boot_opts_t *ebo)
>  }
>  #endif
>  
> +/* SAF-1-

Re: [XEN PATCH 2/2] automation: execute SAF translation before the analysis with ECLAIR

[Stefano Stabellini]

On Fri, 1 Sep 2023, Nicola Vetrini wrote:
> This allows local MISRA deviation comments to be translated into
> the format recognized by ECLAIR.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Stefano Stabellini 


> ---
>  automation/eclair_analysis/prepare.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/automation/eclair_analysis/prepare.sh 
> b/automation/eclair_analysis/prepare.sh
> index 275a1a3f517c..0cac5eba00ae 100755
> --- a/automation/eclair_analysis/prepare.sh
> +++ b/automation/eclair_analysis/prepare.sh
> @@ -38,5 +38,8 @@ fi
>  cd xen
>  cp "${CONFIG_FILE}" .config
>  make clean
> +find . -type f -name "*.safparse" -print -delete
>  make -f ${script_dir}/Makefile.prepare prepare
> +# Translate the /* SAF-n-safe */ comments into ECLAIR CBTs
> +scripts/xen-analysis.py --run-eclair --no-build --no-clean
>  )
> -- 
> 2.34.1
> 

Re: [XEN PATCH 1/2] xen: apply deviation for Rule 8.4 (asm-only definitions)

[Stefano Stabellini]

On Mon, 4 Sep 2023, Nicola Vetrini wrote:
> > Further in the cover letter you say "Deviating variables needs more care,
> > and
> > is therefore postponed to another patch", yet then here you annotate a
> > couple
> > of variables as well. Could you clarify what the criteria are for "needs
> > more
> > care"?
> > 
> 
> I see. I did not intend for those changes to end up in this patch, although
> those two are
> clearly only used by asm code and therefore are excepted.

I reviewed the patch checking that every function and variable is
actually only called/used from asm. Everything checks out, so:

Reviewed-by: Stefano Stabellini 

However, I think we should drop the change below (could be done on
commit) because it doesn't make sense to add SAF-1-safe for l1_fixmap_x
but not for l1_fixmap.  l1_fixmap has a declaration in
xen/arch/x86/include/asm/page.h but it is not actually used.

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 28fdf820ef3b..828786932021 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -143,6 +143,7 @@
 /* Mapping of the fixmap space needed early. */
 l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
 l1_fixmap[L1_PAGETABLE_ENTRIES];
+/* SAF-1-safe */
 l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
 l1_fixmap_x[L1_PAGETABLE_ENTRIES];

Re: [PATCH v2 1/2] automation: Switch ppc64le tests to PowerNV machine type

[Stefano Stabellini]

On Wed, 6 Sep 2023, Shawn Anastasio wrote:
> Run ppc64le tests with the PowerNV machine type (bare metal) instead of
> the paravirtualized pseries machine. This requires a more modern version
> of QEMU than is present in debian bullseye's repository, so update the
> dockerfile to build QEMU from source.
> 
> Support for booting on pseries was broken by 74b725a64d80 ('xen/ppc:
> Implement initial Radix MMU support') which resulted in CI failures. In
> preparation for removing pseries support entirely, switch the CI
> infrastructure to the PowerNV machine type.
> 
> Signed-off-by: Shawn Anastasio 
> ---
>  .../build/debian/bullseye-ppc64le.dockerfile  | 37 ++-
>  automation/gitlab-ci/test.yaml|  2 +-
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/automation/build/debian/bullseye-ppc64le.dockerfile 
> b/automation/build/debian/bullseye-ppc64le.dockerfile
> index 8fad26e903..15827bbcf4 100644
> --- a/automation/build/debian/bullseye-ppc64le.dockerfile
> +++ b/automation/build/debian/bullseye-ppc64le.dockerfile
> @@ -1,4 +1,36 @@
> +FROM debian:bullseye-slim AS builder
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV QEMU_TAG=stable-8.1
> +ENV USER root
> +
> +# dependencies for qemu build
> +RUN apt-get update && \
> +apt-get --quiet --yes install \
> +git \
> +build-essential \
> +python3-minimal \
> +python3-elementpath \
> +python3-pip \
> +ninja-build \
> +libglib2.0-dev \
> +libpixman-1-dev
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +# QEMU build from git
> +RUN git clone --depth 1 --branch "${QEMU_TAG}" https://github.com/qemu/qemu 
> && \
> +mkdir qemu/build && \
> +cd qemu/build && \
> +../configure --target-list=ppc64-softmmu && \
> +ninja && \
> +ninja install

It is totally find to build your QEMU but please use a test-artifact
instead, see automation/tests-artifacts/qemu-system-aarch64/ and
qemu-system-aarch64-6.0.0-arm64-export as an example.


>  FROM debian:bullseye-slim
> +COPY --from=builder /usr/local/bin/qemu-system-ppc64 
> /usr/local/bin/qemu-system-ppc64
> +COPY --from=builder /usr/local/share/qemu/skiboot.lid 
> /usr/local/share/qemu/skiboot.lid
> +
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> @@ -22,8 +54,9 @@ RUN apt-get update && \
>  gcc-powerpc64le-linux-gnu \
>  make \
>  python3-minimal \
> -# for test phase
> -qemu-system-ppc \
> +# QEMU runtime dependencies
> +libglib2.0-0 \
> +libpixman-1-0 \
>  && \
>  apt-get autoremove -y && \
>  apt-get clean && \
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 9aa8deabea..9b26ccd907 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -457,6 +457,6 @@ qemu-smoke-riscv64-gcc:
>  qemu-smoke-ppc64le-pseries-gcc:
>extends: .qemu-ppc64le
>script:
> -- ./automation/scripts/qemu-smoke-ppc64le.sh pseries-5.2 2>&1 | tee 
> ${LOGFILE}
> +- ./automation/scripts/qemu-smoke-ppc64le.sh powernv9 2>&1 | tee 
> ${LOGFILE}
>needs:
>  - debian-bullseye-gcc-ppc64le-debug
> -- 
> 2.30.2
> 

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemuu-win7-amd64 12 windows-install fail pass in 
182698-retest

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

version targeted for testing:
 qemuu2d8fbcb1eecd8d39171f457e583428758321d69d
baseline version:
 qemuu17780edd81d27fcfdb7a802efc870a99788bd2fc

Last test of basis   182606  2023-09-01 17:09:21 Z5 days
Testing same since   182638  2023-09-05 15:37:11 Z1 days1 attempts


People who touched revisions under test:
  Alexander Graf 
  David Hildenbrand 
  Igor Mammedov 
  Ilya Leoshkevich 
  Marc-André Lureau 
  Markus Armbruster 
  Michael To

RE: [EXT] Re: xen arm64 low power sleep support

[Stefano Stabellini]

On Wed, 6 Sep 2023, Anthony Chan wrote:
> On Tue, 05 Sep 2023, Stefano Stabellini wrote:
> > On Thu, 31 Aug 2023, Anthony Chan wrote:
> > > On Thu, 30 Aug 2023, Stefano Stabellini wrote:
> > > > On Wed, 30 Aug 2023, Anthony Chan wrote:
> > > > > On Tue, 29 Aug 2023, Stefano Stabellini wrote:
> > > > > > On Tue, 29 Aug 2023, Anthony Chan wrote:
> > > > > > > Hi all,
> > > > > > >
> > > > > > > My name is Tony and I've been researching/developing using
> > > > > > > Xen for potential upcoming uses in our embedded systems.  I
> > > > > > > started with Xen using Xilinx tools about a year ago and
> > > > > > > still have lots to learn about what it can to do in the
> > > > > > > embedded space.  So far, I've managed to integrate Xen and
> > > > > > > Linux into an existing product that exclusively runs
> > > > > > > bare-metal code on a ZynqMP SoC and migrate some of the
> > > > > > > functionality into custom Linux driver/userspace.
> > > > > > >
> > > > > > > I'm now looking at low power support, for now at least
> > > > > > > between Xen
> > > > > > > (4.16) and Linux (5.15) dom0.  I've tried a few different
> > > > > > > Linux kernel configs around power management and each time I
> > > > > > > try to suspend from linux dom0 (via sysfs or systemctl), Xen
> > > > > > > will watchdog on dom0 guest.
> > > > > > > AFAIK, Xen should trap on a 'WFI' from guests, but from what
> > > > > > > I can tell debugging through the linux suspend process is
> > > > > > > it's spinning in a 'suspend- to-idle' loop before it can get
> > > > > > > to issuing a 'WFI' or using PSCI interface to notify Xen.
> > > > > > > I'm beginning to suspect that 'low power' support for embedded
> > > > > > > arm64 just isn't quite there yet, or am I missing something
> > > > > > > in the configs?
> > > > > > >
> > > > > > > I realize this could very well be a Linux 'issue' but
> > > > > > > checking here first.  I know Xen presents a flattened device
> > > > > > > tree to Linux without CPU idle-state nodes and maybe this is
> > > > > > > causing the linux guest to only do the suspend- to-idle
> > > > > > > mode?  I should mention that I'm booting up using dom0less
> > > > > > > feature if that matters.
> > > > > >
> > > > > >
> > > > > > Hi Anthony,
> > > > > >
> > > > > > Assuming you are using the default Xen command line parameters
> > > > > > for Xilinx boards: sched=null vwfi=native, then if the guest
> > > > > > uses WFI, the CPU will execute WFI directly and go into low
> > > > > > power mode.
> > > > > Yes, using these command line params.
> > > > >
> > > > > > Given the issue you are describing, I am suspecting the guest
> > > > > > is not issuing
> > > > > > WFI: that is simple and known to work. Instead, I suspect that
> > > > > > Linux might be trying to use PSCI_suspend in a way that is not
> > > > > > supported or well- implemented by Xen.
> > > > > >
> > > > > > Can you check? You can add a printk in Linux
> > > > > > drivers/firmware/psci/psci.c:__psci_cpu_suspend or in Xen
> > > > > > xen/arch/arm/vpsci.c:do_psci_0_2_cpu_suspend
> > > > > Instrumented both places it doesn't appear to reach there.  In
> > > > > kernel/power/suspend.c, there's a call to s2idle_loop that it's
> > > > > currently 'stuck' in and I think it doesn't get to the psci suspend 
> > > > > your
> > > > > referring till afterwards, when suspend_ops->enter is called.
> > > > > Unfortunately, without any idle-states nodes in the FDT, the
> > > > > only suspend state Linux is defaults to is 'suspend to idle'.
> > > >
> > > > The fact that Linux uses "suspend to idle" is not a problem
> > > > because as I mentioned WFI or PSCI_suspent are not different on
> > > > Xen. That part is OK.
> > > What if using "suspend to idle" is preventing a WFI/PSCI_suspend?
> > > Which is what I believe I'm currently seeing in my setup.  In
> > > kernel/power/suspend.c, suspend_devices_and_enter(), it gets into
> > > the this s2idle_loop and upon resuming from idle, it jumps past the
> > > point where I believe a WFI/PSCI_suspend can happen.
> > > if (state == PM_SUSPEND_TO_IDLE) {
> > > s2idle_loop();
> > > goto Platform_wake;
> > > }
> >
> > If that is the case, then it looks like a Linux bug. Maybe something
> > along these lines?
> >
> > https://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .k%2F&data=05%7C01%7Canthonychan%40nureva.com%7Ccf5e956c6a4d42b1460a08
> > dbaf13e021%7C5aeb77fa643b473eaee0cb54a11ccba3%7C1%7C0%7C63829627212319
> > 7144%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT
> > iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=lDfILPgJQQQPJOL44%2BcU
> > %2BN2RFOs8L6F4AE11kvOhOQE%3D&reserved=0
> > ernel.org%2Flinux-arm-
> > kernel%2F4665489.GXAFRqVoOG%40kreacher%2FT%2F%23m6edda92d0b5
> > dc09f8e05e7d6db3807501b7249f4&data=05%7C01%7Canthonychan%40n
> > ureva.com%7C144641906bef48b9180f08dbae7bc1d8%7C5aeb77fa643b47
> > 3eaee0cb54a11ccba3%7C1%7C0%7C638295618800415028%7CUnknown
> > %7CTWFpbGZsb3

Re: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0

[kernel test robot]

Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v6.5]
[cannot apply to rafael-pm/linux-next linus/master next-20230906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Jason-Andryuk/acpi-processor-sanitize-_PDC-buffer-bits-when-running-as-Xen-dom0/20230907-022235
base:   tip/x86/core
patch link:
https://lore.kernel.org/r/20230906182125.48642-1-jandryuk%40gmail.com
patch subject: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when 
running as Xen dom0
config: i386-randconfig-003-20230907 
(https://download.01.org/0day-ci/archive/20230907/202309070741.2n0k8fjn-...@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git 
ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20230907/202309070741.2n0k8fjn-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202309070741.2n0k8fjn-...@intel.com/

All errors (new ones prefixed by >>):

>> drivers/acpi/processor_pdc.c:147:3: error: call to undeclared function 
>> 'xen_sanitize_pdc'; ISO C99 and later do not support implicit function 
>> declarations [-Wimplicit-function-declaration]
   xen_sanitize_pdc(buffer);
   ^
   1 error generated.


vim +/xen_sanitize_pdc +147 drivers/acpi/processor_pdc.c

   116  
   117  /*
   118   * _PDC is required for a BIOS-OS handshake for most of the newer
   119   * ACPI processor features.
   120   */
   121  static acpi_status
   122  acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list 
*pdc_in)
   123  {
   124  acpi_status status = AE_OK;
   125  union acpi_object *obj;
   126  u32 *buffer = NULL;
   127  
   128  obj = pdc_in->pointer;
   129  buffer = (u32 *)(obj->buffer.pointer);
   130  
   131  if (boot_option_idle_override == IDLE_NOMWAIT) {
   132  /*
   133   * If mwait is disabled for CPU C-states, the C2C3_FFH 
access
   134   * mode will be disabled in the parameter of _PDC 
object.
   135   * Of course C1_FFH access mode will also be disabled.
   136   */
   137  buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
   138  }
   139  
   140  if (xen_initial_domain()) {
   141  /*
   142   * When Linux is running as Xen dom0, the hypervisor is 
the
   143   * entity in charge of the processor power management, 
and so
   144   * Xen needs to check the OS capabilities reported in 
the _PDC
   145   * buffer matches what the hypervisor driver supports.
   146   */
 > 147  xen_sanitize_pdc(buffer);
   148  }
   149  
   150  status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
   151  
   152  if (ACPI_FAILURE(status))
   153  acpi_handle_debug(handle,
   154  "Could not evaluate _PDC, using legacy perf 
control\n");
   155  
   156  return status;
   157  }
   158  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v2 0/2] ppc: Drop support for QEMU pseries

[Shawn Anastasio]

Hello all,

This small series drops support for the QEMU paravirtualized pseries
machine type in favor of the bare metal PowerNV platform. pseries
support was broken by 74b725a64d80 ('xen/ppc: Implement initial Radix
MMU support'), and instead of maintaining separate code paths to retain
support for the platform, I think it makes the most sense to drop it
entirely.

pseries was originally targeted for initial bringup since it has
historically been much better supported by QEMU, but PowerNV support in
QEMU has recently developed to the point where this is no longer a
concern.  Since I can think of very little practical use for running Xen
under pseries, and supporting it requires non-trivial duplication of
effort, drop support for it entirely.

This also requires a change to the ppc64le CI dockerfile to build a
newer version of QEMU than is available in the bullseye repository.

Thanks,
Shawn

--
v2: Add Fixes: tag to patch 2, add references to broken state of pseries

Shawn Anastasio (2):
  automation: Switch ppc64le tests to PowerNV machine type
  xen/ppc: Drop support for pseries/OpenFirmware

 .../build/debian/bullseye-ppc64le.dockerfile  |  37 +-
 automation/gitlab-ci/test.yaml|   2 +-
 xen/arch/ppc/Makefile |   1 -
 xen/arch/ppc/boot-of.c| 113 --
 xen/arch/ppc/include/asm/boot.h   |  19 ---
 xen/arch/ppc/ppc64/Makefile   |   1 -
 xen/arch/ppc/ppc64/of-call.S  |  83 -
 xen/arch/ppc/setup.c  |  11 +-
 8 files changed, 39 insertions(+), 228 deletions(-)
 delete mode 100644 xen/arch/ppc/boot-of.c
 delete mode 100644 xen/arch/ppc/ppc64/of-call.S

--
2.30.2

[PATCH v2 2/2] xen/ppc: Drop support for pseries/OpenFirmware

[Shawn Anastasio]

Since QEMU's PowerNV support has matured to the point where it is
now suitable for development, drop support for booting on the
paravirtualized pseries machine type and its associated interfaces.

Support for booting on pseries was broken by 74b725a64d80 ('xen/ppc:
Implement initial Radix MMU support'), and since there is little
practical value in continuing to support pseries as a target, just drop
support for it entirely.

Signed-off-by: Shawn Anastasio 
Fixes: 74b725a64d80 ('xen/ppc: Implement initial Radix MMU support')
---
 xen/arch/ppc/Makefile   |   1 -
 xen/arch/ppc/boot-of.c  | 113 
 xen/arch/ppc/include/asm/boot.h |  19 --
 xen/arch/ppc/ppc64/Makefile |   1 -
 xen/arch/ppc/ppc64/of-call.S|  83 ---
 xen/arch/ppc/setup.c|  11 +---
 6 files changed, 3 insertions(+), 225 deletions(-)
 delete mode 100644 xen/arch/ppc/boot-of.c
 delete mode 100644 xen/arch/ppc/ppc64/of-call.S

diff --git a/xen/arch/ppc/Makefile b/xen/arch/ppc/Makefile
index a059ac4c0a..b3205b8f7a 100644
--- a/xen/arch/ppc/Makefile
+++ b/xen/arch/ppc/Makefile
@@ -1,6 +1,5 @@
 obj-$(CONFIG_PPC64) += ppc64/
 
-obj-y += boot-of.init.o
 obj-$(CONFIG_EARLY_PRINTK) += early_printk.init.o
 obj-y += mm-radix.o
 obj-y += opal.o
diff --git a/xen/arch/ppc/boot-of.c b/xen/arch/ppc/boot-of.c
deleted file mode 100644
index d6755ccc8e..00
--- a/xen/arch/ppc/boot-of.c
+++ /dev/null
@@ -1,113 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * This file was derived from Xen 3.2's xen/arch/powerpc/boot_of.c,
- * originally licensed under GPL version 2 or later.
- *
- * Copyright IBM Corp. 2005, 2006, 2007
- * Copyright Raptor Engineering, LLC
- *
- * Authors: Jimi Xenidis 
- *  Hollis Blanchard 
- *  Shawn Anastasio 
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-/*
- * The Open Firmware client interface is called in 32-bit mode with the MMU 
off,
- * so any addresses passed to it must be physical addresses under 4GB.
- *
- * Since the client interface is only used during early boot before the MMU is 
on
- * and Xen itself was loaded by Open Firmware (and therefore resides below 
4GB),
- * we can achieve the desired result with a simple cast to uint32_t.
- */
-#define ADDR(x) ((uint32_t)(unsigned long)(x))
-
-/* OF entrypoint*/
-static unsigned long __initdata of_vec;
-
-static int __initdata of_out;
-
-static int __init of_call(const char *service, uint32_t nargs, uint32_t nrets,
-  int32_t rets[], ...)
-{
-int rc;
-va_list args;
-unsigned int i;
-struct of_service s;
-
-s.ofs_service = cpu_to_be32(ADDR(service));
-s.ofs_nargs = cpu_to_be32(nargs);
-s.ofs_nrets = cpu_to_be32(nrets);
-
-/* Copy all the params into the args array */
-va_start(args, rets);
-
-for ( i = 0; i < nargs; i++ )
-s.ofs_args[i] = cpu_to_be32(va_arg(args, uint32_t));
-
-va_end(args);
-
-rc = enter_of(&s, of_vec);
-
-/* Copy all return values to the output rets array */
-for ( i = 0; i < nrets; i++ )
-rets[i] = be32_to_cpu(s.ofs_args[i + nargs]);
-
-return rc;
-}
-
-static int __init of_finddevice(const char *devspec)
-{
-int32_t rets[1] = { OF_FAILURE };
-
-of_call("finddevice", 1, ARRAY_SIZE(rets), rets, ADDR(devspec));
-return rets[0];
-}
-
-static int __init of_getprop(int ph, const char *name, void *buf, uint32_t 
buflen)
-{
-int32_t rets[1] = { OF_FAILURE };
-
-of_call("getprop", 4, ARRAY_SIZE(rets), rets, ph, ADDR(name), ADDR(buf),
-buflen);
-return rets[0];
-}
-
-int __init of_write(int ih, const char *addr, uint32_t len)
-{
-int32_t rets[1] = { OF_FAILURE };
-
-of_call("write", 3, ARRAY_SIZE(rets), rets, ih, ADDR(addr), len);
-return rets[0];
-}
-
-static void __init of_putchar(char c)
-{
-if ( c == '\n' )
-{
-char buf = '\r';
-of_write(of_out, &buf, 1);
-}
-of_write(of_out, &c, 1);
-}
-
-void __init boot_of_init(unsigned long vec)
-{
-int bof_chosen;
-
-of_vec = vec;
-
-/* Get a handle to the default console */
-bof_chosen = of_finddevice("/chosen");
-of_getprop(bof_chosen, "stdout", &of_out, sizeof(of_out));
-of_out = be32_to_cpu(of_out);
-
-early_printk_init(of_putchar);
-}
diff --git a/xen/arch/ppc/include/asm/boot.h b/xen/arch/ppc/include/asm/boot.h
index 296539cd9e..d62c3ff6e0 100644
--- a/xen/arch/ppc/include/asm/boot.h
+++ b/xen/arch/ppc/include/asm/boot.h
@@ -4,25 +4,6 @@
 
 #include 
 
-/*
- * OpenFirmware boot interfaces
- */
-
-enum {
-OF_FAILURE = -1,
-OF_SUCCESS = 0,
-};
-
-struct of_service {
-__be32 ofs_service;
-__be32 ofs_nargs;
-__be32 ofs_nrets;
-__be32 ofs_args[10];
-};
-
-int enter_of(struct of_service *args, unsigned long entry);
-void boot_of_init(unsigned long vec);
-
 /*
  * OPAL boot interfaces
  */
diff --git a/xen/arch/ppc/ppc64/Makefile b/xen/arch/ppc

[PATCH v2 1/2] automation: Switch ppc64le tests to PowerNV machine type

[Shawn Anastasio]

Run ppc64le tests with the PowerNV machine type (bare metal) instead of
the paravirtualized pseries machine. This requires a more modern version
of QEMU than is present in debian bullseye's repository, so update the
dockerfile to build QEMU from source.

Support for booting on pseries was broken by 74b725a64d80 ('xen/ppc:
Implement initial Radix MMU support') which resulted in CI failures. In
preparation for removing pseries support entirely, switch the CI
infrastructure to the PowerNV machine type.

Signed-off-by: Shawn Anastasio 
---
 .../build/debian/bullseye-ppc64le.dockerfile  | 37 ++-
 automation/gitlab-ci/test.yaml|  2 +-
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/automation/build/debian/bullseye-ppc64le.dockerfile 
b/automation/build/debian/bullseye-ppc64le.dockerfile
index 8fad26e903..15827bbcf4 100644
--- a/automation/build/debian/bullseye-ppc64le.dockerfile
+++ b/automation/build/debian/bullseye-ppc64le.dockerfile
@@ -1,4 +1,36 @@
+FROM debian:bullseye-slim AS builder
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV QEMU_TAG=stable-8.1
+ENV USER root
+
+# dependencies for qemu build
+RUN apt-get update && \
+apt-get --quiet --yes install \
+git \
+build-essential \
+python3-minimal \
+python3-elementpath \
+python3-pip \
+ninja-build \
+libglib2.0-dev \
+libpixman-1-dev
+
+RUN mkdir /build
+WORKDIR /build
+
+# QEMU build from git
+RUN git clone --depth 1 --branch "${QEMU_TAG}" https://github.com/qemu/qemu && 
\
+mkdir qemu/build && \
+cd qemu/build && \
+../configure --target-list=ppc64-softmmu && \
+ninja && \
+ninja install
+
 FROM debian:bullseye-slim
+COPY --from=builder /usr/local/bin/qemu-system-ppc64 
/usr/local/bin/qemu-system-ppc64
+COPY --from=builder /usr/local/share/qemu/skiboot.lid 
/usr/local/share/qemu/skiboot.lid
+
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
@@ -22,8 +54,9 @@ RUN apt-get update && \
 gcc-powerpc64le-linux-gnu \
 make \
 python3-minimal \
-# for test phase
-qemu-system-ppc \
+# QEMU runtime dependencies
+libglib2.0-0 \
+libpixman-1-0 \
 && \
 apt-get autoremove -y && \
 apt-get clean && \
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 9aa8deabea..9b26ccd907 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -457,6 +457,6 @@ qemu-smoke-riscv64-gcc:
 qemu-smoke-ppc64le-pseries-gcc:
   extends: .qemu-ppc64le
   script:
-- ./automation/scripts/qemu-smoke-ppc64le.sh pseries-5.2 2>&1 | tee 
${LOGFILE}
+- ./automation/scripts/qemu-smoke-ppc64le.sh powernv9 2>&1 | tee ${LOGFILE}
   needs:
 - debian-bullseye-gcc-ppc64le-debug
-- 
2.30.2

Re: [PATCH 2/2] xen/ppc: Drop support for pseries/OpenFirmware

[Shawn Anastasio]

On 9/6/23 1:44 AM, Jan Beulich wrote:
> On 05.09.2023 23:46, Shawn Anastasio wrote:
>> Since QEMU's PowerNV support has matured to the point where it is
>> now suitable for development, drop support for booting on the
>> paravirtualized pseries machine type and its associated interfaces.
> 
> I think you want to mention that the pseries functionality was actually
> broken by the earlier change, both verbally and by means of a Fixes:
> tag. The breakage actually may also want mentioning in patch 1 as well
> as the cover letter.
>

Will do. I'll add mentions to the breakage in both patches and add a
Fixes: tag to this patch specifically.

>> --- a/xen/arch/ppc/setup.c
>> +++ b/xen/arch/ppc/setup.c
>> @@ -14,17 +14,12 @@ void __init noreturn start_xen(unsigned long r3, 
>> unsigned long r4,
>>  {
>>  if ( r5 )
>>  {
>> -/* OpenFirmware boot protocol */
>> -boot_of_init(r5);
>> +/* Unsupported OpenFirmware boot protocol */
>> +__builtin_trap();
>>  }
>>  else
>>  {
>> -/*
>> - * kexec boot protocol
>> - *
>> - * TODO: This currently assumes an OPAL/PowerNV system, but it's 
>> also
>> - * possible to be kexec'd on an OF system.
>> - */
>> +/* kexec boot protocol */
>>  boot_opal_init((void *)r3);
> 
> At least part of the comment may want retaining, as the code only handles
> the OPAL case?
>

Since we're now explicitly only supporting Opal/PowerNV I don't think
there's any need to point out that this code path doesn't properly
handle the pseries+kexec case. boot_opal_init will gracefully
fail under that circumstance anyway when it tries to find an /ibm,opal
device tree node.

> Jan

Thanks,
Shawn

Re: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0

[kernel test robot]

Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v6.5]
[cannot apply to rafael-pm/linux-next linus/master next-20230906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Jason-Andryuk/acpi-processor-sanitize-_PDC-buffer-bits-when-running-as-Xen-dom0/20230907-022235
base:   tip/x86/core
patch link:
https://lore.kernel.org/r/20230906182125.48642-1-jandryuk%40gmail.com
patch subject: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when 
running as Xen dom0
config: x86_64-randconfig-r011-20230907 
(https://download.01.org/0day-ci/archive/20230907/202309070625.djudcgzg-...@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20230907/202309070625.djudcgzg-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202309070625.djudcgzg-...@intel.com/

All errors (new ones prefixed by >>):

   drivers/acpi/processor_pdc.c: In function 'acpi_processor_eval_pdc':
>> drivers/acpi/processor_pdc.c:147:17: error: implicit declaration of function 
>> 'xen_sanitize_pdc' [-Werror=implicit-function-declaration]
 147 | xen_sanitize_pdc(buffer);
 | ^~~~
   cc1: some warnings being treated as errors


vim +/xen_sanitize_pdc +147 drivers/acpi/processor_pdc.c

   116  
   117  /*
   118   * _PDC is required for a BIOS-OS handshake for most of the newer
   119   * ACPI processor features.
   120   */
   121  static acpi_status
   122  acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list 
*pdc_in)
   123  {
   124  acpi_status status = AE_OK;
   125  union acpi_object *obj;
   126  u32 *buffer = NULL;
   127  
   128  obj = pdc_in->pointer;
   129  buffer = (u32 *)(obj->buffer.pointer);
   130  
   131  if (boot_option_idle_override == IDLE_NOMWAIT) {
   132  /*
   133   * If mwait is disabled for CPU C-states, the C2C3_FFH 
access
   134   * mode will be disabled in the parameter of _PDC 
object.
   135   * Of course C1_FFH access mode will also be disabled.
   136   */
   137  buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
   138  }
   139  
   140  if (xen_initial_domain()) {
   141  /*
   142   * When Linux is running as Xen dom0, the hypervisor is 
the
   143   * entity in charge of the processor power management, 
and so
   144   * Xen needs to check the OS capabilities reported in 
the _PDC
   145   * buffer matches what the hypervisor driver supports.
   146   */
 > 147  xen_sanitize_pdc(buffer);
   148  }
   149  
   150  status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
   151  
   152  if (ACPI_FAILURE(status))
   153  acpi_handle_debug(handle,
   154  "Could not evaluate _PDC, using legacy perf 
control\n");
   155  
   156  return status;
   157  }
   158  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: Xens handling of MCE

[Elliott Mitchell]

On Thu, Aug 31, 2023 at 07:52:05PM +, Development wrote:
> 
> However, in 2009-02, "cegger" wrote MCA/MCE_in_Xen, a proposal for having 
> xen start checking the information
> Xen started accessing the EDAC information (now called "MCE") at some 
> point after that, which blocks the linux kernel in dom0 from accessing it.
> (I also found what appears to be related sides from a presentation from 
> 2012 at: 
> https://lkml.iu.edu/hypermail/linux/kernel/1206.3/01304/xen_vMCE_design_%28v0_2%29.pdf
>  )
> 

I hadn't seen that before.  Clearly shows someone who had no idea what
they were doing designed it.  The author was thinking "virtualize 
everything!", whereas MCE is a perfect situation for paravirtualization.
Let Dom0 process MCE events (which allows use of Linux's more up to date
MCE drivers), then let Dom0 notify Xen if action is needed (a page was
corrupted, tell the effected domain).

There was a recent proposal to simply import Linux's rather more recent
MCE/EDAC source.  This hasn't happened yet.  For people using Xen this
has been a very concerning issue for some time.

This seems a combination of not enough people using Xen not yet having
gotten quite noisy enough (I think the threshold is being approached),
plus the people with the right development skills being out of touch.

Commit 767f4b620edadac579c9b8b6660761d4285fa6f9 to the Linux kernel yet
again shows someone *REALLY* out of touch!


-- 
(\___(\___(\__  --=> 8-) EHM <=--  __/)___/)___/)
 \BS (| ehem+sig...@m5p.com  PGP 87145445 |)   /
  \_CS\   |  _  -O #include  O-   _  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445

Re: [XEN PATCH] x86/ACPI: Ignore entries with invalid APIC IDs when parsing MADT

[Stefano Stabellini]

On Fri, 1 Sep 2023, Jan Beulich wrote:
> On 07.08.2023 11:38, Simon Gaiser wrote:
> > It seems some firmwares put dummy entries in the ACPI MADT table for non
> > existing processors. On my NUC11TNHi5 those have the invalid APIC ID
> > 0xff. Linux already has code to handle those cases both in
> > acpi_parse_lapic [1] as well as in acpi_parse_x2apic [2]. So add the
> > same check to Xen.
> > 
> > Note that on some older (2nd gen Core i) laptop of mine I also saw dummy
> > entries with a valid APIC ID. Linux would still ignore those because
> > they have !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE. But in Xen
> > this check is only active for madt_revision >= 5. But since this version
> > check seems to be intentionally I leave that alone.
> > 
> > Link: 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f3bf1dbe64b62a2058dd1944c00990df203e8e7a
> >  # [1]
> > Link: 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=10daf10ab154e31237a8c07242be3063fb6a9bf4
> >  # [2]
> > Signed-off-by: Simon Gaiser 
> 
> This patch was committed with unaddressed review comments. The normal action
> in such a case would be to revert, expecting a v2 to arrive. One alternative
> here would be a timely incremental patch submission. Another alternative,
> considering in particular Thomas's most recent reply, would be to properly
> downgrade CPU hotplug support in SUPPORT.md (with a corresponding entry in
> CHANGELOG.md).

I am in favor of downgrading physical CPU hotplug support in
SUPPORT.md.

I noticed that there is no entry for physical CPU hotplug support in
SUPPORT.md today. Should we assume that it is not supported already as
it is not listed as supported?

Specifically, would it be a good idea to add a sentence at the top of
the file saying that anything not explicitly listed is not supported?

Re: [PATCH v2] Arm: constrain {,u}int64_aligned_t in public header

[Stefano Stabellini]

On Tue, 5 Sep 2023, Henry Wang wrote:
> Hi Jan,
> 
> > On Sep 5, 2023, at 16:26, Jan Beulich  wrote:
> > 
> > For using a GNU extension, it may not be exposed in general, just like
> > is done on x86 (except that here we need to also work around not all of
> > the tool stack actually defining __XEN_TOOLS__). External consumers (not
> > using gcc or a compatible compiler) need to make this type available up
> > front (just like we expect {,u}int_t to be supplied) - unlike on x86
> > the type is actually needed outside of tools-only interfaces, because
> > guest handle definitions use it.
> > 
> > While there also add underscores around "aligned".
> > 
> > Signed-off-by: Jan Beulich 
> 
> I used the same testing approach to redo the test for v2, and everything
> works fine this time.
> 
> Tested-by: Henry Wang 

Reviewed-by: Stefano Stabellini 

Re: [PATCH] xen/arm: Fix printk specifiers and arguments in iomem_remove_cb()

[Stefano Stabellini]

On Wed, 6 Sep 2023, Vikram Garhwal wrote:
> On Wed, Sep 06, 2023 at 12:30:14PM +0200, Michal Orzel wrote:
> > When building Xen for arm32 with CONFIG_DTB_OVERLAY, the following
> > error is printed:
> > 
> > common/dt-overlay.c: In function ‘iomem_remove_cb’:
> > ././include/xen/config.h:55:24: error: format ‘%llx’ expects argument of 
> > type ‘long long unsigned int’, but argument 3 has type ‘long unsigned int’ 
> > [-Werror=format=]
> > 
> > Function parameters s and e (denoting MMIO region) are of type unsigned
> > long and indicate frame numbers and not addresses. This also means that
> > the arguments passed to printk() are incorrect (using PAGE_ALIGN() or
> > PAGE_MASK ANDed with a frame number results in unwanted output). Fix it.
> > 
> > Take the opportunity to switch to %pd specifier to print domain id in
> > a consolidated way.
> > 
> > Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal 
> > functionalities")
> > Signed-off-by: Michal Orzel 
> Reviewed-by: Vikram Garhwal 

Acked-by: Stefano Stabellini 


> > ---
> > After this patch (and the one for xl), we are left with one issue breaking 
> > CI:
> > https://gitlab.com/xen-project/patchew/xen/-/jobs/5026938514
> > ---
> >  xen/common/dt-overlay.c | 6 ++
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
> > index b07a6128dc99..5663a049e90a 100644
> > --- a/xen/common/dt-overlay.c
> > +++ b/xen/common/dt-overlay.c
> > @@ -251,10 +251,8 @@ static int iomem_remove_cb(unsigned long s, unsigned 
> > long e, void *dom,
> >  rc = iomem_deny_access(d, s, e);
> >  if ( rc )
> >  {
> > -printk(XENLOG_ERR "Unable to remove dom%d access to"
> > -   " 0x%"PRIx64" - 0x%"PRIx64"\n",
> > -   d->domain_id,
> > -   s & PAGE_MASK, PAGE_ALIGN(e) - 1);
> > +printk(XENLOG_ERR "Unable to remove %pd access to %#lx - %#lx\n",
> > +   d, s, e);
> >  }
> >  else
> >  *c += e - s + 1;
> > -- 
> > 2.25.1
> > 
> 

RE: [EXT] Re: xen arm64 low power sleep support

[Anthony Chan]

On Tue, 05 Sep 2023, Stefano Stabellini wrote:
> On Thu, 31 Aug 2023, Anthony Chan wrote:
> > On Thu, 30 Aug 2023, Stefano Stabellini wrote:
> > > On Wed, 30 Aug 2023, Anthony Chan wrote:
> > > > On Tue, 29 Aug 2023, Stefano Stabellini wrote:
> > > > > On Tue, 29 Aug 2023, Anthony Chan wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > My name is Tony and I've been researching/developing using
> > > > > > Xen for potential upcoming uses in our embedded systems.  I
> > > > > > started with Xen using Xilinx tools about a year ago and
> > > > > > still have lots to learn about what it can to do in the
> > > > > > embedded space.  So far, I've managed to integrate Xen and
> > > > > > Linux into an existing product that exclusively runs
> > > > > > bare-metal code on a ZynqMP SoC and migrate some of the
> > > > > > functionality into custom Linux driver/userspace.
> > > > > >
> > > > > > I'm now looking at low power support, for now at least
> > > > > > between Xen
> > > > > > (4.16) and Linux (5.15) dom0.  I've tried a few different
> > > > > > Linux kernel configs around power management and each time I
> > > > > > try to suspend from linux dom0 (via sysfs or systemctl), Xen
> > > > > > will watchdog on dom0 guest.
> > > > > > AFAIK, Xen should trap on a 'WFI' from guests, but from what
> > > > > > I can tell debugging through the linux suspend process is
> > > > > > it's spinning in a 'suspend- to-idle' loop before it can get
> > > > > > to issuing a 'WFI' or using PSCI interface to notify Xen.
> > > > > > I'm beginning to suspect that 'low power' support for embedded
> > > > > > arm64 just isn't quite there yet, or am I missing something
> > > > > > in the configs?
> > > > > >
> > > > > > I realize this could very well be a Linux 'issue' but
> > > > > > checking here first.  I know Xen presents a flattened device
> > > > > > tree to Linux without CPU idle-state nodes and maybe this is
> > > > > > causing the linux guest to only do the suspend- to-idle
> > > > > > mode?  I should mention that I'm booting up using dom0less
> > > > > > feature if that matters.
> > > > >
> > > > >
> > > > > Hi Anthony,
> > > > >
> > > > > Assuming you are using the default Xen command line parameters
> > > > > for Xilinx boards: sched=null vwfi=native, then if the guest
> > > > > uses WFI, the CPU will execute WFI directly and go into low
> > > > > power mode.
> > > > Yes, using these command line params.
> > > >
> > > > > Given the issue you are describing, I am suspecting the guest
> > > > > is not issuing
> > > > > WFI: that is simple and known to work. Instead, I suspect that
> > > > > Linux might be trying to use PSCI_suspend in a way that is not
> > > > > supported or well- implemented by Xen.
> > > > >
> > > > > Can you check? You can add a printk in Linux
> > > > > drivers/firmware/psci/psci.c:__psci_cpu_suspend or in Xen
> > > > > xen/arch/arm/vpsci.c:do_psci_0_2_cpu_suspend
> > > > Instrumented both places it doesn't appear to reach there.  In
> > > > kernel/power/suspend.c, there's a call to s2idle_loop that it's
> > > > currently 'stuck' in and I think it doesn't get to the psci suspend your
> > > > referring till afterwards, when suspend_ops->enter is called.
> > > > Unfortunately, without any idle-states nodes in the FDT, the
> > > > only suspend state Linux is defaults to is 'suspend to idle'.
> > >
> > > The fact that Linux uses "suspend to idle" is not a problem
> > > because as I mentioned WFI or PSCI_suspent are not different on
> > > Xen. That part is OK.
> > What if using "suspend to idle" is preventing a WFI/PSCI_suspend?
> > Which is what I believe I'm currently seeing in my setup.  In
> > kernel/power/suspend.c, suspend_devices_and_enter(), it gets into
> > the this s2idle_loop and upon resuming from idle, it jumps past the
> > point where I believe a WFI/PSCI_suspend can happen.
> > if (state == PM_SUSPEND_TO_IDLE) {
> > s2idle_loop();
> > goto Platform_wake;
> > }
>
> If that is the case, then it looks like a Linux bug. Maybe something
> along these lines?
>
> https://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> .k%2F&data=05%7C01%7Canthonychan%40nureva.com%7Ccf5e956c6a4d42b1460a08
> dbaf13e021%7C5aeb77fa643b473eaee0cb54a11ccba3%7C1%7C0%7C63829627212319
> 7144%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=lDfILPgJQQQPJOL44%2BcU
> %2BN2RFOs8L6F4AE11kvOhOQE%3D&reserved=0
> ernel.org%2Flinux-arm-
> kernel%2F4665489.GXAFRqVoOG%40kreacher%2FT%2F%23m6edda92d0b5
> dc09f8e05e7d6db3807501b7249f4&data=05%7C01%7Canthonychan%40n
> ureva.com%7C144641906bef48b9180f08dbae7bc1d8%7C5aeb77fa643b47
> 3eaee0cb54a11ccba3%7C1%7C0%7C638295618800415028%7CUnknown
> %7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2Fv%2FHpI1ix3yhDJ
> 75oejWzDfUqB8SqOQzkU3clJzMOtw%3D&reserved=0

Thanks, I've tried patches that stemmed from that discussion but unfortun

Re: [PATCH v5 1/2] xen: asm-generic support

[Oleksii]

Hi all,

Missed the comment (https://lore.kernel.org/xen-
devel/a526c0b285d94550e59a8968786b28d266afba58.ca...@gmail.com/T/#m99a4
dc8280439e84021c4e0546dc9ff539a60191 ) of Anthony so I'll sent a new
one patch series tomorrow.

Sorry for inconvenience.

~ Oleksii

Re: [PATCH v4 1/2] xen: asm-generic support

[Oleksii]

On Wed, 2023-09-06 at 16:57 +0100, Anthony PERARD wrote:
> On Fri, Sep 01, 2023 at 07:02:14PM +0300, Oleksii Kurochko wrote:
> > diff --git a/xen/Makefile b/xen/Makefile
> > index f57e5a596c..698d6ddeb8 100644
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -667,6 +669,12 @@ endif # need-sub-make
> >  PHONY += FORCE
> >  FORCE:
> >  
> > +# Support for using generic headers in asm-generic
> > +PHONY += asm-generic
> > +asm-generic:
> > +   $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \
> > +   obj=arch/$(SRCARCH)/include/generated/asm
> 
> Could you introduce a $(asm-generic) macro in Kbuild.include like we
> already have for $(build) and $(clean)? As I think it would be nicer
> to
> have
> $(Q)$(MAKE) $(asm-
> generic)=arch/$(SRCARCH)/include/generated/asm
> in the main makefile.
> 
> Also, could you move the rule "asm-generic" within the "!config-
> build"
> part of the Makefile? Somewhere around the rule for $(TARGET), maybe
> just after the rule "_distclean".
> 
I pushed a new patch version tomorrow. I'll rework it based on your
comments here.

~ Oleksii

Re: [PATCH] xen/arm: Fix printk specifiers and arguments in iomem_remove_cb()

[Vikram Garhwal]

On Wed, Sep 06, 2023 at 12:30:14PM +0200, Michal Orzel wrote:
> When building Xen for arm32 with CONFIG_DTB_OVERLAY, the following
> error is printed:
> 
> common/dt-overlay.c: In function ‘iomem_remove_cb’:
> ././include/xen/config.h:55:24: error: format ‘%llx’ expects argument of type 
> ‘long long unsigned int’, but argument 3 has type ‘long unsigned int’ 
> [-Werror=format=]
> 
> Function parameters s and e (denoting MMIO region) are of type unsigned
> long and indicate frame numbers and not addresses. This also means that
> the arguments passed to printk() are incorrect (using PAGE_ALIGN() or
> PAGE_MASK ANDed with a frame number results in unwanted output). Fix it.
> 
> Take the opportunity to switch to %pd specifier to print domain id in
> a consolidated way.
> 
> Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal 
> functionalities")
> Signed-off-by: Michal Orzel 
Reviewed-by: Vikram Garhwal 
> ---
> After this patch (and the one for xl), we are left with one issue breaking CI:
> https://gitlab.com/xen-project/patchew/xen/-/jobs/5026938514
> ---
>  xen/common/dt-overlay.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
> index b07a6128dc99..5663a049e90a 100644
> --- a/xen/common/dt-overlay.c
> +++ b/xen/common/dt-overlay.c
> @@ -251,10 +251,8 @@ static int iomem_remove_cb(unsigned long s, unsigned 
> long e, void *dom,
>  rc = iomem_deny_access(d, s, e);
>  if ( rc )
>  {
> -printk(XENLOG_ERR "Unable to remove dom%d access to"
> -   " 0x%"PRIx64" - 0x%"PRIx64"\n",
> -   d->domain_id,
> -   s & PAGE_MASK, PAGE_ALIGN(e) - 1);
> +printk(XENLOG_ERR "Unable to remove %pd access to %#lx - %#lx\n",
> +   d, s, e);
>  }
>  else
>  *c += e - s + 1;
> -- 
> 2.25.1
> 

[PATCH v5 2/2] xen: move arm/include/asm/vm_event.h to stubs

[Oleksii Kurochko]

asm/vm_event.h is common for ARM and RISC-V so it will be moved to
stubs dir.

Original asm/vm_event.h from ARM was updated:
 * use SPDX-License-Identifier.
 * update comment messages of stubs.
 * update #ifdef
 * instead of "include " -> "public/vm_event.h"

As vm_event.h was moved to asm-generic then it is needed to create
Makefile in arm/include/asm/ and add generated-y += vm_event.h to
it.

Signed-off-by: Oleksii Kurochko 
---
Changes in V5:
 - Update the commit message
 - Remove Acked-by:...
 - Switch ARM to use asm-generic/vm_event.h
---
Changes in V4:
 - update path of vm_event.h from include/asm-generic/asm to include/asm-generic
---
Changes in V3:
 - add Acked-by: Stefano Stabellini  for "xen: move 
arm/include/asm/vm_event.h to asm-generic"
 - update SPDX tag.
 - move asm/vm_event.h to asm-generic.
---
Changes in V2:
 - change public/domctl.h to public/vm_event.h.
 - update commit message of [PATCH v2 2/2] xen: move arm/include/asm/vm_event.h 
to stubs
---
 xen/arch/arm/include/asm/Makefile   |  2 +
 xen/arch/arm/include/asm/vm_event.h | 66 -
 xen/include/asm-generic/vm_event.h  | 55 
 3 files changed, 57 insertions(+), 66 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/Makefile
 delete mode 100644 xen/arch/arm/include/asm/vm_event.h
 create mode 100644 xen/include/asm-generic/vm_event.h

diff --git a/xen/arch/arm/include/asm/Makefile 
b/xen/arch/arm/include/asm/Makefile
new file mode 100644
index 00..821addb0bf
--- /dev/null
+++ b/xen/arch/arm/include/asm/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+generic-y += vm_event.h
diff --git a/xen/arch/arm/include/asm/vm_event.h 
b/xen/arch/arm/include/asm/vm_event.h
deleted file mode 100644
index 4d861373b3..00
--- a/xen/arch/arm/include/asm/vm_event.h
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * vm_event.h: architecture specific vm_event handling routines
- *
- * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see .
- */
-
-#ifndef __ASM_ARM_VM_EVENT_H__
-#define __ASM_ARM_VM_EVENT_H__
-
-#include 
-#include 
-
-static inline int vm_event_init_domain(struct domain *d)
-{
-/* Nothing to do. */
-return 0;
-}
-
-static inline void vm_event_cleanup_domain(struct domain *d)
-{
-memset(&d->monitor, 0, sizeof(d->monitor));
-}
-
-static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
-  vm_event_response_t *rsp)
-{
-/* Not supported on ARM. */
-}
-
-static inline
-void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
-{
-/* Not supported on ARM. */
-}
-
-static inline
-void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp)
-{
-/* Not supported on ARM. */
-}
-
-static inline
-void vm_event_sync_event(struct vcpu *v, bool value)
-{
-/* Not supported on ARM. */
-}
-
-static inline
-void vm_event_reset_vmtrace(struct vcpu *v)
-{
-/* Not supported on ARM. */
-}
-
-#endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/asm-generic/vm_event.h 
b/xen/include/asm-generic/vm_event.h
new file mode 100644
index 00..29ab1b01b4
--- /dev/null
+++ b/xen/include/asm-generic/vm_event.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier:  GPL-2.0-only */
+/*
+ * vm_event.h: stubs for architecture specific vm_event handling routines
+ *
+ * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
+ */
+
+#ifndef __ASM_STUB_VM_EVENT_H__
+#define __ASM_STUB_VM_EVENT_H__
+
+#include 
+#include 
+
+static inline int vm_event_init_domain(struct domain *d)
+{
+/* Nothing to do. */
+return 0;
+}
+
+static inline void vm_event_cleanup_domain(struct domain *d)
+{
+memset(&d->monitor, 0, sizeof(d->monitor));
+}
+
+static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v,
+  vm_event_response_t *rsp)
+{
+/* Nothing to do. */
+}
+
+static inline
+void vm_event_register_write_resume(struct vcpu *v, vm_event_response_t *rsp)
+{
+/* Nothing to do. */
+}
+
+static inline
+void vm_event_emulate_check(struct vcpu *v, vm_event_response_t *rsp)
+{
+/* Nothing to do. */
+}
+
+static inline
+void vm_event_sync_event(struct vcpu *v, bool value)
+{
+/* Nothing to do. */
+}
+
+static inline
+void vm_event_reset_vmtrace(struct vcpu *v)
+{
+/* Nothing to do. */
+}
+
+#endif

[PATCH v5 1/2] xen: asm-generic support

[Oleksii Kurochko]

Some headers are shared between individual architectures or are empty.
To avoid duplication of these headers, asm-generic is introduced.

With the following patch, an architecture uses generic headers
mentioned in the file arch/$(ARCH)/include/asm/Makefile

To use a generic header is needed to add to
arch/$(ARCH)/include/asm/Makefile :
generic-y += 

For each mentioned header in arch/$(ARCH)/include/asm/Makefile,
the necessary wrapper in arch/$(ARCH)/include/generated/asm will be
generated.

As the base Makefile.asm-generic from Linux kernel was taken.
( 06c2afb862f9da8 "Linux 6.5-rc1" ).

Signed-off-by: Oleksii Kurochko 
---
Changes in V5:
 - Update the commit message
 - Update SPDX license in Makefile.
 - Remove code related to UML
 - Include $(src)/Makefile instead of $(kbuild-file) 
 - Update comment message in Makefile.asm-generic
 - Update .gitignore
 - Update path to generated headers in CFLAGS.
 - Use the latest version of Linux's Makefile.asm-generic
---
Changes in V4:
 - introduce asm-generic support
 - update commit message
---
Changes in V3:
 - Rename stubs dir to asm-generic
---
Changes in V2:
 - Nothing changed.
---
 .gitignore   |  1 +
 xen/Makefile | 10 +-
 xen/scripts/Makefile.asm-generic | 53 
 3 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 xen/scripts/Makefile.asm-generic

diff --git a/.gitignore b/.gitignore
index 50273adb8d..287166f8fc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -267,6 +267,7 @@ xen/arch/*/efi/efi.h
 xen/arch/*/efi/pe.c
 xen/arch/*/efi/runtime.c
 xen/arch/*/include/asm/asm-offsets.h
+xen/arch/*/include/generated
 xen/build-dir-cppcheck/
 xen/common/config_data.S
 xen/common/config.gz
diff --git a/xen/Makefile b/xen/Makefile
index f57e5a596c..a86b64303d 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -438,6 +438,7 @@ ifdef building_out_of_srctree
 endif
 CFLAGS += -I$(srctree)/include
 CFLAGS += -I$(srctree)/arch/$(SRCARCH)/include
+CFLAGS += -I$(objtree)/arch/$(SRCARCH)/include/generated
 
 # Note that link order matters!
 ALL_OBJS-y:= common/built_in.o
@@ -580,6 +581,7 @@ _clean:
rm -f $(TARGET).efi $(TARGET).efi.map $(TARGET).efi.elf 
$(TARGET).efi.stripped
rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h
rm -f .banner .allconfig.tmp include/xen/compile.h
+   rm -rf $(objtree)/arch/*/include/generated
 
 .PHONY: _distclean
 _distclean: clean
@@ -589,7 +591,7 @@ $(TARGET).gz: $(TARGET)
gzip -n -f -9 < $< > $@.new
mv $@.new $@
 
-$(TARGET): outputmakefile FORCE
+$(TARGET): outputmakefile asm-generic FORCE
$(Q)$(MAKE) $(build)=tools
$(Q)$(MAKE) $(build)=. include/xen/compile.h
$(Q)$(MAKE) $(build)=include all
@@ -667,6 +669,12 @@ endif # need-sub-make
 PHONY += FORCE
 FORCE:
 
+# Support for using generic headers in asm-generic
+PHONY += asm-generic
+asm-generic:
+   $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \
+   obj=arch/$(SRCARCH)/include/generated/asm
+
 # Declare the contents of the PHONY variable as phony.  We keep that
 # information in a variable so we can use it in if_changed and friends.
 .PHONY: $(PHONY)
diff --git a/xen/scripts/Makefile.asm-generic b/xen/scripts/Makefile.asm-generic
new file mode 100644
index 00..92a3a741c5
--- /dev/null
+++ b/xen/scripts/Makefile.asm-generic
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# include/asm-generic contains a lot of files that are used
+# verbatim by several architectures.
+#
+# This Makefile reads the file arch/$(SRCARCH)/include/asm/Makefile
+# and for each file listed in this file with generic-y creates
+# a small wrapper file in arch/$(SRCARCH)/include/generated/asm.
+
+PHONY := all
+all:
+
+src := $(subst /generated,,$(obj))
+
+include $(srctree)/scripts/Kbuild.include
+-include $(src)/Makefile
+
+redundant := $(filter $(mandatory-y) $(generated-y), $(generic-y))
+redundant += $(foreach f, $(generic-y), $(if $(wildcard 
$(srctree)/$(src)/$(f)),$(f)))
+redundant := $(sort $(redundant))
+$(if $(redundant),\
+   $(warning redundant generic-y found in $(src)/Kbuild: $(redundant)))
+
+# If arch does not implement mandatory headers, fallback to asm-generic ones.
+mandatory-y := $(filter-out $(generated-y), $(mandatory-y))
+generic-y   += $(foreach f, $(mandatory-y), $(if $(wildcard 
$(srctree)/$(src)/$(f)),,$(f)))
+
+generic-y   := $(addprefix $(obj)/, $(generic-y))
+generated-y := $(addprefix $(obj)/, $(generated-y))
+
+# Remove stale wrappers when the corresponding files are removed from generic-y
+old-headers := $(wildcard $(obj)/*.h)
+unwanted:= $(filter-out $(generic-y) $(generated-y),$(old-headers))
+
+quiet_cmd_wrap = WRAP$@
+  cmd_wrap = echo "\#include " > $@
+
+quiet_cmd_remove = REMOVE  $(unwanted)
+  cmd_remove = rm -f $(unwanted)
+
+all: $(generic-y)
+   $(if $(unwanted),$(call cmd,remove))
+   @:
+
+$(obj)/%.h:
+   $(call cmd,

[PATCH v5 0/2] introduce stub directory to storing empty/stub headers

[Oleksii Kurochko]

A lot of empty/stub headers should be introduced during the early steps of 
adding
support of new architecture.

An example can be found here:
1. 
https://lore.kernel.org/xen-devel/cover.1692181079.git.oleksii.kuroc...@gmail.com/
2. 
https://lore.kernel.org/xen-devel/a92f99e8f697da99d77bfde562a549dbef3760ce.1692816595.git.sanasta...@raptorengineering.com/

As part of the patch series, asm/vm_event.h was moved to the stubs directory 
because
It is the same for ARM, PPC, and RISC-V.

---
Changes in V5:
- Update SPDX license.
- Remove code related to UML in Makefile.asm-generic.
- Include $(src)/Makefile instead of $(kbuild-file).
- Update comment message in Makefile.asm-generic.
- Update .gitignore.
- Update path to generated headers in CFLAGS.
- Use the latest version of Linux's Makefile.asm-generic.
- Introduce asm-generic's vm_event.h.
- Switch ARM to use asm-generic/vm_event.h.
---
Changes in V4:
 - add asm-generic support
 - update path of vm_event.h from include/asm-generic/asm to include/asm-generic
---
Changes in V3:
 - add Acked-by: Stefano Stabellini  for "xen: move 
arm/include/asm/vm_event.h to asm-generic"
 - update SPDX tag.
 - move asm/vm_event.h to asm-generic.
 - rename stubs dir to asm-generic.

---
Changes in V2:
 - change public/domctl.h to public/vm_event.h.
 - update commit message of [PATCH v2 2/2] xen: move arm/include/asm/vm_event.h 
to stubs


Oleksii Kurochko (2):
  xen: asm-generic support
  xen: move arm/include/asm/vm_event.h to stubs

 .gitignore  |  1 +
 xen/Makefile| 10 -
 xen/arch/arm/include/asm/Makefile   |  2 +
 xen/arch/arm/include/asm/vm_event.h | 66 -
 xen/include/asm-generic/vm_event.h  | 55 
 xen/scripts/Makefile.asm-generic| 53 +++
 6 files changed, 120 insertions(+), 67 deletions(-)
 create mode 100644 xen/arch/arm/include/asm/Makefile
 delete mode 100644 xen/arch/arm/include/asm/vm_event.h
 create mode 100644 xen/include/asm-generic/vm_event.h
 create mode 100644 xen/scripts/Makefile.asm-generic

-- 
2.41.0

[linux-linus test] 182637: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-vhd   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-pvshim8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-ws16-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-credit1   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-libvirt-xsm  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-shadow8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemut-win7-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-win7-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-libvirt-raw  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start   fail REGR. vs. 182531
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-freebsd12-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-xl-multivcpu  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-libvirt-qcow2  8 xen-boot   fail REGR. vs. 182531
 test-amd64-amd64-qemuu-nested-intel  8 xen-boot  fail REGR. vs. 182531
 test-amd64-amd64-pair12 xen-boot/src_hostfail REGR. vs. 182531
 test-amd64-amd64-pair13 xen-boot/dst_hostfail REGR. vs. 182531
 test-amd64-amd64-libvirt-pair 12 xen-boot/src_host   fail REGR. vs. 182531
 test-amd64-amd64-libvirt-pair 13 xen-boot/dst_host   fail REGR. vs. 182531
 test-amd64-amd64-xl-xsm   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemut-ws16-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-qemuu-nested-amd  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-xl-pvhv2-amd  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-ovmf-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. 
vs. 182531
 test-amd64-amd64-xl-qemut-debianhvm-amd64  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 182531
 test-amd64-amd64-xl   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-pvhv2-intel  8 xen-boot  fail REGR. vs. 182531
 test-amd64-amd64-libvirt  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-freebsd11-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 
182531
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. 
vs. 182531
 test-amd64-amd64-pygrub   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-credit2   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-examine-uefi  8 reboot  fail REGR. vs. 182531
 test-amd64-amd64-examine-bios  8 reboot  fail REGR. vs. 182531
 test-amd64-coresched-amd64-xl  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-examine  8 reboot   fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 
182531

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-qcow2 10 host-ping-check-xen  fail pass in 182628

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail in 182628 
like 182531
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-check fail in 182628 never 
pass
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 182531
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 182531
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  

[PATCH v2] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0

[Jason Andryuk]

From: Roger Pau Monne 

The Processor _PDC buffer bits notify ACPI of the OS capabilities, and
so ACPI can adjust the return of other Processor methods taking the OS
capabilities into account.

When Linux is running as a Xen dom0, it's the hypervisor the entity
in charge of processor power management, and hence Xen needs to make
sure the capabilities reported in the _PDC buffer match the
capabilities of the driver in Xen.

Introduce a small helper to sanitize the buffer when running as Xen
dom0.

When Xen supports HWP, this serves as the equivalent of commit
a21211672c9a ("ACPI / processor: Request native thermal interrupt
handling via _OSC") to avoid SMM crashes.  Xen will set bit 12 in the
_PDC bits and the _PDC call will apply it.

[ jandryuk: Mention Xen HWP's need.  Move local variables ]
Signed-off-by: Roger Pau Monné 
Cc: sta...@vger.kernel.org
Signed-off-by: Jason Andryuk 
---
v2:
Move local variables in acpi_processor_eval_pdc() to reuse in both conditions.
---
 arch/x86/include/asm/xen/hypervisor.h |  6 ++
 arch/x86/xen/enlighten.c  | 19 +++
 drivers/acpi/processor_pdc.c  | 22 --
 3 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h 
b/arch/x86/include/asm/xen/hypervisor.h
index 5fc35f889cd1..0f88a7e450d3 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -63,4 +63,10 @@ void __init xen_pvh_init(struct boot_params *boot_params);
 void __init mem_map_via_hcall(struct boot_params *boot_params_p);
 #endif
 
+#ifdef CONFIG_XEN_DOM0
+void xen_sanitize_pdc(uint32_t *buf);
+#else
+static inline void xen_sanitize_pdc(uint32_t *buf) { BUG(); }
+#endif
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index b8db2148c07d..9f7fc11330a3 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -346,3 +346,22 @@ void xen_arch_unregister_cpu(int num)
 }
 EXPORT_SYMBOL(xen_arch_unregister_cpu);
 #endif
+
+#ifdef CONFIG_XEN_DOM0
+void xen_sanitize_pdc(uint32_t *buf)
+{
+   struct xen_platform_op op = {
+   .cmd= XENPF_set_processor_pminfo,
+   .interface_version  = XENPF_INTERFACE_VERSION,
+   .u.set_pminfo.id= -1,
+   .u.set_pminfo.type  = XEN_PM_PDC,
+   };
+   int ret;
+
+   set_xen_guest_handle(op.u.set_pminfo.pdc, buf);
+   ret = HYPERVISOR_platform_op(&op);
+   if (ret)
+   pr_info("sanitize of _PDC buffer bits from Xen failed: %d\n",
+   ret);
+}
+#endif
diff --git a/drivers/acpi/processor_pdc.c b/drivers/acpi/processor_pdc.c
index 18fb04523f93..9393dd4a3158 100644
--- a/drivers/acpi/processor_pdc.c
+++ b/drivers/acpi/processor_pdc.c
@@ -122,6 +122,11 @@ static acpi_status
 acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 {
acpi_status status = AE_OK;
+   union acpi_object *obj;
+   u32 *buffer = NULL;
+
+   obj = pdc_in->pointer;
+   buffer = (u32 *)(obj->buffer.pointer);
 
if (boot_option_idle_override == IDLE_NOMWAIT) {
/*
@@ -129,14 +134,19 @@ acpi_processor_eval_pdc(acpi_handle handle, struct 
acpi_object_list *pdc_in)
 * mode will be disabled in the parameter of _PDC object.
 * Of course C1_FFH access mode will also be disabled.
 */
-   union acpi_object *obj;
-   u32 *buffer = NULL;
-
-   obj = pdc_in->pointer;
-   buffer = (u32 *)(obj->buffer.pointer);
buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
-
}
+
+   if (xen_initial_domain()) {
+   /*
+* When Linux is running as Xen dom0, the hypervisor is the
+* entity in charge of the processor power management, and so
+* Xen needs to check the OS capabilities reported in the _PDC
+* buffer matches what the hypervisor driver supports.
+*/
+   xen_sanitize_pdc(buffer);
+   }
+
status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
 
if (ACPI_FAILURE(status))
-- 
2.41.0

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

[osstest service owner]

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

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  d8c3ff585b9fd10200b05aca77139de2789d0517
baseline version:
 xen  9a216e92de9f9011097e4f1fb55ff67ba0a21704

Last test of basis   182635  2023-09-05 13:00:27 Z1 days
Failing since182641  2023-09-06 02:02:08 Z0 days4 attempts
Testing same since   182674  2023-09-06 14:00:33 Z0 days1 attempts


People who touched revisions under test:
  David Gibson 
  Henry Wang 
  Julien Grall 
  Juline Grall 
  Michal Orzel 
  Stefano Stabellini 
  Stewart Hildebrand 
  Vikram Garhwal 
  Vikram Garhwal 

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
   9a216e92de..d8c3ff585b  d8c3ff585b9fd10200b05aca77139de2789d0517 -> smoke

[xen-unstable-smoke bisection] complete build-amd64

[osstest service owner]

branch xen-unstable-smoke
xenbranch xen-unstable-smoke
job build-amd64
testid xen-build

Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  xen git://xenbits.xen.org/xen.git
  Bug introduced:  61765a07e3d8a4dabbb8323b55f13ee75c80e54a
  Bug not present: aa7354beb1238e68126815e060f7d5ccda48fcb8
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/182682/


  commit 61765a07e3d8a4dabbb8323b55f13ee75c80e54a
  Author: Vikram Garhwal 
  Date:   Tue Sep 5 18:16:30 2023 -0700
  
  tools/xl: Add new xl command overlay for device tree overlay support
  
  Signed-off-by: Vikram Garhwal 
  Reviewed-by: Anthony PERARD 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable-smoke/build-amd64.xen-build.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/xen-unstable-smoke/build-amd64.xen-build 
--summary-out=tmp/182682.bisection-summary --basis-template=182635 
--blessings=real,real-bisect,real-retry xen-unstable-smoke build-amd64 xen-build
Searching for failure / basis pass:
 182661 fail [host=himrod0] / 182635 [host=godello0] 182631 [host=himrod2] 
182596 [host=himrod2] 182577 [host=himrod2] 182565 ok.
Failure / basis pass flights: 182661 / 182565
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
61765a07e3d8a4dabbb8323b55f13ee75c80e54a
Basis pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
e5522c71beaa83f2f5d2118724ace9f90c22e583
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c7437ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764
 
git://xenbits.xen.org/qemu-xen.git#0df9387c8983e1b1e72d8c574356f572342c03e6-0df9387c8983e1b1e72d8c574356f572342c03e6
 
git://xenbits.xen.org/xen.git#e5522c71beaa83f2f5d2118724ace9f90c22e583-61765a07e3d8a4dabbb8323b55f13ee75c80e54a
Loaded 5001 nodes in revision graph
Searching for test results:
 182565 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
e5522c71beaa83f2f5d2118724ace9f90c22e583
 182577 [host=himrod2]
 182596 [host=himrod2]
 182631 [host=himrod2]
 182635 [host=godello0]
 182641 [host=himrod2]
 182644 [host=himrod2]
 182645 [host=himrod2]
 182647 [host=himrod2]
 182648 [host=himrod2]
 182649 [host=himrod2]
 182651 [host=himrod2]
 182652 [host=himrod2]
 182653 [host=himrod2]
 182655 [host=himrod2]
 182646 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
61765a07e3d8a4dabbb8323b55f13ee75c80e54a
 182656 [host=himrod2]
 182657 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
e5522c71beaa83f2f5d2118724ace9f90c22e583
 182659 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
61765a07e3d8a4dabbb8323b55f13ee75c80e54a
 182662 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
427b6a571cd275015c201abd5b6a7843452d95a0
 182663 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
3b27fff436394762b9bc71fbddf075d276fcceb5
 182666 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
7e5c4a8b86f12942de0566b1d61f71d15774364b
 182668 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
f61243530e2a1e10f9414653d37d99df61fd88ad
 182670 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
aa7354beb1238e68126815e060f7d5ccda48fcb8
 182661 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
61765a07e3d8a4dabbb8323b55f13ee75c80e54a
 182672 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
61765a07e3d8a4dabbb8323b55f13ee75c80e54a
 182676 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
aa7354beb1238e68126815e060f7d5ccda48fcb8
 182678 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
61765a07e3d8a4dabbb8323b55f13ee75c80e54a
 182680 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
aa7354beb1238e68126815e060f7d5ccda48fcb8
 182682 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
0df9387c8983e1b1e72d8c574356f572342c03e6 
61765a07e3d8a4dabbb8323b55f13ee75c80e54a
Searching for interesting versions
 Result found: flight 182565 (pa

[ovmf test] 182665: all pass - PUSHED

[osstest service owner]

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

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 4d196352f35ac516b477e568265b4e537b0283d8
baseline version:
 ovmf d1e4a16f868fb1ccfa8bed8eaba5f9617ec8382d

Last test of basis   182654  2023-09-06 08:12:34 Z0 days
Testing same since   182665  2023-09-06 11:10:57 Z0 days1 attempts


People who touched revisions under test:
  Mike Beaton 
  Oliver Smith-Denny 

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
   d1e4a16f86..4d196352f3  4d196352f35ac516b477e568265b4e537b0283d8 -> 
xen-tested-master

Re: [XEN PATCH] xen/types: address Rule 10.1 for macro BITS_TO_LONGS

[Jan Beulich]

On 06.09.2023 17:57, Nicola Vetrini wrote:
> On 05/09/2023 10:33, Jan Beulich wrote:
>> On 05.09.2023 10:20, Nicola Vetrini wrote:
>>> On 05/09/2023 09:46, Jan Beulich wrote:
 On 05.09.2023 09:31, Nicola Vetrini wrote:
> Given its use in the declaration
> 'DECLARE_BITMAP(features, IOMMU_FEAT_count)' the argument
> 'bits' has essential type 'enum iommu_feature', which is not
> allowed by the Rule as an operand to the addition operator.
> Given that its value can be represented by a signed integer,
> the explicit cast resolves the violation.

 Wait - why would this lead to a change to BITS_TO_LONGS()? And if 
 that
 was to be changed, why plain int? I don't think negative input makes
 sense there, and in principle I'd expect values beyond 4 billion to
 also be permissible (even if likely no such use will ever appear in a
 DECLARE_BITMAP(), but elsewhere it may make sense). Even going to
 "unsigned long" may be too limiting ...

>>>
>>> You have a point. I can think of doing it like this:
>>> DECLARE_BITMAP(features, (int)IOMMU_FEAT_count)
>>> on the grounds that the enum constant is representable in an int, and 
>>> it
>>> does not seem likely
>>> to get much bigger.
>>> Having an unsigned cast requires making the whole expression
>>> essentially unsigned, otherwise Rule 10.4 is violated because
>>> BITS_PER_LONG is
>>> essentially signed. This can be done, but it depends on how
>>> BITS_TO_LONGS will be/is used.
>>
>> It'll need looking closely, yes, but I expect that actually wants to be 
>> an
>> unsigned constant. I wouldn't be surprised if some use of 
>> DECLARE_BITMAP()
>> appeared (or already existed) where the 2nd argument involves sizeof() 
>> in
>> some way.
>>
> 
> I think there's one with ARRAY_SIZE. In my opinion this can be resolved 
> as follows:
> 
> #define BYTES_PER_LONG (1U << LONG_BYTEORDER) // the essential type gets 
> from signed to unsigned
> 
> #define BITS_TO_LONGS(bits) \
>  (((unsigned long long)(bits)+BITS_PER_LONG-1U)/BITS_PER_LONG) // 
> same here

Except, as said before, I consider any kind of cast on "bits" latently
problematic.

Jan

Re: [PATCH v4 1/2] xen: asm-generic support

[Anthony PERARD]

On Fri, Sep 01, 2023 at 07:02:14PM +0300, Oleksii Kurochko wrote:
> diff --git a/xen/Makefile b/xen/Makefile
> index f57e5a596c..698d6ddeb8 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -667,6 +669,12 @@ endif # need-sub-make
>  PHONY += FORCE
>  FORCE:
>  
> +# Support for using generic headers in asm-generic
> +PHONY += asm-generic
> +asm-generic:
> + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \
> + obj=arch/$(SRCARCH)/include/generated/asm

Could you introduce a $(asm-generic) macro in Kbuild.include like we
already have for $(build) and $(clean)? As I think it would be nicer to
have
$(Q)$(MAKE) $(asm-generic)=arch/$(SRCARCH)/include/generated/asm
in the main makefile.

Also, could you move the rule "asm-generic" within the "!config-build"
part of the Makefile? Somewhere around the rule for $(TARGET), maybe
just after the rule "_distclean".

Cheers,

-- 
Anthony PERARD

Re: [XEN PATCH] xen/types: address Rule 10.1 for macro BITS_TO_LONGS

[Nicola Vetrini]

On 05/09/2023 10:33, Jan Beulich wrote:

On 05.09.2023 10:20, Nicola Vetrini wrote:

On 05/09/2023 09:46, Jan Beulich wrote:

On 05.09.2023 09:31, Nicola Vetrini wrote:

Given its use in the declaration
'DECLARE_BITMAP(features, IOMMU_FEAT_count)' the argument
'bits' has essential type 'enum iommu_feature', which is not
allowed by the Rule as an operand to the addition operator.
Given that its value can be represented by a signed integer,
the explicit cast resolves the violation.


Wait - why would this lead to a change to BITS_TO_LONGS()? And if 
that

was to be changed, why plain int? I don't think negative input makes
sense there, and in principle I'd expect values beyond 4 billion to
also be permissible (even if likely no such use will ever appear in a
DECLARE_BITMAP(), but elsewhere it may make sense). Even going to
"unsigned long" may be too limiting ...



You have a point. I can think of doing it like this:
DECLARE_BITMAP(features, (int)IOMMU_FEAT_count)
on the grounds that the enum constant is representable in an int, and 
it

does not seem likely
to get much bigger.
Having an unsigned cast requires making the whole expression
essentially unsigned, otherwise Rule 10.4 is violated because
BITS_PER_LONG is
essentially signed. This can be done, but it depends on how
BITS_TO_LONGS will be/is used.


It'll need looking closely, yes, but I expect that actually wants to be 
an
unsigned constant. I wouldn't be surprised if some use of 
DECLARE_BITMAP()
appeared (or already existed) where the 2nd argument involves sizeof() 
in

some way.



I think there's one with ARRAY_SIZE. In my opinion this can be resolved 
as follows:


#define BYTES_PER_LONG (1U << LONG_BYTEORDER) // the essential type gets 
from signed to unsigned


#define BITS_TO_LONGS(bits) \
(((unsigned long long)(bits)+BITS_PER_LONG-1U)/BITS_PER_LONG) // 
same here



--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -23,7 +23,7 @@ typedef signed long ssize_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;

 #define BITS_TO_LONGS(bits) \
-(((bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
+(((int)(bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
 #define DECLARE_BITMAP(name,bits) \
 unsigned long name[BITS_TO_LONGS(bits)]



Furthermore, as always - if this was to be touched, please take care
of style violations (numerous missing blanks) at this occasion.


Then the whole file needs a cleanup.


Perhaps, but going as we touch things is generally deemed better than 
doing
a single cleanup-only patch. First and foremost to not unduly affect 
the

usefulness of "git blame" and alike.



Ok

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-arndale   8 xen-bootfail pass in 182673-retest
 test-amd64-amd64-pygrub   19 guest-localmigrate/x10 fail pass in 182673-retest
 test-amd64-i386-qemuu-rhel6hvm-amd 14 guest-start/redhat.repeat fail pass in 
182673-retest
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat fail pass in 
182673-retest

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

Re: [PATCH v4 1/2] xen: asm-generic support

[Anthony PERARD]

On Wed, Sep 06, 2023 at 01:03:32PM +, Bertrand Marquis wrote:
> > On 6 Sep 2023, at 13:37, Oleksii  wrote:
> > On Mon, 2023-09-04 at 13:39 +, Bertrand Marquis wrote:
> >>> diff --git a/xen/scripts/Makefile.asm-generic
> >>> b/xen/scripts/Makefile.asm-generic
> >>> new file mode 100644
> >>> index 00..0aac3f50b8
> >>> --- /dev/null
> >>> +++ b/xen/scripts/Makefile.asm-generic
> >>> +
> >>> +$(obj)/%.h:
> >>> + $(call cmd,wrap)
> >> 
> >> In fact i think your files are not generated in the srctree so this
> >> will break if you use out of tree compilation.
> > I checked that and in both cases it generated in correct place.
> > 
> > First case:
> > $ CONTAINER_NO_PULL=1 CONTAINER=riscv64
> > ./automation/scripts/containerize make XEN_TARGET_ARCH=riscv64 -C xen
> > build
> > 
> > $ ls -la xen/arch/riscv/include/generated/asm/vm_event.h 
> > -rw-r--r--. 1 ok ok 34 вер  6 14:32
> > xen/arch/riscv/include/generated/asm/vm_event.h
> > 
> > Second case: ( out-of-tree )
> > $ CONTAINER_NO_PULL=1 CONTAINER=riscv64
> > ./automation/scripts/containerize make O=xen_build
> > XEN_TARGET_ARCH=riscv64 -C xen build V=1
> > 
> > $ ls -la xen/xen_build/arch/riscv/include/generated/asm/vm_event.h 
> > -rw-r--r--. 1 ok ok 34 вер  6 14:34
> > xen/xen_build/arch/riscv/include/generated/asm/vm_event.h
> > 
> > Could you please clarify if there's somithing I might have overlooked?
> 
> No I do not think you did.
> 
> So obj is empty I would guess.
> Anyway you just need to make sure that CFLAGS is coherent with what you 
> generate.
> So if you generate in $(obj) you need to make sure you use the same in CFLAGS 
> i would say.
> 
> @anthony: any advice here ?

The use of $(obj) looks fine here. $(obj) is temporary and set for a
single subdirectory. $(obj) is set by whatever is calling
"Makefile.asm-generic", or "Rules.mk" is other cases.

In this patch, you can see $(obj) been set in "xen/Makefile":
$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic 
obj=arch/$(SRCARCH)/include/generated/asm

Another thing to know, `make` runs from the build directory, so in
Oleksii second case, `make` is running in "xen/xen_build", so $(obj) point
to a directory within the build directory.

For the CFLAGS, we can use $(objtree) (even if it is just "."), which
always point to the top directory of the build tree.

I hope that clarifies things.

Cheers,

-- 
Anthony PERARD

Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry

[Jan Beulich]

On 06.09.2023 16:45, Anthony PERARD wrote:
> On Wed, Sep 06, 2023 at 02:50:22PM +0200, Jan Beulich wrote:
>> On 06.09.2023 14:40, Anthony PERARD wrote:
>>> On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote:
 If the F: description is to be trusted, the two xen/arch/x86/hvm/
 lines were fully redundant with the earlier wildcard ones. Arch header
 files, otoh, were no longer covered by anything as of the move from
 include/asm-*/ to arch/*/include/asm/. Further also generalize (by
 folding) the x86- and Arm-specific mem_access.c entries.

 Finally, again assuming the F: description can be trusted, there's no
 point listing arch/, common/, and include/ entries separately. Fold
 them all.

 Signed-off-by: Jan Beulich 
 ---
 -F:xen/arch/*/monitor.c
 -F:xen/arch/*/vm_event.c
 -F:xen/arch/arm/mem_access.c
 -F:xen/arch/x86/include/asm/hvm/monitor.h
 -F:xen/arch/x86/include/asm/hvm/vm_event.h
 -F:xen/arch/x86/mm/mem_access.c
 -F:xen/arch/x86/hvm/monitor.c
 -F:xen/arch/x86/hvm/vm_event.c
 -F:xen/common/mem_access.c
 -F:xen/common/monitor.c
 -F:xen/common/vm_event.c
 -F:xen/include/*/mem_access.h
 -F:xen/include/*/monitor.h
 -F:xen/include/*/vm_event.h
 +F:xen/*/mem_access.[ch]
 +F:xen/*/monitor.[ch]
 +F:xen/*/vm_event.[ch]
>>>
>>>
>>> Hi,
>>>
>>> Did you mean to for example change the maintainer ship of
>>> "xen/arch/x86/mm/mem_access.c"? Before it was:
>>> - VM EVENT, MEM ACCESS and MONITOR
>>> - X86 MEMORY MANAGEMENT
>>> - X86 ARCHITECTURE
>>> And now, it's just:
>>> - X86 MEMORY MANAGEMENT
>>> - X86 ARCHITECTURE
>>>
>>> (see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c)
>>>
>>> Also, now "xen/include/xen/monitor.h" is only "THE REST".
>>
>> No, no change of maintainership was intended. But there was an uncertainty,
>> which is why I said "assuming the F: description can be trusted". So ...
>>
>>> On the other hand, there's no change for "xen/common/monitor.c", so the
>>> pattern works for this particular file.
>>
>> ... together with this observation, I take it that
>>
>> F:   */net/* all files in "any top level directory"/net
>>
>> is actually at best misleading / ambiguous - I read it as not just a single
>> level of directories, but it may well be that that's what is meant. At
> 
> I guess the ambiguity would lie in the word "files". Here, "files" is a
> single file and not a directory, unlike the shell globing which would
> include directories with a '*'.
> 
> The first '*' is described at "any top level directory", but it is also
> "only top level directory". This kind of tells me that there is only a
> single level of directories that is match by '*'.
> 
>> which point the question is how "any number of directories" could be
>> expressed. Would **/ or .../**/... work here? I'm afraid my Perl is far
>> from sufficient to actually spot where (and hence how) this is handled in
>> the script.
> 
> I think you could write a regexp with the "N:" type instead of "F:".
> This is described Linux's MAINTAINERS file, but not ours, yet our
> get_maintainer.pl script has the functionality. It might be nice to be
> able to write just '**' but until someone implement that, we could go
> for a regex, which is more complicated and more prone to mistake.
> 
> So I think in the short-term, you want:
> 
> N:^xen/.*/mem_access\.[ch]$
> N:^xen/.*/monitor\.[ch]$
> N:^xen/.*/vm_event\.[ch]$

Except that as per Linux'es description F: and N: differ beyond how
file names are expressed / matched. I don't think I want to be the
one to introduce something which I've been complaining about on the
Linux side (people being Cc-ed just because at some point they ended
up touching a file).

I conclude that for the time being I ought to revert that change of
mine.

Jan

> As for adding "**", there's maybe something to do with
> "file_match_pattern()" in get_maintainer.pl, this function compare the
> number of '/' in both the pattern and the filepath to find out if a '*'
> only match one level of directory or more.
> 
> Cheers,
> 

Re: [PATCH v4 1/2] xen: asm-generic support

[Oleksii]

On Wed, 2023-09-06 at 16:06 +0100, Anthony PERARD wrote:
> On Wed, Sep 06, 2023 at 02:40:29PM +0200, Jan Beulich wrote:
> > On 06.09.2023 14:28, Oleksii wrote:
> > > On Wed, 2023-09-06 at 14:16 +0200, Jan Beulich wrote:
> > > > You've removed quite a bit of functionality, comparing with the
> > > > Linux
> > > > original. May I ask why that is? What they have all makes sense
> > > > to
> > > > me,
> > > > and looks to be useful down the road.
> > > I decided to take only minimum needed now but if you think it
> > > would be
> > > better to use the latest Linux's Makefile.asm-generic then I will
> > > update it in the next patch version.
> > 
> > Unless there are things getting in the way, I indeed think it would
> > be
> > better to take the whole thing largely verbatim (a few edits are
> > unavoidable afaict).
> 
> Yes, I think it would be useful as well to take most of the file from
> Linux. At least, this would include the "$(unwanted)" variable, which
> is
> used to remove old wrapper when an header is removed from "generic-
> y".
> But instead of the "-include $(kbuild-file)", could you include
> "$(src)/Makefile" instead like we do in "Rules.mk"? We don't have
> $(kbuild-file) macro anyway, and I don't see the point in using files
> name "Kbuild" when "Makefile" works and is enough.
> 
Thank you for your feedback.

I'll take into account your suggestions in the next patch version.

~ Oleksii

Re: [XEN PATCH 1/2] xen: apply deviation for Rule 8.4 (asm-only definitions)

[Nicola Vetrini]

On 04/09/2023 10:09, Jan Beulich wrote:

On 04.09.2023 09:19, Nicola Vetrini wrote:

On 04/09/2023 09:02, Jan Beulich wrote:

Further in the cover letter you say "Deviating variables needs more
care, and
is therefore postponed to another patch", yet then here you annotate 
a

couple
of variables as well. Could you clarify what the criteria are for
"needs more
care"?


I see. I did not intend for those changes to end up in this patch,
although those two are
clearly only used by asm code and therefore are excepted.
Most of the variables I left out are also used by C code, therefore 
they

may be eligible for
a declaration, but where to put this declaration requires a careful
examination in my opinion.
They are not too many, tough.


Anything also used by C code (and not in the same CU) clearly wants a
declaration.

Jan


I think that a declaration (in an header) is a good idea also when a 
symbol is defined in a translation unit
and it used both by C code in that TU and asm modules. This also allows 
C code that uses

that symbol to be moved around freely without other changes.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [PATCH v4 1/2] xen: asm-generic support

[Anthony PERARD]

On Wed, Sep 06, 2023 at 02:40:29PM +0200, Jan Beulich wrote:
> On 06.09.2023 14:28, Oleksii wrote:
> > On Wed, 2023-09-06 at 14:16 +0200, Jan Beulich wrote:
> >> You've removed quite a bit of functionality, comparing with the Linux
> >> original. May I ask why that is? What they have all makes sense to
> >> me,
> >> and looks to be useful down the road.
> > I decided to take only minimum needed now but if you think it would be
> > better to use the latest Linux's Makefile.asm-generic then I will
> > update it in the next patch version.
> 
> Unless there are things getting in the way, I indeed think it would be
> better to take the whole thing largely verbatim (a few edits are
> unavoidable afaict).

Yes, I think it would be useful as well to take most of the file from
Linux. At least, this would include the "$(unwanted)" variable, which is
used to remove old wrapper when an header is removed from "generic-y".
But instead of the "-include $(kbuild-file)", could you include
"$(src)/Makefile" instead like we do in "Rules.mk"? We don't have
$(kbuild-file) macro anyway, and I don't see the point in using files
name "Kbuild" when "Makefile" works and is enough.

Thanks,

-- 
Anthony PERARD

Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry

[Anthony PERARD]

On Wed, Sep 06, 2023 at 02:50:22PM +0200, Jan Beulich wrote:
> On 06.09.2023 14:40, Anthony PERARD wrote:
> > On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote:
> >> If the F: description is to be trusted, the two xen/arch/x86/hvm/
> >> lines were fully redundant with the earlier wildcard ones. Arch header
> >> files, otoh, were no longer covered by anything as of the move from
> >> include/asm-*/ to arch/*/include/asm/. Further also generalize (by
> >> folding) the x86- and Arm-specific mem_access.c entries.
> >>
> >> Finally, again assuming the F: description can be trusted, there's no
> >> point listing arch/, common/, and include/ entries separately. Fold
> >> them all.
> >>
> >> Signed-off-by: Jan Beulich 
> >> ---
> >> -F:xen/arch/*/monitor.c
> >> -F:xen/arch/*/vm_event.c
> >> -F:xen/arch/arm/mem_access.c
> >> -F:xen/arch/x86/include/asm/hvm/monitor.h
> >> -F:xen/arch/x86/include/asm/hvm/vm_event.h
> >> -F:xen/arch/x86/mm/mem_access.c
> >> -F:xen/arch/x86/hvm/monitor.c
> >> -F:xen/arch/x86/hvm/vm_event.c
> >> -F:xen/common/mem_access.c
> >> -F:xen/common/monitor.c
> >> -F:xen/common/vm_event.c
> >> -F:xen/include/*/mem_access.h
> >> -F:xen/include/*/monitor.h
> >> -F:xen/include/*/vm_event.h
> >> +F:xen/*/mem_access.[ch]
> >> +F:xen/*/monitor.[ch]
> >> +F:xen/*/vm_event.[ch]
> > 
> > 
> > Hi,
> > 
> > Did you mean to for example change the maintainer ship of
> > "xen/arch/x86/mm/mem_access.c"? Before it was:
> > - VM EVENT, MEM ACCESS and MONITOR
> > - X86 MEMORY MANAGEMENT
> > - X86 ARCHITECTURE
> > And now, it's just:
> > - X86 MEMORY MANAGEMENT
> > - X86 ARCHITECTURE
> > 
> > (see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c)
> > 
> > Also, now "xen/include/xen/monitor.h" is only "THE REST".
> 
> No, no change of maintainership was intended. But there was an uncertainty,
> which is why I said "assuming the F: description can be trusted". So ...
> 
> > On the other hand, there's no change for "xen/common/monitor.c", so the
> > pattern works for this particular file.
> 
> ... together with this observation, I take it that
> 
>  F:   */net/* all files in "any top level directory"/net
> 
> is actually at best misleading / ambiguous - I read it as not just a single
> level of directories, but it may well be that that's what is meant. At

I guess the ambiguity would lie in the word "files". Here, "files" is a
single file and not a directory, unlike the shell globing which would
include directories with a '*'.

The first '*' is described at "any top level directory", but it is also
"only top level directory". This kind of tells me that there is only a
single level of directories that is match by '*'.

> which point the question is how "any number of directories" could be
> expressed. Would **/ or .../**/... work here? I'm afraid my Perl is far
> from sufficient to actually spot where (and hence how) this is handled in
> the script.

I think you could write a regexp with the "N:" type instead of "F:".
This is described Linux's MAINTAINERS file, but not ours, yet our
get_maintainer.pl script has the functionality. It might be nice to be
able to write just '**' but until someone implement that, we could go
for a regex, which is more complicated and more prone to mistake.

So I think in the short-term, you want:

N:  ^xen/.*/mem_access\.[ch]$
N:  ^xen/.*/monitor\.[ch]$
N:  ^xen/.*/vm_event\.[ch]$

As for adding "**", there's maybe something to do with
"file_match_pattern()" in get_maintainer.pl, this function compare the
number of '/' in both the pattern and the filepath to find out if a '*'
only match one level of directory or more.

Cheers,

-- 
Anthony PERARD

Re: [QEMU PATCH v4 09/13] virtio-gpu: Handle resource blob commands

[Akihiko Odaki]

On 2023/09/06 16:56, Huang Rui wrote:

On Wed, Sep 06, 2023 at 11:39:09AM +0800, Akihiko Odaki wrote:

On 2023/09/06 12:09, Huang Rui wrote:

On Tue, Sep 05, 2023 at 05:20:43PM +0800, Akihiko Odaki wrote:

On 2023/09/05 18:08, Huang Rui wrote:

On Thu, Aug 31, 2023 at 06:24:32PM +0800, Akihiko Odaki wrote:

On 2023/08/31 18:32, Huang Rui wrote:

From: Antonio Caggiano 

Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true

Signed-off-by: Antonio Caggiano 
Signed-off-by: Dmitry Osipenko 
Signed-off-by: Xenia Ragiadakou 
Signed-off-by: Huang Rui 
---

v1->v2:
- Remove unused #include "hw/virtio/virtio-iommu.h"

- Add a local function, called virgl_resource_destroy(), that is used
  to release a vgpu resource on error paths and in resource_unref.

- Remove virtio_gpu_virgl_resource_unmap from 
virtio_gpu_cleanup_mapping(),
  since this function won't be called on blob resources and also because
  blob resources are unmapped via virgl_cmd_resource_unmap_blob().

- In virgl_cmd_resource_create_blob(), do proper cleanup in error paths
  and move QTAILQ_INSERT_HEAD(&g->reslist, res, next) after the resource
  has been fully initialized.

- Memory region has a different life-cycle from virtio gpu resources
  i.e. cannot be released synchronously along with the vgpu resource.
  So, here the field "region" was changed to a pointer that will be
  released automatically once the memory region is unparented and all
  of its references have been released.
  Also, since the pointer can be used to indicate whether the blob
  is mapped, the explicit field "mapped" was removed.

- In virgl_cmd_resource_map_blob(), add check on the value of
  res->region, to prevent beeing called twice on the same resource.

- Remove direct references to parent_obj.

- Separate declarations from code.

 hw/display/virtio-gpu-virgl.c  | 213 +
 hw/display/virtio-gpu.c|   4 +-
 include/hw/virtio/virtio-gpu.h |   5 +
 meson.build|   4 +
 4 files changed, 225 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 312953ec16..17b634d4ee 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -17,6 +17,7 @@
 #include "trace.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
 
 #include "ui/egl-helpers.h"
 
@@ -78,9 +79,24 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,

 virgl_renderer_resource_create(&args, NULL, 0);
 }
 
+static void virgl_resource_destroy(VirtIOGPU *g,

+   struct virtio_gpu_simple_resource *res)
+{
+if (!res)
+return;
+
+QTAILQ_REMOVE(&g->reslist, res, next);
+
+virtio_gpu_cleanup_mapping_iov(g, res->iov, res->iov_cnt);
+g_free(res->addrs);
+
+g_free(res);
+}
+
 static void virgl_cmd_resource_unref(VirtIOGPU *g,
  struct virtio_gpu_ctrl_command *cmd)
 {
+struct virtio_gpu_simple_resource *res;
 struct virtio_gpu_resource_unref unref;
 struct iovec *res_iovs = NULL;
 int num_iovs = 0;
@@ -88,13 +104,22 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
 VIRTIO_GPU_FILL_CMD(unref);
 trace_virtio_gpu_cmd_res_unref(unref.resource_id);
 
+res = virtio_gpu_find_resource(g, unref.resource_id);

+
 virgl_renderer_resource_detach_iov(unref.resource_id,
&res_iovs,
&num_iovs);
 if (res_iovs != NULL && num_iovs != 0) {
 virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
+if (res) {
+res->iov = NULL;
+res->iov_cnt = 0;
+}
 }
+
 virgl_renderer_resource_unref(unref.resource_id);
+
+virgl_resource_destroy(g, res);
 }
 
 static void virgl_cmd_context_create(VirtIOGPU *g,

@@ -426,6 +451,183 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
 g_free(resp);
 }
 
+#ifdef HAVE_VIRGL_RESOURCE_BLOB

+
+static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
+   struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_simple_resource *res;
+struct virtio_gpu_resource_create_blob cblob;
+struct virgl_renderer_resource_create_blob_args virgl_args = { 0 };
+int ret;
+
+VIRTIO_GPU_FILL_CMD(cblob);
+virtio_gpu_create_blob_bswap(&cblob);
+trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+if (cblob.resource_id == 0) {
+qemu

[xen-unstable-smoke test] 182661: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 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  61765a07e3d8a4dabbb8323b55f13ee75c80e54a
baseline version:
 xen  9a216e92de9f9011097e4f1fb55ff67ba0a21704

Last test of basis   182635  2023-09-05 13:00:27 Z1 days
Testing same since   182641  2023-09-06 02:02:08 Z0 days3 attempts


People who touched revisions under test:
  David Gibson 
  Henry Wang 
  Julien Grall 
  Juline Grall 
  Stefano Stabellini 
  Vikram Garhwal 
  Vikram Garhwal 

jobs:
 build-arm64-xsm  pass
 build-amd64  fail
 build-armhf  pass
 build-amd64-libvirt  blocked 
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-amd64-libvirt blocked 



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

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

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

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


Not pushing.

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

[PATCH v1] tools/xentrace/xentrace_format: Add python 3 compatibility

[Javi Merino]

Closes issue #155

Signed-off-by: Javi Merino 
---

Tested using the format file in tools/xentrace/formats.  With this patch, both 
python2 and python3 produce the same output.

 tools/xentrace/xentrace_format | 63 +++---
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/tools/xentrace/xentrace_format b/tools/xentrace/xentrace_format
index 5ff85ae2e8..166ebae325 100644
--- a/tools/xentrace/xentrace_format
+++ b/tools/xentrace/xentrace_format
@@ -4,11 +4,15 @@
 
 # Program for reformatting trace buffer output according to user-supplied rules
 
+from __future__ import division
+from __future__ import print_function
+from __future__ import unicode_literals
+from builtins import str
+from past.utils import old_div
 import re, sys, string, signal, struct, os, getopt
 
 def usage():
-print >> sys.stderr, \
-  "Usage: " + sys.argv[0] + """ defs-file
+print("Usage: " + sys.argv[0] + """ defs-file
   Parses trace data in binary format, as output by Xentrace and
   reformats it according to the rules in a file of definitions.  The
   rules in this file should have the format ({ and } show grouping
@@ -29,7 +33,7 @@ def usage():
   this script may not be able to keep up with the output of xentrace
   if it is piped directly.  In these circumstances you should have
   xentrace output to a file for processing off-line.
-  """
+  """, file=sys.stderr)
 sys.exit(1)
 
 def read_defs(defs_file):
@@ -49,7 +53,7 @@ def read_defs(defs_file):
 
 m = reg.match(line)
 
-if not m: print >> sys.stderr, "Bad format file" ; sys.exit(1)
+if not m: print("Bad format file", file=sys.stderr) ; sys.exit(1)
 
 defs[str(eval(m.group(1)))] = m.group(2)
 
@@ -83,8 +87,8 @@ interrupted = 0
 
 try:
 defs = read_defs(arg[0])
-except IOError, exn:
-print exn
+except IOError as exn:
+print(exn)
 sys.exit(1)
 
 # structure of trace record (as output by xentrace):
@@ -117,12 +121,17 @@ TRC_PV_HYPERCALL_SUBCALL = 0x20100e
 NR_VECTORS = 256
 irq_measure = [{'count':0, 'tot_cycles':0, 'max_cycles':0}] * NR_VECTORS
 
+if sys.version_info >= (3, 0):
+stdin_bytes = sys.stdin.buffer
+else:
+stdin_bytes = sys.stdin
+
 i=0
 
 while not interrupted:
 try:
 i=i+1
-line = sys.stdin.read(struct.calcsize(HDRREC))
+line = stdin_bytes.read(struct.calcsize(HDRREC))
 if not line:
 break
 event = struct.unpack(HDRREC, line)[0]
@@ -140,43 +149,43 @@ while not interrupted:
 tsc = 0
 
 if tsc_in == 1:
-line = sys.stdin.read(struct.calcsize(TSCREC))
+line = stdin_bytes.read(struct.calcsize(TSCREC))
 if not line:
 break
 tsc = struct.unpack(TSCREC, line)[0]
 
 if n_data == 1:
-line = sys.stdin.read(struct.calcsize(D1REC))
+line = stdin_bytes.read(struct.calcsize(D1REC))
 if not line:
 break
 d1 = struct.unpack(D1REC, line)[0]
 if n_data == 2:
-line = sys.stdin.read(struct.calcsize(D2REC))
+line = stdin_bytes.read(struct.calcsize(D2REC))
 if not line:
 break
 (d1, d2) = struct.unpack(D2REC, line)
 if n_data == 3:
-line = sys.stdin.read(struct.calcsize(D3REC))
+line = stdin_bytes.read(struct.calcsize(D3REC))
 if not line:
 break
 (d1, d2, d3) = struct.unpack(D3REC, line)
 if n_data == 4:
-line = sys.stdin.read(struct.calcsize(D4REC))
+line = stdin_bytes.read(struct.calcsize(D4REC))
 if not line:
 break
 (d1, d2, d3, d4) = struct.unpack(D4REC, line)
 if n_data == 5:
-line = sys.stdin.read(struct.calcsize(D5REC))
+line = stdin_bytes.read(struct.calcsize(D5REC))
 if not line:
 break
 (d1, d2, d3, d4, d5) = struct.unpack(D5REC, line)
 if n_data == 6:
-line = sys.stdin.read(struct.calcsize(D6REC))
+line = stdin_bytes.read(struct.calcsize(D6REC))
 if not line:
 break
 (d1, d2, d3, d4, d5, d6) = struct.unpack(D6REC, line)
 if n_data == 7:
-line = sys.stdin.read(struct.calcsize(D7REC))
+line = stdin_bytes.read(struct.calcsize(D7REC))
 if not line:
 break
 (d1, d2, d3, d4, d5, d6, d7) = struct.unpack(D7REC, line)
@@ -211,7 +220,7 @@ while not interrupted:
 if cpu >= len(last_tsc):
 last_tsc += [0] * (cpu - len(last_tsc) + 1)
 elif tsc < last_tsc[cpu] and tsc_in == 1:
-print "TSC stepped backward cpu %d !  %d %d" % 
(cpu,tsc,last_tsc[cpu])
+print("TSC stepped backward cpu %d !  %d %d" % 
(cpu,tsc,last_tsc[cpu]))
 
 # 

Re: [PATCH v4 1/2] xen: asm-generic support

[Bertrand Marquis]

Hi Oleksii,

> On 6 Sep 2023, at 13:37, Oleksii  wrote:
> 
> Hello Bertrand
> 
> On Mon, 2023-09-04 at 13:39 +, Bertrand Marquis wrote:
>>> diff --git a/xen/scripts/Makefile.asm-generic
>>> b/xen/scripts/Makefile.asm-generic
>>> new file mode 100644
>>> index 00..0aac3f50b8
>>> --- /dev/null
>>> +++ b/xen/scripts/Makefile.asm-generic
>>> @@ -0,0 +1,23 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +# include/asm-generic contains a lot of files that are used
>>> +# verbatim by several architectures.
>>> +#
>>> +# This Makefile reads the file arch/$(SRCARCH)/include/asm/Kbuild
>>> +# and for each file listed in this file with generic-y creates
>>> +# a small wrapper file in $(obj)
>>> (arch/$(SRCARCH)/include/generated/asm)
>>> +
>>> +kbuild-file := $(srctree)/arch/$(SRCARCH)/include/asm/Kbuild
>>> +include $(kbuild-file)
>>> +
>>> +include scripts/Kbuild.include
>>> +
>>> +# Create output directory if not already present
>>> +_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
>>> +
>>> +quiet_cmd_wrap = WRAP$@
>>> +cmd_wrap = echo "\#include " >$@
>>> +
>>> +all: $(patsubst %, $(obj)/%, $(generic-y))
>>> +
>>> +$(obj)/%.h:
>>> + $(call cmd,wrap)
>> 
>> In fact i think your files are not generated in the srctree so this
>> will break if you use out of tree compilation.
> I checked that and in both cases it generated in correct place.
> 
> First case:
> $ CONTAINER_NO_PULL=1 CONTAINER=riscv64
> ./automation/scripts/containerize make XEN_TARGET_ARCH=riscv64 -C xen
> build
> 
> $ ls -la xen/arch/riscv/include/generated/asm/vm_event.h 
> -rw-r--r--. 1 ok ok 34 вер  6 14:32
> xen/arch/riscv/include/generated/asm/vm_event.h
> 
> Second case: ( out-of-tree )
> $ CONTAINER_NO_PULL=1 CONTAINER=riscv64
> ./automation/scripts/containerize make O=xen_build
> XEN_TARGET_ARCH=riscv64 -C xen build V=1
> 
> $ ls -la xen/xen_build/arch/riscv/include/generated/asm/vm_event.h 
> -rw-r--r--. 1 ok ok 34 вер  6 14:34
> xen/xen_build/arch/riscv/include/generated/asm/vm_event.h
> 
> Could you please clarify if there's somithing I might have overlooked?

No I do not think you did.

So obj is empty I would guess.
Anyway you just need to make sure that CFLAGS is coherent with what you 
generate.
So if you generate in $(obj) you need to make sure you use the same in CFLAGS i 
would say.

@anthony: any advice here ?

Cheers
Bertrand



> 
> ~ Oleksii
> 
> 
> 

Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry

[Jan Beulich]

On 06.09.2023 14:40, Anthony PERARD wrote:
> On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote:
>> If the F: description is to be trusted, the two xen/arch/x86/hvm/
>> lines were fully redundant with the earlier wildcard ones. Arch header
>> files, otoh, were no longer covered by anything as of the move from
>> include/asm-*/ to arch/*/include/asm/. Further also generalize (by
>> folding) the x86- and Arm-specific mem_access.c entries.
>>
>> Finally, again assuming the F: description can be trusted, there's no
>> point listing arch/, common/, and include/ entries separately. Fold
>> them all.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> -F:  xen/arch/*/monitor.c
>> -F:  xen/arch/*/vm_event.c
>> -F:  xen/arch/arm/mem_access.c
>> -F:  xen/arch/x86/include/asm/hvm/monitor.h
>> -F:  xen/arch/x86/include/asm/hvm/vm_event.h
>> -F:  xen/arch/x86/mm/mem_access.c
>> -F:  xen/arch/x86/hvm/monitor.c
>> -F:  xen/arch/x86/hvm/vm_event.c
>> -F:  xen/common/mem_access.c
>> -F:  xen/common/monitor.c
>> -F:  xen/common/vm_event.c
>> -F:  xen/include/*/mem_access.h
>> -F:  xen/include/*/monitor.h
>> -F:  xen/include/*/vm_event.h
>> +F:  xen/*/mem_access.[ch]
>> +F:  xen/*/monitor.[ch]
>> +F:  xen/*/vm_event.[ch]
> 
> 
> Hi,
> 
> Did you mean to for example change the maintainer ship of
> "xen/arch/x86/mm/mem_access.c"? Before it was:
> - VM EVENT, MEM ACCESS and MONITOR
> - X86 MEMORY MANAGEMENT
> - X86 ARCHITECTURE
> And now, it's just:
> - X86 MEMORY MANAGEMENT
> - X86 ARCHITECTURE
> 
> (see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c)
> 
> Also, now "xen/include/xen/monitor.h" is only "THE REST".

No, no change of maintainership was intended. But there was an uncertainty,
which is why I said "assuming the F: description can be trusted". So ...

> On the other hand, there's no change for "xen/common/monitor.c", so the
> pattern works for this particular file.

... together with this observation, I take it that

   F:   */net/* all files in "any top level directory"/net

is actually at best misleading / ambiguous - I read it as not just a single
level of directories, but it may well be that that's what is meant. At
which point the question is how "any number of directories" could be
expressed. Would **/ or .../**/... work here? I'm afraid my Perl is far
from sufficient to actually spot where (and hence how) this is handled in
the script.

Jan

Re: [PATCH v4 1/2] xen: asm-generic support

[Jan Beulich]

On 06.09.2023 14:28, Oleksii wrote:
> On Wed, 2023-09-06 at 14:16 +0200, Jan Beulich wrote:
>> On 01.09.2023 18:02, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/scripts/Makefile.asm-generic
>>> @@ -0,0 +1,23 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>
>> While this is what the Linux file presently says, you still want to
>> add
>> "-only", which is merely the newer name for the same thing.
>>
>>> +# include/asm-generic contains a lot of files that are used
>>> +# verbatim by several architectures.
>>> +#
>>> +# This Makefile reads the file arch/$(SRCARCH)/include/asm/Kbuild
>>> +# and for each file listed in this file with generic-y creates
>>> +# a small wrapper file in $(obj)
>>> (arch/$(SRCARCH)/include/generated/asm)
>>> +
>>> +kbuild-file := $(srctree)/arch/$(SRCARCH)/include/asm/Kbuild
>>> +include $(kbuild-file)
>>> +
>>> +include scripts/Kbuild.include
>>> +
>>> +# Create output directory if not already present
>>> +_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
>>> +
>>> +quiet_cmd_wrap = WRAP    $@
>>> +cmd_wrap = echo "\#include " >$@
>>> +
>>> +all: $(patsubst %, $(obj)/%, $(generic-y))
>>> +
>>> +$(obj)/%.h:
>>> +   $(call cmd,wrap)
>>
>> You've removed quite a bit of functionality, comparing with the Linux
>> original. May I ask why that is? What they have all makes sense to
>> me,
>> and looks to be useful down the road.
> I decided to take only minimum needed now but if you think it would be
> better to use the latest Linux's Makefile.asm-generic then I will
> update it in the next patch version.

Unless there are things getting in the way, I indeed think it would be
better to take the whole thing largely verbatim (a few edits are
unavoidable afaict).

Jan

Re: [PATCH v2] MAINTAINERS: consolidate vm-event/monitor entry

[Anthony PERARD]

On Thu, Aug 31, 2023 at 08:15:13AM +0200, Jan Beulich wrote:
> If the F: description is to be trusted, the two xen/arch/x86/hvm/
> lines were fully redundant with the earlier wildcard ones. Arch header
> files, otoh, were no longer covered by anything as of the move from
> include/asm-*/ to arch/*/include/asm/. Further also generalize (by
> folding) the x86- and Arm-specific mem_access.c entries.
> 
> Finally, again assuming the F: description can be trusted, there's no
> point listing arch/, common/, and include/ entries separately. Fold
> them all.
> 
> Signed-off-by: Jan Beulich 
> ---
> -F:   xen/arch/*/monitor.c
> -F:   xen/arch/*/vm_event.c
> -F:   xen/arch/arm/mem_access.c
> -F:   xen/arch/x86/include/asm/hvm/monitor.h
> -F:   xen/arch/x86/include/asm/hvm/vm_event.h
> -F:   xen/arch/x86/mm/mem_access.c
> -F:   xen/arch/x86/hvm/monitor.c
> -F:   xen/arch/x86/hvm/vm_event.c
> -F:   xen/common/mem_access.c
> -F:   xen/common/monitor.c
> -F:   xen/common/vm_event.c
> -F:   xen/include/*/mem_access.h
> -F:   xen/include/*/monitor.h
> -F:   xen/include/*/vm_event.h
> +F:   xen/*/mem_access.[ch]
> +F:   xen/*/monitor.[ch]
> +F:   xen/*/vm_event.[ch]


Hi,

Did you mean to for example change the maintainer ship of
"xen/arch/x86/mm/mem_access.c"? Before it was:
- VM EVENT, MEM ACCESS and MONITOR
- X86 MEMORY MANAGEMENT
- X86 ARCHITECTURE
And now, it's just:
- X86 MEMORY MANAGEMENT
- X86 ARCHITECTURE

(see ./scripts/get_maintainer.pl --sections -f xen/arch/x86/mm/mem_access.c)

Also, now "xen/include/xen/monitor.h" is only "THE REST".

On the other hand, there's no change for "xen/common/monitor.c", so the
pattern works for this particular file.

Cheers,

-- 
Anthony PERARD

Re: [XEN PATCH v3] drivers/video: make declarations of defined functions available

[Jan Beulich]

On 06.09.2023 14:36, Nicola Vetrini wrote:
> On 05/09/2023 16:42, Jan Beulich wrote:
>> On 01.09.2023 09:02, Nicola Vetrini wrote:
>>> +#else
>>> +#define vesa_early_init() ((void)0)
>>> +#define vesa_endboot(x)   ((void)0)
>>> +static inline void vesa_init(void) {};
>>
>> So why two #define-s and one inline function? Just because it was that
>> way originally doesn't mean it needs to stay that way. Then again are
>> the two #define-s actually needed at all? It looks like they were added
>> to vga.c just out of precaution, covering the "VGA but no VESA" case.
>> Now that things are properly moved to headers (and keyed to CONFIG_VGA)
>> I think we'd better omit those. They can be introduced again when we
>> gain a VGA/VESA split (and a CONFIG_VESA).
>>
> 
> Ok on uniforming them to inline functions.
> I don't have an opinion on the second matter. If you're otherwise okay 
> with the patch
> and no one objects I can drop them.

I am okay with the rest of the patch (as always subject to spotting
something else when looking at the next version), yes.

Jan

Re: [XEN PATCH v3] drivers/video: make declarations of defined functions available

[Nicola Vetrini]

On 05/09/2023 16:42, Jan Beulich wrote:

On 01.09.2023 09:02, Nicola Vetrini wrote:

The declarations for 'vesa_{init,early_init,endboot}' needed by
'xen/drivers/video/vesa.c' and 'fill_console_start_info' in 'vga.c'
are now available by moving the relative code inside 'vga.h'.

While moving the code, the alternative definitions are now guarded by
CONFIG_VGA, since it implies all previously used constants.


But such an implication doesn't mean said adjustment is correct. 
Easiest

would likely be to simply drop that half sentence.



Maybe I didn't phrase that correctly, I'm ok with just dropping the last 
part

(it can be traced to the Kconfig anyway).


--- a/xen/include/xen/vga.h
+++ b/xen/include/xen/vga.h
@@ -13,6 +13,14 @@

 #ifdef CONFIG_VGA
 extern struct xen_vga_console_info vga_console_info;
+int fill_console_start_info(struct dom0_vga_console_info *);
+void vesa_init(void);
+void vesa_early_init(void);
+void vesa_endboot(bool_t keep);


Nit: Just "bool" please.



Right, I missed it.


+#else
+#define vesa_early_init() ((void)0)
+#define vesa_endboot(x)   ((void)0)
+static inline void vesa_init(void) {};


So why two #define-s and one inline function? Just because it was that
way originally doesn't mean it needs to stay that way. Then again are
the two #define-s actually needed at all? It looks like they were added
to vga.c just out of precaution, covering the "VGA but no VESA" case.
Now that things are properly moved to headers (and keyed to CONFIG_VGA)
I think we'd better omit those. They can be introduced again when we
gain a VGA/VESA split (and a CONFIG_VESA).



Ok on uniforming them to inline functions.
I don't have an opinion on the second matter. If you're otherwise okay 
with the patch

and no one objects I can drop them.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [PATCH v4 1/2] xen: asm-generic support

[Oleksii]

On Wed, 2023-09-06 at 14:16 +0200, Jan Beulich wrote:
> On 01.09.2023 18:02, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/scripts/Makefile.asm-generic
> > @@ -0,0 +1,23 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> While this is what the Linux file presently says, you still want to
> add
> "-only", which is merely the newer name for the same thing.
> 
> > +# include/asm-generic contains a lot of files that are used
> > +# verbatim by several architectures.
> > +#
> > +# This Makefile reads the file arch/$(SRCARCH)/include/asm/Kbuild
> > +# and for each file listed in this file with generic-y creates
> > +# a small wrapper file in $(obj)
> > (arch/$(SRCARCH)/include/generated/asm)
> > +
> > +kbuild-file := $(srctree)/arch/$(SRCARCH)/include/asm/Kbuild
> > +include $(kbuild-file)
> > +
> > +include scripts/Kbuild.include
> > +
> > +# Create output directory if not already present
> > +_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
> > +
> > +quiet_cmd_wrap = WRAP    $@
> > +cmd_wrap = echo "\#include " >$@
> > +
> > +all: $(patsubst %, $(obj)/%, $(generic-y))
> > +
> > +$(obj)/%.h:
> > +   $(call cmd,wrap)
> 
> You've removed quite a bit of functionality, comparing with the Linux
> original. May I ask why that is? What they have all makes sense to
> me,
> and looks to be useful down the road.
I decided to take only minimum needed now but if you think it would be
better to use the latest Linux's Makefile.asm-generic then I will
update it in the next patch version.

~ Oleksii

Re: [PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY

[Anthony PERARD]

On Wed, Sep 06, 2023 at 10:36:14AM +0200, Michal Orzel wrote:
> main_dt_overlay() makes a call to libxl_dt_overlay() which is for now
> only compiled for Arm. This causes the build failure as reported by
> gitlab CI and OSSTEST. Fix it by guarding the function, prototype and
> entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage
> over regular Arm guard so that the code will not need to be modified again
> if other architecture gain support for this feature.
> 
> Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree 
> overlay support")
> Reported-by: Jan Beulich 
> Signed-off-by: Michal Orzel 

Reviewed-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD

Re: [PATCH v4 1/2] xen: asm-generic support

[Jan Beulich]

On 01.09.2023 18:02, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/scripts/Makefile.asm-generic
> @@ -0,0 +1,23 @@
> +# SPDX-License-Identifier: GPL-2.0

While this is what the Linux file presently says, you still want to add
"-only", which is merely the newer name for the same thing.

> +# include/asm-generic contains a lot of files that are used
> +# verbatim by several architectures.
> +#
> +# This Makefile reads the file arch/$(SRCARCH)/include/asm/Kbuild
> +# and for each file listed in this file with generic-y creates
> +# a small wrapper file in $(obj) (arch/$(SRCARCH)/include/generated/asm)
> +
> +kbuild-file := $(srctree)/arch/$(SRCARCH)/include/asm/Kbuild
> +include $(kbuild-file)
> +
> +include scripts/Kbuild.include
> +
> +# Create output directory if not already present
> +_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
> +
> +quiet_cmd_wrap = WRAP$@
> +cmd_wrap = echo "\#include " >$@
> +
> +all: $(patsubst %, $(obj)/%, $(generic-y))
> +
> +$(obj)/%.h:
> + $(call cmd,wrap)

You've removed quite a bit of functionality, comparing with the Linux
original. May I ask why that is? What they have all makes sense to me,
and looks to be useful down the road.

Jan

Re: [PATCH v4 1/2] xen: asm-generic support

[Oleksii]

Hello Bertrand

On Mon, 2023-09-04 at 13:39 +, Bertrand Marquis wrote:
> > diff --git a/xen/scripts/Makefile.asm-generic
> > b/xen/scripts/Makefile.asm-generic
> > new file mode 100644
> > index 00..0aac3f50b8
> > --- /dev/null
> > +++ b/xen/scripts/Makefile.asm-generic
> > @@ -0,0 +1,23 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +# include/asm-generic contains a lot of files that are used
> > +# verbatim by several architectures.
> > +#
> > +# This Makefile reads the file arch/$(SRCARCH)/include/asm/Kbuild
> > +# and for each file listed in this file with generic-y creates
> > +# a small wrapper file in $(obj)
> > (arch/$(SRCARCH)/include/generated/asm)
> > +
> > +kbuild-file := $(srctree)/arch/$(SRCARCH)/include/asm/Kbuild
> > +include $(kbuild-file)
> > +
> > +include scripts/Kbuild.include
> > +
> > +# Create output directory if not already present
> > +_dummy := $(shell [ -d $(obj) ] || mkdir -p $(obj))
> > +
> > +quiet_cmd_wrap = WRAP    $@
> > +cmd_wrap = echo "\#include " >$@
> > +
> > +all: $(patsubst %, $(obj)/%, $(generic-y))
> > +
> > +$(obj)/%.h:
> > + $(call cmd,wrap)
> 
> In fact i think your files are not generated in the srctree so this
> will break if you use out of tree compilation.
I checked that and in both cases it generated in correct place.

First case:
$ CONTAINER_NO_PULL=1 CONTAINER=riscv64
./automation/scripts/containerize make XEN_TARGET_ARCH=riscv64 -C xen
build

$ ls -la xen/arch/riscv/include/generated/asm/vm_event.h 
-rw-r--r--. 1 ok ok 34 вер  6 14:32
xen/arch/riscv/include/generated/asm/vm_event.h

Second case: ( out-of-tree )
$ CONTAINER_NO_PULL=1 CONTAINER=riscv64
./automation/scripts/containerize make O=xen_build
XEN_TARGET_ARCH=riscv64 -C xen build V=1

$ ls -la xen/xen_build/arch/riscv/include/generated/asm/vm_event.h 
-rw-r--r--. 1 ok ok 34 вер  6 14:34
xen/xen_build/arch/riscv/include/generated/asm/vm_event.h

Could you please clarify if there's somithing I might have overlooked?

~ Oleksii

[XEN PATCH v2 1/2] xen: apply deviation for Rule 8.4 (asm-only definitions)

[Nicola Vetrini]

As stated in 'docs/misra/rules.rst' the functions that are used only by
asm modules do not need to conform to MISRA C:2012 Rule 8.4.
The deviations are carried out with a SAF comment.

Signed-off-by: Nicola Vetrini 
---
Changes in v2:
- Removed deviations on variables
---
 docs/misra/safe.json| 8 
 xen/arch/arm/cpuerrata.c| 1 +
 xen/arch/arm/setup.c| 1 +
 xen/arch/arm/smpboot.c  | 1 +
 xen/arch/arm/traps.c| 7 +++
 xen/arch/x86/boot/cmdline.c | 1 +
 xen/arch/x86/boot/reloc.c   | 1 +
 xen/arch/x86/extable.c  | 4 ++--
 xen/arch/x86/setup.c| 1 +
 xen/arch/x86/traps.c| 9 +
 xen/common/efi/boot.c   | 5 +++--
 11 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index e3c8a1d8eb36..39c5c056c7d4 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -12,6 +12,14 @@
 },
 {
 "id": "SAF-1-safe",
+"analyser": {
+"eclair": "MC3R1.R8.4"
+},
+"name": "Rule 8.4: asm-only definition",
+"text": "Functions and variables used only by asm modules do not 
need to have a visible declaration prior to their definition."
+},
+{
+"id": "SAF-2-safe",
 "analyser": {},
 "name": "Sentinel",
 "text": "Next ID to be used"
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index fcf32892a7ef..9137958fb682 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -370,6 +370,7 @@ custom_param("spec-ctrl", parse_spec_ctrl);
 
 /* Arm64 only for now as for Arm32 the workaround is currently handled in C. */
 #ifdef CONFIG_ARM_64
+/* SAF-1-safe */
 void __init arm_enable_wa2_handling(const struct alt_instr *alt,
 const uint32_t *origptr,
 uint32_t *updptr, int nr_inst)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 44ccea03ca14..db748839d383 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -1077,6 +1077,7 @@ static bool __init is_dom0less_mode(void)
 size_t __read_mostly dcache_line_bytes;
 
 /* C entry point for boot CPU */
+/* SAF-1-safe */
 void __init start_xen(unsigned long boot_phys_offset,
   unsigned long fdt_paddr)
 {
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index e107b86b7b44..6efd17eb3500 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -302,6 +302,7 @@ smp_prepare_cpus(void)
 }
 
 /* Boot the current CPU */
+/* SAF-1-safe */
 void start_secondary(void)
 {
 unsigned int cpuid = init_data.cpuid;
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 46c9a4031b30..ce89f16404d9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -161,6 +161,7 @@ void init_traps(void)
 isb();
 }
 
+/* SAF-1-safe */
 void __div0(void)
 {
 printk("Division by zero in hypervisor.\n");
@@ -1954,6 +1955,7 @@ static inline bool needs_ssbd_flip(struct vcpu *v)
  * Actions that needs to be done after entering the hypervisor from the
  * guest and before the interrupts are unmasked.
  */
+/* SAF-1-safe */
 void enter_hypervisor_from_guest_preirq(void)
 {
 struct vcpu *v = current;
@@ -1968,6 +1970,7 @@ void enter_hypervisor_from_guest_preirq(void)
  * guest and before we handle any request. Depending on the exception trap,
  * this may be called with interrupts unmasked.
  */
+/* SAF-1-safe */
 void enter_hypervisor_from_guest(void)
 {
 struct vcpu *v = current;
@@ -1996,6 +1999,7 @@ void enter_hypervisor_from_guest(void)
 vgic_sync_from_lrs(v);
 }
 
+/* SAF-1-safe */
 void do_trap_guest_sync(struct cpu_user_regs *regs)
 {
 const union hsr hsr = { .bits = regs->hsr };
@@ -2191,11 +2195,13 @@ void do_trap_guest_serror(struct cpu_user_regs *regs)
 __do_trap_serror(regs, true);
 }
 
+/* SAF-1-safe */
 void do_trap_irq(struct cpu_user_regs *regs)
 {
 gic_interrupt(regs, 0);
 }
 
+/* SAF-1-safe */
 void do_trap_fiq(struct cpu_user_regs *regs)
 {
 gic_interrupt(regs, 1);
@@ -2269,6 +2275,7 @@ static bool check_for_vcpu_work(void)
  *
  * The function will return with IRQ masked.
  */
+/* SAF-1-safe */
 void leave_hypervisor_to_guest(void)
 {
 local_irq_disable();
diff --git a/xen/arch/x86/boot/cmdline.c b/xen/arch/x86/boot/cmdline.c
index 74997703b31e..f9eee756aaed 100644
--- a/xen/arch/x86/boot/cmdline.c
+++ b/xen/arch/x86/boot/cmdline.c
@@ -340,6 +340,7 @@ static void vga_parse(const char *cmdline, 
early_boot_opts_t *ebo)
 }
 #endif
 
+/* SAF-1-safe */
 void __stdcall cmdline_parse_early(const char *cmdline, early_boot_opts_t *ebo)
 {
 if ( !cmdline )
diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
index e22bb974bf20..609b02cb73dc 100644
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -347,6 +347,7 @@ static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, 
uint32_t video_out)
 retur

[XEN PATCH v2 0/2] xen: deviate asm-only function definitions for Rule 8.4

[Nicola Vetrini]

As stated in 'docs/misra/rules.rst' the community decided to deviate from Rule 
8.4
whereby the definitions concern entities that are only used in asm modules.
The chosen deviation mechanism is that of textual deviations, through the use of
/* SAF-n-safe */ comments.

The deviation of variables is postponed to a future patch.

Nicola Vetrini (2):
  xen: apply deviation for Rule 8.4 (asm-only definitions)
  automation: execute SAF translation before the analysis with ECLAIR

 automation/eclair_analysis/prepare.sh | 3 +++
 docs/misra/safe.json  | 8 
 xen/arch/arm/cpuerrata.c  | 1 +
 xen/arch/arm/setup.c  | 1 +
 xen/arch/arm/smpboot.c| 1 +
 xen/arch/arm/traps.c  | 7 +++
 xen/arch/x86/boot/cmdline.c   | 1 +
 xen/arch/x86/boot/reloc.c | 1 +
 xen/arch/x86/extable.c| 4 ++--
 xen/arch/x86/setup.c  | 1 +
 xen/arch/x86/traps.c  | 9 +
 xen/common/efi/boot.c | 5 +++--
 12 files changed, 38 insertions(+), 4 deletions(-)

--
2.34.1

[XEN PATCH v2 2/2] automation: execute SAF translation before the analysis with ECLAIR

[Nicola Vetrini]

This allows local MISRA deviation comments to be translated into
the format recognized by ECLAIR.

Signed-off-by: Nicola Vetrini 
---
 automation/eclair_analysis/prepare.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/automation/eclair_analysis/prepare.sh 
b/automation/eclair_analysis/prepare.sh
index 275a1a3f517c..0cac5eba00ae 100755
--- a/automation/eclair_analysis/prepare.sh
+++ b/automation/eclair_analysis/prepare.sh
@@ -38,5 +38,8 @@ fi
 cd xen
 cp "${CONFIG_FILE}" .config
 make clean
+find . -type f -name "*.safparse" -print -delete
 make -f ${script_dir}/Makefile.prepare prepare
+# Translate the /* SAF-n-safe */ comments into ECLAIR CBTs
+scripts/xen-analysis.py --run-eclair --no-build --no-clean
 )
-- 
2.34.1

[ovmf test] 182654: all pass - PUSHED

[osstest service owner]

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

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

Last test of basis   182640  2023-09-05 22:41:46 Z0 days
Testing same since   182654  2023-09-06 08:12:34 Z0 days1 attempts


People who touched revisions under test:
  Yin Wang 

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
   f008890ae5..d1e4a16f86  d1e4a16f868fb1ccfa8bed8eaba5f9617ec8382d -> 
xen-tested-master

Re: [PATCH v4 1/2] xen: asm-generic support

[Oleksii]

On Tue, 2023-09-05 at 14:10 +, Bertrand Marquis wrote:
> Hi Oleksii,
> 
> > On 5 Sep 2023, at 15:37, Oleksii 
> > wrote:
> > 
> > Hi Bertrand,
> > 
> > On Mon, 2023-09-04 at 13:39 +, Bertrand Marquis wrote:
> > > Hi Oleksii,
> > > 
> > > > On 1 Sep 2023, at 18:02, Oleksii Kurochko
> > > >  wrote:
> > > > 
> > > > Some headers are shared between individual architectures or are
> > > > empty.
> > > > To avoid duplication of these headers, asm-generic is
> > > > introduced.
> > > > 
> > > > With the following patch, an architecture uses generic headers
> > > > mentioned in the file arch/$(ARCH)/include/asm/Kbuild.
> > > 
> > > Kbuild refers to "Kernel build" I guess.
> > > I am ok to keep that name to keep things simpler when compared to
> > > Linux scripts but it would be good to mention that in the commit
> > > message for future reference.
> > Sure. I'll add that Kbuild refers to "Kernel build"
> > > 
> > > > 
> > > > To use a generic header is needed to add to
> > > > arch/$(ARCH)/include/asm/Kbuild :
> > > >    generic-y += 
> > > > 
> > > > For each mentioned header in arch/$(ARCH)/include/asm/Kbuild,
> > > > kbuild will generate the necessary wrapper in
> > > > arch/$(ARCH)/include/generated/asm.
> > > > 
> > > > Signed-off-by: Oleksii Kurochko 
> > > > ---
> > > > Changes in V4:
> > > > - introduce asm-generic support
> > > > - update commit message
> > > > ---
> > > > Changes in V3:
> > > > - Rename stubs dir to asm-generic
> > > > ---
> > > > Changes in V2:
> > > > - Nothing changed.
> > > > ---
> > > > xen/Makefile  | 10 +-
> > > > xen/arch/arm/include/asm/Kbuild   |  1 +
> > > > xen/arch/ppc/include/asm/Kbuild   |  1 +
> > > > xen/arch/riscv/include/asm/Kbuild |  1 +
> > > > xen/arch/x86/include/asm/Kbuild   |  1 +
> > > > xen/scripts/Makefile.asm-generic  | 23 +++
> > > > 6 files changed, 36 insertions(+), 1 deletion(-)
> > > > create mode 100644 xen/arch/arm/include/asm/Kbuild
> > > > create mode 100644 xen/arch/ppc/include/asm/Kbuild
> > > > create mode 100644 xen/arch/riscv/include/asm/Kbuild
> > > > create mode 100644 xen/arch/x86/include/asm/Kbuild
> > > > create mode 100644 xen/scripts/Makefile.asm-generic
> > > > 
> > > > diff --git a/xen/Makefile b/xen/Makefile
> > > > index f57e5a596c..698d6ddeb8 100644
> > > > --- a/xen/Makefile
> > > > +++ b/xen/Makefile
> > > > @@ -438,6 +438,7 @@ ifdef building_out_of_srctree
> > > > endif
> > > > CFLAGS += -I$(srctree)/include
> > > > CFLAGS += -I$(srctree)/arch/$(SRCARCH)/include
> > > > +CFLAGS += -I$(srctree)/arch/$(SRCARCH)/include/generated
> > > 
> > > Why are we generating files in the source tree ? 
> > > Shouldn't we keep it unmodified ?
> > I think you are right.
> > I got myself confused because I do not use out-of-tree compilation
> > so
> > for me it looked like $(srctree).
> > 
> > It would be better to use $(objtree) instead.
> 
> Ack
> 
> > > 
> > > > 
> > > > # Note that link order matters!
> > > > ALL_OBJS-y    := common/built_in.o
> > > > @@ -580,6 +581,7 @@ _clean:
> > > > rm -f $(TARGET).efi $(TARGET).efi.map $(TARGET).efi.elf
> > > > $(TARGET).efi.stripped
> > > > rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h
> > > > rm -f .banner .allconfig.tmp include/xen/compile.h
> > > > + rm -rf arch/*/include/generated
> > > > 
> > > 
> > > You must use the same as for CFLAGS here so $(srctree) would be
> > > needed.
> > > (or something else depending on the fix for previous comment)
> > Thanks. I'll change to the value mentioned in CFLAGS.
> > 
> > > 
> > > > .PHONY: _distclean
> > > > _distclean: clean
> > > > @@ -589,7 +591,7 @@ $(TARGET).gz: $(TARGET)
> > > > gzip -n -f -9 < $< > $@.new
> > > > mv $@.new $@
> > > > 
> > > > -$(TARGET): outputmakefile FORCE
> > > > +$(TARGET): outputmakefile asm-generic FORCE
> > > > $(Q)$(MAKE) $(build)=tools
> > > > $(Q)$(MAKE) $(build)=. include/xen/compile.h
> > > > $(Q)$(MAKE) $(build)=include all
> > > > @@ -667,6 +669,12 @@ endif # need-sub-make
> > > > PHONY += FORCE
> > > > FORCE:
> > > > 
> > > > +# Support for using generic headers in asm-generic
> > > > +PHONY += asm-generic
> > > > +asm-generic:
> > > > + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.asm-generic \
> > > > +    obj=arch/$(SRCARCH)/include/generated/asm
> > > > +
> > > > # Declare the contents of the PHONY variable as phony.  We keep
> > > > that
> > > > # information in a variable so we can use it in if_changed and
> > > > friends.
> > > > .PHONY: $(PHONY)
> > > > diff --git a/xen/arch/arm/include/asm/Kbuild
> > > > b/xen/arch/arm/include/asm/Kbuild
> > > > new file mode 100644
> > > > index 00..a4e40e534e
> > > > --- /dev/null
> > > > +++ b/xen/arch/arm/include/asm/Kbuild
> > > > @@ -0,0 +1 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > diff --git a/xen/arch/ppc/include/asm/Kbuild
> > > > b/xen/arch/ppc/include/asm/Kbuild
> > > > new file mode 100644
> > > > index 00..a4e40e534e
> > > > --- /dev/null
> > > > +++

[PATCH] xen/arm: Fix printk specifiers and arguments in iomem_remove_cb()

[Michal Orzel]

When building Xen for arm32 with CONFIG_DTB_OVERLAY, the following
error is printed:

common/dt-overlay.c: In function ‘iomem_remove_cb’:
././include/xen/config.h:55:24: error: format ‘%llx’ expects argument of type 
‘long long unsigned int’, but argument 3 has type ‘long unsigned int’ 
[-Werror=format=]

Function parameters s and e (denoting MMIO region) are of type unsigned
long and indicate frame numbers and not addresses. This also means that
the arguments passed to printk() are incorrect (using PAGE_ALIGN() or
PAGE_MASK ANDed with a frame number results in unwanted output). Fix it.

Take the opportunity to switch to %pd specifier to print domain id in
a consolidated way.

Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal 
functionalities")
Signed-off-by: Michal Orzel 
---
After this patch (and the one for xl), we are left with one issue breaking CI:
https://gitlab.com/xen-project/patchew/xen/-/jobs/5026938514
---
 xen/common/dt-overlay.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
index b07a6128dc99..5663a049e90a 100644
--- a/xen/common/dt-overlay.c
+++ b/xen/common/dt-overlay.c
@@ -251,10 +251,8 @@ static int iomem_remove_cb(unsigned long s, unsigned long 
e, void *dom,
 rc = iomem_deny_access(d, s, e);
 if ( rc )
 {
-printk(XENLOG_ERR "Unable to remove dom%d access to"
-   " 0x%"PRIx64" - 0x%"PRIx64"\n",
-   d->domain_id,
-   s & PAGE_MASK, PAGE_ALIGN(e) - 1);
+printk(XENLOG_ERR "Unable to remove %pd access to %#lx - %#lx\n",
+   d, s, e);
 }
 else
 *c += e - s + 1;
-- 
2.25.1

[xen-4.15-testing test] 182633: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-migrupgrade  10 xen-install/src_host fail REGR. vs. 182235
 test-amd64-i386-migrupgrade  11 xen-install/dst_host fail REGR. vs. 182235
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 182235

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

version targeted for testing:
 xen  0517763e771f2e2582bc492fafa42a86400ab957
baseline version:
 xen  da7f151a6310

Re: [PATCH 3/3] sysemu/xen: Allow elision of xen_hvm_modified_memory()

[David Hildenbrand]

On 05.09.23 14:21, Philippe Mathieu-Daudé wrote:

Call xen_enabled() before xen_hvm_modified_memory() to let
the compiler elide its call.

Have xen_enabled() return a boolean to match its declaration
in the CONFIG_XEN_IS_POSSIBLE case.

Suggested-by: Daniel Henrique Barboza 
Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: David Hildenbrand 

--
Cheers,

David / dhildenb

Re: [PATCH] OvmfPkg/OvmfXen: Fix S3

[Ard Biesheuvel]

On Thu, 13 Jul 2023 at 12:48, Xenia Ragiadakou  wrote:
>
> Currently, resuming an S3 suspended guest results in the following
> assertion failure:
> ASSERT 
> MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.c(41): 
> MemoryLength > 0
> This happens because some parts of the S3 suspend and resume paths
> are missing in order for S3 to work. For instance, the variables
> mS3AcpiReservedMemoryBase and mS3AcpiReservedMemoryBase are not
> initialized, regions that are used on S3 resume are either missing
> or not marked as ACPI NVS memory and can be corrupted by the OS.
> This patch adds the missing parts based heavily on the existing S3
> implementation of other virtual platforms.
>
> For S3 support, the provision of fw_cfg is required in order for
> suspend states to be retrieved.
>
> Another issue noticed is that when CalibrateLapicTimer() is called
> on S3 resume path, the shared info page is remapped to a different
> guest physical address. This remapping happens under guest's feet,
> so any subsequent attempt of the guest to access the shared info
> page results in nested page faults. This patch removes any local
> APIC timer initializion and calibration from S3 resume path.
>
> Signed-off-by: Xenia Ragiadakou 

Anyone care to review this?

> ---
>  OvmfPkg/XenPlatformPei/Fv.c   |  2 +-
>  OvmfPkg/XenPlatformPei/MemDetect.c| 60 ++-
>  OvmfPkg/XenPlatformPei/Platform.c | 11 -
>  OvmfPkg/XenPlatformPei/Platform.h |  2 +
>  OvmfPkg/XenPlatformPei/XenPlatformPei.inf |  7 +++
>  5 files changed, 78 insertions(+), 4 deletions(-)
>
> diff --git a/OvmfPkg/XenPlatformPei/Fv.c b/OvmfPkg/XenPlatformPei/Fv.c
> index 871a2c1c5b..37ecb3cfee 100644
> --- a/OvmfPkg/XenPlatformPei/Fv.c
> +++ b/OvmfPkg/XenPlatformPei/Fv.c
> @@ -37,7 +37,7 @@ PeiFvInitialization (
>BuildMemoryAllocationHob (
>  PcdGet32 (PcdOvmfPeiMemFvBase),
>  PcdGet32 (PcdOvmfPeiMemFvSize),
> -EfiBootServicesData
> +mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>  );
>
>//
> diff --git a/OvmfPkg/XenPlatformPei/MemDetect.c 
> b/OvmfPkg/XenPlatformPei/MemDetect.c
> index e552e7a55e..1724a4988f 100644
> --- a/OvmfPkg/XenPlatformPei/MemDetect.c
> +++ b/OvmfPkg/XenPlatformPei/MemDetect.c
> @@ -283,6 +283,19 @@ PublishPeiMemory (
>
>LowerMemorySize = GetSystemMemorySizeBelow4gb ();
>
> +  //
> +  // If S3 is supported, then the S3 permanent PEI memory is placed next,
> +  // downwards. Its size is primarily dictated by CpuMpPei. The formula below
> +  // is an approximation.
> +  //
> +  if (mS3Supported) {
> +mS3AcpiReservedMemorySize = SIZE_512KB +
> +PcdGet32 (PcdCpuMaxLogicalProcessorNumber) *
> +PcdGet32 (PcdCpuApStackSize);
> +mS3AcpiReservedMemoryBase = LowerMemorySize - mS3AcpiReservedMemorySize;
> +LowerMemorySize   = mS3AcpiReservedMemoryBase;
> +  }
> +
>if (mBootMode == BOOT_ON_S3_RESUME) {
>  MemoryBase = mS3AcpiReservedMemoryBase;
>  MemorySize = mS3AcpiReservedMemorySize;
> @@ -328,6 +341,51 @@ InitializeRamRegions (
>  {
>XenPublishRamRegions ();
>
> +  if (mS3Supported && (mBootMode != BOOT_ON_S3_RESUME)) {
> +//
> +// This is the memory range that will be used for PEI on S3 resume
> +//
> +BuildMemoryAllocationHob (
> +  mS3AcpiReservedMemoryBase,
> +  mS3AcpiReservedMemorySize,
> +  EfiACPIMemoryNVS
> +  );
> +
> +//
> +// Cover the initial RAM area used as stack and temporary PEI heap.
> +//
> +// This is reserved as ACPI NVS so it can be used on S3 resume.
> +//
> +BuildMemoryAllocationHob (
> +  PcdGet32 (PcdOvmfSecPeiTempRamBase),
> +  PcdGet32 (PcdOvmfSecPeiTempRamSize),
> +  EfiACPIMemoryNVS
> +  );
> +
> +//
> +// SEC stores its table of GUIDed section handlers here.
> +//
> +BuildMemoryAllocationHob (
> +  PcdGet64 (PcdGuidedExtractHandlerTableAddress),
> +  PcdGet32 (PcdGuidedExtractHandlerTableSize),
> +  EfiACPIMemoryNVS
> +  );
> +
> + #ifdef MDE_CPU_X64
> +//
> +// Reserve the initial page tables built by the reset vector code.
> +//
> +// Since this memory range will be used by the Reset Vector on S3
> +// resume, it must be reserved as ACPI NVS.
> +//
> +BuildMemoryAllocationHob (
> +  (EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfSecPageTablesBase),
> +  (UINT64)(UINTN)PcdGet32 (PcdOvmfSecPageTablesSize),
> +  EfiACPIMemoryNVS
> +  );
> + #endif
> +  }
> +
>if (mBootMode != BOOT_ON_S3_RESUME) {
>  //
>  // Reserve the lock box storage area
> @@ -346,7 +404,7 @@ InitializeRamRegions (
>  BuildMemoryAllocationHob (
>(EFI_PHYSICAL_ADDRESS)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageBase),
>(UINT64)(UINTN)PcdGet32 (PcdOvmfLockBoxStorageSize),
> -  EfiBootServicesData
> +  mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>);

[xen-unstable-smoke test] 182646: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 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  61765a07e3d8a4dabbb8323b55f13ee75c80e54a
baseline version:
 xen  9a216e92de9f9011097e4f1fb55ff67ba0a21704

Last test of basis   182635  2023-09-05 13:00:27 Z0 days
Testing same since   182641  2023-09-06 02:02:08 Z0 days2 attempts


People who touched revisions under test:
  David Gibson 
  Henry Wang 
  Julien Grall 
  Juline Grall 
  Stefano Stabellini 
  Vikram Garhwal 
  Vikram Garhwal 

jobs:
 build-arm64-xsm  pass
 build-amd64  fail
 build-armhf  pass
 build-amd64-libvirt  blocked 
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked 
 test-amd64-amd64-libvirt blocked 



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

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

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

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


Not pushing.

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

[XEN PATCH 2/4] x86/io: address violations of MISRA C:2012 Rule 8.3

[Federico Serafini]

Make declarations consistent, no functional change.

Signed-off-by: Federico Serafini 
---
 xen/arch/x86/include/asm/io.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/io.h b/xen/arch/x86/include/asm/io.h
index 92b784a861..8c8a15bd06 100644
--- a/xen/arch/x86/include/asm/io.h
+++ b/xen/arch/x86/include/asm/io.h
@@ -51,6 +51,6 @@ __OUT(l,,int)
 #define IOEMUL_QUIRK_STUB_BYTES 9
 struct cpu_user_regs;
 extern unsigned int (*ioemul_handle_quirk)(
-u8 opcode, char *io_emul_stub, struct cpu_user_regs *regs);
+uint8_t opcode, char *io_emul_stub, struct cpu_user_regs *regs);
 
 #endif
-- 
2.34.1

[XEN PATCH 0/4] xen/x86: address some violations of MISRA C:2012 Rules 8.2 and 8.3

[Federico Serafini]

This serie addresses some violations of MISRA C:2012 Rules 8.2 and 8.3
for x86-64.

Federico Serafini (4):
  x86/genapic: address a violation of MISRA C:2012 Rule 8.3
  x86/io: address violations of MISRA C:2012 Rule 8.3
  x86/io_apic: address violations of MISRA C:2012 Rules 8.2 and 8.3
  xen/vpci: address a violation of MISRA C:2012 Rule 8.3

 xen/arch/x86/include/asm/genapic.h | 2 +-
 xen/arch/x86/include/asm/hap.h | 2 +-
 xen/arch/x86/include/asm/io.h  | 2 +-
 xen/arch/x86/include/asm/io_apic.h | 4 ++--
 xen/include/xen/vpci.h | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

-- 
2.34.1

[XEN PATCH 3/4] x86/io_apic: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Federico Serafini]

Add missing parameter names and make function declarations and
definitions consistent.

No functional change.

Signed-off-by: Federico Serafini 
---
 xen/arch/x86/include/asm/io_apic.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/include/asm/io_apic.h 
b/xen/arch/x86/include/asm/io_apic.h
index 9165da2281..a7e4c9e146 100644
--- a/xen/arch/x86/include/asm/io_apic.h
+++ b/xen/arch/x86/include/asm/io_apic.h
@@ -199,7 +199,7 @@ extern struct IO_APIC_route_entry __ioapic_read_entry(
 unsigned int apic, unsigned int pin, bool raw);
 void __ioapic_write_entry(
 unsigned int apic, unsigned int pin, bool raw,
-struct IO_APIC_route_entry);
+struct IO_APIC_route_entry e);
 
 extern struct IO_APIC_route_entry **alloc_ioapic_entries(void);
 extern void free_ioapic_entries(struct IO_APIC_route_entry **ioapic_entries);
@@ -211,6 +211,6 @@ extern int restore_IO_APIC_setup(struct IO_APIC_route_entry 
**ioapic_entries,
 unsigned highest_gsi(void);
 
 int ioapic_guest_read( unsigned long physbase, unsigned int reg, u32 *pval);
-int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 pval);
+int ioapic_guest_write(unsigned long physbase, unsigned int reg, u32 val);
 
 #endif
-- 
2.34.1

[XEN PATCH 4/4] xen/vpci: address a violation of MISRA C:2012 Rule 8.3

[Federico Serafini]

Make function declarations and definitions consistent.
No functional change.

Signed-off-by: Federico Serafini 
---
 xen/arch/x86/include/asm/hap.h | 2 +-
 xen/include/xen/vpci.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/include/asm/hap.h b/xen/arch/x86/include/asm/hap.h
index 9d12327b12..05e124ad57 100644
--- a/xen/arch/x86/include/asm/hap.h
+++ b/xen/arch/x86/include/asm/hap.h
@@ -30,7 +30,7 @@ void  hap_vcpu_init(struct vcpu *v);
 int   hap_track_dirty_vram(struct domain *d,
unsigned long begin_pfn,
unsigned int nr_frames,
-   XEN_GUEST_HANDLE(void) dirty_bitmap);
+   XEN_GUEST_HANDLE(void) guest_dirty_bitmap);
 
 extern const struct paging_mode *hap_paging_get_mode(struct vcpu *);
 int hap_set_allocation(struct domain *d, unsigned int pages, bool *preempted);
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index 0b8a2a3c74..d743d96a10 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -26,7 +26,7 @@ typedef int vpci_register_init_t(struct pci_dev *dev);
__used_section(".data.vpci." p) = x
 
 /* Add vPCI handlers to device. */
-int __must_check vpci_add_handlers(struct pci_dev *dev);
+int __must_check vpci_add_handlers(struct pci_dev *pdev);
 
 /* Remove all handlers and free vpci related structures. */
 void vpci_remove_device(struct pci_dev *pdev);
-- 
2.34.1

[XEN PATCH 1/4] x86/genapic: address a violation of MISRA C:2012 Rule 8.3

[Federico Serafini]

Make function delcaration consistent with the corresponding definition.
No functional change.

Signed-off-by: Federico Serafini 
---
 xen/arch/x86/include/asm/genapic.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/genapic.h 
b/xen/arch/x86/include/asm/genapic.h
index beeaddf19d..970df8ffe0 100644
--- a/xen/arch/x86/include/asm/genapic.h
+++ b/xen/arch/x86/include/asm/genapic.h
@@ -43,7 +43,7 @@ void cf_check send_IPI_self_legacy(uint8_t vector);
 
 void cf_check init_apic_ldr_flat(void);
 unsigned int cf_check cpu_mask_to_apicid_flat(const cpumask_t *cpumask);
-void cf_check send_IPI_mask_flat(const cpumask_t *mask, int vector);
+void cf_check send_IPI_mask_flat(const cpumask_t *cpumask, int vector);
 const cpumask_t *cf_check vector_allocation_cpumask_flat(int cpu);
 #define GENAPIC_FLAT \
.int_delivery_mode = dest_LowestPrio, \
-- 
2.34.1

Re: [PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY

[Michal Orzel]

On 06/09/2023 10:42, Jan Beulich wrote:
> 
> 
> On 06.09.2023 10:36, Michal Orzel wrote:
>> main_dt_overlay() makes a call to libxl_dt_overlay() which is for now
>> only compiled for Arm. This causes the build failure as reported by
>> gitlab CI and OSSTEST. Fix it by guarding the function, prototype and
>> entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage
>> over regular Arm guard so that the code will not need to be modified again
>> if other architecture gain support for this feature.
>>
>> Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree 
>> overlay support")
>> Reported-by: Jan Beulich 
>> Signed-off-by: Michal Orzel 
>> ---
>> There are still other parts of dt overlay support in toolstack that would
>> want to be revisited in order to use guards suitable to be used by other
>> arches.
> 
> Since from all I can tell this will do
> Reviewed-by: Jan Beulich 
> 
> But I still wonder: We agreed to leave libxc alone for now, but was it
> really intentional that you didn't adjust libxl.h right here, but instead
> ...
> 
>> --- a/tools/xl/xl.h
>> +++ b/tools/xl/xl.h
>> @@ -138,7 +138,9 @@ int main_shutdown(int argc, char **argv);
>>  int main_reboot(int argc, char **argv);
>>  int main_list(int argc, char **argv);
>>  int main_vm_list(int argc, char **argv);
>> +#ifdef LIBXL_HAVE_DT_OVERLAY
>>  int main_dt_overlay(int argc, char **argv);
>> +#endif
>>  int main_create(int argc, char **argv);
>>  int main_config_update(int argc, char **argv);
>>  int main_button_press(int argc, char **argv);
> 
> ... made this adjustment, which imo isn't strictly necessary.
I'm not a toolstack expert but I decided to guard the prototype as well
given that quite a few of them are guarded either using LIBXL_HAVE or arch 
guard.

~Michal

Re: [PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY

[Jan Beulich]

On 06.09.2023 10:36, Michal Orzel wrote:
> main_dt_overlay() makes a call to libxl_dt_overlay() which is for now
> only compiled for Arm. This causes the build failure as reported by
> gitlab CI and OSSTEST. Fix it by guarding the function, prototype and
> entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage
> over regular Arm guard so that the code will not need to be modified again
> if other architecture gain support for this feature.
> 
> Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree 
> overlay support")
> Reported-by: Jan Beulich 
> Signed-off-by: Michal Orzel 
> ---
> There are still other parts of dt overlay support in toolstack that would
> want to be revisited in order to use guards suitable to be used by other
> arches.

Since from all I can tell this will do
Reviewed-by: Jan Beulich 

But I still wonder: We agreed to leave libxc alone for now, but was it
really intentional that you didn't adjust libxl.h right here, but instead
...

> --- a/tools/xl/xl.h
> +++ b/tools/xl/xl.h
> @@ -138,7 +138,9 @@ int main_shutdown(int argc, char **argv);
>  int main_reboot(int argc, char **argv);
>  int main_list(int argc, char **argv);
>  int main_vm_list(int argc, char **argv);
> +#ifdef LIBXL_HAVE_DT_OVERLAY
>  int main_dt_overlay(int argc, char **argv);
> +#endif
>  int main_create(int argc, char **argv);
>  int main_config_update(int argc, char **argv);
>  int main_button_press(int argc, char **argv);

... made this adjustment, which imo isn't strictly necessary.

Jan

[PATCH] tools/xl: Guard main_dt_overlay() with LIBXL_HAVE_DT_OVERLAY

[Michal Orzel]

main_dt_overlay() makes a call to libxl_dt_overlay() which is for now
only compiled for Arm. This causes the build failure as reported by
gitlab CI and OSSTEST. Fix it by guarding the function, prototype and
entry in cmd_table[] using LIBXL_HAVE_DT_OVERLAY. This has an advantage
over regular Arm guard so that the code will not need to be modified again
if other architecture gain support for this feature.

Fixes: 61765a07e3d8 ("tools/xl: Add new xl command overlay for device tree 
overlay support")
Reported-by: Jan Beulich 
Signed-off-by: Michal Orzel 
---
There are still other parts of dt overlay support in toolstack that would
want to be revisited in order to use guards suitable to be used by other
arches.
---
 tools/xl/xl.h   | 2 ++
 tools/xl/xl_cmdtable.c  | 2 ++
 tools/xl/xl_vmcontrol.c | 3 +++
 3 files changed, 7 insertions(+)

diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index a923daccd340..3045b5a8e3f0 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -138,7 +138,9 @@ int main_shutdown(int argc, char **argv);
 int main_reboot(int argc, char **argv);
 int main_list(int argc, char **argv);
 int main_vm_list(int argc, char **argv);
+#ifdef LIBXL_HAVE_DT_OVERLAY
 int main_dt_overlay(int argc, char **argv);
+#endif
 int main_create(int argc, char **argv);
 int main_config_update(int argc, char **argv);
 int main_button_press(int argc, char **argv);
diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
index 2463521156a1..62bdb2aeaa35 100644
--- a/tools/xl/xl_cmdtable.c
+++ b/tools/xl/xl_cmdtable.c
@@ -631,12 +631,14 @@ const struct cmd_spec cmd_table[] = {
   "Issue a qemu monitor command to the device model of a domain",
   " ",
 },
+#ifdef LIBXL_HAVE_DT_OVERLAY
 { "dt-overlay",
   &main_dt_overlay, 0, 1,
   "Add/Remove a device tree overlay",
   "add/remove <.dtbo>"
   "-h print this help\n"
 },
+#endif
 };
 
 const int cmdtable_len = ARRAY_SIZE(cmd_table);
diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index cea5b4a88e81..98f6bd2e7607 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -1265,6 +1265,7 @@ int main_create(int argc, char **argv)
 return 0;
 }
 
+#ifdef LIBXL_HAVE_DT_OVERLAY
 int main_dt_overlay(int argc, char **argv)
 {
 const char *overlay_ops = NULL;
@@ -1317,6 +1318,8 @@ int main_dt_overlay(int argc, char **argv)
 
 return rc;
 }
+#endif
+
 /*
  * Local variables:
  * mode: C
-- 
2.25.1

Re: [XEN][PATCH v12 20/20] tools/xl: Add new xl command overlay for device tree overlay support

[Jan Beulich]

On 06.09.2023 10:06, Michal Orzel wrote:
> 
> 
> On 06/09/2023 09:57, Jan Beulich wrote:
>>
>>
>> On 06.09.2023 09:32, Michal Orzel wrote:
>>>
>>>
>>> On 06/09/2023 08:55, Jan Beulich wrote:


 On 06.09.2023 03:16, Vikram Garhwal wrote:
> --- a/tools/xl/xl_vmcontrol.c
> +++ b/tools/xl/xl_vmcontrol.c
> @@ -1265,6 +1265,58 @@ 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 EXIT_FAILURE;
> +}
> +
> +if (overlay_config_file) {
> +rc = libxl_read_file_contents(ctx, overlay_config_file,
> +  &overlay_dtb, &overlay_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);

 Because of this being Arm-only (as validly pointed out by osstest), I 
 expect
 the entire function here as well as its entry in cmd_table[] want to be
 Arm-specific, too? Of course it would be nice to not key this to __arm__ /
 __aarch64__, but to something that would not need touching again if the
 underlying infrastructure was made available to, say, RISC-V as well. But 
 of
 course - right now the goal needs to be to address the CI and osstest
 breakage.
>>> I agree. I would suggest to guard it with LIBXL_HAVE_DT_OVERLAY which is 
>>> for now
>>> only defined for arm32/arm64. This way the code will not need to be 
>>> modified if other
>>> arch gain support for the feature.
>>
>> Ah yes, that ought to work. While there perhaps also replace the conditional
>> around the declaration of the function in libxl.h. (But of course Anthony
>> may tell me/us that this isn't the way to go.)
> Hmm, if we change guards for libxl_dt_overlay(), what about xc_dt_overlay()
> for which we cannot use LIBXL guard?

I'd key that to some suitable sysctl definition from the public header. If
need be, the sub-op #define itself could be made conditional and then be
used for that purpose.

> Is it ok in that case or better just focus > on the fix.

Personally I'd consider dealing with just the breakage sufficient for the
moment. The libxc part then of course still wants dealing with later on,
preferably in time for 4.18.

Jan

Re: [PATCH v5 4/5] xen/vpci: header: status register handler

[Jan Beulich]

On 01.09.2023 06:57, Stewart Hildebrand wrote:
> Introduce a handler for the PCI status register, with ability to mask the
> capabilities bit. The status register contains reserved bits, read-only bits,
> and write-1-to-clear bits, so introduce bitmasks to handle these in vPCI. If a
> bit in the bitmask is set, then the special meaning applies:
> 
>   res_mask: read as zero, write ignore
>   ro_mask: read normal, write ignore
>   rw1c_mask: read normal, write 1 to clear

With the last one's name being descriptive of what the behavior is, would
it make sense to name the first one "raz_mask"? (Also a question to Roger
as the maintainer of this code.)

> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -413,6 +413,18 @@ static void cf_check cmd_write(
>  pci_conf_write16(pdev->sbdf, reg, cmd);
>  }
>  
> +static uint32_t cf_check status_read(const struct pci_dev *pdev,
> + unsigned int reg, void *data)
> +{
> +struct vpci_header *header = data;
> +uint32_t status = pci_conf_read16(pdev->sbdf, reg);
> +
> +if ( header->mask_cap_list )
> +status &= ~PCI_STATUS_CAP_LIST;
> +
> +return status;
> +}

Do you actually need this function? Can't you ...

> @@ -544,6 +556,12 @@ static int cf_check init_bars(struct pci_dev *pdev)
>  if ( rc )
>  return rc;
>  
> +rc = vpci_add_register_mask(pdev->vpci, status_read, vpci_hw_write16,
> +PCI_STATUS, 2, header, 
> PCI_STATUS_RESERVED_MASK,
> +PCI_STATUS_RO_MASK, PCI_STATUS_RW1C_MASK);

... conditionally OR in PCI_STATUS_CAP_LIST right here? Without
capabilities the CAP_LIST bit becomes kind of reserved anyway.

> @@ -424,9 +450,13 @@ static void vpci_write_helper(const struct pci_dev *pdev,
>  uint32_t val;
>  
>  val = r->read(pdev, r->offset, r->private);
> +val &= ~r->res_mask;
> +val &= ~r->rw1c_mask;

Personally I'd fold these two lines into just one (and similarly below).

>  data = merge_result(val, data, size, offset);
>  }
>  
> +data &= ~r->res_mask;
> +data &= ~r->ro_mask;

This seems risky to me. I'd rather see the same value being written back
for r/o bits, just in case a device doesn't actually implement a bit as
mandated. For reserved flags it's less clear what's best, because in
principle they could also become rw1c once defined.

> --- a/xen/include/xen/pci_regs.h
> +++ b/xen/include/xen/pci_regs.h
> @@ -66,6 +66,14 @@
>  #define  PCI_STATUS_REC_MASTER_ABORT 0x2000 /* Set on master abort */
>  #define  PCI_STATUS_SIG_SYSTEM_ERROR 0x4000 /* Set when we drive SERR */
>  #define  PCI_STATUS_DETECTED_PARITY  0x8000 /* Set on parity error */
> +#define  PCI_STATUS_RESERVED_MASK0x06

I'd recommend separating the "derived" constants by a blank line. I'd
further like to ask that you add two more padding zeros above.

> +#define  PCI_STATUS_RO_MASK (PCI_STATUS_IMM_READY | PCI_STATUS_INTERRUPT | \
> +PCI_STATUS_CAP_LIST | PCI_STATUS_CAP_LIST | PCI_STATUS_66MHZ | \

CAP_LIST twice?

Jan

Re: [XEN][PATCH v12 20/20] tools/xl: Add new xl command overlay for device tree overlay support

[Michal Orzel]

On 06/09/2023 09:57, Jan Beulich wrote:
> 
> 
> On 06.09.2023 09:32, Michal Orzel wrote:
>>
>>
>> On 06/09/2023 08:55, Jan Beulich wrote:
>>>
>>>
>>> On 06.09.2023 03:16, Vikram Garhwal wrote:
 --- a/tools/xl/xl_vmcontrol.c
 +++ b/tools/xl/xl_vmcontrol.c
 @@ -1265,6 +1265,58 @@ 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 EXIT_FAILURE;
 +}
 +
 +if (overlay_config_file) {
 +rc = libxl_read_file_contents(ctx, overlay_config_file,
 +  &overlay_dtb, &overlay_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);
>>>
>>> Because of this being Arm-only (as validly pointed out by osstest), I expect
>>> the entire function here as well as its entry in cmd_table[] want to be
>>> Arm-specific, too? Of course it would be nice to not key this to __arm__ /
>>> __aarch64__, but to something that would not need touching again if the
>>> underlying infrastructure was made available to, say, RISC-V as well. But of
>>> course - right now the goal needs to be to address the CI and osstest
>>> breakage.
>> I agree. I would suggest to guard it with LIBXL_HAVE_DT_OVERLAY which is for 
>> now
>> only defined for arm32/arm64. This way the code will not need to be modified 
>> if other
>> arch gain support for the feature.
> 
> Ah yes, that ought to work. While there perhaps also replace the conditional
> around the declaration of the function in libxl.h. (But of course Anthony
> may tell me/us that this isn't the way to go.)
Hmm, if we change guards for libxl_dt_overlay(), what about xc_dt_overlay()
for which we cannot use LIBXL guard? Is it ok in that case or better just focus
on the fix.

~Michal

Re: [PATCH v5 3/5] xen/pci: update PCI_STATUS_* constants

[Jan Beulich]

On 01.09.2023 06:57, Stewart Hildebrand wrote:
> Interrupt status introduced in PCI 2.3
> Immediate readiness introduced in PCIe 4.0
> 
> Signed-off-by: Stewart Hildebrand 

Reviewed-by: Jan Beulich 

Re: [XEN][PATCH v12 20/20] tools/xl: Add new xl command overlay for device tree overlay support

[Jan Beulich]

On 06.09.2023 09:32, Michal Orzel wrote:
> 
> 
> On 06/09/2023 08:55, Jan Beulich wrote:
>>
>>
>> On 06.09.2023 03:16, Vikram Garhwal wrote:
>>> --- a/tools/xl/xl_vmcontrol.c
>>> +++ b/tools/xl/xl_vmcontrol.c
>>> @@ -1265,6 +1265,58 @@ 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 EXIT_FAILURE;
>>> +}
>>> +
>>> +if (overlay_config_file) {
>>> +rc = libxl_read_file_contents(ctx, overlay_config_file,
>>> +  &overlay_dtb, &overlay_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);
>>
>> Because of this being Arm-only (as validly pointed out by osstest), I expect
>> the entire function here as well as its entry in cmd_table[] want to be
>> Arm-specific, too? Of course it would be nice to not key this to __arm__ /
>> __aarch64__, but to something that would not need touching again if the
>> underlying infrastructure was made available to, say, RISC-V as well. But of
>> course - right now the goal needs to be to address the CI and osstest
>> breakage.
> I agree. I would suggest to guard it with LIBXL_HAVE_DT_OVERLAY which is for 
> now
> only defined for arm32/arm64. This way the code will not need to be modified 
> if other
> arch gain support for the feature.

Ah yes, that ought to work. While there perhaps also replace the conditional
around the declaration of the function in libxl.h. (But of course Anthony
may tell me/us that this isn't the way to go.)

> If you agree, I can send a patch to unbreak CI unless you want to do this.

Please do.

Thanks, Jan

Re: [QEMU PATCH v4 09/13] virtio-gpu: Handle resource blob commands

[Huang Rui]

On Wed, Sep 06, 2023 at 11:39:09AM +0800, Akihiko Odaki wrote:
> On 2023/09/06 12:09, Huang Rui wrote:
> > On Tue, Sep 05, 2023 at 05:20:43PM +0800, Akihiko Odaki wrote:
> >> On 2023/09/05 18:08, Huang Rui wrote:
> >>> On Thu, Aug 31, 2023 at 06:24:32PM +0800, Akihiko Odaki wrote:
>  On 2023/08/31 18:32, Huang Rui wrote:
> > From: Antonio Caggiano 
> >
> > Support BLOB resources creation, mapping and unmapping by calling the
> > new stable virglrenderer 0.10 interface. Only enabled when available and
> > via the blob config. E.g. -device virtio-vga-gl,blob=true
> >
> > Signed-off-by: Antonio Caggiano 
> > Signed-off-by: Dmitry Osipenko 
> > Signed-off-by: Xenia Ragiadakou 
> > Signed-off-by: Huang Rui 
> > ---
> >
> > v1->v2:
> >- Remove unused #include "hw/virtio/virtio-iommu.h"
> >
> >- Add a local function, called virgl_resource_destroy(), that is 
> > used
> >  to release a vgpu resource on error paths and in 
> > resource_unref.
> >
> >- Remove virtio_gpu_virgl_resource_unmap from 
> > virtio_gpu_cleanup_mapping(),
> >  since this function won't be called on blob resources and also 
> > because
> >  blob resources are unmapped via 
> > virgl_cmd_resource_unmap_blob().
> >
> >- In virgl_cmd_resource_create_blob(), do proper cleanup in 
> > error paths
> >  and move QTAILQ_INSERT_HEAD(&g->reslist, res, next) after the 
> > resource
> >  has been fully initialized.
> >
> >- Memory region has a different life-cycle from virtio gpu 
> > resources
> >  i.e. cannot be released synchronously along with the vgpu 
> > resource.
> >  So, here the field "region" was changed to a pointer that will 
> > be
> >  released automatically once the memory region is unparented 
> > and all
> >  of its references have been released.
> >  Also, since the pointer can be used to indicate whether the 
> > blob
> >  is mapped, the explicit field "mapped" was removed.
> >
> >- In virgl_cmd_resource_map_blob(), add check on the value of
> >  res->region, to prevent beeing called twice on the same 
> > resource.
> >
> >- Remove direct references to parent_obj.
> >
> >- Separate declarations from code.
> >
> > hw/display/virtio-gpu-virgl.c  | 213 
> > +
> > hw/display/virtio-gpu.c|   4 +-
> > include/hw/virtio/virtio-gpu.h |   5 +
> > meson.build|   4 +
> > 4 files changed, 225 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/display/virtio-gpu-virgl.c 
> > b/hw/display/virtio-gpu-virgl.c
> > index 312953ec16..17b634d4ee 100644
> > --- a/hw/display/virtio-gpu-virgl.c
> > +++ b/hw/display/virtio-gpu-virgl.c
> > @@ -17,6 +17,7 @@
> > #include "trace.h"
> > #include "hw/virtio/virtio.h"
> > #include "hw/virtio/virtio-gpu.h"
> > +#include "hw/virtio/virtio-gpu-bswap.h"
> > 
> > #include "ui/egl-helpers.h"
> > 
> > @@ -78,9 +79,24 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU 
> > *g,
> > virgl_renderer_resource_create(&args, NULL, 0);
> > }
> > 
> > +static void virgl_resource_destroy(VirtIOGPU *g,
> > +   struct virtio_gpu_simple_resource 
> > *res)
> > +{
> > +if (!res)
> > +return;
> > +
> > +QTAILQ_REMOVE(&g->reslist, res, next);
> > +
> > +virtio_gpu_cleanup_mapping_iov(g, res->iov, res->iov_cnt);
> > +g_free(res->addrs);
> > +
> > +g_free(res);
> > +}
> > +
> > static void virgl_cmd_resource_unref(VirtIOGPU *g,
> >  struct virtio_gpu_ctrl_command 
> > *cmd)
> > {
> > +struct virtio_gpu_simple_resource *res;
> > struct virtio_gpu_resource_unref unref;
> > struct iovec *res_iovs = NULL;
> > int num_iovs = 0;
> > @@ -88,13 +104,22 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
> > VIRTIO_GPU_FILL_CMD(unref);
> > trace_virtio_gpu_cmd_res_unref(unref.resource_id);
> > 
> > +res = virtio_gpu_find_resource(g, unref.resource_id);
> > +
> > virgl_renderer_resource_detach_iov(unref.resource_id,
> >&res_iovs,
> >&num_iovs);
> > if (res_iovs != NULL && num_iovs != 0) {
> > virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
> > +if (res) {
> > +res->iov = NULL;
> > +res->iov_cnt = 0;

Re: [XEN PATCH 09/13] xen/common: address violations of MISRA C:2012 Directive 4.10

[Simone Ballarin]

On 06/09/23 00:18, Stefano Stabellini wrote:

On Thu, 31 Aug 2023, Simone Ballarin wrote:

On 31/08/23 15:05, Jan Beulich wrote:

On 31.08.2023 14:54, Simone Ballarin wrote:

On 31/08/23 13:10, Jan Beulich wrote:

On 31.08.2023 12:08, Simone Ballarin wrote:

The danger of multi-inclusion also exists for .c files, why do you
want
to avoid guards for them?


Counter question: Why only add guards to some of them? (My personal
answer is "Because it's extra clutter.")


It's not "some of them", it's exactly the ones used in an #include
directive, so I'm not getting your objection.


My point is that by adding guards only for files we presently use in some
#include directive, we set us up for introducing new violations as soon
as another .c file becomes the subject of an #include.The more that it
is unusual to add guards in .c files, i.e. it is to be expected that
people wouldn't think about this extra Misra requirement.

Jan


I can agree to partially adopt the directive: I will add a deviation for C
files in rules.txt.


Sorry for the late reply as I was OOO. Please hold on before adding a
deviation for C files.

In general, I think including .c files is not common behavior, and
should be restricted to special cases. We could say that exactly because
they are special, they follow different rules so we can skip the guards.
Or we could say that they are still at risk of double-inclusion, hence
we should be consistent and protect them too. I think we should discuss
the topic during the next MISRA C meeting.


Ok, I will drop changes in C files without adding the deviation.

--
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)

Re: [XEN][PATCH v12 20/20] tools/xl: Add new xl command overlay for device tree overlay support

[Michal Orzel]

On 06/09/2023 08:55, Jan Beulich wrote:
> 
> 
> On 06.09.2023 03:16, Vikram Garhwal wrote:
>> --- a/tools/xl/xl_vmcontrol.c
>> +++ b/tools/xl/xl_vmcontrol.c
>> @@ -1265,6 +1265,58 @@ 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 EXIT_FAILURE;
>> +}
>> +
>> +if (overlay_config_file) {
>> +rc = libxl_read_file_contents(ctx, overlay_config_file,
>> +  &overlay_dtb, &overlay_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);
> 
> Because of this being Arm-only (as validly pointed out by osstest), I expect
> the entire function here as well as its entry in cmd_table[] want to be
> Arm-specific, too? Of course it would be nice to not key this to __arm__ /
> __aarch64__, but to something that would not need touching again if the
> underlying infrastructure was made available to, say, RISC-V as well. But of
> course - right now the goal needs to be to address the CI and osstest
> breakage.
I agree. I would suggest to guard it with LIBXL_HAVE_DT_OVERLAY which is for now
only defined for arm32/arm64. This way the code will not need to be modified if 
other
arch gain support for the feature.
If you agree, I can send a patch to unbreak CI unless you want to do this.

~Michal