Re: [PATCH v4 1/5] xen/ppc: Implement atomic.h

[Jan Beulich]

On 12.09.2023 01:56, Shawn Anastasio wrote:
> On 9/11/23 6:33 AM, Jan Beulich wrote:
>> On 09.09.2023 00:50, Shawn Anastasio wrote:
>>> +static always_inline void read_atomic_size(const volatile void *p, void 
>>> *res,
>>> +   unsigned int size)
>>> +{
>>> +ASSERT(IS_ALIGNED((vaddr_t)p, size));
>>> +switch ( size )
>>> +{
>>> +case 1:
>>> +*(uint8_t *)res = read_u8_atomic(p);
>>> +break;
>>> +case 2:
>>> +*(uint16_t *)res = read_u16_atomic(p);
>>> +break;
>>> +case 4:
>>> +*(uint32_t *)res = read_u32_atomic(p);
>>> +break;
>>> +case 8:
>>> +*(uint64_t *)res = read_u64_atomic(p);
>>> +break;
>>> +default:
>>> +__bad_atomic_read(p, res);
>>> +break;
>>> +}
>>> +}
>>> +
>>> +static always_inline void write_atomic_size(volatile void *p, const void 
>>> *val,
>>> +unsigned int size)
>>> +{
>>> +ASSERT(IS_ALIGNED((vaddr_t)p, size));
>>> +switch ( size )
>>> +{
>>> +case 1:
>>> +write_u8_atomic(p, *(const uint8_t *)val);
>>> +break;
>>> +case 2:
>>> +write_u16_atomic(p, *(const uint16_t *)val);
>>> +break;
>>> +case 4:
>>> +write_u32_atomic(p, *(const uint32_t *)val);
>>> +break;
>>> +case 8:
>>> +write_u64_atomic(p, *(const uint64_t *)val);
>>> +break;
>>> +default:
>>> +__bad_atomic_size();
>>> +break;
>>> +}
>>> +}
>>
>> These two functions being as similar as is possible, ...
>>
>>> +#define read_atomic(p) 
>>> \
>>> +({ 
>>> \
>>> +union {
>>> \
>>> +typeof(*(p)) val;  
>>> \
>>> +char c[sizeof(*(p))];  
>>> \
>>> +} x_;  
>>> \
>>> +read_atomic_size(p, x_.c, sizeof(*(p)));   
>>> \
>>> +x_.val;
>>> \
>>> +})
>>
>> ... is there a reason you keep using a union here ...
>>
>>> +#define write_atomic(p, x) 
>>> \
>>> +do 
>>> \
>>> +{  
>>> \
>>> +typeof(*(p)) x_ = (x); 
>>> \
>>> +write_atomic_size(p, &x_, sizeof(*(p)));   
>>> \
>>> +} while ( 0 )
>>
>> ... while you did away with its use here?
>>
> 
> Yes. In the case of read_atomic the caller is allowed (expected, even)
> to pass in const pointers for `p`, which wouldn't work if a simple
> typeof(*(p)) declaration was used since it would inherit the constness
> and thus wouldn't be passable to read_atomic_size.

But the helper function's first parameter is const volatile void *, and
while you can't assign to a const variable, such a variable can of course
have an initializer (like you have it in the write case).

Jan

Re: [XEN PATCH 10/13] x86/viridian: address violations of MISRA C:2012 Rule 7.3

[Paul Durrant]

On 03/08/2023 11:22, Simone Ballarin wrote:

From: Gianluca Luparini 

The xen sources contain violations of MISRA C:2012 Rule 7.3 whose headline
states:
"The lowercase character 'l' shall not be used in a literal suffix".

Use the "L" suffix instead of the "l" suffix, to avoid potential ambiguity.
If the "u" suffix is used near "L", use the "U" suffix instead, for consistency.

The changes in this patch are mechanical.

Signed-off-by: Gianluca Luparini 
Signed-off-by: Simone Ballarin 
---
  xen/arch/x86/hvm/viridian/synic.c |  2 +-
  xen/arch/x86/hvm/viridian/time.c  | 10 +-
  2 files changed, 6 insertions(+), 6 deletions(-)



Reviewed-by: Paul Durrant 

Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode

[Jan Beulich]

On 12.09.2023 02:48, Henry Wang wrote:
>> On Sep 11, 2023, at 23:01, Jan Beulich  wrote:
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -788,6 +788,14 @@ Specify the size of the console debug tr
>> additionally a trace buffer of the specified size is allocated per cpu.
>> The debug trace feature is only enabled in debugging builds of Xen.
>>
>> +### dit (x86)
>> +> `= `
>> +
>> +> Default: `CONFIG_DIT_DEFAULT`
>> +
>> +Specify whether Xen and guests should operate in Data Independent Timing
>> +mode.
>> +
> 
> Since a new cmdline interface is added, I am wondering would such
> addtion deserves a CHANGELOG entry?

I'm open to opinions, albeit not so much because of the added command line
option.

Jan

Re: [XEN PATCH v5 3/4] x86/viridian: address violations of MISRA C:2012 Rule 7.2

[Paul Durrant]

On 28/08/2023 12:03, Simone Ballarin wrote:

From: Gianluca Luparini 

The xen sources contains violations of MISRA C:2012 Rule 7.2 whose
headline states:
"A 'u' or 'U' suffix shall be applied to all integer constants
that are represented in an unsigned type".

Add the 'U' suffix to integers literals with unsigned type and also to other
literals used in the same contexts or near violations, when their positive
nature is immediately clear. The latter changes are done for the sake of
uniformity.

Signed-off-by: Gianluca Luparini 
Signed-off-by: Simone Ballarin 
Reviewed-by: Stefano Stabellini 
---
Changes in v4:
- change commit headline
- add Reviewed-by

Changes in v3:
- create this commit for 'viridian.c' and 'hyperv-tlfs.h'
---
  xen/arch/x86/hvm/viridian/viridian.c |  2 +-
  xen/arch/x86/include/asm/guest/hyperv-tlfs.h | 28 ++--
  2 files changed, 15 insertions(+), 15 deletions(-)



Reviewed-by: Paul Durrant 

[linux-linus test] 182949: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. 
vs. 182531
 test-amd64-coresched-amd64-xl  8 xen-bootfail 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-libvirt-raw  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-libvirt-qcow2  8 xen-boot   fail REGR. vs. 182531
 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-freebsd12-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-pvhv2-intel  8 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-xl-qemut-debianhvm-amd64  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-libvirt-xsm  8 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-dom0pvh-xl-amd 14 guest-start   fail REGR. vs. 182531
 test-amd64-amd64-xl-shadow8 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-pair12 xen-boot/src_hostfail REGR. vs. 182531
 test-amd64-amd64-pair13 xen-boot/dst_hostfail REGR. vs. 182531
 test-amd64-amd64-qemuu-nested-intel  8 xen-boot  fail REGR. vs. 182531
 test-amd64-amd64-examine-uefi  8 reboot  fail REGR. vs. 182531
 test-amd64-amd64-xl-qemut-ws16-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-ovmf-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-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 182531
 test-amd64-amd64-xl   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-pygrub   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-debianhvm-i386-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-xl-credit2   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-examine-bios  8 reboot  fail REGR. vs. 182531
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  8 xen-boot fail 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

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 16 saverestore-support-checkfail  like 182531
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail 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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  16 saverestore-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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-x

Re: [PATCH RESEND v2 2/2] xen/arm: Enlarge identity map space to 127TiB

[Leo Yan]

Hi Henry,

On Tue, Sep 12, 2023 at 03:01:30AM +, Henry Wang wrote:

[...]

> At Arm we also test Xen on AVA in our CI, but our setup is based on Yocto
> meta-adlink-ampere layer [1].

Thanks for sharing the info.  Yes, we (Linaro) also work with Yocto
based on EWAOL [1] or TRS [2].

> >> Also, in v1, the problem was also depending on the firmware version. Do you
> >> know if it fails booting on a new or old firmware?
> > 
> > My colleague Alexey (CCed) and I both tested Xen hypervisor on own AVA
> > machine, Alexey produced this issue on his machine with the old
> > firmware, I upgraded to the new firmware so I cannot see the issue
> > anymore.
> > 
> >>>   (XEN) MODULE[0]: 0807f6df - 0807f6f3e000 Xen
> >>>   (XEN) MODULE[1]: 0807f8054000 - 0807f8056000 Device Tree
> >>>   (XEN) MODULE[2]: fa834000 - fc5de1d5 Ramdisk
> >>>   (XEN) MODULE[3]: fc5df000 - ffb3f810 Kernel
> 
> I tried to dig a bit more about Xen log printed by our AVA setup, and [2] is 
> our
> RAM and MODULES setup, just FYI and hopefully it helps.

Yeah, the log shows the high address space is above 8TB:

  (XEN) RAM: 0800 - 08007fff
  (XEN) RAM: 0801 - 08039bff
  (XEN) RAM: 08039c00 - 0803

Appreciate for the log and it's helpful!

Leo

> [1] https://github.com/ADLINK/meta-adlink-ampere
> [2] https://pastebin.com/raw/0DQpmJP1
> 
> Kind regards,
> Henry
> 
> 

Re: [PATCH RESEND v2 2/2] xen/arm: Enlarge identity map space to 127TiB

[Henry Wang]

Hi Leo,

> On Sep 12, 2023, at 10:41, Leo Yan  wrote:
> 
> Hi Julien,
> 
> On Mon, Sep 11, 2023 at 04:13:27PM +0100, Julien Grall wrote:
>> Hi Leo,
>> 
>> I know you said you will respin the series. I'd like to leave some comments
>> to avoid having another round afterwards.
> 
> Thanks!
> 
>> On 09/09/2023 09:34, Leo Yan wrote:
>>> On some platforms, the memory regions could be:
>> 
>> Can you add some details in the commit message on which platform you saw the
>> issue?
> 
> Sure, I will add it.  The issue happens on the ADLink AVA platform.

At Arm we also test Xen on AVA in our CI, but our setup is based on Yocto
meta-adlink-ampere layer [1].

> 
>> Also, in v1, the problem was also depending on the firmware version. Do you
>> know if it fails booting on a new or old firmware?
> 
> My colleague Alexey (CCed) and I both tested Xen hypervisor on own AVA
> machine, Alexey produced this issue on his machine with the old
> firmware, I upgraded to the new firmware so I cannot see the issue
> anymore.
> 
>>>   (XEN) MODULE[0]: 0807f6df - 0807f6f3e000 Xen
>>>   (XEN) MODULE[1]: 0807f8054000 - 0807f8056000 Device Tree
>>>   (XEN) MODULE[2]: fa834000 - fc5de1d5 Ramdisk
>>>   (XEN) MODULE[3]: fc5df000 - ffb3f810 Kernel

I tried to dig a bit more about Xen log printed by our AVA setup, and [2] is our
RAM and MODULES setup, just FYI and hopefully it helps.

[1] https://github.com/ADLINK/meta-adlink-ampere
[2] https://pastebin.com/raw/0DQpmJP1

Kind regards,
Henry

[xen-unstable test] 182941: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-win7-amd64 12 windows-install  fail REGR. vs. 182912

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

version targeted for testing:
 xen  35949b8c58cb6c835da1fdd8843ed51727e654c0
baseline version:
 xen  49b2d6a529122f10e17a17f807d8d51154b5ba14

Last test of basis   182912  2023-09-11 01:52:03 Z1 days
Testing same since   182941  2023-09-11 13:07:03 Z0 days

Re: [PATCH RESEND v2 2/2] xen/arm: Enlarge identity map space to 127TiB

[Leo Yan]

Hi Julien,

On Mon, Sep 11, 2023 at 04:13:27PM +0100, Julien Grall wrote:
> Hi Leo,
> 
> I know you said you will respin the series. I'd like to leave some comments
> to avoid having another round afterwards.

Thanks!

> On 09/09/2023 09:34, Leo Yan wrote:
> > On some platforms, the memory regions could be:
> 
> Can you add some details in the commit message on which platform you saw the
> issue?

Sure, I will add it.  The issue happens on the ADLink AVA platform.

> Also, in v1, the problem was also depending on the firmware version. Do you
> know if it fails booting on a new or old firmware?

My colleague Alexey (CCed) and I both tested Xen hypervisor on own AVA
machine, Alexey produced this issue on his machine with the old
firmware, I upgraded to the new firmware so I cannot see the issue
anymore.

> >(XEN) MODULE[0]: 0807f6df - 0807f6f3e000 Xen
> >(XEN) MODULE[1]: 0807f8054000 - 0807f8056000 Device Tree
> >(XEN) MODULE[2]: fa834000 - fc5de1d5 Ramdisk
> >(XEN) MODULE[3]: fc5df000 - ffb3f810 Kernel
> > 
> > In this case, the Xen binary is loaded above 2TiB.  2TiB is the maximum
> > identity map space supported by Xen, thus it fails to boot up due to the
> > out of the range.
> > 
> > This patch introduces several macros to present the zeroth page table's
> 
> Typo: s/zeroth/zeroeth/

Will fix.

> > slot numbers for easier readable.  Based on the defined macros, it
> > enlarges identity map space to 127TiB, which can support the memory
> > space [0x0 .. 0x7eff__] so has flexibility for various
> > platforms.
> 
> Reserving 127 TiB for just the identity mapping is quite a lot. How did you
> decide the limit?

When I reviewed the existed code, I found it reserves 125TiB:

  0x0280 - 0x7fff (125TB, L0 slots [5..255])
Unused

Seems to me, we can map this area.  Ideally, if we only map for the
first level's page table, we can just fill the zeroeth page and don't
need to allocate extra page tables.

> What limit do you need on your platform?

On AVA platform, we can see the memory layout is:

DRAM memory regions:
  Node[0] Region[0]: 0x8000 - 0x
  Node[0] Region[1]: 0x0800 - 0x08007fff
  Node[0] Region[2]: 0x0801 - 0x0807

So the phycial memory address is splitted into the low address and the
high address.  The high memory address is above 8TiB.

> > Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to 
> > prepare/enable/disable")
> > Reported-by: Alexey Klimov 
> > Signed-off-by: Leo Yan 
> > ---
> >   xen/arch/arm/arm64/mm.c   | 12 
> >   xen/arch/arm/include/asm/config.h | 15 ---
> >   xen/arch/arm/mm.c |  2 +-
> >   3 files changed, 17 insertions(+), 12 deletions(-)
> > 
> > diff --git a/xen/arch/arm/arm64/mm.c b/xen/arch/arm/arm64/mm.c
> > index 78b7c7eb00..802170cf29 100644
> > --- a/xen/arch/arm/arm64/mm.c
> > +++ b/xen/arch/arm/arm64/mm.c
> > @@ -40,8 +40,10 @@ static void __init prepare_boot_identity_mapping(void)
> >   clear_page(boot_second_id);
> >   clear_page(boot_third_id);
> > -if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )
> > -panic("Cannot handle ID mapping above 2TB\n");
> > +if ( id_offsets[0] >= XEN_IDENTITY_MAP_L0_END )
> 
> I don't see the value of renaming IDENTIY_MAPPING_AREA_NR_L0 to
> XEN_IDENTIY_MAP_L0_END. See more below.

Okay.  It's fine for me to keep the naming.

> > +/* 1TiB occupies 2 slots in zeroeth table */
> 
> I don't understand how this comment is related to the message below.
> 
> > +panic("Cannot handle ID mapping above %dTiB\n",
> 
> The value is unsigned, so this you should use "%u". Also we have been using
> "TB" in Xen rather than "TiB". I would rather prefer if we keep the same for
> consistency even if this is not totally accurate.

Sure, will fix it.

> > +  XEN_IDENTITY_MAP_L0_END>>1);
> 
> Coding style: please add a space before and after >>.

Will fix.

> >   /* Link first ID table */
> >   pte = mfn_to_xen_entry(virt_to_mfn(boot_first_id), MT_NORMAL);
> > @@ -73,8 +75,10 @@ static void __init prepare_runtime_identity_mapping(void)
> >   lpae_t pte;
> >   DECLARE_OFFSETS(id_offsets, id_addr);
> > -if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )
> > -panic("Cannot handle ID mapping above 2TB\n");
> > +if ( id_offsets[0] >= XEN_IDENTITY_MAP_L0_END )
> > +/* 1TiB occupies 2 slots in zeroeth table */
> > +panic("Cannot handle ID mapping above %dTiB\n",
> > +  XEN_IDENTITY_MAP_L0_END>>1);
> 
> Similar remarks here.

Will do.

> >   /* Link first ID table */
> >   pte = pte_of_xenaddr((vaddr_t)xen_first_id);
> > diff --git a/xen/arch/arm/include/asm/config.h 
> > b/xen/arch/arm/include/asm/config.h
> > index 21f4e68a40..b772f1574d 100644
> > --- a/xen/arch/arm/include/asm/config.h
> > +++ b/xen/arch/arm/include/asm/c

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

[Stefano Stabellini]

On Mon, 11 Sep 2023, Anthony Chan wrote:
> On Wed, 6 Sep 2023, Stefano Stabellini wrote:
> > On Wed, 6 Sep 2023, Anthony Chan wrote:
> > > Thanks, I've tried patches that stemmed from that discussion but
> > > unfortunately, doesn't resolve the issue.  In fact, the s2idle_loop
> > > branch might not be the problem at all.  I experimented with Xen to allow 
> > > the
> > > 'idle-states' into the FDT and prevented xen_guest_init on Linux from 
> > > disabling
> > > the 'cpuidle' driver (arch/arm/xen/enlighten.c).  When I trigger a 
> > > suspend, I
> > > can see now another thread (believe it's the idle thread) call into
> > > drivers/firmware/psci/psci.c:__psci_cpu_suspend and then the Xen
> > > counterpart at xen/arch/arm/vpsci.c:do_psci_0_2_cpu_suspend.
> >
> > OK but remember that Xen is not implementing do_psci_0_2_cpu_suspend
> > correctly at the moment. Either we need to fix the Xen implementation, or we
> > need to configure Linux so that it calls WFI instead of __psci_cpu_suspend.
> >
> > As a test, can you try to apply the attached patch to Xen as a tenative 
> > fix?  Or
> > you could change drivers/firmware/psci/psci.c:__psci_cpu_suspend to call
> > WFI instead of the PSCI operation (making sure to go to the entry_point
> > instead of returning).
> 
> Tried the patch and substituting a WFI for a PSCI op, but Xen still watchdogs 
> on the VMs in both cases.  I noticed the other Linux generic arm 'cpu-idle' 
> driver which used to do issue a WFI/cpu_do_idle isn't useable anymore either. 
>  I'm not sure if Xen may have used to rely on this generic driver to get the 
> WFI.

I was running out of ideas so I went back to look at the watchdog
console log:

(XEN) do_psci_0_2_cpu_suspend
(XEN) Watchdog timer fired for domain 0
(XEN) Hardware Dom0 shutdown: watchdog rebooting machine

Checking the code, it seems that the Xen watchdog is set by
xen/common/sched/core.c:SCHEDOP_watchdog, which is called by
tools/libs/ctrl/xc_domain.c:xc_watchdog.

xc_watchdog is called by tools/misc/xenwatchdogd.c. Is it possible that
this problem is entirely caused by the daemon xenwatchdogd running in
the background? What happens if you kill xenwatchdogd and try again
without it (even better not start it at all)?

Re: [XEN PATCH v2 1/5] automation: add python3's setuptools to containers

[Stefano Stabellini]

On Mon, 11 Sep 2023, Javi Merino wrote:
> In preparation of dropping python distutils and moving to setuptools,
> add the python3 setuptools module to the containers that need it.
> 
> The centos7 container was building using python2.  Change it to build
> python scripts using python3.
> 
> Debian Stretch is no longer debian oldstable, so move to the archive
> repositories.
> 
> Signed-off-by: Javi Merino 
> ---
>  automation/build/alpine/3.18.dockerfile|  1 +
>  automation/build/archlinux/current.dockerfile  |  1 +
>  automation/build/centos/7.dockerfile   |  3 ++-
>  automation/build/debian/bookworm.dockerfile|  1 +
>  automation/build/debian/stretch.dockerfile | 11 ++-
>  automation/build/suse/opensuse-leap.dockerfile |  1 +
>  automation/build/ubuntu/bionic.dockerfile  |  1 +
>  automation/build/ubuntu/focal.dockerfile   |  1 +
>  automation/build/ubuntu/trusty.dockerfile  |  1 +
>  automation/build/ubuntu/xenial.dockerfile  |  1 +

We are missing:
automation/build/alpine/3.18-arm64v8.dockerfile
automation/build/suse/opensuse-tumbleweed.dockerfile
automation/build/suse/opensuse-leap.dockerfile
automation/build/debian/jessie-i386.dockerfile
automation/build/debian/bookworm-i386.dockerfile
automation/build/debian/stretch-i386.dockerfile
automation/build/debian/jessie.dockerfile
automation/build/debian/bookworm-arm64v8.dockerfile
automation/build/fedora/29.dockerfile

+Bertrand because I am not sure if we need any changes to the Yocto
containers




>  10 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/automation/build/alpine/3.18.dockerfile 
> b/automation/build/alpine/3.18.dockerfile
> index ca5756955e..5d2a69a060 100644
> --- a/automation/build/alpine/3.18.dockerfile
> +++ b/automation/build/alpine/3.18.dockerfile
> @@ -34,6 +34,7 @@ RUN apk --no-cache add \
>ocaml-findlib \
>patch  \
>python3-dev \
> +  py3-setuptools \
>texinfo \
>util-linux-dev \
>xz-dev \
> diff --git a/automation/build/archlinux/current.dockerfile 
> b/automation/build/archlinux/current.dockerfile
> index 13fb472d9e..47e79637a4 100644
> --- a/automation/build/archlinux/current.dockerfile
> +++ b/automation/build/archlinux/current.dockerfile
> @@ -34,6 +34,7 @@ RUN pacman -S --refresh --sysupgrade --noconfirm 
> --noprogressbar --needed \
>  pixman \
>  pkgconfig \
>  python \
> +python-setuptools \
>  sdl \
>  sdl2 \
>  spice \
> diff --git a/automation/build/centos/7.dockerfile 
> b/automation/build/centos/7.dockerfile
> index 69dcefb2f0..f14f70014a 100644
> --- a/automation/build/centos/7.dockerfile
> +++ b/automation/build/centos/7.dockerfile
> @@ -22,7 +22,8 @@ RUN yum -y update \
>  ncurses-devel \
>  zlib-devel \
>  openssl-devel \
> -python-devel \
> +python3-devel \
> +python3-setuptools \
>  libuuid-devel \
>  pkgconfig \
>  flex \
> diff --git a/automation/build/debian/bookworm.dockerfile 
> b/automation/build/debian/bookworm.dockerfile
> index f8415425e6..ae008c8d46 100644
> --- a/automation/build/debian/bookworm.dockerfile
> +++ b/automation/build/debian/bookworm.dockerfile
> @@ -16,6 +16,7 @@ RUN apt-get update && \
>  libncurses5-dev \
>  libssl-dev \
>  python3-dev \
> +python3-setuptools \
>  xorg-dev \
>  uuid-dev \
>  libyajl-dev \
> diff --git a/automation/build/debian/stretch.dockerfile 
> b/automation/build/debian/stretch.dockerfile
> index 1af6c691f8..9f4b91a9e3 100644
> --- a/automation/build/debian/stretch.dockerfile
> +++ b/automation/build/debian/stretch.dockerfile
> @@ -1,4 +1,4 @@
> -FROM debian:stretch
> +FROM debian/eol:stretch
>  LABEL maintainer.name="The Xen Project" \
>maintainer.email="xen-devel@lists.xenproject.org"
>  
> @@ -8,6 +8,14 @@ ENV USER root
>  RUN mkdir /build
>  WORKDIR /build
>  
> +# Debian stretch is now archived.  Fetch packages from
> +# archive.debian.org and skip the stretch-updates repository, as it is
> +# not longer valid.
> +RUN sed -i \
> +-e 's/deb.debian.org/archive.debian.org/' \
> +-e '/stretch-updates/d' \
> +/etc/apt/sources.list

For the records I would prefer to get rid of Stretch altogether, but I
don't want to scope-creep this patch series, so this is OK for now.




>  # build depends
>  RUN apt-get update && \
>  apt-get --quiet --yes install \
> @@ -17,6 +25,7 @@ RUN apt-get update && \
>  libssl-dev \
>  python-dev \
>  python3-dev \
> +python3-setuptools \
>  xorg-dev \
>  uuid-dev \
>  libyajl-dev \
> diff --git a/automation/build/suse/opensuse-leap.dockerfile 
> b/automation/build/suse/opensuse-leap.dockerfile
> index 98ee42970d..7010b71aca 100644
> --- a/automation/build/suse/opensuse-leap.dockerfile
> +++ b/automation/build/suse/opensuse-leap.dockerfile
> @@ -60,6 +60,7 @@ RUN zypper install -y --no-recom

Re: [PATCH v6 00/13] xen/arm: Split MMU code as the prepration of MPU work

[Henry Wang]

Hi Ayan,

> On Sep 11, 2023, at 22:03, Ayan Kumar Halder  wrote:
> 
> Hi Henry,
> 
> On 08/09/2023 23:15, Stefano Stabellini wrote:
>> 
>> I committed patches 1-5
>> 
>> On Mon, 28 Aug 2023, Henry Wang wrote:
>>> 
>>> Henry Wang (9):
>>>   xen/arm: Introduce CONFIG_MMU Kconfig option
>>>   xen/arm64: Split and move MMU-specific head.S to mmu/head.S
>>>   xen/arm64: Fold setup_fixmap() to create_page_tables()
>>>   xen/arm: Split page table related code to mmu/pt.c
>>>   xen/arm: Split MMU system SMP MM bringup code to mmu/smpboot.c
>>>   xen/arm: Fold mmu_init_secondary_cpu() to head.S
>>>   xen/arm: Extract MMU-specific MM code
>>>   xen/arm: Split MMU-specific setup_mm() and related code out
>>>   xen/arm: Fold pmap and fixmap into MMU system
>>> 
>>> Penny Zheng (2):
>>>   xen/arm: Rename init_secondary_pagetables() to prepare_secondary_mm()
>>>   xen/arm: mmu: move MMU specific P2M code to mmu/p2m.{c,h}
>>> 
>>> Wei Chen (2):
>>>   xen/arm64: head.S: Introduce enable_{boot,secondary}_cpu_mm()
>>>   xen/arm: Move MMU related definitions from config.h to mmu/layout.h
> 
> Except for "[PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to head.S 
> " where I have a small comment, I am happy with the current set.

Thanks!

> 
> I have made the corresponding changes for Arm32 MMU (See 
> https://gitlab.com/xen-project/people/ayankuma/xen/-/commits/mmu_arm32_on_arm64?ref_type=heads
>  ),
> 
> and triggered the CI. It looks all good (qemu-smoke-ppc64le-pseries-gcc 
> failure seems unrelated).
> 
> I have sent out the patches for review so that the maintainers/you can have a 
> look and review them

Sure, me and Penny will have a look.

Kind regards,
Henry

> 
> ("[XEN v1 0/4] xen/arm: Split MMU code as the prepration of MPU work form 
> Arm32").
> 
> - Ayan

Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode

[Henry Wang]

Hi Jan,

> On Sep 11, 2023, at 23:01, Jan Beulich  wrote:
> 
> [1] specifies a long list of instructions which are intended to exhibit
> timing behavior independent of the data they operate on. On certain
> hardware this independence is optional, controlled by a bit in a new
> MSR. Provide a command line option to control the mode Xen and its
> guests are to operate in, with a build time control over the default.
> Longer term we may want to allow guests to control this.
> 
> Since Arm64 supposedly also has such a control, put command line option
> and Kconfig control in common files.
> 
> [1] 
> https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/best-practices/data-operand-independent-timing-isa-guidance.html
> 
> Requested-by: Demi Marie Obenour 
> Signed-off-by: Jan Beulich 
> ---
> This may be viewed as a new feature, and hence be too late for 4.18. It
> may, however, also be viewed as security relevant, which is why I'd like
> to propose to at least consider it.

Fine with me if this patch can be properly reviewed on time, because of
the security relevance. 

> 
> Slightly RFC, in particular for whether the Kconfig option should
> default to Y or N.
> 
> I would have wanted to invoke setup_doitm() from cpu_init(), but that
> works only on the BSP. On APs cpu_init() runs before ucode loading.
> Plus recheck_cpu_features() invoking identify_cpu() takes care of the
> BSP during S3 resume.
> ---
> v2: Introduce and use cpu_has_doitm. Add comment "borrowed" from the
>XenServer patch queue patch providing similar functionality.
>Re-base.
> 
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -788,6 +788,14 @@ Specify the size of the console debug tr
> additionally a trace buffer of the specified size is allocated per cpu.
> The debug trace feature is only enabled in debugging builds of Xen.
> 
> +### dit (x86)
> +> `= `
> +
> +> Default: `CONFIG_DIT_DEFAULT`
> +
> +Specify whether Xen and guests should operate in Data Independent Timing
> +mode.
> +

Since a new cmdline interface is added, I am wondering would such
addtion deserves a CHANGELOG entry?

Kind regards,
Henry

Re: [PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to head.S

[Henry Wang]

Hi Julien,

> On Sep 11, 2023, at 22:51, Julien Grall  wrote:
> On 31/08/2023 10:16, Henry Wang wrote:
>>> On Aug 31, 2023, at 17:12, Ayan Kumar Halder  wrote:
>>> 
>>> Hi Henry,
>>> 
>>> On 28/08/2023 02:32, Henry Wang wrote:
 
 diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
 index 33b038e7e0..39218cf15f 100644
 --- a/xen/arch/arm/arm32/head.S
 +++ b/xen/arch/arm/arm32/head.S
 @@ -83,6 +83,25 @@
  isb
  .endm
 
 +/*
 + * Enforce Xen page-tables do not contain mapping that are both
 + * Writable and eXecutables.
 + *
 + * This should be called on each secondary CPU.
 + */
 +.macro pt_enforce_wxn tmp
 +mrc   CP32(\tmp, HSCTLR)
 +orr   \tmp, \tmp, #SCTLR_Axx_ELx_WXN
 +dsb
 +mcr   CP32(\tmp, HSCTLR)
 +/*
 + * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized
 + * before flushing the TLBs.
 + */
 +isb
 +flush_xen_tlb_local \tmp
 +.endm
 +
  /*
   * Common register usage in this file:
   *   r0  -
 @@ -254,6 +273,7 @@ secondary_switched:
  /* Use a virtual address to access the UART. */
  mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
  #endif
 +pt_enforce_wxn
  
>>> 
>>> Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU 
>>> related functionality are bundled together?
>>> 
>>> Also AFAIU, mov_w has not effect on pt_enforce_wxn().
>>> 
>>> So that I can create a function "enable_secondary_cpu_mm()" - similar to 
>>> one you introduced for arm64
>> Sure, I am good with this if other maintainers do not have any objections.
> 
> I am objecting. It would be quite handy to print a message on the console to 
> indicate that we are enforce WXN. So we want to update UART address (stored 
> in r11) before hand.

Good idea about the printing, I am happy to add a printed message on top of the 
macro saying that we are enforcing WXN from here if you agree.

Kind regards,
Henry

> 
> Cheers,
> 
> -- 
> Julien Grall

Re: [PATCH v3] docs/misra: add rule 2.1 exceptions

[Stefano Stabellini]

On Mon, 11 Sep 2023, Julien Grall wrote:
> Hi Stefano,
> 
> On 09/09/2023 00:03, Stefano Stabellini wrote:
> > From: Stefano Stabellini 
> > 
> > During the discussions that led to the acceptance of Rule 2.1, we
> > decided on a few exceptions that were not properly recorded in
> > rules.rst. Add them now.
> > 
> > Signed-off-by: Stefano Stabellini 
> > Acked-by: Jan Beulich 
> > ---
> > Nicola, does this work with ECLAIR?
> > 
> > I am referring to the locations of the SAF-2-safe tag on top of
> > call_psci_system_off, BUG, etc.
> > 
> > Changes in v3:
> > - added SAF-2-safe to safe.json
> > - added a few SAF-2-safe examples
> > ---
> >   docs/misra/rules.rst| 13 -
> >   docs/misra/safe.json|  8 
> >   xen/arch/arm/psci.c |  1 +
> >   xen/arch/x86/shutdown.c |  1 +
> >   xen/include/xen/bug.h   |  2 ++
> >   5 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> > index 34916e266a..82de4c645d 100644
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -107,7 +107,18 @@ maintainers if you want to suggest a change.
> >  * - `Rule 2.1
> > `_
> >- Required
> >- A project shall not contain unreachable code
> > - -
> > + - The following are allowed:
> > + - Invariantly constant conditions, e.g. if(IS_ENABLED(CONFIG_HVM))
> > { S; }
> > + - Switch with a controlling value statically determined not to
> > +   match one or more case statements
> > + - Functions that are intended to be referenced only from
> > +   assembly code (e.g. 'do_trap_fiq')
> > + - Deliberate unreachability caused by certain macros/functions,
> > +   e.g. BUG, assert_failed, panic, etc. See safe.json.
> > + - asm-offsets.c, as they are not linked deliberately, because
> > +   they are used to generate definitions for asm modules
> > + - Declarations without initializer are safe, as they are not
> > +   executed
> >* - `Rule 2.6
> > `_
> >- Advisory
> > diff --git a/docs/misra/safe.json b/docs/misra/safe.json
> > index 39c5c056c7..fc96a99fd5 100644
> > --- a/docs/misra/safe.json
> > +++ b/docs/misra/safe.json
> > @@ -20,6 +20,14 @@
> >   },
> >   {
> >   "id": "SAF-2-safe",
> > +"analyser": {
> > +"eclair": "MC3R1.R2.1"
> > +},
> > +"name": "Rule 2.1: deliberate unreachability",
> > +"text": "Macro or function designed to be unreachable."
> > +},
> > +{
> > +"id": "SAF-3-safe",
> >   "analyser": {},
> >   "name": "Sentinel",
> >   "text": "Next ID to be used"
> > diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
> > index 695d2fa1f1..2a8527cacc 100644
> > --- a/xen/arch/arm/psci.c
> > +++ b/xen/arch/arm/psci.c
> > @@ -59,6 +59,7 @@ void call_psci_cpu_off(void)
> >   }
> >   }
> >   +/* SAF-2-safe */
> 
> I think any use of SAF-2-safe should be accompanied with an attribute...
> 
> >   void call_psci_system_off(void)
> 
> ... noreturn for function or ...
> 
> >   {
> >   if ( psci_ver > PSCI_VERSION(0, 1) )
> > diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
> > index 7619544d14..47e0f59024 100644
> > --- a/xen/arch/x86/shutdown.c
> > +++ b/xen/arch/x86/shutdown.c
> > @@ -118,6 +118,7 @@ static inline void kb_wait(void)
> >   break;
> >   }
> >   +/* SAF-2-safe */
> >   static void noreturn cf_check __machine_halt(void *unused)
> >   {
> >   local_irq_disable();
> > diff --git a/xen/include/xen/bug.h b/xen/include/xen/bug.h
> > index e8a4eea71a..d47c54f034 100644
> > --- a/xen/include/xen/bug.h
> > +++ b/xen/include/xen/bug.h
> > @@ -117,6 +117,7 @@ struct bug_frame {
> >   #endif
> > #ifndef BUG
> > +/* SAF-2-safe */
> >   #define BUG() do {  \
> >   BUG_FRAME(BUGFRAME_bug,  __LINE__, __FILE__, 0, NULL);  \
> >   unreachable();  \
> 
> ... unreachable for macros. But the /* SAF-2-safe */ feels a little bit
> redundant when a function is marked as 'noreturn'.
> 
> Is there any way to teach eclair about noreturn?

Actually I had the same thought while writing this patch. If we can
adopt unreachable and noreturn consistently maybe we don't need
SAF-2-safe. If the checker can support it.

Nicola, what do you think?

Re: [PATCH v3] docs/misra: add rule 2.1 exceptions

[Stefano Stabellini]

On Mon, 11 Sep 2023, Jan Beulich wrote:
> On 09.09.2023 01:03, Stefano Stabellini wrote:
> > From: Stefano Stabellini 
> > 
> > During the discussions that led to the acceptance of Rule 2.1, we
> > decided on a few exceptions that were not properly recorded in
> > rules.rst. Add them now.
> > 
> > Signed-off-by: Stefano Stabellini 
> > Acked-by: Jan Beulich 
> > ---
> > Nicola, does this work with ECLAIR?
> > 
> > I am referring to the locations of the SAF-2-safe tag on top of
> > call_psci_system_off, BUG, etc.
> > 
> > Changes in v3:
> > - added SAF-2-safe to safe.json
> > - added a few SAF-2-safe examples
> > ---
> >  docs/misra/rules.rst| 13 -
> >  docs/misra/safe.json|  8 
> >  xen/arch/arm/psci.c |  1 +
> >  xen/arch/x86/shutdown.c |  1 +
> >  xen/include/xen/bug.h   |  2 ++
> >  5 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> > index 34916e266a..82de4c645d 100644
> > --- a/docs/misra/rules.rst
> > +++ b/docs/misra/rules.rst
> > @@ -107,7 +107,18 @@ maintainers if you want to suggest a change.
> > * - `Rule 2.1 
> > `_
> >   - Required
> >   - A project shall not contain unreachable code
> > - -
> > + - The following are allowed:
> > + - Invariantly constant conditions, e.g. 
> > if(IS_ENABLED(CONFIG_HVM)) { S; }
> > + - Switch with a controlling value statically determined not to
> > +   match one or more case statements
> 
> I (continue to) consider this as too lax. I don't think we want to permit
> something like
> 
> void test(uint8_t val)
> {
> switch ( val )
> {
> case 0x100:
> ...
> 
> Imo like in the earlier bullet point I think this wants limiting to
> compile-time constant values, at least initially.

Yes good point

Re: [PATCH v4 1/5] xen/ppc: Implement atomic.h

[Shawn Anastasio]

On 9/11/23 6:33 AM, Jan Beulich wrote:
> On 09.09.2023 00:50, Shawn Anastasio wrote:
>> Implement atomic.h for PPC, based off of the original Xen 3.2
>> implementation. This implementation depends on some functions that are
>> not yet defined (notably __cmpxchg), so it can't yet be used.
>>
>> Signed-off-by: Shawn Anastasio 
> 
> Acked-by: Jan Beulich 
>

Thanks.

> Just one remaining question:
> 
>> +static always_inline void read_atomic_size(const volatile void *p, void 
>> *res,
>> +   unsigned int size)
>> +{
>> +ASSERT(IS_ALIGNED((vaddr_t)p, size));
>> +switch ( size )
>> +{
>> +case 1:
>> +*(uint8_t *)res = read_u8_atomic(p);
>> +break;
>> +case 2:
>> +*(uint16_t *)res = read_u16_atomic(p);
>> +break;
>> +case 4:
>> +*(uint32_t *)res = read_u32_atomic(p);
>> +break;
>> +case 8:
>> +*(uint64_t *)res = read_u64_atomic(p);
>> +break;
>> +default:
>> +__bad_atomic_read(p, res);
>> +break;
>> +}
>> +}
>> +
>> +static always_inline void write_atomic_size(volatile void *p, const void 
>> *val,
>> +unsigned int size)
>> +{
>> +ASSERT(IS_ALIGNED((vaddr_t)p, size));
>> +switch ( size )
>> +{
>> +case 1:
>> +write_u8_atomic(p, *(const uint8_t *)val);
>> +break;
>> +case 2:
>> +write_u16_atomic(p, *(const uint16_t *)val);
>> +break;
>> +case 4:
>> +write_u32_atomic(p, *(const uint32_t *)val);
>> +break;
>> +case 8:
>> +write_u64_atomic(p, *(const uint64_t *)val);
>> +break;
>> +default:
>> +__bad_atomic_size();
>> +break;
>> +}
>> +}
> 
> These two functions being as similar as is possible, ...
> 
>> +#define read_atomic(p)  
>>\
>> +({  
>>\
>> +union { 
>>\
>> +typeof(*(p)) val;   
>>\
>> +char c[sizeof(*(p))];   
>>\
>> +} x_;   
>>\
>> +read_atomic_size(p, x_.c, sizeof(*(p)));
>>\
>> +x_.val; 
>>\
>> +})
> 
> ... is there a reason you keep using a union here ...
> 
>> +#define write_atomic(p, x)  
>>\
>> +do  
>>\
>> +{   
>>\
>> +typeof(*(p)) x_ = (x);  
>>\
>> +write_atomic_size(p, &x_, sizeof(*(p)));
>>\
>> +} while ( 0 )
> 
> ... while you did away with its use here?
>

Yes. In the case of read_atomic the caller is allowed (expected, even)
to pass in const pointers for `p`, which wouldn't work if a simple
typeof(*(p)) declaration was used since it would inherit the constness
and thus wouldn't be passable to read_atomic_size.

In the case of write_atomic though, there is no need to support const
`p`, so no union is necessary.

> Jan

Thanks,
Shawn

Re: [PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Stefano Stabellini]

On Mon, 11 Sep 2023, Michal Orzel wrote:
> On 11/09/2023 16:48, Julien Grall wrote:
> > On 11/09/2023 15:01, Michal Orzel wrote:
> >> Hi Julien,
> >>
> >> On 11/09/2023 15:14, Julien Grall wrote:
> >>>
> >>>
> >>> Hi,
> >>>
> >>> On 11/09/2023 13:34, Michal Orzel wrote:
>  Host device tree nodes under /chosen with compatible string
>  "xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
>  meant to be exposed to hardware domain.
> >>>
> >>> So, how dom0 is meant to discover the static event channel, shared
> >>> memory it can use?
> >>
> >> For static shared memory:
> >> A node with this compatible is only meant for Xen since it contains 
> >> information like host-guest mapping.
> >> Xen creates a different node for guests under /reserved-memory.
> >> Linux binding:
> >> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt
> >> Xen node generation:
> >> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;hb=HEAD#l1407
> > 
> > Ah good point. I agree with this one.
> > 
> >>
> >> For static event channels:
> >> This is a bit weird so let me put it in a different way.
> >> 1) Xen does not create any node for static evtchn for domU.
> >> 2) The booting.txt states:
> >> There is no need to describe the static event channel info in the domU 
> >> device
> >> tree. Static event channels are only useful in fully static configurations,
> >> and in those configurations, the domU device tree dynamically generated by 
> >> Xen
> >> is not needed.
> >> 3) The "xen,evtchn" property specifies the local port as well as phandle 
> >> of domU node.
> >> dom0 does not have access to domU nodes, therefore exposing a property 
> >> with not existing phandle
> >> makes me think that:
> > 
> > You have a point for the phandle. Yet, this is the only way dom0 can
> > find the event channel today. As we exposed it, I don't think we can
> > remove it until we have a proper replacement.
> > 
> > We might get away if the feature is not supported it at all. But there
> > is no statement, so it is a little unclear whether the feature is meant
> > to be in technical preview.
> > 
> > In any case, I think the commit message deserve a bit more explanation
> > as hiding "xen,evtchn-v1"/"xen,domain-shared-memory-v1" is not
> > uncontroversial.
> > 
> >> a) point 2) applies to dom0 as well and we should hide this node or > b) 
> >> there is a missing property for both dom0 and domUs to tell them
> > about static local port in a proper way
> >>
> >> Exposing Xen specific evtchn node only to dom0 and not to domU with 
> >> required information happen to be found as first value
> >> in xen,evtchn is definitely not a proper solution.
> > 
> > My concern here is we exposed such information to dom0. So as I said
> > above, I don't think we can simply remove it as there is no other way to
> > find such information today.
> > 
> > It doesn't matter that it wasn't exposed to domU.
> > 
> >> Also, there is no Linux binding for it.
> > 
> > AFAIK the static event channel support was not added in Linux until very
> > recently. Also, I think the kernel doesn't directly use the static event
> > channel. So it is possible, this is just an overlook.
> 
> I've found this thread in which Stefano, Rahul and Bertrand agrees on not 
> exposing
> any dt property and the rationale behind:
> https://patchwork.kernel.org/project/xen-devel/patch/4836304496e6fbbea41348ed8cc9fcf6b0f3e893.1648049827.git.rahul.si...@arm.com/

yes it was done on purpose


> I would not call exposing node to dom0 as something done deliberately given 
> that it happens automatically by copying. So my understanding is
> that we did not want to expose any node and dom0 case was overlooked (since 
> done automatically).
> 
> Exposing half the interface (from system POV) in a way it should not be done 
> (phandle) is not great IMO.
> In any case, if you insist on keeping xen,evtchn, I can leave with it.
> 
> This feature is marked as tech preview with no Linux binding in place so I 
> would not be worried.

Yes I agree. I don't think we risk breaking anything. I would remove
that info from Dom0. Even if we wanted to expose it to Dom0, this is not
the right way to do it...

Re: [PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Stefano Stabellini]

On Mon, 11 Sep 2023, Julien Grall wrote:
> On 11/09/2023 13:34, Michal Orzel wrote:
> > Host device tree nodes under /chosen with compatible string
> > "xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
> > meant to be exposed to hardware domain.
> 
> So, how dom0 is meant to discover the static event channel, shared memory it
> can use?

As they are static, they don't necessarily need to be discovered: they
could be hard-coded. I realize we didn't commit the design doc for the
static event channel feature, but it had:
https://marc.info/?l=xen-devel&m=165245272905876

---
+There is no need to describe the static event channel info in the domU device
+tree. Static event channels are only useful in fully static configurations,
+and in those configurations the domU device tree dynamically generated by Xen
+is not needed.
---

But in any case, even if we wanted to generated/expose the static event
channels via Device Tree, the information on the host device tree is in
the "wrong" format. That's meant for Xen and has all the static event
channels, including the ones not relevant for Dom0. So I think that in
any case we should filter them out here (and regenerate as necessary).

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

[Stefano Stabellini]

On Mon, 11 Sep 2023, Andrew Cooper wrote:
> Physical CPU Hotplug does not pass the bar for being anything more than
> experimental.  It's absolutely not tech-preview level because the only
> demo it has had in an environment (admittedly virtual) which does
> implement the spec in a usable way demonstrates that it doesn't function.
> 
> The fact no-one has noticed until now shows that the feature isn't used,
> which comes back around full circle to the fact that Intel never made it
> work and never shipped it.

So we actually have agreement on how to move forward

---
SUPPORT: downgrade Physical CPU Hotplug to Experimental

The feature is not commonly used, and we don't have hardware to test it,
not in OSSTest, not in Gitlab, and not even ad-hoc manually by community
members.

Signed-off-by: Stefano Stabellini 

diff --git a/SUPPORT.md b/SUPPORT.md
index 3461f5cf2f..02e2f6eaa8 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -48,7 +48,7 @@ For the Cortex A77 r0p0 - r1p0, see Errata 1508412.
 
 ### Physical CPU Hotplug
 
-Status, x86: Supported
+Status, x86: Experimental
 
 ### Physical Memory
 

[qemu-mainline test] 182943: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-de

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

[osstest service owner]

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

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  f1546c98e06fec6c2d1e15ff8e80547d10365bc3
baseline version:
 xen  ab4f3e3897ab777f94d44d0450f9b0e844fad830

Last test of basis   182945  2023-09-11 16:00:27 Z0 days
Testing same since   182948  2023-09-11 19:03:12 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Javi Merino 
  Jinoh Kang 

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
   ab4f3e3897..f1546c98e0  f1546c98e06fec6c2d1e15ff8e80547d10365bc3 -> smoke

[seabios test] 182944: tolerable FAIL - PUSHED

[osstest service owner]

flight 182944 seabios real [real]
http://logs.test-lab.xenproject.org/osstest/logs/182944/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 182507
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 182507
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 182507
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 182507
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 182507
 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

version targeted for testing:
 seabios  1e1da7a963007d03a4e0e9a9e0ff17990bb1608d
baseline version:
 seabios  7a4003be25eae462f3c3d8aad96b57e34dc0c2b8

Last test of basis   182507  2023-08-24 13:12:45 Z   18 days
Testing same since   182944  2023-09-11 15:14:00 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm pass
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  pass
 test-amd64-amd64-qemuu-nested-amdfail
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-amd64-qemuu-freebsd11-amd64   pass
 test-amd64-amd64-qemuu-freebsd12-amd64   pass
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-qemuu-ws16-amd64 fail
 test-amd64-i386-xl-qemuu-ws16-amd64  fail
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrictpass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict pass
 test-amd64-amd64-qemuu-nested-intel  pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  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/seabios.git
   7a4003b..1e1da7a  1e1da7a963007d03a4e0e9a9e0ff17990bb1608d -> 
xen-tested-master

[linux-linus test] 182923: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. 
vs. 182531
 test-amd64-amd64-xl-xsm   8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-libvirt-raw  8 xen-boot fail REGR. vs. 182531
 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-freebsd12-amd64  8 xen-boot fail REGR. vs. 182531
 test-amd64-amd64-xl-pvhv2-intel  8 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-xl-qemut-debianhvm-amd64  8 xen-bootfail REGR. vs. 182531
 test-amd64-amd64-libvirt-xsm  8 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-dom0pvh-xl-amd 14 guest-start   fail REGR. vs. 182531
 test-amd64-amd64-xl-shadow8 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-pair12 xen-boot/src_hostfail REGR. vs. 182531
 test-amd64-amd64-pair13 xen-boot/dst_hostfail REGR. vs. 182531
 test-amd64-amd64-qemuu-nested-intel  8 xen-boot  fail 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-examine-uefi  8 reboot  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-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 182531
 test-amd64-amd64-xl   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-pygrub   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-debianhvm-i386-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-xl-credit2   8 xen-boot 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-xl-qemuu-debianhvm-i386-xsm  8 xen-boot fail 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

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 16 saverestore-support-checkfail  like 182531
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail 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-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-x

[PATCH] xen/efi: refactor deprecated strncpy

[Justin Stitt]

`strncpy` is deprecated for use on NUL-terminated destination strings [1].

`efi_loader_signature` has space for 4 bytes. We are copying "Xen" (3 bytes)
plus a NUL-byte which makes 4 total bytes. With that being said, there is
currently not a bug with the current `strncpy()` implementation in terms of
buffer overreads but we should favor a more robust string interface
either way.

A suitable replacement is `strscpy` [2] due to the fact that it guarantees
NUL-termination on the destination buffer while being functionally the
same in this case.

Link: 
www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings[1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-harden...@vger.kernel.org
Cc: Kees Cook 
Signed-off-by: Justin Stitt 
---
Note: build-tested
---
 arch/x86/xen/efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 863d0d6b3edc..7250d0e0e1a9 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -138,7 +138,7 @@ void __init xen_efi_init(struct boot_params *boot_params)
if (efi_systab_xen == NULL)
return;
 
-   strncpy((char *)&boot_params->efi_info.efi_loader_signature, "Xen",
+   strscpy((char *)&boot_params->efi_info.efi_loader_signature, "Xen",
sizeof(boot_params->efi_info.efi_loader_signature));
boot_params->efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params->efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >> 
32);

---
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
change-id: 20230911-strncpy-arch-x86-xen-efi-c-14292f5a79ee

Best regards,
--
Justin Stitt 

Re: [PATCH] include: make domain_page.h's stubs properly use type-unsafe MFN <-> virt helpers

[Shawn Anastasio]

On 9/11/23 1:38 AM, Jan Beulich wrote:
> The first of the commits referenced below didn't go far enough, and the
> 2nd of them, while trying to close (some of) the gap, wrongly kept using
> the potentially type-safe variant. This is getting in the way of new
> ports preferably not having any type-unsafe private code (and in
> particular not having a need for any overrides in newly introduced
> files).
> 
> Fixes: 41c48004d1d8 ("xen/mm: Use __virt_to_mfn in map_domain_page instead of 
> virt_to_mfn")
> Fixes: f46b6197344f ("xen: Convert page_to_mfn and mfn_to_page to use 
> typesafe MFN")
> Reported-by: Shawn Anastasio 
> Signed-off-by: Jan Beulich 
> 
> --- a/xen/include/xen/domain_page.h
> +++ b/xen/include/xen/domain_page.h
> @@ -56,11 +56,11 @@ static inline void *__map_domain_page_gl
>  #define map_domain_page(mfn)__mfn_to_virt(mfn_x(mfn))
>  #define __map_domain_page(pg)   page_to_virt(pg)
>  #define unmap_domain_page(va)   ((void)(va))
> -#define domain_page_map_to_mfn(va)  _mfn(virt_to_mfn((unsigned 
> long)(va)))
> +#define domain_page_map_to_mfn(va)  _mfn(__virt_to_mfn((unsigned 
> long)(va)))
>  
>  static inline void *map_domain_page_global(mfn_t mfn)
>  {
> -return mfn_to_virt(mfn_x(mfn));
> +return __mfn_to_virt(mfn_x(mfn));
>  }
>  
>  static inline void *__map_domain_page_global(const struct page_info *pg)

Reviewed-by: Shawn Anastasio 

Thanks,
Shawn

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

[osstest service owner]

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

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  ab4f3e3897ab777f94d44d0450f9b0e844fad830
baseline version:
 xen  35949b8c58cb6c835da1fdd8843ed51727e654c0

Last test of basis   182930  2023-09-11 08:00:26 Z0 days
Testing same since   182945  2023-09-11 16:00:27 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Anthony PERARD 
  Federico Serafini 
  George Dunlap 
  Jan Beulich 
  Javi Merino 

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
   35949b8c58..ab4f3e3897  ab4f3e3897ab777f94d44d0450f9b0e844fad830 -> smoke

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

[Andrew Cooper]

On 06/09/2023 9:49 pm, 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.

Count the number of x86 maintainer tags on the patch, and see that they
were given after discussion, not before.

You're outvoted 2:1 by Xen x86 maintainers alone, and more than 2:1 if
you include the x86 maintainers from other projects who participated in
the discussion.


I'm not willing to let Xen keep on malfunctioning on millions of systems
just because you think an unfinished spec is more correct that practical
reality which invalidates it.

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

SUPPORT.md does look bogus and wants correcting, but it is laughable to
claim that this ever worked, let alone that it's supported.

Intel got half way through specifying ACPI CPU Hotplug, and abandoned it
as a technology because they couldn't get it to work.  Hence the feature
has never been tested.

Furthermore, cursory testing that Thomas did for the Linux topology work
demonstrates that it is broken anyway for reasons unrelated to ACPI parsing.

Even furthermore, it's an area of the Xen / dom0 boundary which is
fundamentally broken for non-PV cases, and undocumented for the PV case,
hence why it's broken in Linux.


Physical CPU Hotplug does not pass the bar for being anything more than
experimental.  It's absolutely not tech-preview level because the only
demo it has had in an environment (admittedly virtual) which does
implement the spec in a usable way demonstrates that it doesn't function.

The fact no-one has noticed until now shows that the feature isn't used,
which comes back around full circle to the fact that Intel never made it
work and never shipped it.

~Andrew

Re: [PATCH v6 3/4] xen/vpci: header: status register handler

[Stewart Hildebrand]

On 9/11/23 07:10, Jan Beulich wrote:
> On 09.09.2023 04:16, Stewart Hildebrand wrote:
>> @@ -544,6 +545,18 @@ static int cf_check init_bars(struct pci_dev *pdev)
>>  if ( rc )
>>  return rc;
>>
>> +/*
>> + * If mask_cap_list is true, PCI_STATUS_CAP_LIST will be set in both
>> + * rsvdz_mask and ro_mask, and thus will effectively behave as rsvdp
>> + * (reserved/RAZ and preserved on write).
>> + */
>> +rc = vpci_add_register_mask(pdev->vpci, vpci_hw_read16, vpci_hw_write16,
>> +PCI_STATUS, 2, header, 
>> PCI_STATUS_RSVDZ_MASK |
>> +(mask_cap_list ? PCI_STATUS_CAP_LIST : 0),
>> +PCI_STATUS_RO_MASK, PCI_STATUS_RW1C_MASK);
> 
> While not formally disallowed by ./CODING_STYLE, I think this kind of argument
> wrapping is bad practice. If an argument is too long for the current line, it
> should start on a fresh one. Otherwise at the very least I think we'd want the
> continuation to stand out, by parenthesizing the entire argument, thus leading
> to one deeper indentation on the continuing line(s). (This may even be useful
> to do when starting on a fresh line.)

I will change it to this:
/*
 * Utilize rsvdz_mask to hide PCI_STATUS_CAP_LIST from the guest for now. If
 * support for rsvdp (reserved & preserved) is added in the future, the
 * rsvdp mask should be used instead.
 */
rc = vpci_add_register_mask(pdev->vpci, vpci_hw_read16, vpci_hw_write16,
PCI_STATUS, 2, NULL,
PCI_STATUS_RSVDZ_MASK |
(mask_cap_list ? PCI_STATUS_CAP_LIST : 0),
PCI_STATUS_RO_MASK &
~(mask_cap_list ? PCI_STATUS_CAP_LIST : 0),
PCI_STATUS_RW1C_MASK);

>> @@ -155,7 +165,8 @@ int vpci_add_register(struct vpci *vpci, vpci_read_t 
>> *read_handler,
>>  /* Some sanity checks. */
>>  if ( (size != 1 && size != 2 && size != 4) ||
>>   offset >= PCI_CFG_SPACE_EXP_SIZE || (offset & (size - 1)) ||
>> - (!read_handler && !write_handler) )
>> + (!read_handler && !write_handler) || (ro_mask & rw1c_mask) ||
>> + (rsvdz_mask & rw1c_mask) )
>>  return -EINVAL;
> 
> What about rsvdz_mask and ro_mask? They better wouldn't overlap either
> (requiring an adjustment to the code you add to init_bars()).

I will add a case for (rsvdz_mask & ro_mask). See above for adjustment in 
init_bars().

>> @@ -407,26 +439,25 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, 
>> unsigned int size)
>>
>>  /*
>>   * Perform a maybe partial write to a register.
>> - *
>> - * Note that this will only work for simple registers, if Xen needs to
>> - * trap accesses to rw1c registers (like the status PCI header register)
>> - * the logic in vpci_write will have to be expanded in order to correctly
>> - * deal with them.
>>   */
>>  static void vpci_write_helper(const struct pci_dev *pdev,
>>const struct vpci_register *r, unsigned int 
>> size,
>>unsigned int offset, uint32_t data)
>>  {
>> +uint32_t val = 0;
>> +
>>  ASSERT(size <= r->size);
>>
>> -if ( size != r->size )
>> +if ( (size != r->size) || r->ro_mask )
>>  {
>> -uint32_t val;
>> -
>>  val = r->read(pdev, r->offset, r->private);
>> +val &= ~r->rw1c_mask;
>>  data = merge_result(val, data, size, offset);
>>  }
>>
>> +data &= ~r->rsvdz_mask & ~r->ro_mask;
> 
> ~(r->rsvdz_mask | r->ro_mask) ? But maybe that's indeed just me ...

I will make this change.

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

[Simon Gaiser]

Jan Beulich:
> 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'll give it until the end of next week for either of the alternatives to be
> carried out. If nothing appears by then, I'll assume I may rightfully revert.
> (This timeline also allows putting this topic on the Community Call agenda,
> should anyone think this is necessary.)

To avoid misunderstandings, since you mentioned the above in your
response to the related patch I submitted today [3]:

My understanding is that your main concern is that those entries with
invalid APIC IDs shouldn't be ignored, since some firmwares later update
them with valid IDs (on hotplug). This fully conflicts with the
intention of the patch. Resolving the disagreement that followed is
outside of my control, as being only the submitter of the patch.

You also commented about not logging the ignored entries. Until it's
clear if the change in general is accepted in the end, I considered it
pointless to address that detail. If you disagree and want a follow up
for that, just let me know.

Simon

[3]: 
https://lore.kernel.org/xen-devel/20230911101238.18005-1-si...@invisiblethingslab.com/


OpenPGP_signature
Description: OpenPGP digital signature

Re: [XEN PATCH] x86/ACPI: Ignore entries marked as unusable when parsing MADT

[Simon Gaiser]

Jan Beulich:
> On 11.09.2023 12:12, Simon Gaiser wrote:
>> Up to version 6.2 Errata B [2] the ACPI spec only defines
>> ACPI_MADT_ENABLE as:
>>
>> If zero, this processor is unusable, and the operating system
>> support will not attempt to use it.
>>
>> The bit that later will be ACPI_MADT_ONLINE_CAPABLE is reserved with
>> "Must be zero".
>>
>> Version 6.3 [3] then adds ACPI_MADT_ONLINE_CAPABLE and changes the
>> meaning of ACPI_MADT_ENABLE:
>>
>> Enabled
>> If this bit is set the processor is ready for use. If this bit
>> is clear and the Online Capable bit is set, system hardware
>> supports enabling this processor during OS runtime. If this bit
>> is clear and the Online Capable bit is also clear, this
>> processor is unusable, and OSPM shall ignore the contents of the
>> Processor Local APIC Structure.
>>
>> Online Capbable
>> The information conveyed by this bit depends on the value of the
>> Enabled bit. If the Enabled bit is set, this bit is reserved and
>> must be zero. Otherwise, if this this bit is set, system
>> hardware supports enabling this processor during OS runtime.
>>
>> So with conforming firmwares it should be safe to simply ignore the
>> entry if !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE
>>
>> As a precaution against buggy firmwares this change, like Linux [4],
>> ignores ACPI_MADT_ONLINE_CAPABLE completely if MADT revision < 5. Note
>> that the MADT revision was already increased to 5 with spec version 6.2
>> Errata A [1], so before introducing the online capable flag. But it
>> wasn't changed for the new flag, so this is the best we can do here.
>>
>> For previous discussion see thread [5].
>>
>> Link: 
>> http://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf # 
>> [1]
>> Link: 
>> https://uefi.org/sites/default/files/resources/ACPI_6_2_B_final_Jan30.pdf # 
>> [2]
>> Link: https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf # [3]
>> Link: 
>> https://git.kernel.org/torvalds/c/e2869bd7af608c343988429ceb1c2fe99644a01f # 
>> [4]
>> Link: 
>> https://lore.kernel.org/xen-devel/80bae614-052e-0f90-cf13-0e5e4ed1a...@invisiblethingslab.com/
>>  # [5]
>> Signed-off-by: Simon Gaiser 
> 
> Just to avoid misunderstandings: This change addresses a comment from
> Andrew. I does not attempt to address my feedback on the earlier change,
> which - as indicated - I intend to revert (timeline extended until mid
> of the week), unless by then my earlier comments are addressed or the
> suggested possible alternative is carried out.
> 
> I think it goes without saying that this patch can't sensibly go in
> until the earlier one was properly settled upon. In particular, in case
> of reverting aiui this patch won't even apply anymore.

It textually conflicts with the other patch [6], and obviously was
triggered by that discussion, but addresses a slightly different aspect.
The textual conflict is trivial to resolve. I wasn't sure if it also
conflicts with the concern you raised there, see below.

The other patch is about ignoring entries with invalid APIC IDs. As the
discussion there showed the spec isn't very clear on that and there are
different opinions how they should be handled. Regarding the flags the
spec is much more concrete although given the response above I assume
you also interpret "is unusable" of the old version of the
ACPI_MADT_ENABLE flag as such that Xen should still allocate resources
for those processors?

If I understood your main concern with the other patch correctly you
have seen firmwares that later update those invalid APIC IDs with valid
ones. Do those firmwares make use of the online capable flag? (Given
above response probably not)

If not, then it indeed doesn't address your concern, and I see no way,
at least by parsing MADT, how to distinguish those cases from firmwares
that have dummy entries (the motivation for these patches).

Simon

[6]: 
https://lore.kernel.org/xen-devel/7f158a54548456daba9f2e105d099d2e5e2c2f38.1691399031.git.si...@invisiblethingslab.com/


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] coverage: update gcov info for newer versions of gcc

[Alex Bennée]

Jan Beulich  writes:

> On 11.09.2023 17:26, Alex Bennée wrote:
>> Jan Beulich  writes:
>>> On 02.09.2023 17:11, Javi Merino wrote:
 --- a/xen/common/coverage/Makefile
 +++ b/xen/common/coverage/Makefile
 @@ -5,7 +5,9 @@ obj-y += $(call cc-ifversion,-lt,0407, \
gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
>> 
>> This isn't even supported, see below:
>> 
gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
 -  gcc_5.o, gcc_7.o
 +  gcc_5.o, $(call cc-ifversion,-lt,1000, \
 +  gcc_7.o,  $(call cc-ifversion,-lt,1200, \
 +  gcc_10.o, gcc_12.o))
>>>
>>> This is getting unwieldy, so I think we ought to try to limit the number
>>> of different files we have. Already gcc_4_9.c and gcc_7.c specify the
>>> same GCOV_COUNTERS and differ only in the version checks (which could be
>>> combined). Therefore ...
>> 
>> You may want to consider your policy on supported GCC versions. I see
>> you still support:
>> 
>> * C compiler and linker:
>>   - For x86:
>> - GCC 4.1.2_20070115 or later
>> - GNU Binutils 2.16.91.0.5 or later
>> or
>> - Clang/LLVM 3.5 or later
>>   - For ARM 32-bit:
>> - GCC 4.9 or later
>> - GNU Binutils 2.24 or later
>>   - For ARM 64-bit:
>> - GCC 5.1 or later
>> - GNU Binutils 2.24 or later
>> 
>> While also having some commented out warnings because the base GCC is so old:
>> 
>>   CFLAGS   += -Wmissing-prototypes
>>   # (gcc 4.3x and later)   -Wconversion -Wno-sign-conversion
>> 
>> For reference QEMU's minimum GCC is 7.4
>> 
>>   if compiler.get_id() == 'gcc' and 
>> compiler.version().version_compare('>=7.4')
>> 
>> and while our support cycle is based off distro LTS releases I have to
>> wonder do you actually need to support users building the
>> latest/greatest version of the hypervisor on ancient user-spaces with
>> old compilers?
>> 
>> I think the oldest distro you have in your CI is jessie (still hanging
>> on with extended LTS support) and that uses GCC 4.9.
>> 
>> I see there are various scripts to gather support status into the
>> documentation but I couldn't find a general statement on the projects
>> overall approach to supported versions of components.
>
> That's the problem - we've been intending to raise the baseline for quite
> some time, but figuring at least a rule of thumb by which to go both now
> and in the future turns out problematic.

FWIW QEMU states:

  The project aims to support the most recent major version at all times
  for up to five years after its initial release. Support for the
  previous major version will be dropped 2 years after the new major
  version is released or when the vendor itself drops support, whichever
  comes first. In this context, third-party efforts to extend the
  lifetime of a distro are not considered, even when they are endorsed
  by the vendor (eg. Debian LTS); the same is true of repositories that
  contain packages backported from later releases (e.g. Debian
  backports). Within each major release, only the most recent minor
  release is considered.

  For the purposes of identifying supported software versions available
  on Linux, the project will look at CentOS, Debian, Fedora, openSUSE,
  RHEL, SLES and Ubuntu LTS. Other distros will be assumed to ship
  similar software versions.

We also make some specific statements about python runtimes and optional
dependencies. Again this is perhaps easier for us as a user-space
program but it's worthwhile writing something up as a policy even if you
find you need to tweak it later. We also document our feature
deprecation policy and what has been removed.

If you want to crib anything from QEMU as an initial stab at it please
see:

  https://qemu.readthedocs.io/en/master/about/index.html

(this is docs/about/ in our source tree).

>
> Jan


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [XEN PATCH v2] tools/xentrace/xentrace_format: Add python 3 compatibility

[Andrew Cooper]

On 11/09/2023 5:07 pm, Javi Merino wrote:
> Resolves: xen-project/xen#155
>
> Signed-off-by: Javi Merino 

Acked-by: Andrew Cooper 

[XEN PATCH v2 5/5] README: update to remove old note about the build system's python expectation

[Javi Merino]

The configure script tests for the availability of python3, python and
python2 in that order and sets PYTHON to the first one found in path.
You don't need to have a symlink to python.

Signed-off-by: Javi Merino 
---
 README | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/README b/README
index f8cc426f78..6212a3a9c5 100644
--- a/README
+++ b/README
@@ -184,13 +184,6 @@ Various tools, such as pygrub, have the following runtime 
dependencies:
   URL:https://www.python.org/
   Debian: python
 
-Note that the build system expects `python` to be available. If your system
-only has `python2` or `python3` but not `python` (as in Linux From Scratch),
-you will need to create a symlink for it, or specify PYTHON= when invoking
-make, like (note the position of PYTHON= matters):
-
-# make PYTHON=/usr/bin/python3
-
 Intel(R) Trusted Execution Technology Support
 =
 
-- 
2.41.0

[XEN PATCH v2 3/5] tools: don't use distutils in configure nor Makefile

[Javi Merino]

From: Marek Marczykowski-Górecki 

Python distutils is deprecated and is going to be removed in Python
3.12. The distutils.sysconfig is available as sysconfig module in
stdlib since Python 3.2, so use that directly.

Signed-off-by: Marek Marczykowski-Górecki 
---
 m4/python_devel.m4   | 28 ++--
 tools/libs/stat/Makefile |  4 ++--
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/m4/python_devel.m4 b/m4/python_devel.m4
index bbf1e0354b..bb60857b03 100644
--- a/m4/python_devel.m4
+++ b/m4/python_devel.m4
@@ -5,21 +5,21 @@ ac_previous_libs=$LIBS
 AC_PATH_PROG([pyconfig], [$PYTHON-config], [no])
 AS_IF([test x"$pyconfig" = x"no"], [
 dnl For those that don't have python-config
-CPPFLAGS="$CFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print("-I" + distutils.sysconfig.get_config_var("INCLUDEPY"))'`"
-CPPFLAGS="$CPPFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("CFLAGS"))'`"
-LDFLAGS="$LDFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print("-L" + distutils.sysconfig.get_python_lib(plat_specific=1,\
+CPPFLAGS="$CFLAGS `$PYTHON -c 'import sysconfig; \
+print("-I" + sysconfig.get_config_var("INCLUDEPY"))'`"
+CPPFLAGS="$CPPFLAGS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("CFLAGS"))'`"
+LDFLAGS="$LDFLAGS `$PYTHON -c 'import sysconfig; \
+print("-L" + sysconfig.get_python_lib(plat_specific=1,\
 standard_lib=1) + "/config")'`"
-LDFLAGS="$LDFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("LINKFORSHARED"))'`"
-LDFLAGS="$LDFLAGS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("LDFLAGS"))'`"
-LIBS="$LIBS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("LIBS"))'`"
-LIBS="$LIBS `$PYTHON -c 'import distutils.sysconfig; \
-print(distutils.sysconfig.get_config_var("SYSLIBS"))'`"
+LDFLAGS="$LDFLAGS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("LINKFORSHARED"))'`"
+LDFLAGS="$LDFLAGS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("LDFLAGS"))'`"
+LIBS="$LIBS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("LIBS"))'`"
+LIBS="$LIBS `$PYTHON -c 'import sysconfig; \
+print(sysconfig.get_config_var("SYSLIBS"))'`"
 ], [
 dnl If python-config is found use it
 CPPFLAGS="$CFLAGS `$PYTHON-config --cflags`"
diff --git a/tools/libs/stat/Makefile b/tools/libs/stat/Makefile
index ee5c42bf7b..a968eaff48 100644
--- a/tools/libs/stat/Makefile
+++ b/tools/libs/stat/Makefile
@@ -73,8 +73,8 @@ $(PYLIB): $(PYSRC)
 python-bindings: $(PYLIB) $(PYMOD)
 
 pythonlibdir = $(shell $(PYTHON) -c \
-  'import distutils.sysconfig as cfg; \
-   print(cfg.get_python_lib(False, False, prefix="$(prefix)"))')
+  'import sysconfig; \
+   print(sysconfig.get_python_lib("platlib", vars={"platbase": 
"$(prefix)"}))')
 
 .PHONY: install-python-bindings
 install-python-bindings: $(PYLIB) $(PYMOD)
-- 
2.41.0

[XEN PATCH v2 2/5] tools: convert setup.py to use setuptools

[Javi Merino]

From: Marek Marczykowski-Górecki 

Python distutils is deprecated and is going to be removed in Python
3.12. Migrate to setuptools.
Setuptools in Python 3.11 complains:
SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip 
and other standards-based tools.
Keep using setup.py anyway to build C extension.

Signed-off-by: Marek Marczykowski-Górecki 
---
 tools/pygrub/setup.py | 8 ++--
 tools/python/setup.py | 8 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/pygrub/setup.py b/tools/pygrub/setup.py
index 502aa4df2d..f9e8feb2e6 100644
--- a/tools/pygrub/setup.py
+++ b/tools/pygrub/setup.py
@@ -1,5 +1,9 @@
-from distutils.core import setup, Extension
-from distutils.ccompiler import new_compiler
+try:
+from setuptools import setup, Extension
+except ImportError:
+# distutils was removed in Python 3.12.  If this import fails,
+# install setuptools.
+from distutils.core import setup, Extension
 import os
 import sys
 
diff --git a/tools/python/setup.py b/tools/python/setup.py
index 721a3141d7..e8111bd098 100644
--- a/tools/python/setup.py
+++ b/tools/python/setup.py
@@ -1,5 +1,9 @@
-
-from distutils.core import setup, Extension
+try:
+from setuptools import setup, Extension
+except ImportError:
+# distutils was removed in Python 3.12.  If this import fails,
+# install setuptools.
+from distutils.core import setup, Extension
 import os, sys
 
 XEN_ROOT = "../.."
-- 
2.41.0

[XEN PATCH v2 1/5] automation: add python3's setuptools to containers

[Javi Merino]

In preparation of dropping python distutils and moving to setuptools,
add the python3 setuptools module to the containers that need it.

The centos7 container was building using python2.  Change it to build
python scripts using python3.

Debian Stretch is no longer debian oldstable, so move to the archive
repositories.

Signed-off-by: Javi Merino 
---
 automation/build/alpine/3.18.dockerfile|  1 +
 automation/build/archlinux/current.dockerfile  |  1 +
 automation/build/centos/7.dockerfile   |  3 ++-
 automation/build/debian/bookworm.dockerfile|  1 +
 automation/build/debian/stretch.dockerfile | 11 ++-
 automation/build/suse/opensuse-leap.dockerfile |  1 +
 automation/build/ubuntu/bionic.dockerfile  |  1 +
 automation/build/ubuntu/focal.dockerfile   |  1 +
 automation/build/ubuntu/trusty.dockerfile  |  1 +
 automation/build/ubuntu/xenial.dockerfile  |  1 +
 10 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/automation/build/alpine/3.18.dockerfile 
b/automation/build/alpine/3.18.dockerfile
index ca5756955e..5d2a69a060 100644
--- a/automation/build/alpine/3.18.dockerfile
+++ b/automation/build/alpine/3.18.dockerfile
@@ -34,6 +34,7 @@ RUN apk --no-cache add \
   ocaml-findlib \
   patch  \
   python3-dev \
+  py3-setuptools \
   texinfo \
   util-linux-dev \
   xz-dev \
diff --git a/automation/build/archlinux/current.dockerfile 
b/automation/build/archlinux/current.dockerfile
index 13fb472d9e..47e79637a4 100644
--- a/automation/build/archlinux/current.dockerfile
+++ b/automation/build/archlinux/current.dockerfile
@@ -34,6 +34,7 @@ RUN pacman -S --refresh --sysupgrade --noconfirm 
--noprogressbar --needed \
 pixman \
 pkgconfig \
 python \
+python-setuptools \
 sdl \
 sdl2 \
 spice \
diff --git a/automation/build/centos/7.dockerfile 
b/automation/build/centos/7.dockerfile
index 69dcefb2f0..f14f70014a 100644
--- a/automation/build/centos/7.dockerfile
+++ b/automation/build/centos/7.dockerfile
@@ -22,7 +22,8 @@ RUN yum -y update \
 ncurses-devel \
 zlib-devel \
 openssl-devel \
-python-devel \
+python3-devel \
+python3-setuptools \
 libuuid-devel \
 pkgconfig \
 flex \
diff --git a/automation/build/debian/bookworm.dockerfile 
b/automation/build/debian/bookworm.dockerfile
index f8415425e6..ae008c8d46 100644
--- a/automation/build/debian/bookworm.dockerfile
+++ b/automation/build/debian/bookworm.dockerfile
@@ -16,6 +16,7 @@ RUN apt-get update && \
 libncurses5-dev \
 libssl-dev \
 python3-dev \
+python3-setuptools \
 xorg-dev \
 uuid-dev \
 libyajl-dev \
diff --git a/automation/build/debian/stretch.dockerfile 
b/automation/build/debian/stretch.dockerfile
index 1af6c691f8..9f4b91a9e3 100644
--- a/automation/build/debian/stretch.dockerfile
+++ b/automation/build/debian/stretch.dockerfile
@@ -1,4 +1,4 @@
-FROM debian:stretch
+FROM debian/eol:stretch
 LABEL maintainer.name="The Xen Project" \
   maintainer.email="xen-devel@lists.xenproject.org"
 
@@ -8,6 +8,14 @@ ENV USER root
 RUN mkdir /build
 WORKDIR /build
 
+# Debian stretch is now archived.  Fetch packages from
+# archive.debian.org and skip the stretch-updates repository, as it is
+# not longer valid.
+RUN sed -i \
+-e 's/deb.debian.org/archive.debian.org/' \
+-e '/stretch-updates/d' \
+/etc/apt/sources.list
+
 # build depends
 RUN apt-get update && \
 apt-get --quiet --yes install \
@@ -17,6 +25,7 @@ RUN apt-get update && \
 libssl-dev \
 python-dev \
 python3-dev \
+python3-setuptools \
 xorg-dev \
 uuid-dev \
 libyajl-dev \
diff --git a/automation/build/suse/opensuse-leap.dockerfile 
b/automation/build/suse/opensuse-leap.dockerfile
index 98ee42970d..7010b71aca 100644
--- a/automation/build/suse/opensuse-leap.dockerfile
+++ b/automation/build/suse/opensuse-leap.dockerfile
@@ -60,6 +60,7 @@ RUN zypper install -y --no-recommends \
 'pkgconfig(sdl)' \
 'pkgconfig(sdl2)' \
 python3-devel \
+python3-setuptools \
 systemd-devel \
 tar \
 transfig \
diff --git a/automation/build/ubuntu/bionic.dockerfile 
b/automation/build/ubuntu/bionic.dockerfile
index e15f54431e..9cceb11ef9 100644
--- a/automation/build/ubuntu/bionic.dockerfile
+++ b/automation/build/ubuntu/bionic.dockerfile
@@ -17,6 +17,7 @@ RUN apt-get update && \
 libssl-dev \
 python-dev \
 python3-dev \
+python3-setuptools \
 xorg-dev \
 uuid-dev \
 libyajl-dev \
diff --git a/automation/build/ubuntu/focal.dockerfile 
b/automation/build/ubuntu/focal.dockerfile
index 9c851c0620..e3d11194ee 100644
--- a/automation/build/ubuntu/focal.dockerfile
+++ b/automation/build/ubuntu/focal.dockerfile
@@ -16,6 +16,7 @@ RUN apt-get update && \
 libncurses5-dev \
 libssl-dev \

[XEN PATCH v2 0/5] python: Use setuptools instead of the deprecated distutils

[Javi Merino]

This series picks up Marek's v1 from
https://lore.kernel.org/xen-devel/20230316171634.320626-1-marma...@invisiblethingslab.com/
Changes since v1:
  - Update all containers to have setuptools, as python 3.12 depecrates 
distutils in favour of setuptools
  - Keep python2's support by falling back to distutils if setuptools is not 
installed
  - Drop the commit about raising the baseline requirement for python, as we 
keep supporting python2

Javi Merino (2):
  automation: add python3's setuptools to containers
  README: update to remove old note about the build system's python
expectation

Marek Marczykowski-Górecki (3):
  tools: convert setup.py to use setuptools
  tools: don't use distutils in configure nor Makefile
  tools: regenerate configure

 README|7 -
 automation/build/alpine/3.18.dockerfile   |1 +
 automation/build/archlinux/current.dockerfile |1 +
 automation/build/centos/7.dockerfile  |3 +-
 automation/build/debian/bookworm.dockerfile   |1 +
 automation/build/debian/stretch.dockerfile|   11 +-
 .../build/suse/opensuse-leap.dockerfile   |1 +
 automation/build/ubuntu/bionic.dockerfile |1 +
 automation/build/ubuntu/focal.dockerfile  |1 +
 automation/build/ubuntu/trusty.dockerfile |1 +
 automation/build/ubuntu/xenial.dockerfile |1 +
 m4/python_devel.m4|   28 +-
 tools/configure   | 6551 +
 tools/libs/stat/Makefile  |4 +-
 tools/pygrub/setup.py |8 +-
 tools/python/setup.py |8 +-
 16 files changed, 3678 insertions(+), 2950 deletions(-)

-- 
2.41.0

Re: [PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to head.S

[Ayan Kumar Halder]

Hi Julien,

On 11/09/2023 15:51, Julien Grall wrote:



On 31/08/2023 10:16, Henry Wang wrote:

On Aug 31, 2023, at 17:12, Ayan Kumar Halder  wrote:

Hi Henry,

On 28/08/2023 02:32, Henry Wang wrote:


diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 33b038e7e0..39218cf15f 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -83,6 +83,25 @@
  isb
  .endm

+/*
+ * Enforce Xen page-tables do not contain mapping that are both
+ * Writable and eXecutables.
+ *
+ * This should be called on each secondary CPU.
+ */
+.macro pt_enforce_wxn tmp
+    mrc   CP32(\tmp, HSCTLR)
+    orr   \tmp, \tmp, #SCTLR_Axx_ELx_WXN
+    dsb
+    mcr   CP32(\tmp, HSCTLR)
+    /*
+ * The TLBs may cache SCTLR_EL2.WXN. So ensure it is 
synchronized

+ * before flushing the TLBs.
+ */
+    isb
+    flush_xen_tlb_local \tmp
+.endm
+
  /*
   * Common register usage in this file:
   *   r0  -
@@ -254,6 +273,7 @@ secondary_switched:
  /* Use a virtual address to access the UART. */
  mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
  #endif
+    pt_enforce_wxn


Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the 
MMU related functionality are bundled together?


Also AFAIU, mov_w has not effect on pt_enforce_wxn().

So that I can create a function "enable_secondary_cpu_mm()" - 
similar to one you introduced for arm64


Sure, I am good with this if other maintainers do not have any 
objections.


I am objecting. It would be quite handy to print a message on the 
console to indicate that we are enforce WXN. So we want to update UART 
address (stored in r11) before hand.


You mean you want to add this snippet in the current patch

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 39218cf15f..282b89a96e 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -273,6 +273,7 @@ secondary_switched:
 /* Use a virtual address to access the UART. */
 mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
 #endif
+    PRINT("- Enforce WXN -\r\n")
 pt_enforce_wxn r0
 PRINT("- Ready -\r\n")
 /* Jump to C world */

- Ayan



Cheers,

Re: [PATCH] build: restrict gcc11 workaround to versions earlier than 11.3.0

[Julien Grall]

Hi Jan,

On 22/08/2023 15:40, Jan Beulich wrote:

The fix for this issue was backported to 11.3, so let's not unduly
engage the workaround.

Signed-off-by: Jan Beulich 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

[Javi Merino]

Resolves: xen-project/xen#155

Signed-off-by: Javi Merino 
---
Changes since v1:
   - Remove unneeded import from past.utils to avoid depending on python3's 
future module.

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

 tools/xentrace/xentrace_format | 60 +++---
 1 file changed, 34 insertions(+), 26 deletions(-)

diff --git a/tools/xentrace/xentrace_format b/tools/xentrace/xentrace_format
index 5ff85ae2e8..83a1b04d0c 100644
--- a/tools/xentrace/xentrace_format
+++ b/tools/xentrace/xentrace_format
@@ -4,11 +4,14 @@
 
 # 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
 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 +32,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 +52,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 +86,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 +120,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 +148,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 +219,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[

Re: [PATCH] coverage: update gcov info for newer versions of gcc

[Jan Beulich]

On 11.09.2023 17:26, Alex Bennée wrote:
> Jan Beulich  writes:
>> On 02.09.2023 17:11, Javi Merino wrote:
>>> --- a/xen/common/coverage/Makefile
>>> +++ b/xen/common/coverage/Makefile
>>> @@ -5,7 +5,9 @@ obj-y += $(call cc-ifversion,-lt,0407, \
>>> gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
> 
> This isn't even supported, see below:
> 
>>> gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
>>> gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
>>> -   gcc_5.o, gcc_7.o
>>> +   gcc_5.o, $(call cc-ifversion,-lt,1000, \
>>> +   gcc_7.o,  $(call cc-ifversion,-lt,1200, \
>>> +   gcc_10.o, gcc_12.o))
>>
>> This is getting unwieldy, so I think we ought to try to limit the number
>> of different files we have. Already gcc_4_9.c and gcc_7.c specify the
>> same GCOV_COUNTERS and differ only in the version checks (which could be
>> combined). Therefore ...
> 
> You may want to consider your policy on supported GCC versions. I see
> you still support:
> 
> * C compiler and linker:
>   - For x86:
> - GCC 4.1.2_20070115 or later
> - GNU Binutils 2.16.91.0.5 or later
> or
> - Clang/LLVM 3.5 or later
>   - For ARM 32-bit:
> - GCC 4.9 or later
> - GNU Binutils 2.24 or later
>   - For ARM 64-bit:
> - GCC 5.1 or later
> - GNU Binutils 2.24 or later
> 
> While also having some commented out warnings because the base GCC is so old:
> 
>   CFLAGS   += -Wmissing-prototypes
>   # (gcc 4.3x and later)   -Wconversion -Wno-sign-conversion
> 
> For reference QEMU's minimum GCC is 7.4
> 
>   if compiler.get_id() == 'gcc' and 
> compiler.version().version_compare('>=7.4')
> 
> and while our support cycle is based off distro LTS releases I have to
> wonder do you actually need to support users building the
> latest/greatest version of the hypervisor on ancient user-spaces with
> old compilers?
> 
> I think the oldest distro you have in your CI is jessie (still hanging
> on with extended LTS support) and that uses GCC 4.9.
> 
> I see there are various scripts to gather support status into the
> documentation but I couldn't find a general statement on the projects
> overall approach to supported versions of components.

That's the problem - we've been intending to raise the baseline for quite
some time, but figuring at least a rule of thumb by which to go both now
and in the future turns out problematic.

Jan

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

[Anthony Chan]

On Wed, 6 Sep 2023, Stefano Stabellini wrote:
> On Wed, 6 Sep 2023, Anthony Chan wrote:
> > Thanks, I've tried patches that stemmed from that discussion but
> > unfortunately, doesn't resolve the issue.  In fact, the s2idle_loop
> > branch might not be the problem at all.  I experimented with Xen to allow 
> > the
> > 'idle-states' into the FDT and prevented xen_guest_init on Linux from 
> > disabling
> > the 'cpuidle' driver (arch/arm/xen/enlighten.c).  When I trigger a suspend, 
> > I
> > can see now another thread (believe it's the idle thread) call into
> > drivers/firmware/psci/psci.c:__psci_cpu_suspend and then the Xen
> > counterpart at xen/arch/arm/vpsci.c:do_psci_0_2_cpu_suspend.
>
> OK but remember that Xen is not implementing do_psci_0_2_cpu_suspend
> correctly at the moment. Either we need to fix the Xen implementation, or we
> need to configure Linux so that it calls WFI instead of __psci_cpu_suspend.
>
> As a test, can you try to apply the attached patch to Xen as a tenative fix?  
> Or
> you could change drivers/firmware/psci/psci.c:__psci_cpu_suspend to call
> WFI instead of the PSCI operation (making sure to go to the entry_point
> instead of returning).

Tried the patch and substituting a WFI for a PSCI op, but Xen still watchdogs 
on the VMs in both cases.  I noticed the other Linux generic arm 'cpu-idle' 
driver which used to do issue a WFI/cpu_do_idle isn't useable anymore either.  
I'm not sure if Xen may have used to rely on this generic driver to get the WFI.

CONFIDENTIALITY NOTICE: This e-mail, including any attachments, may contain 
information that is confidential and privileged. Any unauthorized disclosure, 
reproduction or use of this e-mail is prohibited. If you are not the intended 
recipient, please notify the sender by reply e-mail or telephone and 
permanently delete this e-mail and any reproductions immediately.

Re: [PATCH] coverage: update gcov info for newer versions of gcc

[Alex Bennée]

Jan Beulich  writes:

> On 02.09.2023 17:11, Javi Merino wrote:
>> --- a/xen/common/coverage/Makefile
>> +++ b/xen/common/coverage/Makefile
>> @@ -5,7 +5,9 @@ obj-y += $(call cc-ifversion,-lt,0407, \
>>  gcc_3_4.o, $(call cc-ifversion,-lt,0409, \

This isn't even supported, see below:

>>  gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
>>  gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
>> -gcc_5.o, gcc_7.o
>> +gcc_5.o, $(call cc-ifversion,-lt,1000, \
>> +gcc_7.o,  $(call cc-ifversion,-lt,1200, \
>> +gcc_10.o, gcc_12.o))
>
> This is getting unwieldy, so I think we ought to try to limit the number
> of different files we have. Already gcc_4_9.c and gcc_7.c specify the
> same GCOV_COUNTERS and differ only in the version checks (which could be
> combined). Therefore ...

You may want to consider your policy on supported GCC versions. I see
you still support:

* C compiler and linker:
  - For x86:
- GCC 4.1.2_20070115 or later
- GNU Binutils 2.16.91.0.5 or later
or
- Clang/LLVM 3.5 or later
  - For ARM 32-bit:
- GCC 4.9 or later
- GNU Binutils 2.24 or later
  - For ARM 64-bit:
- GCC 5.1 or later
- GNU Binutils 2.24 or later

While also having some commented out warnings because the base GCC is so old:

  CFLAGS   += -Wmissing-prototypes
  # (gcc 4.3x and later)   -Wconversion -Wno-sign-conversion

For reference QEMU's minimum GCC is 7.4

  if compiler.get_id() == 'gcc' and compiler.version().version_compare('>=7.4')

and while our support cycle is based off distro LTS releases I have to
wonder do you actually need to support users building the
latest/greatest version of the hypervisor on ancient user-spaces with
old compilers?

I think the oldest distro you have in your CI is jessie (still hanging
on with extended LTS support) and that uses GCC 4.9.

I see there are various scripts to gather support status into the
documentation but I couldn't find a general statement on the projects
overall approach to supported versions of components.

>
>> --- /dev/null
>> +++ b/xen/common/coverage/gcc_10.c
>> @@ -0,0 +1,31 @@
>> +/*
>> + *  This code provides functions to handle gcc's profiling data format
>> + *  introduced with gcc 10.
>> + *
>> + *  For a better understanding, refer to gcc source:
>> + *  gcc/gcov-io.h
>> + *  libgcc/libgcov.c
>> + *
>> + *  Uses gcc-internal data definitions.
>> + */
>> +
>> +#include "gcov.h"
>> +
>> +#if GCC_VERSION < 10 || GCC_VERSION > 12
>> +#error "Wrong version of GCC used to compile gcov"
>> +#endif
>> +
>> +#define GCOV_COUNTERS 8
>> +#define GCOV_UNIT_SIZE 1
>> +
>> +#include "gcc_4_7.c"
>
> ... this could simply re-use gcc_4_7.c directly, with just the version
> check there suitably tweaked.
>
>> --- a/xen/common/coverage/gcc_4_7.c
>> +++ b/xen/common/coverage/gcc_4_7.c
>> @@ -27,6 +27,7 @@
>>  #  error "Wrong version of GCC used to compile gcov"
>>  # endif
>>  #define GCOV_COUNTERS 8
>> +#define GCOV_UNIT_SIZE 1
>>  #endif
>
> If further this became a separate #ifdef, ...
>
>> --- a/xen/common/coverage/gcc_4_9.c
>> +++ b/xen/common/coverage/gcc_4_9.c
>> @@ -19,6 +19,7 @@
>>  #endif
>>  
>>  #define GCOV_COUNTERS 9
>> +#define GCOV_UNIT_SIZE 1
>>  
>>  #include "gcc_4_7.c"
>>  
>> --- a/xen/common/coverage/gcc_5.c
>> +++ b/xen/common/coverage/gcc_5.c
>> @@ -19,6 +19,7 @@
>>  #endif
>>  
>>  #define GCOV_COUNTERS 10
>> +#define GCOV_UNIT_SIZE 1
>>  
>>  #include "gcc_4_7.c"
>>  
>
> ... touching these two files could be avoided altogether.
>
> Henry - afaict this was submitted after the feature submission deadline,
> so you may want to consider giving it an exception.
>
> Jan


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Ping: [PATCH] build: restrict gcc11 workaround to versions earlier than 11.3.0

[Jan Beulich]

On 22.08.2023 16:40, Jan Beulich wrote:
> The fix for this issue was backported to 11.3, so let's not unduly
> engage the workaround.
> 
> Signed-off-by: Jan Beulich 

Anyone? Please?

Thanks, Jan

> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -146,7 +146,8 @@
>  __asm__ ("" : "=r"(__ptr) : "0"(ptr));  \
>  (typeof(ptr)) (__ptr + (off)); })
>  
> -#if CONFIG_GCC_VERSION >= 11 /* See gcc bug 100680. */
> +/* See gcc bug 100680. */
> +#if CONFIG_GCC_VERSION >= 11 && CONFIG_GCC_VERSION < 110300
>  # define gcc11_wrap(x) RELOC_HIDE(x, 0)
>  #else
>  # define gcc11_wrap(x) (x)
> 

Re: [PATCH v3] docs/misra: add rule 2.1 exceptions

[Julien Grall]

Hi Stefano,

On 09/09/2023 00:03, Stefano Stabellini wrote:

From: Stefano Stabellini 

During the discussions that led to the acceptance of Rule 2.1, we
decided on a few exceptions that were not properly recorded in
rules.rst. Add them now.

Signed-off-by: Stefano Stabellini 
Acked-by: Jan Beulich 
---
Nicola, does this work with ECLAIR?

I am referring to the locations of the SAF-2-safe tag on top of
call_psci_system_off, BUG, etc.

Changes in v3:
- added SAF-2-safe to safe.json
- added a few SAF-2-safe examples
---
  docs/misra/rules.rst| 13 -
  docs/misra/safe.json|  8 
  xen/arch/arm/psci.c |  1 +
  xen/arch/x86/shutdown.c |  1 +
  xen/include/xen/bug.h   |  2 ++
  5 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index 34916e266a..82de4c645d 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -107,7 +107,18 @@ maintainers if you want to suggest a change.
 * - `Rule 2.1 
`_
   - Required
   - A project shall not contain unreachable code
- -
+ - The following are allowed:
+ - Invariantly constant conditions, e.g. if(IS_ENABLED(CONFIG_HVM)) { 
S; }
+ - Switch with a controlling value statically determined not to
+   match one or more case statements
+ - Functions that are intended to be referenced only from
+   assembly code (e.g. 'do_trap_fiq')
+ - Deliberate unreachability caused by certain macros/functions,
+   e.g. BUG, assert_failed, panic, etc. See safe.json.
+ - asm-offsets.c, as they are not linked deliberately, because
+   they are used to generate definitions for asm modules
+ - Declarations without initializer are safe, as they are not
+   executed
  
 * - `Rule 2.6 `_

   - Advisory
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 39c5c056c7..fc96a99fd5 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -20,6 +20,14 @@
  },
  {
  "id": "SAF-2-safe",
+"analyser": {
+"eclair": "MC3R1.R2.1"
+},
+"name": "Rule 2.1: deliberate unreachability",
+"text": "Macro or function designed to be unreachable."
+},
+{
+"id": "SAF-3-safe",
  "analyser": {},
  "name": "Sentinel",
  "text": "Next ID to be used"
diff --git a/xen/arch/arm/psci.c b/xen/arch/arm/psci.c
index 695d2fa1f1..2a8527cacc 100644
--- a/xen/arch/arm/psci.c
+++ b/xen/arch/arm/psci.c
@@ -59,6 +59,7 @@ void call_psci_cpu_off(void)
  }
  }
  
+/* SAF-2-safe */


I think any use of SAF-2-safe should be accompanied with an attribute...


  void call_psci_system_off(void)


... noreturn for function or ...


  {
  if ( psci_ver > PSCI_VERSION(0, 1) )
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 7619544d14..47e0f59024 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -118,6 +118,7 @@ static inline void kb_wait(void)
  break;
  }
  
+/* SAF-2-safe */

  static void noreturn cf_check __machine_halt(void *unused)
  {
  local_irq_disable();
diff --git a/xen/include/xen/bug.h b/xen/include/xen/bug.h
index e8a4eea71a..d47c54f034 100644
--- a/xen/include/xen/bug.h
+++ b/xen/include/xen/bug.h
@@ -117,6 +117,7 @@ struct bug_frame {
  #endif
  
  #ifndef BUG

+/* SAF-2-safe */
  #define BUG() do {  \
  BUG_FRAME(BUGFRAME_bug,  __LINE__, __FILE__, 0, NULL);  \
  unreachable();  \


... unreachable for macros. But the /* SAF-2-safe */ feels a little bit 
redundant when a function is marked as 'noreturn'.


Is there any way to teach eclair about noreturn?

Cheers,

--
Julien Grall

Re: [XEN PATCH] xen/PCI: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Jan Beulich]

On 11.09.2023 17:13, Federico Serafini wrote:
> On 11/09/23 10:42, Jan Beulich wrote:
>> On 11.09.2023 10:15, Federico Serafini wrote:
>>> Add missing parameter names and make function declarations and
>>> definitions consistent. No functional change.
>>>
>>> Signed-off-by: Federico Serafini 
>>
>> Since formally correct:
>> Acked-by: Jan Beulich 
>>
>> Nevertheless, it is probably about time to mention that ...
>>
>>> @@ -198,10 +198,11 @@ int pci_find_next_cap(u16 seg, u8 bus, unsigned int 
>>> devfn, u8 pos, int cap);
>>>   int pci_find_ext_capability(int seg, int bus, int devfn, int cap);
>>>   int pci_find_next_ext_capability(int seg, int bus, int devfn, int start,
>>>int cap);
>>> -const char *parse_pci(const char *, unsigned int *seg, unsigned int *bus,
>>> -  unsigned int *dev, unsigned int *func);
>>> -const char *parse_pci_seg(const char *, unsigned int *seg, unsigned int 
>>> *bus,
>>> -  unsigned int *dev, unsigned int *func, bool 
>>> *def_seg);
>>> +const char *parse_pci(const char *s, unsigned int *seg_p, unsigned int 
>>> *bus_p,
>>> +  unsigned int *dev_p, unsigned int *func_p);
>>> +const char *parse_pci_seg(const char *s, unsigned int *seg_p,
>>> +  unsigned int *bus_p, unsigned int *dev_p,
>>> +  unsigned int *func_p, bool *def_seg);
>>
>> ... parameter renaming like this, while fulfilling the word of Misra, is
>> actually hampering readability. To someone wanting to use the function
>> and hence looking at the declaration it is entirely uninteresting
>> whether the _p suffixes are there; there were similar changes earlier on.
>> The longer names merely make reading harder and - as is the case here -
>> also may require a single declaration to wrap across more lines. I view
>> such as going against the spirit of Misra (aiming to improve code
>> clarity).
> 
> I agree with you, but, the removal of _p would lead to
> other (mechanical) changes to the function body.
> In such cases, do you think that an improvement in readability
> justifies the code churn?

I didn't suggest changing the definition. I suggested keeping declaration
and definition (slightly) out of sync.

Jan

Re: [XEN PATCH] xen/PCI: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Federico Serafini]

On 11/09/23 10:42, Jan Beulich wrote:

On 11.09.2023 10:15, Federico Serafini wrote:

Add missing parameter names and make function declarations and
definitions consistent. No functional change.

Signed-off-by: Federico Serafini 


Since formally correct:
Acked-by: Jan Beulich 

Nevertheless, it is probably about time to mention that ...


@@ -198,10 +198,11 @@ int pci_find_next_cap(u16 seg, u8 bus, unsigned int 
devfn, u8 pos, int cap);
  int pci_find_ext_capability(int seg, int bus, int devfn, int cap);
  int pci_find_next_ext_capability(int seg, int bus, int devfn, int start,
   int cap);
-const char *parse_pci(const char *, unsigned int *seg, unsigned int *bus,
-  unsigned int *dev, unsigned int *func);
-const char *parse_pci_seg(const char *, unsigned int *seg, unsigned int *bus,
-  unsigned int *dev, unsigned int *func, bool 
*def_seg);
+const char *parse_pci(const char *s, unsigned int *seg_p, unsigned int *bus_p,
+  unsigned int *dev_p, unsigned int *func_p);
+const char *parse_pci_seg(const char *s, unsigned int *seg_p,
+  unsigned int *bus_p, unsigned int *dev_p,
+  unsigned int *func_p, bool *def_seg);


... parameter renaming like this, while fulfilling the word of Misra, is
actually hampering readability. To someone wanting to use the function
and hence looking at the declaration it is entirely uninteresting
whether the _p suffixes are there; there were similar changes earlier on.
The longer names merely make reading harder and - as is the case here -
also may require a single declaration to wrap across more lines. I view
such as going against the spirit of Misra (aiming to improve code
clarity).

Jan



I agree with you, but, the removal of _p would lead to
other (mechanical) changes to the function body.
In such cases, do you think that an improvement in readability
justifies the code churn?

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

Re: [PATCH RESEND v2 2/2] xen/arm: Enlarge identity map space to 127TiB

[Julien Grall]

Hi Leo,

I know you said you will respin the series. I'd like to leave some 
comments to avoid having another round afterwards.


On 09/09/2023 09:34, Leo Yan wrote:

On some platforms, the memory regions could be:


Can you add some details in the commit message on which platform you saw 
the issue?


Also, in v1, the problem was also depending on the firmware version. Do 
you know if it fails booting on a new or old firmware?




   (XEN) MODULE[0]: 0807f6df - 0807f6f3e000 Xen
   (XEN) MODULE[1]: 0807f8054000 - 0807f8056000 Device Tree
   (XEN) MODULE[2]: fa834000 - fc5de1d5 Ramdisk
   (XEN) MODULE[3]: fc5df000 - ffb3f810 Kernel

In this case, the Xen binary is loaded above 2TiB.  2TiB is the maximum
identity map space supported by Xen, thus it fails to boot up due to the
out of the range.

This patch introduces several macros to present the zeroth page table's


Typo: s/zeroth/zeroeth/


slot numbers for easier readable.  Based on the defined macros, it
enlarges identity map space to 127TiB, which can support the memory
space [0x0 .. 0x7eff__] so has flexibility for various
platforms.


Reserving 127 TiB for just the identity mapping is quite a lot. How did 
you decide the limit? What limit do you need on your platform?




Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to prepare/enable/disable")
Reported-by: Alexey Klimov 
Signed-off-by: Leo Yan 
---
  xen/arch/arm/arm64/mm.c   | 12 
  xen/arch/arm/include/asm/config.h | 15 ---
  xen/arch/arm/mm.c |  2 +-
  3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/xen/arch/arm/arm64/mm.c b/xen/arch/arm/arm64/mm.c
index 78b7c7eb00..802170cf29 100644
--- a/xen/arch/arm/arm64/mm.c
+++ b/xen/arch/arm/arm64/mm.c
@@ -40,8 +40,10 @@ static void __init prepare_boot_identity_mapping(void)
  clear_page(boot_second_id);
  clear_page(boot_third_id);
  
-if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )

-panic("Cannot handle ID mapping above 2TB\n");
+if ( id_offsets[0] >= XEN_IDENTITY_MAP_L0_END )


I don't see the value of renaming IDENTIY_MAPPING_AREA_NR_L0 to 
XEN_IDENTIY_MAP_L0_END. See more below.



+/* 1TiB occupies 2 slots in zeroeth table */


I don't understand how this comment is related to the message below.


+panic("Cannot handle ID mapping above %dTiB\n",


The value is unsigned, so this you should use "%u". Also we have been 
using "TB" in Xen rather than "TiB". I would rather prefer if we keep 
the same for consistency even if this is not totally accurate.



+  XEN_IDENTITY_MAP_L0_END>>1);


Coding style: please add a space before and after >>.

  
  /* Link first ID table */

  pte = mfn_to_xen_entry(virt_to_mfn(boot_first_id), MT_NORMAL);
@@ -73,8 +75,10 @@ static void __init prepare_runtime_identity_mapping(void)
  lpae_t pte;
  DECLARE_OFFSETS(id_offsets, id_addr);
  
-if ( id_offsets[0] >= IDENTITY_MAPPING_AREA_NR_L0 )

-panic("Cannot handle ID mapping above 2TB\n");
+if ( id_offsets[0] >= XEN_IDENTITY_MAP_L0_END )
+/* 1TiB occupies 2 slots in zeroeth table */
+panic("Cannot handle ID mapping above %dTiB\n",
+  XEN_IDENTITY_MAP_L0_END>>1);


Similar remarks here.

  
  /* Link first ID table */

  pte = pte_of_xenaddr((vaddr_t)xen_first_id);
diff --git a/xen/arch/arm/include/asm/config.h 
b/xen/arch/arm/include/asm/config.h
index 21f4e68a40..b772f1574d 100644
--- a/xen/arch/arm/include/asm/config.h
+++ b/xen/arch/arm/include/asm/config.h
@@ -87,11 +87,11 @@
   *   2G -   4G   Domheap: on-demand-mapped
   *
   * ARM64 layout:
- * 0x - 0x01ff (2TB, L0 slots [0..3])
+ * 0x - 0x7eff (127TB, L0 slots [0..253])
   *
   *  Reserved to identity map Xen
   *
- * 0x0200 - 0x027f (512GB, L0 slot [4])
+ * 0x07f0 - 0x7fff (1TB, L0 slot [254..255])


I don't understand why this is changed.


   *  (Relative offsets)
   *   0  -   2M   Unmapped
   *   2M -  10M   Xen text, data, bss
@@ -103,9 +103,6 @@
   *
   *  32G -  64G   Frametable: 56 bytes per page for 2TB of RAM
   *
- * 0x0280 - 0x7fff (125TB, L0 slots [5..255])
- *  Unused
- *
   * 0x8000 - 0x84ff (5TB, L0 slots [256..265])
   *  1:1 mapping of RAM
   *
@@ -117,8 +114,12 @@
  #define XEN_VIRT_START  _AT(vaddr_t, MB(2))
  #else
  
-#define IDENTITY_MAPPING_AREA_NR_L0	4

-#define XEN_VM_MAPPING SLOT0(IDENTITY_MAPPING_AREA_NR_L0)
+#define XEN_IDENTITY_MAP_L0_START   0


So what's the value of L0_START when you don't use it in the code? 
Overall, I think it will actually mislead the developper because none of 
the code in Xen can cope with the value been non-zero.


And FAOD, I don't think we should handle it in Xen. So I would rather 
prefer if the renaming is avoided.



+#def

Re: [PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Michal Orzel]

On 11/09/2023 16:48, Julien Grall wrote:
> Caution: This message originated from an External Source. Use proper caution 
> when opening attachments, clicking links, or responding.
> 
> 
> On 11/09/2023 15:01, Michal Orzel wrote:
>> Hi Julien,
>>
>> On 11/09/2023 15:14, Julien Grall wrote:
>>>
>>>
>>> Hi,
>>>
>>> On 11/09/2023 13:34, Michal Orzel wrote:
 Host device tree nodes under /chosen with compatible string
 "xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
 meant to be exposed to hardware domain.
>>>
>>> So, how dom0 is meant to discover the static event channel, shared
>>> memory it can use?
>>
>> For static shared memory:
>> A node with this compatible is only meant for Xen since it contains 
>> information like host-guest mapping.
>> Xen creates a different node for guests under /reserved-memory.
>> Linux binding:
>> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt
>> Xen node generation:
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;hb=HEAD#l1407
> 
> Ah good point. I agree with this one.
> 
>>
>> For static event channels:
>> This is a bit weird so let me put it in a different way.
>> 1) Xen does not create any node for static evtchn for domU.
>> 2) The booting.txt states:
>> There is no need to describe the static event channel info in the domU device
>> tree. Static event channels are only useful in fully static configurations,
>> and in those configurations, the domU device tree dynamically generated by 
>> Xen
>> is not needed.
>> 3) The "xen,evtchn" property specifies the local port as well as phandle of 
>> domU node.
>> dom0 does not have access to domU nodes, therefore exposing a property with 
>> not existing phandle
>> makes me think that:
> 
> You have a point for the phandle. Yet, this is the only way dom0 can
> find the event channel today. As we exposed it, I don't think we can
> remove it until we have a proper replacement.
> 
> We might get away if the feature is not supported it at all. But there
> is no statement, so it is a little unclear whether the feature is meant
> to be in technical preview.
> 
> In any case, I think the commit message deserve a bit more explanation
> as hiding "xen,evtchn-v1"/"xen,domain-shared-memory-v1" is not
> uncontroversial.
> 
>> a) point 2) applies to dom0 as well and we should hide this node or > b) 
>> there is a missing property for both dom0 and domUs to tell them
> about static local port in a proper way
>>
>> Exposing Xen specific evtchn node only to dom0 and not to domU with required 
>> information happen to be found as first value
>> in xen,evtchn is definitely not a proper solution.
> 
> My concern here is we exposed such information to dom0. So as I said
> above, I don't think we can simply remove it as there is no other way to
> find such information today.
> 
> It doesn't matter that it wasn't exposed to domU.
> 
>> Also, there is no Linux binding for it.
> 
> AFAIK the static event channel support was not added in Linux until very
> recently. Also, I think the kernel doesn't directly use the static event
> channel. So it is possible, this is just an overlook.

I've found this thread in which Stefano, Rahul and Bertrand agrees on not 
exposing
any dt property and the rationale behind:
https://patchwork.kernel.org/project/xen-devel/patch/4836304496e6fbbea41348ed8cc9fcf6b0f3e893.1648049827.git.rahul.si...@arm.com/

I would not call exposing node to dom0 as something done deliberately given 
that it happens automatically by copying. So my understanding is
that we did not want to expose any node and dom0 case was overlooked (since 
done automatically).

Exposing half the interface (from system POV) in a way it should not be done 
(phandle) is not great IMO.
In any case, if you insist on keeping xen,evtchn, I can leave with it.

This feature is marked as tech preview with no Linux binding in place so I 
would not be worried.

~Michal

Re: [PATCH] OvmfPkg/OvmfXen: Fix S3

[Anthony PERARD]

Hi Xenia,

On Thu, Jul 13, 2023 at 01:47:12PM +0300, 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.

Is it possible to have S3 work without fw_cfg? We normally disable
fw_cfg in QEMU when using it to start a Xen guest. We only enable fw_cfg
if we want to boot a kernel directly in HVM (like PV guest, called
Direct Kernel Boot in xl.cfg(5)).

But even trying the direct kernel boot method, I've got issue trying to
resume a suspended guest. I ran into:
ASSERT /root/build/ovmf/UefiCpuPkg/Library/MpInitLib/MpLib.c(2093): 
(LibPcdGet64(9U) & (0x0001 | 0x0002)) == (0x0001 | 0x0002)

By the way, I'm using `xl` to boot a guest to test this patch.

So, I've got some question, which version of QEMU do you use, which work
with this patch?
What toolstack do you use to boot a guest?


Thanks,

-- 
Anthony PERARD

[PATCH v2 for-4.18?] x86: support data operand independent timing mode

[Jan Beulich]

[1] specifies a long list of instructions which are intended to exhibit
timing behavior independent of the data they operate on. On certain
hardware this independence is optional, controlled by a bit in a new
MSR. Provide a command line option to control the mode Xen and its
guests are to operate in, with a build time control over the default.
Longer term we may want to allow guests to control this.

Since Arm64 supposedly also has such a control, put command line option
and Kconfig control in common files.

[1] 
https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/best-practices/data-operand-independent-timing-isa-guidance.html

Requested-by: Demi Marie Obenour 
Signed-off-by: Jan Beulich 
---
This may be viewed as a new feature, and hence be too late for 4.18. It
may, however, also be viewed as security relevant, which is why I'd like
to propose to at least consider it.

Slightly RFC, in particular for whether the Kconfig option should
default to Y or N.

I would have wanted to invoke setup_doitm() from cpu_init(), but that
works only on the BSP. On APs cpu_init() runs before ucode loading.
Plus recheck_cpu_features() invoking identify_cpu() takes care of the
BSP during S3 resume.
---
v2: Introduce and use cpu_has_doitm. Add comment "borrowed" from the
XenServer patch queue patch providing similar functionality.
Re-base.

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -788,6 +788,14 @@ Specify the size of the console debug tr
 additionally a trace buffer of the specified size is allocated per cpu.
 The debug trace feature is only enabled in debugging builds of Xen.
 
+### dit (x86)
+> `= `
+
+> Default: `CONFIG_DIT_DEFAULT`
+
+Specify whether Xen and guests should operate in Data Independent Timing
+mode.
+
 ### dma_bits
 > `= `
 
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -15,6 +15,7 @@ config X86
select HAS_ALTERNATIVE
select HAS_COMPAT
select HAS_CPUFREQ
+   select HAS_DIT
select HAS_EHCI
select HAS_EX_TABLE
select HAS_FAST_MULTIPLY
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -204,6 +204,28 @@ void ctxt_switch_levelling(const struct
alternative_vcall(ctxt_switch_masking, next);
 }
 
+static void setup_doitm(void)
+{
+uint64_t msr;
+
+if ( !cpu_has_doitm )
+return;
+
+/*
+ * We don't currently enumerate DOITM to guests.  As a conseqeuence, guest
+ * kernels will believe they're safe even when they are not.
+ *
+ * For now, set it unilaterally.  This prevents otherwise-correct crypto
+ * code from becoming vulnerable to timing sidechannels.
+ */
+
+rdmsrl(MSR_UARCH_MISC_CTRL, msr);
+msr |= UARCH_CTRL_DOITM;
+if ( !opt_dit )
+msr &= ~UARCH_CTRL_DOITM;
+wrmsrl(MSR_UARCH_MISC_CTRL, msr);
+}
+
 bool opt_cpu_info;
 boolean_param("cpuinfo", opt_cpu_info);
 
@@ -589,6 +611,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
 
mtrr_bp_init();
}
+
+   setup_doitm();
 }
 
 /* leaf 0xb SMT level */
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -201,6 +201,7 @@ static inline bool boot_cpu_has(unsigned
 #define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
 #define cpu_has_tsx_ctrlboot_cpu_has(X86_FEATURE_TSX_CTRL)
 #define cpu_has_taa_no  boot_cpu_has(X86_FEATURE_TAA_NO)
+#define cpu_has_doitm   boot_cpu_has(X86_FEATURE_DOITM)
 #define cpu_has_fb_clearboot_cpu_has(X86_FEATURE_FB_CLEAR)
 #define cpu_has_rrsba   boot_cpu_has(X86_FEATURE_RRSBA)
 #define cpu_has_gds_ctrlboot_cpu_has(X86_FEATURE_GDS_CTRL)
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -41,6 +41,9 @@ config HAS_COMPAT
 config HAS_DEVICE_TREE
bool
 
+config HAS_DIT # Data Independent Timing
+   bool
+
 config HAS_EX_TABLE
bool
 
@@ -175,6 +178,18 @@ config SPECULATIVE_HARDEN_GUEST_ACCESS
 
 endmenu
 
+config DIT_DEFAULT
+   bool "Data Independent Timing default"
+   depends on HAS_DIT
+   help
+ Hardware often surfaces instructions the timing of which is dependent
+ on the data they process.  Some of these instructions may be used in
+ timing sensitive environments, e.g. cryptography.  When such
+ instructions exist, hardware may further surface a control allowing
+ to make the behavior of such instructions independent of the data
+ they act upon.  Choose the default here for when no "dit" command line
+ option is present.
+
 config HYPFS
bool "Hypervisor file system support"
default y
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -28,6 +28,11 @@ CHECK_feature_info;
 
 enum system_state system_state = SYS_STATE_early_boot;
 
+#ifdef CONFIG_HAS_DIT
+bool __ro_after_init opt_dit = IS_ENABLED(CONFIG_DIT_DEFAULT);
+boolean_param("dit", opt_dit);
+#end

Re: [PATCH] include: make domain_page.h's stubs properly use type-unsafe MFN <-> virt helpers

[Julien Grall]

Hi Jan,

On 11/09/2023 07:38, Jan Beulich wrote:

The first of the commits referenced below didn't go far enough, and the
2nd of them, while trying to close (some of) the gap, wrongly kept using
the potentially type-safe variant. This is getting in the way of new
ports preferably not having any type-unsafe private code (and in
particular not having a need for any overrides in newly introduced
files).

Fixes: 41c48004d1d8 ("xen/mm: Use __virt_to_mfn in map_domain_page instead of 
virt_to_mfn")
Fixes: f46b6197344f ("xen: Convert page_to_mfn and mfn_to_page to use typesafe 
MFN")
Reported-by: Shawn Anastasio 
Signed-off-by: Jan Beulich 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to head.S

[Julien Grall]

On 31/08/2023 10:16, Henry Wang wrote:

On Aug 31, 2023, at 17:12, Ayan Kumar Halder  wrote:

Hi Henry,

On 28/08/2023 02:32, Henry Wang wrote:


diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 33b038e7e0..39218cf15f 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -83,6 +83,25 @@
  isb
  .endm

+/*
+ * Enforce Xen page-tables do not contain mapping that are both
+ * Writable and eXecutables.
+ *
+ * This should be called on each secondary CPU.
+ */
+.macro pt_enforce_wxn tmp
+mrc   CP32(\tmp, HSCTLR)
+orr   \tmp, \tmp, #SCTLR_Axx_ELx_WXN
+dsb
+mcr   CP32(\tmp, HSCTLR)
+/*
+ * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized
+ * before flushing the TLBs.
+ */
+isb
+flush_xen_tlb_local \tmp
+.endm
+
  /*
   * Common register usage in this file:
   *   r0  -
@@ -254,6 +273,7 @@ secondary_switched:
  /* Use a virtual address to access the UART. */
  mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
  #endif
+pt_enforce_wxn
  


Can you move ^^^ to before "#ifdef CONFIG_EARLY_PRINTK" so that the MMU related 
functionality are bundled together?

Also AFAIU, mov_w has not effect on pt_enforce_wxn().

So that I can create a function "enable_secondary_cpu_mm()" - similar to one 
you introduced for arm64


Sure, I am good with this if other maintainers do not have any objections.


I am objecting. It would be quite handy to print a message on the 
console to indicate that we are enforce WXN. So we want to update UART 
address (stored in r11) before hand.


Cheers,

--
Julien Grall

Re: [PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Julien Grall]

On 11/09/2023 15:01, Michal Orzel wrote:

Hi Julien,

On 11/09/2023 15:14, Julien Grall wrote:



Hi,

On 11/09/2023 13:34, Michal Orzel wrote:

Host device tree nodes under /chosen with compatible string
"xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
meant to be exposed to hardware domain.


So, how dom0 is meant to discover the static event channel, shared
memory it can use?


For static shared memory:
A node with this compatible is only meant for Xen since it contains information 
like host-guest mapping.
Xen creates a different node for guests under /reserved-memory.
Linux binding:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt
Xen node generation:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;hb=HEAD#l1407


Ah good point. I agree with this one.



For static event channels:
This is a bit weird so let me put it in a different way.
1) Xen does not create any node for static evtchn for domU.
2) The booting.txt states:
There is no need to describe the static event channel info in the domU device
tree. Static event channels are only useful in fully static configurations,
and in those configurations, the domU device tree dynamically generated by Xen
is not needed.
3) The "xen,evtchn" property specifies the local port as well as phandle of 
domU node.
dom0 does not have access to domU nodes, therefore exposing a property with not 
existing phandle
makes me think that:


You have a point for the phandle. Yet, this is the only way dom0 can 
find the event channel today. As we exposed it, I don't think we can 
remove it until we have a proper replacement.


We might get away if the feature is not supported it at all. But there 
is no statement, so it is a little unclear whether the feature is meant 
to be in technical preview.


In any case, I think the commit message deserve a bit more explanation 
as hiding "xen,evtchn-v1"/"xen,domain-shared-memory-v1" is not 
uncontroversial.


a) point 2) applies to dom0 as well and we should hide this node or > b) there is a missing property for both dom0 and domUs to tell them 

about static local port in a proper way


Exposing Xen specific evtchn node only to dom0 and not to domU with required 
information happen to be found as first value
in xen,evtchn is definitely not a proper solution.


My concern here is we exposed such information to dom0. So as I said 
above, I don't think we can simply remove it as there is no other way to 
find such information today.


It doesn't matter that it wasn't exposed to domU.


Also, there is no Linux binding for it.


AFAIK the static event channel support was not added in Linux until very 
recently. Also, I think the kernel doesn't directly use the static event 
channel. So it is possible, this is just an overlook.


Cheers,

--
Julien Grall

Re: [PATCH v4 3/5] xen/ppc: Define minimal stub headers required for full build

[Jan Beulich]

On 09.09.2023 00:50, Shawn Anastasio wrote:
> Additionally, change inclusion of asm/ headers to corresponding xen/ ones
> throughout arch/ppc now that they work.
> 
> Signed-off-by: Shawn Anastasio 

Acked-by: Jan Beulich 
albeit ...

> 
>  xen/arch/ppc/Kconfig |   1 +
>  xen/arch/ppc/include/asm/altp2m.h|  25 +++
>  xen/arch/ppc/include/asm/bug.h   |   9 +
>  xen/arch/ppc/include/asm/cache.h |   2 +
>  xen/arch/ppc/include/asm/config.h|  10 +
>  xen/arch/ppc/include/asm/cpufeature.h|  10 +
>  xen/arch/ppc/include/asm/current.h   |  43 
>  xen/arch/ppc/include/asm/delay.h |  12 ++
>  xen/arch/ppc/include/asm/device.h|  53 +
>  xen/arch/ppc/include/asm/div64.h |  14 ++
>  xen/arch/ppc/include/asm/domain.h|  47 +
>  xen/arch/ppc/include/asm/event.h |  36 
>  xen/arch/ppc/include/asm/flushtlb.h  |  24 +++
>  xen/arch/ppc/include/asm/grant_table.h   |   5 +
>  xen/arch/ppc/include/asm/guest_access.h  |  68 +++
>  xen/arch/ppc/include/asm/guest_atomics.h |  23 +++
>  xen/arch/ppc/include/asm/hardirq.h   |  19 ++
>  xen/arch/ppc/include/asm/hypercall.h |   5 +
>  xen/arch/ppc/include/asm/io.h|  16 ++
>  xen/arch/ppc/include/asm/iocap.h |   8 +
>  xen/arch/ppc/include/asm/iommu.h |   8 +
>  xen/arch/ppc/include/asm/irq.h   |  33 +++
>  xen/arch/ppc/include/asm/mem_access.h|   5 +
>  xen/arch/ppc/include/asm/mm.h| 243 ++-
>  xen/arch/ppc/include/asm/monitor.h   |  43 
>  xen/arch/ppc/include/asm/nospec.h|  15 ++
>  xen/arch/ppc/include/asm/numa.h  |  26 +++
>  xen/arch/ppc/include/asm/p2m.h   |  95 +
>  xen/arch/ppc/include/asm/page.h  |  18 ++
>  xen/arch/ppc/include/asm/paging.h|   7 +
>  xen/arch/ppc/include/asm/pci.h   |   7 +
>  xen/arch/ppc/include/asm/percpu.h|  24 +++
>  xen/arch/ppc/include/asm/processor.h |  10 +
>  xen/arch/ppc/include/asm/random.h|   9 +
>  xen/arch/ppc/include/asm/setup.h |   6 +
>  xen/arch/ppc/include/asm/smp.h   |  18 ++
>  xen/arch/ppc/include/asm/softirq.h   |   8 +
>  xen/arch/ppc/include/asm/spinlock.h  |  15 ++
>  xen/arch/ppc/include/asm/system.h| 219 +++-
>  xen/arch/ppc/include/asm/time.h  |  23 +++
>  xen/arch/ppc/include/asm/vm_event.h  |  49 +

... this file now wants dropping, and xen/arch/ppc/include/asm/Makefile
instead indicating to use the asm-generic variant.

Note further that xen/arch/ppc/include/asm/numa.h has a leftover Arm
reference that likely wasn't meant to be there.

Jan

Re: [PATCH] cmdline: move irq-max-guests doc entry

[Jan Beulich]

On 11.09.2023 16:34, George Dunlap wrote:
> On Thu, Sep 7, 2023 at 3:17 PM Jan Beulich  wrote:
>>
>> ... to adhere to intended sorting.
>>
>> Fixes: e373bc1bdc59 ("x86/IRQ: make max number of guests for a shared IRQ 
>> configurable")
> 
> This seems a bit strong, but in any case...

I can drop it if you think that's better. I wasn't really sure either way,
to be honest.

>> Signed-off-by: Jan Beulich 
> 
> Acked-by: George Dunlap 

Thanks, Jan

Re: [PATCH] cmdline: move irq-max-guests doc entry

[George Dunlap]

On Thu, Sep 7, 2023 at 3:17 PM Jan Beulich  wrote:
>
> ... to adhere to intended sorting.
>
> Fixes: e373bc1bdc59 ("x86/IRQ: make max number of guests for a shared IRQ 
> configurable")

This seems a bit strong, but in any case...

> Signed-off-by: Jan Beulich 

Acked-by: George Dunlap 

[ovmf test] 182940: all pass - PUSHED

[osstest service owner]

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

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

Last test of basis   182916  2023-09-11 03:51:19 Z0 days
Testing same since   182940  2023-09-11 12:42:16 Z0 days1 attempts


People who touched revisions under test:
  Pierre Gondois 

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
   cc26a2cb31..a5cbb59905  a5cbb5990593559eb9d93ebd1df1e8da0fa5d72b -> 
xen-tested-master

Re: [PATCH v6 00/13] xen/arm: Split MMU code as the prepration of MPU work

[Ayan Kumar Halder]

Hi Henry,

On 08/09/2023 23:15, Stefano Stabellini wrote:

CAUTION: This message has originated from an External Source. Please use proper 
judgment and caution when opening attachments, clicking links, or responding to 
this email.


I committed patches 1-5

On Mon, 28 Aug 2023, Henry Wang wrote:

Based on the discussion in the Xen Summit [1], sending this series out after
addressing the comments in v5 [2] as the preparation work to add MPU support.
The series passed the GitLab CI check in [3].

Mostly code movement and function folding, with some of Kconfig and build
system (mainly Makefiles) adjustment.

This series is based on:
ec272d8d4c CI: Always move the bisect build log back

[1] 
https://lore.kernel.org/xen-devel/as8pr08mb799122f8b0cb841ded64f48192...@as8pr08mb7991.eurprd08.prod.outlook.com/
[2] 
https://lore.kernel.org/xen-devel/20230814042536.878720-1-henry.w...@arm.com/
[3] https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/982592897

Henry Wang (9):
   xen/arm: Introduce CONFIG_MMU Kconfig option
   xen/arm64: Split and move MMU-specific head.S to mmu/head.S
   xen/arm64: Fold setup_fixmap() to create_page_tables()
   xen/arm: Split page table related code to mmu/pt.c
   xen/arm: Split MMU system SMP MM bringup code to mmu/smpboot.c
   xen/arm: Fold mmu_init_secondary_cpu() to head.S
   xen/arm: Extract MMU-specific MM code
   xen/arm: Split MMU-specific setup_mm() and related code out
   xen/arm: Fold pmap and fixmap into MMU system

Penny Zheng (2):
   xen/arm: Rename init_secondary_pagetables() to prepare_secondary_mm()
   xen/arm: mmu: move MMU specific P2M code to mmu/p2m.{c,h}

Wei Chen (2):
   xen/arm64: head.S: Introduce enable_{boot,secondary}_cpu_mm()
   xen/arm: Move MMU related definitions from config.h to mmu/layout.h


Except for "[PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to 
head.S " where I have a small comment, I am happy with the current set.


I have made the corresponding changes for Arm32 MMU (See 
https://gitlab.com/xen-project/people/ayankuma/xen/-/commits/mmu_arm32_on_arm64?ref_type=heads 
),


and triggered the CI. It looks all good (qemu-smoke-ppc64le-pseries-gcc 
failure seems unrelated).


I have sent out the patches for review so that the maintainers/you can 
have a look and review them


("[XEN v1 0/4] xen/arm: Split MMU code as the prepration of MPU work 
form Arm32").


- Ayan



  xen/arch/arm/Kconfig|5 +-
  xen/arch/arm/Makefile   |1 +
  xen/arch/arm/arm32/Makefile |1 +
  xen/arch/arm/arm32/head.S   |   22 +-
  xen/arch/arm/arm32/mmu/Makefile |1 +
  xen/arch/arm/arm32/mmu/mm.c |  301 
  xen/arch/arm/arm64/Makefile |2 +-
  xen/arch/arm/arm64/head.S   |  460 +-
  xen/arch/arm/arm64/mmu/Makefile |2 +
  xen/arch/arm/arm64/mmu/head.S   |  481 ++
  xen/arch/arm/arm64/{ => mmu}/mm.c   |   84 ++
  xen/arch/arm/include/asm/arm32/mm.h |1 +
  xen/arch/arm/include/asm/arm64/macros.h |   36 +
  xen/arch/arm/include/asm/config.h   |  132 +-
  xen/arch/arm/include/asm/mm.h   |   28 +-
  xen/arch/arm/include/asm/mmu/layout.h   |  146 ++
  xen/arch/arm/include/asm/mmu/mm.h   |   41 +
  xen/arch/arm/include/asm/mmu/p2m.h  |   18 +
  xen/arch/arm/include/asm/p2m.h  |   26 +-
  xen/arch/arm/include/asm/page.h |   15 -
  xen/arch/arm/include/asm/setup.h|5 +
  xen/arch/arm/kernel.c   |   28 -
  xen/arch/arm/mm.c   | 1212 ---
  xen/arch/arm/mmu/Makefile   |4 +
  xen/arch/arm/mmu/p2m.c  | 1736 +
  xen/arch/arm/mmu/pt.c   |  743 +
  xen/arch/arm/mmu/setup.c|  372 +
  xen/arch/arm/mmu/smpboot.c  |  121 ++
  xen/arch/arm/p2m.c  | 1837 +--
  xen/arch/arm/setup.c|  324 +---
  xen/arch/arm/smpboot.c  |4 +-
  xen/arch/arm/xen.lds.S  |1 +
  32 files changed, 4222 insertions(+), 3968 deletions(-)
  create mode 100644 xen/arch/arm/arm32/mmu/Makefile
  create mode 100644 xen/arch/arm/arm32/mmu/mm.c
  create mode 100644 xen/arch/arm/arm64/mmu/Makefile
  create mode 100644 xen/arch/arm/arm64/mmu/head.S
  rename xen/arch/arm/arm64/{ => mmu}/mm.c (60%)
  create mode 100644 xen/arch/arm/include/asm/mmu/layout.h
  create mode 100644 xen/arch/arm/include/asm/mmu/mm.h
  create mode 100644 xen/arch/arm/include/asm/mmu/p2m.h
  create mode 100644 xen/arch/arm/mmu/Makefile
  create mode 100644 xen/arch/arm/mmu/p2m.c
  create mode 100644 xen/arch/arm/mmu/pt.c
  create mode 100644 xen/arch/arm/mmu/setup.c
  create mode 100644 xen/arch/arm/mmu/smpboot.c

--
2.25.1

[qemu-mainline test] 182929: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-ws16-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-shadow 1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-vhd1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-raw   1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-coresched-i386-xl  1 build-check(1)   blocked  n/a
 test-amd64-coresched-amd64-xl  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-shadow1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 build-arm64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ws16-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked 
n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow  1 build-check(1) blocked n/a
 test-amd64-amd64-dom0pvh-xl-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-de

Re: [PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Michal Orzel]

Hi Julien,

On 11/09/2023 15:14, Julien Grall wrote:
> 
> 
> Hi,
> 
> On 11/09/2023 13:34, Michal Orzel wrote:
>> Host device tree nodes under /chosen with compatible string
>> "xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
>> meant to be exposed to hardware domain.
> 
> So, how dom0 is meant to discover the static event channel, shared
> memory it can use?

For static shared memory:
A node with this compatible is only meant for Xen since it contains information 
like host-guest mapping.
Xen creates a different node for guests under /reserved-memory.
Linux binding:
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt
Xen node generation:
https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;hb=HEAD#l1407

For static event channels:
This is a bit weird so let me put it in a different way.
1) Xen does not create any node for static evtchn for domU.
2) The booting.txt states:
There is no need to describe the static event channel info in the domU device
tree. Static event channels are only useful in fully static configurations,
and in those configurations, the domU device tree dynamically generated by Xen
is not needed.
3) The "xen,evtchn" property specifies the local port as well as phandle of 
domU node.
dom0 does not have access to domU nodes, therefore exposing a property with not 
existing phandle
makes me think that:
a) point 2) applies to dom0 as well and we should hide this node or
b) there is a missing property for both dom0 and domUs to tell them about 
static local port in a proper way

Exposing Xen specific evtchn node only to dom0 and not to domU with required 
information happen to be found as first value
in xen,evtchn is definitely not a proper solution. Also, there is no Linux 
binding for it.
All that makes me think that the author's idea was not to expose it.

~Michal

[XEN v1 4/4] xen/arm: traps.c: Enclose VMSA specific registers within CONFIG_MMU

[Ayan Kumar Halder]

The VMSA specific registers (ie TCR, TTBR0, TTBR1, VTTBR, etc) are valid when
MMU is used, thus we can enclose them with CONFIG_MMU.

Signed-off-by: Ayan Kumar Halder 
---
 xen/arch/arm/traps.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 46c9a4031b..83522fcc5a 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -698,8 +698,10 @@ static void __do_trap_serror(struct cpu_user_regs *regs, 
bool guest)
 struct reg_ctxt {
 /* Guest-side state */
 register_t sctlr_el1;
+#ifdef CONFIG_MMU
 register_t tcr_el1;
 uint64_t ttbr0_el1, ttbr1_el1;
+#endif
 #ifdef CONFIG_ARM_32
 uint32_t dfsr, ifsr;
 uint32_t dfar, ifar;
@@ -801,9 +803,11 @@ static void show_registers_32(const struct cpu_user_regs 
*regs,
 if ( guest_mode_on )
 {
 printk(" SCTLR: %"PRIregister"\n", ctxt->sctlr_el1);
+#ifdef CONFIG_MMU
 printk("   TCR: %"PRIregister"\n", ctxt->tcr_el1);
 printk(" TTBR0: %016"PRIx64"\n", ctxt->ttbr0_el1);
 printk(" TTBR1: %016"PRIx64"\n", ctxt->ttbr1_el1);
+#endif
 printk("  IFAR: %08"PRIx32", IFSR: %08"PRIx32"\n"
"  DFAR: %08"PRIx32", DFSR: %08"PRIx32"\n",
 #ifdef CONFIG_ARM_64
@@ -873,9 +877,11 @@ static void show_registers_64(const struct cpu_user_regs 
*regs,
 printk("   FAR_EL1: %016"PRIx64"\n", ctxt->far);
 printk("\n");
 printk(" SCTLR_EL1: %"PRIregister"\n", ctxt->sctlr_el1);
+#ifdef CONFIG_MMU
 printk("   TCR_EL1: %"PRIregister"\n", ctxt->tcr_el1);
 printk(" TTBR0_EL1: %016"PRIx64"\n", ctxt->ttbr0_el1);
 printk(" TTBR1_EL1: %016"PRIx64"\n", ctxt->ttbr1_el1);
+#endif
 printk("\n");
 }
 }
@@ -907,13 +913,15 @@ static void _show_registers(const struct cpu_user_regs 
*regs,
 show_registers_32(regs, ctxt, guest_mode_on, v);
 #endif
 }
+#ifdef CONFIG_MMU
 printk("  VTCR_EL2: %"PRIregister"\n", READ_SYSREG(VTCR_EL2));
 printk(" VTTBR_EL2: %016"PRIx64"\n", ctxt->vttbr_el2);
+printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
 printk("\n");
+#endif
 
 printk(" SCTLR_EL2: %"PRIregister"\n", READ_SYSREG(SCTLR_EL2));
 printk("   HCR_EL2: %"PRIregister"\n", READ_SYSREG(HCR_EL2));
-printk(" TTBR0_EL2: %016"PRIx64"\n", READ_SYSREG64(TTBR0_EL2));
 printk("\n");
 printk("   ESR_EL2: %"PRIregister"\n", regs->hsr);
 printk(" HPFAR_EL2: %"PRIregister"\n", READ_SYSREG(HPFAR_EL2));
@@ -931,9 +939,13 @@ void show_registers(const struct cpu_user_regs *regs)
 {
 struct reg_ctxt ctxt;
 ctxt.sctlr_el1 = READ_SYSREG(SCTLR_EL1);
+#ifdef CONFIG_MMU
 ctxt.tcr_el1 = READ_SYSREG(TCR_EL1);
 ctxt.ttbr0_el1 = READ_SYSREG64(TTBR0_EL1);
 ctxt.ttbr1_el1 = READ_SYSREG64(TTBR1_EL1);
+ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
+#endif
+
 #ifdef CONFIG_ARM_32
 ctxt.dfar = READ_CP32(DFAR);
 ctxt.ifar = READ_CP32(IFAR);
@@ -945,7 +957,6 @@ void show_registers(const struct cpu_user_regs *regs)
 if ( guest_mode(regs) && is_32bit_domain(current->domain) )
 ctxt.ifsr32_el2 = READ_SYSREG(IFSR32_EL2);
 #endif
-ctxt.vttbr_el2 = READ_SYSREG64(VTTBR_EL2);
 
 _show_registers(regs, &ctxt, guest_mode(regs), current);
 }
@@ -954,9 +965,11 @@ void vcpu_show_registers(const struct vcpu *v)
 {
 struct reg_ctxt ctxt;
 ctxt.sctlr_el1 = v->arch.sctlr;
+#ifdef CONFIG_MMU
 ctxt.tcr_el1 = v->arch.ttbcr;
 ctxt.ttbr0_el1 = v->arch.ttbr0;
 ctxt.ttbr1_el1 = v->arch.ttbr1;
+#endif
 #ifdef CONFIG_ARM_32
 ctxt.dfar = v->arch.dfar;
 ctxt.ifar = v->arch.ifar;
-- 
2.25.1

[XEN v1 3/4] xen/arm32: Split and move MMU-specific head.S to mmu/head.S

[Ayan Kumar Halder]

This is based on
"[PATCH v6 03/13] xen/arm64: Split and move MMU-specific head.S to mmu/head.S"

https://www.mail-archive.com/xen-devel@lists.xenproject.org/msg151920.html

Signed-off-by: Ayan Kumar Halder 
---
 xen/arch/arm/arm32/head.S   | 603 +---
 xen/arch/arm/arm32/mmu/Makefile |   1 +
 xen/arch/arm/arm32/mmu/head.S   | 556 ++
 xen/arch/arm/include/asm/arm32/macros.h |  62 +++
 4 files changed, 620 insertions(+), 602 deletions(-)
 create mode 100644 xen/arch/arm/arm32/mmu/head.S

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index c0c425eac6..77095152b9 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -22,86 +22,10 @@
 
 #define ZIMAGE_MAGIC_NUMBER 0x016f2818
 
-#define PT_PT 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
-#define PT_MEM0xf7d /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=0 P=1 */
-#define PT_MEM_L3 0xf7f /* nG=1 AF=1 SH=11 AP=01 NS=1 ATTR=111 T=1 P=1 */
-#define PT_DEV0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */
-#define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
-
-#define PT_UPPER(x) (PT_##x & 0xf00)
-#define PT_LOWER(x) (PT_##x & 0x0ff)
-
-/* Convenience defines to get slot used by Xen mapping. */
-#define XEN_FIRST_SLOT  first_table_offset(XEN_VIRT_START)
-#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
-
-/* Offset between the early boot xen mapping and the runtime xen mapping */
-#define XEN_TEMPORARY_OFFSET  (TEMPORARY_XEN_VIRT_START - XEN_VIRT_START)
-
 #if defined(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
 #include CONFIG_EARLY_PRINTK_INC
 #endif
 
-/*
- * Move an immediate constant into a 32-bit register using movw/movt
- * instructions.
- */
-.macro mov_w reg, word
-movw  \reg, #:lower16:\word
-movt  \reg, #:upper16:\word
-.endm
-
-/*
- * Pseudo-op for PC relative adr ,  where  is
- * within the range +/- 4GB of the PC.
- *
- * @dst: destination register
- * @sym: name of the symbol
- */
-.macro adr_l, dst, sym
-mov_w \dst, \sym - .Lpc\@
-.set  .Lpc\@, .+ 8  /* PC bias */
-add   \dst, \dst, pc
-.endm
-
-.macro load_paddr rb, sym
-mov_w \rb, \sym
-add   \rb, \rb, r10
-.endm
-
-/*
- * Flush local TLBs
- *
- * @tmp: Scratch register
- *
- * See asm/arm32/flushtlb.h for the explanation of the sequence.
- */
-.macro flush_xen_tlb_local tmp
-dsb   nshst
-mcr   CP32(\tmp, TLBIALLH)
-dsb   nsh
-isb
-.endm
-
-/*
- * Enforce Xen page-tables do not contain mapping that are both
- * Writable and eXecutables.
- *
- * This should be called on each secondary CPU.
- */
-.macro pt_enforce_wxn tmp
-mrc   CP32(\tmp, HSCTLR)
-orr   \tmp, \tmp, #SCTLR_Axx_ELx_WXN
-dsb
-mcr   CP32(\tmp, HSCTLR)
-/*
- * The TLBs may cache SCTLR_EL2.WXN. So ensure it is synchronized
- * before flushing the TLBs.
- */
-isb
-flush_xen_tlb_local \tmp
-.endm
-
 /*
  * Common register usage in this file:
  *   r0  -
@@ -121,38 +45,6 @@
  *   r14 - LR
  *   r15 - PC
  */
-#ifdef CONFIG_EARLY_PRINTK
-/*
- * Macro to print a string to the UART, if there is one.
- *
- * Clobbers r0 - r3
- */
-#define PRINT(_s)   \
-mov   r3, lr   ;\
-adr_l r0, 98f  ;\
-blputs ;\
-mov   lr, r3   ;\
-RODATA_STR(98, _s)
-
-/*
- * Macro to print the value of register \rb
- *
- * Clobbers r0 - r4
- */
-.macro print_reg rb
-mov   r0, \rb
-mov   r4, lr
-blputn
-mov   lr, r4
-.endm
-
-#else /* CONFIG_EARLY_PRINTK */
-#define PRINT(s)
-
-.macro print_reg rb
-.endm
-
-#endif /* !CONFIG_EARLY_PRINTK */
 
 .section .text.header, "ax", %progbits
 .arm
@@ -354,467 +246,6 @@ cpu_init_done:
 mov   pc, r5/* Return address is in r5 */
 ENDPROC(cpu_init)
 
-/*
- * Macro to find the slot number at a given page-table level
- *
- * slot: slot computed
- * virt: virtual address
- * lvl:  page-table level
- *
- * Note that ubxf is unpredictable when the end bit is above 32-bit. So we
- * can't use it for first level offset.
- */
-.macro get_table_slot, slot, virt, lvl
-.if \lvl == 1
-lsr   \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl)
-.else
-ubfx  \slot, \virt, #XEN_PT_LEVEL_SHIFT(\lvl), #XEN_PT_LPAE_SHIFT
-.endif
-.endm
-
-/*
- * Macro to create a page table entry in \ptbl to \tbl (physical
- * address)
- *
- * ptbl:table symbol where the entry will be created
- * tbl: physical address of the table to point to
- * virt:virtual address
- * lvl: page-table level
- *
- * Preserves \virt
- * Clobbers \tbl, r1 - r3
- *
- * Note that \tbl and \virt should be in a register other than r1 - r3
- */
-.macro create_table_entry_from_paddr, ptbl, tbl, virt, lvl
-get_table_slot r1, \virt, \lvl

[XEN v1 2/4] xen/arm32: head.S: Introduce enable_{boot,secondary}_cpu_mm()

[Ayan Kumar Halder]

This is based on:-
"[PATCH v6 01/13] xen/arm64: head.S: Introduce enable_{boot,secondary}_cpu_mm()"
https://www.mail-archive.com/xen-devel@lists.xenproject.org/msg151918.html

This is being done for Arm32 as MPU support will be added for Arm32 as well.

Signed-off-by: Ayan Kumar Halder 
---
 xen/arch/arm/arm32/head.S | 90 +--
 1 file changed, 67 insertions(+), 23 deletions(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 057c44a5a2..c0c425eac6 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -201,13 +201,10 @@ past_zImage:
 
 blcheck_cpu_mode
 blcpu_init
-blcreate_page_tables
+ldr   lr, =primary_switched
+b enable_boot_cpu_mm
 
-/* Address in the runtime mapping to jump to after the MMU is enabled 
*/
-mov_w lr, primary_switched
-b enable_mmu
 primary_switched:
-blsetup_fixmap
 #ifdef CONFIG_EARLY_PRINTK
 /* Use a virtual address to access the UART. */
 mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
@@ -249,27 +246,11 @@ GLOBAL(init_secondary)
 #endif
 blcheck_cpu_mode
 blcpu_init
-blcreate_page_tables
 
-/* Address in the runtime mapping to jump to after the MMU is enabled 
*/
 mov_w lr, secondary_switched
-b enable_mmu
-secondary_switched:
-/*
- * Non-boot CPUs need to move on to the proper pagetables, which were
- * setup in prepare_secondary_mm.
- *
- * XXX: This is not compliant with the Arm Arm.
- */
-mov_w r4, init_ttbr  /* VA of HTTBR value stashed by CPU 0 */
-ldrd  r4, r5, [r4]   /* Actual value */
-dsb
-mcrr  CP64(r4, r5, HTTBR)
-dsb
-isb
-flush_xen_tlb_local r0
-pt_enforce_wxn r0
+b enable_secondary_cpu_mm
 
+secondary_switched:
 #ifdef CONFIG_EARLY_PRINTK
 /* Use a virtual address to access the UART. */
 mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
@@ -692,6 +673,69 @@ ready_to_switch:
 mov   pc, lr
 ENDPROC(switch_to_runtime_mapping)
 
+/*
+ * Enable mm (turn on the data cache and the MMU) for secondary CPUs.
+ * The function will return to the virtual address provided in LR (e.g. the
+ * runtime mapping).
+ *
+ * Inputs:
+ *   lr : Virtual address to return to.
+ *
+ * Clobbers r0 - r6
+ */
+enable_secondary_cpu_mm:
+mov   r6, lr
+blcreate_page_tables
+
+mov_w lr, secondary_cpu_mmu_on
+b enable_mmu
+secondary_cpu_mmu_on:
+/*
+ * Non-boot CPUs need to move on to the proper pagetables, which were
+ * setup in prepare_secondary_mm.
+ *
+ * XXX: This is not compliant with the Arm Arm.
+ */
+mov_w r4, init_ttbr  /* VA of HTTBR value stashed by CPU 0 */
+ldrd  r4, r5, [r4]   /* Actual value */
+dsb
+mcrr  CP64(r4, r5, HTTBR)
+dsb
+isb
+flush_xen_tlb_local r0
+pt_enforce_wxn r0
+mov   lr, r6
+
+/* Return to the virtual address requested by the caller. */
+mov   pc, lr
+ENDPROC(enable_secondary_cpu_mm)
+
+/*
+ * Enable mm (turn on the data cache and the MMU) for the boot CPU.
+ * The function will return to the virtual address provided in LR (e.g. the
+ * runtime mapping).
+ *
+ * Inputs:
+ *   lr : Virtual address to return to.
+ *
+ * Clobbers r0 - r6
+ */
+enable_boot_cpu_mm:
+mov   r6, lr
+blcreate_page_tables
+
+/* Address in the runtime mapping to jump to after the MMU is enabled 
*/
+mov_w lr, boot_cpu_mmu_on
+b enable_mmu
+boot_cpu_mmu_on:
+blsetup_fixmap
+
+mov   lr, r6
+
+/* Return to the virtual address requested by the caller. */
+mov   pc, lr
+ENDPROC(enable_boot_cpu_mm)
+
 /*
  * Remove the 1:1 map from the page-tables. It is not easy to keep track
  * where the 1:1 map was mapped, so we will look for the top-level entry
-- 
2.25.1

[XEN v1 1/4] xen/arm: arm32: Move pt_enforce_wxn() so that it can be bundled with other MMU functionality

[Ayan Kumar Halder]

pt_enforce_wxn() is moved up (as the previous instruction ie mov_w has
no effect on this). This way it is grouped with other MMU related
functions so that they can be moved together in the subsequent patch.

Signed-off-by: Ayan Kumar Halder 
---
This is based on my comment on
"[PATCH v6 08/13] xen/arm: Fold mmu_init_secondary_cpu() to head.S".
This patch can be ignored if the comment is addressed.

 xen/arch/arm/arm32/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index c7b2efb8f0..057c44a5a2 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -268,12 +268,12 @@ secondary_switched:
 dsb
 isb
 flush_xen_tlb_local r0
+pt_enforce_wxn r0
 
 #ifdef CONFIG_EARLY_PRINTK
 /* Use a virtual address to access the UART. */
 mov_w r11, EARLY_UART_VIRTUAL_ADDRESS
 #endif
-pt_enforce_wxn r0
 PRINT("- Ready -\r\n")
 /* Jump to C world */
 mov_w r2, start_secondary
-- 
2.25.1

[XEN v1 0/4] xen/arm: Split MMU code as the prepration of MPU work form Arm32

[Ayan Kumar Halder]

Hi,

These are the set of patches based on top of
"[PATCH v6 00/13] xen/arm: Split MMU code as the prepration of MPU work".
These patches aim to do similar MMU rework for Arm32.

Similar to the former series, this is the preparation work to add MPU
support for Arm32.

Ayan Kumar Halder (4):
  xen/arm: arm32: Move pt_enforce_wxn() so that it can be bundled with
other MMU functionality
  xen/arm32: head.S: Introduce enable_{boot,secondary}_cpu_mm()
  xen/arm32: Split and move MMU-specific head.S to mmu/head.S
  xen/arm: traps.c: Enclose VMSA specific registers within CONFIG_MMU

 xen/arch/arm/arm32/head.S   | 567 +---
 xen/arch/arm/arm32/mmu/Makefile |   1 +
 xen/arch/arm/arm32/mmu/head.S   | 556 +++
 xen/arch/arm/include/asm/arm32/macros.h |  62 +++
 xen/arch/arm/traps.c|  17 +-
 5 files changed, 639 insertions(+), 564 deletions(-)
 create mode 100644 xen/arch/arm/arm32/mmu/head.S

-- 
2.25.1

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

[Javi Merino]

On Mon, Sep 11, 2023 at 02:45:56PM +0100, Andrew Cooper wrote:
> On 11/09/2023 2:44 pm, Javi Merino wrote:
> > On Mon, Sep 11, 2023 at 11:52:43AM +0100, Andrew Cooper wrote:
> >> On 06/09/2023 2:14 pm, Javi Merino wrote:
> >>> 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
> >> This adds a new dependency on a package we (upstream Xen) don't
> >> currently use.  AFAICT, it's only for...
> >>
> >>> @@ -223,7 +232,7 @@ while not interrupted:
> >>>  last_tsc[cpu] = tsc
> >>>  
> >>>  if mhz:
> >>> -tsc = tsc / (mhz*100.0)
> >>> +tsc = old_div(tsc, (mhz*100.0))
> >> ... this, which is always int / float and doesn't fall into Py2's
> >> int/int behaviour in the first place.
> >>
> >> I'm pretty sure the code can just stay as it is, without needing to use
> >> old_div().
> > Ok, I will the dependency on python3 future and the old_div() for v2.
> 
> My point is that I don't think we need this dependency at all, and I
> don't think you need to change this line at all.

Yes.  And I forgot to add "remove" and the sentence made no sense.
What I meant was:

Ok, I will remove the dependency on python3 future and the old_div() for v2.

Cheers,
Javi

Re: [PATCH v4 2/5] xen/ppc: Implement bitops.h

[Jan Beulich]

On 09.09.2023 00:50, Shawn Anastasio wrote:
> +/**
> + * test_bit - Determine whether a bit is set
> + * @nr: bit number to test
> + * @addr: Address to start counting from
> + */
> +static inline int test_bit(int nr, const volatile void *addr)
> +{
> +const volatile unsigned long *p = (const volatile unsigned long *)addr;
> +return 1 & (p[BITOP_WORD(nr)] >> (nr & (BITOP_BITS_PER_WORD - 1)));
> +}

Do you really mean unsigned long in here? (As an aside I'd also recommend
to drop the cast; I don't think it's needed for anything.)

> +static inline unsigned long test_and_clear_bits(
> +unsigned long mask,

While apparently benign here, perhaps also better to use unsigned int.
(Looks like I even missed instances yet further up.)

> +volatile unsigned int *p)
> +{
> +unsigned long old, t;

I'm less certain here, because I don't know what exactly "r" permits on
ppc. (Having said that I notice that mask also is used with an "r"
constraint, so restrictions would then apply there as well. But if long
is really needed in various placed, then I would say that a comment is
warranted, perhaps next to the BITOP_* defines.)

> +asm volatile ( PPC_ATOMIC_ENTRY_BARRIER
> +   "1: lwarx %0,0,%3,0\n"
> +   "andc %1,%0,%2\n"
> +   "stwcx. %1,0,%3\n"
> +   "bne- 1b\n"
> +   PPC_ATOMIC_EXIT_BARRIER
> +   : "=&r" (old), "=&r" (t)
> +   : "r" (mask), "r" (p)
> +   : "cc", "memory" );
> +
> +return (old & mask);
> +}
> +
> +static inline int test_and_clear_bit(unsigned int nr,
> + volatile void *addr)
> +{
> +return test_and_clear_bits(BITOP_MASK(nr), (volatile unsigned int *)addr 
> +
> +   BITOP_WORD(nr)) != 0;

I think this is misleading wrapping of arguments. Even if not strictly
mandated, imo if an argument doesn't fit on the rest of a line it should
start on a fresh one.

Jan

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

[Andrew Cooper]

On 11/09/2023 2:44 pm, Javi Merino wrote:
> On Mon, Sep 11, 2023 at 11:52:43AM +0100, Andrew Cooper wrote:
>> On 06/09/2023 2:14 pm, Javi Merino wrote:
>>> 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
>> This adds a new dependency on a package we (upstream Xen) don't
>> currently use.  AFAICT, it's only for...
>>
>>> @@ -223,7 +232,7 @@ while not interrupted:
>>>  last_tsc[cpu] = tsc
>>>  
>>>  if mhz:
>>> -tsc = tsc / (mhz*100.0)
>>> +tsc = old_div(tsc, (mhz*100.0))
>> ... this, which is always int / float and doesn't fall into Py2's
>> int/int behaviour in the first place.
>>
>> I'm pretty sure the code can just stay as it is, without needing to use
>> old_div().
> Ok, I will the dependency on python3 future and the old_div() for v2.

My point is that I don't think we need this dependency at all, and I
don't think you need to change this line at all.

~Andrew

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

[Javi Merino]

On Mon, Sep 11, 2023 at 11:52:43AM +0100, Andrew Cooper wrote:
> On 06/09/2023 2:14 pm, Javi Merino wrote:
> > 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
> 
> This adds a new dependency on a package we (upstream Xen) don't
> currently use.  AFAICT, it's only for...
>
> > @@ -223,7 +232,7 @@ while not interrupted:
> >  last_tsc[cpu] = tsc
> >  
> >  if mhz:
> > -tsc = tsc / (mhz*100.0)
> > +tsc = old_div(tsc, (mhz*100.0))
> 
> ... this, which is always int / float and doesn't fall into Py2's
> int/int behaviour in the first place.
> 
> I'm pretty sure the code can just stay as it is, without needing to use
> old_div().

Ok, I will the dependency on python3 future and the old_div() for v2.

Cheers,
Javi

Re: [PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Julien Grall]

Hi,

On 11/09/2023 13:34, Michal Orzel wrote:

Host device tree nodes under /chosen with compatible string
"xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
meant to be exposed to hardware domain.


So, how dom0 is meant to discover the static event channel, shared 
memory it can use?



The same applies to
"xen,static-heap" property. Skip them from being included into hwdom
/chosen node.


I agree with hiding "xen,static-heap" property.

Cheers,

--
Julien Grall

Re: [PATCH] tools/misc/xencov_split: Add python 3 compatibility

[Javi Merino]

On Mon, Sep 11, 2023 at 11:40:59AM +0100, Anthony PERARD wrote:
> On Sat, Sep 02, 2023 at 05:21:08PM +0100, Javi Merino wrote:
> > diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
> > index e4f68ebb6e..8f1271bfe7 100755
> > --- a/tools/misc/xencov_split
> > +++ b/tools/misc/xencov_split
> > @@ -1,5 +1,7 @@
> > -#!/usr/bin/env python
> > +#!/usr/bin/env python3
> 
> Beside this shebang change, the patch looks good.
> With the shebang change reverted: Acked-by: Anthony PERARD 
> 

In v2 I did not change the shebang:
https://lore.kernel.org/xen-devel/20230905201653.98425-1-javi.mer...@cloud.com/

Cheers,
Javi

[xen-unstable test] 182912: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  49b2d6a529122f10e17a17f807d8d51154b5ba14
baseline version:
 xen  49b2d6a529122f10e17a17f807d8d51154b5ba14

Last test of basis   182912  2023-09-11 01:52:03 Z0 days
Testing same since  (not found) 0 attempts

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

[PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Michal Orzel]

Host device tree nodes under /chosen with compatible string
"xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
meant to be exposed to hardware domain. The same applies to
"xen,static-heap" property. Skip them from being included into hwdom
/chosen node.

Signed-off-by: Michal Orzel 
---
 xen/arch/arm/domain_build.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 29dcbb8a2ee6..413568c0e2fd 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1149,7 +1149,7 @@ static int __init write_properties(struct domain *d, 
struct kernel_info *kinfo,
  * * remember xen,dom0-bootargs if we don't already have
  *   bootargs (from module #1, above).
  * * remove bootargs,  xen,dom0-bootargs, xen,xen-bootargs,
- *   linux,initrd-start and linux,initrd-end.
+ *   xen,static-heap, linux,initrd-start and linux,initrd-end.
  * * remove stdout-path.
  * * remove bootargs, linux,uefi-system-table,
  *   linux,uefi-mmap-start, linux,uefi-mmap-size,
@@ -1158,7 +1158,8 @@ static int __init write_properties(struct domain *d, 
struct kernel_info *kinfo,
  */
 if ( dt_node_path_is_equal(node, "/chosen") )
 {
-if ( dt_property_name_is_equal(prop, "xen,xen-bootargs") ||
+if ( dt_property_name_is_equal(prop, "xen,static-heap") ||
+ dt_property_name_is_equal(prop, "xen,xen-bootargs") ||
  dt_property_name_is_equal(prop, "linux,initrd-start") ||
  dt_property_name_is_equal(prop, "linux,initrd-end") ||
  dt_property_name_is_equal(prop, "stdout-path") ||
@@ -2300,6 +2301,8 @@ static int __init handle_node(struct domain *d, struct 
kernel_info *kinfo,
 static const struct dt_device_match skip_matches[] __initconst =
 {
 DT_MATCH_COMPATIBLE("xen,domain"),
+DT_MATCH_COMPATIBLE("xen,domain-shared-memory-v1"),
+DT_MATCH_COMPATIBLE("xen,evtchn-v1"),
 DT_MATCH_COMPATIBLE("xen,xen"),
 DT_MATCH_COMPATIBLE("xen,multiboot-module"),
 DT_MATCH_COMPATIBLE("multiboot,module"),
-- 
2.25.1

[qemu-mainline bisection] complete build-i386-xsm

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job build-i386-xsm
testid xen-build

Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu https://gitlab.com/qemu-project/qemu.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  qemuu https://gitlab.com/qemu-project/qemu.git
  Bug introduced:  ca056f4499c259c0de68ed7cefad7ee7b62bfa43
  Bug not present: a5e9fbf1dfd5b3f901f987755c89f1dc636c3747
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/182938/


  commit ca056f4499c259c0de68ed7cefad7ee7b62bfa43
  Author: Paolo Bonzini 
  Date:   Wed May 3 12:48:02 2023 +0200
  
  Python: Drop support for Python 3.7
  
  Debian 10 is not anymore a supported distro, since Debian 12 was
  released on June 10, 2023.  Our supported build platforms as of today
  all support at least 3.8 (and all of them except for Ubuntu 20.04
  support 3.9):
  
  openSUSE Leap 15.5: 3.6.15 (3.11.2)
  CentOS Stream 8:3.6.8  (3.8.13, 3.9.16, 3.11.4)
  CentOS Stream 9:3.9.17 (3.11.4)
  Fedora 37:  3.11.4
  Fedora 38:  3.11.4
  Debian 11:  3.9.2
  Debian 12:  3.11.2
  Alpine 3.14, 3.15:  3.9.16
  Alpine 3.16, 3.17:  3.10.10
  Ubuntu 20.04 LTS:   3.8.10
  Ubuntu 22.04 LTS:   3.10.12
  NetBSD 9.3: 3.9.13*
  FreeBSD 12.4:   3.9.16
  FreeBSD 13.1:   3.9.18
  OpenBSD 7.2:3.9.17
  
  Note: NetBSD does not appear to have a default meta-package, but offers
  several options, the lowest of which is 3.7.15. However, "python39"
  appears to be a pre-requisite to one of the other packages we request
  in tests/vm/netbsd.
  
  Since it is safe under our supported platform policy, bump our
  minimum supported version of Python to 3.8.  The two most interesting
  features to have by default include:
  
  - the importlib.metadata module, whose lack is responsible for over 100
lines of code in mkvenv.py
  
  - improvements to asyncio, for example asyncio.CancelledError
inherits from BaseException rather than Exception
  
  In addition, code can now use the assignment operator ':='
  
  Because mypy now learns about importlib.metadata, a small change to
  mkvenv.py is needed to pass type checking.
  
  Signed-off-by: Paolo Bonzini 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu-mainline/build-i386-xsm.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/qemu-mainline/build-i386-xsm.xen-build 
--summary-out=tmp/182938.bisection-summary --basis-template=182707 
--blessings=real,real-bisect,real-retry qemu-mainline build-i386-xsm xen-build
Searching for failure / basis pass:
 182914 fail [host=albana0] / 182707 [host=pinot1] 182638 [host=fiano0] 182606 
[host=nobling0] 182588 [host=nobling0] 182584 [host=albana1] 182570 
[host=nobling1] 182555 [host=nobling0] 182543 [host=albana1] 182520 
[host=nobling1] 182508 [host=fiano1] 182422 [host=albana1] 182355 
[host=nobling1] 182338 [host=nobling1] 182310 ok.
Failure / basis pass flights: 182914 / 182310
(tree with no url: minios)
Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu https://gitlab.com/qemu-project/qemu.git
Tree: seabios git://xenbits.xen.org/osstest/seabios.git
Tree: xen git://xenbits.xen.org/xen.git
Latest b240eab03530f063ef5438497d70a731b19a201e 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
c5ea91da443b458352c1b629b490ee6631775cb4 
7a4003be25eae462f3c3d8aad96b57e34dc0c2b8 
49b2d6a529122f10e17a17f807d8d51154b5ba14
Basis pass 991515a0583f65a64b3a6fa354409c64e670a762 
3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 
bb5f142cb320d45d3d8dee2c82dae003cad39da8 
1281e340ad1d90c0cc8e8d902bb34f1871eb48cf 
a9a3b432a87b27e4debd6f80f14e2cfc9f52218c
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/osstest/ovmf.git#991515a0583f65a64b3a6fa354409c64e670a762-b240eab03530f063ef5438497d70a731b19a201e
 
git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c7437ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764
 
https://gitlab.com/qemu-project/qemu.git#bb5f142cb320d45d3d8dee2c82dae003cad39da8-c5ea91da443b458352c1b629b490ee6631775cb4
 git://xenbits.xen.org/osstest/seabios.git#1281e340ad1d90c0cc8e8d902bb34f1\
 871eb48cf-7a4003be25eae462f3c3d8aad96b57e34dc0c2b8 
git://xenbits.xen.org/xen.git#a9a3b432a87b27e4debd6f80f14e2cfc9f52218c-49b2d6a529122f10e17a17f807d8d51154b5ba14
Loaded 30516 nodes in revision graph
Searching for test results:
 182298 [host=huxelrebe0]
 182310 pass 991515a0583f

Re: [PATCH v4 09/10] xen/arm: fix duplicate /reserved-memory node in Dom0

[Michal Orzel]

Hi Penny,

On 11/09/2023 12:39, Penny Zheng wrote:
> 
> 
> Hi Michal
> 
> On 2023/9/11 17:40, Michal Orzel wrote:
>> Hi Penny,
>>
>> On 11/09/2023 06:04, Penny Zheng wrote:
>>>
>>>
>>> In case there is a /reserved-memory node already present in the host dtb,
>>> current Xen codes would create yet another /reserved-memory node specially
>> s/codes/code/
>>
>>> for the static shm in Dom0 Device Tree.
>>>
>>> Xen will use write_properties() to copy the reserved memory nodes from host 
>>> dtb
>>> to Dom0 FDT, so we want to insert the shm node along with the copying.
>>> And avoiding duplication, we add a checking before make_resv_memory_node().
>>>
>>> Signed-off-by: Penny Zheng 
>>>
>>> ---
>>> v3 -> v4:
>>> new commit
>>> ---
>>>   xen/arch/arm/domain_build.c   | 31 ---
>>>   xen/arch/arm/include/asm/kernel.h |  2 ++
>>>   2 files changed, 30 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index 02d903be78..dad234e4b5 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -1401,6 +1401,10 @@ static int __init handle_linux_pci_domain(struct 
>>> kernel_info *kinfo,
>>>   return fdt_property_cell(kinfo->fdt, "linux,pci-domain", segment);
>>>   }
>>>
>>> +static int __init make_shm_memory_node(const struct domain *d,
>>> +   void *fdt,
>>> +   int addrcells, int sizecells,
>>> +   const struct kernel_info *kinfo);
>>>   static int __init write_properties(struct domain *d, struct kernel_info 
>>> *kinfo,
>>>  const struct dt_device_node *node)
>>>   {
>>> @@ -1549,6 +1553,23 @@ static int __init write_properties(struct domain *d, 
>>> struct kernel_info *kinfo,
>>>   }
>>>   }
>>>
>>> +if ( dt_node_path_is_equal(node, "/reserved-memory") )
>>> +{
>>> +kinfo->resv_mem = true;
>> kinfo structure is used to store per-domain parameters and presence of 
>> reserved memory in host dtb
>> does not fit into this. Moreover, there is no need to introduce yet another 
>> flag for that given
>> that in other parts of the code in Xen we use bootinfo.reserved_mem.nr_banks 
>> to check if there are
>> reserved regions.
>>
>>> +
>>> +/* shared memory provided. */
>>> +if ( kinfo->shminfo.nr_banks != 0 )
>>> +{
>>> +uint32_t addrcells = dt_n_addr_cells(node),
>>> + sizecells = dt_n_size_cells(node);
>>> +
>>> +res = make_shm_memory_node(d, kinfo->fdt,
>>> +   addrcells, sizecells, kinfo);
>> I haven't yet looked at previous patches but does it make sense to request 
>> passing both kinfo->fdt and kinfo?
>> IMO, the latter would be sufficient. This would apply to other functions you 
>> modified.
>>
> 
> yes, the latter would be sufficient. I'll fix it in next version.
> 
> 
>>> +if ( res )
>>> +return res;
>>> +}
>>> +}
>>> +
>>>   return 0;
>>>   }
>>>
>>> @@ -2856,9 +2877,13 @@ static int __init handle_node(struct domain *d, 
>>> struct kernel_info *kinfo,
>>>   return res;
>>>   }
>>>
>>> -res = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, 
>>> kinfo);
>>> -if ( res )
>>> -return res;
>>> +/* Avoid duplicate /reserved-memory nodes in Device Tree */
>>> +if ( !kinfo->resv_mem )
>> No need for a new flag as mentioned above. Just before this line of code 
>> there is a check:
>> if ( bootinfo.reserved_mem.nr_banks > 0 )
>> {
>>  res = make_memory_node(d, kinfo->fdt, addrcells, sizecells,
>>  &bootinfo.reserved_mem);
>>  if ( res )
>>  return res;
>> }
>> so you can just append the following:
>> else
>> {
>>  res = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
>>  if ( res )
>>  return res;
>> }
>> to achieve the same without a need for a new flag.
> 
> 
> Right now, bootinfo.reserved_mem is not only containing reserved regions
> described in host FDT /reserved-memory, but also the reserved ones for
> domain only, like in xen,static-mem = .
> So simply with non-zero bootinfo.reserved_mem.nr_banks, we couldn't tell
> whether we had a /reserved-memory node in host FDT.
> 
> I agree that kinfo is not a good place to store whether host FDT has a
> /reserved-memory node. Maybe bootinfo.has_resv_memory_node?
Yes, I see. So we have 2 options:
1) Introduce flag under bootinfo (just like static_heap)
2) Inside make_resv_memory_node(), do a check e.g.:
for ( i = 0; i < bootinfo.reserved_mem.nr_banks && 
(bootinfo.reserved_mem.bank[i].type == MEMBANK_DEFAULT); i++ );
to see if there is a /reserved-memory node (i > 0).

~Michal

Re: [PATCH v4 1/5] xen/ppc: Implement atomic.h

[Jan Beulich]

On 09.09.2023 00:50, Shawn Anastasio wrote:
> Implement atomic.h for PPC, based off of the original Xen 3.2
> implementation. This implementation depends on some functions that are
> not yet defined (notably __cmpxchg), so it can't yet be used.
> 
> Signed-off-by: Shawn Anastasio 

Acked-by: Jan Beulich 

Just one remaining question:

> +static always_inline void read_atomic_size(const volatile void *p, void *res,
> +   unsigned int size)
> +{
> +ASSERT(IS_ALIGNED((vaddr_t)p, size));
> +switch ( size )
> +{
> +case 1:
> +*(uint8_t *)res = read_u8_atomic(p);
> +break;
> +case 2:
> +*(uint16_t *)res = read_u16_atomic(p);
> +break;
> +case 4:
> +*(uint32_t *)res = read_u32_atomic(p);
> +break;
> +case 8:
> +*(uint64_t *)res = read_u64_atomic(p);
> +break;
> +default:
> +__bad_atomic_read(p, res);
> +break;
> +}
> +}
> +
> +static always_inline void write_atomic_size(volatile void *p, const void 
> *val,
> +unsigned int size)
> +{
> +ASSERT(IS_ALIGNED((vaddr_t)p, size));
> +switch ( size )
> +{
> +case 1:
> +write_u8_atomic(p, *(const uint8_t *)val);
> +break;
> +case 2:
> +write_u16_atomic(p, *(const uint16_t *)val);
> +break;
> +case 4:
> +write_u32_atomic(p, *(const uint32_t *)val);
> +break;
> +case 8:
> +write_u64_atomic(p, *(const uint64_t *)val);
> +break;
> +default:
> +__bad_atomic_size();
> +break;
> +}
> +}

These two functions being as similar as is possible, ...

> +#define read_atomic(p)   
>   \
> +({   
>   \
> +union {  
>   \
> +typeof(*(p)) val;
>   \
> +char c[sizeof(*(p))];
>   \
> +} x_;
>   \
> +read_atomic_size(p, x_.c, sizeof(*(p))); 
>   \
> +x_.val;  
>   \
> +})

... is there a reason you keep using a union here ...

> +#define write_atomic(p, x)   
>   \
> +do   
>   \
> +{
>   \
> +typeof(*(p)) x_ = (x);   
>   \
> +write_atomic_size(p, &x_, sizeof(*(p))); 
>   \
> +} while ( 0 )

... while you did away with its use here?

Jan

Re: [PATCH 6/7] block: Clean up local variable shadowing

[Ilya Dryomov]

On Thu, Aug 31, 2023 at 3:25 PM Markus Armbruster  wrote:
>
> Local variables shadowing other local variables or parameters make the
> code needlessly hard to understand.  Tracked down with -Wshadow=local.
> Clean up: delete inner declarations when they are actually redundant,
> else rename variables.

For rbd

>  block/rbd.c  |  2 +-

Acked-by: Ilya Dryomov 

Thanks,

Ilya

Re: [PATCH v6 3/4] xen/vpci: header: status register handler

[Jan Beulich]

On 09.09.2023 04:16, Stewart Hildebrand wrote:
> @@ -544,6 +545,18 @@ static int cf_check init_bars(struct pci_dev *pdev)
>  if ( rc )
>  return rc;
>  
> +/*
> + * If mask_cap_list is true, PCI_STATUS_CAP_LIST will be set in both
> + * rsvdz_mask and ro_mask, and thus will effectively behave as rsvdp
> + * (reserved/RAZ and preserved on write).
> + */
> +rc = vpci_add_register_mask(pdev->vpci, vpci_hw_read16, vpci_hw_write16,
> +PCI_STATUS, 2, header, PCI_STATUS_RSVDZ_MASK 
> |
> +(mask_cap_list ? PCI_STATUS_CAP_LIST : 0),
> +PCI_STATUS_RO_MASK, PCI_STATUS_RW1C_MASK);

While not formally disallowed by ./CODING_STYLE, I think this kind of argument
wrapping is bad practice. If an argument is too long for the current line, it
should start on a fresh one. Otherwise at the very least I think we'd want the
continuation to stand out, by parenthesizing the entire argument, thus leading
to one deeper indentation on the continuing line(s). (This may even be useful
to do when starting on a fresh line.)

> @@ -155,7 +165,8 @@ int vpci_add_register(struct vpci *vpci, vpci_read_t 
> *read_handler,
>  /* Some sanity checks. */
>  if ( (size != 1 && size != 2 && size != 4) ||
>   offset >= PCI_CFG_SPACE_EXP_SIZE || (offset & (size - 1)) ||
> - (!read_handler && !write_handler) )
> + (!read_handler && !write_handler) || (ro_mask & rw1c_mask) ||
> + (rsvdz_mask & rw1c_mask) )
>  return -EINVAL;

What about rsvdz_mask and ro_mask? They better wouldn't overlap either
(requiring an adjustment to the code you add to init_bars()).

> @@ -407,26 +439,25 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, 
> unsigned int size)
>  
>  /*
>   * Perform a maybe partial write to a register.
> - *
> - * Note that this will only work for simple registers, if Xen needs to
> - * trap accesses to rw1c registers (like the status PCI header register)
> - * the logic in vpci_write will have to be expanded in order to correctly
> - * deal with them.
>   */
>  static void vpci_write_helper(const struct pci_dev *pdev,
>const struct vpci_register *r, unsigned int 
> size,
>unsigned int offset, uint32_t data)
>  {
> +uint32_t val = 0;
> +
>  ASSERT(size <= r->size);
>  
> -if ( size != r->size )
> +if ( (size != r->size) || r->ro_mask )
>  {
> -uint32_t val;
> -
>  val = r->read(pdev, r->offset, r->private);
> +val &= ~r->rw1c_mask;
>  data = merge_result(val, data, size, offset);
>  }
>  
> +data &= ~r->rsvdz_mask & ~r->ro_mask;

~(r->rsvdz_mask | r->ro_mask) ? But maybe that's indeed just me ...

Jan

Re: [PATCH 2/2] tools/light: Revoke permissions when a PCI detach for HVM domain

[Anthony PERARD]

On Thu, Aug 24, 2023 at 06:27:12PM +0100, Julien Grall wrote:
> On 24/08/2023 17:58, Anthony PERARD wrote:
> > On Thu, Aug 24, 2023 at 05:46:45PM +0100, Julien Grall wrote:
> > > On 24/08/2023 17:34, Anthony PERARD wrote:
> > > > On Thu, Aug 24, 2023 at 12:15:39PM +0100, Julien Grall wrote:
> > > > > On 18/08/2023 18:04, Anthony PERARD wrote:
> > > > > > So, this new pci_revoke_permissions() function been place before
> > > > > > do_pci_remove() will make it harder to follow what do_pci_remove() 
> > > > > > does.
> > > > > > Does it need to be a separate function? Can't you inline it in
> > > > > > pci_remove_detached() ?
> > > > > 
> > > > > I decided to go with an inline function to avoid increasing the size 
> > > > > of
> > > > > pci_remove_detached() and also separate the logic from cleaning-up 
> > > > > QMP an
> > > > > resetting the PCI device.
> > > > 
> > > > It's fine to have a long function, if there's that much to do. You can
> > > > add a comment if you want to separate a bit from the rest.
> > > > 
> > > > Having a new function would be ok if it was used from different places,
> > > > or if it was needed for a new callback in the chain of callbacks of a
> > > > long-running operation.
> > > 
> > > I don't agree with this definition on when to create a new function. 
> > > Smaller
> > > functions help to logically split your code and also avoids to have to
> > > define 20 local variables right at the beginning of the function (this is
> > > what will happen with folding the function) among other advantages.
> > 
> > You can always create a new block {} in the function, if that help with
> > local variables.
> 
> I thought about it... But this is just a function in disguised with downside
> (the extra layer of indentation).
> 
> I was actually going to try the folding version. But given this suggestion,
> I am now struggling to understand why this is a problem to have a function
> only called once. This is fairly common in Xen Hypervisor and I can see at
> least one instance in libxl (see libxl_pci_assignable()). So what is the
> rationale behind this request?

It is to try to keep the code laid out in chronological order for this
async operation. See CODING_STYLE L153:

https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libs/light/CODING_STYLE;h=b1a6a45c74df083cdd793e5cb6a67a76cb5c1174;hb=refs/heads/master#l153

(libxl_pci_assignable() isn't a good example, as it should be part of
the API ;-) )

Cheers,

-- 
Anthony PERARD

Re: [PATCH v4 00/10] Follow-up static shared memory PART I

[Penny Zheng]

Hi Michal

On 2023/9/11 17:01, Michal Orzel wrote:

Hi Penny,

On 11/09/2023 06:04, Penny Zheng wrote:



There are some unsolving issues on current 4.17 static shared memory
feature[1], including:
- In order to avoid keeping growing 'membank', having the shared memory
info in separate structures is preferred.
- Missing implementation on having the host address optional in
"xen,shared-mem" property
- Removing static shared memory from extended regions
- Missing reference release on foreign superpage
- Fix duplicated /reserved-memory node on Dom0
- Missing static shm node declaration on guest /memory node
- Missing "xen,offset" feature, which is introduced in Linux DOC[2]

All above objects have been divided into two parts to complete. And this
patch serie is PART I.


Just like I pointed out in the previous revision, there is a gitlab CI failure 
on shared-memory jobs:
https://gitlab.com/xen-project/patchew/xen/-/pipelines/999098293
Did you change the interface that could lead to this (I cannot spot any change 
in the docs refinment) ?
No Xen logs meaning the early boot failure. Please check.



Soo sorry. I miss-looked that comment. I found that bug exists in 
bootfdt.c.

```
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 7d86dffd45..290dd27bf4 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -532,6 +532,8 @@ static int __init process_shm_node(const void *fdt, 
int node,

size, tot_size);
 return -EINVAL;
 }
+
+break;
 }
 else if ( paddr_assigned )
 {
```
I accidentally delete a `break;` here, and I will also comment in the 
related commit and fix in next version!



~Michal

Re: [PATCH v4 04/10] xen/arm: use paddr_assigned to indicate whether host address is provided

[Penny Zheng]

Hi,

On 2023/9/11 12:04, Penny Zheng wrote:

We use paddr_assigned to indicate whether host address is provided, by
checking the length of "xen,shared-mem" property.

The shm matching criteria shall also be adapt to cover the new scenario, by
adding when host address is not provided, if SHMID matches, the region size
must exactly match too.

During domain creation, right now, a static shared memory node could be
banked with a statically configured host memory bank, or a set of arbitrary
host memory banks allocated from heap. To cover both scenarios, we create
a new structure shm_meminfo. It is very alike meminfo, but with the maximum
array size being a smaller number NR_SHM_BANKS(16).
As "shm_meminfo" is also a new member of "enum meminfo_type", we shall implement
its own callback "retrieve_shm_meminfo" to have access to all MACRO
helpers, e.g. GET_MEMBANK(...).

Also, to make codes tidy and clear, we extract codes about parsing
"xen,shared-mem" property from function "process_shm" and move them into
a new helper "parse_shm_property".

Signed-off-by: Penny Zheng 
---
v1 -> v2
- In order to get allocated banked host memory info during domain creation,
we create a new structure shm_meminfo. It is very alike meminfo, with
the maximum array size being NR_SHM_BANKS. As shm_meminfo is a new
member of type meminfo_type, we shall implement its own callback
retrieve_shm_meminfo to have access to all MACRO helpers, e.g.
GET_MEMBANK(...)
- rename "acquire_shm_memnode" to "find_shm_memnode"
---
v2 -> v3
- rebase and no changes
---
v3 -> v4:
rebase and no change
---
  xen/arch/arm/bootfdt.c   | 100 ++-
  xen/arch/arm/domain_build.c  | 207 +--
  xen/arch/arm/include/asm/setup.h |   3 +
  3 files changed, 243 insertions(+), 67 deletions(-)

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 66ad3ab3db..7d86dffd45 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -21,7 +21,15 @@ static __initdata struct {
  unsigned int nr_nodes;
  struct {
  const struct shm_node *node;
-const struct membank *membank;
+/*
+ * For a static shared memory node, it is either banked with a reserved
+ * host memory bank, or arbitrary host memory which shall
+ * be allocated by Xen with a specified total size(tot_size).
+ */
+union {
+const struct membank *membank;
+paddr_t tot_size;
+};
  } shm_nodes[NR_MEM_BANKS];
  } shm_data;
  #endif
@@ -421,7 +429,7 @@ static int __init process_shm_node(const void *fdt, int 
node,
  paddr_t paddr, gaddr, size;
  unsigned int i;
  int len;
-bool owner = false;
+bool owner = false, paddr_assigned = true;
  const char *shm_id;
  
  if ( address_cells < 1 || size_cells < 1 )

@@ -462,7 +470,7 @@ static int __init process_shm_node(const void *fdt, int 
node,
  }
  
  /*

- * xen,shared-mem = ;
+ * xen,shared-mem = , and paddr could be optional
   * Memory region starting from physical address #paddr of #size shall
   * be mapped to guest physical address #gaddr as static shared memory
   * region.
@@ -473,16 +481,24 @@ static int __init process_shm_node(const void *fdt, int 
node,
  
  if ( len != dt_cells_to_size(address_cells + size_cells + address_cells) )

  {
+/* paddr is not provided in "xen,shared-mem" */
  if ( len == dt_cells_to_size(size_cells + address_cells) )
-printk("fdt: host physical address must be chosen by users at the 
moment.\n");
-
-printk("fdt: invalid `xen,shared-mem` property.\n");
-return -EINVAL;
+paddr_assigned = false;
+else
+{
+printk("fdt: invalid `xen,shared-mem` property.\n");
+return -EINVAL;
+}
  }
  
  cell = (const __be32 *)prop->data;

-device_tree_get_reg(&cell, address_cells, address_cells, &paddr, &gaddr);
-size = dt_next_cell(size_cells, &cell);
+if ( !paddr_assigned )
+device_tree_get_reg(&cell, address_cells, size_cells, &gaddr, &size);
+else
+{
+device_tree_get_reg(&cell, address_cells, address_cells, &paddr, 
&gaddr);
+size = dt_next_cell(size_cells, &cell);
+}
  
  if ( !size )

  {
@@ -495,23 +511,37 @@ static int __init process_shm_node(const void *fdt, int 
node,
  paddr_t bank_start = shm_data.shm_nodes[i].membank->start;
  paddr_t bank_size = shm_data.shm_nodes[i].membank->size;
  const char *bank_id = shm_data.shm_nodes[i].node->shm_id;
+paddr_t tot_size = shm_data.shm_nodes[i].tot_size;
  
  /*

   * Meet the following check:
+ * when host address is provided:
   * 1) The shm ID matches and the region exactly match
   * 2) The shm ID doesn't match and the region doesn't overlap
   * with an existing one
+ * when host address is not provided:
+ 

Re: [PATCH v4 09/10] xen/arm: fix duplicate /reserved-memory node in Dom0

[Penny Zheng]

Hi Michal

On 2023/9/11 17:40, Michal Orzel wrote:

Hi Penny,

On 11/09/2023 06:04, Penny Zheng wrote:



In case there is a /reserved-memory node already present in the host dtb,
current Xen codes would create yet another /reserved-memory node specially

s/codes/code/


for the static shm in Dom0 Device Tree.

Xen will use write_properties() to copy the reserved memory nodes from host dtb
to Dom0 FDT, so we want to insert the shm node along with the copying.
And avoiding duplication, we add a checking before make_resv_memory_node().

Signed-off-by: Penny Zheng 

---
v3 -> v4:
new commit
---
  xen/arch/arm/domain_build.c   | 31 ---
  xen/arch/arm/include/asm/kernel.h |  2 ++
  2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 02d903be78..dad234e4b5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1401,6 +1401,10 @@ static int __init handle_linux_pci_domain(struct 
kernel_info *kinfo,
  return fdt_property_cell(kinfo->fdt, "linux,pci-domain", segment);
  }

+static int __init make_shm_memory_node(const struct domain *d,
+   void *fdt,
+   int addrcells, int sizecells,
+   const struct kernel_info *kinfo);
  static int __init write_properties(struct domain *d, struct kernel_info 
*kinfo,
 const struct dt_device_node *node)
  {
@@ -1549,6 +1553,23 @@ static int __init write_properties(struct domain *d, 
struct kernel_info *kinfo,
  }
  }

+if ( dt_node_path_is_equal(node, "/reserved-memory") )
+{
+kinfo->resv_mem = true;

kinfo structure is used to store per-domain parameters and presence of reserved 
memory in host dtb
does not fit into this. Moreover, there is no need to introduce yet another 
flag for that given
that in other parts of the code in Xen we use bootinfo.reserved_mem.nr_banks to 
check if there are
reserved regions.


+
+/* shared memory provided. */
+if ( kinfo->shminfo.nr_banks != 0 )
+{
+uint32_t addrcells = dt_n_addr_cells(node),
+ sizecells = dt_n_size_cells(node);
+
+res = make_shm_memory_node(d, kinfo->fdt,
+   addrcells, sizecells, kinfo);

I haven't yet looked at previous patches but does it make sense to request passing 
both kinfo->fdt and kinfo?
IMO, the latter would be sufficient. This would apply to other functions you 
modified.



yes, the latter would be sufficient. I'll fix it in next version.



+if ( res )
+return res;
+}
+}
+
  return 0;
  }

@@ -2856,9 +2877,13 @@ static int __init handle_node(struct domain *d, struct 
kernel_info *kinfo,
  return res;
  }

-res = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, 
kinfo);
-if ( res )
-return res;
+/* Avoid duplicate /reserved-memory nodes in Device Tree */
+if ( !kinfo->resv_mem )

No need for a new flag as mentioned above. Just before this line of code there 
is a check:
if ( bootinfo.reserved_mem.nr_banks > 0 )
{
 res = make_memory_node(d, kinfo->fdt, addrcells, sizecells,
 &bootinfo.reserved_mem);
 if ( res )
 return res;
}
so you can just append the following:
else
{
 res = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
 if ( res )
 return res;
}
to achieve the same without a need for a new flag.



Right now, bootinfo.reserved_mem is not only containing reserved regions 
described in host FDT /reserved-memory, but also the reserved ones for 
domain only, like in xen,static-mem = .

So simply with non-zero bootinfo.reserved_mem.nr_banks, we couldn't tell
whether we had a /reserved-memory node in host FDT.

I agree that kinfo is not a good place to store whether host FDT has a
/reserved-memory node. Maybe bootinfo.has_resv_memory_node?



~Michal

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

[osstest service owner]

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

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  35949b8c58cb6c835da1fdd8843ed51727e654c0
baseline version:
 xen  49b2d6a529122f10e17a17f807d8d51154b5ba14

Last test of basis   182789  2023-09-08 23:00:26 Z2 days
Testing same since   182930  2023-09-11 08:00:26 Z0 days1 attempts


People who touched revisions under test:
  Federico Serafini 
  Jan Beulich 
  Oleksii Kurochko 
  Tamas K Lengyel 

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
   49b2d6a529..35949b8c58  35949b8c58cb6c835da1fdd8843ed51727e654c0 -> smoke

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

[Andrew Cooper]

On 06/09/2023 2:14 pm, Javi Merino wrote:
> 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

This adds a new dependency on a package we (upstream Xen) don't
currently use.  AFAICT, it's only for...

> @@ -223,7 +232,7 @@ while not interrupted:
>  last_tsc[cpu] = tsc
>  
>  if mhz:
> -tsc = tsc / (mhz*100.0)
> +tsc = old_div(tsc, (mhz*100.0))

... this, which is always int / float and doesn't fall into Py2's
int/int behaviour in the first place.

I'm pretty sure the code can just stay as it is, without needing to use
old_div().

~Andrew

Re: [XEN PATCH] x86/ACPI: Ignore entries marked as unusable when parsing MADT

[Jan Beulich]

On 11.09.2023 12:12, Simon Gaiser wrote:
> Up to version 6.2 Errata B [2] the ACPI spec only defines
> ACPI_MADT_ENABLE as:
> 
> If zero, this processor is unusable, and the operating system
> support will not attempt to use it.
> 
> The bit that later will be ACPI_MADT_ONLINE_CAPABLE is reserved with
> "Must be zero".
> 
> Version 6.3 [3] then adds ACPI_MADT_ONLINE_CAPABLE and changes the
> meaning of ACPI_MADT_ENABLE:
> 
> Enabled
> If this bit is set the processor is ready for use. If this bit
> is clear and the Online Capable bit is set, system hardware
> supports enabling this processor during OS runtime. If this bit
> is clear and the Online Capable bit is also clear, this
> processor is unusable, and OSPM shall ignore the contents of the
> Processor Local APIC Structure.
> 
> Online Capbable
> The information conveyed by this bit depends on the value of the
> Enabled bit. If the Enabled bit is set, this bit is reserved and
> must be zero. Otherwise, if this this bit is set, system
> hardware supports enabling this processor during OS runtime.
> 
> So with conforming firmwares it should be safe to simply ignore the
> entry if !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE
> 
> As a precaution against buggy firmwares this change, like Linux [4],
> ignores ACPI_MADT_ONLINE_CAPABLE completely if MADT revision < 5. Note
> that the MADT revision was already increased to 5 with spec version 6.2
> Errata A [1], so before introducing the online capable flag. But it
> wasn't changed for the new flag, so this is the best we can do here.
> 
> For previous discussion see thread [5].
> 
> Link: 
> http://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf # 
> [1]
> Link: 
> https://uefi.org/sites/default/files/resources/ACPI_6_2_B_final_Jan30.pdf # 
> [2]
> Link: https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf # [3]
> Link: 
> https://git.kernel.org/torvalds/c/e2869bd7af608c343988429ceb1c2fe99644a01f # 
> [4]
> Link: 
> https://lore.kernel.org/xen-devel/80bae614-052e-0f90-cf13-0e5e4ed1a...@invisiblethingslab.com/
>  # [5]
> Signed-off-by: Simon Gaiser 

Just to avoid misunderstandings: This change addresses a comment from
Andrew. I does not attempt to address my feedback on the earlier change,
which - as indicated - I intend to revert (timeline extended until mid
of the week), unless by then my earlier comments are addressed or the
suggested possible alternative is carried out.

I think it goes without saying that this patch can't sensibly go in
until the earlier one was properly settled upon. In particular, in case
of reverting aiui this patch won't even apply anymore.

Jan

Re: [PATCH 6/7] block: Clean up local variable shadowing

[Anthony PERARD]

On Thu, Aug 31, 2023 at 03:25:45PM +0200, Markus Armbruster wrote:
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index 3906b9058b..a07cd7eb5d 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -369,7 +369,7 @@ static void xen_block_get_vdev(Object *obj, Visitor *v, 
> const char *name,
>  case XEN_BLOCK_VDEV_TYPE_XVD:
>  case XEN_BLOCK_VDEV_TYPE_HD:
>  case XEN_BLOCK_VDEV_TYPE_SD: {
> -char *name = disk_to_vbd_name(vdev->disk);
> +char *vbd_name = disk_to_vbd_name(vdev->disk);
>  
>  str = g_strdup_printf("%s%s%lu",
>(vdev->type == XEN_BLOCK_VDEV_TYPE_XVD) ?
> @@ -377,8 +377,8 @@ static void xen_block_get_vdev(Object *obj, Visitor *v, 
> const char *name,
>(vdev->type == XEN_BLOCK_VDEV_TYPE_HD) ?
>"hd" :
>"sd",
> -  name, vdev->partition);
> -g_free(name);
> +  vbd_name, vdev->partition);
> +g_free(vbd_name);
>  break;
>  }
>  default:

Acked-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD

Re: [PATCH v2] tools/misc/xencov_split: Add python 3 compatibility

[Andrew Cooper]

On 05/09/2023 9:15 pm, Javi Merino wrote:
> Closes #154

This wants to be:

Resolves: xen-project/xen#154

I'll fix on commit.

> Signed-off-by: Javi Merino 

Acked-by: Andrew Cooper 

Re: [PATCH] tools/misc/xencov_split: Add python 3 compatibility

[Anthony PERARD]

On Sat, Sep 02, 2023 at 05:21:08PM +0100, Javi Merino wrote:
> diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
> index e4f68ebb6e..8f1271bfe7 100755
> --- a/tools/misc/xencov_split
> +++ b/tools/misc/xencov_split
> @@ -1,5 +1,7 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python3

Beside this shebang change, the patch looks good.
With the shebang change reverted: Acked-by: Anthony PERARD 


Thanks,

-- 
Anthony PERARD

Re: [XEN PATCH v2 1/2] coverage: simplify the logic of choosing the number of gcov counters depending on the gcc version

[Jan Beulich]

On 11.09.2023 11:59, Andrew Cooper wrote:
> On 11/09/2023 10:53 am, Jan Beulich wrote:
>> On 11.09.2023 11:48, Andrew Cooper wrote:
>>> On 11/09/2023 8:54 am, Jan Beulich wrote:
 On 08.09.2023 18:20, Javi Merino wrote:
> The current structure of choosing the correct file based on the
> compiler version makes us make 33 line files just to define a
> constant.  The changes after gcc 4.7 are minimal, just the number of
> counters.
>
> Fold the changes in gcc_4_9.c, gcc_5.c and gcc_7.c into gcc_4_7.c to
> remove a lot of the boilerplate and keep the logic of choosing the
> GCOV_COUNTER in gcc_4_7.c.
>
> Signed-off-by: Javi Merino 
> ---
>  xen/common/coverage/Makefile  |  6 +-
>  xen/common/coverage/gcc_4_7.c | 17 +
>  xen/common/coverage/gcc_4_9.c | 33 -
>  xen/common/coverage/gcc_5.c   | 33 -
>  xen/common/coverage/gcc_7.c   | 30 --
>  5 files changed, 10 insertions(+), 109 deletions(-)
>  delete mode 100644 xen/common/coverage/gcc_4_9.c
>  delete mode 100644 xen/common/coverage/gcc_5.c
>  delete mode 100644 xen/common/coverage/gcc_7.c
>
> diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile
> index 63f98c71d6..d729afc9c7 100644
> --- a/xen/common/coverage/Makefile
> +++ b/xen/common/coverage/Makefile
> @@ -1,11 +1,7 @@
>  obj-y += coverage.o
>  ifneq ($(CONFIG_CC_IS_CLANG),y)
>  obj-y += gcov_base.o gcov.o
> -obj-y += $(call cc-ifversion,-lt,0407, \
> - gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
> - gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
> - gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
> - gcc_5.o, gcc_7.o
> +obj-y += $(call cc-ifversion,-lt,0407, gcc_3_4.o, gcc_4_7.o)
>  else
>  obj-y += llvm.o
>  endif
> diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
> index 25b4a8bcdc..ddbc9f4bb0 100644
> --- a/xen/common/coverage/gcc_4_7.c
> +++ b/xen/common/coverage/gcc_4_7.c
> @@ -18,15 +18,16 @@
>  
>  #include "gcov.h"
>  
> -/*
> - * GCOV_COUNTERS will be defined if this file is included by other
> - * source files.
> - */
> -#ifndef GCOV_COUNTERS
> -# if !(GCC_VERSION >= 40700 && GCC_VERSION < 40900)
> -#  error "Wrong version of GCC used to compile gcov"
> -# endif
> +#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900)
>  #define GCOV_COUNTERS 8
> +#elif (GCC_VERSION >= 40900 && GCC_VERSION < 5)
> +#define GCOV_COUNTERS 9
> +#elif GCC_VERSION >= 5 && GCC_VERSION < 7
> +#define GCOV_COUNTERS 10
> +#elif GCC_VERSION >= 7
> +#define GCOV_COUNTERS 9
> +#else
> +#error "Wrong version of GCC used to compile gcov"
>  #endif
 How about inverse order:

 #if GCC_VERSION >= 7
 #define GCOV_COUNTERS 9
 #elif GCC_VERSION >= 5
 #define GCOV_COUNTERS 10
 #elif GCC_VERSION >= 40900
 #define GCOV_COUNTERS 9
 #elif GCC_VERSION >= 40700
 #define GCOV_COUNTERS 8
 #else
 #error "Wrong version of GCC used to compile gcov"
 #endif
>>> Forward order is the one that results in a smaller diff when inserting
>>> new entries.
>> Hmm, even in forward order one prior #elif will need touching
> 
>  #if GCC_VERSION < 40700
>  #define GCOV_COUNTERS 8
>  #elif GCC_VERSION < 40900
>  #define GCOV_COUNTERS 9
>  #elif GCC_VERSION < 5
>  #define GCOV_COUNTERS 10
>  #elif GCC_VERSION < 7
>  #define GCOV_COUNTERS 9
> +#elif GCC_VERSION < FOO
> +#define GCOV_COUNTERS BAR
>  #else
>  #error "Wrong version of GCC used to compile gcov"
>  #endif

But that doesn't achieve the same as the other construct: No #error for
gcc earlier than 4.7, and your (presumed; i.e. taking just patch context
without the two added lines) construct would fail to build for gcc 7. We
want an open upper bound but a determined lower one.

Jan

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

[Simon Gaiser]

Andrew Cooper:
> On 07/08/2023 3:45 pm, Simon Gaiser wrote:
>> Andrew Cooper:
>>> On 07/08/2023 10:38 am, 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.
>>> I recall there being a discussion over this, ultimately with the version
>>> check being removed.  IIRC it was a defacto standard used for a long
>>> time prior to actually getting written into the ACPI spec, so does exist
>>> in practice in older MADTs.
>> So I noticed that the check in Linux is actually slightly different than
>> I thought. Since [3] it always considers the CPU usable if
>> ACPI_MADT_ENABLED is set. Otherwise it consider it only usable if
>> MADT revision >= 5 and ACPI_MADT_ONLINE_CAPABLE is set.
>>
>> So I checked what the ACPI spec says:
>>
>> Up to 6.2 Errata B [6] it only defines ACPI_MADT_ENABLE as:
>>
>> If zero, this processor is unusable, and the operating system
>> support will not attempt to use it.
>>
>> And the bit that later will be ACPI_MADT_ONLINE_CAPABLE is reserved with
>> "Must be zero".
>>
>> 6.3 [7] Then adds ACPI_MADT_ONLINE_CAPABLE and changes the meaning of
>> ACPI_MADT_ENABLE:
>>
>> Enabled
>> If this bit is set the processor is ready for use. If this bit
>> is clear and the Online Capable bit is set, system hardware
>> supports enabling this processor during OS runtime. If this bit
>> is clear and the Online Capable bit is also clear, this
>> processor is unusable, and OSPM shall ignore the contents of the
>> Processor Local APIC Structure.
>>
>> Online Capbable
>> The information conveyed by this bit depends on the value of the
>> Enabled bit. If the Enabled bit is set, this bit is reserved and
>> must be zero. Otherwise, if this this bit is set, system
>> hardware supports enabling this processor during OS runtime.
>>
>> So with confirming firmwares it should be safe change the simply ignore
>> the entry if !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE
>>
>> We can also do it like Linux and ignore ACPI_MADT_ONLINE_CAPABLE
>> completely if revision < 5.
>>
>> Note that the revision was already increased to 5 before 6.3.
>>
>> ACPI spec versionMADT revision
>>   
>> 6.2 [4]  4
>> 6.2 Errata A [5] 45 (typo I guess)
>> 6.2 Errata B 5
>> 6.3  5
>>
>> [3]: 
>> https://git.kernel.org/torvalds/c/e2869bd7af608c343988429ceb1c2fe99644a01f
>> [4]: http://www.uefi.org/sites/default/files/resources/ACPI_6_2.pdf
>> [5]: 
>> http://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf
>> [6]: 
>> https://uefi.org/sites/default/files/resources/ACPI_6_2_B_final_Jan30.pdf
>> [7]: https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf
> 
> Honestly, the reserved must be zero means there's no need for a version
> check at all.  That bit will not be set even in older MADT revisions.
> 
> That said, it's likely easier to retain the version check than to set up
> a quirks infrastructure for buggy older MADTs.

Yes, that's what I was thinking too. Send a patch:
https://lore.kernel.org/xen-devel/20230911101238.18005-1-si...@invisiblethingslab.com/

Simon


OpenPGP_signature
Description: OpenPGP digital signature

[XEN PATCH] x86/ACPI: Ignore entries marked as unusable when parsing MADT

[Simon Gaiser]

Up to version 6.2 Errata B [2] the ACPI spec only defines
ACPI_MADT_ENABLE as:

If zero, this processor is unusable, and the operating system
support will not attempt to use it.

The bit that later will be ACPI_MADT_ONLINE_CAPABLE is reserved with
"Must be zero".

Version 6.3 [3] then adds ACPI_MADT_ONLINE_CAPABLE and changes the
meaning of ACPI_MADT_ENABLE:

Enabled
If this bit is set the processor is ready for use. If this bit
is clear and the Online Capable bit is set, system hardware
supports enabling this processor during OS runtime. If this bit
is clear and the Online Capable bit is also clear, this
processor is unusable, and OSPM shall ignore the contents of the
Processor Local APIC Structure.

Online Capbable
The information conveyed by this bit depends on the value of the
Enabled bit. If the Enabled bit is set, this bit is reserved and
must be zero. Otherwise, if this this bit is set, system
hardware supports enabling this processor during OS runtime.

So with conforming firmwares it should be safe to simply ignore the
entry if !ACPI_MADT_ENABLED && !ACPI_MADT_ONLINE_CAPABLE

As a precaution against buggy firmwares this change, like Linux [4],
ignores ACPI_MADT_ONLINE_CAPABLE completely if MADT revision < 5. Note
that the MADT revision was already increased to 5 with spec version 6.2
Errata A [1], so before introducing the online capable flag. But it
wasn't changed for the new flag, so this is the best we can do here.

For previous discussion see thread [5].

Link: http://www.uefi.org/sites/default/files/resources/ACPI%206_2_A_Sept29.pdf 
# [1]
Link: https://uefi.org/sites/default/files/resources/ACPI_6_2_B_final_Jan30.pdf 
# [2]
Link: https://uefi.org/sites/default/files/resources/ACPI_6_3_May16.pdf # [3]
Link: 
https://git.kernel.org/torvalds/c/e2869bd7af608c343988429ceb1c2fe99644a01f # [4]
Link: 
https://lore.kernel.org/xen-devel/80bae614-052e-0f90-cf13-0e5e4ed1a...@invisiblethingslab.com/
 # [5]
Signed-off-by: Simon Gaiser 
---
 xen/arch/x86/acpi/boot.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
index 4a62822fa9..2d0b8a9afc 100644
--- a/xen/arch/x86/acpi/boot.c
+++ b/xen/arch/x86/acpi/boot.c
@@ -77,6 +77,17 @@ static int __init cf_check acpi_parse_madt(struct 
acpi_table_header *table)
return 0;
 }
 
+static bool __init acpi_is_processor_usable(uint32_t lapic_flags)
+{
+   if (lapic_flags & ACPI_MADT_ENABLED)
+   return true;
+
+   if (madt_revision >= 5 && (lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
+   return true;
+
+   return false;
+}
+
 static int __init cf_check
 acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 {
@@ -92,9 +103,7 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const 
unsigned long end)
return 0;
 
/* Don't register processors that cannot be onlined. */
-   if (madt_revision >= 5 &&
-   !(processor->lapic_flags & ACPI_MADT_ENABLED) &&
-   !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
+   if (!acpi_is_processor_usable(processor->lapic_flags))
return 0;
 
if ((processor->lapic_flags & ACPI_MADT_ENABLED) || opt_cpu_info) {
@@ -151,9 +160,7 @@ acpi_parse_lapic(struct acpi_subtable_header * header, 
const unsigned long end)
return 0;
 
/* Don't register processors that cannot be onlined. */
-   if (madt_revision >= 5 &&
-   !(processor->lapic_flags & ACPI_MADT_ENABLED) &&
-   !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
+   if (!acpi_is_processor_usable(processor->lapic_flags))
return 0;
 
if ((processor->lapic_flags & ACPI_MADT_ENABLED) || opt_cpu_info)
-- 
2.40.1

Re: [XEN PATCH v2 1/2] coverage: simplify the logic of choosing the number of gcov counters depending on the gcc version

[Andrew Cooper]

On 11/09/2023 10:53 am, Jan Beulich wrote:
> On 11.09.2023 11:48, Andrew Cooper wrote:
>> On 11/09/2023 8:54 am, Jan Beulich wrote:
>>> On 08.09.2023 18:20, Javi Merino wrote:
 The current structure of choosing the correct file based on the
 compiler version makes us make 33 line files just to define a
 constant.  The changes after gcc 4.7 are minimal, just the number of
 counters.

 Fold the changes in gcc_4_9.c, gcc_5.c and gcc_7.c into gcc_4_7.c to
 remove a lot of the boilerplate and keep the logic of choosing the
 GCOV_COUNTER in gcc_4_7.c.

 Signed-off-by: Javi Merino 
 ---
  xen/common/coverage/Makefile  |  6 +-
  xen/common/coverage/gcc_4_7.c | 17 +
  xen/common/coverage/gcc_4_9.c | 33 -
  xen/common/coverage/gcc_5.c   | 33 -
  xen/common/coverage/gcc_7.c   | 30 --
  5 files changed, 10 insertions(+), 109 deletions(-)
  delete mode 100644 xen/common/coverage/gcc_4_9.c
  delete mode 100644 xen/common/coverage/gcc_5.c
  delete mode 100644 xen/common/coverage/gcc_7.c

 diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile
 index 63f98c71d6..d729afc9c7 100644
 --- a/xen/common/coverage/Makefile
 +++ b/xen/common/coverage/Makefile
 @@ -1,11 +1,7 @@
  obj-y += coverage.o
  ifneq ($(CONFIG_CC_IS_CLANG),y)
  obj-y += gcov_base.o gcov.o
 -obj-y += $(call cc-ifversion,-lt,0407, \
 -  gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
 -  gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
 -  gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
 -  gcc_5.o, gcc_7.o
 +obj-y += $(call cc-ifversion,-lt,0407, gcc_3_4.o, gcc_4_7.o)
  else
  obj-y += llvm.o
  endif
 diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
 index 25b4a8bcdc..ddbc9f4bb0 100644
 --- a/xen/common/coverage/gcc_4_7.c
 +++ b/xen/common/coverage/gcc_4_7.c
 @@ -18,15 +18,16 @@
  
  #include "gcov.h"
  
 -/*
 - * GCOV_COUNTERS will be defined if this file is included by other
 - * source files.
 - */
 -#ifndef GCOV_COUNTERS
 -# if !(GCC_VERSION >= 40700 && GCC_VERSION < 40900)
 -#  error "Wrong version of GCC used to compile gcov"
 -# endif
 +#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900)
  #define GCOV_COUNTERS 8
 +#elif (GCC_VERSION >= 40900 && GCC_VERSION < 5)
 +#define GCOV_COUNTERS 9
 +#elif GCC_VERSION >= 5 && GCC_VERSION < 7
 +#define GCOV_COUNTERS 10
 +#elif GCC_VERSION >= 7
 +#define GCOV_COUNTERS 9
 +#else
 +#error "Wrong version of GCC used to compile gcov"
  #endif
>>> How about inverse order:
>>>
>>> #if GCC_VERSION >= 7
>>> #define GCOV_COUNTERS 9
>>> #elif GCC_VERSION >= 5
>>> #define GCOV_COUNTERS 10
>>> #elif GCC_VERSION >= 40900
>>> #define GCOV_COUNTERS 9
>>> #elif GCC_VERSION >= 40700
>>> #define GCOV_COUNTERS 8
>>> #else
>>> #error "Wrong version of GCC used to compile gcov"
>>> #endif
>> Forward order is the one that results in a smaller diff when inserting
>> new entries.
> Hmm, even in forward order one prior #elif will need touching

 #if GCC_VERSION < 40700
 #define GCOV_COUNTERS 8
 #elif GCC_VERSION < 40900
 #define GCOV_COUNTERS 9
 #elif GCC_VERSION < 5
 #define GCOV_COUNTERS 10
 #elif GCC_VERSION < 7
 #define GCOV_COUNTERS 9
+#elif GCC_VERSION < FOO
+#define GCOV_COUNTERS BAR
 #else
 #error "Wrong version of GCC used to compile gcov"
 #endif

~Andrew

Re: [XEN PATCH v2 1/2] coverage: simplify the logic of choosing the number of gcov counters depending on the gcc version

[Jan Beulich]

On 11.09.2023 11:48, Andrew Cooper wrote:
> On 11/09/2023 8:54 am, Jan Beulich wrote:
>> On 08.09.2023 18:20, Javi Merino wrote:
>>> The current structure of choosing the correct file based on the
>>> compiler version makes us make 33 line files just to define a
>>> constant.  The changes after gcc 4.7 are minimal, just the number of
>>> counters.
>>>
>>> Fold the changes in gcc_4_9.c, gcc_5.c and gcc_7.c into gcc_4_7.c to
>>> remove a lot of the boilerplate and keep the logic of choosing the
>>> GCOV_COUNTER in gcc_4_7.c.
>>>
>>> Signed-off-by: Javi Merino 
>>> ---
>>>  xen/common/coverage/Makefile  |  6 +-
>>>  xen/common/coverage/gcc_4_7.c | 17 +
>>>  xen/common/coverage/gcc_4_9.c | 33 -
>>>  xen/common/coverage/gcc_5.c   | 33 -
>>>  xen/common/coverage/gcc_7.c   | 30 --
>>>  5 files changed, 10 insertions(+), 109 deletions(-)
>>>  delete mode 100644 xen/common/coverage/gcc_4_9.c
>>>  delete mode 100644 xen/common/coverage/gcc_5.c
>>>  delete mode 100644 xen/common/coverage/gcc_7.c
>>>
>>> diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile
>>> index 63f98c71d6..d729afc9c7 100644
>>> --- a/xen/common/coverage/Makefile
>>> +++ b/xen/common/coverage/Makefile
>>> @@ -1,11 +1,7 @@
>>>  obj-y += coverage.o
>>>  ifneq ($(CONFIG_CC_IS_CLANG),y)
>>>  obj-y += gcov_base.o gcov.o
>>> -obj-y += $(call cc-ifversion,-lt,0407, \
>>> -   gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
>>> -   gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
>>> -   gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
>>> -   gcc_5.o, gcc_7.o
>>> +obj-y += $(call cc-ifversion,-lt,0407, gcc_3_4.o, gcc_4_7.o)
>>>  else
>>>  obj-y += llvm.o
>>>  endif
>>> diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
>>> index 25b4a8bcdc..ddbc9f4bb0 100644
>>> --- a/xen/common/coverage/gcc_4_7.c
>>> +++ b/xen/common/coverage/gcc_4_7.c
>>> @@ -18,15 +18,16 @@
>>>  
>>>  #include "gcov.h"
>>>  
>>> -/*
>>> - * GCOV_COUNTERS will be defined if this file is included by other
>>> - * source files.
>>> - */
>>> -#ifndef GCOV_COUNTERS
>>> -# if !(GCC_VERSION >= 40700 && GCC_VERSION < 40900)
>>> -#  error "Wrong version of GCC used to compile gcov"
>>> -# endif
>>> +#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900)
>>>  #define GCOV_COUNTERS 8
>>> +#elif (GCC_VERSION >= 40900 && GCC_VERSION < 5)
>>> +#define GCOV_COUNTERS 9
>>> +#elif GCC_VERSION >= 5 && GCC_VERSION < 7
>>> +#define GCOV_COUNTERS 10
>>> +#elif GCC_VERSION >= 7
>>> +#define GCOV_COUNTERS 9
>>> +#else
>>> +#error "Wrong version of GCC used to compile gcov"
>>>  #endif
>> How about inverse order:
>>
>> #if GCC_VERSION >= 7
>> #define GCOV_COUNTERS 9
>> #elif GCC_VERSION >= 5
>> #define GCOV_COUNTERS 10
>> #elif GCC_VERSION >= 40900
>> #define GCOV_COUNTERS 9
>> #elif GCC_VERSION >= 40700
>> #define GCOV_COUNTERS 8
>> #else
>> #error "Wrong version of GCC used to compile gcov"
>> #endif
> 
> Forward order is the one that results in a smaller diff when inserting
> new entries.

Hmm, even in forward order one prior #elif will need touching (to amend
the version check), so I'm afraid I don't see such a diff being smaller
(it's uniformly -1/+3 afaict).

> More importantly, it's the more natural way to structure such a list.

I would say multiple views are possible: Naming recent compiler versions
first may also be viewed as beneficial. In the end what counts is how
easy it is to follow the overall construct. And I'm pretty sure less
complex expressions help there.

Jan

Re: [XEN PATCH v2 1/2] coverage: simplify the logic of choosing the number of gcov counters depending on the gcc version

[Andrew Cooper]

On 11/09/2023 8:54 am, Jan Beulich wrote:
> On 08.09.2023 18:20, Javi Merino wrote:
>> The current structure of choosing the correct file based on the
>> compiler version makes us make 33 line files just to define a
>> constant.  The changes after gcc 4.7 are minimal, just the number of
>> counters.
>>
>> Fold the changes in gcc_4_9.c, gcc_5.c and gcc_7.c into gcc_4_7.c to
>> remove a lot of the boilerplate and keep the logic of choosing the
>> GCOV_COUNTER in gcc_4_7.c.
>>
>> Signed-off-by: Javi Merino 
>> ---
>>  xen/common/coverage/Makefile  |  6 +-
>>  xen/common/coverage/gcc_4_7.c | 17 +
>>  xen/common/coverage/gcc_4_9.c | 33 -
>>  xen/common/coverage/gcc_5.c   | 33 -
>>  xen/common/coverage/gcc_7.c   | 30 --
>>  5 files changed, 10 insertions(+), 109 deletions(-)
>>  delete mode 100644 xen/common/coverage/gcc_4_9.c
>>  delete mode 100644 xen/common/coverage/gcc_5.c
>>  delete mode 100644 xen/common/coverage/gcc_7.c
>>
>> diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile
>> index 63f98c71d6..d729afc9c7 100644
>> --- a/xen/common/coverage/Makefile
>> +++ b/xen/common/coverage/Makefile
>> @@ -1,11 +1,7 @@
>>  obj-y += coverage.o
>>  ifneq ($(CONFIG_CC_IS_CLANG),y)
>>  obj-y += gcov_base.o gcov.o
>> -obj-y += $(call cc-ifversion,-lt,0407, \
>> -gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
>> -gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
>> -gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
>> -gcc_5.o, gcc_7.o
>> +obj-y += $(call cc-ifversion,-lt,0407, gcc_3_4.o, gcc_4_7.o)
>>  else
>>  obj-y += llvm.o
>>  endif
>> diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
>> index 25b4a8bcdc..ddbc9f4bb0 100644
>> --- a/xen/common/coverage/gcc_4_7.c
>> +++ b/xen/common/coverage/gcc_4_7.c
>> @@ -18,15 +18,16 @@
>>  
>>  #include "gcov.h"
>>  
>> -/*
>> - * GCOV_COUNTERS will be defined if this file is included by other
>> - * source files.
>> - */
>> -#ifndef GCOV_COUNTERS
>> -# if !(GCC_VERSION >= 40700 && GCC_VERSION < 40900)
>> -#  error "Wrong version of GCC used to compile gcov"
>> -# endif
>> +#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900)
>>  #define GCOV_COUNTERS 8
>> +#elif (GCC_VERSION >= 40900 && GCC_VERSION < 5)
>> +#define GCOV_COUNTERS 9
>> +#elif GCC_VERSION >= 5 && GCC_VERSION < 7
>> +#define GCOV_COUNTERS 10
>> +#elif GCC_VERSION >= 7
>> +#define GCOV_COUNTERS 9
>> +#else
>> +#error "Wrong version of GCC used to compile gcov"
>>  #endif
> How about inverse order:
>
> #if GCC_VERSION >= 7
> #define GCOV_COUNTERS 9
> #elif GCC_VERSION >= 5
> #define GCOV_COUNTERS 10
> #elif GCC_VERSION >= 40900
> #define GCOV_COUNTERS 9
> #elif GCC_VERSION >= 40700
> #define GCOV_COUNTERS 8
> #else
> #error "Wrong version of GCC used to compile gcov"
> #endif

Forward order is the one that results in a smaller diff when inserting
new entries.

More importantly, it's the more natural way to structure such a list.

~Andrew

[PATCH v6 05/45] xenbus/backend: dynamically allocate the xen-backend shrinker

[Qi Zheng]

Use new APIs to dynamically allocate the xen-backend shrinker.

Signed-off-by: Qi Zheng 
Reviewed-by: Muchun Song 
Acked-by: Juergen Gross 
CC: Stefano Stabellini 
CC: Oleksandr Tyshchenko 
CC: xen-devel@lists.xenproject.org
---
 drivers/xen/xenbus/xenbus_probe_backend.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/xen/xenbus/xenbus_probe_backend.c 
b/drivers/xen/xenbus/xenbus_probe_backend.c
index da96c260e26b..5ebb7233076f 100644
--- a/drivers/xen/xenbus/xenbus_probe_backend.c
+++ b/drivers/xen/xenbus/xenbus_probe_backend.c
@@ -284,13 +284,9 @@ static unsigned long backend_shrink_memory_count(struct 
shrinker *shrinker,
return 0;
 }
 
-static struct shrinker backend_memory_shrinker = {
-   .count_objects = backend_shrink_memory_count,
-   .seeks = DEFAULT_SEEKS,
-};
-
 static int __init xenbus_probe_backend_init(void)
 {
+   struct shrinker *backend_memory_shrinker;
static struct notifier_block xenstore_notifier = {
.notifier_call = backend_probe_and_watch
};
@@ -305,8 +301,15 @@ static int __init xenbus_probe_backend_init(void)
 
register_xenstore_notifier(&xenstore_notifier);
 
-   if (register_shrinker(&backend_memory_shrinker, "xen-backend"))
-   pr_warn("shrinker registration failed\n");
+   backend_memory_shrinker = shrinker_alloc(0, "xen-backend");
+   if (!backend_memory_shrinker) {
+   pr_warn("shrinker allocation failed\n");
+   return 0;
+   }
+
+   backend_memory_shrinker->count_objects = backend_shrink_memory_count;
+
+   shrinker_register(backend_memory_shrinker);
 
return 0;
 }
-- 
2.30.2

Re: [PATCH v4 09/10] xen/arm: fix duplicate /reserved-memory node in Dom0

[Michal Orzel]

Hi Penny,

On 11/09/2023 06:04, Penny Zheng wrote:
> 
> 
> In case there is a /reserved-memory node already present in the host dtb,
> current Xen codes would create yet another /reserved-memory node specially
s/codes/code/

> for the static shm in Dom0 Device Tree.
> 
> Xen will use write_properties() to copy the reserved memory nodes from host 
> dtb
> to Dom0 FDT, so we want to insert the shm node along with the copying.
> And avoiding duplication, we add a checking before make_resv_memory_node().
> 
> Signed-off-by: Penny Zheng 
> 
> ---
> v3 -> v4:
> new commit
> ---
>  xen/arch/arm/domain_build.c   | 31 ---
>  xen/arch/arm/include/asm/kernel.h |  2 ++
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 02d903be78..dad234e4b5 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1401,6 +1401,10 @@ static int __init handle_linux_pci_domain(struct 
> kernel_info *kinfo,
>  return fdt_property_cell(kinfo->fdt, "linux,pci-domain", segment);
>  }
> 
> +static int __init make_shm_memory_node(const struct domain *d,
> +   void *fdt,
> +   int addrcells, int sizecells,
> +   const struct kernel_info *kinfo);
>  static int __init write_properties(struct domain *d, struct kernel_info 
> *kinfo,
> const struct dt_device_node *node)
>  {
> @@ -1549,6 +1553,23 @@ static int __init write_properties(struct domain *d, 
> struct kernel_info *kinfo,
>  }
>  }
> 
> +if ( dt_node_path_is_equal(node, "/reserved-memory") )
> +{
> +kinfo->resv_mem = true;
kinfo structure is used to store per-domain parameters and presence of reserved 
memory in host dtb
does not fit into this. Moreover, there is no need to introduce yet another 
flag for that given
that in other parts of the code in Xen we use bootinfo.reserved_mem.nr_banks to 
check if there are
reserved regions.

> +
> +/* shared memory provided. */
> +if ( kinfo->shminfo.nr_banks != 0 )
> +{
> +uint32_t addrcells = dt_n_addr_cells(node),
> + sizecells = dt_n_size_cells(node);
> +
> +res = make_shm_memory_node(d, kinfo->fdt,
> +   addrcells, sizecells, kinfo);
I haven't yet looked at previous patches but does it make sense to request 
passing both kinfo->fdt and kinfo?
IMO, the latter would be sufficient. This would apply to other functions you 
modified.

> +if ( res )
> +return res;
> +}
> +}
> +
>  return 0;
>  }
> 
> @@ -2856,9 +2877,13 @@ static int __init handle_node(struct domain *d, struct 
> kernel_info *kinfo,
>  return res;
>  }
> 
> -res = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, 
> kinfo);
> -if ( res )
> -return res;
> +/* Avoid duplicate /reserved-memory nodes in Device Tree */
> +if ( !kinfo->resv_mem )
No need for a new flag as mentioned above. Just before this line of code there 
is a check:
if ( bootinfo.reserved_mem.nr_banks > 0 )
{
res = make_memory_node(d, kinfo->fdt, addrcells, sizecells,
&bootinfo.reserved_mem);
if ( res )
return res;
}
so you can just append the following:
else
{
res = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo);
if ( res )
return res;
}
to achieve the same without a need for a new flag.

~Michal

Re: [XEN PATCH v2 2/2] coverage: update gcov info for newer versions of gcc

[Javi Merino]

On Mon, Sep 11, 2023 at 10:00:14AM +0200, Jan Beulich wrote:
> On 08.09.2023 18:20, Javi Merino wrote:
> > Shamelessly copy changes to gcov_info structures from linux so that we
> > can capture coverage for xen built with newer compilers.
> > 
> > Signed-off-by: Javi Merino 
> > ---
> >  xen/common/coverage/gcc_4_7.c | 24 +---
> >  1 file changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
> > index ddbc9f4bb0..8f6e287474 100644
> > --- a/xen/common/coverage/gcc_4_7.c
> > +++ b/xen/common/coverage/gcc_4_7.c
> > @@ -24,14 +24,23 @@
> >  #define GCOV_COUNTERS 9
> >  #elif GCC_VERSION >= 5 && GCC_VERSION < 7
> >  #define GCOV_COUNTERS 10
> > -#elif GCC_VERSION >= 7
> > +#elif GCC_VERSION >= 7 && GCC_VERSION < 10
> >  #define GCOV_COUNTERS 9
> > +#elif GCC_VERSION >= 10
> > +#define GCOV_COUNTERS 8
> >  #else
> >  #error "Wrong version of GCC used to compile gcov"
> >  #endif
> 
> This would need adjustment if patch 1 is adjusted as suggested, then ...
> 
> >  #define GCOV_TAG_FUNCTION_LENGTH3
> >  
> > +#if GCC_VERSION >= 12
> > +/* Since GCC 12.1, sizes are in BYTES and not in WORDS (4B). */
> > +#define GCOV_UNIT_SIZE 4
> > +#else
> > +#define GCOV_UNIT_SIZE 1
> > +#endif
> 
> ... making the earlier group also match this one (which already works
> downwards in version number space).

True

> As to the comments (here and below): Since the version check is for
> 12.0, may I suggest to simply say "gcc 12" everywhere?
> 
> > @@ -89,6 +98,10 @@ struct gcov_info {
> >  unsigned int version;
> >  struct gcov_info *next;
> >  unsigned int stamp;
> > +#if (GCC_VERSION >= 12)
> 
> Nit: Here any below parentheses likely want omitting, to match there
> being none further up. Preferably with the adjustments (which again
> I'd be okay doing while committing, so long as you agree)
> Acked-by: Jan Beulich 

I'm ok with those changes and I'm fine with you doing them.

Cheers,
Javi

Re: [XEN PATCH v2 1/2] coverage: simplify the logic of choosing the number of gcov counters depending on the gcc version

[Javi Merino]

On Mon, Sep 11, 2023 at 09:54:53AM +0200, Jan Beulich wrote:
> On 08.09.2023 18:20, Javi Merino wrote:
> > The current structure of choosing the correct file based on the
> > compiler version makes us make 33 line files just to define a
> > constant.  The changes after gcc 4.7 are minimal, just the number of
> > counters.
> > 
> > Fold the changes in gcc_4_9.c, gcc_5.c and gcc_7.c into gcc_4_7.c to
> > remove a lot of the boilerplate and keep the logic of choosing the
> > GCOV_COUNTER in gcc_4_7.c.
> > 
> > Signed-off-by: Javi Merino 
> > ---
> >  xen/common/coverage/Makefile  |  6 +-
> >  xen/common/coverage/gcc_4_7.c | 17 +
> >  xen/common/coverage/gcc_4_9.c | 33 -
> >  xen/common/coverage/gcc_5.c   | 33 -
> >  xen/common/coverage/gcc_7.c   | 30 --
> >  5 files changed, 10 insertions(+), 109 deletions(-)
> >  delete mode 100644 xen/common/coverage/gcc_4_9.c
> >  delete mode 100644 xen/common/coverage/gcc_5.c
> >  delete mode 100644 xen/common/coverage/gcc_7.c
> > 
> > diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile
> > index 63f98c71d6..d729afc9c7 100644
> > --- a/xen/common/coverage/Makefile
> > +++ b/xen/common/coverage/Makefile
> > @@ -1,11 +1,7 @@
> >  obj-y += coverage.o
> >  ifneq ($(CONFIG_CC_IS_CLANG),y)
> >  obj-y += gcov_base.o gcov.o
> > -obj-y += $(call cc-ifversion,-lt,0407, \
> > -   gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
> > -   gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
> > -   gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
> > -   gcc_5.o, gcc_7.o
> > +obj-y += $(call cc-ifversion,-lt,0407, gcc_3_4.o, gcc_4_7.o)
> >  else
> >  obj-y += llvm.o
> >  endif
> > diff --git a/xen/common/coverage/gcc_4_7.c b/xen/common/coverage/gcc_4_7.c
> > index 25b4a8bcdc..ddbc9f4bb0 100644
> > --- a/xen/common/coverage/gcc_4_7.c
> > +++ b/xen/common/coverage/gcc_4_7.c
> > @@ -18,15 +18,16 @@
> >  
> >  #include "gcov.h"
> >  
> > -/*
> > - * GCOV_COUNTERS will be defined if this file is included by other
> > - * source files.
> > - */
> > -#ifndef GCOV_COUNTERS
> > -# if !(GCC_VERSION >= 40700 && GCC_VERSION < 40900)
> > -#  error "Wrong version of GCC used to compile gcov"
> > -# endif
> > +#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900)
> >  #define GCOV_COUNTERS 8
> > +#elif (GCC_VERSION >= 40900 && GCC_VERSION < 5)
> > +#define GCOV_COUNTERS 9
> > +#elif GCC_VERSION >= 5 && GCC_VERSION < 7
> > +#define GCOV_COUNTERS 10
> > +#elif GCC_VERSION >= 7
> > +#define GCOV_COUNTERS 9
> > +#else
> > +#error "Wrong version of GCC used to compile gcov"
> >  #endif
> 
> How about inverse order:
> 
> #if GCC_VERSION >= 7
> #define GCOV_COUNTERS 9
> #elif GCC_VERSION >= 5
> #define GCOV_COUNTERS 10
> #elif GCC_VERSION >= 40900
> #define GCOV_COUNTERS 9
> #elif GCC_VERSION >= 40700
> #define GCOV_COUNTERS 8
> #else
> #error "Wrong version of GCC used to compile gcov"
> #endif
> 
> Otherwise a nit and a question: Parentheses would want using consistently.

True, the parenthesis are unnecessary and inconsistent in the patch.

> And wouldn't it make sense to combine the two ranges resulting in 9 being
> chosen? (Imo in the alternative layout that's less desirable.)
> 
> Since the adjustment would be easy to make, I'd be fine doing so while
> committing, and then
> Reviewed-by: Jan Beulich 

Happy for you to do the changes.  Or I can do it and fix the next
patch as well.

Cheers,
Javi

> As an unrelated remark: gcc_3_4.c is clearly outdated as well, simply by
> its name. Imo it would have wanted to be gcc_4_1.c the latest as of commit
> 03f22f0070f3 ("README: adjust gcc version requirement"), i.e. for over 10
> years.
> 
> Jan