[xen-unstable test] 183640: tolerable trouble: fail/pass/starved - PUSHED

2023-10-31 Thread osstest service owner
flight 183640 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183640/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-pair 11 xen-install/dst_host fail in 183626 pass in 183640
 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-install fail in 183626 pass in 183640
 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-install fail in 183626 pass in 183640
 test-armhf-armhf-libvirt-raw 18 guest-start.2  fail pass in 183626

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

Re: exynos-mixer 14450000.mixer: [drm:exynos_drm_register_dma] *ERROR* Device 14450000.mixer lacks support for IOMMU

2023-10-31 Thread Chuck Zmudzinski
On 10/31/2023 7:45 PM, Stefano Stabellini wrote:
> Unfortunately there is no easy solution.
> 
> Do you know the version of the SMMU available on the platform? 

I am trying to discern, but I doubt we have v3 because we are
working on a very old chromebook from 2012, and I am finding
patches for smmv3 in Linux not starting until 2015. It is good to
know about this option, though, for future work we might do on newer
devices.

> If it is a SMMUv3 you can try to use the nested SMMU patch series to
> enable a virtual SMMU in Dom0: 
> https://marc.info/?l=xen-devel=166991020831005
> That way, Xen can use the SMMU to protect VMs, and Dom0 can also use the
> SMMU for its own purposes at the same time.
> 
> Alternatively, you can dig into the details of the exynos-drm driver to
> see what exactly is the dependency on the IOMMU framework in Linux and
> remove the dependency. Unfortunately none of us in this thread are
> expert on exynos-drm so it would be difficult to advise on how to do
> that. For example, I don't know how you could debug the x11 problem you
> described because I don't typically work with x11 or with the exynos. If
> there is an open source mailing list for exynos-drm development they
> might be able to advise on how to remove the IOMMU dependency there.

We have received this message from Marek Szyprowski of Samsung:

https://lore.kernel.org/lkml/7a71e348-f892-4fd4-8857-b72f35ab5...@samsung.com/

Marek recommends this patch to possibly help with this issue:

https://github.com/torvalds/linux/commit/bbc4d205d93f52ee18dfa7858d51489c0506547f

and also these kernel config settings:

On 10/31/2023 8:08 AM, Marek Szyprowski wrote:
> If not, then as a temporary workaround please disable 
> CONFIG_DRM_EXYNOS_MIXER and CONFIG_DRM_EXYNOS_HDMI in your kernel config 
> and check what will happen (You will lose the HDMI output, but maybe 
> this won't a big issue).

Mario and I have preliminary evidence that applying both of Marek's
recommendations to the 6.1.59 kernel have improved the situation to
the point where now the Chromebook can run X.org on Xen. We are doing
further tests to see how Marek's patch and/or the kernel config settings
to disable the mixer and the HDMI affect the behavior of the GPU in dom0
on Xen.

> 
> The final option, which is a gross hack, would be to let Dom0 use the
> IOMMU for its own gain. Xen doesn't use the IOMMU. If you do that you
> lose freedom from interference between the VMs and you cannot run driver
> domains or directly assign devices to DomUs. But if you are running a
> research project you might be OK with that. To get it to work, you need
> to hack Xen so that it remaps the IOMMU to Dom0 to let Dom0 program it
> directly. The attached patch (untested) would be a place to start. You
> also need to pass iommu=false to the Xen command line to prevent Xen
> from using the iommu itself.

I am interested to investigate if only the mixer and the HDMI is causing
the trouble. Based on what you are telling me about Xen not exposing the
IOMMU to dom0, I don't fully understand the original log messages I was
getting when I followed Julien's suggestion to find the symbols associated
with each address in the original message, and those seemed to indicate that
the exynos_drm device was using the IOMMU in dom0, but the mixer was not,
and the fact that they both were not using the IOMMU is what caused the
test to fail and Linux refuse to initialize the GPU on dom0, whereas on
bare metal, the logs indicated both the exynos mixer, which I think is a
sub device of the exynos_drm, and the exynos_drm, use the IOMMU on bare
metal.

I also found this patch which suggests if we can get the devices to work
we will be compromising the security and isolation between guests:

https://patchwork.kernel.org/project/linux-arm-kernel/patch/20190301192017.39770-1-diand...@chromium.org/

There are plenty of unanswered questions here in my mind,

Cheers,

Chuck

> 
> Cheers,
> 
> Stefano
> 
> 
> On Wed, 1 Nov 2023, Mario Marietto wrote:
>> I'm aware of the presence of that post. I'm working on the same
>> project with the guy who explained the problem. Unfortunately,the
>> solution proposed does not work well. Xen is working,but the screen is
>> still black.
>> 
>> On Wed, Nov 1, 2023 at 12:04 AM Stefano Stabellini
>>  wrote:
>> >
>> > Hi Mario,
>> >
>> > I am adding xen-devel and a couple of other Xen maintainers that might
>> > know how to help make progress on this issues.
>> >
>> > Replies inline below.
>> >
>> >
>> > On Tue, 31 Oct 2023, Mario Marietto wrote:
>> > > Hello,
>> > >
>> > > We are a team of linux enthusiasts who are trying to boot Xen on a
>> > > Samsung XE303C12 Chromebook aka "snow"
>> > > following the suggestions in the slide show presentation here:
>> > >
>> > > https://www.slideshare.net/xen_com_mgr/xpds16-porting-xen-on-arm-to-a-new-soc-julien-grall-arm
>> > >
>> > > This device uses an exynos5250 SOC dual core 1.7 GHz with 2 MB RAM, it is
>> > > a Samsung armv7 chip with 

Re: [PATCH v8 3/8] xen/arm: Fold mmu_init_secondary_cpu() to head.S

2023-10-31 Thread Henry Wang
Hi Julien,

> On Nov 1, 2023, at 02:29, Julien Grall  wrote:
> 
> Hi Henry,
> 
> +Ayan
> 
> On 23/10/2023 03:13, Henry Wang wrote:
>> Currently mmu_init_secondary_cpu() only enforces the page table
>> should not contain mapping that are both Writable and eXecutables
>> after boot. To ease the arch/arm/mm.c split work, fold this function
>> to head.S.
>> For arm32, introduce an assembly macro pt_enforce_wxn. The macro is
>> called before secondary CPUs jumping into the C world.
>> For arm64, set the SCTLR_Axx_ELx_WXN flag right when the MMU is
>> enabled. This would avoid the extra TLB flush and SCTLR dance.
> 
> For a random reader, it is not clear why you can't set WnX early for arm32 as 
> well. I think it would helpful to explain the difference. I.e. at the point 
> the MMU is enabled, the page-tables may still contain mapping which are 
> writable and executable.

Sounds good, I will add the suggested sentence.

>>  .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 r0
> 
> From recent discussion on IRC, Ayan reminded me this patch [1]. Ideally, I 
> would want to print a message just before to indicate that the bit is set. 
> But I understand that this would need to be droppped in Ayan rework as we 
> don't yet support early printk in enable_mmu().
> 
> While debugging an MMU issue on Arm32, I wrote a patch to sprinkle prints in 
> the enable_mmu() code. I will clean-up the patch and send it.

Just to make sure, your patch is for both Arm32 and Arm64, is my understanding 
correct?
If it is only for Arm32, do you need me adding the print for Arm64 as well in 
this patch?

> I will add a print at that point. Meanwhile, I would move the call a few 
> lines above? This will allow Ayan to drop [1].

Yeah I will include Ayan’s change in this patch and add his sign-off.

>>  PRINT("- Ready -\r\n")
>>  /* Jump to C world */
>>  mov_w r2, start_secondary
>> diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
>> index 88075ef083..df06cefbbe 100644
>> --- a/xen/arch/arm/arm64/mmu/head.S
>> +++ b/xen/arch/arm/arm64/mmu/head.S
>> @@ -264,10 +264,11 @@ ENDPROC(create_page_tables)
>>   * Inputs:
>>   *   x0 : Physical address of the page tables.
> 
> The inputs list should be updated to mention what x1 means.

I will use “x1: Extra flags of the SCTLR.” if this looks good to you.

>>   *
>> - * Clobbers x0 - x4
>> + * Clobbers x0 - x6
> 
> Below, you only seem to introduce x5. So shouldn't this be: "Clobbers x0 - 
> x5"?

Hmmm yes you are correct, I blindly copied the code from [2]. Sorry for the 
mess, I will
correct it in v9.

> Cheers,
> 
> [1] 
> https://lore.kernel.org/all/20230911135942.791206-2-ayan.kumar.hal...@amd.com/

[2] 
https://lore.kernel.org/xen-devel/4d7a9849-8990-8ddd-3531-93f4e2e26...@xen.org/

Kind regards,
Henry

> 
> -- 
> Julien Grall



[ovmf test] 183643: all pass - PUSHED

2023-10-31 Thread osstest service owner
flight 183643 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183643/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf ccbe2e938386ed1ec49b3ad8ed6d107e7416e273
baseline version:
 ovmf 36812d6c3e0c4402ea90e20566ac80de634d210b

Last test of basis   183639  2023-10-31 15:12:49 Z0 days
Testing same since   183643  2023-10-31 21:14:30 Z0 days1 attempts


People who touched revisions under test:
  Yuanhao Xie 

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
   36812d6c3e..ccbe2e9383  ccbe2e938386ed1ec49b3ad8ed6d107e7416e273 -> 
xen-tested-master



Re: [PATCH] x86/x2apic: introduce a mixed physical/cluster mode

2023-10-31 Thread Elliott Mitchell
On Mon, Oct 30, 2023 at 04:27:22PM +0100, Roger Pau Monné wrote:
> On Mon, Oct 30, 2023 at 07:50:27AM -0700, Elliott Mitchell wrote:
> > On Tue, Oct 24, 2023 at 03:51:50PM +0200, Roger Pau Monne wrote:
> > > diff --git a/xen/arch/x86/genapic/x2apic.c b/xen/arch/x86/genapic/x2apic.c
> > > index 707deef98c27..15632cc7332e 100644
> > > --- a/xen/arch/x86/genapic/x2apic.c
> > > +++ b/xen/arch/x86/genapic/x2apic.c
> > > @@ -220,38 +239,56 @@ static struct notifier_block x2apic_cpu_nfb = {
> > >  static int8_t __initdata x2apic_phys = -1;
> > >  boolean_param("x2apic_phys", x2apic_phys);
> > >  
> > > +enum {
> > > +   unset, physical, cluster, mixed
> > > +} static __initdata x2apic_mode = unset;
> > > +
> > > +static int __init parse_x2apic_mode(const char *s)
> > > +{
> > > +if ( !cmdline_strcmp(s, "physical") )
> > > +x2apic_mode = physical;
> > > +else if ( !cmdline_strcmp(s, "cluster") )
> > > +x2apic_mode = cluster;
> > > +else if ( !cmdline_strcmp(s, "mixed") )
> > > +x2apic_mode = mixed;
> > > +else
> > > +return EINVAL;
> > > +
> > > +return 0;
> > > +}
> > > +custom_param("x2apic-mode", parse_x2apic_mode);
> > > +
> > >  const struct genapic *__init apic_x2apic_probe(void)
> > >  {
> > > -if ( x2apic_phys < 0 )
> > > +/* x2apic-mode option has preference over x2apic_phys. */
> > > +if ( x2apic_phys >= 0 && x2apic_mode == unset )
> > > +x2apic_mode = x2apic_phys ? physical : cluster;
> > > +
> > > +if ( x2apic_mode == unset )
> > >  {
> > > -/*
> > > - * Force physical mode if there's no (full) interrupt remapping 
> > > support:
> > > - * The ID in clustered mode requires a 32 bit destination field 
> > > due to
> > > - * the usage of the high 16 bits to hold the cluster ID.
> > > - */
> > > -x2apic_phys = iommu_intremap != iommu_intremap_full ||
> > > -  (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL) ||
> > > -  IS_ENABLED(CONFIG_X2APIC_PHYSICAL);
> > > -}
> > > -else if ( !x2apic_phys )
> > > -switch ( iommu_intremap )
> > > +if ( acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL )
> > >  {
> > 
> > Could this explain the issues with recent AMD processors/motherboards?
> > 
> > Mainly the firmware had been setting this flag, but Xen was previously
> > ignoring it?
> 
> No, not unless you pass {no-}x2apic_phys={false,0} on the Xen command
> line to force logical (clustered) destination mode.
> 
> > As such Xen had been attempting to use cluster mode on an
> > x2APIC where that mode was broken for physical interrupts?
> 
> No, not realy, x2apic_phys was already forced to true if
> acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL is set on the FADT (I
> just delete that line in this same chunk and move it here).

I think I inverted the case in my speculation.  Perhaps AMD's firmware
SHOULD have been setting ACPI_FADT_APIC_PHYSICAL, but failed to?

Given the rough launch with AMD's latest, I could readily see there
being a goof like this.  Question will then be, will mixed mode work on
these processors?


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





Re: exynos-mixer 14450000.mixer: [drm:exynos_drm_register_dma] *ERROR* Device 14450000.mixer lacks support for IOMMU

2023-10-31 Thread Stefano Stabellini
Unfortunately there is no easy solution.

Do you know the version of the SMMU available on the platform? If it is
a SMMUv3 you can try to use the nested SMMU patch series to enable a
virtual SMMU in Dom0: https://marc.info/?l=xen-devel=166991020831005
That way, Xen can use the SMMU to protect VMs, and Dom0 can also use the
SMMU for its own purposes at the same time.

Alternatively, you can dig into the details of the exynos-drm driver to
see what exactly is the dependency on the IOMMU framework in Linux and
remove the dependency. Unfortunately none of us in this thread are
expert on exynos-drm so it would be difficult to advise on how to do
that. For example, I don't know how you could debug the x11 problem you
described because I don't typically work with x11 or with the exynos. If
there is an open source mailing list for exynos-drm development they
might be able to advise on how to remove the IOMMU dependency there.

The final option, which is a gross hack, would be to let Dom0 use the
IOMMU for its own gain. Xen doesn't use the IOMMU. If you do that you
lose freedom from interference between the VMs and you cannot run driver
domains or directly assign devices to DomUs. But if you are running a
research project you might be OK with that. To get it to work, you need
to hack Xen so that it remaps the IOMMU to Dom0 to let Dom0 program it
directly. The attached patch (untested) would be a place to start. You
also need to pass iommu=false to the Xen command line to prevent Xen
from using the iommu itself.

Cheers,

Stefano


On Wed, 1 Nov 2023, Mario Marietto wrote:
> I'm aware of the presence of that post. I'm working on the same
> project with the guy who explained the problem. Unfortunately,the
> solution proposed does not work well. Xen is working,but the screen is
> still black.
> 
> On Wed, Nov 1, 2023 at 12:04 AM Stefano Stabellini
>  wrote:
> >
> > Hi Mario,
> >
> > I am adding xen-devel and a couple of other Xen maintainers that might
> > know how to help make progress on this issues.
> >
> > Replies inline below.
> >
> >
> > On Tue, 31 Oct 2023, Mario Marietto wrote:
> > > Hello,
> > >
> > > We are a team of linux enthusiasts who are trying to boot Xen on a
> > > Samsung XE303C12 Chromebook aka "snow"
> > > following the suggestions in the slide show presentation here:
> > >
> > > https://www.slideshare.net/xen_com_mgr/xpds16-porting-xen-on-arm-to-a-new-soc-julien-grall-arm
> > >
> > > This device uses an exynos5250 SOC dual core 1.7 GHz with 2 MB RAM, it is
> > > a Samsung armv7 chip with virtualization extensions.
> > >
> > > In particular, we have it working fairly well both on the bare metal with
> > > a recent 6.1.59 Linux LTS kernel and also with a recent 5.4.257 LTS
> > > kernel with KVM, the older LTS kernel version is used to test KVM because
> > > support for KVM on arm v7 was removed from Linux around kernel version
> > > 5.7. So we know we have the hypervisor mode enabled because we were able
> > > to use it with KVM.
> > >
> > > For Xen, we are using the latest Debian build of Xen 4.17 for the Debian
> > > armhf architecture:
> > >
> > > (XEN) Xen version 4.17.2-pre (Debian 4.17.1+2-gb773c48e36-1)
> > > (pkg-xen-devel@xxx
> > > ) (arm-linux-gnueabihf-gcc (Debian
> > > 12.2.0-14) 12.2.0) debug=n Thu May 18 19:26:30 UTC 2023
> > >
> > > The Linux kernel is a custom build that adds the Xen config kernel
> > > options (CONFIG_XEN_DOM0, etc) on top of a kernel that works well on the
> > > same Chromebook model on the bare metal. I can provide the config options
> > > of the kernel that was used if that is helpful.
> > >
> > > Our method of booting is to have u-boot boot the Xen hypervisor and load
> > > the device tree after adding the dom0 to the otherwise unaltered device
> > > tree from the Linux kernel using u-boot fdt commands to add a /chosen
> > > node, as described on the Xen wiki and in the pages linked from there. We
> > > have also tried adding and loading an initrd.img using the device tree
> > > /chosen node but that made no difference in our tests.
> > >
> > > We actually have the Linux LTS kernel version 6.1.59 working as dom0 with
> > > Xen using the same version of u-boot that we used for KVM, but with a big
> > > problem.
> > >
> > > The problem we see is that when booting the 6.1.59 kernel version as dom0
> > > with Xen, the screen is totally dark and the only way to access the
> > > system is remotely through ssh. Logs indicate most everything else is
> > > working, such as the wifi card so we can access it remotely via ssh and a
> > > USB optical mouse lights up when connected so USB is also working.
> > > Obviously, the disk is also working. The Chromebook is configured to boot
> > > from the device's SD card slot by turning on Chrome OS developer mode
> > > options to enable booting from the SD card slot.
> > >
> > > The mystery is that when booting the exact same 6.1.59 kernel on the bare
> > > metal instead of booting it as dom0 on Xen, it boots 

Re: exynos-mixer 14450000.mixer: [drm:exynos_drm_register_dma] *ERROR* Device 14450000.mixer lacks support for IOMMU

2023-10-31 Thread Mario Marietto
I'm aware of the presence of that post. I'm working on the same
project with the guy who explained the problem. Unfortunately,the
solution proposed does not work well. Xen is working,but the screen is
still black.

On Wed, Nov 1, 2023 at 12:04 AM Stefano Stabellini
 wrote:
>
> Hi Mario,
>
> I am adding xen-devel and a couple of other Xen maintainers that might
> know how to help make progress on this issues.
>
> Replies inline below.
>
>
> On Tue, 31 Oct 2023, Mario Marietto wrote:
> > Hello,
> >
> > We are a team of linux enthusiasts who are trying to boot Xen on a
> > Samsung XE303C12 Chromebook aka "snow"
> > following the suggestions in the slide show presentation here:
> >
> > https://www.slideshare.net/xen_com_mgr/xpds16-porting-xen-on-arm-to-a-new-soc-julien-grall-arm
> >
> > This device uses an exynos5250 SOC dual core 1.7 GHz with 2 MB RAM, it is
> > a Samsung armv7 chip with virtualization extensions.
> >
> > In particular, we have it working fairly well both on the bare metal with
> > a recent 6.1.59 Linux LTS kernel and also with a recent 5.4.257 LTS
> > kernel with KVM, the older LTS kernel version is used to test KVM because
> > support for KVM on arm v7 was removed from Linux around kernel version
> > 5.7. So we know we have the hypervisor mode enabled because we were able
> > to use it with KVM.
> >
> > For Xen, we are using the latest Debian build of Xen 4.17 for the Debian
> > armhf architecture:
> >
> > (XEN) Xen version 4.17.2-pre (Debian 4.17.1+2-gb773c48e36-1)
> > (pkg-xen-devel@xxx
> > ) (arm-linux-gnueabihf-gcc (Debian
> > 12.2.0-14) 12.2.0) debug=n Thu May 18 19:26:30 UTC 2023
> >
> > The Linux kernel is a custom build that adds the Xen config kernel
> > options (CONFIG_XEN_DOM0, etc) on top of a kernel that works well on the
> > same Chromebook model on the bare metal. I can provide the config options
> > of the kernel that was used if that is helpful.
> >
> > Our method of booting is to have u-boot boot the Xen hypervisor and load
> > the device tree after adding the dom0 to the otherwise unaltered device
> > tree from the Linux kernel using u-boot fdt commands to add a /chosen
> > node, as described on the Xen wiki and in the pages linked from there. We
> > have also tried adding and loading an initrd.img using the device tree
> > /chosen node but that made no difference in our tests.
> >
> > We actually have the Linux LTS kernel version 6.1.59 working as dom0 with
> > Xen using the same version of u-boot that we used for KVM, but with a big
> > problem.
> >
> > The problem we see is that when booting the 6.1.59 kernel version as dom0
> > with Xen, the screen is totally dark and the only way to access the
> > system is remotely through ssh. Logs indicate most everything else is
> > working, such as the wifi card so we can access it remotely via ssh and a
> > USB optical mouse lights up when connected so USB is also working.
> > Obviously, the disk is also working. The Chromebook is configured to boot
> > from the device's SD card slot by turning on Chrome OS developer mode
> > options to enable booting from the SD card slot.
> >
> > The mystery is that when booting the exact same 6.1.59 kernel on the bare
> > metal instead of booting it as dom0 on Xen, it boots up with full access
> > to the screen and we can interact with the system using the X.org windows
> > system. But booting as dom0 with Xen, the screen is totally dark and the
> > only access we have to the system is through the network via ssh. Also,
> > when booting the 5.4.257 kernel with KVM in hypervisor mode, the screen
> > works and we can interact with the system through the X.org windows
> > system.
> >
> > Exploring the log file,we have seen the errors below :
> >
> >
> > With Xen (or in bare metal):
> >
> > devuan-bunsen kernel: [drm] Exynos DRM: using 1440.fimd device for
> > DMA mapping operations
> > devuan-bunsen kernel: exynos-drm exynos-drm: bound 1440.fimd (ops
> > 0xc0d96354)
> > devuan-bunsen kernel: exynos-drm exynos-drm: bound 1445.mixer (ops
> > 0xc0d97554)
> > devuan-bunsen kernel: exynos-drm exynos-drm: bound
> > 145b.dp-controller (ops 0xc0d97278)
> > devuan-bunsen kernel: exynos-drm exynos-drm: bound 1453.hdmi (ops
> > 0xc0d97bd0)
> > ...
> > devuan-bunsen kernel: Console: switching to colour frame buffer device 
> > 170x48
> > devuan-bunsen kernel: exynos-drm exynos-drm: [drm] fb0: exynosdrmfb
> > frame buffer device
> > devuan-bunsen kernel: [drm] Initialized exynos 1.1.0 20180330 for
> > exynos-drm on minor 0
> >
> > In this case,the kernel is able to use the exynos-drm kernel to start
> > the fb0 device. But with Xen we get this error with exynos-drm:
> >
> > devuan-bunsen kernel: [drm] Exynos DRM: using 1440.fimd device for
> > DMA mapping operations
> > devuan-bunsen kernel: exynos-drm exynos-drm: bound 1440.fimd (ops
> > 0xc0d96354)
> > devuan-bunsen kernel: exynos-mixer 1445.mixer:
> > [drm:exynos_drm_register_dma] *ERROR* Device 

Re: [XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3

2023-10-31 Thread Stefano Stabellini
On Tue, 31 Oct 2023, Julien Grall wrote:
> Hi,
> 
> On 31/10/2023 10:31, Jan Beulich wrote:
> > On 31.10.2023 10:25, Federico Serafini wrote:
> > > Make function defintions and declarations consistent.
> 
> typo: s/defintions/definitions/
> 
> > > No functional change.
> > > 
> > > Signed-off-by: Federico Serafini 
> > 
> > Acked-by: Jan Beulich 
> > 
> > However, ...
> > 
> > > ---
> > > Changes in v2:
> > > - use 'ptr' do denote a const void * parameter.
> > 
> > ... not even this (let alone the description) clarifies what the
> > inconsistency was. I had to go check to figure that x86 already uses
> > "ptr". Such things could do with spelling out.
> 
> +1. The more that x86 was the "odd" one but it was chosen to use the variant
> everywhere.
> 
> With the commit message clarified:
> 
> Acked-by: Julien Grall 

I made the changes on commit to my for-4.19 branch (I am going to wait
until staging fully reopens before moving commits to staging).



Re: exynos-mixer 14450000.mixer: [drm:exynos_drm_register_dma] *ERROR* Device 14450000.mixer lacks support for IOMMU

2023-10-31 Thread Stefano Stabellini
Hi Mario,

I am adding xen-devel and a couple of other Xen maintainers that might
know how to help make progress on this issues.

Replies inline below.


On Tue, 31 Oct 2023, Mario Marietto wrote:
> Hello,
> 
> We are a team of linux enthusiasts who are trying to boot Xen on a
> Samsung XE303C12 Chromebook aka "snow"
> following the suggestions in the slide show presentation here:
> 
> https://www.slideshare.net/xen_com_mgr/xpds16-porting-xen-on-arm-to-a-new-soc-julien-grall-arm
> 
> This device uses an exynos5250 SOC dual core 1.7 GHz with 2 MB RAM, it is
> a Samsung armv7 chip with virtualization extensions.
> 
> In particular, we have it working fairly well both on the bare metal with
> a recent 6.1.59 Linux LTS kernel and also with a recent 5.4.257 LTS
> kernel with KVM, the older LTS kernel version is used to test KVM because
> support for KVM on arm v7 was removed from Linux around kernel version
> 5.7. So we know we have the hypervisor mode enabled because we were able
> to use it with KVM.
> 
> For Xen, we are using the latest Debian build of Xen 4.17 for the Debian
> armhf architecture:
> 
> (XEN) Xen version 4.17.2-pre (Debian 4.17.1+2-gb773c48e36-1)
> (pkg-xen-devel@xxx
> ) (arm-linux-gnueabihf-gcc (Debian
> 12.2.0-14) 12.2.0) debug=n Thu May 18 19:26:30 UTC 2023
> 
> The Linux kernel is a custom build that adds the Xen config kernel
> options (CONFIG_XEN_DOM0, etc) on top of a kernel that works well on the
> same Chromebook model on the bare metal. I can provide the config options
> of the kernel that was used if that is helpful.
> 
> Our method of booting is to have u-boot boot the Xen hypervisor and load
> the device tree after adding the dom0 to the otherwise unaltered device
> tree from the Linux kernel using u-boot fdt commands to add a /chosen
> node, as described on the Xen wiki and in the pages linked from there. We
> have also tried adding and loading an initrd.img using the device tree
> /chosen node but that made no difference in our tests.
> 
> We actually have the Linux LTS kernel version 6.1.59 working as dom0 with
> Xen using the same version of u-boot that we used for KVM, but with a big
> problem.
> 
> The problem we see is that when booting the 6.1.59 kernel version as dom0
> with Xen, the screen is totally dark and the only way to access the
> system is remotely through ssh. Logs indicate most everything else is
> working, such as the wifi card so we can access it remotely via ssh and a
> USB optical mouse lights up when connected so USB is also working.
> Obviously, the disk is also working. The Chromebook is configured to boot
> from the device's SD card slot by turning on Chrome OS developer mode
> options to enable booting from the SD card slot.
> 
> The mystery is that when booting the exact same 6.1.59 kernel on the bare
> metal instead of booting it as dom0 on Xen, it boots up with full access
> to the screen and we can interact with the system using the X.org windows
> system. But booting as dom0 with Xen, the screen is totally dark and the
> only access we have to the system is through the network via ssh. Also,
> when booting the 5.4.257 kernel with KVM in hypervisor mode, the screen
> works and we can interact with the system through the X.org windows
> system.
> 
> Exploring the log file,we have seen the errors below :
> 
> 
> With Xen (or in bare metal):
> 
> devuan-bunsen kernel: [drm] Exynos DRM: using 1440.fimd device for
> DMA mapping operations
> devuan-bunsen kernel: exynos-drm exynos-drm: bound 1440.fimd (ops
> 0xc0d96354)
> devuan-bunsen kernel: exynos-drm exynos-drm: bound 1445.mixer (ops
> 0xc0d97554)
> devuan-bunsen kernel: exynos-drm exynos-drm: bound
> 145b.dp-controller (ops 0xc0d97278)
> devuan-bunsen kernel: exynos-drm exynos-drm: bound 1453.hdmi (ops
> 0xc0d97bd0)
> ...
> devuan-bunsen kernel: Console: switching to colour frame buffer device 170x48
> devuan-bunsen kernel: exynos-drm exynos-drm: [drm] fb0: exynosdrmfb
> frame buffer device
> devuan-bunsen kernel: [drm] Initialized exynos 1.1.0 20180330 for
> exynos-drm on minor 0
> 
> In this case,the kernel is able to use the exynos-drm kernel to start
> the fb0 device. But with Xen we get this error with exynos-drm:
> 
> devuan-bunsen kernel: [drm] Exynos DRM: using 1440.fimd device for
> DMA mapping operations
> devuan-bunsen kernel: exynos-drm exynos-drm: bound 1440.fimd (ops
> 0xc0d96354)
> devuan-bunsen kernel: exynos-mixer 1445.mixer:
> [drm:exynos_drm_register_dma] *ERROR* Device 1445.mixer lacks
> support for IOMMU
> devuan-bunsen kernel: exynos-drm exynos-drm: failed to bind
> 1445.mixer (ops 0xc0d97554): -22
> devuan-bunsen kernel: exynos-drm exynos-drm: adev bind failed: -22
> devuan-bunsen kernel: exynos-dp: probe of 145b.dp-controller
> failed with error -22
> 
> 
> Any ideas why booting the same Linux kernel that results in a working
> X.org display on the bare metal instead as dom0 on Xen would cause the
> display to 

Re: [XEN PATCH][for-4.19 v5 2/8] x86: add deviation for asm-only functions

2023-10-31 Thread Stefano Stabellini
On Tue, 31 Oct 2023, Jan Beulich wrote:
> On 31.10.2023 09:22, Nicola Vetrini wrote:
> > On 2023-10-30 16:12, Jan Beulich wrote:
> >> On 30.10.2023 10:11, Nicola Vetrini wrote:
> >>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> >>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> >>> @@ -163,6 +163,15 @@ Therefore the absence of prior declarations is 
> >>> safe."
> >>>  -config=MC3R1.R8.4,reports+={safe, "first_area(any_loc(file(gcov)))"}
> >>>  -doc_end
> >>>
> >>> +-doc_begin="Recognize the occurrence of current_stack_pointer as a 
> >>> declaration."
> >>> +-file_tag+={asm_defns, "^xen/arch/x86/include/asm/asm_defns\\.h$"}
> >>> +-config=MC3R1.R8.4,declarations+={safe, 
> >>> "loc(file(asm_defns))&&^current_stack_pointer$"}
> >>> +-doc_end
> >>> +
> >>> +-doc_begin="asmlinkage is a marker to indicate that the function is 
> >>> only used from asm modules."
> >>> +-config=MC3R1.R8.4,declarations+={safe,"loc(text(^.*asmlinkage.*$, 
> >>> -1..0))"}
> >>> +-doc_end
> >>
> >> In the longer run asmlinkage will want using for functions used either 
> >> way
> >> between C and assembly (i.e. C->asm calls as well as asm->C ones). I'd 
> >> like
> >> to ask that the text please allow for that (e.g. s/from/to interface 
> >> with/).
> >>
> >>> --- a/xen/arch/x86/hvm/svm/intr.c
> >>> +++ b/xen/arch/x86/hvm/svm/intr.c
> >>> @@ -123,7 +123,7 @@ static void svm_enable_intr_window(struct vcpu *v, 
> >>> struct hvm_intack intack)
> >>>  vmcb, general1_intercepts | GENERAL1_INTERCEPT_VINTR);
> >>>  }
> >>>
> >>> -void svm_intr_assist(void)
> >>> +asmlinkage void svm_intr_assist(void)
> >>
> >> Nit (here and below): Attributes, unless impossible for some specific
> >> reason, should always go between type and identifier. Not all our code
> >> is conforming to that, but I think a majority is, and hence you should
> >> be able to find ample examples (taking e.g. __init).
> >>
> >>> --- a/xen/include/xen/compiler.h
> >>> +++ b/xen/include/xen/compiler.h
> >>> @@ -159,6 +159,9 @@
> >>>  # define ASM_FLAG_OUT(yes, no) no
> >>>  #endif
> >>>
> >>> +/* Mark a function or variable as used only from asm */
> >>> +#define asmlinkage
> >>
> >> I appreciate this being an immediately "natural" place, but considering
> >> what we know from Linux I think we ought to allow for arch overrides 
> >> here
> >> right away. For that I'm afraid compiler.h isn't best; it may still be
> >> okay as long as at least an #ifndef is put around it. Imo, however, 
> >> this
> >> ought to go into xen/linkage.h, as is being introduced by "common:
> >> assembly entry point type/size annotations". It's somewhat a shame that
> >> this and the rest of that series has missed 4.18 ...
> > 
> > An #ifndef around what, exactly?
> 
> The #define. That way at least an arch's config.h can override it.

I think the #ifdef is the way to go for now to reduce dependencies
between series


> > Anyway, making (part of) this series 
> > wait for approval
> > until the other has been accepted into 4.19 (for which I have no 
> > specific timeframe)
> > does not seem that desirable to me.
> 
> It's not ideal, but you or anyone else are free to help that other work
> make progress.



[linux-linus test] 183636: regressions - FAIL

2023-10-31 Thread osstest service owner
flight 183636 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183636/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-examine  8 reboot   fail REGR. vs. 183617
 test-arm64-arm64-libvirt-xsm  8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-credit1   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-thunderx  8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-vhd   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-libvirt-raw  8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-xsm   8 xen-boot fail REGR. vs. 183617

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183617
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183617
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 183617
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183617
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 183617
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 183617
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183617
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183617
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linux5a6a09e97199d6600d31383055f9d43fbbcbe86f
baseline version:
 linuxffc253263a1375a65fa6c9f62a893e9767fbebfa

Last test of basis   183617  2023-10-30 08:36:55 Z1 days
Failing since183625  2023-10-31 02:27:44 Z0 days2 attempts
Testing same since   183636  2023-10-31 12:42:21 Z0 days1 attempts


People who touched revisions under test:
  "Darrick J. Wong" 
  "Eric W. Biederman" 
  "Gustavo A. R. Silva" 
  "Kuyo Chang (張建文)" 
  "Md. Haris Iqbal" 
  "Peter Zijlstra (Intel)" 
  "Rafael J. Wysocki" 
  Aaron Lu 
  Aaron Plattner 
  Adam Dunlap 
  Alejandro Colomar 
  Alexander Aring 
  Alexander Shishkin 
  Alexander Sverdlin 
  Alexandre Belloni 
  Alexey Dobriyan 
  Alice Ryhl 
  Alison Schofield 
  Amir Goldstein 
  Ammar Faizi 
  Anand Jain 
  Anders Roxell 
  Anup Patel 
  Ard Biesheuvel 
  Arnd Bergmann 
  Atul Kumar Pant 
  Azeem Shaikh 
  Babu Moger 
  Baolin Liu 
  Baoquan He 
  Barry Song 
  Ben Wolsieffer 
  Bernd Schubert 
  Biju Das 
  Binbin Wu 
  Bjorn Helgaas 
  Boris Burkov 
  Borislav Petkov (AMD) 
  Borislav Petkov 
  Brett Holman 
  Brett Holman 
  Brian Foster 
  Brian Gerst 
  Catalin Marinas 
  Chen Hanxiao 
  Chen Yu 
  Chengming Zhou 
  Chris Webb 
  Christian Brauner 
  Christian Brauner 
  Christian Göttsche 
  Christoph Hellwig 
  Christoph Paasch 
  Christophe JAILLET 
  Christopher James Halse Rogers 
  Chuck Lever 
  Colin Ian King 
  Coly Li 
  Conor Dooley 
  Cuda-Chen 
  Cyril Hrubis 
  Dai Ngo 
  Dan 

Failing eclair-ARM64 job

2023-10-31 Thread Stefano Stabellini
Hi Simone,

As you might have noticed, all the eclair-ARM64 jobs have been failing
recently for the upstream Xen "staging" branch:

https://gitlab.com/xen-project/xen/-/pipelines/1056527466
https://gitlab.com/xen-project/xen/-/pipelines/1056520898

Although eclair-ARM64 is "allow_failure: true" still is the only job
currently failing and it would be nice to get it fixed, especially as we
are about to make gitlab-ci pipelines gating.

Cheers,

Stefano



Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Stefano Stabellini
On Tue, 30 Oct 2023, Julien Grall wrote:
> Hi Stefano,
> 
> On 30/10/2023 22:49, Stefano Stabellini wrote:
> > On Mon, 30 Oct 2023, Julien Grall wrote:
> > > Hi Nicola,
> > > 
> > > On 27/10/2023 16:11, Nicola Vetrini wrote:
> > > > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > > > index 8511a189253b..81473fb4 100644
> > > > --- a/docs/misra/deviations.rst
> > > > +++ b/docs/misra/deviations.rst
> > > > @@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
> > > > - __emulate_2op and __emulate_2op_nobyte
> > > > - read_debugreg and write_debugreg
> > > >+   * - R7.1
> > > > + - It is safe to use certain octal constants the way they are
> > > > defined
> > > > +   in specifications, manuals, and algorithm descriptions. Such
> > > > places
> > > > +   are marked safe with a /\* octal-ok \*/ in-code comment, or with
> > > > a
> > > > SAF
> > > > +   comment (see safe.json).
> > > 
> > > Reading this, it is unclear to me why we have two ways to deviate the rule
> > > r7.1. And more importantely, how would the developper decide which one to
> > > use?
> > 
> > I agree with you on this and we were discussing this topic just this
> > morning in the FUSA community call. I think we need a way to do this
> > with the SAF framework:
> > 
> > if (some code with violation) /* SAF-xx-safe */
> > 
> > This doesn't work today unfortunately. It can only be done this way:
> > 
> > /* SAF-xx-safe */
> > if (some code with violation)
> > 
> > Which is not always desirable. octal-ok is just an ad-hoc solution for
> > one specific violation but we need a generic way to do this. Luca is
> > investigating possible ways to support the previous format in SAF.
> 
> Why can't we use octal-ok everywhere for now?

I think this is a good option for now, yes


> My point here is to make simple for the developper to know what to use.
>
> > 
> > I think we should take this patch for now and harmonize it once SAF is
> > improved.
> 
> The description of the deviation needs some improvement.

+1


> To give an example,
> with the current wording, one could they can use octal-ok everywhere. But
> above, you are implying that SAF-xx-safe should be
> preferred.
> 
> I would still strongly prefer if we use octal-ok everywhere because this is
> simple to remember. But if the other are happy to have both SAF-XX and
> octal-ok, then the description needs to be completely unambiguous and the
> patch should contain some explanation why we have two different ways to
> deviate.

I think we could say "octal-ok" only and not mention SAF. As you can see
from the other messages we still have work to do on SAF to be able to
use it the way we would like to use it.



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

2023-10-31 Thread osstest service owner
flight 183642 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183642/

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  7befef87cc9b1bb8ca15d866ce1ecd9165ccb58c
baseline version:
 xen  9659b2a6d73b14620e187f9c626a09323853c459

Last test of basis   183618  2023-10-30 10:00:39 Z1 days
Testing same since   183642  2023-10-31 17:00:27 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  David Woodhouse 
  Federico Serafini 
  Henry Wang 
  Jan Beulich 
  Julien Grall 
  Marek Marczykowski-Górecki 
  Nicola Vetrini 
  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
   9659b2a6d7..7befef87cc  7befef87cc9b1bb8ca15d866ce1ecd9165ccb58c -> smoke



Re: [PATCH v8 3/8] xen/arm: Fold mmu_init_secondary_cpu() to head.S

2023-10-31 Thread Julien Grall

Hi Henry,

+Ayan

On 23/10/2023 03:13, Henry Wang wrote:

Currently mmu_init_secondary_cpu() only enforces the page table
should not contain mapping that are both Writable and eXecutables
after boot. To ease the arch/arm/mm.c split work, fold this function
to head.S.

For arm32, introduce an assembly macro pt_enforce_wxn. The macro is
called before secondary CPUs jumping into the C world.

For arm64, set the SCTLR_Axx_ELx_WXN flag right when the MMU is
enabled. This would avoid the extra TLB flush and SCTLR dance.


For a random reader, it is not clear why you can't set WnX early for 
arm32 as well. I think it would helpful to explain the difference. I.e. 
at the point the MMU is enabled, the page-tables may still contain 
mapping which are writable and executable.




Signed-off-by: Henry Wang 
Co-authored-by: Julien Grall 
Signed-off-by: Julien Grall 
---
v8:
- Change the setting of SCTLR_Axx_ELx_WXN for arm64 to set the
   flag right when the MMU is enabled.
v7:
- No change.
v6:
- New patch.
---
  xen/arch/arm/arm32/head.S | 20 
  xen/arch/arm/arm64/mmu/head.S | 18 +++---
  xen/arch/arm/include/asm/mm.h |  2 --
  xen/arch/arm/mm.c |  6 --
  xen/arch/arm/smpboot.c|  2 --
  5 files changed, 31 insertions(+), 17 deletions(-)

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 r0


From recent discussion on IRC, Ayan reminded me this patch [1]. 
Ideally, I would want to print a message just before to indicate that 
the bit is set. But I understand that this would need to be droppped in 
Ayan rework as we don't yet support early printk in enable_mmu().


While debugging an MMU issue on Arm32, I wrote a patch to sprinkle 
prints in the enable_mmu() code. I will clean-up the patch and send it.


I will add a print at that point. Meanwhile, I would move the call a few 
lines above? This will allow Ayan to drop [1].



  PRINT("- Ready -\r\n")
  /* Jump to C world */
  mov_w r2, start_secondary
diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
index 88075ef083..df06cefbbe 100644
--- a/xen/arch/arm/arm64/mmu/head.S
+++ b/xen/arch/arm/arm64/mmu/head.S
@@ -264,10 +264,11 @@ ENDPROC(create_page_tables)
   * Inputs:
   *   x0 : Physical address of the page tables.


The inputs list should be updated to mention what x1 means.


   *
- * Clobbers x0 - x4
+ * Clobbers x0 - x6


Below, you only seem to introduce x5. So shouldn't this be: "Clobbers x0 
- x5"?



   */
  enable_mmu:
  mov   x4, x0
+mov   x5, x1
  PRINT("- Turning on paging -\r\n")
  
  /*

@@ -283,6 +284,7 @@ enable_mmu:
  mrs   x0, SCTLR_EL2
  orr   x0, x0, #SCTLR_Axx_ELx_M  /* Enable MMU */
  orr   x0, x0, #SCTLR_Axx_ELx_C  /* Enable D-cache */
+orr   x0, x0, x5/* Enable extra flags */
  dsb   sy /* Flush PTE writes and finish reads */
  msr   SCTLR_EL2, x0  /* now paging is enabled */
  isb  /* Now, flush the icache */
@@ -297,16 +299,17 @@ ENDPROC(enable_mmu)
   * Inputs:
   *   lr : Virtual address to return to. >*
- * Clobbers x0 - x5
+ * Clobbers x0 - x6
   */
  ENTRY(enable_secondary_cpu_mm)
-mov   x5, lr
+mov   x6, lr
  
  load_paddr x0, init_ttbr

  ldr   x0, [x0]
  
+mov   x1, #SCTLR_Axx_ELx_WXN/* Enable WxN from the start */

  blenable_mmu
-mov   lr, x5
+mov   lr, x6
  
  /* Return to the virtual address requested by the caller. */

  ret
@@ -320,14 +323,15 @@ ENDPROC(enable_secondary_cpu_mm)
   * Inputs:
   *   lr : Virtual address to return to.
   *
- * Clobbers x0 - x5
+ * Clobbers x0 - x6
   */
  ENTRY(enable_boot_cpu_mm)
-mov   x5, lr
+mov   x6, lr
  
  blcreate_page_tables

  load_paddr x0, boot_pgtable
  
+mov   x1, #0/* No extra SCTLR flags */

  blenable_mmu
  
  /*

@@ -337,7 +341,7 @@ ENTRY(enable_boot_cpu_mm)
  

Re: [PATCH v2] xen: remove

2023-10-31 Thread Julien Grall




On 31/10/2023 14:28, Oleksii Kurochko wrote:

 only declares udelay() function so udelay()
declaration was moved to xen/delay.h.

For x86, __udelay() was renamed to udelay() and removed
inclusion of  in x86 code.

For ppc, udelay() stub definition was moved to ppc/stubs.c.

Suggested-by: Jan Beulich 
Signed-off-by: Oleksii Kurochko 
Reviewed-by: Jan Beulich 


For Arm:

Acked-by: Julien Grall 

Cheers,


---
Changes in v2:
  - rebase on top of the latest staging.
  - add Suggested-by:/Reviewed-by: Jan Beulich .
  - remove  for PPC.
  - remove changes related to RISC-V's  as they've not
introduced in staging branch yet.
---
  xen/arch/arm/include/asm/delay.h  | 14 --
  xen/arch/ppc/include/asm/delay.h  | 12 
  xen/arch/ppc/stubs.c  |  7 +++
  xen/arch/x86/cpu/microcode/core.c |  2 +-
  xen/arch/x86/delay.c  |  2 +-
  xen/arch/x86/include/asm/delay.h  | 13 -
  xen/include/xen/delay.h   |  2 +-
  7 files changed, 10 insertions(+), 42 deletions(-)
  delete mode 100644 xen/arch/arm/include/asm/delay.h
  delete mode 100644 xen/arch/ppc/include/asm/delay.h
  delete mode 100644 xen/arch/x86/include/asm/delay.h

diff --git a/xen/arch/arm/include/asm/delay.h b/xen/arch/arm/include/asm/delay.h
deleted file mode 100644
index 042907d9d5..00
--- a/xen/arch/arm/include/asm/delay.h
+++ /dev/null
@@ -1,14 +0,0 @@
-#ifndef _ARM_DELAY_H
-#define _ARM_DELAY_H
-
-extern void udelay(unsigned long usecs);
-
-#endif /* defined(_ARM_DELAY_H) */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/ppc/include/asm/delay.h b/xen/arch/ppc/include/asm/delay.h
deleted file mode 100644
index da6635888b..00
--- a/xen/arch/ppc/include/asm/delay.h
+++ /dev/null
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_DELAY_H__
-#define __ASM_PPC_DELAY_H__
-
-#include 
-
-static inline void udelay(unsigned long usecs)
-{
-BUG_ON("unimplemented");
-}
-
-#endif /* __ASM_PPC_DELAY_H__ */
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index 4c276b0e39..a96e45626d 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -337,3 +337,10 @@ int __init parse_arch_dom0_param(const char *s, const char 
*e)
  {
  BUG_ON("unimplemented");
  }
+
+/* delay.c */
+
+void udelay(unsigned long usecs)
+{
+BUG_ON("unimplemented");
+}
diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index 65ebeb50de..22d5e04552 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -23,6 +23,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -35,7 +36,6 @@
  
  #include 

  #include 
-#include 
  #include 
  #include 
  #include 
diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c
index 2662c26272..b3a41881a1 100644
--- a/xen/arch/x86/delay.c
+++ b/xen/arch/x86/delay.c
@@ -15,7 +15,7 @@
  #include 
  #include 
  
-void __udelay(unsigned long usecs)

+void udelay(unsigned long usecs)
  {
  unsigned long ticks = usecs * (cpu_khz / 1000);
  unsigned long s, e;
diff --git a/xen/arch/x86/include/asm/delay.h b/xen/arch/x86/include/asm/delay.h
deleted file mode 100644
index 9be2f46590..00
--- a/xen/arch/x86/include/asm/delay.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef _X86_DELAY_H
-#define _X86_DELAY_H
-
-/*
- * Copyright (C) 1993 Linus Torvalds
- *
- * Delay routines calling functions in arch/i386/lib/delay.c
- */
-
-extern void __udelay(unsigned long usecs);
-#define udelay(n) __udelay(n)
-
-#endif /* defined(_X86_DELAY_H) */
diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h
index 9150226271..8fd3b8f99f 100644
--- a/xen/include/xen/delay.h
+++ b/xen/include/xen/delay.h
@@ -3,7 +3,7 @@
  
  /* Copyright (C) 1993 Linus Torvalds */
  
-#include 

+void udelay(unsigned long usecs);
  
  static inline void mdelay(unsigned long msec)

  {


--
Julien Grall



Re: [PATCH] xen: remove

2023-10-31 Thread Julien Grall




On 31/10/2023 10:12, Oleksii Kurochko wrote:

 only declares udelay() function so udelay()
declaration was moved to xen/delay.h.

For x86, __udelay() was renamed to udelay() and removed
inclusion of  in x86 code.

Signed-off-by: Oleksii Kurochko 


For Arm:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall



[ovmf test] 183639: all pass - PUSHED

2023-10-31 Thread osstest service owner
flight 183639 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183639/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 36812d6c3e0c4402ea90e20566ac80de634d210b
baseline version:
 ovmf a671a14e63fdaa9490e5c61cf11346416f1d1463

Last test of basis   183619  2023-10-30 12:41:05 Z1 days
Testing same since   183639  2023-10-31 15:12:49 Z0 days1 attempts


People who touched revisions under test:
  Jeff Brasen 
  Jiewen Yao 
  Jinlong Xu 
  Joey Vagedes 
  Joey Vagedes 
  Laszlo Ersek 
  Michael Kubacki 

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
   a671a14e63..36812d6c3e  36812d6c3e0c4402ea90e20566ac80de634d210b -> 
xen-tested-master



Re: [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common

2023-10-31 Thread Stewart Hildebrand
On 10/31/23 06:56, Jan Beulich wrote:
> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -607,7 +607,8 @@ int arch_sanitise_domain_config(struct 
>> xen_domctl_createdomain *config)
>>  {
>>  unsigned int max_vcpus;
>>  unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
>> -unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
>> XEN_DOMCTL_CDF_vpmu);
>> +unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
>> XEN_DOMCTL_CDF_vpmu |
>> +   XEN_DOMCTL_CDF_vpci);
> 
> Is the flag (going to be, with the initial work) okay to have for Dom0
> on Arm?

Hm. Allowing/enabling vPCI for dom0 on ARM should follow or be part of the PCI 
passthrough SMMU series [1]. I'll move this change to the next patch ("xen/arm: 
enable vPCI for dom0") and add a note over there.

[1] https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg00210.html

> 
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -712,7 +712,8 @@ int arch_sanitise_domain_config(struct 
>> xen_domctl_createdomain *config)
>>  return 0;
>>  }
>>  
>> -static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
>> +static bool emulation_flags_ok(const struct domain *d, uint32_t emflags,
>> +   uint32_t cdf)
> 
> While apparently views differ, ./CODING_STYLE wants "unsigned int" to be
> used for the latter two arguments.

OK, I'll change both to unsigned int.

> 
>> @@ -722,14 +723,17 @@ static bool emulation_flags_ok(const struct domain *d, 
>> uint32_t emflags)
>>  if ( is_hvm_domain(d) )
>>  {
>>  if ( is_hardware_domain(d) &&
>> - emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
>> + (!( cdf & XEN_DOMCTL_CDF_vpci ) ||
> 
> Nit: Stray blanks inside the inner parentheses.

OK, I'll fix.

> 
>> +  emflags != (X86_EMU_LAPIC | X86_EMU_IOAPIC)) )
>>  return false;
>>  if ( !is_hardware_domain(d) &&
>> - emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
>> - emflags != X86_EMU_LAPIC )
>> + ((cdf & XEN_DOMCTL_CDF_vpci) ||
>> +  (emflags != X86_EMU_ALL &&
>> +   emflags != X86_EMU_LAPIC)) )
>>  return false;
>>  }
>> -else if ( emflags != 0 && emflags != X86_EMU_PIT )
>> +else if ( (cdf & XEN_DOMCTL_CDF_vpci) ||
> 
> Wouldn't this better be enforced in common code?

Yes, I will move it to xen/common/domain.c:sanitise_domain_config().

> 
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -892,10 +892,11 @@ static struct domain *__init create_dom0(const 
>> module_t *image,
>>  {
>>  dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
>> ((hvm_hap_supported() && !opt_dom0_shadow) ?
>> -XEN_DOMCTL_CDF_hap : 0));
>> +XEN_DOMCTL_CDF_hap : 0) |
>> +   XEN_DOMCTL_CDF_vpci);
> 
> Less of a change and imo slightly neater as a result would be to simply
> put the addition on the same line where CDF_hvm already is. But as with
> many style aspects, views may differ here of course ...

I'll change it.

> 
> Jan



Re: [PATCH] x86/irq: do not insert IRQ_MSI_EMU in emuirq mappings

2023-10-31 Thread Roger Pau Monné
On Tue, Oct 31, 2023 at 03:30:37PM +0200, Xenia Ragiadakou wrote:
> Do not use emuirq mappings for MSIs injected by emulated devices.
> This kind of pirq shares the same emuirq value and is not remapped.

AFAICT adding the extra emuirq mappings is harmless, and just adds
an extra layer of translation?

Or is this causing issues, but we haven't realized because we don't
provide emulated devices that use MSI(-X) by default?

> Fixes: 88fccdd11ca0 ('xen: event channel remapping for emulated MSIs')
> Signed-off-by: Xenia Ragiadakou 
> ---
> 
> Question: is there any strong reason why Linux HVM guests still use pirqs?

Baggage I guess.  I've suggested in the past to switch PIRQs off by
default for HVM, but I had no figures to show how much of a
performance penalty that would be for passthrough devices.

My suggestion would be to introduce an xl.cfg option to select the
availability of PIRQs for HVM guests, and set it to off by default.
You would also need to make sure that migration (or save/restore)
works fine, and that incoming guests from previous Xen versions (that
won't have the option) will result in PIRQs still being enabled.

There's already a XEN_X86_EMU_USE_PIRQ flag in xen_arch_domainconfig,
so you just need to wire the tools side in order to allow selection by
users.

> 
>  xen/arch/x86/irq.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index f42ad539dc..cdc8dc5a55 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -2684,7 +2684,7 @@ int map_domain_emuirq_pirq(struct domain *d, int pirq, 
> int emuirq)
>  }
>  
>  old_emuirq = domain_pirq_to_emuirq(d, pirq);
> -if ( emuirq != IRQ_PT )
> +if ( (emuirq != IRQ_PT) && (emuirq != IRQ_MSI_EMU) )
>  old_pirq = domain_emuirq_to_pirq(d, emuirq);

I think you can just use emuirq >= 0, as we then only need the emuirq
translation for passthrough interrupts, same for the rest of the
changed conditions.

Looking further, the function seems to be useless when called with
emuirq < 0, and hence it might be better to avoid such calls in the
first place?

I have to admit I've always been very confused with the PIRQ logic, so
it's possible I'm missing some relevant stuff here.

Thanks, Roger.



[RFC PATCH v3 3/3] tools/misc: Add xen-vcpus-stats tool

2023-10-31 Thread Matias Ezequiel Vara Larsen
Add a demonstration tool that uses the stats_table resource to
query vcpus' RUNSTATE_running counter for a DomU.

Signed-off-by: Matias Ezequiel Vara Larsen 
---
Changes in v3:
- use memory layout as discussed at
  https://lists.xenproject.org/archives/html/xen-devel/2023-03/msg00383.html
- use virt_*()
- issue xenforeignmemory_close()

Changes in v2:
- use period instead of frec
- rely on version to ensure reading is coherent 

Changes in v1:
- change the name of the tool to xen-vcpus-stats
- set command line parameters in the same order that are passed
- remove header libs.h
- build by default
- remove errno, strerrno, "\n", and identation
- use errx when errno is not needed
- address better the number of pages requested and error msgs
- use the shared_vcpustatspage_t structure
- use the correct frame id when requesting the resource
---
 tools/misc/Makefile  |   6 ++
 tools/misc/xen-vcpus-stats.c | 132 +++
 2 files changed, 138 insertions(+)
 create mode 100644 tools/misc/xen-vcpus-stats.c

diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 8b9558b93f..9c7cb50483 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -51,6 +51,7 @@ TARGETS_COPY += xenpvnetboot
 
 # Everything which needs to be built
 TARGETS_BUILD := $(filter-out $(TARGETS_COPY),$(TARGETS_ALL))
+TARGETS_BUILD += xen-vcpus-stats
 
 # ... including build-only targets
 TARGETS_BUILD += $(TARGETS_BUILD-y)
@@ -139,4 +140,9 @@ xencov: xencov.o
 xen-ucode: xen-ucode.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
 
+xen-vcpus-stats.o: CFLAGS += $(CFLAGS_libxenforeginmemory)
+
+xen-vcpus-stats: xen-vcpus-stats.o
+   $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) 
$(LDLIBS_libxenforeignmemory) $(APPEND_LDFLAGS)
+
 -include $(DEPS_INCLUDE)
diff --git a/tools/misc/xen-vcpus-stats.c b/tools/misc/xen-vcpus-stats.c
new file mode 100644
index 00..f277b6ce8f
--- /dev/null
+++ b/tools/misc/xen-vcpus-stats.c
@@ -0,0 +1,132 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+/*
+ * Note that virt_*() is used when ordering is required between the hypevisor
+ * and the tool domain. This tool is meant to be arch-agnostic so add the
+ * corresponding barrier for each architecture.
+ *
+ */
+#if defined(__x86_64__)
+#define barrier() asm volatile("" ::: "memory")
+#define virt_rmb() barrier()
+#elif defined(__aarch64__)
+#define dmb(opt) asm volatile("dmb " #opt : : : "memory")
+#define virt_rmb() dmb(ishld)
+#else
+#error Please fill in barrier macros
+#endif
+
+static sig_atomic_t interrupted;
+static void close_handler(int signum)
+{
+interrupted = 1;
+}
+
+int main(int argc, char **argv)
+{
+xenforeignmemory_handle *fh;
+xenforeignmemory_resource_handle *res;
+size_t size;
+int rc, domid, period, vcpu;
+xen_vcpu_shmemstats_t *info_shmem;
+xen_shared_vcpustats_t *info;
+struct sigaction act;
+uint32_t seq;
+uint64_t value;
+
+if ( argc != 4 )
+{
+fprintf(stderr, "Usage: %s   \n", argv[0]);
+return 1;
+}
+
+domid = atoi(argv[1]);
+vcpu = atoi(argv[2]);
+period = atoi(argv[3]);
+
+act.sa_handler = close_handler;
+act.sa_flags = 0;
+sigemptyset(_mask);
+sigaction(SIGHUP,  , NULL);
+sigaction(SIGTERM, , NULL);
+sigaction(SIGINT,  , NULL);
+sigaction(SIGALRM, , NULL);
+
+fh = xenforeignmemory_open(NULL, 0);
+
+if ( !fh )
+err(1, "xenforeignmemory_open");
+
+rc = xenforeignmemory_resource_size(
+fh, domid, XENMEM_resource_stats_table,
+XENMEM_resource_stats_table_id_vcpustats, );
+
+if ( rc )
+err(1, "Fail: Get size");
+
+res = xenforeignmemory_map_resource(
+fh, domid, XENMEM_resource_stats_table,
+XENMEM_resource_stats_table_id_vcpustats, 0, size >> XC_PAGE_SHIFT,
+(void **)_shmem, PROT_READ, 0);
+
+if ( !res )
+err(1, "Fail: Map");
+
+if ( info_shmem->magic != VCPU_STATS_MAGIC )
+{
+fprintf(stderr, "Wrong magic number\n");
+return 1;
+}
+
+if ( offsetof(struct vcpu_stats, runstate_running_time) > info_shmem->size 
)
+{
+fprintf(stderr, "The counter is not produced\n");
+return 1;
+}
+
+info = (xen_shared_vcpustats_t*)((void*)info_shmem
+ + info_shmem->offset
+ + info_shmem->size * vcpu);
+
+if ( info->runstate_running_time & ((uint64_t)1 << 63) )
+{
+fprintf(stderr, "The counter is inactived or has overflowed\n");
+return 1;
+}
+
+while ( !interrupted )
+{
+sleep(period);
+do {
+seq = info[vcpu].seq;
+virt_rmb();
+value = info[vcpu].runstate_running_time;
+virt_rmb();
+} while ( (info[vcpu].seq & 1) ||
+  (seq != info[vcpu].seq) );
+if ( 

[RFC PATCH v3 2/3] x86/mm: Do not validate/devalidate PGT_none type

2023-10-31 Thread Matias Ezequiel Vara Larsen
This commit prevents PGT_none type pages to be validated/devalidated.
This is required for the use-case in which a guest-agnostic resource is
allocated. In this case, these pages may be accessible by an "owning" PV
domain. To lock the page from guest pov, pages are required to be marked
with PGT_none with a single type ref. In particular, this commit makes
the stats_table resource type to use this flag during
get_page_and_type(). 

Signed-off-by: Matias Ezequiel Vara Larsen 
---
 xen/arch/x86/mm.c   | 3 ++-
 xen/common/memory.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5812321cae..d2f311abe4 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2787,6 +2787,7 @@ static int _put_page_type(struct page_info *page, 
unsigned int flags,
 {
 case 0:
 if ( unlikely((nx & PGT_type_mask) <= PGT_l4_page_table) &&
+ unlikely((nx & PGT_type_mask) >= PGT_l1_page_table) &&
  likely(nx & (PGT_validated|PGT_partial)) )
 {
 int rc;
@@ -3072,7 +3073,7 @@ static int _get_page_type(struct page_info *page, 
unsigned long type,
  *
  * per validate_page(), non-atomic updates are fine here.
  */
-if ( type == PGT_writable_page || type == PGT_shared_page )
+if ( type == PGT_writable_page || type == PGT_shared_page || type == 
PGT_none )
 page->u.inuse.type_info |= PGT_validated;
 else
 {
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 2acac40c63..e26ba21d75 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1254,7 +1254,7 @@ static int stats_vcpu_alloc_mfn(struct domain *d)
 
 for ( i = 0; i < nr_frames; i++ )
 {
-if ( unlikely(!get_page_and_type([i], d, PGT_writable_page)) )
+if ( unlikely(!get_page_and_type([i], d, PGT_none)) )
 /*
  * The domain can't possibly know about this page yet, so failure
  * here is a clear indication of something fishy going on.
-- 
2.34.1




[RFC PATCH v3 1/3] xen/memory : Add a stats_table resource type

2023-10-31 Thread Matias Ezequiel Vara Larsen
This commit proposes a new mechanism to query the RUNSTATE_running counter for
a given vcpu from a dom0 userspace application. This commit proposes to expose
that counter by using the acquire_resource interface. For this purpose, the
commit adds a new resource named XENMEM_resource_stats_table and a
type-specific resource named XENMEM_resource_stats_table_id_vcpustats. This
type-specific resource is aiming at exposing per-VCPU counters.

The current mechanism relies on the XEN_DOMCTL_getvcpuinfo and holds a single
global domctl_lock for the entire hypercall; and iterate over every vcpu in the
system for every update thus impacting operations that share that lock. This
commit proposes to expose vcpu RUNSTATE_running via the xenforeignmemory
interface thus preventing to issue the hypercall and holding the lock.

For that purpose, a new resource type named XENMEM_resource_stats_table is
added. In particular, the type-specific resource
XENMEM_resource_stats_table_id_vcpustats is added to host per-VCPU counters.
These counters are mapped in frames. The allocation of these frames only
happens if the resource is requested. The number of allocated frames is equal
to the domain's max_vcpus. Frames are released after the domain is destroyed.

To expose this information to a consumer, two structures are required:
1. vcpu_shmem_stats
2. vcpu_stats[]

The memory layout has been discussed at [1]. The laylout looks like:

struct vcpu_shmem_stats {
#define VCPU_STATS_MAGIC 0xaabbccdd
uint32_t magic;
uint32_t offset;  // roundup(sizeof(vcpu_shmem_stats), cacheline_size)
uint32_t size;// sizeof(vcpu_stats)
uint32_t stride;  // roundup(sizeof(vcpu_stats), cacheline_size)
};

struct vcpu_stats {
/*
 * If the least-significant bit of the seq number is set then an update
 * is in progress and the consumer must wait to read a consistent set of
 * values. This mechanism is similar to Linux's seqlock.
 */
uint32_t seq;
uint32 _pad;
/*
 * If the most-significant bit of a counter is set then the counter
 * is inactive and the consumer must ignore its value. Note that this
 * could also indicate that the counter has overflowed.
 */
uint64_t stats_a; // e.g., runstate_running_time
uint64_t stats_b; // e.g.,
uint64_t stats_c; // e.g.,
...
};

All padding fields shall be marked as "inactive". The consumer can't
distinguish inactive from overflowed. Also, the consumer shall always
verify before reading that:

  offsetof(struct vcpu_stats, stats_y) < size.

in case the consumer knows about a counter, e.g., stats_y, that Xen does
not it.

The vcpu_shmem_stats structure exposes a magic number, the size of the
vcpu_stats structure, the offset in which the vcpu_stats structures begin and
the stride for each instance. The address of the vcpu_shmem_stats and
vcpu_stats instances are cache line aligned to prevent cache ping-pong when
accessing per-vcpu elements. In the vcpu_stats structure, most-significant bit
of a counter is set to indicate that either the counter has overflowed or it is
inactive so the consumer must ignore it.

Note that the updating of vcpu's counters is in a hot path, thus, in this 
commit,
copying only happens if it is specifically required.

Note that the resource is extensible in two ways. First, the structure
vcpu_stats can be extended with new per-vcpu counters. To do so, new per-vcpu
counters shall be added after the last element of the structure definition to
not break existing consumers. Second, new type-specific resources can be added
to host a different sort of counters.

[1]
https://lists.xenproject.org/archives/html/xen-devel/2023-03/msg00383.html

Signed-off-by: Matias Ezequiel Vara Larsen 
---
Changes in v3:
- allow to host an arbitrary nuumber of vcpus
- release resource during domain_kill() since it is guest-type
  independent and arch agnostic
- rework stats_table_max_frames()
- use memory layout as discussed at
  https://lists.xenproject.org/archives/html/xen-devel/2023-03/msg00383.html

Changes in v2:
- rework to ensure that guest reads a coherent value by using a version
  number in the vcpu_stats structure
- add version to the vcpu_stats structure

Changes in v1:
- rework the allocation and releasing of the frames
- use the zero frame for per-vcpu counters that are listed as an array
- allocate vcpu stats frames only when the resource is requested
- rewrite commit message
- add the vcpu_stats structure to keep per-vcpu counters
- add the shared_vcpustatspage to keep an array of per-vcpu counters for a
  given domain
- declare the structures in a public header 
- define the vcpustats_page in the domain structure
---
 xen/common/domain.c |   1 +
 xen/common/memory.c | 167 
 xen/common/sched/core.c |  20 +
 xen/include/public/memory.h |   3 +
 xen/include/public/vcpu.h   |  27 ++
 xen/include/xen/mm.h|   2 +
 xen/include/xen/sched.h 

[RFC PATCH v3 0/3] Add a new acquire resource to query vcpu statistics

2023-10-31 Thread Matias Ezequiel Vara Larsen
Hello all and apologies for the delay in sending v3,

the purpose of this RFC is to get feedback about a new acquire resource that
exposes vcpu statistics for a given domain. The current mechanism to get those
statistics is by querying the hypervisor. This mechanism relies on a hypercall
and holds the domctl spinlock during its execution. When a pv tool like xcp-rrdd
periodically samples these counters, it ends up affecting other paths that share
that spinlock. By using acquire resources, the pv tool only requires a few
hypercalls to set the shared memory region and samples are got without issuing
any other hypercall. The original idea has been suggested by Andrew Cooper to
which I have been discussing about how to implement the current PoC. You can
find the RFC patch series at [1]. The series is rebased on top of stable-4.16.

The current series includes a simple pv tool that shows how this new interface 
is
used. This tool maps the counter and periodically samples it.

Any feedback/help would be appreciated.

Thanks, Matias.

[1] https://github.com/MatiasVara/xen/commits/feature_vcpu_stats

Changes in v3:
- use memory layout discussed at
  https://lists.xenproject.org/archives/html/xen-devel/2023-03/msg00383.html
 
Changes in v2:
- rework to ensure that consumer fetches consistent data

Changes in v1:
- rework how the resource is allocated and released
- rework when the resource is allocated that happens only when the resource is
  requested 
- rework the structure shared between the tool and Xen to make it extensible to
  new counters and declare it in a public header

There are still the following questions:
   - resource shall be released when there are no more readers otherwise we keep
 updating it during a hot path

Matias Ezequiel Vara Larsen (3):
  xen/memory : Add a stats_table resource type
  x86/mm: Do not validate/devalidate PGT_none type
  tools/misc: Add xen-vcpus-stats tool

 tools/misc/Makefile  |   6 ++
 tools/misc/xen-vcpus-stats.c | 132 +++
 xen/arch/x86/mm.c|   3 +-
 xen/common/domain.c  |   1 +
 xen/common/memory.c  | 167 +++
 xen/common/sched/core.c  |  20 +
 xen/include/public/memory.h  |   3 +
 xen/include/public/vcpu.h|  27 ++
 xen/include/xen/mm.h |   2 +
 xen/include/xen/sched.h  |   5 ++
 10 files changed, 365 insertions(+), 1 deletion(-)
 create mode 100644 tools/misc/xen-vcpus-stats.c

-- 
2.34.1




Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs

2023-10-31 Thread Jan Beulich
On 31.10.2023 15:15, Stewart Hildebrand wrote:
> On 10/31/23 09:17, Julien Grall wrote:
>> On 31/10/2023 11:03, Jan Beulich wrote:
>>> On 31.10.2023 00:52, Stewart Hildebrand wrote:
 --- a/xen/drivers/passthrough/pci.c
 +++ b/xen/drivers/passthrough/pci.c
 @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
   bus = PCI_BUS(machine_sbdf);
   devfn = PCI_DEVFN(machine_sbdf);
   +    if ( IS_ENABLED(CONFIG_ARM) &&
 + !is_hardware_domain(d) &&
 + !is_system_domain(d) &&
 + (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) 
 )
>>>
>>> I don't think you need the explicit ARM check; that's redundant with
>>> checking !HAS_VPCI_GUEST_SUPPORT.
> 
> Currently that is true. However, this is allowing for the possibility that we 
> eventually may want to enable PCI passthrough for PVH domU using vPCI (e.g. 
> hyperlaunch, or eliminating qemu backend), in which case we may want to 
> enable CONFIG_HAS_VPCI_GUEST_SUPPORT=y on x86.

That's precisely why I'd like to see the ARM check go away here.

>>> It's also not really clear why you
>>> need to check for the system domain here.
> 
> xl pci-assignable-add will assign the device to domIO, which doesn't have 
> vPCI, but it is still a valid assignment. Perhaps an in code comment would be 
> helpful for clarity?

And/or specifically check for DomIO, not any system domain.

Jan



Re: Commit moratorium for branching 4.18

2023-10-31 Thread Henry Wang
Hi all,

> On Nov 1, 2023, at 00:01, Henry Wang  wrote:
> 
> Hi,
> 
>> On Oct 25, 2023, at 18:18, Henry Wang  wrote:
>> 
>> Hi committers,
>> 
>> We will be branching the tree for Xen 4.18 in the next few days. Please 
>> avoid committing any new patches to staging until further notice.
> 
> The branching has been finished. The staging is reopened for 4.19 development.

One small note, given the state that we definitely cannot freeze for another few
weeks and we are technically not 4.18 released, I would say currently the 
staging
is "soft-open”, basically "nothing that's likely to make backports to 4.18 
hard” would
be candidates to be pushed to staging now.

Kind regards,
Henry


> 
> Thanks.
> 
> Kind regards,
> Henry
> 
>> 
>> Kind regards,
>> Henry
> 



Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Luca Fancellu


> On 31 Oct 2023, at 15:36, Julien Grall  wrote:
> 
> 
> 
> On 31/10/2023 15:32, Luca Fancellu wrote:
>>> On 31 Oct 2023, at 15:27, Julien Grall  wrote:
>>> 
>>> Hi,
>>> 
>>> On 31/10/2023 15:12, Luca Fancellu wrote:
> On 31 Oct 2023, at 15:10, Nicola Vetrini  
> wrote:
> 
> On 2023-10-31 15:13, Luca Fancellu wrote:
>>> On 31 Oct 2023, at 13:27, Julien Grall  wrote:
>>> Hi Stefano,
>>> On 30/10/2023 22:49, Stefano Stabellini wrote:
 On Mon, 30 Oct 2023, Julien Grall wrote:
> Hi Nicola,
> On 27/10/2023 16:11, Nicola Vetrini wrote:
>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>> index 8511a189253b..81473fb4 100644
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
>>   - __emulate_2op and __emulate_2op_nobyte
>>   - read_debugreg and write_debugreg
>>  +   * - R7.1
>> + - It is safe to use certain octal constants the way they are 
>> defined
>> +   in specifications, manuals, and algorithm descriptions. Such 
>> places
>> +   are marked safe with a /\* octal-ok \*/ in-code comment, or 
>> with a
>> SAF
>> +   comment (see safe.json).
> Reading this, it is unclear to me why we have two ways to deviate the 
> rule
> r7.1. And more importantely, how would the developper decide which 
> one to use?
 I agree with you on this and we were discussing this topic just this
 morning in the FUSA community call. I think we need a way to do this
 with the SAF framework:
 if (some code with violation) /* SAF-xx-safe */
 This doesn't work today unfortunately. It can only be done this way:
 /* SAF-xx-safe */
 if (some code with violation)
 Which is not always desirable. octal-ok is just an ad-hoc solution for
 one specific violation but we need a generic way to do this. Luca is
 investigating possible ways to support the previous format in SAF.
>>> Why can't we use octal-ok everywhere for now? My point here is to make 
>>> simple for the developper to know what to use.
 I think we should take this patch for now and harmonize it once SAF is
 improved.
>>> The description of the deviation needs some improvement. To give an 
>>> example, with the current wording, one could they can use octal-ok 
>>> everywhere. But above, you are implying that SAF-xx-safe should be
>>> preferred.
>>> I would still strongly prefer if we use octal-ok everywhere because 
>>> this is simple to remember. But if the other are happy to have both 
>>> SAF-XX and octal-ok, then the description needs to be completely 
>>> unambiguous and the patch should contain some explanation why we have 
>>> two different ways to deviate.
>> Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */
>> So that the suppression engine do what it should (currently it doesn’t 
>> suppress the same line, but we could do something about it) and the 
>> developer
>> has a way to understand what is the violation here without going to the 
>> justification database.
> 
> I guess. It could overflow the 80-char limit in 
> xen/arch/x86/hvm/svm/svm.h, though.
 Yeah, but we could rule out something in code_style to allow only this 
 kind of trailing comments to exceed the 80 chars
>>> 
>>> In the past I expressed concerned with this kind of the rule because it is 
>>> not entirely clear how an automatic formatter will be able to check it.
>>> 
>>> Can you clarify whether clang-format would be able to handle your proposed 
>>> rule?
>> So, yesterday Bertrand pointed out a StackOverflow thread for this issue and 
>> if we use ReflowComments: false we should
>> be able to let the line as it is (not tested).
> 
> Wouldn't that prevent reflow for all the comments? If so, I don't think this 
> is we want. Instead, we want to allow reflow for any comments but the one 
> done at the end of the line.

Ok well, I was optimistic, in reality with the option as false, it would anyway 
reflow the line leaving the comment untouched.

E.g. from this:

if ( modrm_mod == MASK_EXTR(instr_modrm, 0300) && /* SAF-2-safe 
octal-ok */
 (modrm_reg & 7) == MASK_EXTR(instr_modrm, 0070) && /* SAF-2-safe 
octal-ok */
 (modrm_rm & 7) == MASK_EXTR(instr_modrm, 0007) ) /* SAF-2-safe 
octal-ok */
return emul_len;

To this:

if ( modrm_mod ==
 MASK_EXTR(instr_modrm, 0300) && /* SAF-2-safe octal-ok */
 (modrm_reg & 7) ==
 MASK_EXTR(instr_modrm, 0070) && /* SAF-2-safe octal-ok */
 (modrm_rm & 7) ==
 MASK_EXTR(instr_modrm, 0007) ) /* SAF-2-safe 

Re: [PATCH v3 0/4] virtio-blk: use blk_io_plug_call() instead of notification BH

2023-10-31 Thread Kevin Wolf
Am 13.09.2023 um 22:00 hat Stefan Hajnoczi geschrieben:
> v3:
> - Add comment pointing to API documentation in .c file [Philippe]
> - Add virtio_notify_irqfd_deferred_fn trace event [Ilya]
> - Remove outdated #include [Ilya]
> v2:
> - Rename blk_io_plug() to defer_call() and move it to util/ so the net
>   subsystem can use it [Ilya]
> - Add defer_call_begin()/end() to thread_pool_completion_bh() to match Linux
>   AIO and io_uring completion batching
> 
> Replace the seldom-used virtio-blk notification BH mechanism with
> blk_io_plug(). This is part of an effort to enable the multi-queue block layer
> in virtio-blk. The notification BH was not multi-queue friendly.
> 
> The blk_io_plug() mechanism improves fio rw=randread bs=4k iodepth=64 
> numjobs=8
> IOPS by ~9% with a single IOThread and 8 vCPUs (this is not even a multi-queue
> block layer configuration) compared to no completion batching. iodepth=1
> decreases by ~1% but this could be noise. Benchmark details are available 
> here:
> https://gitlab.com/stefanha/virt-playbooks/-/tree/blk_io_plug-irqfd

Thanks, applied to the block branch.

Kevin




Re: Commit moratorium for branching 4.18

2023-10-31 Thread Henry Wang
Hi,

> On Oct 25, 2023, at 18:18, Henry Wang  wrote:
> 
> Hi committers,
> 
> We will be branching the tree for Xen 4.18 in the next few days. Please 
> avoid committing any new patches to staging until further notice.

The branching has been finished. The staging is reopened for 4.19 development.

Thanks.

Kind regards,
Henry

> 
> Kind regards,
> Henry




[xen-unstable test] 183626: regressions - FAIL

2023-10-31 Thread osstest service owner
flight 183626 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183626/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 183615
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-installfail REGR. vs. 183615

Tests which are failing intermittently (not blocking):
 test-amd64-i386-examine-uefi  6 xen-install  fail in 183620 pass in 183626
 test-amd64-i386-pair 10 xen-install/src_host fail in 183620 pass in 183626
 test-amd64-i386-examine-bios  6 xen-install  fail in 183620 pass in 183626
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-install  fail pass in 183620

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

Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Julien Grall




On 31/10/2023 15:32, Luca Fancellu wrote:




On 31 Oct 2023, at 15:27, Julien Grall  wrote:

Hi,

On 31/10/2023 15:12, Luca Fancellu wrote:

On 31 Oct 2023, at 15:10, Nicola Vetrini  wrote:

On 2023-10-31 15:13, Luca Fancellu wrote:

On 31 Oct 2023, at 13:27, Julien Grall  wrote:
Hi Stefano,
On 30/10/2023 22:49, Stefano Stabellini wrote:

On Mon, 30 Oct 2023, Julien Grall wrote:

Hi Nicola,
On 27/10/2023 16:11, Nicola Vetrini wrote:

diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 8511a189253b..81473fb4 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
   - __emulate_2op and __emulate_2op_nobyte
   - read_debugreg and write_debugreg
  +   * - R7.1
+ - It is safe to use certain octal constants the way they are defined
+   in specifications, manuals, and algorithm descriptions. Such places
+   are marked safe with a /\* octal-ok \*/ in-code comment, or with a
SAF
+   comment (see safe.json).

Reading this, it is unclear to me why we have two ways to deviate the rule
r7.1. And more importantely, how would the developper decide which one to use?

I agree with you on this and we were discussing this topic just this
morning in the FUSA community call. I think we need a way to do this
with the SAF framework:
if (some code with violation) /* SAF-xx-safe */
This doesn't work today unfortunately. It can only be done this way:
/* SAF-xx-safe */
if (some code with violation)
Which is not always desirable. octal-ok is just an ad-hoc solution for
one specific violation but we need a generic way to do this. Luca is
investigating possible ways to support the previous format in SAF.

Why can't we use octal-ok everywhere for now? My point here is to make simple 
for the developper to know what to use.

I think we should take this patch for now and harmonize it once SAF is
improved.

The description of the deviation needs some improvement. To give an example, 
with the current wording, one could they can use octal-ok everywhere. But 
above, you are implying that SAF-xx-safe should be
preferred.
I would still strongly prefer if we use octal-ok everywhere because this is 
simple to remember. But if the other are happy to have both SAF-XX and 
octal-ok, then the description needs to be completely unambiguous and the patch 
should contain some explanation why we have two different ways to deviate.

Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */
So that the suppression engine do what it should (currently it doesn’t suppress 
the same line, but we could do something about it) and the developer
has a way to understand what is the violation here without going to the 
justification database.


I guess. It could overflow the 80-char limit in xen/arch/x86/hvm/svm/svm.h, 
though.

Yeah, but we could rule out something in code_style to allow only this kind of 
trailing comments to exceed the 80 chars


In the past I expressed concerned with this kind of the rule because it is not 
entirely clear how an automatic formatter will be able to check it.

Can you clarify whether clang-format would be able to handle your proposed rule?


So, yesterday Bertrand pointed out a StackOverflow thread for this issue and if 
we use ReflowComments: false we should
be able to let the line as it is (not tested).


Wouldn't that prevent reflow for all the comments? If so, I don't think 
this is we want. Instead, we want to allow reflow for any comments but 
the one done at the end of the line.


Cheers,

--
Julien Grall



[ANNOUNCE] Call for agenda items for 2 November Community Call @ 16:00 GMT

2023-10-31 Thread Kelly Choi
Hi all,

Please add your proposed agenda and name next to any items in this *link
here* 

If there are any action items that have been resolved, please remove them
from the sheet.

*COMMUNITY CALL INFORMATION*

*CALL LINK: https://meet.jit.si/XenProjectCommunityCall
*

*DATE: 1st Thursday of each month*

*TIME: 16:00 British Time (either BST or GMT)*
*To allow time to switch between meetings, we will start at 16:05.  Aim to
join by 16:00 if possible to allocate time for technical difficulties etc. *

*SIGN UP SHEET:* Please add or remove yourself from the sign-up-sheet to be
CC'd in these emails in this *link here
 *

--

*Dial-in info and pin can be found here:*

https://meet.jit.si/static/dialInInfo.html?room=XenProjectCommunityCall

*Meeting time:*

https://www.timeanddate.com/worldclock/meetingdetails.html?year=2023=11=2=16=0=0=1234=37=224=179

Many thanks,
Kelly Choi

Open Source Community Manager
XenServer, Cloud Software Group


Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Luca Fancellu


> On 31 Oct 2023, at 15:27, Julien Grall  wrote:
> 
> Hi,
> 
> On 31/10/2023 15:12, Luca Fancellu wrote:
>>> On 31 Oct 2023, at 15:10, Nicola Vetrini  wrote:
>>> 
>>> On 2023-10-31 15:13, Luca Fancellu wrote:
> On 31 Oct 2023, at 13:27, Julien Grall  wrote:
> Hi Stefano,
> On 30/10/2023 22:49, Stefano Stabellini wrote:
>> On Mon, 30 Oct 2023, Julien Grall wrote:
>>> Hi Nicola,
>>> On 27/10/2023 16:11, Nicola Vetrini wrote:
 diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
 index 8511a189253b..81473fb4 100644
 --- a/docs/misra/deviations.rst
 +++ b/docs/misra/deviations.rst
 @@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
   - __emulate_2op and __emulate_2op_nobyte
   - read_debugreg and write_debugreg
  +   * - R7.1
 + - It is safe to use certain octal constants the way they are 
 defined
 +   in specifications, manuals, and algorithm descriptions. Such 
 places
 +   are marked safe with a /\* octal-ok \*/ in-code comment, or 
 with a
 SAF
 +   comment (see safe.json).
>>> Reading this, it is unclear to me why we have two ways to deviate the 
>>> rule
>>> r7.1. And more importantely, how would the developper decide which one 
>>> to use?
>> I agree with you on this and we were discussing this topic just this
>> morning in the FUSA community call. I think we need a way to do this
>> with the SAF framework:
>> if (some code with violation) /* SAF-xx-safe */
>> This doesn't work today unfortunately. It can only be done this way:
>> /* SAF-xx-safe */
>> if (some code with violation)
>> Which is not always desirable. octal-ok is just an ad-hoc solution for
>> one specific violation but we need a generic way to do this. Luca is
>> investigating possible ways to support the previous format in SAF.
> Why can't we use octal-ok everywhere for now? My point here is to make 
> simple for the developper to know what to use.
>> I think we should take this patch for now and harmonize it once SAF is
>> improved.
> The description of the deviation needs some improvement. To give an 
> example, with the current wording, one could they can use octal-ok 
> everywhere. But above, you are implying that SAF-xx-safe should be
> preferred.
> I would still strongly prefer if we use octal-ok everywhere because this 
> is simple to remember. But if the other are happy to have both SAF-XX and 
> octal-ok, then the description needs to be completely unambiguous and the 
> patch should contain some explanation why we have two different ways to 
> deviate.
 Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */
 So that the suppression engine do what it should (currently it doesn’t 
 suppress the same line, but we could do something about it) and the 
 developer
 has a way to understand what is the violation here without going to the 
 justification database.
>>> 
>>> I guess. It could overflow the 80-char limit in xen/arch/x86/hvm/svm/svm.h, 
>>> though.
>> Yeah, but we could rule out something in code_style to allow only this kind 
>> of trailing comments to exceed the 80 chars
> 
> In the past I expressed concerned with this kind of the rule because it is 
> not entirely clear how an automatic formatter will be able to check it.
> 
> Can you clarify whether clang-format would be able to handle your proposed 
> rule?

So, yesterday Bertrand pointed out a StackOverflow thread for this issue and if 
we use ReflowComments: false we should
be able to let the line as it is (not tested).

https://clang.llvm.org/docs/ClangFormatStyleOptions.html#reflowcomments



> 
> Cheers,
> 
> -- 
> Julien Grall



Re: [PATCH v5 2/2] CHANGELOG.md: Start new "unstable" section

2023-10-31 Thread Henry Wang
Hi Andrew,

> On Oct 31, 2023, at 23:20, Andrew Cooper  wrote:
> 
> On 31/10/2023 2:49 pm, Henry Wang wrote:
>> Signed-off-by: Henry Wang 
>> ---
>> v5:
>> - Rebase on previous patches.
>> ---
>> CHANGELOG.md | 8 
>> 1 file changed, 8 insertions(+)
>> 
>> diff --git a/CHANGELOG.md b/CHANGELOG.md
>> index 94dbd83894..cbdc9bceac 100644
>> --- a/CHANGELOG.md
>> +++ b/CHANGELOG.md
>> @@ -4,6 +4,14 @@ Notable changes to Xen will be documented in this file.
>> 
>> The format is based on [Keep a 
>> Changelog](https://keepachangelog.com/en/1.0.0/)
>> 
>> +## [unstable 
>> UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
>>  - TBD
> 
> As I've just found it in the checklist, this should be [4.19.0 UNRELEASED].

Actually, good point, somehow we seems to have a mix in previous releases (see 
[1] - as an incorrect
example I took for 4.17). I am ok to use 4.19 instead of unstable.

> 
> Happy to fix on commit.

Thank you very much for doing so.

[1] 
https://github.com/xen-project/xen/commit/6c1c97e24f830a921a23e3b9694e20493c9986ee

Kind regards,
Henry

> 
> ~Andrew




Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Julien Grall

Hi,

On 31/10/2023 15:12, Luca Fancellu wrote:

On 31 Oct 2023, at 15:10, Nicola Vetrini  wrote:

On 2023-10-31 15:13, Luca Fancellu wrote:

On 31 Oct 2023, at 13:27, Julien Grall  wrote:
Hi Stefano,
On 30/10/2023 22:49, Stefano Stabellini wrote:

On Mon, 30 Oct 2023, Julien Grall wrote:

Hi Nicola,
On 27/10/2023 16:11, Nicola Vetrini wrote:

diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 8511a189253b..81473fb4 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
   - __emulate_2op and __emulate_2op_nobyte
   - read_debugreg and write_debugreg
  +   * - R7.1
+ - It is safe to use certain octal constants the way they are defined
+   in specifications, manuals, and algorithm descriptions. Such places
+   are marked safe with a /\* octal-ok \*/ in-code comment, or with a
SAF
+   comment (see safe.json).

Reading this, it is unclear to me why we have two ways to deviate the rule
r7.1. And more importantely, how would the developper decide which one to use?

I agree with you on this and we were discussing this topic just this
morning in the FUSA community call. I think we need a way to do this
with the SAF framework:
if (some code with violation) /* SAF-xx-safe */
This doesn't work today unfortunately. It can only be done this way:
/* SAF-xx-safe */
if (some code with violation)
Which is not always desirable. octal-ok is just an ad-hoc solution for
one specific violation but we need a generic way to do this. Luca is
investigating possible ways to support the previous format in SAF.

Why can't we use octal-ok everywhere for now? My point here is to make simple 
for the developper to know what to use.

I think we should take this patch for now and harmonize it once SAF is
improved.

The description of the deviation needs some improvement. To give an example, 
with the current wording, one could they can use octal-ok everywhere. But 
above, you are implying that SAF-xx-safe should be
preferred.
I would still strongly prefer if we use octal-ok everywhere because this is 
simple to remember. But if the other are happy to have both SAF-XX and 
octal-ok, then the description needs to be completely unambiguous and the patch 
should contain some explanation why we have two different ways to deviate.

Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */
So that the suppression engine do what it should (currently it doesn’t suppress 
the same line, but we could do something about it) and the developer
has a way to understand what is the violation here without going to the 
justification database.


I guess. It could overflow the 80-char limit in xen/arch/x86/hvm/svm/svm.h, 
though.


Yeah, but we could rule out something in code_style to allow only this kind of 
trailing comments to exceed the 80 chars


In the past I expressed concerned with this kind of the rule because it 
is not entirely clear how an automatic formatter will be able to check it.


Can you clarify whether clang-format would be able to handle your 
proposed rule?


Cheers,

--
Julien Grall



Re: [PATCH v5 2/2] CHANGELOG.md: Start new "unstable" section

2023-10-31 Thread Andrew Cooper
On 31/10/2023 2:49 pm, Henry Wang wrote:
> Signed-off-by: Henry Wang 
> ---
> v5:
> - Rebase on previous patches.
> ---
>  CHANGELOG.md | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 94dbd83894..cbdc9bceac 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -4,6 +4,14 @@ Notable changes to Xen will be documented in this file.
>  
>  The format is based on [Keep a 
> Changelog](https://keepachangelog.com/en/1.0.0/)
>  
> +## [unstable 
> UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
>  - TBD

As I've just found it in the checklist, this should be [4.19.0 UNRELEASED].

Happy to fix on commit.

~Andrew



Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Luca Fancellu


> On 31 Oct 2023, at 15:10, Nicola Vetrini  wrote:
> 
> On 2023-10-31 15:13, Luca Fancellu wrote:
>>> On 31 Oct 2023, at 13:27, Julien Grall  wrote:
>>> Hi Stefano,
>>> On 30/10/2023 22:49, Stefano Stabellini wrote:
 On Mon, 30 Oct 2023, Julien Grall wrote:
> Hi Nicola,
> On 27/10/2023 16:11, Nicola Vetrini wrote:
>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>> index 8511a189253b..81473fb4 100644
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
>>   - __emulate_2op and __emulate_2op_nobyte
>>   - read_debugreg and write_debugreg
>>  +   * - R7.1
>> + - It is safe to use certain octal constants the way they are 
>> defined
>> +   in specifications, manuals, and algorithm descriptions. Such 
>> places
>> +   are marked safe with a /\* octal-ok \*/ in-code comment, or with 
>> a
>> SAF
>> +   comment (see safe.json).
> Reading this, it is unclear to me why we have two ways to deviate the rule
> r7.1. And more importantely, how would the developper decide which one to 
> use?
 I agree with you on this and we were discussing this topic just this
 morning in the FUSA community call. I think we need a way to do this
 with the SAF framework:
 if (some code with violation) /* SAF-xx-safe */
 This doesn't work today unfortunately. It can only be done this way:
 /* SAF-xx-safe */
 if (some code with violation)
 Which is not always desirable. octal-ok is just an ad-hoc solution for
 one specific violation but we need a generic way to do this. Luca is
 investigating possible ways to support the previous format in SAF.
>>> Why can't we use octal-ok everywhere for now? My point here is to make 
>>> simple for the developper to know what to use.
 I think we should take this patch for now and harmonize it once SAF is
 improved.
>>> The description of the deviation needs some improvement. To give an 
>>> example, with the current wording, one could they can use octal-ok 
>>> everywhere. But above, you are implying that SAF-xx-safe should be
>>> preferred.
>>> I would still strongly prefer if we use octal-ok everywhere because this is 
>>> simple to remember. But if the other are happy to have both SAF-XX and 
>>> octal-ok, then the description needs to be completely unambiguous and the 
>>> patch should contain some explanation why we have two different ways to 
>>> deviate.
>> Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */
>> So that the suppression engine do what it should (currently it doesn’t 
>> suppress the same line, but we could do something about it) and the developer
>> has a way to understand what is the violation here without going to the 
>> justification database.
> 
> I guess. It could overflow the 80-char limit in xen/arch/x86/hvm/svm/svm.h, 
> though.

Yeah, but we could rule out something in code_style to allow only this kind of 
trailing comments to exceed the 80 chars

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



Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Nicola Vetrini

On 2023-10-31 15:13, Luca Fancellu wrote:

On 31 Oct 2023, at 13:27, Julien Grall  wrote:

Hi Stefano,

On 30/10/2023 22:49, Stefano Stabellini wrote:

On Mon, 30 Oct 2023, Julien Grall wrote:

Hi Nicola,

On 27/10/2023 16:11, Nicola Vetrini wrote:

diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 8511a189253b..81473fb4 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
   - __emulate_2op and __emulate_2op_nobyte
   - read_debugreg and write_debugreg
  +   * - R7.1
+ - It is safe to use certain octal constants the way they are 
defined
+   in specifications, manuals, and algorithm descriptions. 
Such places
+   are marked safe with a /\* octal-ok \*/ in-code comment, or 
with a

SAF
+   comment (see safe.json).


Reading this, it is unclear to me why we have two ways to deviate 
the rule
r7.1. And more importantely, how would the developper decide which 
one to use?

I agree with you on this and we were discussing this topic just this
morning in the FUSA community call. I think we need a way to do this
with the SAF framework:
if (some code with violation) /* SAF-xx-safe */
This doesn't work today unfortunately. It can only be done this way:
/* SAF-xx-safe */
if (some code with violation)
Which is not always desirable. octal-ok is just an ad-hoc solution 
for

one specific violation but we need a generic way to do this. Luca is
investigating possible ways to support the previous format in SAF.


Why can't we use octal-ok everywhere for now? My point here is to make 
simple for the developper to know what to use.


I think we should take this patch for now and harmonize it once SAF 
is

improved.


The description of the deviation needs some improvement. To give an 
example, with the current wording, one could they can use octal-ok 
everywhere. But above, you are implying that SAF-xx-safe should be

preferred.

I would still strongly prefer if we use octal-ok everywhere because 
this is simple to remember. But if the other are happy to have both 
SAF-XX and octal-ok, then the description needs to be completely 
unambiguous and the patch should contain some explanation why we have 
two different ways to deviate.


Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */

So that the suppression engine do what it should (currently it doesn’t 
suppress the same line, but we could do something about it) and the 
developer
has a way to understand what is the violation here without going to the 
justification database.


I guess. It could overflow the 80-char limit in 
xen/arch/x86/hvm/svm/svm.h, though.


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



[PATCH v2] x86/x2apic: introduce a mixed physical/cluster mode

2023-10-31 Thread Roger Pau Monne
The current implementation of x2APIC requires to either use Cluster Logical or
Physical mode for all interrupts.  However the selection of Physical vs Logical
is not done at APIC setup, an APIC can be addressed both in Physical or Logical
destination modes concurrently.

Introduce a new x2APIC mode called mixed, which uses Logical Cluster mode for
IPIs, and Physical mode for external interrupts, thus attempting to use the
best method for each interrupt type.

Using Physical mode for external interrupts allows more vectors to be used, and
interrupt balancing to be more accurate.

Using Logical Cluster mode for IPIs allows less accesses to the ICR register
when sending those, as multiple CPUs can be targeted with a single ICR register
write.

A simple test calling flush_tlb_all() 1 times in a tight loop on a 96 CPU
box gives the following average figures:

Physical mode: 26617931ns
Mixed mode:23865337ns

So ~10% improvement versus plain Physical mode.  Note that Xen uses Cluster
mode by default, and hence is already using the fastest way for IPI delivery at
the cost of reducing the amount of vectors available system-wide.

Make the newly introduced mode the default one.

Suggested-by: Andrew Cooper 
Signed-off-by: Roger Pau Monné 
---
Changes since v1:
 - Add change log entry.
 - Fix indentation and usage of tristate in Kconfig.
 - Adjust comment regarding hooks used by external interrupts in
   apic_x2apic_mixed.
---
 CHANGELOG.md  |  6 ++
 docs/misc/xen-command-line.pandoc | 12 
 xen/arch/x86/Kconfig  | 35 ++--
 xen/arch/x86/genapic/x2apic.c | 91 ++-
 4 files changed, 114 insertions(+), 30 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ca796969990..9b04849b0336 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,12 @@ Notable changes to Xen will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
+## [unstable 
UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
 - TBD
+
+### Added
+ - On x86 introduce a new x2APIC driver that uses Cluster Logical addressing
+   mode for IPIs and Physical addressing mode for external interrupts.
+
 ## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-XX-XX
 
 ### Changed
diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index 6b07d0f3a17f..cbe9b4802c61 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2802,6 +2802,15 @@ the watchdog.
 
 Permit use of x2apic setup for SMP environments.
 
+### x2apic-mode (x86)
+> `= physical | cluster | mixed`
+
+> Default: `physical` if **FADT** mandates physical mode, otherwise set at
+>  build time by CONFIG_X2APIC_{PHYSICAL,LOGICAL,MIXED}.
+
+In the case that x2apic is in use, this option switches between modes to
+address APICs in the system as interrupt destinations.
+
 ### x2apic_phys (x86)
 > `= `
 
@@ -2812,6 +2821,9 @@ In the case that x2apic is in use, this option switches 
between physical and
 clustered mode.  The default, given no hint from the **FADT**, is cluster
 mode.
 
+**WARNING: `x2apic_phys` is deprecated and superseded by `x2apic-mode`.
+The latter takes precedence if both are set.**
+
 ### xenheap_megabytes (arm32)
 > `= `
 
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index eac77573bd75..cd9286f295e5 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -228,11 +228,18 @@ config XEN_ALIGN_2M
 
 endchoice
 
-config X2APIC_PHYSICAL
-   bool "x2APIC Physical Destination mode"
+choice
+   prompt "x2APIC Destination mode"
+   default X2APIC_MIXED
help
- Use x2APIC Physical Destination mode by default when available.
+ Select APIC addressing when x2APIC is enabled.
+
+ The default mode is mixed which should provide the best aspects
+ of both physical and cluster modes.
 
+config X2APIC_PHYSICAL
+   bool "Physical Destination mode"
+   help
  When using this mode APICs are addressed using the Physical
  Destination mode, which allows using all dynamic vectors on each
  CPU independently.
@@ -242,9 +249,27 @@ config X2APIC_PHYSICAL
  destination inter processor interrupts (IPIs) slightly slower than
  Logical Destination mode.
 
- The mode when this option is not selected is Logical Destination.
+config X2APIC_CLUSTER
+   bool "Cluster Destination mode"
+   help
+ When using this mode APICs are addressed using the Cluster Logical
+ Destination mode.
+
+ Cluster Destination has the benefit of sending IPIs faster since
+ multiple APICs can be targeted as destinations of a single IPI.
+ However the vector space is shared between all CPUs on the cluster,
+ and hence using this mode reduces the number of available vectors
+ 

Re: [PATCH v5 2/2] CHANGELOG.md: Start new "unstable" section

2023-10-31 Thread Julien Grall

Hi Henry,

On 31/10/2023 14:49, Henry Wang wrote:

Signed-off-by: Henry Wang 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



Re: [PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Henry Wang
Hi both,

> On Oct 31, 2023, at 22:19, Julien Grall  wrote:
> On 31/10/2023 14:06, Andrew Cooper wrote:
>> On 31/10/2023 1:45 pm, Julien Grall wrote:
>>> If you want to go down that route, then please update the
>>> docs/process/branching-checklist.txt. Otherwise, I will continue to do
>>> as I did previously.
>> It *is* in the checklist, and for all previous releases even 4.17, the
>> staging section was opened at the time of branching.
> 
> It doesn't tell me when it has to be done. The difference with 4.17 is we 
> don't yet have a date for the release. Hence why I delayed.

I’ve sent the updated [1] out, hopefully this will make both of you happy 
(Still I am thinking
both of you are actually mentioning the same thing, i.e. starting a new 
unstable section after
the branching).

[1] 
https://lore.kernel.org/xen-devel/20231031144925.2416266-1-henry.w...@arm.com/

Kind regards,
Henry

Re: [PATCH v5 1/2] CHANGELOG.md: Finalize the 4.18 release date

2023-10-31 Thread Julien Grall




On 31/10/2023 14:49, Henry Wang wrote:

Signed-off-by: Henry Wang 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall



[PATCH v5 1/2] CHANGELOG.md: Finalize the 4.18 release date

2023-10-31 Thread Henry Wang
Signed-off-by: Henry Wang 
---
v5:
- New patch
---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ca7969699..94dbd83894 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,7 @@ Notable changes to Xen will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
-## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-XX-XX
+## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-11-16
 
 ### Changed
  - Repurpose command line gnttab_max_{maptrack_,}frames options so they don't
-- 
2.25.1




[PATCH v5 2/2] CHANGELOG.md: Start new "unstable" section

2023-10-31 Thread Henry Wang
Signed-off-by: Henry Wang 
---
v5:
- Rebase on previous patches.
---
 CHANGELOG.md | 8 
 1 file changed, 8 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94dbd83894..cbdc9bceac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,14 @@ Notable changes to Xen will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
+## [unstable 
UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
 - TBD
+
+### Changed
+
+### Added
+
+### Removed
+
 ## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-11-16
 
 ### Changed
-- 
2.25.1




[PATCH v5 0/2] Finalize the 4.18 release date

2023-10-31 Thread Henry Wang
Hi all,

This series finializes the 4.18 release date and starts a
new unstable release after branching.

Thanks.

Henry Wang (2):
  CHANGELOG.md: Finalize the 4.18 release date
  CHANGELOG.md: Start new "unstable" section

 CHANGELOG.md | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

-- 
2.25.1




Re: [PATCH v4] CHANGELOG.md: Start new "unstable" section

2023-10-31 Thread Henry Wang
Hi,

> On Oct 31, 2023, at 22:34, Henry Wang  wrote:
> 
> Signed-off-by: Henry Wang 
> ---
> v4:
> - Set the release date.
> ---
> CHANGELOG.md | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 3ca7969699..cbdc9bceac 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -4,7 +4,15 @@ Notable changes to Xen will be documented in this file.
> 
> The format is based on [Keep a 
> Changelog](https://keepachangelog.com/en/1.0.0/)
> 
> -## 
> [4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
>  - 2023-XX-XX
> +## [unstable 
> UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
>  - TBD
> +
> +### Changed
> +
> +### Added
> +
> +### Removed
> +
> +## 
> [4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
>  - 2023-11-16

To be honest I will split them for the convenience of the release technician’s 
work. Sorry..

Kind regards,
Henry



[PATCH v4] CHANGELOG.md: Start new "unstable" section

2023-10-31 Thread Henry Wang
Signed-off-by: Henry Wang 
---
v4:
- Set the release date.
---
 CHANGELOG.md | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ca7969699..cbdc9bceac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,15 @@ Notable changes to Xen will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
-## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-XX-XX
+## [unstable 
UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
 - TBD
+
+### Changed
+
+### Added
+
+### Removed
+
+## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-11-16
 
 ### Changed
  - Repurpose command line gnttab_max_{maptrack_,}frames options so they don't
-- 
2.25.1




Re: [PATCH v1 22/29] xen/asm-generic: introduce stub header delay.h

2023-10-31 Thread Oleksii
Instead of introducing stub header for delay.h it was decided to remove
 in a separate patch:
https://lore.kernel.org/xen-devel/3d55bce44bd6ab9973cbe0ea2fc136cc44d35df2.1698759633.git.oleksii.kuroc...@gmail.com/T/#u

~ Oleksii



[PATCH v2] xen: remove

2023-10-31 Thread Oleksii Kurochko
 only declares udelay() function so udelay()
declaration was moved to xen/delay.h.

For x86, __udelay() was renamed to udelay() and removed
inclusion of  in x86 code.

For ppc, udelay() stub definition was moved to ppc/stubs.c.

Suggested-by: Jan Beulich 
Signed-off-by: Oleksii Kurochko 
Reviewed-by: Jan Beulich 
---
Changes in v2:
 - rebase on top of the latest staging.
 - add Suggested-by:/Reviewed-by: Jan Beulich .
 - remove  for PPC.
 - remove changes related to RISC-V's  as they've not
   introduced in staging branch yet.
---
 xen/arch/arm/include/asm/delay.h  | 14 --
 xen/arch/ppc/include/asm/delay.h  | 12 
 xen/arch/ppc/stubs.c  |  7 +++
 xen/arch/x86/cpu/microcode/core.c |  2 +-
 xen/arch/x86/delay.c  |  2 +-
 xen/arch/x86/include/asm/delay.h  | 13 -
 xen/include/xen/delay.h   |  2 +-
 7 files changed, 10 insertions(+), 42 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/delay.h
 delete mode 100644 xen/arch/ppc/include/asm/delay.h
 delete mode 100644 xen/arch/x86/include/asm/delay.h

diff --git a/xen/arch/arm/include/asm/delay.h b/xen/arch/arm/include/asm/delay.h
deleted file mode 100644
index 042907d9d5..00
--- a/xen/arch/arm/include/asm/delay.h
+++ /dev/null
@@ -1,14 +0,0 @@
-#ifndef _ARM_DELAY_H
-#define _ARM_DELAY_H
-
-extern void udelay(unsigned long usecs);
-
-#endif /* defined(_ARM_DELAY_H) */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/ppc/include/asm/delay.h b/xen/arch/ppc/include/asm/delay.h
deleted file mode 100644
index da6635888b..00
--- a/xen/arch/ppc/include/asm/delay.h
+++ /dev/null
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-#ifndef __ASM_PPC_DELAY_H__
-#define __ASM_PPC_DELAY_H__
-
-#include 
-
-static inline void udelay(unsigned long usecs)
-{
-BUG_ON("unimplemented");
-}
-
-#endif /* __ASM_PPC_DELAY_H__ */
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index 4c276b0e39..a96e45626d 100644
--- a/xen/arch/ppc/stubs.c
+++ b/xen/arch/ppc/stubs.c
@@ -337,3 +337,10 @@ int __init parse_arch_dom0_param(const char *s, const char 
*e)
 {
 BUG_ON("unimplemented");
 }
+
+/* delay.c */
+
+void udelay(unsigned long usecs)
+{
+BUG_ON("unimplemented");
+}
diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index 65ebeb50de..22d5e04552 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -23,6 +23,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -35,7 +36,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c
index 2662c26272..b3a41881a1 100644
--- a/xen/arch/x86/delay.c
+++ b/xen/arch/x86/delay.c
@@ -15,7 +15,7 @@
 #include 
 #include 
 
-void __udelay(unsigned long usecs)
+void udelay(unsigned long usecs)
 {
 unsigned long ticks = usecs * (cpu_khz / 1000);
 unsigned long s, e;
diff --git a/xen/arch/x86/include/asm/delay.h b/xen/arch/x86/include/asm/delay.h
deleted file mode 100644
index 9be2f46590..00
--- a/xen/arch/x86/include/asm/delay.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef _X86_DELAY_H
-#define _X86_DELAY_H
-
-/*
- * Copyright (C) 1993 Linus Torvalds
- *
- * Delay routines calling functions in arch/i386/lib/delay.c
- */
-
-extern void __udelay(unsigned long usecs);
-#define udelay(n) __udelay(n)
-
-#endif /* defined(_X86_DELAY_H) */
diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h
index 9150226271..8fd3b8f99f 100644
--- a/xen/include/xen/delay.h
+++ b/xen/include/xen/delay.h
@@ -3,7 +3,7 @@
 
 /* Copyright (C) 1993 Linus Torvalds */
 
-#include 
+void udelay(unsigned long usecs);
 
 static inline void mdelay(unsigned long msec)
 {
-- 
2.41.0




Re: Cambridge University Talk - 9th November 2023

2023-10-31 Thread Roger Pau Monné
On Tue, Oct 31, 2023 at 01:05:11PM +, Ayan Kumar Halder wrote:
> Hi Xen Maintainers/developers,
> 
> 
> As part of my talk, I wish to provide some examples of tasks that a newbie
> can easily pick up and contribute.
> 
> This need not be a dedicated project, but something that can be contributed
> on an ad-hoc basis.
> 
> The idea is to get more people interested in Xen project. :)
> 
> 
> I found some examples of this :-
> 
> 1. Misra C fixes - Refer "Misra rule 10.3 violations report script" . Luca
> has provided an awesome script to identify the MISRA violations. This can be
> used to provide fixes.

TBH, I think doing MISRA fixes is not that attractive for a new comer,
as those are (mostly?) non-functional fixes, but I might be wrong.

> 2. https://wiki.xenproject.org/wiki/Outreach_Program_Projects - I think this
> page provides some pointers, but I am not sure if this is up to date.

I'm not sure how up to date this is.

> 
> Please let me know if there are more of these examples.

gitlab contains some:

https://gitlab.com/groups/xen-project/-/issues/?label_name%5B%5D=Difficulty%3A%3A1-GOOD%20FIRST%20ISSUE
https://gitlab.com/groups/xen-project/-/issues/?label_name%5B%5D=Difficulty%3A%3A1-EASY

Regards, Roger.



Re: [PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Julien Grall




On 31/10/2023 14:06, Andrew Cooper wrote:

On 31/10/2023 1:45 pm, Julien Grall wrote:

If you want to go down that route, then please update the
docs/process/branching-checklist.txt. Otherwise, I will continue to do
as I did previously.


It *is* in the checklist, and for all previous releases even 4.17, the
staging section was opened at the time of branching.


It doesn't tell me when it has to be done. The difference with 4.17 is 
we don't yet have a date for the release. Hence why I delayed.




The thing that is different between 4.17 and previously is that 4.17
called it "unstable" where previously (and in the checklist) it says to
make the new section match the updated Xen major/minor number.



I still don't see the problem of delaying the CHANGELOG as I decided to 
do. If you are not happy with it, how about you take over the release 
technician process? It is already complex enough that I don't need 
someone else to tell me exactly how I should do it.


Cheers,

--
Julien Grall



Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs

2023-10-31 Thread Stewart Hildebrand
On 10/31/23 09:17, Julien Grall wrote:
> Hi,
> 
> On 31/10/2023 11:03, Jan Beulich wrote:
>> On 31.10.2023 00:52, Stewart Hildebrand wrote:
>>> --- a/xen/drivers/passthrough/pci.c
>>> +++ b/xen/drivers/passthrough/pci.c
>>> @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
>>>   bus = PCI_BUS(machine_sbdf);
>>>   devfn = PCI_DEVFN(machine_sbdf);
>>>   +    if ( IS_ENABLED(CONFIG_ARM) &&
>>> + !is_hardware_domain(d) &&
>>> + !is_system_domain(d) &&
>>> + (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )
>>
>> I don't think you need the explicit ARM check; that's redundant with
>> checking !HAS_VPCI_GUEST_SUPPORT.

Currently that is true. However, this is allowing for the possibility that we 
eventually may want to enable PCI passthrough for PVH domU using vPCI (e.g. 
hyperlaunch, or eliminating qemu backend), in which case we may want to enable 
CONFIG_HAS_VPCI_GUEST_SUPPORT=y on x86.

>> It's also not really clear why you
>> need to check for the system domain here.

xl pci-assignable-add will assign the device to domIO, which doesn't have vPCI, 
but it is still a valid assignment. Perhaps an in code comment would be helpful 
for clarity?

> 
> I might be missing but I wouldn't expect the domain to have vPCI enabled if 
> CONFIG_HAVE_VPCI_GUEST_SUPPORT=n. So why can't this simply be:
> 
> if ( !has_vcpi(d) )
> {
>    ...
> }

Right, the CONFIG_HAVE_VPCI_GUEST_SUPPORT check here is not strictly needed 
because this case is already caught by the other half of this patch in 
xen/arch/arm/vpci.c. This simplifies it to:

if ( IS_ENABLED(CONFIG_ARM) &&
 !is_hardware_domain(d) &&
 !is_system_domain(d) /* !domIO */ &&
 !has_vpci(d) )

On x86, unless I misunderstood something, I think it's valid to assign PCI 
devices to a domU without has_vpci().

BTW, it's valid for has_vpci() to be true and CONFIG_HAVE_VPCI_GUEST_SUPPORT=n 
for dom0.



Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Luca Fancellu


> On 31 Oct 2023, at 13:27, Julien Grall  wrote:
> 
> Hi Stefano,
> 
> On 30/10/2023 22:49, Stefano Stabellini wrote:
>> On Mon, 30 Oct 2023, Julien Grall wrote:
>>> Hi Nicola,
>>> 
>>> On 27/10/2023 16:11, Nicola Vetrini wrote:
 diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
 index 8511a189253b..81473fb4 100644
 --- a/docs/misra/deviations.rst
 +++ b/docs/misra/deviations.rst
 @@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
- __emulate_2op and __emulate_2op_nobyte
- read_debugreg and write_debugreg
   +   * - R7.1
 + - It is safe to use certain octal constants the way they are defined
 +   in specifications, manuals, and algorithm descriptions. Such places
 +   are marked safe with a /\* octal-ok \*/ in-code comment, or with a
 SAF
 +   comment (see safe.json).
>>> 
>>> Reading this, it is unclear to me why we have two ways to deviate the rule
>>> r7.1. And more importantely, how would the developper decide which one to 
>>> use?
>> I agree with you on this and we were discussing this topic just this
>> morning in the FUSA community call. I think we need a way to do this
>> with the SAF framework:
>> if (some code with violation) /* SAF-xx-safe */
>> This doesn't work today unfortunately. It can only be done this way:
>> /* SAF-xx-safe */
>> if (some code with violation)
>> Which is not always desirable. octal-ok is just an ad-hoc solution for
>> one specific violation but we need a generic way to do this. Luca is
>> investigating possible ways to support the previous format in SAF.
> 
> Why can't we use octal-ok everywhere for now? My point here is to make simple 
> for the developper to know what to use.
> 
>> I think we should take this patch for now and harmonize it once SAF is
>> improved.
> 
> The description of the deviation needs some improvement. To give an example, 
> with the current wording, one could they can use octal-ok everywhere. But 
> above, you are implying that SAF-xx-safe should be
> preferred.
> 
> I would still strongly prefer if we use octal-ok everywhere because this is 
> simple to remember. But if the other are happy to have both SAF-XX and 
> octal-ok, then the description needs to be completely unambiguous and the 
> patch should contain some explanation why we have two different ways to 
> deviate.

Would it be ok to have both, for example: /* SAF-XX-safe octal-ok */

So that the suppression engine do what it should (currently it doesn’t suppress 
the same line, but we could do something about it) and the developer
has a way to understand what is the violation here without going to the 
justification database.




Re: [PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Henry Wang
Hi Julien, Andrew,

> On Oct 31, 2023, at 21:45, Julien Grall  wrote:
> On 31/10/2023 13:38, Andrew Cooper wrote:
>> On 31/10/2023 1:31 pm, Julien Grall wrote:
>>> Hi,
>>> 
>>> On 31/10/2023 13:19, Andrew Cooper wrote:
 Signed-off-by: Andrew Cooper 
>>> 
>>> Henry already provided a similar patch [1]. The only reason it is not
>>> yet committed is because we haven't yet set a final date for 4.18 and
>>> I want to avoid any clash when that patch will appear.
>>> 
>>> Cheers,
>>> 
>>> [1] 20231023092123.1756426-5-henry.w...@arm.com
>> This section should not have been deleted in d9f07b06cfc9.
> 
> Why? This has always been our process. We should not ship 4.18 with the 
> UNSTABLE section. So it was correct to delete it in d9f07b06cfc9.

I agree with Julien on this. removing the “unstable” section is the tradition 
that we had previously,
see below (I am using github link for a easier finding of history commits):
- [1] for 4.15
- [2] for 4.16
- [3] for 4.17

I am not sure there is any specific reason for changing the existing process.

The other two patches in this series looks good and I’ve acked both of them. 
Thanks for taking care
of it.

[1] 
https://github.com/xen-project/xen/commit/b8eaedbb51e8e51808728d7de392d3e9117fdece
[2] 
https://github.com/xen-project/xen/commit/eef266eb770128db0d5258009b744f0e0c31c9bd
[3] 
https://github.com/xen-project/xen/commit/0829a2f3fcaba9233dfdce9a8cee9d126a51bd4d

Kind regards,
Henry

Re: [PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Andrew Cooper
On 31/10/2023 1:45 pm, Julien Grall wrote:
> If you want to go down that route, then please update the
> docs/process/branching-checklist.txt. Otherwise, I will continue to do
> as I did previously.

It *is* in the checklist, and for all previous releases even 4.17, the
staging section was opened at the time of branching.

The thing that is different between 4.17 and previously is that 4.17
called it "unstable" where previously (and in the checklist) it says to
make the new section match the updated Xen major/minor number.

~Andrew



Re: [PATCH for-4.18 2/3] CHANGELOG: More 4.18 content

2023-10-31 Thread Henry Wang
Hi Andrew,

> On Oct 31, 2023, at 21:19, Andrew Cooper  wrote:
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Henry Wang 
Release-acked-by: Henry Wang 

Kind regards,
Henry




Re: [PATCH for-4.18 1/3] CHANGELOG: Reformat

2023-10-31 Thread Henry Wang
Hi Andrew,

> On Oct 31, 2023, at 21:19, Andrew Cooper  wrote:
> 
> Collect all x86 and ARM changes together instead of having them scattered.
> Tweak grammar as necessary.
> 
> No change.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Henry Wang 
Release-acked-by: Henry Wang 

Kind regards,
Henry




Re: [PATCH for-4.18] docs: Fix IOMMU command line docs some more

2023-10-31 Thread Andrew Cooper
On 31/10/2023 1:45 pm, Roger Pau Monné wrote:
> On Tue, Oct 31, 2023 at 01:29:04PM +, Andrew Cooper wrote:
>> On 31/10/2023 12:24 pm, Roger Pau Monné wrote:
>>> On Tue, Oct 31, 2023 at 12:02:15PM +, Andrew Cooper wrote:
 Make the command line docs match the actual implementation, and state that 
 the
 default behaviour is selected at compile time.

 Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices 
 optional")
 Signed-off-by: Andrew Cooper 
> Reviewed-by: Roger Pau Monné 

Thanks.

>
 ---
 CC: Jan Beulich 
 CC: Roger Pau Monné 
 CC: Wei Liu 
 CC: Marek Marczykowski-Górecki 
 CC: Henry Wang 
 ---
  docs/misc/xen-command-line.pandoc | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

 diff --git a/docs/misc/xen-command-line.pandoc 
 b/docs/misc/xen-command-line.pandoc
 index 6b07d0f3a17f..9a19a04157cb 100644
 --- a/docs/misc/xen-command-line.pandoc
 +++ b/docs/misc/xen-command-line.pandoc
 @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon 
 accesses to that port.
  > Default: `new` unless directed-EOI is supported
  
  ### iommu
 -= List of [ , verbose, debug, force, required, 
 quarantine[=scratch-page],
 += List of [ , verbose, debug, force, required,
 +quarantine=|scratch-page,
>>> I think this should be quarantine=[|scratch-page], as just using
>>> iommu=quarantine is a valid syntax and will enable basic quarantine.
>>> IOW: the bool or scratch-page parameters are optional.
>> = already has that meaning, and this is the form we use elsewhere.
> I guess I got confused by some other options using `[ ]` to denote
> optional parameters, but I see it's not used by all of them.

Yeah, it's a mess, sadly.  One of many things I've not had time to fix,
but at least this is closer to the normal syntax than before.

~Andrew



Re: [PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Julien Grall




On 31/10/2023 13:38, Andrew Cooper wrote:

On 31/10/2023 1:31 pm, Julien Grall wrote:

Hi,

On 31/10/2023 13:19, Andrew Cooper wrote:

Signed-off-by: Andrew Cooper 


Henry already provided a similar patch [1]. The only reason it is not
yet committed is because we haven't yet set a final date for 4.18 and
I want to avoid any clash when that patch will appear.

Cheers,

[1] 20231023092123.1756426-5-henry.w...@arm.com


This section should not have been deleted in d9f07b06cfc9.


Why? This has always been our process. We should not ship 4.18 with the 
UNSTABLE section. So it was correct to delete it in d9f07b06cfc9.




It's fine to have an unstable section before the 4.18 date is confirmed,
and the section must exist before staging is re-opened for 4.19 content.


I disagree. 4.19 will not be fully re-open until we finally release. So 
I wouldn't expect any new features to be merged.




I don't mind which of these two patches gets committed, but one of them
is getting committed today ahead of staging re-opening.  Part of
branching ought to ensure that this section exists.


If you want to go down that route, then please update the 
docs/process/branching-checklist.txt. Otherwise, I will continue to do 
as I did previously.


Cheers,

--
Julien Grall



Re: [PATCH for-4.18] docs: Fix IOMMU command line docs some more

2023-10-31 Thread Roger Pau Monné
On Tue, Oct 31, 2023 at 01:29:04PM +, Andrew Cooper wrote:
> On 31/10/2023 12:24 pm, Roger Pau Monné wrote:
> > On Tue, Oct 31, 2023 at 12:02:15PM +, Andrew Cooper wrote:
> >> Make the command line docs match the actual implementation, and state that 
> >> the
> >> default behaviour is selected at compile time.
> >>
> >> Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices 
> >> optional")
> >> Signed-off-by: Andrew Cooper 

Reviewed-by: Roger Pau Monné 

> >> ---
> >> CC: Jan Beulich 
> >> CC: Roger Pau Monné 
> >> CC: Wei Liu 
> >> CC: Marek Marczykowski-Górecki 
> >> CC: Henry Wang 
> >> ---
> >>  docs/misc/xen-command-line.pandoc | 6 --
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/docs/misc/xen-command-line.pandoc 
> >> b/docs/misc/xen-command-line.pandoc
> >> index 6b07d0f3a17f..9a19a04157cb 100644
> >> --- a/docs/misc/xen-command-line.pandoc
> >> +++ b/docs/misc/xen-command-line.pandoc
> >> @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon 
> >> accesses to that port.
> >>  > Default: `new` unless directed-EOI is supported
> >>  
> >>  ### iommu
> >> -= List of [ , verbose, debug, force, required, 
> >> quarantine[=scratch-page],
> >> += List of [ , verbose, debug, force, required,
> >> +quarantine=|scratch-page,
> > I think this should be quarantine=[|scratch-page], as just using
> > iommu=quarantine is a valid syntax and will enable basic quarantine.
> > IOW: the bool or scratch-page parameters are optional.
> 
> = already has that meaning, and this is the form we use elsewhere.

I guess I got confused by some other options using `[ ]` to denote
optional parameters, but I see it's not used by all of them.

Thanks, Roger.



Re: [PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Andrew Cooper
On 31/10/2023 1:31 pm, Julien Grall wrote:
> Hi,
>
> On 31/10/2023 13:19, Andrew Cooper wrote:
>> Signed-off-by: Andrew Cooper 
>
> Henry already provided a similar patch [1]. The only reason it is not
> yet committed is because we haven't yet set a final date for 4.18 and
> I want to avoid any clash when that patch will appear.
>
> Cheers,
>
> [1] 20231023092123.1756426-5-henry.w...@arm.com

This section should not have been deleted in d9f07b06cfc9.

It's fine to have an unstable section before the 4.18 date is confirmed,
and the section must exist before staging is re-opened for 4.19 content.

I don't mind which of these two patches gets committed, but one of them
is getting committed today ahead of staging re-opening.  Part of
branching ought to ensure that this section exists.

Henry, your choice.

~Andrew



Re: [PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Julien Grall

Hi,

On 31/10/2023 13:19, Andrew Cooper wrote:

Signed-off-by: Andrew Cooper 


Henry already provided a similar patch [1]. The only reason it is not 
yet committed is because we haven't yet set a final date for 4.18 and I 
want to avoid any clash when that patch will appear.


Cheers,

[1] 20231023092123.1756426-5-henry.w...@arm.com


---
CC: George Dunlap 
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Wei Liu 
CC: Julien Grall 
CC: Roger Pau Monné 
CC: Henry Wang 
---
  CHANGELOG.md | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a827054cf27d..cf0c9c3f8cb9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@ Notable changes to Xen will be documented in this file.
  
  The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
  
+## [unstable UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging) - TBD

+
  ## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-XX-XX
  
  ### Changed


--
Julien Grall



[PATCH] x86/irq: do not insert IRQ_MSI_EMU in emuirq mappings

2023-10-31 Thread Xenia Ragiadakou
Do not use emuirq mappings for MSIs injected by emulated devices.
This kind of pirq shares the same emuirq value and is not remapped.

Fixes: 88fccdd11ca0 ('xen: event channel remapping for emulated MSIs')
Signed-off-by: Xenia Ragiadakou 
---

Question: is there any strong reason why Linux HVM guests still use pirqs?

 xen/arch/x86/irq.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index f42ad539dc..cdc8dc5a55 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2684,7 +2684,7 @@ int map_domain_emuirq_pirq(struct domain *d, int pirq, 
int emuirq)
 }
 
 old_emuirq = domain_pirq_to_emuirq(d, pirq);
-if ( emuirq != IRQ_PT )
+if ( (emuirq != IRQ_PT) && (emuirq != IRQ_MSI_EMU) )
 old_pirq = domain_emuirq_to_pirq(d, emuirq);
 
 if ( (old_emuirq != IRQ_UNBOUND && (old_emuirq != emuirq) ) ||
@@ -2699,8 +2699,8 @@ int map_domain_emuirq_pirq(struct domain *d, int pirq, 
int emuirq)
 if ( !info )
 return -ENOMEM;
 
-/* do not store emuirq mappings for pt devices */
-if ( emuirq != IRQ_PT )
+/* do not store emuirq mappings for pt devices and emulated MSIs */
+if ( (emuirq != IRQ_PT) && (emuirq != IRQ_MSI_EMU) )
 {
 int err = radix_tree_insert(>arch.hvm.emuirq_pirq, emuirq,
 radix_tree_int_to_ptr(pirq));
@@ -2753,7 +2753,7 @@ int unmap_domain_pirq_emuirq(struct domain *d, int pirq)
 info->arch.hvm.emuirq = IRQ_UNBOUND;
 pirq_cleanup_check(info, d);
 }
-if ( emuirq != IRQ_PT )
+if ( (emuirq != IRQ_PT) && (emuirq != IRQ_MSI_EMU) )
 radix_tree_delete(>arch.hvm.emuirq_pirq, emuirq);
 
  done:
-- 
2.34.1




Re: [PATCH for-4.18] docs: Fix IOMMU command line docs some more

2023-10-31 Thread Andrew Cooper
On 31/10/2023 12:24 pm, Roger Pau Monné wrote:
> On Tue, Oct 31, 2023 at 12:02:15PM +, Andrew Cooper wrote:
>> Make the command line docs match the actual implementation, and state that 
>> the
>> default behaviour is selected at compile time.
>>
>> Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices 
>> optional")
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Jan Beulich 
>> CC: Roger Pau Monné 
>> CC: Wei Liu 
>> CC: Marek Marczykowski-Górecki 
>> CC: Henry Wang 
>> ---
>>  docs/misc/xen-command-line.pandoc | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/misc/xen-command-line.pandoc 
>> b/docs/misc/xen-command-line.pandoc
>> index 6b07d0f3a17f..9a19a04157cb 100644
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses 
>> to that port.
>>  > Default: `new` unless directed-EOI is supported
>>  
>>  ### iommu
>> -= List of [ , verbose, debug, force, required, 
>> quarantine[=scratch-page],
>> += List of [ , verbose, debug, force, required,
>> +quarantine=|scratch-page,
> I think this should be quarantine=[|scratch-page], as just using
> iommu=quarantine is a valid syntax and will enable basic quarantine.
> IOW: the bool or scratch-page parameters are optional.

= already has that meaning, and this is the form we use elsewhere.

>
>>  sharept, superpages, intremap, intpost, crash-disable,
>>  snoop, qinval, igfx, amd-iommu-perdev-intremap,
>>  dom0-{passthrough,strict} ]
>> @@ -1519,7 +1520,8 @@ boolean (e.g. `iommu=no`) can override this and leave 
>> the IOMMUs disabled.
>>  successfully.
>>  
>>  *   The `quarantine` option can be used to control Xen's behavior when
>> -de-assigning devices from guests.
>> +de-assigning devices from guests.  The default behaviour is chosen at
>> +compile time, and is one of 
>> `CONFIG_IOMMU_QUARANTINE_{NONE,BASIC,SCRATCH_PAGE}`.
> Do we also want to state that the current build time default is BASIC
> if the user hasn't selected otherwise?

This is an instruction to look at the .config file and see which it is.

The exceptional case of someone doing a build from clean isn't
particularly interesting.  Not least because they will be prompted for
it and given the choices.

~Andrew



Re: [XEN PATCH][for-4.19 v5] xen: Add deviations for MISRA C:2012 Rule 7.1

2023-10-31 Thread Julien Grall

Hi Stefano,

On 30/10/2023 22:49, Stefano Stabellini wrote:

On Mon, 30 Oct 2023, Julien Grall wrote:

Hi Nicola,

On 27/10/2023 16:11, Nicola Vetrini wrote:

diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 8511a189253b..81473fb4 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -90,6 +90,13 @@ Deviations related to MISRA C:2012 Rules:
- __emulate_2op and __emulate_2op_nobyte
- read_debugreg and write_debugreg
   +   * - R7.1
+ - It is safe to use certain octal constants the way they are defined
+   in specifications, manuals, and algorithm descriptions. Such places
+   are marked safe with a /\* octal-ok \*/ in-code comment, or with a
SAF
+   comment (see safe.json).


Reading this, it is unclear to me why we have two ways to deviate the rule
r7.1. And more importantely, how would the developper decide which one to use?


I agree with you on this and we were discussing this topic just this
morning in the FUSA community call. I think we need a way to do this
with the SAF framework:

if (some code with violation) /* SAF-xx-safe */

This doesn't work today unfortunately. It can only be done this way:

/* SAF-xx-safe */
if (some code with violation)

Which is not always desirable. octal-ok is just an ad-hoc solution for
one specific violation but we need a generic way to do this. Luca is
investigating possible ways to support the previous format in SAF.


Why can't we use octal-ok everywhere for now? My point here is to make 
simple for the developper to know what to use.




I think we should take this patch for now and harmonize it once SAF is
improved.


The description of the deviation needs some improvement. To give an 
example, with the current wording, one could they can use octal-ok 
everywhere. But above, you are implying that SAF-xx-safe should be

preferred.

I would still strongly prefer if we use octal-ok everywhere because this 
is simple to remember. But if the other are happy to have both SAF-XX 
and octal-ok, then the description needs to be completely unambiguous 
and the patch should contain some explanation why we have two different 
ways to deviate.


Cheers,

--
Julien Grall



[RFC PATCH v2 0/8] clang-format for Xen

2023-10-31 Thread Luca Fancellu
## Introduction 

In this serie, I would like to get feedbacks on the output generated by the
configuration of clang-format, unfortunately we can't use only clang-format, but
we need to call it using a wrapper, because we need the information of what
files need to be excluded from the tool.

Another reason is that clang-format has some limitation when formatting asm()
instruction and most of the time it format them in a very ugly way or it breaks
the code for example removing spaces that were there for a reason (I don't think
it's a tool to format asm), so in the wrapper script we protect all asm()
invocation or macros where there are asm() invocation with in-code comments that
stops clang-format to act on that section:

/* clang-format off */section/* clang-format on */

I've read the past threads about the brave people who dared to try to introduce
clang-format for the xen codebase, some of them from 5 years ago, two points
were clear: 1) goto label needs to be indented and 2) do-while loops have the
braket in the same line.
While point 1) was quite a blocker, it seemd to me that point 2) was less
controversial to be changed in the Xen codestyle, so the current wrapper script
handles only the point 1 (which is easy), the point 2 can be more tricky to
handle.

## The clang-format configuration ##

In my clang-format configuration I've taken inspiration from EPAM's work, then
from the configuration in Linux and finally from the clang-format manual, to try
to produce a comprehensive configuration.

Every configuration parameter has on top a comment with the description and
when it was supported, finally I've added also a [not specified] if that
behavior is not clearly specified in the Xen coding style, I've done that so
we could discuss about adding more specification in our CODING_STYLE.
Every comment can be stripped out in the final release of the file, but I think
that now they are useful for the discussion.

The minimum clang-format version for the file is 15, my ubuntu 22.04 comes with
it, we can reason if it's too high, or if we could also use the latest version
maybe shipped inside a docker image.

For every [not specified] behavior, I've tried to guess it from the codebase,
I've seen that also in that case it's not easy as there is (sometimes) low
consistency between modules, so we can discuss on every configurable.

Worth to mention, the public header are all excluded from the format tool,
because formatting them breaks the build on X86, because there are scripts for
auto-generation that don't handle the formatted headers, I didn't investigate
on it, maybe it can be added as technical debt.

So I've tried building arm32, arm64 and x86_64 with the formatted output and
they build, I've used Yocto for that.

## How to try it? ##

So how to generate everything? Just invoke the codestyle.py script without
parameter and it will format every .c and .h file in the hypervisor codebase.

./xen/scripts/codestyle.py

Optionally you can also pass one or more relative path from the folder you are
invoking the script and it will format only them.

## What I expect from this RFC #

I expect feedback on the output, some agreement on what configuration to use,
and I expect to find possible blocker before working seriously on this serie,
because if there are outstanding blockers on the adoption of the tool and we
can't reach an agreement, I won't spend further time on it.

v2 changes: I've introduced a way to suppress the code formatter on certain
piece of code, currently it is implemented inside the exclude-list, but we could
do in a different list, it's not very important at this stage and I did in this
way just to save time.

Here is a link to the current serie output:
https://gitlab.com/luca.fancellu/xen-clang/-/commit/8938bf2196be66b05693a48752ebbdf363e8d8e1.patch

We could reason about the output, arrange some meetings to discuss about the
clang-format configuration if we think we could go ahead with the adoption.

Luca Fancellu (8):
  cppcheck: rework exclusion_file_list.py code
  exclude-list: generalise exclude-list
  [WIP]xen/scripts: add codestyle.py script
  exclude-list: add entries to the excluded list for codestyle
  [WIP]codestyle.py: Protect generic piece of code
  [WIP]x86/exclude-list: protect mm_type_tbl in mtrr from being
formatted
  xen: Add clang-format configuration
  feedback from the community

 docs/misra/exclude-list.json  | 113 +++
 docs/misra/exclude-list.rst   |  37 +-
 xen/.clang-format | 693 ++
 xen/scripts/codestyle.py  | 289 
 xen/scripts/xen_analysis/cppcheck_analysis.py |   6 +-
 .../xen_analysis/exclusion_file_list.py   |  52 +-
 6 files changed, 1159 

[RFC PATCH v2 8/8] feedback from the community

2023-10-31 Thread Luca Fancellu
Signed-off-by: Luca Fancellu 
---
 xen/.clang-format | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/.clang-format b/xen/.clang-format
index 7880709fe1fd..bfc1d104af84 100644
--- a/xen/.clang-format
+++ b/xen/.clang-format
@@ -29,8 +29,8 @@ AlignArrayOfStructures: Left
 # [not specified]
 # Align consecutive assignments (supported in clang-format 3.8)
 AlignConsecutiveAssignments:
-  Enabled: true
-  AcrossEmptyLines: true
+  Enabled: false
+  AcrossEmptyLines: false
   AcrossComments: false
 
 # [not specified]
@@ -46,8 +46,8 @@ AlignConsecutiveDeclarations: None
 # Align values of consecutive macros (supported in clang-format 9)
 AlignConsecutiveMacros:
   Enabled: true
-  AcrossEmptyLines: true
-  AcrossComments: true
+  AcrossEmptyLines: false
+  AcrossComments: false
 
 # [not specified]
 # Align escaped newlines to the right (supported in clang-format 5)
-- 
2.34.1




[RFC PATCH v2 2/8] exclude-list: generalise exclude-list

2023-10-31 Thread Luca Fancellu
Currently exclude-list.json is used by the xen-analysis tool to
remove from the report (cppcheck for now) violations from the
files listed in it, however that list can be used by different
users that might want to exclude some of the files from their
computation for many reason.

So add a new field that can be part of each entry to link
the tool supposed to consider that exclusion.

Update exclusion_file_list.py to implement the logic and update
the documentation to reflect this change.

Signed-off-by: Luca Fancellu 
Reviewed-by: Stefano Stabellini 
---
 docs/misra/exclude-list.rst   | 31 ---
 .../xen_analysis/exclusion_file_list.py   | 16 --
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/docs/misra/exclude-list.rst b/docs/misra/exclude-list.rst
index c97431a86120..42dbceb82523 100644
--- a/docs/misra/exclude-list.rst
+++ b/docs/misra/exclude-list.rst
@@ -1,17 +1,16 @@
 .. SPDX-License-Identifier: CC-BY-4.0
 
-Exclude file list for xen-analysis script
-=
+Exclude file list for xen scripts
+=
 
-The code analysis is performed on the Xen codebase for both MISRA
-checkers and static analysis checkers, there are some files however that
-needs to be removed from the findings report for various reasons (e.g.
-they are imported from external sources, they generate too many false
-positive results, etc.).
+Different Xen scripts can perform operations on the codebase to check its
+compliance for a set of rules, however Xen contains some files that are taken
+from other projects (e.g. linux) and they can't be updated to ease backporting
+fixes from their source, for this reason the file docs/misra/exclude-list.json
+is kept as a source of all these files that are external to the Xen project.
 
-For this reason the file docs/misra/exclude-list.json is used to exclude every
-entry listed in that file from the final report.
-Currently only the cppcheck analysis will use this file.
+Every entry of the file can be linked to different checkers, so that this list
+can be used by multiple scripts selecting only the required entries.
 
 Here is an example of the exclude-list.json file::
 
@@ -20,11 +19,13 @@ Here is an example of the exclude-list.json file::
 |"content": [
 |{
 |"rel_path": "relative/path/from/xen/file",
-|"comment": "This file is originated from ..."
+|"comment": "This file is originated from ...",
+|"checkers": "xen-analysis"
 |},
 |{
 |"rel_path": "relative/path/from/xen/folder/*",
-|"comment": "This folder is a library"
+|"comment": "This folder is a library",
+|"checkers": "xen-analysis some-checker"
 |},
 |{
 |"rel_path": "relative/path/from/xen/mem*.c",
@@ -39,6 +40,12 @@ Here is an explanation of the fields inside an object of the 
"content" array:
match more than one file/folder at the time. This field is mandatory.
  - comment: an optional comment to explain why the file is removed from the
analysis.
+ - checkers: an optional list of checkers that will exclude this entries from
+   their results. This field is optional and when not specified, it means every
+   checker will use that entry.
+   Current implemented values for this field are:
+- xen-analysis: the xen-analysis.py script exclude this entry for both 
MISRA
+  and static analysis scan. (Implemented only for Cppcheck tool)
 
 To ease the review and the modifications of the entries, they shall be listed 
in
 alphabetical order referring to the rel_path field.
diff --git a/xen/scripts/xen_analysis/exclusion_file_list.py 
b/xen/scripts/xen_analysis/exclusion_file_list.py
index 79ebd34f55ec..8b10665a19e8 100644
--- a/xen/scripts/xen_analysis/exclusion_file_list.py
+++ b/xen/scripts/xen_analysis/exclusion_file_list.py
@@ -9,7 +9,7 @@ class ExclusionFileListError(Exception):
 
 def cppcheck_exclusion_file_list(input_file):
 ret = []
-excl_list = load_exclusion_file_list(input_file)
+excl_list = load_exclusion_file_list(input_file, "xen-analysis")
 
 for entry in excl_list:
 # Prepending * to the relative path to match every path where the Xen
@@ -25,7 +25,7 @@ def cppcheck_exclusion_file_list(input_file):
 # If the first entry contained a wildcard '*', the second entry will have an
 # array of the solved absolute path for that entry.
 # Returns [('path',[path,path,...]), ('path',[path,path,...]), ...]
-def load_exclusion_file_list(input_file):
+def load_exclusion_file_list(input_file, checker=""):
 ret = []
 try:
 with open(input_file, "rt") as handle:
@@ -51,6 +51,18 @@ def load_exclusion_file_list(input_file):
 raise ExclusionFileListError(
 "Malformed JSON entry: rel_path field not found!"
 )
+# Check the checker field
+try:
+

[RFC PATCH v2 4/8] exclude-list: add entries to the excluded list for codestyle

2023-10-31 Thread Luca Fancellu
Add entries to the exclusion list, so that they can be excluded
from the formatting tool.

Signed-off-by: Luca Fancellu 
---
 docs/misra/exclude-list.json | 100 +++
 docs/misra/exclude-list.rst  |   2 +
 2 files changed, 102 insertions(+)

diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
index 575ed22a7f67..d48dcf3ac971 100644
--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -1,6 +1,11 @@
 {
 "version": "1.0",
 "content": [
+{
+"rel_path": "arch/arm/arm32/lib/assembler.h",
+"comment": "Includes mostly assembly macro and it's meant to be 
included only in assembly code",
+"checkers": "codestyle"
+},
 {
 "rel_path": "arch/arm/arm64/cpufeature.c",
 "comment": "Imported from Linux, ignore for now"
@@ -13,6 +18,31 @@
 "rel_path": "arch/arm/arm64/lib/find_next_bit.c",
 "comment": "Imported from Linux, ignore for now"
 },
+{
+"rel_path": "arch/arm/include/asm/arm32/macros.h",
+"comment": "Includes only assembly macro",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/arm/include/asm/arm64/macros.h",
+"comment": "Includes only assembly macro",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/arm/include/asm/alternative.h",
+"comment": "Imported from Linux, ignore for now",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/arm/include/asm/asm_defns.h",
+"comment": "Includes mostly assembly macro",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/arm/include/asm/macros.h",
+"comment": "Includes mostly assembly macro and it's meant to be 
included only in assembly code",
+"checkers": "codestyle"
+},
 {
 "rel_path": "arch/x86/acpi/boot.c",
 "comment": "Imported from Linux, ignore for now"
@@ -69,6 +99,36 @@
 "rel_path": "arch/x86/cpu/mwait-idle.c",
 "comment": "Imported from Linux, ignore for now"
 },
+{
+"rel_path": "arch/x86/include/asm/alternative-asm.h",
+"comment": "Includes mostly assembly macro and it's meant to be 
included only in assembly code",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/x86/include/asm/asm_defns.h",
+"comment": "Includes mostly assembly macro",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/x86/include/asm/asm-defns.h",
+"comment": "Includes mostly assembly macro",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/x86/include/asm/bug.h",
+"comment": "Includes mostly assembly macro",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/x86/include/asm/mpspec.h",
+"comment": "Imported from Linux, also designated initializers 
ranges are not handled very well by clang-format, ignore for now",
+"checkers": "codestyle"
+},
+{
+"rel_path": "arch/x86/include/asm/spec_ctrl_asm.h",
+"comment": "Includes mostly assembly macro",
+"checkers": "codestyle"
+},
 {
 "rel_path": "arch/x86/delay.c",
 "comment": "Imported from Linux, ignore for now"
@@ -189,10 +249,45 @@
 "rel_path": "include/acpi/acpixf.h",
 "comment": "Imported from Linux, ignore for now"
 },
+{
+"rel_path": "include/efi/*.h",
+"comment": "Imported from gnu-efi-3.0k, prefer their formatting",
+"checkers": "codestyle"
+},
+{
+"rel_path": "include/public/arch-x86/cpufeatureset.h",
+"comment": "This file contains some inputs for the gen-cpuid.py 
script, leave it out",
+"checkers": "codestyle"
+},
+{
+"rel_path": "include/public/*",
+"comment": "Public headers are quite sensitive to format tools",
+"checkers": "codestyle"
+},
 {
 "rel_path": "include/xen/acpi.h",
 "comment": "Imported from Linux, ignore for now"
 },
+{
+"rel_path": "include/xen/cper.h",
+"comment": "Header does not follow Xen coding style",
+"checkers": "codestyle"
+},
+{
+"rel_path": "include/xen/nodemask.h",
+"comment": "Imported from Linux, also designated initializers 
ranges are not handled by clang-format, ignore for now",
+"checkers": "codestyle"
+},
+{
+"rel_path": "include/xen/xen.lds.h",
+"comment": "This file contains only macros used 

[RFC PATCH v2 6/8] [WIP]x86/exclude-list: protect mm_type_tbl in mtrr from being formatted

2023-10-31 Thread Luca Fancellu
The array mm_type_tbl initialization is formatted in a way that
the formatting tool can't keep, so disable the formatting on that
array initialization.

Signed-off-by: Luca Fancellu 
---
 docs/misra/exclude-list.json | 13 +
 1 file changed, 13 insertions(+)

diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
index d48dcf3ac971..b8976bc671a4 100644
--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -99,6 +99,19 @@
 "rel_path": "arch/x86/cpu/mwait-idle.c",
 "comment": "Imported from Linux, ignore for now"
 },
+{
+"rel_path": "arch/x86/hvm/mtrr.c",
+"comment": "Contains structure formatted in a particular way",
+"checkers": "codestyle",
+"codestyle": {
+"protect": [
+{
+"syntax_opening": "static const uint8_t mm_type_tbl",
+"syntax_closing": "};"
+}
+]
+}
+},
 {
 "rel_path": "arch/x86/include/asm/alternative-asm.h",
 "comment": "Includes mostly assembly macro and it's meant to be 
included only in assembly code",
-- 
2.34.1




[RFC PATCH v2 7/8] xen: Add clang-format configuration

2023-10-31 Thread Luca Fancellu
Add a clang format configuration for the Xen Hypervisor.

Signed-off-by: Luca Fancellu 
---
 xen/.clang-format | 693 ++
 1 file changed, 693 insertions(+)
 create mode 100644 xen/.clang-format

diff --git a/xen/.clang-format b/xen/.clang-format
new file mode 100644
index ..7880709fe1fd
--- /dev/null
+++ b/xen/.clang-format
@@ -0,0 +1,693 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# clang-format configuration file. Intended for clang-format >= 15.
+#
+# For more information, see:
+#
+#   Documentation/process/clang-format.rst
+#   https://clang.llvm.org/docs/ClangFormat.html
+#   https://clang.llvm.org/docs/ClangFormatStyleOptions.html
+#
+---
+
+# [not specified]
+# Align function parameter that goes into a new line, under the open bracket
+# (supported in clang-format 3.8)
+AlignAfterOpenBracket: Align
+
+# [not specified]
+# Align array of struct's elements by column and justify
+# struct test demo[] =
+# {
+# {56, 23,"hello"},
+# {-1, 93463, "world"},
+# {7,  5, "!!"   }
+# };
+# (supported in clang-format 13)
+AlignArrayOfStructures: Left
+
+# [not specified]
+# Align consecutive assignments (supported in clang-format 3.8)
+AlignConsecutiveAssignments:
+  Enabled: true
+  AcrossEmptyLines: true
+  AcrossComments: false
+
+# [not specified]
+# Do not align consecutive bit fields (supported in clang-format 11)
+AlignConsecutiveBitFields: None
+
+# [not specified]
+# Do not align values of consecutive declarations
+# (supported in clang-format 3.8)
+AlignConsecutiveDeclarations: None
+
+# [not specified]
+# Align values of consecutive macros (supported in clang-format 9)
+AlignConsecutiveMacros:
+  Enabled: true
+  AcrossEmptyLines: true
+  AcrossComments: true
+
+# [not specified]
+# Align escaped newlines to the right (supported in clang-format 5)
+AlignEscapedNewlines: Right
+
+# [not specified]
+# Aligns operands of a single expression that needs to be split over multiple
+# lines (supported in clang-format 3.5)
+AlignOperands: Align
+
+# Do not align trailing consecutive comments (It helps to make clang-format
+# reproduce the same output when it runs on an already formatted file)
+# (supported in clang-format 3.7)
+AlignTrailingComments: false
+
+# [not specified]
+# Do not put all function call arguments on a new line, try to have at least
+# the first one close to the opening parenthesis (supported in clang-format 9)
+AllowAllArgumentsOnNextLine: false
+
+# [not specified]
+# Do not put all function declaration parameters on a new line, try to have at
+# least the first one close to the opening parenthesis
+# (supported in clang-format 3.3)
+AllowAllParametersOfDeclarationOnNextLine: false
+
+# Bracing condition needs to be respected even if the line is so short that the
+# final block brace can stay on a single line
+# (supported in clang-format 3.5)
+AllowShortBlocksOnASingleLine: Never
+
+# (supported in clang-format 3.6)
+AllowShortCaseLabelsOnASingleLine: false
+
+# (supported in clang-format 3.5)
+AllowShortFunctionsOnASingleLine: None
+
+# (supported in clang-format 3.3)
+AllowShortIfStatementsOnASingleLine: Never
+
+# (supported in clang-format 3.7)
+AllowShortLoopsOnASingleLine: false
+
+# [not specified]
+# Do not add a break after the definition return type
+# (supported in clang-format 3.8)
+AlwaysBreakAfterReturnType: None
+
+# [not specified]
+# There is no need to use a break after an assigment to a multiline string
+# (supported in clang-format 3.4)
+AlwaysBreakBeforeMultilineStrings: false
+
+# (supported in clang-format 3.4)
+AlwaysBreakTemplateDeclarations: false
+
+# Specify Xen's macro attributes (supported in clang-format 12)
+AttributeMacros:
+  - '__init'
+  - '__exit'
+  - '__initdata'
+  - '__initconst'
+  - '__initconstrel'
+  - '__initdata_cf_clobber'
+  - '__initconst_cf_clobber'
+  - '__hwdom_init'
+  - '__hwdom_initdata'
+  - '__maybe_unused'
+  - '__packed'
+  - '__stdcall'
+  - '__vfp_aligned'
+  - '__alt_call_maybe_initdata'
+  - '__cacheline_aligned'
+  - '__ro_after_init'
+  - 'always_inline'
+  - 'noinline'
+  - 'noreturn'
+  - '__weak'
+  - '__inline__'
+  - '__attribute_const__'
+  - '__transparent__'
+  - '__used'
+  - '__must_check'
+  - '__kprobes'
+
+# [not specified]
+# Try always to pack function call arguments on the same line before breaking
+# (supported in clang-format 3.7)
+BinPackArguments: true
+
+# [not specified]
+# Try always to pack function declaration parameters on the same line before
+# breaking (supported in clang-format 3.7)
+BinPackParameters: true
+
+# [not specified]
+# Do not add a spaces on bitfield 'unsigned bf:2;'
+# (supported in clang-format 12)
+BitFieldColonSpacing: None
+
+# Xen's coding style does not follow clang-format already available profiles 
for
+# breaking before braces, so set it to Custom and specify each case separately
+# (supported in clang-format 3.8)
+BraceWrapping:
+  # Braces ('{' and '}') are usually placed on a line of their 

[RFC PATCH v2 5/8] [WIP]codestyle.py: Protect generic piece of code

2023-10-31 Thread Luca Fancellu
Add a way to protect generic piece of code from being formatted.

Use the exclude-list to pass also a structure to the scripts,
that structure will be used from the codestyle.py script to
understand which piece of code of which file needs to be left
with the original format.

Update exclude-list.rst documentation.

Signed-off-by: Luca Fancellu 
---
 docs/misra/exclude-list.rst   |  6 +-
 xen/scripts/codestyle.py  | 96 ---
 .../xen_analysis/exclusion_file_list.py   | 15 ++-
 3 files changed, 76 insertions(+), 41 deletions(-)

diff --git a/docs/misra/exclude-list.rst b/docs/misra/exclude-list.rst
index ade314100663..946f3793aad7 100644
--- a/docs/misra/exclude-list.rst
+++ b/docs/misra/exclude-list.rst
@@ -25,7 +25,8 @@ Here is an example of the exclude-list.json file::
 |{
 |"rel_path": "relative/path/from/xen/folder/*",
 |"comment": "This folder is a library",
-|"checkers": "xen-analysis some-checker"
+|"checkers": "xen-analysis some-checker",
+|"xen-analysis": {...}
 |},
 |{
 |"rel_path": "relative/path/from/xen/mem*.c",
@@ -48,6 +49,9 @@ Here is an explanation of the fields inside an object of the 
"content" array:
   and static analysis scan. (Implemented only for Cppcheck tool)
 - codestyle: the codestyle.py script exclude this entry from the formatting
   tool.
+ - : an optional parameter to pass a configuration to the checker 
about
+   this entry. The parameter to be specified is one of the value listed for the
+   "checkers" value.
 
 To ease the review and the modifications of the entries, they shall be listed 
in
 alphabetical order referring to the rel_path field.
diff --git a/xen/scripts/codestyle.py b/xen/scripts/codestyle.py
index ab3df66fc2e2..92482d586f7a 100755
--- a/xen/scripts/codestyle.py
+++ b/xen/scripts/codestyle.py
@@ -36,36 +36,36 @@ def action_protect_asm(filename, file_lines, protect):
 opening_asm = False
 cf_off_comment = '/* clang-format off */'
 cf_on_comment = '/* clang-format on */'
-asm_stx = '(?:asm|__asm__)(?:\s(?:volatile|__volatile__))?\s?\('
-asm_stx_close = ');'
+
+config = filename[1]["protect"]
 
 if protect:
 # Look for closing parenthesis with semicolon ');'
-closing_asm_rgx_rule = rf'^.*{re.escape(asm_stx_close)}.*$'
+closing_asm_rgx_rule = lambda cl_stx: rf'^.*{re.escape(cl_stx)}.*$'
 # Look for opening asm syntax
-opening_asm_rgx_rule = rf'^\s*{asm_stx}.*$'
+opening_asm_rgx_rule = lambda op_stx: rf'^\s*{op_stx}.*$'
 macro_start_rgx_rule = r'^\s?#\s?define.*\\$'
-opening_asm_find = rf'({asm_stx})'
+opening_asm_find = lambda op_stx: rf'({op_stx})'
 opening_asm_replace = cf_off_comment + r'\1'
 opening_def_find = r'#\s?define'
 opening_def_replace = f'{cf_off_comment}#define'
-closing_asm_find= re.escape(asm_stx_close)
-closing_asm_replace = asm_stx_close + cf_on_comment
+closing_asm_find= lambda cl_stx: re.escape(cl_stx)
+closing_asm_replace = lambda cl_stx: cl_stx + cf_on_comment
 closing_def_find= '\n'
 closing_def_replace = cf_on_comment + '\n'
 else:
 # Look for closing parenthesis with semicolon ');' and with the
 # special clang-format comment
-closing_asm_rgx_rule = \
-rf'^.*{re.escape(asm_stx_close)}.*{re.escape(cf_on_comment)}.*$'
+closing_asm_rgx_rule = lambda cl_stx: \
+rf'^.*{re.escape(cl_stx)}.*{re.escape(cf_on_comment)}.*$'
 # Look for opening asm syntax preceded by the special clang-format
 # comment, the comment is optional to generalise the algorithm to
 # un-protect asm outside and inside macros. The case outside is easy
 # because we will find '/* clang-format off */asm', instead the case
 # inside is more tricky and we are going to find only 'asm' and then
 # go backwards until we find '/* clang-format off */#define'
-opening_asm_rgx_rule = \
-rf'^\s*({re.escape(cf_off_comment)})?{asm_stx}.*$'
+opening_asm_rgx_rule = lambda op_stx: \
+rf'^\s*({re.escape(cf_off_comment)})?{op_stx}.*$'
 # Look for the define just before the asm invocation, here we look for
 # '/* clang-format off */#define' or '#define', this is to handle a 
rare
 # corner case where an asm invocation is inside a macro, but was not
@@ -76,24 +76,27 @@ def action_protect_asm(filename, file_lines, protect):
 # comments, but at least the tool won't complain
 macro_start_rgx_rule = \
 rf'^\s?(?:{re.escape(cf_off_comment)})?#\s?define.*\\$'
-opening_asm_find = rf'({re.escape(cf_off_comment)}({asm_stx}))'
+opening_asm_find = \
+lambda op_stx: rf'({re.escape(cf_off_comment)}({op_stx}))'
 opening_asm_replace = 

[RFC PATCH v2 3/8] [WIP]xen/scripts: add codestyle.py script

2023-10-31 Thread Luca Fancellu
This script finds every .c and .h file in the xen hypervisor
codebase, takes the exclusion list from docs/misra, removes the
file excluded from the list and for the remaining files is
calling clang-format on them.

TBD: write it better

Signed-off-by: Luca Fancellu 
---
 xen/scripts/codestyle.py | 265 +++
 1 file changed, 265 insertions(+)
 create mode 100755 xen/scripts/codestyle.py

diff --git a/xen/scripts/codestyle.py b/xen/scripts/codestyle.py
new file mode 100755
index ..ab3df66fc2e2
--- /dev/null
+++ b/xen/scripts/codestyle.py
@@ -0,0 +1,265 @@
+#!/usr/bin/env python3
+
+import glob
+import os
+import re
+import sys
+from xen_analysis.settings import xen_dir, repo_dir
+from xen_analysis import utils
+from xen_analysis import exclusion_file_list
+from xen_analysis.exclusion_file_list import ExclusionFileListError
+
+# The Xen codestyle states that labels needs to be indented by at least one
+# blank, but clang-format doesn't have an option for that and if it encounters
+# a label indented by blank characters that are less than its indent
+# configuration, it removes the indentation.
+# So this action is meant as post step and checks every label syntax match and
+# it adds one blank before the label
+def action_fix_label_indent(filename, file_lines):
+label_rgx = re.compile('^[a-zA-Z_][a-zA-Z0-9_]*\s*:.*$')
+
+for i in range(0, len(file_lines)):
+if label_rgx.match(file_lines[i]):
+file_lines[i] = ' ' + file_lines[i]
+
+return file_lines
+
+
+# clang-format most of the time breaks the content of asm(...) instructions,
+# so with this function, we protect all the asm sections using the special
+# in code comments that tells clang-format to don't touch the block.
+# asm(...) instruction could be also inside macros, so in that case we protect
+# the entire macro that is enclosing the instruction, in the un-protect stage
+# however, we need to do clang-format's job at least on the tab-space 
conversion
+# and to put the backslash on the right side.
+def action_protect_asm(filename, file_lines, protect):
+opening_asm = False
+cf_off_comment = '/* clang-format off */'
+cf_on_comment = '/* clang-format on */'
+asm_stx = '(?:asm|__asm__)(?:\s(?:volatile|__volatile__))?\s?\('
+asm_stx_close = ');'
+
+if protect:
+# Look for closing parenthesis with semicolon ');'
+closing_asm_rgx_rule = rf'^.*{re.escape(asm_stx_close)}.*$'
+# Look for opening asm syntax
+opening_asm_rgx_rule = rf'^\s*{asm_stx}.*$'
+macro_start_rgx_rule = r'^\s?#\s?define.*\\$'
+opening_asm_find = rf'({asm_stx})'
+opening_asm_replace = cf_off_comment + r'\1'
+opening_def_find = r'#\s?define'
+opening_def_replace = f'{cf_off_comment}#define'
+closing_asm_find= re.escape(asm_stx_close)
+closing_asm_replace = asm_stx_close + cf_on_comment
+closing_def_find= '\n'
+closing_def_replace = cf_on_comment + '\n'
+else:
+# Look for closing parenthesis with semicolon ');' and with the
+# special clang-format comment
+closing_asm_rgx_rule = \
+rf'^.*{re.escape(asm_stx_close)}.*{re.escape(cf_on_comment)}.*$'
+# Look for opening asm syntax preceded by the special clang-format
+# comment, the comment is optional to generalise the algorithm to
+# un-protect asm outside and inside macros. The case outside is easy
+# because we will find '/* clang-format off */asm', instead the case
+# inside is more tricky and we are going to find only 'asm' and then
+# go backwards until we find '/* clang-format off */#define'
+opening_asm_rgx_rule = \
+rf'^\s*({re.escape(cf_off_comment)})?{asm_stx}.*$'
+# Look for the define just before the asm invocation, here we look for
+# '/* clang-format off */#define' or '#define', this is to handle a 
rare
+# corner case where an asm invocation is inside a macro, but was not
+# protected in the 'protect stage', because it was on the same line
+# of the define and was not ending with backslash but it was exceeding
+# line width so clang-format formatted anyway.
+# It's safe because we won't change code that has no clang-format
+# comments, but at least the tool won't complain
+macro_start_rgx_rule = \
+rf'^\s?(?:{re.escape(cf_off_comment)})?#\s?define.*\\$'
+opening_asm_find = rf'({re.escape(cf_off_comment)}({asm_stx}))'
+opening_asm_replace = r'\2'
+opening_def_find = rf'(?:{re.escape(cf_off_comment)})?#\s?define'
+opening_def_replace = '#define'
+closing_asm_find \
+= rf'{re.escape(asm_stx_close)}.*{re.escape(cf_on_comment)}'
+closing_asm_replace = asm_stx_close
+closing_def_find= cf_on_comment + '\n'
+closing_def_replace = '\n'
+
+opening_asm_rgx = 

[RFC PATCH v2 1/8] cppcheck: rework exclusion_file_list.py code

2023-10-31 Thread Luca Fancellu
Rework the exclusion_file_list.py code to have the function
load_exclusion_file_list() detached from the xen-analysis.py tool,
in a way so that other modules can use the function.
The xen-analysis tool and in particular its module cppcheck_analysis.py
will use a new function cppcheck_exclusion_file_list().

No functional changes are intended.

Signed-off-by: Luca Fancellu 
Reviewed-by: Stefano Stabellini 
---
 xen/scripts/xen_analysis/cppcheck_analysis.py |  6 ++--
 .../xen_analysis/exclusion_file_list.py   | 31 ++-
 2 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py 
b/xen/scripts/xen_analysis/cppcheck_analysis.py
index 8dc45e653b79..e54848aa5339 100644
--- a/xen/scripts/xen_analysis/cppcheck_analysis.py
+++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
@@ -2,7 +2,8 @@
 
 import os, re, shutil
 from . import settings, utils, cppcheck_report_utils, exclusion_file_list
-from .exclusion_file_list import ExclusionFileListError
+from .exclusion_file_list import (ExclusionFileListError,
+  cppcheck_exclusion_file_list)
 
 class GetMakeVarsPhaseError(Exception):
 pass
@@ -54,8 +55,7 @@ def __generate_suppression_list(out_file):
 try:
 exclusion_file = \
 "{}/docs/misra/exclude-list.json".format(settings.repo_dir)
-exclusion_list = \
-
exclusion_file_list.load_exclusion_file_list(exclusion_file)
+exclusion_list = cppcheck_exclusion_file_list(exclusion_file)
 except ExclusionFileListError as e:
 raise CppcheckDepsPhaseError(
 "Issue with reading file {}: {}".format(exclusion_file, e)
diff --git a/xen/scripts/xen_analysis/exclusion_file_list.py 
b/xen/scripts/xen_analysis/exclusion_file_list.py
index 871e480586bb..79ebd34f55ec 100644
--- a/xen/scripts/xen_analysis/exclusion_file_list.py
+++ b/xen/scripts/xen_analysis/exclusion_file_list.py
@@ -7,16 +7,24 @@ class ExclusionFileListError(Exception):
 pass
 
 
-def __cppcheck_path_exclude_syntax(path):
-# Prepending * to the relative path to match every path where the Xen
-# codebase could be
-path = "*" + path
+def cppcheck_exclusion_file_list(input_file):
+ret = []
+excl_list = load_exclusion_file_list(input_file)
+
+for entry in excl_list:
+# Prepending * to the relative path to match every path where the Xen
+# codebase could be
+ret.append("*" + entry[0])
 
-return path
+return ret
 
 
-# Reads the exclusion file list and returns a list of relative path to be
-# excluded.
+# Reads the exclusion file list and returns an array containing a set where the
+# first entry is what was listed in the exclusion list file, and the second
+# entry is the absolute path of the first entry.
+# If the first entry contained a wildcard '*', the second entry will have an
+# array of the solved absolute path for that entry.
+# Returns [('path',[path,path,...]), ('path',[path,path,...]), ...]
 def load_exclusion_file_list(input_file):
 ret = []
 try:
@@ -58,13 +66,6 @@ def load_exclusion_file_list(input_file):
 .format(path, filepath_object)
 )
 
-if settings.analysis_tool == "cppcheck":
-path = __cppcheck_path_exclude_syntax(path)
-else:
-raise ExclusionFileListError(
-"Unimplemented for {}!".format(settings.analysis_tool)
-)
-
-ret.append(path)
+ret.append((path, check_path))
 
 return ret
-- 
2.34.1




[PATCH for-4.19 3/3] CHANGELOG: Keep unstable section

2023-10-31 Thread Andrew Cooper
Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Wei Liu 
CC: Julien Grall 
CC: Roger Pau Monné 
CC: Henry Wang 
---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a827054cf27d..cf0c9c3f8cb9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@ Notable changes to Xen will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
+## [unstable 
UNRELEASED](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=staging)
 - TBD
+
 ## 
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
 - 2023-XX-XX
 
 ### Changed
-- 
2.30.2




[PATCH for-4.18 1/3] CHANGELOG: Reformat

2023-10-31 Thread Andrew Cooper
Collect all x86 and ARM changes together instead of having them scattered.
Tweak grammar as necessary.

No change.

Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Wei Liu 
CC: Julien Grall 
CC: Roger Pau Monné 
CC: Henry Wang 
---
 CHANGELOG.md | 35 +++
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3ca796969990..edc0d69898ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,24 +17,27 @@ The format is based on [Keep a 
Changelog](https://keepachangelog.com/en/1.0.0/)
Hotplug" for clarity
 
 ### Added
- - On x86, support for features new in Intel Sapphire Rapids CPUs:
-   - PKS (Protection Key Supervisor) available to HVM/PVH guests.
-   - VM-Notify used by Xen to mitigate certain micro-architectural pipeline
- livelocks, instead of crashing the entire server.
-   - Bus-lock detection, used by Xen to mitigate (by rate-limiting) the system
- wide impact of a guest misusing atomic instructions.
- - xl/libxl can customize SMBIOS strings for HVM guests.
- - Add support for AVX512-FP16 on x86.
- - On Arm, Xen supports guests running SVE/SVE2 instructions. (Tech Preview)
- - On Arm, add suport for Firmware Framework for Arm A-profile (FF-A) Mediator
-   (Tech Preview)
- - Add Intel Hardware P-States (HWP) cpufreq driver.
- - On Arm, experimental support for dynamic addition/removal of Xen device tree
-   nodes using a device tree overlay binary (.dtbo).
+ - On x86:
+   - xl/libxl can customize SMBIOS strings for HVM guests.
+   - Support for enforcing system-wide operation in Data Operand Independent
+ Timing Mode.
+   - Add Intel Hardware P-States (HWP) cpufreq driver.
+   - Support for features new in Intel Sapphire Rapids CPUs:
+ - PKS (Protection Key Supervisor) available to HVM/PVH guests.
+ - VM-Notify used by Xen to mitigate certain micro-architectural pipeline
+   livelocks, instead of crashing the entire server.
+ - Bus-lock detection, used by Xen to mitigate (by rate-limiting) the
+   system wide impact of a guest misusing atomic instructions.
+   - Support for features new in Intel Granite Rapids CPUs:
+ - AVX512-FP16.
+ - On Arm:
+   - Xen supports guests running SVE/SVE2 instructions. (Tech Preview)
+   - Add suport for Firmware Framework for Arm A-profile (FF-A) Mediator (Tech
+ Preview)
+   - Experimental support for dynamic addition/removal of Xen device tree
+ nodes using a device tree overlay binary (.dtbo).
  - Introduce two new hypercalls to map the vCPU runstate and time areas by
physical rather than linear/virtual addresses.
- - On x86, support for enforcing system-wide operation in Data Operand
-   Independent Timing Mode.
  - The project has now officially adopted 6 directives and 65 rules of MISRA-C.
 
 ### Removed
-- 
2.30.2




[PATCH for-4.18 2/3] CHANGELOG: More 4.18 content

2023-10-31 Thread Andrew Cooper
Signed-off-by: Andrew Cooper 
---
CC: George Dunlap 
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Wei Liu 
CC: Julien Grall 
CC: Roger Pau Monné 
CC: Henry Wang 
---
 CHANGELOG.md | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index edc0d69898ed..a827054cf27d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,10 +18,17 @@ The format is based on [Keep a 
Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ### Added
  - On x86:
+   - On all Intel systems, MSR_ARCH_CAPS is now visible in guests, and
+ controllable from the VM's config file.  For CPUs from ~2019 onwards,
+ this allows guest kernels to see details about hardware fixes for
+ speculative mitigations.  (Backported as XSA-435 to older releases).
- xl/libxl can customize SMBIOS strings for HVM guests.
- Support for enforcing system-wide operation in Data Operand Independent
  Timing Mode.
- Add Intel Hardware P-States (HWP) cpufreq driver.
+   - Support for features new in AMD Genoa CPUs:
+ - CPUID_USER_DIS (CPUID Faulting) used by Xen to control PV guest's view
+   of CPUID data.
- Support for features new in Intel Sapphire Rapids CPUs:
  - PKS (Protection Key Supervisor) available to HVM/PVH guests.
  - VM-Notify used by Xen to mitigate certain micro-architectural pipeline
-- 
2.30.2




[PATCH for-4.18 0/3] CHANGELOG: More 4.18 content

2023-10-31 Thread Andrew Cooper
Andrew Cooper (3):
  CHANGELOG: Reformat
  CHANGELOG: More 4.18 content
  CHANGELOG: Keep unstable section

 CHANGELOG.md | 44 
 1 file changed, 28 insertions(+), 16 deletions(-)


base-commit: 9659b2a6d73b14620e187f9c626a09323853c459
-- 
2.30.2




Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs

2023-10-31 Thread Julien Grall

Hi,

On 31/10/2023 11:03, Jan Beulich wrote:

On 31.10.2023 00:52, Stewart Hildebrand wrote:

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
  bus = PCI_BUS(machine_sbdf);
  devfn = PCI_DEVFN(machine_sbdf);
  
+if ( IS_ENABLED(CONFIG_ARM) &&

+ !is_hardware_domain(d) &&
+ !is_system_domain(d) &&
+ (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )


I don't think you need the explicit ARM check; that's redundant with
checking !HAS_VPCI_GUEST_SUPPORT. It's also not really clear why you
need to check for the system domain here.


I might be missing but I wouldn't expect the domain to have vPCI enabled 
if CONFIG_HAVE_VPCI_GUEST_SUPPORT=n. So why can't this simply be:


if ( !has_vcpi(d) )
{
   ...
}

Cheers,

--
Julien Grall



Re: Cambridge University Talk - 9th November 2023

2023-10-31 Thread Ayan Kumar Halder

Hi Xen Maintainers/developers,


As part of my talk, I wish to provide some examples of tasks that a 
newbie can easily pick up and contribute.


This need not be a dedicated project, but something that can be 
contributed on an ad-hoc basis.


The idea is to get more people interested in Xen project. :)


I found some examples of this :-

1. Misra C fixes - Refer "Misra rule 10.3 violations report script" . 
Luca has provided an awesome script to identify the MISRA violations. 
This can be used to provide fixes.


2. https://wiki.xenproject.org/wiki/Outreach_Program_Projects - I think 
this page provides some pointers, but I am not sure if this is up to date.



Please let me know if there are more of these examples.


Kind regards,

Ayan


On 30/10/2023 17:54, Kelly Choi wrote:

Hello Xen Community!

I'm excited to share that we will be presenting a talk at Cambridge 
University!

This is free and open to everyone, including students and the public.

Make sure to add this to your calendars and come along.

*Date: Thursday 9th November 2023*
*Time: 3 - 4pm *
*Location:
*
*Computer Laboratory
William Gates Building
15 JJ Thomson Avenue*
*Cambridge CB3 0FD
https://www.cl.cam.ac.uk/directions/ *

Title: Navigating the Open Source Landscape: Insights from Ayan Kumar 
and Edwin Torok 


Join us for an illuminating seminar featuring two distinguished 
speakers, Ayan Kumar and Edwin Torok, who will delve into the 
intricate world of open-source projects.


Ayan Kumar: In his engaging presentation, Ayan Kumar will be your 
guide through the inner workings of open-source projects, using the 
Xen hypervisor as a compelling example. With a keen focus on 
demystifying the nuances of open-source collaborations, Ayan will walk 
you through the step-by-step workflow for contributions, shedding 
light on the collaborative modes that fuel innovation. Get ready to be 
inspired by the fascinating ongoing developments in the Xen 
hypervisor. Ayan will also provide invaluable insights for newcomers, 
outlining promising avenues for their initial contributions. The 
session will culminate in a hands-on demonstration featuring a 
selection of noteworthy open-source projects.


Edwin Torok: Edwin Torok will offer invaluable wisdom on the unique 
challenges of joining and maintaining a venerable, decade-old 
codebase, drawing from his extensive experience with the XAPI project. 
With a deep dive into the strategies and practices that sustain such a 
longstanding project, Edwin will equip you with the insights needed to 
navigate and contribute effectively to large-scale, established 
codebases.


Don't miss this opportunity to gain firsthand knowledge from these two 
seasoned experts in the open-source arena. Join us for an enriching 
seminar that promises to empower both beginners and seasoned 
developers alike.


Many thanks,
Kelly Choi

Open Source Community Manager
XenServer, Cloud Software Group




Re: [PATCH for-4.18] docs: Fix IOMMU command line docs some more

2023-10-31 Thread Henry Wang
Hi Andrew,

> On Oct 31, 2023, at 20:02, Andrew Cooper  wrote:
> 
> Make the command line docs match the actual implementation, and state that the
> default behaviour is selected at compile time.
> 
> Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices 
> optional")
> Signed-off-by: Andrew Cooper 

Release-acked-by: Henry Wang 

Kind regards,
Henry




Re: [PATCH v1 12/29] xen/asm-generic: introduce stub header pci.h

2023-10-31 Thread Oleksii
On Mon, 2023-10-30 at 17:43 +0100, Jan Beulich wrote:
> On 30.10.2023 17:34, Oleksii wrote:
> > Hello Jan,
> > 
> > On Thu, 2023-10-19 at 11:55 +0200, Jan Beulich wrote:
> > > On 14.09.2023 16:56, Oleksii Kurochko wrote:
> > > > --- /dev/null
> > > > +++ b/xen/include/asm-generic/pci.h
> > > > @@ -0,0 +1,18 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > > +#ifndef __ASM_GENERIC_PCI_H__
> > > > +#define __ASM_GENERIC_PCI_H__
> > > > +
> > > > +struct arch_pci_dev {
> > > > +};
> > > > +
> > > > +#endif /* __ASM_GENERIC_PCI_H__ */
> > > 
> > > While more involved, I still wonder whether xen/pci.h could also
> > > avoid
> > > including asm/pci.h when !HAS_PCI. Of course there's more than
> > > just
> > > the
> > > #include which then would need #ifdef-ing out.
> > > 
> > > Jan
> > 
> > It looks like we can do that but only one question should be
> > resolved.
> > In ARM case, in  there is !HAS_PCI branch:
> > 
> > #else   /*!CONFIG_HAS_PCI*/
> > 
> > struct arch_pci_dev { };
> > 
> > static always_inline bool is_pci_passthrough_enabled(void)
> > {
> >     return false;
> > }
> > 
> > struct pci_dev;
> > 
> > static inline void arch_pci_init_pdev(struct pci_dev *pdev) {}
> > 
> > static inline int pci_get_host_bridge_segment(const struct
> > dt_device_node *node,
> >   uint16_t *segment)
> > {
> >     ASSERT_UNREACHABLE();
> >     return -EINVAL;
> > }
> > 
> > static inline int pci_get_new_domain_nr(void)
> > {
> >     ASSERT_UNREACHABLE();
> >     return -1;
> > }
> > 
> > #endif  /*!CONFIG_HAS_PCI*/
> > 
> > And if is_pci_passthrough_enabled(), arch_pci_init_pdev() is used
> > by
> > all architrectures but pci_get_host_bridge_segment() and
> > pci_get_new_domain_nr() is ARM specific.
> > Does it make sense to add them to  and ifdef them?
> 
> Counter question: Is the arch_pci_init_pdev() stub actually needed?
> The sole caller looks to be in a file which is only built when
> HAS_PCI=y.
You are right. It seems that there is no need for pci_init_pdev() stub.

> 
> For the Arm-only stubs (which are called from Arm-specific code
> afaics)
> all it would take is that the respective .c files include asm/pci.h
> (possibly alongside xen/pci.h).
We can do in that way.

Thanks.

~ Oleksii



Re: [PATCH for-4.18] docs: Fix IOMMU command line docs some more

2023-10-31 Thread Roger Pau Monné
On Tue, Oct 31, 2023 at 12:02:15PM +, Andrew Cooper wrote:
> Make the command line docs match the actual implementation, and state that the
> default behaviour is selected at compile time.
> 
> Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices 
> optional")
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Roger Pau Monné 
> CC: Wei Liu 
> CC: Marek Marczykowski-Górecki 
> CC: Henry Wang 
> ---
>  docs/misc/xen-command-line.pandoc | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index 6b07d0f3a17f..9a19a04157cb 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses 
> to that port.
>  > Default: `new` unless directed-EOI is supported
>  
>  ### iommu
> -= List of [ , verbose, debug, force, required, 
> quarantine[=scratch-page],
> += List of [ , verbose, debug, force, required,
> +quarantine=|scratch-page,

I think this should be quarantine=[|scratch-page], as just using
iommu=quarantine is a valid syntax and will enable basic quarantine.
IOW: the bool or scratch-page parameters are optional.

>  sharept, superpages, intremap, intpost, crash-disable,
>  snoop, qinval, igfx, amd-iommu-perdev-intremap,
>  dom0-{passthrough,strict} ]
> @@ -1519,7 +1520,8 @@ boolean (e.g. `iommu=no`) can override this and leave 
> the IOMMUs disabled.
>  successfully.
>  
>  *   The `quarantine` option can be used to control Xen's behavior when
> -de-assigning devices from guests.
> +de-assigning devices from guests.  The default behaviour is chosen at
> +compile time, and is one of 
> `CONFIG_IOMMU_QUARANTINE_{NONE,BASIC,SCRATCH_PAGE}`.

Do we also want to state that the current build time default is BASIC
if the user hasn't selected otherwise?

It's kind of problematic though, as distros might select a different
build time default and then the documentation would be out of sync.

Thanks, Roger.



[linux-linus test] 183625: regressions - FAIL

2023-10-31 Thread osstest service owner
flight 183625 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183625/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-examine  8 reboot   fail REGR. vs. 183617
 test-arm64-arm64-xl-credit2   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-thunderx  8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-credit1   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-xl-vhd   8 xen-boot fail REGR. vs. 183617
 test-arm64-arm64-libvirt-raw  8 xen-boot fail REGR. vs. 183617
 build-arm64-xsm   6 xen-buildfail REGR. vs. 183617
 test-armhf-armhf-libvirt 10 host-ping-check-xen  fail REGR. vs. 183617

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

version targeted for testing:
 linuxecb8cd2a9f7af7f99a6d4fa0a5a31822f6cfe255
baseline version:
 linuxffc253263a1375a65fa6c9f62a893e9767fbebfa

Last test of basis   183617  2023-10-30 08:36:55 Z1 days
Testing same since   183625  2023-10-31 02:27:44 Z0 days1 attempts


People who touched revisions under test:
  "Darrick J. Wong" 
  "Kuyo Chang (張建文)" 
  "Md. Haris Iqbal" 
  "Peter Zijlstra (Intel)" 
  "Rafael J. Wysocki" 
  Aaron Lu 
  Aaron Plattner 
  Adam Dunlap 
  Alexander Aring 
  Alexander Shishkin 
  Alexey Dobriyan 
  Alison Schofield 
  Amir Goldstein 
  Anand Jain 
  Anders Roxell 
  Ard Biesheuvel 
  Atul Kumar Pant 
  Babu Moger 
  Baolin Liu 
  Barry Song 
  Ben Wolsieffer 
  Bernd Schubert 
  Binbin Wu 
  Bjorn Helgaas 
  Boris Burkov 
  Borislav Petkov (AMD) 
  Borislav Petkov 
  Brett Holman 
  Brett Holman 
  Brian Foster 
  Brian Gerst 
  Chen Hanxiao 
  Chengming Zhou 
  Chris Webb 
  Christian Brauner 
  Christoph Hellwig 
  Christophe JAILLET 
  Christopher James Halse Rogers 
  Chuck Lever 
  Colin Ian King 
  Coly Li 
  Cuda-Chen 
  Cyril Hrubis 
  Dai Ngo 
  Dan Carpenter 
  Dan Robertson 
  Daniel B. Hill 
  Daniel Hill 
  Dave Hansen 
  Dave Kleikamp 
  David Hildenbrand 
  David Howells 
  David Kaplan 
  David Reaver 
  David Sterba 
  Derick Marks 
  Dominique Martinet 
  Elliot Berman 
  Eric Biggers 
  Fan Yu 
  Fangrui Song 
  Fenghua Yu 
  Filipe Manana 
  Finn Thain 
  Gao Xiang 
  Gautham R. Shenoy 
  Geert Uytterhoeven 
  Greg Kroah-Hartman 
  Guo Ren 
  Guo 

Re: [RFC PATCH 04/22] x86/msr-index: add references to vendor manuals

2023-10-31 Thread Edwin Torok



> On 31 Oct 2023, at 11:34, Andrew Cooper  wrote:
> 
> On 30/10/2023 4:15 pm, Jan Beulich wrote:
>> 
>>> --- a/xen/arch/x86/include/asm/msr-index.h
>>> +++ b/xen/arch/x86/include/asm/msr-index.h
>>> @@ -13,6 +13,16 @@
>>> * Blocks of related constants should be sorted by MSR index. The constant
>>> * names should be as concise as possible, and the bit names may have an
>>> * abbreviated name. Exceptions will be considered on a case-by-case basis.
>>> + *
>>> + * References:
>>> + * - 
>>> https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html
>>> + * Intel(R) 64 and IA-32 architectures SDM volume 4: Model-specific 
>>> registers
>>> + * Chapter 2, "Model-Specific Registers (MSRs)"
>>> 
>> ... at least Intel's URL has changed several times over the years. Volume
>> and chapter numbers change even more frequently. Any such is liable to go
>> stale at some point.
> 
> https://intel.com/sdm
> 
> This one has been valid for roughly the lifetime of intel.com, and is 
> committed to stay so.

That is useful to know, I'll update the URL.

> 
>> 
>> Jan
>> 
>> 
>>> + * - https://developer.amd.com/resources/developer-guides-manuals/
> 
> whereas AMD really have broken this one, and don't seem to be showing any 
> urgency in unbreaking it...  Right now there is no landing page at all for 
> manuals.
> 


Linux commits appear to reference a certain bugzilla that has the manuals 
uploaded: https://bugzilla.kernel.org/show_bug.cgi?id=206537
(although they will go stale in another way, e.g. I see no 2023 manuals there, 
but at least you know which manual a given commit referenced).
Although referencing someone else's bugzilla in the Xen codebase wouldn't be a 
nice thing to do, so if we do this it'd probably have to be something hosted on 
Xen infra.

For now I'll probably drop the URL and just keep the name (so at least you'd 
know what to search for).


Best regards,
--Edwin

> ~Andrew




Re: [XEN PATCH][for-4.19 v4 1/8] xen/include: add macro ISOLATE_LOW_BIT

2023-10-31 Thread Nicola Vetrini

On 2023-10-31 11:20, Jan Beulich wrote:

On 31.10.2023 11:03, Nicola Vetrini wrote:

On 2023-10-31 09:28, Nicola Vetrini wrote:

On 2023-10-31 08:43, Jan Beulich wrote:

On 30.10.2023 23:44, Stefano Stabellini wrote:

On Mon, 30 Oct 2023, Jan Beulich wrote:

On 27.10.2023 15:34, Nicola Vetrini wrote:

--- a/xen/include/xen/macros.h
+++ b/xen/include/xen/macros.h
@@ -8,8 +8,14 @@
 #define DIV_ROUND(n, d) (((n) + (d) / 2) / (d))
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))

-#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
-#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
+/*
+ * Given an unsigned integer argument, expands to a mask where
just the least
+ * significant nonzero bit of the argument is set, or 0 if no 
bits

are set.
+ */
+#define ISOLATE_LOW_BIT(x) ((x) & -(x))


Not even considering future Misra changes (which aiui may require
that
anyway), this generalization of the macro imo demands that its
argument
now be evaluated only once.


Fur sure that would be an improvement, but I don't see a trivial 
way

to
do it and this issue is also present today before the patch.


This was an issue here for MASK_EXTR() and MASK_INSR(), yes, but the
new
macro has wider use, and there was no issue elsewhere so far.


I think it
would be better to avoid scope-creeping this patch as we are 
already

at
v4 for something that was expected to be a trivial mechanical 
change.

I
would rather review the fix as a separate patch, maybe sent by you 
as

you probably have a specific implementation in mind?


#define ISOLATE_LOW_BIT(x) ({ \
typeof(x) x_ = (x); \
x_ & -x_; \
})

Hard to see the scope creep here. What I would consider scope creep 
I

specifically didn't even ask for: I'd like this macro to be
overridable
by an arch. Specifically (see my earlier naming hint) I'd like to 
use
x86's BMI insn BLSI in the context of "x86: allow Kconfig control 
over

psABI level", when ABI v2 or higher is in use.


I appreciate you suggesting an implementation; I'll send a v5
incorporating it.


There's an issue with this approach, though: since the macro is used
indirectly
in expressions that are e.g. case labels or array sizes, the build 
fails

(see [1] for instance).
Perhaps it's best to leave it as is?


Hmm. I'm afraid it's not an option to "leave as is", not the least 
because

- as said - I'm under the impression that another Misra rule requires
macro arguments to be evaluated exactly once. Best I can think of right
away is to have a macro for limited use (to address such build issues)
plus an inline function (for general use). But yes, maybe that then 
indeed

needs to be a 2nd step.

Jan

[1] 
https://gitlab.com/xen-project/people/bugseng/xen/-/jobs/5423693947




There is no such MISRA Rule afaik: R23.7 is similar, but only for C11 
generic selections.


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



[PATCH for-4.18] docs: Fix IOMMU command line docs some more

2023-10-31 Thread Andrew Cooper
Make the command line docs match the actual implementation, and state that the
default behaviour is selected at compile time.

Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices 
optional")
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
CC: Wei Liu 
CC: Marek Marczykowski-Górecki 
CC: Henry Wang 
---
 docs/misc/xen-command-line.pandoc | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index 6b07d0f3a17f..9a19a04157cb 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses to 
that port.
 > Default: `new` unless directed-EOI is supported
 
 ### iommu
-= List of [ , verbose, debug, force, required, 
quarantine[=scratch-page],
+= List of [ , verbose, debug, force, required,
+quarantine=|scratch-page,
 sharept, superpages, intremap, intpost, crash-disable,
 snoop, qinval, igfx, amd-iommu-perdev-intremap,
 dom0-{passthrough,strict} ]
@@ -1519,7 +1520,8 @@ boolean (e.g. `iommu=no`) can override this and leave the 
IOMMUs disabled.
 successfully.
 
 *   The `quarantine` option can be used to control Xen's behavior when
-de-assigning devices from guests.
+de-assigning devices from guests.  The default behaviour is chosen at
+compile time, and is one of 
`CONFIG_IOMMU_QUARANTINE_{NONE,BASIC,SCRATCH_PAGE}`.
 
 When a PCI device is assigned to an untrusted domain, it is possible
 for that domain to program the device to DMA to an arbitrary address.

base-commit: 9659b2a6d73b14620e187f9c626a09323853c459
-- 
2.30.2




Re: [RFC PATCH 04/22] x86/msr-index: add references to vendor manuals

2023-10-31 Thread Andrew Cooper
On 30/10/2023 4:15 pm, Jan Beulich wrote:
>> --- a/xen/arch/x86/include/asm/msr-index.h
>> +++ b/xen/arch/x86/include/asm/msr-index.h
>> @@ -13,6 +13,16 @@
>>   * Blocks of related constants should be sorted by MSR index.  The constant
>>   * names should be as concise as possible, and the bit names may have an
>>   * abbreviated name.  Exceptions will be considered on a case-by-case basis.
>> + *
>> + * References:
>> + * - 
>> https://software.intel.com/content/www/us/en/develop/articles/intel-sdm.html
>> + *Intel(R) 64 and IA-32 architectures SDM volume 4: Model-specific 
>> registers
>> + *Chapter 2, "Model-Specific Registers (MSRs)"
> ... at least Intel's URL has changed several times over the years. Volume
> and chapter numbers change even more frequently. Any such is liable to go
> stale at some point.

https://intel.com/sdm

This one has been valid for roughly the lifetime of intel.com, and is
committed to stay so.

>
> Jan
>
>> + * - https://developer.amd.com/resources/developer-guides-manuals/

whereas AMD really have broken this one, and don't seem to be showing
any urgency in unbreaking it...  Right now there is no landing page at
all for manuals.

~Andrew

Re: [PATCH v4 4/5] [FUTURE] xen/arm: enable vPCI for domUs

2023-10-31 Thread Jan Beulich
On 31.10.2023 00:52, Stewart Hildebrand wrote:
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1618,6 +1618,16 @@ int iommu_do_pci_domctl(
>  bus = PCI_BUS(machine_sbdf);
>  devfn = PCI_DEVFN(machine_sbdf);
>  
> +if ( IS_ENABLED(CONFIG_ARM) &&
> + !is_hardware_domain(d) &&
> + !is_system_domain(d) &&
> + (!IS_ENABLED(CONFIG_HAS_VPCI_GUEST_SUPPORT) || !has_vpci(d)) )

I don't think you need the explicit ARM check; that's redundant with
checking !HAS_VPCI_GUEST_SUPPORT. It's also not really clear why you
need to check for the system domain here.

> +{
> +printk(XENLOG_G_WARNING "Cannot assign %pp to %pd: vPCI support 
> not enabled\n",
> +   _SBDF(seg, bus, devfn), d);

ret = -EPERM;

(or some other suitable error indicator)

Jan

> +break;
> +}
> +
>  pcidevs_lock();
>  ret = device_assigned(seg, bus, devfn);
>  if ( domctl->cmd == XEN_DOMCTL_test_assign_device )




Re: [PATCH v4 2/5] xen/vpci: move xen_domctl_createdomain vPCI flag to common

2023-10-31 Thread Jan Beulich
On 31.10.2023 00:52, Stewart Hildebrand wrote:
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -607,7 +607,8 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  {
>  unsigned int max_vcpus;
>  unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap);
> -unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> XEN_DOMCTL_CDF_vpmu);
> +unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | 
> XEN_DOMCTL_CDF_vpmu |
> +   XEN_DOMCTL_CDF_vpci);

Is the flag (going to be, with the initial work) okay to have for Dom0
on Arm?

> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -712,7 +712,8 @@ int arch_sanitise_domain_config(struct 
> xen_domctl_createdomain *config)
>  return 0;
>  }
>  
> -static bool emulation_flags_ok(const struct domain *d, uint32_t emflags)
> +static bool emulation_flags_ok(const struct domain *d, uint32_t emflags,
> +   uint32_t cdf)

While apparently views differ, ./CODING_STYLE wants "unsigned int" to be
used for the latter two arguments.

> @@ -722,14 +723,17 @@ static bool emulation_flags_ok(const struct domain *d, 
> uint32_t emflags)
>  if ( is_hvm_domain(d) )
>  {
>  if ( is_hardware_domain(d) &&
> - emflags != (X86_EMU_VPCI | X86_EMU_LAPIC | X86_EMU_IOAPIC) )
> + (!( cdf & XEN_DOMCTL_CDF_vpci ) ||

Nit: Stray blanks inside the inner parentheses.

> +  emflags != (X86_EMU_LAPIC | X86_EMU_IOAPIC)) )
>  return false;
>  if ( !is_hardware_domain(d) &&
> - emflags != (X86_EMU_ALL & ~X86_EMU_VPCI) &&
> - emflags != X86_EMU_LAPIC )
> + ((cdf & XEN_DOMCTL_CDF_vpci) ||
> +  (emflags != X86_EMU_ALL &&
> +   emflags != X86_EMU_LAPIC)) )
>  return false;
>  }
> -else if ( emflags != 0 && emflags != X86_EMU_PIT )
> +else if ( (cdf & XEN_DOMCTL_CDF_vpci) ||

Wouldn't this better be enforced in common code?

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -892,10 +892,11 @@ static struct domain *__init create_dom0(const module_t 
> *image,
>  {
>  dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
> ((hvm_hap_supported() && !opt_dom0_shadow) ?
> -XEN_DOMCTL_CDF_hap : 0));
> +XEN_DOMCTL_CDF_hap : 0) |
> +   XEN_DOMCTL_CDF_vpci);

Less of a change and imo slightly neater as a result would be to simply
put the addition on the same line where CDF_hvm already is. But as with
many style aspects, views may differ here of course ...

Jan



Re: [PATCH for-4.18 v2] automation: fix race condition in adl-suspend test

2023-10-31 Thread Andrew Cooper
On 31/10/2023 9:58 am, Henry Wang wrote:
> Hi Marek,
>
>> On Oct 31, 2023, at 10:16, Marek Marczykowski-Górecki 
>>  wrote:
>>
>> If system suspends too quickly, the message for the test controller to
>> wake up the system may be not sent to the console before suspending.
>> This will cause the test to timeout.
>>
>> Fix this by calling sync on the console and waiting a bit after printing
>> the message. The test controller then resumes the system 30s after the
>> message, so as long as the delay + suspending takes less time it is
>> okay.
>>
>> Signed-off-by: Marek Marczykowski-Górecki 
> I think now that we branched, this patch should be committed to both staging 
> and staging-4.18.
> For staging 4.18:
>
> Release-acked-by: Henry Wang 
>
> I will remove the commit moratorium for staging once OSSTest does a 
> successful sync between
> staging and master. Thanks.

Acked-by: Andrew Cooper 

I'll get this sorted now.

~Andrew



Re: [XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3

2023-10-31 Thread Julien Grall

Hi,

On 31/10/2023 10:31, Jan Beulich wrote:

On 31.10.2023 10:25, Federico Serafini wrote:

Make function defintions and declarations consistent.


typo: s/defintions/definitions/


No functional change.

Signed-off-by: Federico Serafini 


Acked-by: Jan Beulich 

However, ...


---
Changes in v2:
- use 'ptr' do denote a const void * parameter.


... not even this (let alone the description) clarifies what the
inconsistency was. I had to go check to figure that x86 already uses
"ptr". Such things could do with spelling out.


+1. The more that x86 was the "odd" one but it was chosen to use the 
variant everywhere.


With the commit message clarified:

Acked-by: Julien Grall 

Cheers,




@@ -55,8 +55,8 @@ static inline void *__map_domain_page_global(const struct 
page_info *pg)
  
  #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 unmap_domain_page(ptr)   ((void)(ptr))
+#define domain_page_map_to_mfn(ptr)  _mfn(__virt_to_mfn((unsigned 
long)(ptr)))


Padding wants to not be screwed by the change (one of the blanks will
want dropping). I guess this can be taken care of while committing.

Jan


--
Julien Grall



Re: [PATCH] xen: remove

2023-10-31 Thread Jan Beulich
On 31.10.2023 11:12, Oleksii Kurochko wrote:
>  only declares udelay() function so udelay()  
> declaration was moved to xen/delay.h.
> 
> For x86, __udelay() was renamed to udelay() and removed
> inclusion of  in x86 code.
> 
> Signed-off-by: Oleksii Kurochko 
> ---
>  xen/arch/arm/include/asm/delay.h   | 14 --
>  xen/arch/riscv/include/asm/delay.h | 13 -
>  xen/arch/x86/cpu/microcode/core.c  |  2 +-
>  xen/arch/x86/delay.c   |  2 +-
>  xen/arch/x86/include/asm/delay.h   | 13 -
>  xen/include/xen/delay.h|  3 ++-
>  6 files changed, 4 insertions(+), 43 deletions(-)
>  delete mode 100644 xen/arch/arm/include/asm/delay.h
>  delete mode 100644 xen/arch/riscv/include/asm/delay.h
>  delete mode 100644 xen/arch/x86/include/asm/delay.h

What about xen/arch/ppc/include/asm/delay.h? With that also removed
Reviewed-by: Jan Beulich 
(and maybe also Suggested-by:?)

Jan



Re: [XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3

2023-10-31 Thread Jan Beulich
On 31.10.2023 10:25, Federico Serafini wrote:
> Make function defintions and declarations consistent.
> No functional change.
> 
> Signed-off-by: Federico Serafini 

Acked-by: Jan Beulich 

However, ...

> ---
> Changes in v2:
> - use 'ptr' do denote a const void * parameter.

... not even this (let alone the description) clarifies what the
inconsistency was. I had to go check to figure that x86 already uses
"ptr". Such things could do with spelling out.

> @@ -55,8 +55,8 @@ static inline void *__map_domain_page_global(const struct 
> page_info *pg)
>  
>  #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 unmap_domain_page(ptr)   ((void)(ptr))
> +#define domain_page_map_to_mfn(ptr)  _mfn(__virt_to_mfn((unsigned 
> long)(ptr)))

Padding wants to not be screwed by the change (one of the blanks will
want dropping). I guess this can be taken care of while committing.

Jan



[PATCH v3] x86/hvm/dom0: fix PVH initrd and metadata placement

2023-10-31 Thread Xenia Ragiadakou
Zephyr image consists of multiple non-contiguous load segments
that reside in different RAM regions. For instance:
ELF: phdr: paddr=0x1000 memsz=0x8000
ELF: phdr: paddr=0x10 memsz=0x28a90
ELF: phdr: paddr=0x128aa0 memsz=0x7560
ELF: memory: 0x1000 -> 0x13

However, the logic that determines the best placement for dom0
initrd and metadata, assumes that the image is fully contained
in a single RAM region, not taking into account the cases where:
(1) start > kernel_start && end > kernel_end
(2) start < kernel_start && end < kernel_end
(3) start > kernel_start && end < kernel_end

In case (1), the evaluation will result in end = kernel_start,
i.e. end < start, and will load initrd in the middle of the kernel.
In case (2), the evaluation will result in start = kernel_end,
i.e. end < start, and will load initrd at kernel_end, that is out
of the memory region under evaluation.
In case (3), the evaluation will result in either end = kernel_start
or start = kernel_end but in both cases will be end < start, and
will either load initrd in the middle of the image, or arbitrarily
at kernel_end.

This patch reorganizes the conditionals to include so far unconsidered
cases as well, uniformly returning the lowest available address.

Fixes: 73b47eea2104 ('x86/dom0: improve PVH initrd and metadata placement')
Signed-off-by: Xenia Ragiadakou 
Signed-off-by: Jan Beulich 
Reviewed-by: Roger Pau Monné 
---

Changes in v3:
- mention Zephyr in commit message (Roger)

Changes in v2:
- cover further cases of overlap (Jan)
- mention with an in-code comment that a proper, more fine-grained
  solution can be implemented using a rangeset (Roger)

 xen/arch/x86/hvm/dom0_build.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index c7d47d0d4c..62debc7415 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -515,16 +515,23 @@ static paddr_t __init find_memory(
 
 ASSERT(IS_ALIGNED(start, PAGE_SIZE) && IS_ALIGNED(end, PAGE_SIZE));
 
+/*
+ * NB: Even better would be to use rangesets to determine a suitable
+ * range, in particular in case a kernel requests multiple heavily
+ * discontiguous regions (which right now we fold all into one big
+ * region).
+ */
 if ( end <= kernel_start || start >= kernel_end )
-; /* No overlap, nothing to do. */
+{
+/* No overlap, just check whether the region is large enough. */
+if ( end - start >= size )
+return start;
+}
 /* Deal with the kernel already being loaded in the region. */
-else if ( kernel_start - start > end - kernel_end )
-end = kernel_start;
-else
-start = kernel_end;
-
-if ( end - start >= size )
+else if ( kernel_start > start && kernel_start - start >= size )
 return start;
+else if ( kernel_end < end && end - kernel_end >= size )
+return kernel_end;
 }
 
 return INVALID_PADDR;
-- 
2.34.1




Re: [XEN PATCH][for-4.19 v4 1/8] xen/include: add macro ISOLATE_LOW_BIT

2023-10-31 Thread Jan Beulich
On 31.10.2023 11:03, Nicola Vetrini wrote:
> On 2023-10-31 09:28, Nicola Vetrini wrote:
>> On 2023-10-31 08:43, Jan Beulich wrote:
>>> On 30.10.2023 23:44, Stefano Stabellini wrote:
 On Mon, 30 Oct 2023, Jan Beulich wrote:
> On 27.10.2023 15:34, Nicola Vetrini wrote:
>> --- a/xen/include/xen/macros.h
>> +++ b/xen/include/xen/macros.h
>> @@ -8,8 +8,14 @@
>>  #define DIV_ROUND(n, d) (((n) + (d) / 2) / (d))
>>  #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
>>
>> -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
>> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
>> +/*
>> + * Given an unsigned integer argument, expands to a mask where 
>> just the least
>> + * significant nonzero bit of the argument is set, or 0 if no bits 
>> are set.
>> + */
>> +#define ISOLATE_LOW_BIT(x) ((x) & -(x))
>
> Not even considering future Misra changes (which aiui may require 
> that
> anyway), this generalization of the macro imo demands that its 
> argument
> now be evaluated only once.

 Fur sure that would be an improvement, but I don't see a trivial way 
 to
 do it and this issue is also present today before the patch.
>>>
>>> This was an issue here for MASK_EXTR() and MASK_INSR(), yes, but the 
>>> new
>>> macro has wider use, and there was no issue elsewhere so far.
>>>
 I think it
 would be better to avoid scope-creeping this patch as we are already 
 at
 v4 for something that was expected to be a trivial mechanical change. 
 I
 would rather review the fix as a separate patch, maybe sent by you as
 you probably have a specific implementation in mind?
>>>
>>> #define ISOLATE_LOW_BIT(x) ({ \
>>> typeof(x) x_ = (x); \
>>> x_ & -x_; \
>>> })
>>>
>>> Hard to see the scope creep here. What I would consider scope creep I
>>> specifically didn't even ask for: I'd like this macro to be 
>>> overridable
>>> by an arch. Specifically (see my earlier naming hint) I'd like to use
>>> x86's BMI insn BLSI in the context of "x86: allow Kconfig control over
>>> psABI level", when ABI v2 or higher is in use.
>>
>> I appreciate you suggesting an implementation; I'll send a v5 
>> incorporating it.
> 
> There's an issue with this approach, though: since the macro is used 
> indirectly
> in expressions that are e.g. case labels or array sizes, the build fails 
> (see [1] for instance).
> Perhaps it's best to leave it as is?

Hmm. I'm afraid it's not an option to "leave as is", not the least because
- as said - I'm under the impression that another Misra rule requires
macro arguments to be evaluated exactly once. Best I can think of right
away is to have a macro for limited use (to address such build issues)
plus an inline function (for general use). But yes, maybe that then indeed
needs to be a 2nd step.

Jan

> [1] https://gitlab.com/xen-project/people/bugseng/xen/-/jobs/5423693947
> 




[PATCH] xen: remove

2023-10-31 Thread Oleksii Kurochko
 only declares udelay() function so udelay()  
declaration was moved to xen/delay.h.

For x86, __udelay() was renamed to udelay() and removed
inclusion of  in x86 code.

Signed-off-by: Oleksii Kurochko 
---
 xen/arch/arm/include/asm/delay.h   | 14 --
 xen/arch/riscv/include/asm/delay.h | 13 -
 xen/arch/x86/cpu/microcode/core.c  |  2 +-
 xen/arch/x86/delay.c   |  2 +-
 xen/arch/x86/include/asm/delay.h   | 13 -
 xen/include/xen/delay.h|  3 ++-
 6 files changed, 4 insertions(+), 43 deletions(-)
 delete mode 100644 xen/arch/arm/include/asm/delay.h
 delete mode 100644 xen/arch/riscv/include/asm/delay.h
 delete mode 100644 xen/arch/x86/include/asm/delay.h

diff --git a/xen/arch/arm/include/asm/delay.h b/xen/arch/arm/include/asm/delay.h
deleted file mode 100644
index 042907d9d5..00
--- a/xen/arch/arm/include/asm/delay.h
+++ /dev/null
@@ -1,14 +0,0 @@
-#ifndef _ARM_DELAY_H
-#define _ARM_DELAY_H
-
-extern void udelay(unsigned long usecs);
-
-#endif /* defined(_ARM_DELAY_H) */
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/riscv/include/asm/delay.h 
b/xen/arch/riscv/include/asm/delay.h
deleted file mode 100644
index 2d59622c75..00
--- a/xen/arch/riscv/include/asm/delay.h
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2009 Chen Liqin 
- * Copyright (C) 2016 Regents of the University of California
- */
-
-#ifndef _ASM_RISCV_DELAY_H
-#define _ASM_RISCV_DELAY_H
-
-#define udelay udelay
-extern void udelay(unsigned long usecs);
-
-#endif /* _ASM_RISCV_DELAY_H */
diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index c3fee62906..48822360c0 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -23,6 +23,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -35,7 +36,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c
index 2662c26272..b3a41881a1 100644
--- a/xen/arch/x86/delay.c
+++ b/xen/arch/x86/delay.c
@@ -15,7 +15,7 @@
 #include 
 #include 
 
-void __udelay(unsigned long usecs)
+void udelay(unsigned long usecs)
 {
 unsigned long ticks = usecs * (cpu_khz / 1000);
 unsigned long s, e;
diff --git a/xen/arch/x86/include/asm/delay.h b/xen/arch/x86/include/asm/delay.h
deleted file mode 100644
index 9be2f46590..00
--- a/xen/arch/x86/include/asm/delay.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef _X86_DELAY_H
-#define _X86_DELAY_H
-
-/*
- * Copyright (C) 1993 Linus Torvalds
- *
- * Delay routines calling functions in arch/i386/lib/delay.c
- */
-
-extern void __udelay(unsigned long usecs);
-#define udelay(n) __udelay(n)
-
-#endif /* defined(_X86_DELAY_H) */
diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h
index 9d70ef035f..a5189329c7 100644
--- a/xen/include/xen/delay.h
+++ b/xen/include/xen/delay.h
@@ -3,8 +3,9 @@
 
 /* Copyright (C) 1993 Linus Torvalds */
 
-#include 
 #define mdelay(n) (\
{unsigned long msec=(n); while (msec--) udelay(1000);})
 
+void udelay(unsigned long usecs);
+
 #endif /* defined(_LINUX_DELAY_H) */
-- 
2.41.0




Re: [RFC PATCH 03/22] x86/msr: always allow a pinned Dom0 to read any unknown MSR

2023-10-31 Thread Jan Beulich
On 31.10.2023 10:31, Edwin Torok wrote:
>> On 30 Oct 2023, at 16:29, Jan Beulich  wrote:
>> On 25.10.2023 21:29, Edwin Török wrote:
>>> This can be useful if you realize you have to inspect the value of an
>>> MSR in production, without having to change into a new Xen first that
>>> handles the MSR.
>>
>> Yet on a non-pinned Dom0 you'd still be lost. Since iirc we generally
>> advise against pinning,
> 
> You can temporarily pin while debugging the issue (e.g. pin just 1 CPU from 
> Dom0, and "walk" all your physical CPUs with it if you have to,
> so that you query them all), e.g. with 'xl vcpu-pin'.
> Although that is more invasive than reading a value.
>  
> Or alternatively have another (privileged) interface to read the MSR for a 
> given core without exposing it to any guests, that way you don't affect the 
> running system at all
> (which would be preferable in a production environment), i.e. a Xen 
> equivalent of 'rdmsr'.

The interface we have (XENPF_resource_op) is, despite being privileged,
deliberately (so far at least) not permitting access to arbitrary MSRs.

In our old XenoLinux forward port we had an extension to the msr.ko
module to allow pCPU-based MSR accesses (and I had a private extension
to the rdmsr/wrmsr user space tools making use of that), but even that
would have been subject to restrictions enforced by Xen as to which
MSRs are accessible.

>> I wonder of how much use such a change would be,
>> when it effectively undoes what we deliberately did a while ago.
>>
>>> --- a/xen/arch/x86/hvm/svm/svm.c
>>> +++ b/xen/arch/x86/hvm/svm/svm.c
>>> @@ -1933,6 +1933,9 @@ static int cf_check svm_msr_read_intercept(
>>> break;
>>>
>>> default:
>>> +if ( is_hwdom_pinned_vcpu(v) && !rdmsr_safe(msr, *msr_content) )
>>> +break;
>>> +
>>> if ( d->arch.msr_relaxed && !rdmsr_safe(msr, tmp) )
>>> {
>>> *msr_content = 0;
>>
>> If we went as far as undoing some of what was done, I'd then wonder
>> whether instead we should mandate relaxed mode to be enabled on such a
>> Dom0. Then, instead of returning fake 0 here, the actual value could
>> be returned in the specific case of (pinned?) Dom0.
> 
> 
> Can relaxed mode be enabled at runtime?

Not right now, no. But a hypfs control could certainly be added, with
suitable justification.

> I'd be happy with either solution, but it should be something that can be 
> enabled at runtime
> (if you have to reboot Xen then you may lose the bug repro that you want to 
> gather more information on).
> Although changing such a setting in a production environment may still be 
> risky, because the guest will then become very confused that it has 
> previously read some 0s, now there are some real values, and later when you 
> flip the switch off it gets 0s again.

Indeed. If you flipped such a control for any domain at runtime, you'd
better first check that this wouldn't cause any such issues.

Jan



live migration fails: qemu placing pci devices at different locations

2023-10-31 Thread James Dingwall
Hi,

I'm having a bit of trouble performing live migration between hvm guests.  The
sending side is xen 4.14.5 (qemu 5.0), receiving 4.15.5 (qemu 5.1).  The error
message recorded in qemu-dm---incoming.log:

qemu-system-i386: Unknown savevm section or instance ':00:04.0/vga' 0. Make 
sure that your current VM setup matches your saved VM setup, including any 
hotplugged devices

I have patched libxl_dm.c to explicitly assign `addr=xx` values for various
devices and when these are correct the domain migrates correctly.  However
the configuration differences between guests means that the values are not
consistent.  The domain config file doesn't allow the pci address to be
expressed in the configuration for, e.g. `soundhw="DEVICE"`

e.g. 

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index 6e531863ac0..daa7c49846f 100644
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -1441,7 +1441,7 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
 flexarray_append(dm_args, "-spice");
 flexarray_append(dm_args, spiceoptions);
 if (libxl_defbool_val(b_info->u.hvm.spice.vdagent)) {
-flexarray_vappend(dm_args, "-device", "virtio-serial",
+flexarray_vappend(dm_args, "-device", "virtio-serial,addr=04",
 "-chardev", "spicevmc,id=vdagent,name=vdagent", "-device",
 "virtserialport,chardev=vdagent,name=com.redhat.spice.0",
 NULL);

The order of devices on the qemu command line (below) appears to be the same
so my assumption is that the internals of qemu have resulted in things being
connected in a different order.  The output of a Windows `lspci` tool is
also included.

Could anyone make any additional suggestions on how I could try to gain
consistency between the different qemu versions?

Thanks,
James


xen 4.14.5

/usr/lib/xen/bin/qemu-system-i386 -xen-domid 19 -no-shutdown
  -chardev socket,id=libxl-cmd,fd=19,server,nowait -S 
  -mon chardev=libxl-cmd,mode=control
  -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-19,server,nowait
  -mon chardev=libxenstat-cmd,mode=control
  -nodefaults -no-user-config -name  -vnc 0.0.0.0:93 -display none
  -k en-us
  -spice 
port=35993,tls-port=0,addr=127.0.0.1,disable-ticketing,agent-mouse=on,disable-copy-paste,image-compression=auto_glz
 
  -device virtio-serial -chardev spicevmc,id=vdagent,name=vdagent
  -device virtserialport,chardev=vdagent,name=com.redhat.spice.0
  -device VGA,vgamem_mb=16
  -boot order=cn
  -usb -usbdevice tablet
  -soundhw hda
  -smp 2,maxcpus=2
  -device rtl8139,id=nic0,netdev=net0,mac=00:16:3e:64:c8:68
  -netdev type=tap,id=net0,ifname=vif19.0-emu,script=no,downscript=no
  -object 
tls-creds-x509,id=tls0,endpoint=client,dir=/etc/certificates/usbredir,verify-peer=yes
  -chardev 
socket,id=charredir_serial0,host=127.0.0.1,port=48052,reconnect=2,nodelay,keepalive=on,user-timeout=5
  -device isa-serial,chardev=charredir_serial0
  -chardev 
socket,id=charredir_serial1,host=127.0.0.1,port=48054,reconnect=2,nodelay,keepalive=on,user-timeout=5
  -device isa-serial,chardev=charredir_serial1
  -chardev 
socket,id=charredir_serial2,host=127.0.0.1,port=48055,reconnect=2,nodelay,keepalive=on,user-timeout=5
  -device pci-serial,chardev=charredir_serial2
  -trace events=/etc/xen/qemu-trace-options -machine xenfv -m 2032
  -drive file=/dev/drbd1002,if=ide,index=0,media=disk,format=raw,cache=writeback
  -drive file=/dev/drbd1003,if=ide,index=1,media=disk,format=raw,cache=writeback
  -runas 131091:131072

00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.2 USB controller: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] 
(rev 01)
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
00:03.0 Audio device: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) 
High Definition Audio Controller (rev 01)
00:04.0 Communication controller: Red Hat, Inc Virtio console
00:05.0 VGA compatible controller: Device 1234: (rev 02)
00:07.0 Serial controller: Red Hat, Inc. QEMU PCI 16550A Adapter (rev 01)



xen 4.15.5

/usr/lib/xen/bin/qemu-system-i386 -xen-domid 15 -no-shutdown
  -chardev socket,id=libxl-cmd,fd=19,server=on,wait=off -S
  -mon chardev=libxl-cmd,mode=control
  -chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-15,server=on,wait=off
  -mon chardev=libxenstat-cmd,mode=control
  -nodefaults -no-user-config -name  -vnc 0.0.0.0:93 -display none
  -k en-us
  -spice 
port=35993,tls-port=0,addr=127.0.0.1,disable-ticketing=on,agent-mouse=on,disable-copy-paste=on,image-compression=auto_glz
  -device virtio-serial -chardev spicevmc,id=vdagent,name=vdagent
  -device 

Re: [XEN PATCH][for-4.19 v4 1/8] xen/include: add macro ISOLATE_LOW_BIT

2023-10-31 Thread Nicola Vetrini

On 2023-10-31 09:28, Nicola Vetrini wrote:

On 2023-10-31 08:43, Jan Beulich wrote:

On 30.10.2023 23:44, Stefano Stabellini wrote:

On Mon, 30 Oct 2023, Jan Beulich wrote:

On 27.10.2023 15:34, Nicola Vetrini wrote:

--- a/xen/include/xen/macros.h
+++ b/xen/include/xen/macros.h
@@ -8,8 +8,14 @@
 #define DIV_ROUND(n, d) (((n) + (d) / 2) / (d))
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))

-#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
-#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
+/*
+ * Given an unsigned integer argument, expands to a mask where 
just the least
+ * significant nonzero bit of the argument is set, or 0 if no bits 
are set.

+ */
+#define ISOLATE_LOW_BIT(x) ((x) & -(x))


Not even considering future Misra changes (which aiui may require 
that
anyway), this generalization of the macro imo demands that its 
argument

now be evaluated only once.


Fur sure that would be an improvement, but I don't see a trivial way 
to

do it and this issue is also present today before the patch.


This was an issue here for MASK_EXTR() and MASK_INSR(), yes, but the 
new

macro has wider use, and there was no issue elsewhere so far.


I think it
would be better to avoid scope-creeping this patch as we are already 
at
v4 for something that was expected to be a trivial mechanical change. 
I

would rather review the fix as a separate patch, maybe sent by you as
you probably have a specific implementation in mind?


#define ISOLATE_LOW_BIT(x) ({ \
typeof(x) x_ = (x); \
x_ & -x_; \
})

Hard to see the scope creep here. What I would consider scope creep I
specifically didn't even ask for: I'd like this macro to be 
overridable

by an arch. Specifically (see my earlier naming hint) I'd like to use
x86's BMI insn BLSI in the context of "x86: allow Kconfig control over
psABI level", when ABI v2 or higher is in use.

Jan


I appreciate you suggesting an implementation; I'll send a v5 
incorporating it.


There's an issue with this approach, though: since the macro is used 
indirectly
in expressions that are e.g. case labels or array sizes, the build fails 
(see [1] for instance).

Perhaps it's best to leave it as is?

[1] https://gitlab.com/xen-project/people/bugseng/xen/-/jobs/5423693947

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



  1   2   >