[PATCH] tools/libxs: Open /dev/xen/xenbus fds as O_CLOEXEC

[Andrew Cooper]

The header description for xs_open() goes as far as to suggest that the fd is
O_CLOEXEC, but it isn't actually.

`xl devd` has been observed leaking /dev/xen/xenbus into children.

Link: https://github.com/QubesOS/qubes-issues/issues/8292
Reported-by: Demi Marie Obenour 
Signed-off-by: Andrew Cooper 
---
CC: Anthony PERARD 
CC: Juergen Gross 
CC: Demi Marie Obenour 
CC: Marek Marczykowski-Górecki 

Entirely speculative patch based on a Matrix report
---
 tools/libs/store/xs.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index 140b9a28395e..1f74fb3c44a2 100644
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -54,6 +54,10 @@ struct xs_stored_msg {
 #include 
 #endif
 
+#ifndef O_CLOEXEC
+#define O_CLOEXEC 0
+#endif
+
 struct xs_handle {
/* Communications channel to xenstore daemon. */
int fd;
@@ -227,7 +231,7 @@ static int get_socket(const char *connect_to)
 static int get_dev(const char *connect_to)
 {
/* We cannot open read-only because requests are writes */
-   return open(connect_to, O_RDWR);
+   return open(connect_to, O_RDWR|O_CLOEXEC);
 }
 
 static int all_restrict_cb(Xentoolcore__Active_Handle *ah, domid_t domid) {

base-commit: feb9158a620040846d76981acbe8ea9e2255a07b
-- 
2.30.2

Re: [PATCH v6 3/7] x86/hvm: Allow access to registers on the same page as MSI-X table

[Marek Marczykowski-Górecki]

On Fri, May 03, 2024 at 10:33:38AM +0200, Roger Pau Monné wrote:
> On Fri, Apr 26, 2024 at 07:54:00PM +0200, Marek Marczykowski-Górecki wrote:
> > Some devices (notably Intel Wifi 6 AX210 card) keep auxiliary registers
> > on the same page as MSI-X table. Device model (especially one in
> > stubdomain) cannot really handle those, as direct writes to that page is
> > refused (page is on the mmio_ro_ranges list). Instead, extend
> > msixtbl_mmio_ops to handle such accesses too.
> > 
> > Doing this, requires correlating read/write location with guest
> > of MSI-X table address. Since QEMU doesn't map MSI-X table to the guest,
>   ^ extra 'of'?
> > it requires msixtbl_entry->gtable, which is HVM-only. Similar feature
> > for PV would need to be done separately.
> > 
> > This will be also used to read Pending Bit Array, if it lives on the same
> > page, making QEMU not needing /dev/mem access at all (especially helpful
> > with lockdown enabled in dom0). If PBA lives on another page, QEMU will
> > map it to the guest directly.
> > If PBA lives on the same page, discard writes and log a message.
> > Technically, writes outside of PBA could be allowed, but at this moment
> > the precise location of PBA isn't saved, and also no known device abuses
> > the spec in this way (at least yet).
> > 
> > To access those registers, msixtbl_mmio_ops need the relevant page
> > mapped. MSI handling already has infrastructure for that, using fixmap,
> > so try to map first/last page of the MSI-X table (if necessary) and save
> > their fixmap indexes. Note that msix_get_fixmap() does reference
> > counting and reuses existing mapping, so just call it directly, even if
> > the page was mapped before. Also, it uses a specific range of fixmap
> > indexes which doesn't include 0, so use 0 as default ("not mapped")
> > value - which simplifies code a bit.
> > 
> > GCC 12.2.1 gets confused about 'desc' variable:
> > 
> > arch/x86/hvm/vmsi.c: In function ‘msixtbl_range’:
> > arch/x86/hvm/vmsi.c:553:8: error: ‘desc’ may be used uninitialized 
> > [-Werror=maybe-uninitialized]
> >   553 | if ( desc )
> >   |^
> > arch/x86/hvm/vmsi.c:537:28: note: ‘desc’ was declared here
> >   537 | const struct msi_desc *desc;
> >   |^~~~
> > 
> > It's conditional initialization is actually correct (in the case where
> > it isn't initialized, function returns early), but to avoid
> > build failure initialize it explicitly to NULL anyway.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki 
> > ---
> > Changes in v6:
> > - use MSIX_CHECK_WARN macro
> > - extend assert on fixmap_idx
> > - add break in default label, after ASSERT_UNREACHABLE(), and move
> >   setting default there
> > - style fixes
> > Changes in v5:
> > - style fixes
> > - include GCC version in the commit message
> > - warn only once (per domain, per device) about failed adjacent access
> > Changes in v4:
> > - drop same_page parameter of msixtbl_find_entry(), distinguish two
> >   cases in relevant callers
> > - rename adj_access_table_idx to adj_access_idx
> > - code style fixes
> > - drop alignment check in adjacent_{read,write}() - all callers already
> >   have it earlier
> > - delay mapping first/last MSI-X pages until preparing device for a
> >   passthrough
> > v3:
> >  - merge handling into msixtbl_mmio_ops
> >  - extend commit message
> > v2:
> >  - adjust commit message
> >  - pass struct domain to msixtbl_page_handler_get_hwaddr()
> >  - reduce local variables used only once
> >  - log a warning if write is forbidden if MSI-X and PBA lives on the same
> >page
> >  - do not passthrough unaligned accesses
> >  - handle accesses both before and after MSI-X table
> > ---
> >  xen/arch/x86/hvm/vmsi.c| 200 --
> >  xen/arch/x86/include/asm/msi.h |   5 +-
> >  xen/arch/x86/msi.c |  41 +++-
> >  3 files changed, 236 insertions(+), 10 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
> > index 17983789..230e3a5dee3f 100644
> > --- a/xen/arch/x86/hvm/vmsi.c
> > +++ b/xen/arch/x86/hvm/vmsi.c
> > @@ -180,6 +180,10 @@ static bool msixtbl_initialised(const struct domain *d)
> >  return d->arch.hvm.msixtbl_list.next;
> >  }
> >  
> > +/*
> > + * Lookup an msixtbl_entry on the same page as given addr. It's up to the
> > + * caller to check if address is strictly part of the table - if relevant.
> > + */
> >  static struct msixtbl_entry *msixtbl_find_entry(
> >  struct vcpu *v, unsigned long addr)
> >  {
> > @@ -187,8 +191,8 @@ static struct msixtbl_entry *msixtbl_find_entry(
> >  struct domain *d = v->domain;
> >  
> >  list_for_each_entry( entry, >arch.hvm.msixtbl_list, list )
> > -if ( addr >= entry->gtable &&
> > - addr < entry->gtable + entry->table_len )
> > +if ( PFN_DOWN(addr) >= PFN_DOWN(entry->gtable) &&
> > + PFN_DOWN(addr) <= PFN_DOWN(entry->gtable + 

[linux-linus test] 185911: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. 
vs. 185870
 test-armhf-armhf-libvirt-vhd  8 xen-boot fail REGR. vs. 185870

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit2   8 xen-boot fail in 185907 pass in 185911
 test-armhf-armhf-xl-rtds  8 xen-boot fail in 185907 pass in 185911
 test-armhf-armhf-xl-credit1   8 xen-boot fail in 185907 pass in 185911
 test-armhf-armhf-xl   8 xen-boot   fail pass in 185907

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

version targeted for testing:
 linuxf03359bca01bf4372cf2c118cd9a987a5951b1c8
baseline version:
 linuxb947cc5bf6d793101135265352e205aeb30b54f0

Last test of basis   185870  2024-04-29 17:43:51 Z4 days
Failing since185888  2024-04-30 20:31:50 Z3 days7 attempts
Testing same since   185907  2024-05-03 02:43:45 Z0 days2 attempts


People who touched revisions under test:
  Alexander Gordeev 
  Alexander Potapenko 
  Alexandra Winter 
  Alexei Starovoitov 
  Andrii 

Re: [PATCH v3 9/9] xen/arm64: lib: Use the generic xen/linkage.h macros

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols.
> 
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Stefano Stabellini 


> ---
>  xen/arch/arm/arm64/lib/memchr.S  | 4 ++--
>  xen/arch/arm/arm64/lib/memcmp.S  | 4 ++--
>  xen/arch/arm/arm64/lib/memcpy.S  | 4 ++--
>  xen/arch/arm/arm64/lib/memmove.S | 4 ++--
>  xen/arch/arm/arm64/lib/memset.S  | 4 ++--
>  xen/arch/arm/arm64/lib/strchr.S  | 4 ++--
>  xen/arch/arm/arm64/lib/strcmp.S  | 4 ++--
>  xen/arch/arm/arm64/lib/strlen.S  | 4 ++--
>  xen/arch/arm/arm64/lib/strncmp.S | 4 ++--
>  xen/arch/arm/arm64/lib/strnlen.S | 4 ++--
>  xen/arch/arm/arm64/lib/strrchr.S | 4 ++--
>  11 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/lib/memchr.S b/xen/arch/arm/arm64/lib/memchr.S
> index 81f113bb1c..3d8aeca3ca 100644
> --- a/xen/arch/arm/arm64/lib/memchr.S
> +++ b/xen/arch/arm/arm64/lib/memchr.S
> @@ -29,7 +29,7 @@
>   * Returns:
>   *   x0 - address of first occurrence of 'c' or 0
>   */
> -ENTRY(memchr)
> +FUNC(memchr)
>   and w1, w1, #0xff
>  1:   subsx2, x2, #1
>   b.mi2f
> @@ -40,4 +40,4 @@ ENTRY(memchr)
>   ret
>  2:   mov x0, #0
>   ret
> -ENDPROC(memchr)
> +END(memchr)
> diff --git a/xen/arch/arm/arm64/lib/memcmp.S b/xen/arch/arm/arm64/lib/memcmp.S
> index 87c2537ffe..d77dd4ce52 100644
> --- a/xen/arch/arm/arm64/lib/memcmp.S
> +++ b/xen/arch/arm/arm64/lib/memcmp.S
> @@ -57,7 +57,7 @@ pos .reqx11
>  limit_wd .reqx12
>  mask .reqx13
>  
> -ENTRY(memcmp)
> +FUNC(memcmp)
>   cbz limit, .Lret0
>   eor tmp1, src1, src2
>   tst tmp1, #7
> @@ -254,4 +254,4 @@ CPU_LE( rev   data2, data2 )
>  .Lret0:
>   mov result, #0
>   ret
> -ENDPROC(memcmp)
> +END(memcmp)
> diff --git a/xen/arch/arm/arm64/lib/memcpy.S b/xen/arch/arm/arm64/lib/memcpy.S
> index d90d20ef3e..1e04b79010 100644
> --- a/xen/arch/arm/arm64/lib/memcpy.S
> +++ b/xen/arch/arm/arm64/lib/memcpy.S
> @@ -55,7 +55,7 @@ C_h .reqx12
>  D_l  .reqx13
>  D_h  .reqx14
>  
> -ENTRY(memcpy)
> +FUNC(memcpy)
>   mov dst, dstin
>   cmp count, #16
>   /*When memory length is less than 16, the accessed are not aligned.*/
> @@ -197,4 +197,4 @@ ENTRY(memcpy)
>   tst count, #0x3f
>   b.ne.Ltail63
>   ret
> -ENDPROC(memcpy)
> +END(memcpy)
> diff --git a/xen/arch/arm/arm64/lib/memmove.S 
> b/xen/arch/arm/arm64/lib/memmove.S
> index a49de845d0..14438dbe9c 100644
> --- a/xen/arch/arm/arm64/lib/memmove.S
> +++ b/xen/arch/arm/arm64/lib/memmove.S
> @@ -56,7 +56,7 @@ C_h .reqx12
>  D_l  .reqx13
>  D_h  .reqx14
>  
> -ENTRY(memmove)
> +FUNC(memmove)
>   cmp dstin, src
>   b.lomemcpy
>   add tmp1, src, count
> @@ -193,4 +193,4 @@ ENTRY(memmove)
>   tst count, #0x3f
>   b.ne.Ltail63
>   ret
> -ENDPROC(memmove)
> +END(memmove)
> diff --git a/xen/arch/arm/arm64/lib/memset.S b/xen/arch/arm/arm64/lib/memset.S
> index 5bf751521b..367fa60175 100644
> --- a/xen/arch/arm/arm64/lib/memset.S
> +++ b/xen/arch/arm/arm64/lib/memset.S
> @@ -53,7 +53,7 @@ dst .reqx8
>  tmp3w.reqw9
>  tmp3 .reqx9
>  
> -ENTRY(memset)
> +FUNC(memset)
>   mov dst, dstin  /* Preserve return value.  */
>   and A_lw, val, #255
>   orr A_lw, A_lw, A_lw, lsl #8
> @@ -212,4 +212,4 @@ ENTRY(memset)
>   andscount, count, zva_bits_x
>   b.ne.Ltail_maybe_long
>   ret
> -ENDPROC(memset)
> +END(memset)
> diff --git a/xen/arch/arm/arm64/lib/strchr.S b/xen/arch/arm/arm64/lib/strchr.S
> index 0506b0ff7f..83fd81e8ef 100644
> --- a/xen/arch/arm/arm64/lib/strchr.S
> +++ b/xen/arch/arm/arm64/lib/strchr.S
> @@ -27,7 +27,7 @@
>   * Returns:
>   *   x0 - address of first occurrence of 'c' or 0
>   */
> -ENTRY(strchr)
> +FUNC(strchr)
>   and w1, w1, #0xff
>  1:   ldrbw2, [x0], #1
>   cmp w2, w1
> @@ -37,4 +37,4 @@ ENTRY(strchr)
>   cmp w2, w1
>   cselx0, x0, xzr, eq
>   ret
> -ENDPROC(strchr)
> +END(strchr)
> diff --git a/xen/arch/arm/arm64/lib/strcmp.S b/xen/arch/arm/arm64/lib/strcmp.S
> index c6f42dd255..7677108e26 100644
> --- a/xen/arch/arm/arm64/lib/strcmp.S
> +++ b/xen/arch/arm/arm64/lib/strcmp.S
> @@ -59,7 +59,7 @@ tmp3.reqx9
>  zeroones .reqx10
>  pos  .reqx11
>  
> -ENTRY(strcmp)
> +FUNC(strcmp)
>   eor tmp1, src1, src2
>   mov zeroones, #REP8_01
>   tst tmp1, #7
> @@ -230,4 +230,4 @@ CPU_BE(   orr syndrome, diff, has_nul )
>   lsr data1, data1, #56
>   sub result, data1, data2, lsr #56
>   ret
> -ENDPROC(strcmp)
> +END(strcmp)
> diff --git a/xen/arch/arm/arm64/lib/strlen.S b/xen/arch/arm/arm64/lib/strlen.S
> index fb6aaf1a6a..10feedaf81 100644
> --- a/xen/arch/arm/arm64/lib/strlen.S
> +++ b/xen/arch/arm/arm64/lib/strlen.S
> @@ -56,7 

Re: [PATCH v3 8/9] xen/arm64: cache: Use the generic xen/linkage.h macros

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols.
> 
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Stefano Stabellini 


> ---
>  xen/arch/arm/arm64/cache.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S
> index 9a88a2b497..66ed85f735 100644
> --- a/xen/arch/arm/arm64/cache.S
> +++ b/xen/arch/arm/arm64/cache.S
> @@ -40,7 +40,7 @@
>   *   - kaddr   - kernel address
>   *   - size- size in question
>   */
> -ENTRY(__flush_dcache_area)
> +FUNC(__flush_dcache_area)
>   dcache_line_size x2, x3
>   add x1, x0, x1
>   sub x3, x2, #1
> @@ -51,4 +51,4 @@ ENTRY(__flush_dcache_area)
>   b.lo1b
>   dsb sy
>   ret
> -ENDPROC(__flush_dcache_area)
> +END(__flush_dcache_area)
> -- 
> 2.40.1
> 

Re: [PATCH v3 7/9] xen/arm64: mmu/head: Add missing code symbol annotations

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols
> and add missing annotations.
> 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  xen/arch/arm/arm64/mmu/head.S | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/mmu/head.S b/xen/arch/arm/arm64/mmu/head.S
> index fa40b696dd..7788bb95e5 100644
> --- a/xen/arch/arm/arm64/mmu/head.S
> +++ b/xen/arch/arm/arm64/mmu/head.S
> @@ -138,7 +138,7 @@
>   *
>   * Clobbers x0 - x4
>   */
> -create_page_tables:
> +FUNC_LOCAL(create_page_tables)
>  /* Prepare the page-tables for mapping Xen */
>  ldr   x0, =XEN_VIRT_START
>  create_table_entry boot_pgtable, boot_first, x0, 0, x1, x2, x3
> @@ -260,7 +260,7 @@ virtphys_clash:
>  /* Identity map clashes with boot_third, which we cannot handle yet 
> */
>  PRINT("- Unable to build boot page tables - virt and phys addresses 
> clash. -\r\n")
>  b fail
> -ENDPROC(create_page_tables)
> +END(create_page_tables)
>  
>  /*
>   * Turn on the Data Cache and the MMU. The function will return on the 1:1
> @@ -273,7 +273,7 @@ ENDPROC(create_page_tables)
>   *
>   * Clobbers x0 - x5
>   */
> -enable_mmu:
> +FUNC_LOCAL(enable_mmu)
>  mov   x4, x0
>  mov   x5, x1
>  PRINT_ID("- Turning on paging -\r\n")
> @@ -304,7 +304,7 @@ enable_mmu:
>  PRINT_ID("- Paging turned on -\r\n")
>  
>  ret
> -ENDPROC(enable_mmu)
> +END(enable_mmu)
>  
>  /*
>   * Enable mm (turn on the data cache and the MMU) for secondary CPUs.
> @@ -316,7 +316,7 @@ ENDPROC(enable_mmu)
>   *
>   * Clobbers x0 - x6
>   */
> -ENTRY(enable_secondary_cpu_mm)
> +FUNC(enable_secondary_cpu_mm)
>  mov   x6, lr
>  
>  load_paddr x0, init_ttbr
> @@ -328,7 +328,7 @@ ENTRY(enable_secondary_cpu_mm)
>  
>  /* Return to the virtual address requested by the caller. */
>  ret
> -ENDPROC(enable_secondary_cpu_mm)
> +END(enable_secondary_cpu_mm)
>  
>  /*
>   * Enable mm (turn on the data cache and the MMU) for the boot CPU.
> @@ -340,7 +340,7 @@ ENDPROC(enable_secondary_cpu_mm)
>   *
>   * Clobbers x0 - x6
>   */
> -ENTRY(enable_boot_cpu_mm)
> +FUNC(enable_boot_cpu_mm)
>  mov   x6, lr
>  
>  blcreate_page_tables
> @@ -365,7 +365,7 @@ ENTRY(enable_boot_cpu_mm)
>   * by the caller.
>   */
>  b remove_identity_mapping
> -ENDPROC(enable_boot_cpu_mm)
> +END(enable_boot_cpu_mm)
>  
>  /*
>   * Remove the 1:1 map from the page-tables. It is not easy to keep track
> @@ -377,7 +377,7 @@ ENDPROC(enable_boot_cpu_mm)
>   *
>   * Clobbers x0 - x1
>   */
> -remove_identity_mapping:
> +FUNC_LOCAL(remove_identity_mapping)
>  /*
>   * Find the zeroeth slot used. Remove the entry from zeroeth
>   * table if the slot is not XEN_ZEROETH_SLOT.
> @@ -419,20 +419,20 @@ identity_mapping_removed:
>  flush_xen_tlb_local
>  
>  ret
> -ENDPROC(remove_identity_mapping)
> +END(remove_identity_mapping)
>  
>  /* Fail-stop */
> -fail:   PRINT("- Boot failed -\r\n")
> +FUNC_LOCAL(fail)   PRINT("- Boot failed -\r\n")

move PRINT to newline


>  1:  wfe
>  b 1b
> -ENDPROC(fail)
> +END(fail)
>  
>  /*
>   * Switch TTBR
>   *
>   * x0ttbr
>   */
> -ENTRY(switch_ttbr_id)
> +FUNC(switch_ttbr_id)
>  /* 1) Ensure any previous read/write have completed */
>  dsbish
>  isb
> @@ -464,7 +464,7 @@ ENTRY(switch_ttbr_id)
>  isb
>  
>  ret
> -ENDPROC(switch_ttbr_id)
> +END(switch_ttbr_id)
>  
>  /*
>   * Local variables:
> -- 
> 2.40.1
> 

Re: [PATCH v3 6/9] xen/arm64: bpi: Add missing code symbol annotations

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols
> and add missing annotations.
> 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  xen/arch/arm/arm64/bpi.S | 20 
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> index 4e63825220..b16e4d1e29 100644
> --- a/xen/arch/arm/arm64/bpi.S
> +++ b/xen/arch/arm/arm64/bpi.S
> @@ -52,14 +52,15 @@
>   * micro-architectures in a system.
>   */
>  .align   11
> -ENTRY(__bp_harden_hyp_vecs_start)
> +FUNC(__bp_harden_hyp_vecs_start)
>  .rept 4
>  vectors hyp_traps_vector
>  .endr
> -ENTRY(__bp_harden_hyp_vecs_end)
> +GLOBAL(__bp_harden_hyp_vecs_end)
> +END(__bp_harden_hyp_vecs_start)

Shouldn't GLOBAL be changed to FUNC as well?


>  .macro mitigate_spectre_bhb_loop count
> -ENTRY(__mitigate_spectre_bhb_loop_start_\count)
> +FUNC(__mitigate_spectre_bhb_loop_start_\count)
>  stp x0, x1, [sp, #-16]!
>  mov x0, \count
>  .Lspectre_bhb_loop\@:
> @@ -68,11 +69,12 @@ ENTRY(__mitigate_spectre_bhb_loop_start_\count)
>  b.ne.Lspectre_bhb_loop\@
>  sb
>  ldp x0, x1, [sp], #16
> -ENTRY(__mitigate_spectre_bhb_loop_end_\count)
> +GLOBAL(__mitigate_spectre_bhb_loop_end_\count)

Also here?


> +END(__mitigate_spectre_bhb_loop_start_\count)
>  .endm
>  
>  .macro smccc_workaround num smcc_id
> -ENTRY(__smccc_workaround_smc_start_\num)
> +FUNC(__smccc_workaround_smc_start_\num)
>  sub sp, sp, #(8 * 4)
>  stp x0, x1, [sp, #(8 * 2)]
>  stp x2, x3, [sp, #(8 * 0)]
> @@ -81,13 +83,15 @@ ENTRY(__smccc_workaround_smc_start_\num)
>  ldp x2, x3, [sp, #(8 * 0)]
>  ldp x0, x1, [sp, #(8 * 2)]
>  add sp, sp, #(8 * 4)
> -ENTRY(__smccc_workaround_smc_end_\num)
> +GLOBAL(__smccc_workaround_smc_end_\num)

And here?


> +END(__smccc_workaround_smc_start_\num)
>  .endm
>  
> -ENTRY(__mitigate_spectre_bhb_clear_insn_start)
> +FUNC(__mitigate_spectre_bhb_clear_insn_start)
>  clearbhb
>  isb
> -ENTRY(__mitigate_spectre_bhb_clear_insn_end)
> +GLOBAL(__mitigate_spectre_bhb_clear_insn_end)

and here?


> +END(__mitigate_spectre_bhb_clear_insn_start)
>  
>  mitigate_spectre_bhb_loop 8
>  mitigate_spectre_bhb_loop 24
> -- 
> 2.40.1
> 

Re: [PATCH v3 5/9] xen/arm64: debug: Add missing code symbol annotations

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols
> and add missing annotations.
> 
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Stefano Stabellini 


> ---
>  xen/arch/arm/arm64/debug.S | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
> index 71cad9d762..c3d02c33d7 100644
> --- a/xen/arch/arm/arm64/debug.S
> +++ b/xen/arch/arm/arm64/debug.S
> @@ -27,17 +27,19 @@
>   * Print a character on the UART - this function is called by C
>   * x0: character to print
>   */
> -GLOBAL(early_putch)
> +FUNC(early_putch)
>  ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS
>  early_uart_ready x15, 1
>  early_uart_transmit x15, w0
>  ret
> +END(early_putch)
>  
>  /* Flush the UART - this function is called by C */
> -GLOBAL(early_flush)
> +FUNC(early_flush)
>  ldr   x15, =EARLY_UART_VIRTUAL_ADDRESS  /* x15 := VA UART base 
> address */
>  early_uart_ready x15, 1
>  ret
> +END(early_flush)
>  
>  /*
>   * Local variables:
> -- 
> 2.40.1
> 

Re: [PATCH v3 4/9] xen/arm64: head: Add missing code symbol annotations

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols
> and add missing annotations.
> 
> Signed-off-by: Edgar E. Iglesias 
> ---
>  xen/arch/arm/arm64/head.S | 50 ---
>  1 file changed, 26 insertions(+), 24 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index fb297e9eb5..7acedb4f8f 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -90,7 +90,7 @@
>   * 4K-aligned address.
>   */
>  
> -GLOBAL(start)
> +FUNC(start)
>  /*
>   * DO NOT MODIFY. Image header expected by Linux boot-loaders.
>   */
> @@ -102,6 +102,7 @@ efi_head:
>   */
>  add x13, x18, #0x16
>  b   real_start   /* branch to kernel start */
> +END(start)
>  .quad   0/* Image load offset from start of RAM 
> */
>  .quad   _end - start /* Effective size of kernel image, 
> little-endian */
>  .quad   __HEAD_FLAGS /* Informative flags, little-endian */
> @@ -223,7 +224,7 @@ section_table:
>  .align  5
>  #endif /* CONFIG_ARM_EFI */
>  
> -real_start:
> +FUNC_LOCAL(real_start)
>  /* BSS should be zeroed when booting without EFI */
>  mov   x26, #0/* x26 := skip_zero_bss */
>  
> @@ -263,9 +264,9 @@ primary_switched:
>  mov   x1, x21/* x1 := paddr(FDT) */
>  ldr   x2, =start_xen
>  b launch
> -ENDPROC(real_start)
> +END(real_start)
>  
> -GLOBAL(init_secondary)
> +FUNC(init_secondary)
>  msr   DAIFSet, 0xf   /* Disable all interrupts */
>  
>  /* Find out where we are */
> @@ -304,7 +305,7 @@ secondary_switched:
>  /* Jump to C world */
>  ldr   x2, =start_secondary
>  b launch
> -ENDPROC(init_secondary)
> +END(init_secondary)
>  
>  /*
>   * Check if the CPU has been booted in Hypervisor mode.
> @@ -313,7 +314,7 @@ ENDPROC(init_secondary)
>   *
>   * Clobbers x0 - x5
>   */
> -check_cpu_mode:
> +FUNC_LOCAL(check_cpu_mode)
>  PRINT_ID("- Current EL ")
>  mrs   x5, CurrentEL
>  print_reg x5
> @@ -329,7 +330,7 @@ check_cpu_mode:
>  PRINT_ID("- Xen must be entered in NS EL2 mode -\r\n")
>  PRINT_ID("- Please update the bootloader -\r\n")
>  b fail
> -ENDPROC(check_cpu_mode)
> +END(check_cpu_mode)
>  
>  /*
>   * Zero BSS
> @@ -339,7 +340,7 @@ ENDPROC(check_cpu_mode)
>   *
>   * Clobbers x0 - x3
>   */
> -zero_bss:
> +FUNC_LOCAL(zero_bss)
>  /* Zero BSS only when requested */
>  cbnz  x26, skip_bss
>  
> @@ -353,14 +354,14 @@ zero_bss:
>  
>  skip_bss:
>  ret
> -ENDPROC(zero_bss)
> +END(zero_bss)
>  
>  /*
>   * Initialize the processor for turning the MMU on.
>   *
>   * Clobbers x0 - x3
>   */
> -cpu_init:
> +FUNC_LOCAL(cpu_init)
>  PRINT_ID("- Initialize CPU -\r\n")
>  
>  /* Set up memory attribute type tables */
> @@ -399,7 +400,7 @@ cpu_init:
>   */
>  msr spsel, #1
>  ret
> -ENDPROC(cpu_init)
> +END(cpu_init)
>  
>  /*
>   * Setup the initial stack and jump to the C world
> @@ -411,7 +412,7 @@ ENDPROC(cpu_init)
>   *
>   * Clobbers x3
>   */
> -launch:
> +FUNC_LOCAL(launch)
>  ldr   x3, =init_data
>  add   x3, x3, #INITINFO_stack /* Find the boot-time stack */
>  ldr   x3, [x3]
> @@ -421,13 +422,13 @@ launch:
>  
>  /* Jump to C world */
>  brx2
> -ENDPROC(launch)
> +END(launch)
>  
>  /* Fail-stop */
> -fail:   PRINT_ID("- Boot failed -\r\n")
> +FUNC_LOCAL(fail)   PRINT_ID("- Boot failed -\r\n")

Maybe we should move PRINT_ID to a newline?
I am not sure FUNC_LOCAL supports having a command on the same line.



>  1:  wfe
>  b 1b
> -ENDPROC(fail)
> +END(fail)
>  
>  #ifdef CONFIG_EARLY_PRINTK
>  /*
> @@ -438,14 +439,14 @@ ENDPROC(fail)
>   *
>   * Clobbers x0 - x1
>   */
> -init_uart:
> +FUNC_LOCAL(init_uart)
>  ldr   x23, =CONFIG_EARLY_UART_BASE_ADDRESS
>  #ifdef CONFIG_EARLY_UART_INIT
>  early_uart_init x23, 0
>  #endif
>  PRINT("- UART enabled -\r\n")
>  ret
> -ENDPROC(init_uart)
> +END(init_uart)
>  
>  /*
>   * Print early debug messages.
> @@ -454,7 +455,7 @@ ENDPROC(init_uart)
>   * x23: Early UART base address
>   * Clobbers x0-x1
>   */
> -ENTRY(asm_puts)
> +FUNC(asm_puts)
>  early_uart_ready x23, 1
>  ldrb  w1, [x0], #1   /* Load next char */
>  cbz   w1, 1f /* Exit on nul */
> @@ -462,7 +463,7 @@ ENTRY(asm_puts)
>  b asm_puts
>  1:
>  ret
> -ENDPROC(asm_puts)
> +END(asm_puts)
>  
>  /*
>   * Print a 64-bit number in hex.
> @@ -471,7 +472,7 @@ ENDPROC(asm_puts)
>   * x23: Early UART base address
>   * Clobbers x0-x3
>   */
> -ENTRY(asm_putn)
> +FUNC(asm_putn)
>  adr_l x1, hex
>  mov   x3, #16
>  1:
> @@ -484,7 

Re: [PATCH v3 2/9] xen/arm64: smc: Add missing code symbol annotations

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols
> and add missing annotations.
> 
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Stefano Stabellini 


> ---
>  xen/arch/arm/arm64/smc.S | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
> index fc6b676e2e..68b05e8ddd 100644
> --- a/xen/arch/arm/arm64/smc.S
> +++ b/xen/arch/arm/arm64/smc.S
> @@ -19,7 +19,7 @@
>   *  register_t a6, register_t a7,
>   *  struct arm_smccc_res *res)
>   */
> -ENTRY(__arm_smccc_1_0_smc)
> +FUNC(__arm_smccc_1_0_smc)
>  smc #0
>  ldr x4, [sp]
>  cbz x4, 1f  /* No need to store the result */
> @@ -27,12 +27,13 @@ ENTRY(__arm_smccc_1_0_smc)
>  stp x2, x3, [x4, #SMCCC_RES_a2]
>  1:
>  ret
> +END(__arm_smccc_1_0_smc)
>  
>  /*
>   * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
>   *struct arm_smccc_1_2_regs *res)
>   */
> -ENTRY(arm_smccc_1_2_smc)
> +FUNC(arm_smccc_1_2_smc)
>  /* Save `res` and free a GPR that won't be clobbered by SMC call */
>  stp x1, x19, [sp, #-16]!
>  
> @@ -69,3 +70,4 @@ ENTRY(arm_smccc_1_2_smc)
>  /* Restore original x19 */
>  ldp xzr, x19, [sp], #16
>  ret
> +END(arm_smccc_1_2_smc)
> -- 
> 2.40.1
> 

Re: [PATCH v3 3/9] xen/arm64: sve: Add missing code symbol annotations

[Stefano Stabellini]

On Wed, 1 May 2024, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Use the generic xen/linkage.h macros to annotate code symbols
> and add missing annotations.
> 
> Signed-off-by: Edgar E. Iglesias 

Reviewed-by: Stefano Stabellini 


> ---
>  xen/arch/arm/arm64/sve-asm.S | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/sve-asm.S b/xen/arch/arm/arm64/sve-asm.S
> index 59dbefbbb2..2d8b895f07 100644
> --- a/xen/arch/arm/arm64/sve-asm.S
> +++ b/xen/arch/arm/arm64/sve-asm.S
> @@ -161,9 +161,10 @@
>  .endm
>  
>  /* Gets the current vector register size in bytes */
> -GLOBAL(sve_get_hw_vl)
> +FUNC(sve_get_hw_vl)
>  _sve_rdvl 0, 1
>  ret
> +END(sve_get_hw_vl)
>  
>  /*
>   * Save the SVE context
> @@ -172,9 +173,10 @@ GLOBAL(sve_get_hw_vl)
>   * x1 - pointer to buffer for P0-15
>   * x2 - Save FFR if non-zero
>   */
> -GLOBAL(sve_save_ctx)
> +FUNC(sve_save_ctx)
>  sve_save 0, 1, x2
>  ret
> +END(sve_save_ctx)
>  
>  /*
>   * Load the SVE context
> @@ -183,9 +185,10 @@ GLOBAL(sve_save_ctx)
>   * x1 - pointer to buffer for P0-15
>   * x2 - Restore FFR if non-zero
>   */
> -GLOBAL(sve_load_ctx)
> +FUNC(sve_load_ctx)
>  sve_load 0, 1, x2
>  ret
> +END(sve_load_ctx)
>  
>  /*
>   * Local variables:
> -- 
> 2.40.1
> 

Re: [XEN PATCH 2/2] automation/eclair: add deviation for Rule 16.4

[Stefano Stabellini]

On Fri, 3 May 2024, Federico Serafini wrote:
> MISRA C:2012 Rule 16.4 states that "Every switch statement shall have a
> default label".
> Update ECLAIR configuration to take into account the deviations
> agreed during MISRA meetings.
> 
> Signed-off-by: Federico Serafini 

Reviewed-by: Stefano Stabellini 


> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl |  8 
>  docs/misra/deviations.rst| 13 +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index d21f112a9b..f09ad71acf 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -384,6 +384,14 @@ explicit comment indicating the fallthrough intention is 
> present."
>  -config=MC3R1.R16.3,reports+={safe, 
> "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? 
> \\*/.*$,0..1"}
>  -doc_end
>  
> +-doc_begin="Switch statements having a controlling expression of enum type 
> deliberately do not have a default case: gcc -Wall enables -Wswitch which 
> warns (and breaks the build as we use -Werror) if one of the enum labels is 
> missing from the switch."
> +-config=MC3R1.R16.4,reports+={deliberate,'any_area(kind(context)&&^.* has no 
> `default.*$&(node(switch_stmt)&(cond,skip(__non_syntactic_paren_stmts,type(canonical(enum_underlying_type(any('}
> +-doc_end
> +
> +-doc_begin="A switch statement with a single switch clause and no default 
> label may be used in place of an equivalent if statement if it is considered 
> to improve readability."
> +-config=MC3R1.R16.4,switch_clauses+={deliberate,"switch(1)&(0)"}
> +-doc_end
> +
>  -doc_begin="A switch statement with a single switch clause and no default 
> label may be used in place of an equivalent if statement if it is considered 
> to improve readability."
>  -config=MC3R1.R16.6,switch_clauses+={deliberate, "default(0)"}
>  -doc_end
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index ed0c1e8ed0..39cc321a27 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -334,6 +334,19 @@ Deviations related to MISRA C:2012 Rules:
>   - /\* Fallthrough \*/
>   - /\* Fallthrough. \*/
>  
> +   * - R16.4
> + - Switch statements having a controlling expression of enum type
> +   deliberately do not have a default case: gcc -Wall enables -Wswitch
> +   which warns (and breaks the build as we use -Werror) if one of the 
> enum
> +   labels is missing from the switch.
> + - Tagged as `deliberate` for ECLAIR.
> +
> +   * - R16.4
> + - A switch statement with a single switch clause and no default label 
> may
> +   be used in place of an equivalent if statement if it is considered to
> +   improve readability.
> + - Tagged as `deliberate` for ECLAIR.
> +
> * - R16.6
>   - A switch statement with a single switch clause and no default label 
> may
> be used in place of an equivalent if statement if it is considered to
> -- 
> 2.34.1
> 

Re: [XEN PATCH 1/2] docs/misra: add Terms & Definitions section to rules.rst

[Stefano Stabellini]

On Fri, 3 May 2024, Federico Serafini wrote:
> Add a section for terms and definitions used by MISRA but expressed
> in terms of the C specification.
> 
> Add a definition of "switch clause" to the newly-introduced section.
> 
> Link the first use of the term "switch clause" in the document to its
> definition.
> 
> Suggested-by: Jan Beulich 
> Signed-off-by: Federico Serafini 

Reviewed-by: Stefano Stabellini 


> ---
> Jan you were not completely satisfied by the definition but I didn't find
> a better one.
> ---
>  docs/misra/rules.rst | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
> index b7b447e152..d3b70fdf04 100644
> --- a/docs/misra/rules.rst
> +++ b/docs/misra/rules.rst
> @@ -489,8 +489,7 @@ maintainers if you want to suggest a change.
>  
> * - `Rule 16.3 
> `_
>   - Required
> - - An unconditional break statement shall terminate every
> -   switch-clause
> + - An unconditional break statement shall terminate every switch-clause_
>   - In addition to break, also other unconditional flow control statements
> such as continue, return, goto are allowed.
>  
> @@ -712,3 +711,14 @@ maintainers if you want to suggest a change.
>   - The value of a pointer to a FILE shall not be used after the 
> associated
> stream has been closed
>   -
> +
> +Terms & Definitions
> +---
> +
> +.. _switch-clause:
> +
> +A *switch clause* can be defined as:
> +"the non-empty list of statements which follows a non-empty list of
> +case/default labels".
> +A formal definition is available within the amplification of MISRA C:2012
> +Rule 16.1.
> -- 
> 2.34.1
> 

Re: [XEN PATCH] automation/eclair: hide reports coming from adopted code in scheduled analysis

[Stefano Stabellini]

On Fri, 3 May 2024, Federico Serafini wrote:
> To improve clarity and ease of navigation do not show reports related
> to adopted code in the scheduled analysis.
> Configuration options are commented out because they may be useful
> in the future.
> 
> Signed-off-by: Simone Ballarin 
> Signed-off-by: Federico Serafini 

Acked-by: Stefano Stabellini 


> ---
>  .../eclair_analysis/ECLAIR/analysis.ecl   | 24 +++
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/analysis.ecl 
> b/automation/eclair_analysis/ECLAIR/analysis.ecl
> index 66ed7f952c..67be38f03c 100644
> --- a/automation/eclair_analysis/ECLAIR/analysis.ecl
> +++ b/automation/eclair_analysis/ECLAIR/analysis.ecl
> @@ -4,11 +4,11 @@
>  
>  setq(data_dir,getenv("ECLAIR_DATA_DIR"))
>  setq(analysis_kind,getenv("ANALYSIS_KIND"))
> -setq(scheduled_analysis,nil)
> +# setq(scheduled_analysis,nil)
>  
> -strings_map("scheduled-analysis",500,"","^.*scheduled$",0,setq(scheduled_analysis,t))
> -strings_map("scheduled-analysis",500,"","^.*$",0)
> -map_strings("scheduled-analysis",analysis_kind)
> +# 
> strings_map("scheduled-analysis",500,"","^.*scheduled$",0,setq(scheduled_analysis,t))
> +# strings_map("scheduled-analysis",500,"","^.*$",0)
> +# map_strings("scheduled-analysis",analysis_kind)
>  
>  -verbose
>  
> @@ -25,12 +25,16 @@ map_strings("scheduled-analysis",analysis_kind)
>  -doc="Initially, there are no files tagged as adopted."
>  -file_tag+={adopted,"none()"}
>  
> -if(not(scheduled_analysis),
> -eval_file("adopted.ecl")
> -)
> -if(not(scheduled_analysis),
> -eval_file("out_of_scope.ecl")
> -)
> +# if(not(scheduled_analysis),
> +# eval_file("adopted.ecl")
> +# )
> +# if(not(scheduled_analysis),
> +# eval_file("out_of_scope.ecl")
> +# )
> +
> +-eval_file=adopted.ecl
> +-eval_file=out_of_scope.ecl
> +
>  -eval_file=deviations.ecl
>  -eval_file=call_properties.ecl
>  -eval_file=tagging.ecl
> -- 
> 2.34.1
> 

Re: [GIT PULL] xen: branch for v6.9-rc7

[pr-tracker-bot]

The pull request you sent on Fri,  3 May 2024 14:20:28 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
> for-linus-6.9a-rc7-tag

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ddb4c3f25b7b95df3d6932db0b379d768a6ebdf7

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-qemuu-freebsd12-amd64 21 guest-start/freebsd.repeat fail pass 
in 185913-retest

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

version targeted for testing:
 xen  543bbddf79f105682904c3b5a09c5a1692ae4b15
baseline version:
 xen  05653eb44314cb90f2e3e7b2d405e86b5657

Last test of basis   185877  2024-04-30 10:01:57 Z3 days
Testing same since   185909  2024-05-03 09:40:06 Z0 days1 attempts


People who touched revisions under test:
  Julien Grall 

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

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

[osstest service owner]

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

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  f95cd010cbf0914154a0c2775c979d9158b1a3cb
baseline version:
 xen  feb9158a620040846d76981acbe8ea9e2255a07b

Last test of basis   185895  2024-05-01 22:02:19 Z1 days
Testing same since   185912  2024-05-03 17:02:06 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Juergen Gross 

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
   feb9158a62..f95cd010cb  f95cd010cbf0914154a0c2775c979d9158b1a3cb -> smoke

Re: [REGRESSION] Re: [XEN PATCH 0/3] automation/eclair: do not allow failure for triggered analyses

[Andrew Cooper]

On 03/05/2024 9:01 pm, Federico Serafini wrote:
> On 03/05/24 21:46, Andrew Cooper wrote:
>> On 03/05/2024 8:44 pm, Federico Serafini wrote:
>>> On 03/05/24 21:14, Andrew Cooper wrote:
 On 29/04/2024 4:21 pm, Federico Serafini wrote:
> Patch 1/3 does some preparation work.
>
> Patch 2/3, as the title says, removes allow_failure = true for
> triggered
> analyses.
>
> Patch 3/3 makes explicit that initally no files are tagged as
> adopted, this
> is needed by the scheduled analysis.

 I'm afraid that something in this series is broken.

 Since these patches went in, all pipelines are now getting a status of
 blocked rather than passed.

 If I manually start the Eclair jobs, then eventually the pipeline gets
 to Passed.
>>>
>>> Can you provide us a link to those failures?
>>> I am looking at gitlab xen-project/xen and xen-project/patchew
>>> and everything seems ok.
>>>
>>
>> https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1276081658
>> is the first one I noticed as blocked, and I manually ran.  That ended
>> up working.
>>
>> https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1277724847
>> is still in the blocked state.  The build-each-commit failure is
>> unrelated.
>
> This is intentional and was introduced by
> commit 7c1bf8661db5c00bd8c9a25015fe8678b2ff9ac6
>
> The ECLAIR analysis under people/* need to be activated
> manually.

Yes.  I know, and that matches the behaviour I saw.

>
> Is this causing some problems to the CI?
>

Yes.

See https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines

Prior to this series, the manual actions were not used but the pipeline
was overall in the Passed state.  Specifically, they ended up being skipped.

After this series, the manual actions are now blocking the pipeline, not
letting it complete, and not marking it as passed.

~Andrew

Re: [REGRESSION] Re: [XEN PATCH 0/3] automation/eclair: do not allow failure for triggered analyses

[Federico Serafini]

On 03/05/24 21:46, Andrew Cooper wrote:

On 03/05/2024 8:44 pm, Federico Serafini wrote:

On 03/05/24 21:14, Andrew Cooper wrote:

On 29/04/2024 4:21 pm, Federico Serafini wrote:

Patch 1/3 does some preparation work.

Patch 2/3, as the title says, removes allow_failure = true for
triggered
analyses.

Patch 3/3 makes explicit that initally no files are tagged as
adopted, this
is needed by the scheduled analysis.


I'm afraid that something in this series is broken.

Since these patches went in, all pipelines are now getting a status of
blocked rather than passed.

If I manually start the Eclair jobs, then eventually the pipeline gets
to Passed.


Can you provide us a link to those failures?
I am looking at gitlab xen-project/xen and xen-project/patchew
and everything seems ok.



https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1276081658
is the first one I noticed as blocked, and I manually ran.  That ended
up working.

https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1277724847
is still in the blocked state.  The build-each-commit failure is unrelated.


This is intentional and was introduced by
commit 7c1bf8661db5c00bd8c9a25015fe8678b2ff9ac6

The ECLAIR analysis under people/* need to be activated
manually.

Is this causing some problems to the CI?

--
Federico Serafini, M.Sc.

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

Re: [REGRESSION] Re: [XEN PATCH 0/3] automation/eclair: do not allow failure for triggered analyses

[Andrew Cooper]

On 03/05/2024 8:44 pm, Federico Serafini wrote:
> On 03/05/24 21:14, Andrew Cooper wrote:
>> On 29/04/2024 4:21 pm, Federico Serafini wrote:
>>> Patch 1/3 does some preparation work.
>>>
>>> Patch 2/3, as the title says, removes allow_failure = true for
>>> triggered
>>> analyses.
>>>
>>> Patch 3/3 makes explicit that initally no files are tagged as
>>> adopted, this
>>> is needed by the scheduled analysis.
>>
>> I'm afraid that something in this series is broken.
>>
>> Since these patches went in, all pipelines are now getting a status of
>> blocked rather than passed.
>>
>> If I manually start the Eclair jobs, then eventually the pipeline gets
>> to Passed.
>
> Can you provide us a link to those failures?
> I am looking at gitlab xen-project/xen and xen-project/patchew
> and everything seems ok.
>

https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1276081658
is the first one I noticed as blocked, and I manually ran.  That ended
up working.

https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/1277724847
is still in the blocked state.  The build-each-commit failure is unrelated.

~Andrew

Re: [REGRESSION] Re: [XEN PATCH 0/3] automation/eclair: do not allow failure for triggered analyses

[Federico Serafini]

On 03/05/24 21:14, Andrew Cooper wrote:

On 29/04/2024 4:21 pm, Federico Serafini wrote:

Patch 1/3 does some preparation work.

Patch 2/3, as the title says, removes allow_failure = true for triggered
analyses.

Patch 3/3 makes explicit that initally no files are tagged as adopted, this
is needed by the scheduled analysis.


I'm afraid that something in this series is broken.

Since these patches went in, all pipelines are now getting a status of
blocked rather than passed.

If I manually start the Eclair jobs, then eventually the pipeline gets
to Passed.


Can you provide us a link to those failures?
I am looking at gitlab xen-project/xen and xen-project/patchew
and everything seems ok.

--
Federico Serafini, M.Sc.

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

[REGRESSION] Re: [XEN PATCH 0/3] automation/eclair: do not allow failure for triggered analyses

[Andrew Cooper]

On 29/04/2024 4:21 pm, Federico Serafini wrote:
> Patch 1/3 does some preparation work.
>
> Patch 2/3, as the title says, removes allow_failure = true for triggered
> analyses.
>
> Patch 3/3 makes explicit that initally no files are tagged as adopted, this
> is needed by the scheduled analysis.

I'm afraid that something in this series is broken.

Since these patches went in, all pipelines are now getting a status of
blocked rather than passed.

If I manually start the Eclair jobs, then eventually the pipeline gets
to Passed.

~Andrew

Re: [PATCH v6 8/8] xen: allow up to 16383 cpus

[Stefano Stabellini]

On Fri, 3 May 2024, Julien Grall wrote:
> Hi Stefano,
> 
> On 02/05/2024 19:13, Stefano Stabellini wrote:
> > On Mon, 29 Apr 2024, Julien Grall wrote:
> > > Hi Juergen,
> > > 
> > > On 29/04/2024 12:28, Jürgen Groß wrote:
> > > > On 29.04.24 13:04, Julien Grall wrote:
> > > > > Hi Juergen,
> > > > > 
> > > > > Sorry for the late reply.
> > > > > 
> > > > > On 29/04/2024 11:33, Juergen Gross wrote:
> > > > > > On 08.04.24 09:10, Jan Beulich wrote:
> > > > > > > On 27.03.2024 16:22, Juergen Gross wrote:
> > > > > > > > With lock handling now allowing up to 16384 cpus (spinlocks can
> > > > > > > > handle
> > > > > > > > 65535 cpus, rwlocks can handle 16384 cpus), raise the allowed
> > > > > > > > limit
> > > > > > > > for
> > > > > > > > the number of cpus to be configured to 16383.
> > > > > > > > 
> > > > > > > > The new limit is imposed by IOMMU_CMD_BUFFER_MAX_ENTRIES and
> > > > > > > > QINVAL_MAX_ENTRY_NR required to be larger than 2 *
> > > > > > > > CONFIG_NR_CPUS.
> > > > > > > > 
> > > > > > > > Signed-off-by: Juergen Gross 
> > > > > > > 
> > > > > > > Acked-by: Jan Beulich 
> > > > > > > 
> > > > > > > I'd prefer this to also gain an Arm ack, though.
> > > > > > 
> > > > > > Any comment from Arm side?
> > > > > 
> > > > > Can you clarify what the new limits mean in term of (security)
> > > > > support?
> > > > > Are we now claiming that Xen will work perfectly fine on platforms
> > > > > with up
> > > > > to 16383?
> > > > > 
> > > > > If so, I can't comment for x86, but for Arm, I am doubtful that it
> > > > > would
> > > > > work without any (at least performance) issues. AFAIK, this is also an
> > > > > untested configuration. In fact I would be surprised if Xen on Arm was
> > > > > tested with more than a couple of hundreds cores (AFAICT the Ampere
> > > > > CPUs
> > > > > has 192 CPUs).
> > > > 
> > > > I think we should add a security support limit for the number of
> > > > physical
> > > > cpus similar to the memory support limit we already have in place.
> > > > 
> > > > For x86 I'd suggest 4096 cpus for security support (basically the limit
> > > > we
> > > > have with this patch), but I'm open for other suggestions, too.
> > > > 
> > > > I have no idea about any sensible limits for Arm32/Arm64.
> > > 
> > > I am not entirely. Bertrand, Michal, Stefano, should we use 192 (the
> > > number of
> > > CPUs from Ampere)?
> > 
> > I am OK with that. If we want to be a bit more future proof we could say
> > 256 or 512.
> 
> Sorry, I don't follow your argument. A limit can be raised at time point in
> the future. The question is more whether we are confident that Xen on Arm will
> run well if a user has a platform with 256/512 pCPUs.
> 
> So are you saying that from Xen point of view, you are expecting no difference
> between 256 and 512. And therefore you would be happy if to backport patches
> if someone find differences (or even security issues) when using > 256 pCPUs?

It is difficult to be sure about anything that it is not regularly
tested. I am pretty sure someone in the community got Xen running on an
Ampere, so like you said 192 is a good number. However, that is not
regularly tested, so we don't have any regression checks in gitlab-ci or
OSSTest for it.

One approach would be to only support things regularly tested either by
OSSTest, Gitlab-ci, or also Xen community members. I am not sure what
would be the highest number with this way of thinking but likely no
more than 192, probably less. I don't know the CPU core count of the
biggest ARM machine in OSSTest.

Another approach is to support a "sensible" number: not something tested
but something we believe it should work. No regular testing. (In safety,
they only believe in things that are actually tested, so this would not
be OK. But this is security, not safety, just FYI.) With this approach,
we could round up the number to a limit we think it won't break. If 192
works, 256/512 should work? I don't know but couldn't think of something
that would break going from 192 to 256.

It depends on how strict we want to be on testing requirements. I am not
sure what approach was taken by x86 so far. I am OK either way.

Re: [PATCH v8 02/17] xen: introduce generic non-atomic test_*bit()

[Oleksii]

On Thu, 2024-04-25 at 17:35 +0200, Jan Beulich wrote:
> >   #include 
> >   
> > +#ifndef arch_check_bitop_size
> > +#define arch_check_bitop_size(addr)
> 
> Can this really do nothing? Passing the address of an object smaller
> than
> bitop_uint_t will read past the object in the generic__*_bit()
> functions.
It seems RISC-V isn' happy with the following generic definition:
   extern void __bitop_bad_size(void);
   
   /* - Please tidy above here 
   - */
   
   #include 
   
   #ifndef arch_check_bitop_size
   
   #define bitop_bad_size(addr) sizeof(*(addr)) < sizeof(bitop_uint_t)
   
   #define arch_check_bitop_size(addr) \
   if ( bitop_bad_size(addr) ) __bitop_bad_size();
   
   #endif /* arch_check_bitop_size */

The following errors occurs. bitop_uint_t for RISC-V is defined as
unsigned long for now:
./common/symbols-dummy.o -o ./.xen-syms.0
riscv64-linux-gnu-ld: prelink.o: in function `atomic_sub':
/build/xen/./arch/riscv/include/asm/atomic.h:152: undefined reference
to `__bitop_bad_size'
riscv64-linux-gnu-ld: prelink.o: in function `evtchn_check_pollers':
/build/xen/common/event_channel.c:1531: undefined reference to
`__bitop_bad_size'
riscv64-linux-gnu-ld: /build/xen/common/event_channel.c:1521: undefined
reference to `__bitop_bad_size'
riscv64-linux-gnu-ld: prelink.o: in function `evtchn_init':
/build/xen/common/event_channel.c:1541: undefined reference to
`__bitop_bad_size'
riscv64-linux-gnu-ld: prelink.o: in function `_read_lock':
/build/xen/./include/xen/rwlock.h:94: undefined reference to
`__bitop_bad_size'
riscv64-linux-gnu-ld:
prelink.o:/build/xen/./arch/riscv/include/asm/atomic.h:195: more
undefined references to `__bitop_bad_size' follow
riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol `__bitop_bad_size'
isn't defined
riscv64-linux-gnu-ld: final link failed: bad value
make[2]: *** [arch/riscv/Makefile:15: xen-syms] Error 1

~ Oleksii

Xen 4.19 release status tracking list [ May ]

[Oleksii]

Hello everyone,

I would like to share with you a list for status tracking based on Xen
ML and community members comments:

*** Arm ***:
  * Arm cache coloring [
https://lore.kernel.org/xen-devel/20240502165533.319988-1-carlo.non...@minervasys.tech/
]:
- new patch series version [v8] was sent

  * PCI devices passthrough on Arm, part 3 [
https://lore.kernel.org/xen-devel/20240202213321.1920347-1-stewart.hildebr...@amd.com/
]
 
  * DOMCTL-based guest magic region allocation for 11
domUs [
https://lore.kernel.org/xen-devel/20240409045357.236802-1-xin.wa...@amd.com/
]
 - new patch series verstion [v4] was sent
 
  * [XEN v6 0/3] xen/arm: Add emulation of Debug Data Transfer
Registers [
https://patchew.org/Xen/20240307123943.1991755-1-ayan.kumar.hal...@amd.com/
]

*** PPC ***:
  * [PATCH v4 0/6] Early Boot Allocation on Power [
https://lore.kernel.org/xen-devel/cover.1712893887.git.sanasta...@raptorengineering.com/
]:
new patch series version [v4] was sent

*** RISC-V ***:
  * [PATCH v8 00/17] Enable build of full Xen for RISC-V
https://lore.kernel.org/xen-devel/cover.1713347222.git.oleksii.kuroc...@gmail.com/
]:
- several patches were merged
- new patch series version [v8] were sent


*** x86 ***:
  * [PATCH 0/4] iommu/x86: fixes/improvements for unity range checks [
https://lore.kernel.org/xen-devel/20240201170159.66330-1-roger@citrix.com/
]:
- almost patch series have been merged already except the patch:
[PATCH 4/4] iommu/x86: make unity range checking more strict

  * [PATCH 0/8] x86: support AVX10.1 [
https://lore.kernel.org/xen-devel/298db76f-d0ee-4d47-931f-1baa1a754...@suse.com/
]:
- two patches of patch series are waitng to merged/reviewed:
  [PATCH 1/4] amd-vi: fix IVMD memory type checks
  [PATCH 4/4] iommu/x86: make unity range checking more strict 
  
  * APX support?

  * [PATCH v4 0/8] x86emul: misc additions [
https://lore.kernel.org/xen-devel/9dd23064-c79e-4a50-9c71-c0e73b189...@suse.com/
]
   
  * [PATCH v2 00/12] VT-d: SATC handling; ATS: tidying[
https://lore.kernel.org/xen-devel/64b028be-2197-4951-ae5b-32f9eabfa...@suse.com/
]:
new version was sent [ v2 ]

  * [XEN PATCH 0/9] x86: parallelize AP bring-up during boot [
https://lore.kernel.org/xen-devel/cover.1699982111.git.krystian.he...@3mdeb.com/
]

  * [PATCH v2 00/12] x86: memcpy() / memset() (non-)ERMS flavors plus
fallout [
https://lore.kernel.org/xen-devel/8f56a8f4-0482-932f-96a9-c791bebb4...@suse.com/
]
- 6/12 are merged.
 
  * [PATCH v6 0/4] x86/pvh: Support relocating dom0 kernel [
https://patchew.org/Xen/20240327215102.136001-1-jason.andr...@amd.com/
]
 
  * x86/spec-ctrl: IBPB improvements [
https://patchew.org/Xen/06591b64-2f05-a4cc-a2f3-a74c3c4a7...@suse.com/
]


*** common ***:
  * annotate entry points with type and size" series:
The bulk of this has gone in, but there'll want to be follow-ups.

  * [PATCH v2 (resend) 00/27] Remove the directmap [
https://lore.kernel.org/xen-devel/20240116192611.41112-1-elias...@amazon.com/
]
- 7/27 were merged.
 
  * [PATCH] move __read_mostly to xen/cache.h [
https://lore.kernel.org/xen-devel/f25eb5c9-7c14-6e23-8535-2c66772b3...@suse.com/
]

  * [XEN PATCH v2 1/3] xen: introduce STATIC_ASSERT_UNREACHABLE() [
https://lore.kernel.org/xen-devel/42fc6ae8d3eb802429d29c774502ff232340dc84.1706259490.git.federico.seraf...@bugseng.com/
]

  * MISRA rules updates:
   - [PATCH v2] docs/misra/rules.rst update [
https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2402131431070.1925432@ubuntu-linux-20-04-desktop/T/#maded3df1bebe68d0fe53c73e89f996ec395a39e5
]: 2/3 were merged.

   - [XEN PATCH v3 0/7] address violations of MISRA C Rule 20.7[
https://patchew.org/Xen/cover.1711700095.git.nicola.vetr...@bugseng.com/
]: new patch series version (v3) were sent.

   - [XEN PATCH v3 00/16] xen: address violation of MISRA C:2012
Directive 4.10 [
https://patchew.org/Xen/cover.1710145041.git.simone.balla...@bugseng.com/
]: 2/16 were merged.

  * [PATCH v6 00/8] xen/spinlock: make recursive spinlocks a dedicated
type [ https://patchew.org/Xen/20240327152229.25847-1-jgr...@suse.com/
]:
- only one patch required to be merged:
 
https://patchew.org/Xen/20240327152229.25847-1-jgr...@suse.com/20240327152229.25847-9-jgr...@suse.com/
   
  * [PATCH 0/7] GRUB: Supporting Secure Boot of xen.gz [
https://patchew.org/Xen/20240313150748.791236-1-ross.lagerw...@citrix.com/
]:
   
  * [PATCH v6 0/7] MSI-X support with qemu in stubdomain, and other
related changes:
- new patch version was sent
 
  * [PATCH 0/7] xen/bitops: Reduce the mess, starting with ffs() [
https://patchew.org/Xen/20240313172716.2325427-1-andrew.coop...@citrix.com/
]:
 
  * [PATCH 0/7] xen/trace: Treewide API cleanup [
https://patchew.org/Xen/20240318163552.3808695-1-andrew.coop...@citrix.com/
]:
patches were merged to staging:
 - [PATCH 3/7] xen/rt: Clean up trace handling
 - [PATCH 4/7] xen/sched: Clean up trace handling
 
  * [PATCH v3 0/4] xenwatchdogd bugfixes and 

[RFC PATCH 1/5] tools/libs/light: Add vid field to libxl_device_nic

[Leigh Brown]

Add integer member `vid' to libxl_device_nic, to represent the
VLAN ID to assign to the VIF when adding it to the bridge device.

Update libxl_nic.c to read and write the vid field from the
xenstore.

This provides the capability for supported operating systems (e.g.
Linux) to perform VLAN filtering on bridge ports.  The Xen
hotplug scripts need to be updated to read this information from
then xenstore and perform the required configuration.

Signed-off-by: Leigh Brown 
---
 tools/libs/light/libxl_nic.c | 20 
 tools/libs/light/libxl_types.idl |  1 +
 2 files changed, 21 insertions(+)

diff --git a/tools/libs/light/libxl_nic.c b/tools/libs/light/libxl_nic.c
index d6bf06fc34..e28b9101ee 100644
--- a/tools/libs/light/libxl_nic.c
+++ b/tools/libs/light/libxl_nic.c
@@ -233,6 +233,11 @@ static int libxl__set_xenstore_nic(libxl__gc *gc, uint32_t 
domid,
 flexarray_append(back, GCSPRINTF("%u", nic->mtu));
 }
 
+if (nic->vid) {
+flexarray_append(back, "vid");
+flexarray_append(back, GCSPRINTF("%u", nic->vid));
+}
+
 flexarray_append(back, "bridge");
 flexarray_append(back, libxl__strdup(gc, nic->bridge));
 flexarray_append(back, "handle");
@@ -313,6 +318,21 @@ static int libxl__nic_from_xenstore(libxl__gc *gc, const 
char *libxl_path,
 nic->mtu = LIBXL_DEVICE_NIC_MTU_DEFAULT;
 }
 
+rc = libxl__xs_read_checked(gc, XBT_NULL,
+GCSPRINTF("%s/vid", libxl_path), );
+if (rc) goto out;
+if (tmp) {
+char *endptr;
+
+nic->vid = strtol(tmp, , 10);
+if (*endptr != '\0') {
+rc = ERROR_INVAL;
+goto out;
+}
+} else {
+nic->vid = 0;
+}
+
 rc = libxl__xs_read_checked(gc, XBT_NULL,
 GCSPRINTF("%s/mac", libxl_path), );
 if (rc) goto out;
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 7d8bd5d216..df5185128c 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -809,6 +809,7 @@ libxl_device_nic = Struct("device_nic", [
 ("backend_domname", string),
 ("devid", libxl_devid),
 ("mtu", integer),
+("vid", integer),
 ("model", string),
 ("mac", libxl_mac),
 ("ip", string),
-- 
2.39.2

[RFC PATCH 0/5] Add bridge VLAN support

[Leigh Brown]

For many years I have been configuring VLANs on my Linux Dom0 by
creating VLAN interfaces for each VLAN I wanted to connect a domain
to and then a corresponding bridge. So I would tend to have things
like:

enp0s0-> br0 -> vif1, vif2
enp0s0.10 -> br0vl10 -> vif3, vif4
enp0s0.20 -> br0vl20 -> vif5
dummy0-> br1 -> vif6

I recently discovered that iproute2 supports creating bridge VLANs that
allows you to assign a VLAN to each of the interfaces associated to a 
bridge. This allows a greatly simplified configuration where a single 
bridge can support all the domains, and the iproute2 bridge command can 
assign each VIF to the required VLAN.  This looks like this:

# bridge vlan
port  vlan-id  
enp0s01 PVID Egress Untagged
  10
  20
br0   1 PVID Egress Untagged
vif1.01 PVID Egress Untagged
vif2.010 PVID Egress Untagged
vif3.010 PVID Egress Untagged
vif4.020 PVID Egress Untagged
vif5.020 PVID Egress Untagged
vif6.030 PVID Egress Untagged

This patch set enables this capability as follows:

1. Adds `vid' as a new member of the libxl_device_nic structure;
2. Adds support to read and write vid from the xenstore;
3. Adds `vid' as a new keyword for the vif configuration option;
4. Adds support for assign the bridge VLAN in the Linux hotplug scripts.

I don't believe NetBSD or FreeBSD support this capability, but if they
do please point me in the direction of some documentation and/or examples.

NB: I'm not very familiar with Xen code base so may have missed
something important, although I have tested it and it is working well
for me.

Cheers,

Leigh.


le...@solinno.co.uk (5):
  tools/libs/light: Add vid field to libxl_device_nic
  tools/xl: add vid keyword vif option
  tools/hotplug/Linux: Add bridge VLAN support
  docs/man: document VIF vid keyword
  tools/examples: Examples Linux bridge VLAN config

 docs/man/xl-network-configuration.5.pod.in|  6 +++
 tools/examples/linux-bridge-vlan/README   | 52 +++
 tools/examples/linux-bridge-vlan/br0.netdev   |  7 +++
 tools/examples/linux-bridge-vlan/br0.network  |  8 +++
 .../examples/linux-bridge-vlan/enp0s0.network | 16 ++
 tools/hotplug/Linux/xen-network-common.sh |  9 
 tools/libs/light/libxl_nic.c  | 20 +++
 tools/libs/light/libxl_types.idl  |  1 +
 tools/xl/xl_parse.c   |  2 +
 9 files changed, 121 insertions(+)
 create mode 100644 tools/examples/linux-bridge-vlan/README
 create mode 100644 tools/examples/linux-bridge-vlan/br0.netdev
 create mode 100644 tools/examples/linux-bridge-vlan/br0.network
 create mode 100644 tools/examples/linux-bridge-vlan/enp0s0.network

-- 
2.39.2

[RFC PATCH 4/5] docs/man: document VIF vid keyword

[Leigh Brown]

Document the new `vid' keyword in xl-network-configuration(5).

Signed-off-by: Leigh Brown 
---
 docs/man/xl-network-configuration.5.pod.in | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/docs/man/xl-network-configuration.5.pod.in 
b/docs/man/xl-network-configuration.5.pod.in
index f3e379bcf8..fe2615ae30 100644
--- a/docs/man/xl-network-configuration.5.pod.in
+++ b/docs/man/xl-network-configuration.5.pod.in
@@ -259,6 +259,12 @@ Specifies the MTU (i.e. the maximum size of an IP payload, 
exclusing headers). T
 default value is 1500 but, if the VIF is attached to a bridge, it will be set 
to match
 unless overridden by this parameter.
 
+=head2 vid
+
+Specifies the VLAN ID. If this is set to a non-zero value, it will be specified
+when attaching the VIF to a bridge.  This can be used on operating systems that
+support bridge VLANs (e.g. Linux using iproute2).
+
 =head2 trusted / untrusted
 
 An advisory setting for the frontend driver on whether the backend should be
-- 
2.39.2

[RFC PATCH 3/5] tools/hotplug/Linux: Add bridge VLAN support

[Leigh Brown]

Update add_to_bridge shell function to read the vid parameter
from xenstore and set the bridge LAN for the VID to the given
value. This only works when using the iproute2 bridge command,
so a warning is issued if using the legacy brctl command and a
vid is set.

Signed-off-by: Leigh Brown 
---
 tools/hotplug/Linux/xen-network-common.sh | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tools/hotplug/Linux/xen-network-common.sh 
b/tools/hotplug/Linux/xen-network-common.sh
index 42fa704e8d..19a8b3c7e5 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -125,14 +125,23 @@ create_bridge () {
 add_to_bridge () {
 local bridge=$1
 local dev=$2
+local vid=$(xenstore_read_default "$XENBUS_PATH/vid" "")
 
 # Don't add $dev to $bridge if it's already on the bridge.
 if [ ! -e "/sys/class/net/${bridge}/brif/${dev}" ]; then
 log debug "adding $dev to bridge $bridge"
 if which brctl >&/dev/null; then
 brctl addif ${bridge} ${dev}
+if [ -n "${vid}" ] ;then
+log warning "bridge command not available, ignoring vid"
+fi
 else
 ip link set ${dev} master ${bridge}
+if [ -n "${vid}" ] ;then
+log debug "Assigning $dev to vid $vid"
+bridge vlan del dev ${dev} vid 1
+bridge vlan add dev ${dev} vid ${vid} pvid untagged
+fi
 fi
 else
 log debug "$dev already on bridge $bridge"
-- 
2.39.2

[RFC PATCH 2/5] tools/xl: add vid keyword vif option

[Leigh Brown]

Update parse_nic_config() to support a new `vid' keyword. This
keyword specifies the numeric VLAN ID to assign to the VIF when
attaching it to the bridge port, on operating systems that support
the capability (e.g. Linux).

Signed-off-by: Leigh Brown 
---
 tools/xl/xl_parse.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index ed983200c3..5d5dd4ec04 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -565,6 +565,8 @@ int parse_nic_config(libxl_device_nic *nic, XLU_Config 
**config, char *token)
 nic->devid = parse_ulong(oparg);
 } else if (MATCH_OPTION("mtu", token, oparg)) {
 nic->mtu = parse_ulong(oparg);
+} else if (MATCH_OPTION("vid", token, oparg)) {
+nic->vid = parse_ulong(oparg);
 } else if (!strcmp("trusted", token)) {
 libxl_defbool_set(>trusted, true);
 } else if (!strcmp("untrusted", token)) {
-- 
2.39.2

[RFC PATCH 5/5] tools/examples: Examples Linux bridge VLAN config

[Leigh Brown]

Add a new directory linux-bridge-vlan showing how to configure
systemd-networkd to support a bridge VLAN configuration.

Signed-off-by: Leigh Brown 
---
 tools/examples/linux-bridge-vlan/README   | 52 +++
 tools/examples/linux-bridge-vlan/br0.netdev   |  7 +++
 tools/examples/linux-bridge-vlan/br0.network  |  8 +++
 .../examples/linux-bridge-vlan/enp0s0.network | 16 ++
 4 files changed, 83 insertions(+)
 create mode 100644 tools/examples/linux-bridge-vlan/README
 create mode 100644 tools/examples/linux-bridge-vlan/br0.netdev
 create mode 100644 tools/examples/linux-bridge-vlan/br0.network
 create mode 100644 tools/examples/linux-bridge-vlan/enp0s0.network

diff --git a/tools/examples/linux-bridge-vlan/README 
b/tools/examples/linux-bridge-vlan/README
new file mode 100644
index 00..b287710e0f
--- /dev/null
+++ b/tools/examples/linux-bridge-vlan/README
@@ -0,0 +1,52 @@
+Linux Xen Dom0 single bridge multiple VLAN configuration with systemd
+=
+
+Introduction
+
+
+This directory contains example files to be placed in /etc/systemd/network
+to enable a single bridge with multiple VLAN support.
+
+The example is to support the scenario where the Xen host network interface
+is connected to an Ethernet switch configured as a trunk port. Each domain
+VIF can then be configured with the VLAN id (vid) of the required VLAN.
+
+The example files create a bridge device called br0, with a physical interface 
+called enp0s0. You will need to update this with your system's device name.
+
+Key points of the configuration are:
+
+1. In br0.netdev, VLANFiltering=on is set. This is required to ensure the
+   VLAN tags are handled correctly.  If it is not set then the packets
+   from the vif interfaces will not have the correct VLAN tags set.  I
+   observed them with the pvid in the switch MAC address table.
+
+2. In br0.network, a system IPv4 address is configured that can be updated
+   according to your local network settings.
+
+3. In enp0s0.network, Bridge=br0 sets the bridge device to connect to and
+   there is a [BridgeVLAN] section for each VLAN you want to give access
+   to the switch. Note, if you want to create an internal VLAN private to
+   the host, do not include that VLAN id in this file.
+
+
+Domain configuration
+
+
+Add the vid= keyword to the vif definition in the domain. For example:
+
+vif = [ 'mac=xx:xx:xx:xx:xx:xx, bridge=br0, vid=10' ]
+
+
+Hints and tips
+--
+
+1. To check if vlan_filtering is enabled, run:
+   # cat /sys/devices/virtual/net//bridge/vlan_filtering
+
+2. To check the bridge port VLAN assignments, run:
+   # bridge vlan
+
+3. To check the vid setting in the xenstore, run:
+   # xenstore-ls -f | grep 'vid ='
+
diff --git a/tools/examples/linux-bridge-vlan/br0.netdev 
b/tools/examples/linux-bridge-vlan/br0.netdev
new file mode 100644
index 00..ae1fe487c3
--- /dev/null
+++ b/tools/examples/linux-bridge-vlan/br0.netdev
@@ -0,0 +1,7 @@
+[NetDev]
+Name=br0
+Kind=bridge
+MACAddress=xx:xx:xx:xx:xx:xx
+
+[Bridge]
+VLANFiltering=on
diff --git a/tools/examples/linux-bridge-vlan/br0.network 
b/tools/examples/linux-bridge-vlan/br0.network
new file mode 100644
index 00..b56203b66a
--- /dev/null
+++ b/tools/examples/linux-bridge-vlan/br0.network
@@ -0,0 +1,8 @@
+[Match]
+Name=br0
+
+[Network]
+DNS=8.8.8.8
+#Domains=example.com
+Address=10.1.1.10/24
+Gateway=10.1.1.1
diff --git a/tools/examples/linux-bridge-vlan/enp0s0.network 
b/tools/examples/linux-bridge-vlan/enp0s0.network
new file mode 100644
index 00..6ee3154dfc
--- /dev/null
+++ b/tools/examples/linux-bridge-vlan/enp0s0.network
@@ -0,0 +1,16 @@
+[Match]
+Name=enp0s0
+
+[Network]
+Bridge=br0
+
+# If Jumbo frames are required
+#[Link]
+#MTUBytes=9000
+
+[BridgeVLAN]
+VLAN=10
+
+[BridgeVLAN]
+VLAN=20
+
-- 
2.39.2

Re: [XEN PATCH v4 5/5] xen/arm: ffa: support notification

[Julien Grall]

Hi Jens,

On 03/05/2024 14:54, Jens Wiklander wrote:

+static int ffa_setup_irq_callback(struct notifier_block *nfb,
+  unsigned long action, void *hcpu)
+{
+unsigned int cpu = (unsigned long)hcpu;
+struct notif_irq_info irq_info = { };
+
+switch ( action )
+{
+case CPU_ONLINE:


Can't you execute the notifier in CPU_STARTING? This will be called on
the CPU directly, so you should be able to use request_irq(...).


I tried that first but it failed with the ASSERT_ALLOC_CONTEXT() in _xmalloc().

I've also tested a three-step solution with CPU_UP_PREPARE,
CPU_STARTING, and CPU_UP_CANCELED.
My approach here is more direct, but it still suffers from a weakness
in error handling even if it seems quite unlikely to run out of heap
or for setup_irq() to fail at this stage.


Ah I didn't notice that notify_cpu_starting() is called with IRQ 
disabled. I assumed they would be enabled.


Then I would consider to do:

diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index 6efed876782e..db322672e508 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -389,6 +389,7 @@ void asmlinkage start_secondary(void)
  */
 init_maintenance_interrupt();
 init_timer_interrupt();
+init_tee_interrupt();

 local_abort_enable();

And plumb through the TEE subsystem.

Cheers,

--
Julien Grall

[linux-linus test] 185907: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-vhd  8 xen-bootfail pass in 185910-retest
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 185910-retest
 test-armhf-armhf-xl-credit1   8 xen-bootfail pass in 185910-retest

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

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

version targeted for testing:
 linuxf03359bca01bf4372cf2c118cd9a987a5951b1c8
baseline version:
 linuxb947cc5bf6d793101135265352e205aeb30b54f0

Last test of basis   185870  2024-04-29 17:43:51 Z3 days
Failing since185888  2024-04-30 20:31:50 Z2 days6 attempts
Testing same since   185907  2024-05-03 02:43:45 Z0 days1 attempts


People who touched revisions under test:
  Alexander Gordeev 
  Alexander Potapenko 
  Alexandra Winter 
  Alexei Starovoitov 
  Andrii Nakryiko 
  Andy Shevchenko 
  AngeloGioacchino Del Regno 
  Anton Protopopov 
  Arnd Bergmann 
  Asbjørn Sloth Tønnesen 
  Audra Mitchell 
  Barry Song 
  Billy Tsai 
  Björn Töpel 
  Björn Töpel 
  Bui Quang Minh 
  

Re: [RFC PATCH v3 3/5] KVM: x86: Add notifications for Heki policy configuration and violation

[Sean Christopherson]

On Fri, May 03, 2024, Mickaël Salaün wrote:
> Add an interface for user space to be notified about guests' Heki policy
> and related violations.
> 
> Extend the KVM_ENABLE_CAP IOCTL with KVM_CAP_HEKI_CONFIGURE and
> KVM_CAP_HEKI_DENIAL. Each one takes a bitmask as first argument that can
> contains KVM_HEKI_EXIT_REASON_CR0 and KVM_HEKI_EXIT_REASON_CR4. The
> returned value is the bitmask of known Heki exit reasons, for now:
> KVM_HEKI_EXIT_REASON_CR0 and KVM_HEKI_EXIT_REASON_CR4.
> 
> If KVM_CAP_HEKI_CONFIGURE is set, a VM exit will be triggered for each
> KVM_HC_LOCK_CR_UPDATE hypercalls according to the requested control
> register. This enables to enlighten the VMM with the guest
> auto-restrictions.
> 
> If KVM_CAP_HEKI_DENIAL is set, a VM exit will be triggered for each
> pinned CR violation. This enables the VMM to react to a policy
> violation.
> 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: H. Peter Anvin 
> Cc: Ingo Molnar 
> Cc: Kees Cook 
> Cc: Madhavan T. Venkataraman 
> Cc: Paolo Bonzini 
> Cc: Sean Christopherson 
> Cc: Thomas Gleixner 
> Cc: Vitaly Kuznetsov 
> Cc: Wanpeng Li 
> Signed-off-by: Mickaël Salaün 
> Link: https://lore.kernel.org/r/20240503131910.307630-4-...@digikod.net
> ---
> 
> Changes since v1:
> * New patch. Making user space aware of Heki properties was requested by
>   Sean Christopherson.

No, I suggested having userspace _control_ the pinning[*], not merely be 
notified
of pinning.

 : IMO, manipulation of protections, both for memory (this patch) and CPU state
 : (control registers in the next patch) should come from userspace.  I have no
 : objection to KVM providing plumbing if necessary, but I think userspace 
needs to
 : to have full control over the actual state.
 : 
 : One of the things that caused Intel's control register pinning series to 
stall
 : out was how to handle edge cases like kexec() and reboot.  Deferring to 
userspace
 : means the kernel doesn't need to define policy, e.g. when to unprotect 
memory,
 : and avoids questions like "should userspace be able to overwrite pinned 
control
 : registers".
 : 
 : And like the confidential VM use case, keeping userspace in the loop is a big
 : beneifit, e.g. the guest can't circumvent protections by coercing userspace 
into
 : writing to protected memory.

I stand by that suggestion, because I don't see a sane way to handle things like
kexec() and reboot without having a _much_ more sophisticated policy than would
ever be acceptable in KVM.

I think that can be done without KVM having any awareness of CR pinning 
whatsoever.
E.g. userspace just needs to ability to intercept CR writes and inject #GPs.  
Off
the cuff, I suspect the uAPI could look very similar to MSR filtering.  E.g. I 
bet
userspace could enforce MSR pinning without any new KVM uAPI at all.

[*] https://lore.kernel.org/all/zfuyhpuhtmbyd...@google.com

Re: [PATCH v2 01/12] VT-d: correct ATS checking for root complex integrated devices

[Roger Pau Monné]

On Thu, Feb 15, 2024 at 11:13:24AM +0100, Jan Beulich wrote:
> Spec version 4.1 says
> 
> "The ATSR structures identifies PCI Express Root-Ports supporting
>  Address Translation Services (ATS) transactions. Software must enable
>  ATS on endpoint devices behind a Root Port only if the Root Port is
>  reported as supporting ATS transactions."
> 
> Clearly root complex integrated devices aren't "behind root ports",
> matching my observation on a SapphireRapids system having an ATS-
> capable root complex integrated device. Hence for such devices we
> shouldn't try to locate a corresponding ATSR.
> 
> Since both pci_find_ext_capability() and pci_find_cap_offset() return
> "unsigned int", change "pos" to that type at the same time.
> 
> Fixes: 903b93211f56 ("[VTD] laying the ground work for ATS")
> Signed-off-by: Jan Beulich 

Acked-by: Roger Pau Monné 

> ---
> v2: New.
> 
> --- a/xen/drivers/passthrough/vtd/x86/ats.c
> +++ b/xen/drivers/passthrough/vtd/x86/ats.c
> @@ -44,7 +44,7 @@ struct acpi_drhd_unit *find_ats_dev_drhd
>  int ats_device(const struct pci_dev *pdev, const struct acpi_drhd_unit *drhd)
>  {
>  struct acpi_drhd_unit *ats_drhd;
> -int pos;
> +unsigned int pos, expfl = 0;
>  
>  if ( !ats_enabled || !iommu_qinval )
>  return 0;
> @@ -53,7 +53,12 @@ int ats_device(const struct pci_dev *pde
>   !ecap_dev_iotlb(drhd->iommu->ecap) )
>  return 0;
>  
> -if ( !acpi_find_matched_atsr_unit(pdev) )
> +pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_EXP);
> +if ( pos )
> +expfl = pci_conf_read16(pdev->sbdf, pos + PCI_EXP_FLAGS);
> +
> +if ( MASK_EXTR(expfl, PCI_EXP_FLAGS_TYPE) != PCI_EXP_TYPE_RC_END &&
> + !acpi_find_matched_atsr_unit(pdev) )

Given the spec quote above, it might also be helpful to check that the
type is PCI_EXP_TYPE_ENDPOINT before attempting the ATSR check?

I would assume a well formed ATSR won't list non-endpoint devices.

Thanks, Roger.

[libvirt test] 185908: tolerable all pass - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 185891
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-vhd 15 saverestore-support-checkfail   never pass

version targeted for testing:
 libvirt  3146305fd3a610573963fe4858cc12ec1c4cf5c7
baseline version:
 libvirt  63f00d09e3bf7442cddf3657b51b0bc645f8c434

Last test of basis   185891  2024-05-01 04:18:46 Z2 days
Testing same since   185908  2024-05-03 04:20:40 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrangé 
  Göran Uddeborg 
  Jim Fehlig 
  Jiri Denemark 
  Kristina Hanicova 
  Martin Kletzander 
  Martin Shirokov 
  Michal Privoznik 
  Peter Krempa 
  Rayhan Faizel 
  Tim Wiederhake 

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



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

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

Explanation of these reports, and of osstest 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/libvirt.git
   63f00d09e3..3146305fd3  3146305fd3a610573963fe4858cc12ec1c4cf5c7 -> 
xen-tested-master

Re: [XEN PATCH v4 5/5] xen/arm: ffa: support notification

[Jens Wiklander]

Hi Julien,

On Fri, May 3, 2024 at 11:59 AM Julien Grall  wrote:
>
> Hi Jens,
>
> On 02/05/2024 15:56, Jens Wiklander wrote:
> > -static bool ffa_probe(void)
> > +static int __init ffa_init(void)
> >   {
> >   uint32_t vers;
> >   unsigned int major_vers;
> > @@ -460,16 +511,16 @@ static bool ffa_probe(void)
> >   printk(XENLOG_ERR
> >  "ffa: unsupported SMCCC version %#x (need at least %#x)\n",
> >  smccc_ver, ARM_SMCCC_VERSION_1_2);
> > -return false;
> > +return 0;
> >   }
> >
> >   if ( !ffa_get_version() )
> > -return false;
> > +return 0;
> >
> >   if ( vers < FFA_MIN_SPMC_VERSION || vers > FFA_MY_VERSION )
> >   {
> >   printk(XENLOG_ERR "ffa: Incompatible version %#x found\n", vers);
> > -return false;
> > +return 0;
> >   }
> >
> >   major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT) & 
> > FFA_VERSION_MAJOR_MASK;
> > @@ -492,26 +543,33 @@ static bool ffa_probe(void)
> >!check_mandatory_feature(FFA_MEM_SHARE_32) ||
> >!check_mandatory_feature(FFA_MEM_RECLAIM) ||
> >!check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
> > -return false;
> > +return 0;
> >
> >   if ( !ffa_rxtx_init() )
> > -return false;
> > +return 0;
> >
> >   ffa_version = vers;
> >
> >   if ( !ffa_partinfo_init() )
> >   goto err_rxtx_destroy;
> >
> > +ffa_notif_init();
> >   INIT_LIST_HEAD(_teardown_head);
> >   init_timer(_teardown_timer, ffa_teardown_timer_callback, NULL, 0);
> >
> > -return true;
> > +return 0;
> >
> >   err_rxtx_destroy:
> >   ffa_rxtx_destroy();
> >   ffa_version = 0;
> >
> > -return false;
> > +return 0;
> > +}
> > +presmp_initcall(ffa_init);
> I would rather prefer if tee_init() is called from presmp_initcall().
> This would avoid FFA to have to try to initialize itself before all the
> others.

OK, I'll update.

>
> This is what I had in mind with my original request, but I guess I
> wasn't clear enough. Sorry for that.

No worries.

>
> [...]
>
> > +static void notif_irq_handler(int irq, void *data)
> > +{
> > +const struct arm_smccc_1_2_regs arg = {
> > +.a0 = FFA_NOTIFICATION_INFO_GET_64,
> > +};
> > +struct arm_smccc_1_2_regs resp;
> > +unsigned int id_pos;
> > +unsigned int list_count;
> > +uint64_t ids_count;
> > +unsigned int n;
> > +int32_t res;
> > +
> > +do {
> > +arm_smccc_1_2_smc(, );
> > +res = ffa_get_ret_code();
> > +if ( res )
> > +{
> > +if ( res != FFA_RET_NO_DATA )
> > +printk(XENLOG_ERR "ffa: notification info get failed: 
> > error %d\n",
> > +   res);
> > +return;
> > +}
> > +
> > +ids_count = resp.a2 >> FFA_NOTIF_INFO_GET_ID_LIST_SHIFT;
> > +list_count = ( resp.a2 >> FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT ) &
> > + FFA_NOTIF_INFO_GET_ID_COUNT_MASK;
> > +
> > +id_pos = 0;
> > +for ( n = 0; n < list_count; n++ )
> > +{
> > +unsigned int count = ((ids_count >> 2 * n) & 0x3) + 1;
> > +uint16_t vm_id = get_id_from_resp(, id_pos);
> > +struct domain *d;
> > +
> > +/*
> > + * vm_id == 0 means a notifications pending for Xen itself, but
> > + * we don't support that yet.
> > + */
> > +if (vm_id)
> > +d = ffa_rcu_lock_domain_by_vm_id(vm_id);
>
> I am still unconvinced that this is sufficient. This will prevent
> "struct domain *" to be freed. But ...
>
> > +else
> > +d = NULL;
> > +
> > +if ( d )
> > +{
> > +struct ffa_ctx *ctx = d->arch.tee;
>
> ... I don't think it will protect d->arch.tee to be freed as this is
> happening during teardorwn. You mostly need some other sort of locking
> to avoid d->arch.tee been freed behind your back.

OK, I'll need to work more on this. The outcome of the [1] thread may
affect this too.

[1] https://lists.xenproject.org/archives/html/xen-devel/2024-05/msg00156.html

>
> > +struct vcpu *v;
> > +
> > +ACCESS_ONCE(ctx->notif.secure_pending) = true;
> > +
> > +/*
> > + * Since we're only delivering global notification, always
> > + * deliver to the first online vCPU. It doesn't matter
> > + * which we chose, as long as it's available.
> > + */
> > +for_each_vcpu(d, v)
> > +{
> > +if ( is_vcpu_online(v) )
> > +{
> > +vgic_inject_irq(d, v, GUEST_FFA_NOTIF_PEND_INTR_ID,
> > +true);
> > +break;
> > +}
> > +}
> > +if ( !v )
> 

Re: [RFC PATCH v3 0/5] Hypervisor-Enforced Kernel Integrity - CR pinning

[Sean Christopherson]

On Fri, May 03, 2024, Mickaël Salaün wrote:
> Hi,
> 
> This patch series implements control-register (CR) pinning for KVM and
> provides an hypervisor-agnostic API to protect guests.  It includes the
> guest interface, the host interface, and the KVM implementation.
> 
> It's not ready for mainline yet (see the current limitations), but we
> think the overall design and interfaces are good and we'd like to have
> some feedback on that.

...

> # Current limitations
> 
> This patch series doesn't handle VM reboot, kexec, nor hybernate yet.
> We'd like to leverage the realated feature from KVM CR-pinning patch
> series [3].  Help appreciated!

Until you have a story for those scenarios, I don't expect you'll get a lot of
valuable feedback, or much feedback at all.  They were the hot topic for KVM CR
pinning, and they'll likely be the hot topic now.

[RFC PATCH v3 1/5] virt: Introduce Hypervisor Enforced Kernel Integrity (Heki)

[Mickaël Salaün]

From: Madhavan T. Venkataraman 

Hypervisor Enforced Kernel Integrity (Heki) is a feature that will use
the hypervisor to enhance guest virtual machine security.

Implement minimal code to introduce Heki:

- Define the config variables.

- Define a kernel command line parameter "heki" to turn the feature
  on or off. By default, Heki is on.

- Define heki_early_init() and call it in start_kernel(). Currently,
  this function only prints the value of the "heki" command
  line parameter.

Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: H. Peter Anvin 
Cc: Ingo Molnar 
Cc: Kees Cook 
Cc: Paolo Bonzini 
Cc: Sean Christopherson 
Cc: Thomas Gleixner 
Cc: Vitaly Kuznetsov 
Cc: Wanpeng Li 
Co-developed-by: Mickaël Salaün 
Signed-off-by: Mickaël Salaün 
Signed-off-by: Madhavan T. Venkataraman 
Link: https://lore.kernel.org/r/20240503131910.307630-2-...@digikod.net
---

Changes since v2:
* Move CONFIG_HEKI under a new CONFIG_HEKI_MENU to group it with the
  test configuration (see following patches).
* Hide CONFIG_ARCH_SUPPORS_HEKI from users.

Changes since v1:
* Shrinked this patch to only contain the minimal common parts.
* Moved heki_early_init() to start_kernel().
* Use kstrtobool().
---
 Kconfig  |  2 ++
 arch/x86/Kconfig |  1 +
 include/linux/heki.h | 31 +++
 init/main.c  |  2 ++
 mm/mm_init.c |  1 +
 virt/Makefile|  1 +
 virt/heki/Kconfig| 25 +
 virt/heki/Makefile   |  3 +++
 virt/heki/common.h   | 16 
 virt/heki/main.c | 33 +
 10 files changed, 115 insertions(+)
 create mode 100644 include/linux/heki.h
 create mode 100644 virt/heki/Kconfig
 create mode 100644 virt/heki/Makefile
 create mode 100644 virt/heki/common.h
 create mode 100644 virt/heki/main.c

diff --git a/Kconfig b/Kconfig
index 745bc773f567..0c844d9bcb03 100644
--- a/Kconfig
+++ b/Kconfig
@@ -29,4 +29,6 @@ source "lib/Kconfig"
 
 source "lib/Kconfig.debug"
 
+source "virt/heki/Kconfig"
+
 source "Documentation/Kconfig"
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 928820e61cb5..d2fba63c289b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -34,6 +34,7 @@ config X86_64
select SWIOTLB
select ARCH_HAS_ELFCORE_COMPAT
select ZONE_DMA32
+   select ARCH_SUPPORTS_HEKI
 
 config FORCE_DYNAMIC_FTRACE
def_bool y
diff --git a/include/linux/heki.h b/include/linux/heki.h
new file mode 100644
index ..4c18d2283392
--- /dev/null
+++ b/include/linux/heki.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Hypervisor Enforced Kernel Integrity (Heki) - Definitions
+ *
+ * Copyright © 2023 Microsoft Corporation
+ */
+
+#ifndef __HEKI_H__
+#define __HEKI_H__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#ifdef CONFIG_HEKI
+
+extern bool heki_enabled;
+
+void heki_early_init(void);
+
+#else /* !CONFIG_HEKI */
+
+static inline void heki_early_init(void)
+{
+}
+
+#endif /* CONFIG_HEKI */
+
+#endif /* __HEKI_H__ */
diff --git a/init/main.c b/init/main.c
index 5dcf5274c09c..bec2c8d939aa 100644
--- a/init/main.c
+++ b/init/main.c
@@ -102,6 +102,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -1059,6 +1060,7 @@ void start_kernel(void)
uts_ns_init();
key_init();
security_init();
+   heki_early_init();
dbg_late_init();
net_ns_init();
vfs_caches_init();
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 549e76af8f82..89d9f97bd471 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "internal.h"
 #include "slab.h"
 #include "shuffle.h"
diff --git a/virt/Makefile b/virt/Makefile
index 1cfea9436af9..856b5ccedb5a 100644
--- a/virt/Makefile
+++ b/virt/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-y  += lib/
+obj-$(CONFIG_HEKI_MENU) += heki/
diff --git a/virt/heki/Kconfig b/virt/heki/Kconfig
new file mode 100644
index ..66e73d212856
--- /dev/null
+++ b/virt/heki/Kconfig
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Hypervisor Enforced Kernel Integrity (Heki)
+
+config ARCH_SUPPORTS_HEKI
+   bool
+   # An architecture should select this when it can successfully build
+   # and run with CONFIG_HEKI. That is, it should provide all of the
+   # architecture support required for the HEKI feature.
+
+menuconfig HEKI_MENU
+   bool "Virtualization hardening"
+
+if HEKI_MENU
+
+config HEKI
+   bool "Hypervisor Enforced Kernel Integrity (Heki)"
+   depends on ARCH_SUPPORTS_HEKI
+   help
+ This feature enhances guest virtual machine security by taking
+ advantage of security features provided by the hypervisor for guests.
+ This feature is helpful in maintaining guest virtual machine security
+ even after the guest kernel has been compromised.
+
+endif
diff --git a/virt/heki/Makefile b/virt/heki/Makefile
new 

[RFC PATCH v3 5/5] virt: Add Heki KUnit tests

[Mickaël Salaün]

The new CONFIG_HEKI_KUNIT_TEST option enables to run tests in a a kernel
module.  The minimal required configuration is listed in the
virt/heki-test/.kunitconfig file.

test_cr_disable_smep checks control-register pinning by trying to
disable SMEP.  This test should then failed on a non-protected kernel,
and only succeed with a kernel protected by Heki.

This test doesn't rely on native_write_cr4() because of the
cr4_pinned_mask hardening, which means that this *test* module loads a
valid kernel code to arbitrary change CR4.  This simulate an attack
scenario where an attaker would use ROP to directly jump to the related
cr4 instruction.

As for any KUnit test, the kernel is tainted with TAINT_TEST when the
test is executed.

It is interesting to create new KUnit tests instead of extending KVM's
Kselftests because Heki is design to be hypervisor-agnostic, it relies
on a set of hypercalls (for KVM or others), and we also want to test
kernel's configuration (actual pinned CR).  However, new KVM's
Kselftests would be useful to test KVM's interface with the host.

When using Qemu, we need to pass the following arguments: -cpu host
-enable-kvm

For now, it is not possible to run these tests as built-in but we are
working on that [1].  If tests are built-in anyway, they will just be
skipped because Heki would not be enabled.

Run Heki tests with:
  insmod heki-test.ko

  KTAP version 1
  1..1
  KTAP version 1
  # Subtest: heki_x86
  # module: heki_test
  1..1
  ok 1 test_cr_disable_smep
  ok 1 heki_x86

Link: https://lore.kernel.org/r/20240229170409.365386-2-...@digikod.net [1]
Signed-off-by: Mickaël Salaün 
Link: https://lore.kernel.org/r/20240503131910.307630-6-...@digikod.net
---

Changes since v2:
* Make tests standalone (e.g. don't depends on CONFIG_HEKI).
* Enable to create a test kernel module.
* Don't rely on private kernel symbols.
* Handle GP fault for CR-pinning test case.
* Rename option to CONFIG_HEKI_KUNIT_TEST.
* Add the list of required kernel options.
* Move tests to virt/heki-test/ [FIXME]
* Only keep CR pinning test.
* Restore previous state (with SMEP enabled).
* Add a Kconfig menu for Heki and update the description.
* Skip tests if Heki is not protecting the running kernel.

Changes since v1:
* Move all tests to virt/heki/tests.c
---
 include/linux/heki.h   |   1 +
 virt/heki/.kunitconfig |   9 
 virt/heki/Kconfig  |  12 +
 virt/heki/Makefile |   1 +
 virt/heki/heki-test.c  | 114 +
 virt/heki/main.c   |  10 
 6 files changed, 147 insertions(+)
 create mode 100644 virt/heki/.kunitconfig
 create mode 100644 virt/heki/heki-test.c

diff --git a/include/linux/heki.h b/include/linux/heki.h
index 96ccb17657e5..3294c4d583e5 100644
--- a/include/linux/heki.h
+++ b/include/linux/heki.h
@@ -35,6 +35,7 @@ struct heki {
 
 extern struct heki heki;
 extern bool heki_enabled;
+extern bool heki_enforcing;
 
 void heki_early_init(void);
 void heki_late_init(void);
diff --git a/virt/heki/.kunitconfig b/virt/heki/.kunitconfig
new file mode 100644
index ..ad4454800579
--- /dev/null
+++ b/virt/heki/.kunitconfig
@@ -0,0 +1,9 @@
+CONFIG_HEKI=y
+CONFIG_HEKI_KUNIT_TEST=m
+CONFIG_HEKI_MENU=y
+CONFIG_HIGH_RES_TIMERS=y
+CONFIG_HYPERVISOR_GUEST=y
+CONFIG_KUNIT=y
+CONFIG_KVM=y
+CONFIG_KVM_GUEST=y
+CONFIG_PARAVIRT=y
diff --git a/virt/heki/Kconfig b/virt/heki/Kconfig
index 0c764e342f48..18895a81a9af 100644
--- a/virt/heki/Kconfig
+++ b/virt/heki/Kconfig
@@ -28,4 +28,16 @@ config HEKI
  This feature is helpful in maintaining guest virtual machine security
  even after the guest kernel has been compromised.
 
+config HEKI_KUNIT_TEST
+   tristate "KUnit tests for Heki" if !KUNIT_ALL_TESTS
+   depends on KUNIT
+   depends on X86
+   default KUNIT_ALL_TESTS
+   help
+ Build KUnit tests for Landlock.
+
+ See the KUnit documentation in Documentation/dev-tools/kunit
+
+ If you are unsure how to answer this question, answer N.
+
 endif
diff --git a/virt/heki/Makefile b/virt/heki/Makefile
index 8b10e73a154b..7133545eb5ae 100644
--- a/virt/heki/Makefile
+++ b/virt/heki/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 obj-$(CONFIG_HEKI) += main.o
+obj-$(CONFIG_HEKI_KUNIT_TEST) += heki-test.o
diff --git a/virt/heki/heki-test.c b/virt/heki/heki-test.c
new file mode 100644
index ..b4e11c21ac5d
--- /dev/null
+++ b/virt/heki/heki-test.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Hypervisor Enforced Kernel Integrity (Heki) - Tests
+ *
+ * Copyright © 2023-2024 Microsoft Corporation
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Returns true on error (i.e. GP fault), false otherwise. */
+static __always_inline bool set_cr4(unsigned long value)
+{
+   int err = 0;
+
+   might_sleep();
+   /* clang-format off */
+   asm volatile("1: mov %[value],%%cr4 \n"
+"2: \n"
+ 

[RFC PATCH v3 3/5] KVM: x86: Add notifications for Heki policy configuration and violation

[Mickaël Salaün]

Add an interface for user space to be notified about guests' Heki policy
and related violations.

Extend the KVM_ENABLE_CAP IOCTL with KVM_CAP_HEKI_CONFIGURE and
KVM_CAP_HEKI_DENIAL. Each one takes a bitmask as first argument that can
contains KVM_HEKI_EXIT_REASON_CR0 and KVM_HEKI_EXIT_REASON_CR4. The
returned value is the bitmask of known Heki exit reasons, for now:
KVM_HEKI_EXIT_REASON_CR0 and KVM_HEKI_EXIT_REASON_CR4.

If KVM_CAP_HEKI_CONFIGURE is set, a VM exit will be triggered for each
KVM_HC_LOCK_CR_UPDATE hypercalls according to the requested control
register. This enables to enlighten the VMM with the guest
auto-restrictions.

If KVM_CAP_HEKI_DENIAL is set, a VM exit will be triggered for each
pinned CR violation. This enables the VMM to react to a policy
violation.

Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: H. Peter Anvin 
Cc: Ingo Molnar 
Cc: Kees Cook 
Cc: Madhavan T. Venkataraman 
Cc: Paolo Bonzini 
Cc: Sean Christopherson 
Cc: Thomas Gleixner 
Cc: Vitaly Kuznetsov 
Cc: Wanpeng Li 
Signed-off-by: Mickaël Salaün 
Link: https://lore.kernel.org/r/20240503131910.307630-4-...@digikod.net
---

Changes since v1:
* New patch. Making user space aware of Heki properties was requested by
  Sean Christopherson.
---
 arch/x86/kvm/vmx/vmx.c   |   5 +-
 arch/x86/kvm/x86.c   | 114 +++
 arch/x86/kvm/x86.h   |   7 +--
 include/linux/kvm_host.h |   2 +
 include/uapi/linux/kvm.h |  22 
 5 files changed, 136 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 7ba970b525f7..5869a1ed7866 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5445,6 +5445,7 @@ static int handle_cr(struct kvm_vcpu *vcpu)
int reg;
int err;
int ret;
+   bool exit = false;
 
exit_qualification = vmx_get_exit_qual(vcpu);
cr = exit_qualification & 15;
@@ -5454,8 +5455,8 @@ static int handle_cr(struct kvm_vcpu *vcpu)
val = kvm_register_read(vcpu, reg);
trace_kvm_cr_write(cr, val);
 
-   ret = heki_check_cr(vcpu, cr, val);
-   if (ret)
+   ret = heki_check_cr(vcpu, cr, val, );
+   if (exit)
return ret;
 
switch (cr) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a5f47be59abc..865e88f2b0fc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -119,6 +119,10 @@ static u64 __read_mostly cr4_reserved_bits = 
CR4_RESERVED_BITS;
 
 #define KVM_CAP_PMU_VALID_MASK KVM_PMU_CAP_DISABLE
 
+#define KVM_HEKI_EXIT_REASON_VALID_MASK ( \
+   KVM_HEKI_EXIT_REASON_CR0 | \
+   KVM_HEKI_EXIT_REASON_CR4)
+
 #define KVM_X2APIC_API_VALID_FLAGS (KVM_X2APIC_API_USE_32BIT_IDS | \
 KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK)
 
@@ -4836,6 +4840,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long 
ext)
if (kvm_is_vm_type_supported(KVM_X86_SW_PROTECTED_VM))
r |= BIT(KVM_X86_SW_PROTECTED_VM);
break;
+   case KVM_CAP_HEKI_CONFIGURE:
+   case KVM_CAP_HEKI_DENIAL:
+   r = KVM_HEKI_EXIT_REASON_VALID_MASK;
+   break;
default:
break;
}
@@ -6729,6 +6737,22 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
}
mutex_unlock(>lock);
break;
+#ifdef CONFIG_HEKI
+   case KVM_CAP_HEKI_CONFIGURE:
+   r = -EINVAL;
+   if (cap->args[0] & ~KVM_HEKI_EXIT_REASON_VALID_MASK)
+   break;
+   kvm->heki_configure_exit_reason = cap->args[0];
+   r = 0;
+   break;
+   case KVM_CAP_HEKI_DENIAL:
+   r = -EINVAL;
+   if (cap->args[0] & ~KVM_HEKI_EXIT_REASON_VALID_MASK)
+   break;
+   kvm->heki_denial_exit_reason = cap->args[0];
+   r = 0;
+   break;
+#endif
default:
r = -EINVAL;
break;
@@ -8283,11 +8307,60 @@ static unsigned long emulator_get_cr(struct 
x86_emulate_ctxt *ctxt, int cr)
 
 #ifdef CONFIG_HEKI
 
+static int complete_heki_configure_exit(struct kvm_vcpu *const vcpu)
+{
+   kvm_rax_write(vcpu, 0);
+   ++vcpu->stat.hypercalls;
+   return kvm_skip_emulated_instruction(vcpu);
+}
+
+static int complete_heki_denial_exit(struct kvm_vcpu *const vcpu)
+{
+   kvm_inject_gp(vcpu, 0);
+   return 1;
+}
+
+/* Returns true if the @exit_reason is handled by @vcpu->kvm. */
+static bool heki_exit_cr(struct kvm_vcpu *const vcpu, const __u32 exit_reason,
+const u64 heki_reason, unsigned long value)
+{
+   switch (exit_reason) {
+   case KVM_EXIT_HEKI_CONFIGURE:
+   if (!(vcpu->kvm->heki_configure_exit_reason & heki_reason))
+   return false;
+
+   vcpu->run->heki_configure.reason = heki_reason;
+   

[RFC PATCH v3 4/5] heki: Lock guest control registers at the end of guest kernel init

[Mickaël Salaün]

The hypervisor needs to provide some functions to support Heki. These
form the Heki-Hypervisor API.

Define a heki_hypervisor structure to house the API functions. A
hypervisor that supports Heki must instantiate a heki_hypervisor
structure and pass it to the Heki common code. This allows the common
code to access these functions in a hypervisor-agnostic way.

The first function that is implemented is lock_crs() (lock control
registers). That is, certain flags in the control registers are pinned
so that they can never be changed for the lifetime of the guest.

Implement Heki support in the guest:

- Each supported hypervisor in x86 implements a set of functions for the
  guest kernel. Add an init_heki() function to that set.  This function
  initializes Heki-related stuff. Call init_heki() for the detected
  hypervisor in init_hypervisor_platform().

- Implement init_heki() for the guest.

- Implement kvm_lock_crs() in the guest to lock down control registers.
  This function calls a KVM hypercall to do the job.

- Instantiate a heki_hypervisor structure that contains a pointer to
  kvm_lock_crs().

- Pass the heki_hypervisor structure to Heki common code in init_heki().

Implement a heki_late_init() function and call it at the end of kernel
init. This function calls lock_crs(). In other words, control registers
of a guest are locked down at the end of guest kernel init.

Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: H. Peter Anvin 
Cc: Ingo Molnar 
Cc: Kees Cook 
Cc: Paolo Bonzini 
Cc: Sean Christopherson 
Cc: Thomas Gleixner 
Cc: Vitaly Kuznetsov 
Cc: Wanpeng Li 
Co-developed-by: Madhavan T. Venkataraman 
Signed-off-by: Madhavan T. Venkataraman 
Signed-off-by: Mickaël Salaün 
Link: https://lore.kernel.org/r/20240503131910.307630-5-...@digikod.net
---

Changes since v2:
* Hide CONFIG_HYPERVISOR_SUPPORTS_HEKI from users.

Changes since v1:
* Shrinked the patch to only manage the CR pinning.
---
 arch/x86/include/asm/x86_init.h  |  1 +
 arch/x86/kernel/cpu/hypervisor.c |  1 +
 arch/x86/kernel/kvm.c| 56 
 arch/x86/kvm/Kconfig |  1 +
 include/linux/heki.h | 22 +
 init/main.c  |  1 +
 virt/heki/Kconfig|  8 -
 virt/heki/main.c | 25 ++
 8 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 6149eabe200f..113998799473 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -128,6 +128,7 @@ struct x86_hyper_init {
bool (*msi_ext_dest_id)(void);
void (*init_mem_mapping)(void);
void (*init_after_bootmem)(void);
+   void (*init_heki)(void);
 };
 
 /**
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index 553bfbfc3a1b..6085c8129e0c 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -106,4 +106,5 @@ void __init init_hypervisor_platform(void)
 
x86_hyper_type = h->type;
x86_init.hyper.init_platform();
+   x86_init.hyper.init_heki();
 }
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 7f0732bc0ccd..a54f2c0d7cd0 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -999,6 +1000,60 @@ static bool kvm_sev_es_hcall_finish(struct ghcb *ghcb, 
struct pt_regs *regs)
 }
 #endif
 
+#ifdef CONFIG_HEKI
+
+extern unsigned long cr4_pinned_mask;
+
+/*
+ * TODO: Check SMP policy consistency, e.g. with
+ * this_cpu_read(cpu_tlbstate.cr4)
+ */
+static int kvm_lock_crs(void)
+{
+   unsigned long cr4;
+   int err;
+
+   err = kvm_hypercall3(KVM_HC_LOCK_CR_UPDATE, 0, X86_CR0_WP, 0);
+   if (err)
+   return err;
+
+   cr4 = __read_cr4();
+   err = kvm_hypercall3(KVM_HC_LOCK_CR_UPDATE, 4, cr4 & cr4_pinned_mask,
+0);
+   return err;
+}
+
+static struct heki_hypervisor kvm_heki_hypervisor = {
+   .lock_crs = kvm_lock_crs,
+};
+
+static void kvm_init_heki(void)
+{
+   long err;
+
+   if (!kvm_para_available()) {
+   /* Cannot make KVM hypercalls. */
+   return;
+   }
+
+   err = kvm_hypercall3(KVM_HC_LOCK_CR_UPDATE, 0, 0,
+KVM_LOCK_CR_UPDATE_VERSION);
+   if (err < 1) {
+   /* Ignores host not supporting at least the first version. */
+   return;
+   }
+
+   heki.hypervisor = _heki_hypervisor;
+}
+
+#else /* CONFIG_HEKI */
+
+static void kvm_init_heki(void)
+{
+}
+
+#endif /* CONFIG_HEKI */
+
 const __initconst struct hypervisor_x86 x86_hyper_kvm = {
.name   = "KVM",
.detect = kvm_detect,
@@ -1007,6 +1062,7 @@ const __initconst struct hypervisor_x86 x86_hyper_kvm = {
.init.x2apic_available  = 

[RFC PATCH v3 2/5] KVM: x86: Add new hypercall to lock control registers

[Mickaël Salaün]

This enables guests to lock their CR0 and CR4 registers with a subset of
X86_CR0_WP, X86_CR4_SMEP, X86_CR4_SMAP, X86_CR4_UMIP, X86_CR4_FSGSBASE
and X86_CR4_CET flags.

The new KVM_HC_LOCK_CR_UPDATE hypercall takes three arguments.  The
first is to identify the control register, the second is a bit mask to
pin (i.e. mark as read-only), and the third is for optional flags.

These register flags should already be pinned by Linux guests, but once
compromised, this self-protection mechanism could be disabled, which is
not the case with this dedicated hypercall.

Once the CRs are pinned by the guest, if it attempts to change them,
then a general protection fault is sent to the guest.

This hypercall may evolve and support new kind of registers or pinning.
The optional KVM_LOCK_CR_UPDATE_VERSION flag enables guests to know the
supported abilities by mapping the returned version with the related
features.

Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: H. Peter Anvin 
Cc: Ingo Molnar 
Cc: Kees Cook 
Cc: Madhavan T. Venkataraman 
Cc: Paolo Bonzini 
Cc: Sean Christopherson 
Cc: Thomas Gleixner 
Cc: Vitaly Kuznetsov 
Cc: Wanpeng Li 
Signed-off-by: Mickaël Salaün 
Link: https://lore.kernel.org/r/20240503131910.307630-3-...@digikod.net
---

Changes since v1:
* Guard KVM_HC_LOCK_CR_UPDATE hypercall with CONFIG_HEKI.
* Move extern cr4_pinned_mask to x86.h (suggested by Kees Cook).
* Move VMX CR checks from vmx_set_cr*() to handle_cr() to make it
  possible to return to user space (see next commit).
* Change the heki_check_cr()'s first argument to vcpu.
* Don't use -KVM_EPERM in heki_check_cr().
* Generate a fault when the guest requests a denied CR update.
* Add a flags argument to get the version of this hypercall. Being able
  to do a preper version check was suggested by Wei Liu.
---
 Documentation/virt/kvm/x86/hypercalls.rst | 17 +
 arch/x86/include/uapi/asm/kvm_para.h  |  2 +
 arch/x86/kernel/cpu/common.c  |  7 +-
 arch/x86/kvm/vmx/vmx.c|  5 ++
 arch/x86/kvm/x86.c| 84 +++
 arch/x86/kvm/x86.h| 22 ++
 include/linux/kvm_host.h  |  5 ++
 include/uapi/linux/kvm_para.h |  1 +
 8 files changed, 141 insertions(+), 2 deletions(-)

diff --git a/Documentation/virt/kvm/x86/hypercalls.rst 
b/Documentation/virt/kvm/x86/hypercalls.rst
index 10db7924720f..3178576f4c47 100644
--- a/Documentation/virt/kvm/x86/hypercalls.rst
+++ b/Documentation/virt/kvm/x86/hypercalls.rst
@@ -190,3 +190,20 @@ the KVM_CAP_EXIT_HYPERCALL capability. Userspace must 
enable that capability
 before advertising KVM_FEATURE_HC_MAP_GPA_RANGE in the guest CPUID.  In
 addition, if the guest supports KVM_FEATURE_MIGRATION_CONTROL, userspace
 must also set up an MSR filter to process writes to MSR_KVM_MIGRATION_CONTROL.
+
+9. KVM_HC_LOCK_CR_UPDATE
+
+
+:Architecture: x86
+:Status: active
+:Purpose: Request some control registers to be restricted.
+
+- a0: identify a control register
+- a1: bit mask to make some flags read-only
+- a2: optional KVM_LOCK_CR_UPDATE_VERSION flag that will return the version of
+  this hypercall. Version 1 supports CR0 and CR4 pinning.
+
+The hypercall lets a guest request control register flags to be pinned for
+itself.
+
+Returns 0 on success or a KVM error code otherwise.
diff --git a/arch/x86/include/uapi/asm/kvm_para.h 
b/arch/x86/include/uapi/asm/kvm_para.h
index a1efa7907a0b..cfc17f3d1877 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -149,4 +149,6 @@ struct kvm_vcpu_pv_apf_data {
 #define KVM_PV_EOI_ENABLED KVM_PV_EOI_MASK
 #define KVM_PV_EOI_DISABLED 0x0
 
+#define KVM_LOCK_CR_UPDATE_VERSION (1 << 0)
+
 #endif /* _UAPI_ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 605c26c009c8..69695d9d6e2a 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -398,8 +398,11 @@ static __always_inline void setup_umip(struct cpuinfo_x86 
*c)
 }
 
 /* These bits should not change their value after CPU init is finished. */
-static const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP | 
X86_CR4_UMIP |
-X86_CR4_FSGSBASE | X86_CR4_CET | 
X86_CR4_FRED;
+const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP |
+ X86_CR4_UMIP | X86_CR4_FSGSBASE |
+ X86_CR4_CET | X86_CR4_FRED;
+EXPORT_SYMBOL_GPL(cr4_pinned_mask);
+
 static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning);
 static unsigned long cr4_pinned_bits __ro_after_init;
 
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 22411f4aff53..7ba970b525f7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5453,6 +5453,11 @@ static int handle_cr(struct kvm_vcpu *vcpu)
case 0: /* mov to cr */
val = kvm_register_read(vcpu, reg);
   

[RFC PATCH v3 0/5] Hypervisor-Enforced Kernel Integrity - CR pinning

[Mickaël Salaün]

Hi,

This patch series implements control-register (CR) pinning for KVM and
provides an hypervisor-agnostic API to protect guests.  It includes the
guest interface, the host interface, and the KVM implementation.

It's not ready for mainline yet (see the current limitations), but we
think the overall design and interfaces are good and we'd like to have
some feedback on that.

# Changes since previous version

We choose to remove as much as possible from the previous version of
this patch series to only keep the CR pinning feature and the API.  This
makes the patches simpler and brings the foundation for future
enhancement.  This will also enables us to quickly iterate on new
versions.  We are still working on memory protection but that should be
part of another patch series, if possible once this one land.

We implemented proper KUnit tests and we are also improving the test
framework to make it easier to run tests (and another series is planed):
https://lore.kernel.org/r/20240408074625.65017-1-...@digikod.net
It makes sense to use KUnit for hypervisor-agnostic features.

This series is rebased on top of v6.9-rc6 . guest_memfd is now merged
in mainline, which will help upcoming memory-related changes.

# Overview

The main idea being that kernel self-protection mechanisms should be
delegated to a more privileged part of the system, that is the
hypervisor (see the Threat model below for more details).  It is still
the role of the guest kernel to request such restrictions according to
its configuration. The high-level security guarantees provided by the
hypervisor are semantically the same as a subset of those the kernel
already enforces on itself (CR pinning hardening), but with much strong
guarantees.

The guest kernel API layer contains a global struct heki_hypervisor to
share data and functions between the common code and the hypervisor
support code.  The struct heki_hypervisor enables to plug in different
backend implementations that are initialized with the heki_early_init()
and heki_late_init() calls.

We took inspiration from previous patches, mainly the KVMI [1] [2] and
KVM CR-pinning [3] series, revamped and simplified relevant parts to fit
well with our goal, added one hypercall, and created a kernel API for
VMs to request protection in a generic way that can be leveraged by any
hypervisor.

When a guest request to change one of its previously protected CR, KVM
creates a GP fault.

Because the VMM needs to be involved and know the guests' requested
memory permissions, we implemented two new kind of VM exits to be able
to notify the VMM about guests' Heki configurations and policy
violations.  Indeed, forwarding such signals to the VMM could help
improve attack detection, and react to such attempt (e.g. log events,
stop the VM).  Giving visibility to the VMM would also enable us to
migrate VMs.

# Threat model

The main threat model is a malicious user space process exploiting a
kernel vulnerability to gain more privileges or to bypass the
access-control system.  This threat also covers attacks coming from
network or storage data (e.g., malformed network packet, inconsistent
drive content).

Considering all potential ways to compromise a kernel, Heki's goal is to
harden a sane kernel before a runtime attack to make it more difficult,
and potentially to cause such an attack to fail. Because current attack
mitigations are only mitigations, we consider the kernel itself to be
partially malicious during its lifetime e.g., because a ROP attack that
could disable kernel self-protection mechanisms and make kernel
exploitation much easier. Indeed, an exploit is often split into several
stages, each bypassing some security measures (including CFI).

CR pinning should already be enforced by the guest kernel and the reason
to pin such registers is the same.  With this patch series it
significantly improve such protection.

Getting the guarantee that these control registers cannot be changed
increases the cost of an attack.

# Prerequisites

For this new security layer to be effective, guest kernels must be
trusted by the VM owners at boot time, before launching any user space
processes nor receiving potentially malicious network packets. It is
then required to have a security mechanism to provide or check this
initial trust (e.g., secure boot, kernel module signing).  To protect
against persistent attacks, complementary security mechanisms should be
used (e.g., IMA, IPE, Lockdown).

# How does it work?

The KVM_HC_LOCK_CR_UPDATE hypercall enables guests to pin some of its
CPU control register flags (e.g., X86_CR0_WP, X86_CR4_SMEP,
X86_CR4_SMAP).

Two new kinds of VM exits are implemented: one for a guest Heki request
(i.e. hypercall), and another for a guest attempt to change its pinned
CRs.  When the guest attempts to update pinned CRs or to access memory
in a way that is not allowed, the VMM can then be notified and react to
such attack attempt. After that, if the VM is still running, KVM sends
a GP fault 

[XEN PATCH] automation/eclair: hide reports coming from adopted code in scheduled analysis

[Federico Serafini]

To improve clarity and ease of navigation do not show reports related
to adopted code in the scheduled analysis.
Configuration options are commented out because they may be useful
in the future.

Signed-off-by: Simone Ballarin 
Signed-off-by: Federico Serafini 
---
 .../eclair_analysis/ECLAIR/analysis.ecl   | 24 +++
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/automation/eclair_analysis/ECLAIR/analysis.ecl 
b/automation/eclair_analysis/ECLAIR/analysis.ecl
index 66ed7f952c..67be38f03c 100644
--- a/automation/eclair_analysis/ECLAIR/analysis.ecl
+++ b/automation/eclair_analysis/ECLAIR/analysis.ecl
@@ -4,11 +4,11 @@
 
 setq(data_dir,getenv("ECLAIR_DATA_DIR"))
 setq(analysis_kind,getenv("ANALYSIS_KIND"))
-setq(scheduled_analysis,nil)
+# setq(scheduled_analysis,nil)
 
-strings_map("scheduled-analysis",500,"","^.*scheduled$",0,setq(scheduled_analysis,t))
-strings_map("scheduled-analysis",500,"","^.*$",0)
-map_strings("scheduled-analysis",analysis_kind)
+# 
strings_map("scheduled-analysis",500,"","^.*scheduled$",0,setq(scheduled_analysis,t))
+# strings_map("scheduled-analysis",500,"","^.*$",0)
+# map_strings("scheduled-analysis",analysis_kind)
 
 -verbose
 
@@ -25,12 +25,16 @@ map_strings("scheduled-analysis",analysis_kind)
 -doc="Initially, there are no files tagged as adopted."
 -file_tag+={adopted,"none()"}
 
-if(not(scheduled_analysis),
-eval_file("adopted.ecl")
-)
-if(not(scheduled_analysis),
-eval_file("out_of_scope.ecl")
-)
+# if(not(scheduled_analysis),
+# eval_file("adopted.ecl")
+# )
+# if(not(scheduled_analysis),
+# eval_file("out_of_scope.ecl")
+# )
+
+-eval_file=adopted.ecl
+-eval_file=out_of_scope.ecl
+
 -eval_file=deviations.ecl
 -eval_file=call_properties.ecl
 -eval_file=tagging.ecl
-- 
2.34.1

Re: [PATCH for-4.19? 0/2] xen/x86: support foreign mappings for HVM

[Roger Pau Monné]

On Tue, Apr 30, 2024 at 06:58:43PM +0200, Roger Pau Monne wrote:
> Hello,
> 
> The following series attempts to solve a shortcoming of HVM/PVH guests
> with the lack of support for foreign mappings.  Such lack of support
> prevents using PVH based guests as stubdomains for example.
> 
> Add support in a way similar to how it was done on Arm, by iterating
> over the p2m based on the maximum gfn.
> 
> Mostly untested, sending early in case it could be considered for 4.19.

I've now tested this using the following dummy XTF test:

void test_main(void)
{
unsigned long idxs = 0;
xen_pfn_t gpfns = 0;
int errs = 0, error;
struct xen_add_to_physmap_batch add = {
.domid = DOMID_SELF,
.space = XENMAPSPACE_gmfn_foreign,
.u.foreign_domid = 0,
.size = 1,
.idxs.p = ,
.gpfns.p = ,
.errs.p = ,
};

error = hypercall_memory_op(XENMEM_add_to_physmap_batch, );
if ( error || errs )
xtf_error("add_to_physmap_batch failed: %d (errs: %d)\n", error, errs);

while ( 1 );

xtf_success(NULL);
}

And avoiding some of the permissions checks in Xen so that an
arbitrary domU could map dom0 gfn 0.  I see count_info increasing by 1
until the XTF guest is killed, at which point the count_info goes back
to the value previous to the map.

Regards, Roger.

Re: [PATCH] xen/Kconfig: Drop the final remnants of ---help---

[Oleksii]

On Thu, 2024-05-02 at 19:10 +0100, Andrew Cooper wrote:
> We deprecated the use of ---help--- a while ago, but a lot of new
> content
> copy bad examples.  Convert the remaining instances, and
> update
> Kconfig's parser to no longer recongise it.
> 
> This now causes builds to fail with:
> 
>   Kconfig.debug:8: syntax error
>   Kconfig.debug:7: unknown statement "---help---"
> 
> which short circuits one common piece of churn in new content.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: George Dunlap 
> CC: Jan Beulich 
> CC: Stefano Stabellini 
> CC: Julien Grall 
> CC: Oleksii Kurochko 
> 
> For 4.19.  This cleans up a legacy we've been wanting to get rid of
> for a
> while, and will be least disruptive on people if it gets in ahead of
> most
> people starting work for 4.20.
I am okay with commiting the patch during 4.19 release:
 Release-acked-by: Oleksii Kurochko 
> ---
>  xen/Kconfig |  2 +-
>  xen/Kconfig.debug   | 28 +--
>  xen/arch/arm/Kconfig    |  8 +++---
>  xen/arch/arm/platforms/Kconfig  | 12 -
>  xen/arch/x86/Kconfig    | 32 +++---
>  xen/common/Kconfig  | 48 ---
> --
>  xen/common/sched/Kconfig    | 10 +++
>  xen/drivers/passthrough/Kconfig |  8 +++---
>  xen/drivers/video/Kconfig   |  2 +-
>  xen/tools/kconfig/lexer.l   |  2 +-
>  10 files changed, 76 insertions(+), 76 deletions(-)
> 
> diff --git a/xen/Kconfig b/xen/Kconfig
> index 1e1b041fd52f..e459cdac0cd7 100644
> --- a/xen/Kconfig
> +++ b/xen/Kconfig
> @@ -84,7 +84,7 @@ config UNSUPPORTED
>  config LTO
>   bool "Link Time Optimisation"
>   depends on BROKEN
> - ---help---
> + help
>     Enable Link Time Optimisation.
>  
>     If unsure, say N.
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index fa81853e9385..61b24ac552cd 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -4,7 +4,7 @@ menu "Debugging Options"
>  config DEBUG
>   bool "Developer Checks"
>   default y
> - ---help---
> + help
>     If you say Y here this will enable developer checks such
> as asserts
>     and extra printks. This option is intended for development
> purposes
>     only, and not for production use.
> @@ -17,14 +17,14 @@ config GDBSX
>   bool "Guest debugging with gdbsx"
>   depends on X86
>   default y
> - ---help---
> + help
>     If you want to enable support for debugging guests from
> dom0 via
>     gdbsx then say Y.
>  
>  config FRAME_POINTER
>   bool "Compile Xen with frame pointers"
>   default DEBUG
> - ---help---
> + help
>     If you say Y here the resulting Xen will be slightly
> larger and
>     maybe slower, but it gives very useful debugging
> information
>     in case of any Xen bugs.
> @@ -33,7 +33,7 @@ config COVERAGE
>   bool "Code coverage support"
>   depends on !LIVEPATCH
>   select SUPPRESS_DUPLICATE_SYMBOL_WARNINGS if
> !ENFORCE_UNIQUE_SYMBOLS
> - ---help---
> + help
>     Enable code coverage support.
>  
>     If unsure, say N here.
> @@ -41,7 +41,7 @@ config COVERAGE
>  config DEBUG_LOCK_PROFILE
>   bool "Lock Profiling"
>   select DEBUG_LOCKS
> - ---help---
> + help
>     Lock profiling allows you to see how often locks are taken
> and blocked.
>     You can use serial console to print (and reset) using 'l'
> and 'L'
>     respectively, or the 'xenlockprof' tool.
> @@ -49,13 +49,13 @@ config DEBUG_LOCK_PROFILE
>  config DEBUG_LOCKS
>   bool "Lock debugging"
>   default DEBUG
> - ---help---
> + help
>     Enable debugging features of lock handling.  Some
> additional
>     checks will be performed when acquiring and releasing
> locks.
>  
>  config PERF_COUNTERS
>   bool "Performance Counters"
> - ---help---
> + help
>     Enables software performance counters that allows you to
> analyze
>     bottlenecks in the system.  To access this data you can
> use serial
>     console to print (and reset) using 'p' and 'P'
> respectively, or
> @@ -64,21 +64,21 @@ config PERF_COUNTERS
>  config PERF_ARRAYS
>   bool "Performance Counter Array Histograms"
>   depends on PERF_COUNTERS
> - ---help---
> + help
>     Enables software performance counter array histograms.
>  
>  
>  config VERBOSE_DEBUG
>   bool "Verbose debug messages"
>   default DEBUG
> - ---help---
> + help
>     Guest output from HYPERVISOR_console_io and hypervisor
> parsing
>     ELF images (dom0) will be logged in the Xen ring buffer.
>  
>  config DEVICE_TREE_DEBUG
>   bool "Device tree debug messages"
>   depends on HAS_DEVICE_TREE
> - ---help---
> + help
>     Device tree parsing and DOM0 device tree building messages
> are
>     logged in the Xen ring buffer.
>     If unsure, say N here.
> @@ 

Re: [PATCH v1.1] xen/commom/dt-overlay: Fix missing lock when remove the device

[Julien Grall]

Hi Henry,

On 26/04/2024 02:55, Henry Wang wrote:

If CONFIG_DEBUG=y, below assertion will be triggered:
(XEN) Assertion 'rw_is_locked(_host_lock)' failed at 
drivers/passthrough/device_tree.c:146
(XEN) [ Xen-4.19-unstable  arm64  debug=y  Not tainted ]
(XEN) CPU:    0
(XEN) PC: 0a257418 iommu_remove_dt_device+0x8c/0xd4
(XEN) LR: 0a2573a0
(XEN) SP: 8000fff7fb30
(XEN) CPSR:   0249 MODE:64-bit EL2h (Hypervisor, handler)
[...]

(XEN) Xen call trace:
(XEN)    [<0a257418>] iommu_remove_dt_device+0x8c/0xd4 (PC)
(XEN)    [<0a2573a0>] iommu_remove_dt_device+0x14/0xd4 (LR)
(XEN)    [<0a20797c>] dt-overlay.c#remove_node_resources+0x8c/0x90
(XEN)    [<0a207f14>] dt-overlay.c#remove_nodes+0x524/0x648
(XEN)    [<0a208460>] dt_overlay_sysctl+0x428/0xc68
(XEN)    [<0a2707f8>] arch_do_sysctl+0x1c/0x2c
(XEN)    [<0a230b40>] do_sysctl+0x96c/0x9ec
(XEN)    [<0a271e08>] traps.c#do_trap_hypercall+0x1e8/0x288
(XEN)    [<0a273490>] do_trap_guest_sync+0x448/0x63c
(XEN)    [<0a25c480>] entry.o#guest_sync_slowpath+0xa8/0xd8
(XEN)
(XEN)
(XEN) 
(XEN) Panic on CPU 0:
(XEN) Assertion 'rw_is_locked(_host_lock)' failed at 
drivers/passthrough/device_tree.c:146
(XEN) 

This is because iommu_remove_dt_device() is called without taking the
dt_host_lock. Fix the issue by taking and releasing the lock properly.

Fixes: 7e5c4a8b86f1 ("xen/arm: Implement device tree node removal 
functionalities")
Signed-off-by: Henry Wang 
---
v1.1:
- Move the unlock position before the check of rc.
---
  xen/common/dt-overlay.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
index 1b197381f6..ab8f43aea2 100644
--- a/xen/common/dt-overlay.c
+++ b/xen/common/dt-overlay.c
@@ -381,7 +381,9 @@ static int remove_node_resources(struct dt_device_node 
*device_node)
  {
  if ( dt_device_is_protected(device_node) )
  {
+write_lock(_host_lock);


Looking at the code, we are not modifying the device_node, so shouldn't 
this be a read_lock()?


That said, even though either fix your issue, I am not entirely 
convinced this is the correct position for the lock. From my 
understanding, dt_host_lock is meant to ensure that the DT node will not 
disappear behind your back. So in theory, shouldn't the lock be taken as 
soon as you get hold of device_node?


Cheers,

--
Julien Grall

[GIT PULL] xen: branch for v6.9-rc7

[Juergen Gross]

Linus,

Please git pull the following tag:

 git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git 
for-linus-6.9a-rc7-tag

xen: branch for v6.9-rc7

It contains two fixes when running as Xen PV guests for issues
introduced in the 6.9 merge window, both related to apic id handling.


Thanks.

Juergen

 arch/x86/xen/enlighten_pv.c | 11 ++-
 arch/x86/xen/smp_pv.c   |  4 ++--
 2 files changed, 12 insertions(+), 3 deletions(-)

Juergen Gross (1):
  x86/xen: return a sane initial apic id when running as PV guest

Thomas Gleixner (1):
  x86/xen/smp_pv: Register the boot CPU APIC properly

Re: Referencing domain struct from interrupt handler

[Jan Beulich]

On 03.05.2024 09:45, Jens Wiklander wrote:
> Hi Xen maintainers,
> 
> In my patch series [XEN PATCH v4 0/5] FF-A notifications [1] I need to
> get a reference to a domain struct from a domain ID and keep it from
> being destroyed while using it in the interrupt handler
> notif_irq_handler() (introduced in the last patch "[XEN PATCH v4 5/5]
> xen/arm: ffa: support notification"). In my previous patch set [2] I
> used get_domain_by_id(), but from the following discussion
> rcu_lock_live_remote_domain_by_id() seems to be a better choice so
> that's what I'm using now in the v4 patch set. The domain lock is held
> during a call to vgic_inject_irq() and unlocked right after.
> 
> While we're reviewing the patch set in [1] I'd like to check the
> approach with rcu_lock_live_remote_domain_by_id() here.
> 
> What do you think? Is using rcu_lock_live_remote_domain_by_id() the
> best approach?

Is it guaranteed that the IRQ handler won't ever run in the context of a
vCPU belonging to the domain in question? If not, why the "remote" form
of the function?

Furthermore, is it guaranteed that the IRQ handler won't interrupt code
fiddling with the domain list? I don't think it is, since
domlist_update_lock isn't acquired in an IRQ-safe manner. Looks like
you need to defer the operation on the domain until softirq or tasklet
context.

Jan

> [1] 
> https://patchew.org/Xen/20240502145645.1201613-1-jens.wiklan...@linaro.org/
> [2] 
> https://patchew.org/Xen/20240426084723.4149648-1-jens.wiklan...@linaro.org/
> 
> Thanks,
> Jens

Re: [XEN PATCH v2 1/3] drivers: char: address violation of MISRA C Rule 20.7

[Nicola Vetrini]

On 2024-05-03 12:10, Jan Beulich wrote:

On 03.05.2024 09:29, Nicola Vetrini wrote:

On 2024-05-01 21:57, Stefano Stabellini wrote:

On Tue, 30 Apr 2024, Nicola Vetrini wrote:

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, 
some

macro definitions should gain additional parentheses to ensure that
all
current and future users will be safe with respect to expansions 
that

can possibly alter the semantics of the passed-in macro parameter.

No functional chage.

Signed-off-by: Nicola Vetrini 


Reviewed-by: Stefano Stabellini 



---
Changes in v2:
- drop excess parentheses from val parameter.
---
 xen/drivers/char/omap-uart.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/omap-uart.c
b/xen/drivers/char/omap-uart.c
index 03b5b66e7acb..e0128225f927 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -48,8 +48,9 @@
 /* System configuration register */
 #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup 
is

enabled */

-#define omap_read(uart, off)   readl((uart)->regs +
(offregs + \
+   ((off) << 
REG_SHIFT))


the alignment looks off but could be fixed on commit



Can you clarify what you mean here? I aligned readl and writeln and 
the

operands in writel to avoid the line being too long.


#define omap_write(uart, off, val) writel(val, \
  (uart)->regs + ((off) << 
REG_SHIFT))


The main point being that before you start splitting an argument 
following

another one on the same line, you'd move that argument to a new line.

Jan


Oh, ok. Didn't think of that. Thanks, can amend it

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

Re: [XEN PATCH v2 1/3] drivers: char: address violation of MISRA C Rule 20.7

[Jan Beulich]

On 03.05.2024 09:29, Nicola Vetrini wrote:
> On 2024-05-01 21:57, Stefano Stabellini wrote:
>> On Tue, 30 Apr 2024, Nicola Vetrini wrote:
>>> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
>>> of macro parameters shall be enclosed in parentheses". Therefore, some
>>> macro definitions should gain additional parentheses to ensure that 
>>> all
>>> current and future users will be safe with respect to expansions that
>>> can possibly alter the semantics of the passed-in macro parameter.
>>>
>>> No functional chage.
>>>
>>> Signed-off-by: Nicola Vetrini 
>>
>> Reviewed-by: Stefano Stabellini 
>>
>>
>>> ---
>>> Changes in v2:
>>> - drop excess parentheses from val parameter.
>>> ---
>>>  xen/drivers/char/omap-uart.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/drivers/char/omap-uart.c 
>>> b/xen/drivers/char/omap-uart.c
>>> index 03b5b66e7acb..e0128225f927 100644
>>> --- a/xen/drivers/char/omap-uart.c
>>> +++ b/xen/drivers/char/omap-uart.c
>>> @@ -48,8 +48,9 @@
>>>  /* System configuration register */
>>>  #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup is 
>>> enabled */
>>>
>>> -#define omap_read(uart, off)   readl((uart)->regs + 
>>> (off<>> -#define omap_write(uart, off, val) writel((val), (uart)->regs + 
>>> (off<>> +#define omap_read(uart, off)   readl((uart)->regs + ((off) << 
>>> REG_SHIFT))
>>> +#define omap_write(uart, off, val) writel(val, (uart)->regs + \
>>> +   ((off) << REG_SHIFT))
>>
>> the alignment looks off but could be fixed on commit
>>
> 
> Can you clarify what you mean here? I aligned readl and writeln and the 
> operands in writel to avoid the line being too long.

#define omap_write(uart, off, val) writel(val, \
  (uart)->regs + ((off) << REG_SHIFT))

The main point being that before you start splitting an argument following
another one on the same line, you'd move that argument to a new line.

Jan

Re: [XEN PATCH v4 5/5] xen/arm: ffa: support notification

[Julien Grall]

Hi Jens,

On 02/05/2024 15:56, Jens Wiklander wrote:

-static bool ffa_probe(void)
+static int __init ffa_init(void)
  {
  uint32_t vers;
  unsigned int major_vers;
@@ -460,16 +511,16 @@ static bool ffa_probe(void)
  printk(XENLOG_ERR
 "ffa: unsupported SMCCC version %#x (need at least %#x)\n",
 smccc_ver, ARM_SMCCC_VERSION_1_2);
-return false;
+return 0;
  }
  
  if ( !ffa_get_version() )

-return false;
+return 0;
  
  if ( vers < FFA_MIN_SPMC_VERSION || vers > FFA_MY_VERSION )

  {
  printk(XENLOG_ERR "ffa: Incompatible version %#x found\n", vers);
-return false;
+return 0;
  }
  
  major_vers = (vers >> FFA_VERSION_MAJOR_SHIFT) & FFA_VERSION_MAJOR_MASK;

@@ -492,26 +543,33 @@ static bool ffa_probe(void)
   !check_mandatory_feature(FFA_MEM_SHARE_32) ||
   !check_mandatory_feature(FFA_MEM_RECLAIM) ||
   !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
-return false;
+return 0;
  
  if ( !ffa_rxtx_init() )

-return false;
+return 0;
  
  ffa_version = vers;
  
  if ( !ffa_partinfo_init() )

  goto err_rxtx_destroy;
  
+ffa_notif_init();

  INIT_LIST_HEAD(_teardown_head);
  init_timer(_teardown_timer, ffa_teardown_timer_callback, NULL, 0);
  
-return true;

+return 0;
  
  err_rxtx_destroy:

  ffa_rxtx_destroy();
  ffa_version = 0;
  
-return false;

+return 0;
+}
+presmp_initcall(ffa_init);
I would rather prefer if tee_init() is called from presmp_initcall(). 
This would avoid FFA to have to try to initialize itself before all the 
others.


This is what I had in mind with my original request, but I guess I 
wasn't clear enough. Sorry for that.


[...]


+static void notif_irq_handler(int irq, void *data)
+{
+const struct arm_smccc_1_2_regs arg = {
+.a0 = FFA_NOTIFICATION_INFO_GET_64,
+};
+struct arm_smccc_1_2_regs resp;
+unsigned int id_pos;
+unsigned int list_count;
+uint64_t ids_count;
+unsigned int n;
+int32_t res;
+
+do {
+arm_smccc_1_2_smc(, );
+res = ffa_get_ret_code();
+if ( res )
+{
+if ( res != FFA_RET_NO_DATA )
+printk(XENLOG_ERR "ffa: notification info get failed: error 
%d\n",
+   res);
+return;
+}
+
+ids_count = resp.a2 >> FFA_NOTIF_INFO_GET_ID_LIST_SHIFT;
+list_count = ( resp.a2 >> FFA_NOTIF_INFO_GET_ID_COUNT_SHIFT ) &
+ FFA_NOTIF_INFO_GET_ID_COUNT_MASK;
+
+id_pos = 0;
+for ( n = 0; n < list_count; n++ )
+{
+unsigned int count = ((ids_count >> 2 * n) & 0x3) + 1;
+uint16_t vm_id = get_id_from_resp(, id_pos);
+struct domain *d;
+
+/*
+ * vm_id == 0 means a notifications pending for Xen itself, but
+ * we don't support that yet.
+ */
+if (vm_id)
+d = ffa_rcu_lock_domain_by_vm_id(vm_id);


I am still unconvinced that this is sufficient. This will prevent 
"struct domain *" to be freed. But ...



+else
+d = NULL;
+
+if ( d )
+{
+struct ffa_ctx *ctx = d->arch.tee;


... I don't think it will protect d->arch.tee to be freed as this is 
happening during teardorwn. You mostly need some other sort of locking 
to avoid d->arch.tee been freed behind your back.



+struct vcpu *v;
+
+ACCESS_ONCE(ctx->notif.secure_pending) = true;
+
+/*
+ * Since we're only delivering global notification, always
+ * deliver to the first online vCPU. It doesn't matter
+ * which we chose, as long as it's available.
+ */
+for_each_vcpu(d, v)
+{
+if ( is_vcpu_online(v) )
+{
+vgic_inject_irq(d, v, GUEST_FFA_NOTIF_PEND_INTR_ID,
+true);
+break;
+}
+}
+if ( !v )
+printk(XENLOG_ERR "ffa: can't inject NPI, all vCPUs 
offline\n");
+
+rcu_unlock_domain(d);
+}
+
+id_pos += count;
+}
+
+} while (resp.a2 & FFA_NOTIF_INFO_GET_MORE_FLAG);
+}


[...]


+static int ffa_setup_irq_callback(struct notifier_block *nfb,
+  unsigned long action, void *hcpu)
+{
+unsigned int cpu = (unsigned long)hcpu;
+struct notif_irq_info irq_info = { };
+
+switch ( action )
+{
+case CPU_ONLINE:


Can't you execute the notifier in CPU_STARTING? This will be called on 
the CPU directly, so you should be able to use request_irq(...).



+/*
+ * The notifier call is done on the primary or 

[XEN PATCH 1/2] docs/misra: add Terms & Definitions section to rules.rst

[Federico Serafini]

Add a section for terms and definitions used by MISRA but expressed
in terms of the C specification.

Add a definition of "switch clause" to the newly-introduced section.

Link the first use of the term "switch clause" in the document to its
definition.

Suggested-by: Jan Beulich 
Signed-off-by: Federico Serafini 
---
Jan you were not completely satisfied by the definition but I didn't find
a better one.
---
 docs/misra/rules.rst | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
index b7b447e152..d3b70fdf04 100644
--- a/docs/misra/rules.rst
+++ b/docs/misra/rules.rst
@@ -489,8 +489,7 @@ maintainers if you want to suggest a change.
 
* - `Rule 16.3 
`_
  - Required
- - An unconditional break statement shall terminate every
-   switch-clause
+ - An unconditional break statement shall terminate every switch-clause_
  - In addition to break, also other unconditional flow control statements
such as continue, return, goto are allowed.
 
@@ -712,3 +711,14 @@ maintainers if you want to suggest a change.
  - The value of a pointer to a FILE shall not be used after the associated
stream has been closed
  -
+
+Terms & Definitions
+---
+
+.. _switch-clause:
+
+A *switch clause* can be defined as:
+"the non-empty list of statements which follows a non-empty list of
+case/default labels".
+A formal definition is available within the amplification of MISRA C:2012
+Rule 16.1.
-- 
2.34.1

[XEN PATCH 2/2] automation/eclair: add deviation for Rule 16.4

[Federico Serafini]

MISRA C:2012 Rule 16.4 states that "Every switch statement shall have a
default label".
Update ECLAIR configuration to take into account the deviations
agreed during MISRA meetings.

Signed-off-by: Federico Serafini 
---
 automation/eclair_analysis/ECLAIR/deviations.ecl |  8 
 docs/misra/deviations.rst| 13 +
 2 files changed, 21 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index d21f112a9b..f09ad71acf 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -384,6 +384,14 @@ explicit comment indicating the fallthrough intention is 
present."
 -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* 
[fF]all ?through.? \\*/.*$,0..1"}
 -doc_end
 
+-doc_begin="Switch statements having a controlling expression of enum type 
deliberately do not have a default case: gcc -Wall enables -Wswitch which warns 
(and breaks the build as we use -Werror) if one of the enum labels is missing 
from the switch."
+-config=MC3R1.R16.4,reports+={deliberate,'any_area(kind(context)&&^.* has no 
`default.*$&(node(switch_stmt)&(cond,skip(__non_syntactic_paren_stmts,type(canonical(enum_underlying_type(any('}
+-doc_end
+
+-doc_begin="A switch statement with a single switch clause and no default 
label may be used in place of an equivalent if statement if it is considered to 
improve readability."
+-config=MC3R1.R16.4,switch_clauses+={deliberate,"switch(1)&(0)"}
+-doc_end
+
 -doc_begin="A switch statement with a single switch clause and no default 
label may be used in place of an equivalent if statement if it is considered to 
improve readability."
 -config=MC3R1.R16.6,switch_clauses+={deliberate, "default(0)"}
 -doc_end
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index ed0c1e8ed0..39cc321a27 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -334,6 +334,19 @@ Deviations related to MISRA C:2012 Rules:
  - /\* Fallthrough \*/
  - /\* Fallthrough. \*/
 
+   * - R16.4
+ - Switch statements having a controlling expression of enum type
+   deliberately do not have a default case: gcc -Wall enables -Wswitch
+   which warns (and breaks the build as we use -Werror) if one of the enum
+   labels is missing from the switch.
+ - Tagged as `deliberate` for ECLAIR.
+
+   * - R16.4
+ - A switch statement with a single switch clause and no default label may
+   be used in place of an equivalent if statement if it is considered to
+   improve readability.
+ - Tagged as `deliberate` for ECLAIR.
+
* - R16.6
  - A switch statement with a single switch clause and no default label may
be used in place of an equivalent if statement if it is considered to
-- 
2.34.1

[XEN PATCH 0/2] misra: deviations of Rule 16.4

[Federico Serafini]

Define "switch-clause" in terms of the C specification.
Deviate Rule 16.4.

Federico Serafini (2):
  docs/misra: add Terms & Definitions section to rules.rst
  automation/eclair: add deviation for Rule 16.4

 automation/eclair_analysis/ECLAIR/deviations.ecl |  8 
 docs/misra/deviations.rst| 13 +
 docs/misra/rules.rst | 14 --
 3 files changed, 33 insertions(+), 2 deletions(-)

-- 
2.34.1

Re: [PATCH v6 8/8] xen: allow up to 16383 cpus

[Julien Grall]

Hi Stefano,

On 02/05/2024 19:13, Stefano Stabellini wrote:

On Mon, 29 Apr 2024, Julien Grall wrote:

Hi Juergen,

On 29/04/2024 12:28, Jürgen Groß wrote:

On 29.04.24 13:04, Julien Grall wrote:

Hi Juergen,

Sorry for the late reply.

On 29/04/2024 11:33, Juergen Gross wrote:

On 08.04.24 09:10, Jan Beulich wrote:

On 27.03.2024 16:22, Juergen Gross wrote:

With lock handling now allowing up to 16384 cpus (spinlocks can
handle
65535 cpus, rwlocks can handle 16384 cpus), raise the allowed limit
for
the number of cpus to be configured to 16383.

The new limit is imposed by IOMMU_CMD_BUFFER_MAX_ENTRIES and
QINVAL_MAX_ENTRY_NR required to be larger than 2 * CONFIG_NR_CPUS.

Signed-off-by: Juergen Gross 


Acked-by: Jan Beulich 

I'd prefer this to also gain an Arm ack, though.


Any comment from Arm side?


Can you clarify what the new limits mean in term of (security) support?
Are we now claiming that Xen will work perfectly fine on platforms with up
to 16383?

If so, I can't comment for x86, but for Arm, I am doubtful that it would
work without any (at least performance) issues. AFAIK, this is also an
untested configuration. In fact I would be surprised if Xen on Arm was
tested with more than a couple of hundreds cores (AFAICT the Ampere CPUs
has 192 CPUs).


I think we should add a security support limit for the number of physical
cpus similar to the memory support limit we already have in place.

For x86 I'd suggest 4096 cpus for security support (basically the limit we
have with this patch), but I'm open for other suggestions, too.

I have no idea about any sensible limits for Arm32/Arm64.


I am not entirely. Bertrand, Michal, Stefano, should we use 192 (the number of
CPUs from Ampere)?


I am OK with that. If we want to be a bit more future proof we could say
256 or 512.


Sorry, I don't follow your argument. A limit can be raised at time point 
in the future. The question is more whether we are confident that Xen on 
Arm will run well if a user has a platform with 256/512 pCPUs.


So are you saying that from Xen point of view, you are expecting no 
difference between 256 and 512. And therefore you would be happy if to 
backport patches if someone find differences (or even security issues) 
when using > 256 pCPUs?


Cheers,

--
Julien Grall

Re: [XEN PATCH] xen/mem_access: address violations of MISRA C: 2012 Rule 8.4

[Julien Grall]

Hi,

On 03/05/2024 08:09, Alessandro Zucchelli wrote:

On 2024-04-29 17:58, Jan Beulich wrote:

On 29.04.2024 17:45, Alessandro Zucchelli wrote:

Change #ifdef CONFIG_MEM_ACCESS by OR-ing defined(CONFIG_ARM),
allowing asm/mem_access.h to be included in all ARM build 
configurations.

This is to address the violation of MISRA C: 2012 Rule 8.4 which states:
"A compatible declaration shall be visible when an object or function
with external linkage is defined". Functions p2m_mem_access_check
and p2m_mem_access_check_and_get_page when CONFIG_MEM_ACCESS is not
defined in ARM builds don't have visible declarations in the file
containing their definitions.

Signed-off-by: Alessandro Zucchelli 
---
 xen/include/xen/mem_access.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/xen/mem_access.h b/xen/include/xen/mem_access.h
index 87d93b31f6..ec0630677d 100644
--- a/xen/include/xen/mem_access.h
+++ b/xen/include/xen/mem_access.h
@@ -33,7 +33,7 @@
  */
 struct vm_event_st;

-#ifdef CONFIG_MEM_ACCESS
+#if defined(CONFIG_MEM_ACCESS) || defined(CONFIG_ARM)
 #include 
 #endif


This doesn't look quite right. If Arm supports mem-access, why would it
not set MEM_ACCESS=y? Whereas if it's only stubs that Arm supplies, then
those would better move here, thus eliminating the need for a per-arch
stub header (see what was e.g. done for numa.h). This way RISC-V and PPC
(and whatever is to come) would then be taken care of as well.

ARM does support mem-access, so I don't think this is akin to the 
changes done to handle numa.h.
ARM also allows users to set MEM_ACCESS=n (e.g. 
xen/arch/arm/configs/tiny64_defconfig) and builds just fine; however, 
the implementation file mem_access.c is compiled unconditionally in 
ARM's makefile, hence why the violation was spotted.
This is a bit unusual, so I was also hoping to get some feedback from 
mem-access maintainers as to why this discrepancy from x86 exists. I 
probably should have also included some ARM maintainers as well, so I'm 
going to loop them in now.


An alternative option I think is to make the compilation of arm's 
mem_access.c conditional on CONFIG_MEM_ACCESS (as for x86/mm and common).


I can't think of a reason to have mem_access.c unconditional compiled 
in. So I think it should be conditional like on x86.


Cheers,

--
Julien Grall

[xen-unstable test] 185906: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

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

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

version targeted for testing:
 xen  feb9158a620040846d76981acbe8ea9e2255a07b
baseline version:
 xen  feb9158a620040846d76981acbe8ea9e2255a07b

Last test of basis   185906  2024-05-03 01:53:38 Z0 days
Testing same since  (not found) 0 attempts

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

Re: [PATCH v6 3/7] x86/hvm: Allow access to registers on the same page as MSI-X table

[Roger Pau Monné]

On Fri, Apr 26, 2024 at 07:54:00PM +0200, Marek Marczykowski-Górecki wrote:
> Some devices (notably Intel Wifi 6 AX210 card) keep auxiliary registers
> on the same page as MSI-X table. Device model (especially one in
> stubdomain) cannot really handle those, as direct writes to that page is
> refused (page is on the mmio_ro_ranges list). Instead, extend
> msixtbl_mmio_ops to handle such accesses too.
> 
> Doing this, requires correlating read/write location with guest
> of MSI-X table address. Since QEMU doesn't map MSI-X table to the guest,
  ^ extra 'of'?
> it requires msixtbl_entry->gtable, which is HVM-only. Similar feature
> for PV would need to be done separately.
> 
> This will be also used to read Pending Bit Array, if it lives on the same
> page, making QEMU not needing /dev/mem access at all (especially helpful
> with lockdown enabled in dom0). If PBA lives on another page, QEMU will
> map it to the guest directly.
> If PBA lives on the same page, discard writes and log a message.
> Technically, writes outside of PBA could be allowed, but at this moment
> the precise location of PBA isn't saved, and also no known device abuses
> the spec in this way (at least yet).
> 
> To access those registers, msixtbl_mmio_ops need the relevant page
> mapped. MSI handling already has infrastructure for that, using fixmap,
> so try to map first/last page of the MSI-X table (if necessary) and save
> their fixmap indexes. Note that msix_get_fixmap() does reference
> counting and reuses existing mapping, so just call it directly, even if
> the page was mapped before. Also, it uses a specific range of fixmap
> indexes which doesn't include 0, so use 0 as default ("not mapped")
> value - which simplifies code a bit.
> 
> GCC 12.2.1 gets confused about 'desc' variable:
> 
> arch/x86/hvm/vmsi.c: In function ‘msixtbl_range’:
> arch/x86/hvm/vmsi.c:553:8: error: ‘desc’ may be used uninitialized 
> [-Werror=maybe-uninitialized]
>   553 | if ( desc )
>   |^
> arch/x86/hvm/vmsi.c:537:28: note: ‘desc’ was declared here
>   537 | const struct msi_desc *desc;
>   |^~~~
> 
> It's conditional initialization is actually correct (in the case where
> it isn't initialized, function returns early), but to avoid
> build failure initialize it explicitly to NULL anyway.
> 
> Signed-off-by: Marek Marczykowski-Górecki 
> ---
> Changes in v6:
> - use MSIX_CHECK_WARN macro
> - extend assert on fixmap_idx
> - add break in default label, after ASSERT_UNREACHABLE(), and move
>   setting default there
> - style fixes
> Changes in v5:
> - style fixes
> - include GCC version in the commit message
> - warn only once (per domain, per device) about failed adjacent access
> Changes in v4:
> - drop same_page parameter of msixtbl_find_entry(), distinguish two
>   cases in relevant callers
> - rename adj_access_table_idx to adj_access_idx
> - code style fixes
> - drop alignment check in adjacent_{read,write}() - all callers already
>   have it earlier
> - delay mapping first/last MSI-X pages until preparing device for a
>   passthrough
> v3:
>  - merge handling into msixtbl_mmio_ops
>  - extend commit message
> v2:
>  - adjust commit message
>  - pass struct domain to msixtbl_page_handler_get_hwaddr()
>  - reduce local variables used only once
>  - log a warning if write is forbidden if MSI-X and PBA lives on the same
>page
>  - do not passthrough unaligned accesses
>  - handle accesses both before and after MSI-X table
> ---
>  xen/arch/x86/hvm/vmsi.c| 200 --
>  xen/arch/x86/include/asm/msi.h |   5 +-
>  xen/arch/x86/msi.c |  41 +++-
>  3 files changed, 236 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
> index 17983789..230e3a5dee3f 100644
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -180,6 +180,10 @@ static bool msixtbl_initialised(const struct domain *d)
>  return d->arch.hvm.msixtbl_list.next;
>  }
>  
> +/*
> + * Lookup an msixtbl_entry on the same page as given addr. It's up to the
> + * caller to check if address is strictly part of the table - if relevant.
> + */
>  static struct msixtbl_entry *msixtbl_find_entry(
>  struct vcpu *v, unsigned long addr)
>  {
> @@ -187,8 +191,8 @@ static struct msixtbl_entry *msixtbl_find_entry(
>  struct domain *d = v->domain;
>  
>  list_for_each_entry( entry, >arch.hvm.msixtbl_list, list )
> -if ( addr >= entry->gtable &&
> - addr < entry->gtable + entry->table_len )
> +if ( PFN_DOWN(addr) >= PFN_DOWN(entry->gtable) &&
> + PFN_DOWN(addr) <= PFN_DOWN(entry->gtable + entry->table_len - 
> 1) )
>  return entry;
>  
>  return NULL;
> @@ -213,6 +217,138 @@ static struct msi_desc *msixtbl_addr_to_desc(
>  return NULL;
>  }
>  
> +/*
> + * Returns:
> + *  - ADJACENT_DONT_HANDLE if no handling should be done
> + *  - 

Referencing domain struct from interrupt handler

[Jens Wiklander]

Hi Xen maintainers,

In my patch series [XEN PATCH v4 0/5] FF-A notifications [1] I need to
get a reference to a domain struct from a domain ID and keep it from
being destroyed while using it in the interrupt handler
notif_irq_handler() (introduced in the last patch "[XEN PATCH v4 5/5]
xen/arm: ffa: support notification"). In my previous patch set [2] I
used get_domain_by_id(), but from the following discussion
rcu_lock_live_remote_domain_by_id() seems to be a better choice so
that's what I'm using now in the v4 patch set. The domain lock is held
during a call to vgic_inject_irq() and unlocked right after.

While we're reviewing the patch set in [1] I'd like to check the
approach with rcu_lock_live_remote_domain_by_id() here.

What do you think? Is using rcu_lock_live_remote_domain_by_id() the
best approach?

[1] https://patchew.org/Xen/20240502145645.1201613-1-jens.wiklan...@linaro.org/
[2] https://patchew.org/Xen/20240426084723.4149648-1-jens.wiklan...@linaro.org/

Thanks,
Jens

Re: [XEN PATCH v2 1/3] drivers: char: address violation of MISRA C Rule 20.7

[Nicola Vetrini]

On 2024-05-01 21:57, Stefano Stabellini wrote:

On Tue, 30 Apr 2024, Nicola Vetrini wrote:

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that 
all

current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional chage.

Signed-off-by: Nicola Vetrini 


Reviewed-by: Stefano Stabellini 



---
Changes in v2:
- drop excess parentheses from val parameter.
---
 xen/drivers/char/omap-uart.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/omap-uart.c 
b/xen/drivers/char/omap-uart.c

index 03b5b66e7acb..e0128225f927 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -48,8 +48,9 @@
 /* System configuration register */
 #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup is 
enabled */


-#define omap_read(uart, off)   readl((uart)->regs + 
(off<-#define omap_write(uart, off, val) writel((val), (uart)->regs + 
(off<+#define omap_read(uart, off)   readl((uart)->regs + ((off) << 
REG_SHIFT))

+#define omap_write(uart, off, val) writel(val, (uart)->regs + \
+   ((off) << REG_SHIFT))


the alignment looks off but could be fixed on commit



Can you clarify what you mean here? I aligned readl and writeln and the 
operands in writel to avoid the line being too long.


Thanks,

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

Re: [XEN PATCH] xen/mem_access: address violations of MISRA C: 2012 Rule 8.4

[Alessandro Zucchelli]

On 2024-04-29 17:58, Jan Beulich wrote:

On 29.04.2024 17:45, Alessandro Zucchelli wrote:

Change #ifdef CONFIG_MEM_ACCESS by OR-ing defined(CONFIG_ARM),
allowing asm/mem_access.h to be included in all ARM build 
configurations.
This is to address the violation of MISRA C: 2012 Rule 8.4 which 
states:

"A compatible declaration shall be visible when an object or function
with external linkage is defined". Functions p2m_mem_access_check
and p2m_mem_access_check_and_get_page when CONFIG_MEM_ACCESS is not
defined in ARM builds don't have visible declarations in the file
containing their definitions.

Signed-off-by: Alessandro Zucchelli 
---
 xen/include/xen/mem_access.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/xen/mem_access.h 
b/xen/include/xen/mem_access.h

index 87d93b31f6..ec0630677d 100644
--- a/xen/include/xen/mem_access.h
+++ b/xen/include/xen/mem_access.h
@@ -33,7 +33,7 @@
  */
 struct vm_event_st;

-#ifdef CONFIG_MEM_ACCESS
+#if defined(CONFIG_MEM_ACCESS) || defined(CONFIG_ARM)
 #include 
 #endif


This doesn't look quite right. If Arm supports mem-access, why would it
not set MEM_ACCESS=y? Whereas if it's only stubs that Arm supplies, 
then

those would better move here, thus eliminating the need for a per-arch
stub header (see what was e.g. done for numa.h). This way RISC-V and 
PPC

(and whatever is to come) would then be taken care of as well.

ARM does support mem-access, so I don't think this is akin to the 
changes done to handle numa.h.
ARM also allows users to set MEM_ACCESS=n (e.g. 
xen/arch/arm/configs/tiny64_defconfig) and builds just fine; however, 
the implementation file mem_access.c is compiled unconditionally in 
ARM's makefile, hence why the violation was spotted.
This is a bit unusual, so I was also hoping to get some feedback from 
mem-access maintainers as to why this discrepancy from x86 exists. I 
probably should have also included some ARM maintainers as well, so I'm 
going to loop them in now.


An alternative option I think is to make the compilation of arm's 
mem_access.c conditional on CONFIG_MEM_ACCESS (as for x86/mm and 
common).


--
Alessandro Zucchelli, B.Sc.

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