Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
Hi Bertrand, On Wed, Apr 10, 2024 at 6:30 PM Bertrand Marquis wrote: > > Hi Jens, > > > On 10 Apr 2024, at 17:45, Jens Wiklander wrote: > > > > On Tue, Apr 9, 2024 at 5:36 PM Jens Wiklander > > wrote: > >> > >> Add support for FF-A notifications, currently limited to an SP (Secure > >> Partition) sending an asynchronous notification to a guest. > >> > >> Guests and Xen itself are made aware of pending notifications with an > >> interrupt. The interrupt handler retrieves the notifications using the > >> FF-A ABI and deliver them to their destinations. > >> > >> Signed-off-by: Jens Wiklander > >> --- [snip] > >> +case FFA_FEATURE_NOTIF_PEND_INTR: > >> +if ( ctx->notif.enabled ) > >> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0); > >> +else > >> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED); > >> +break; > >> +case FFA_FEATURE_SCHEDULE_RECV_INTR: > >> +if ( ctx->notif.enabled ) > >> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0); > >> +else > >> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED); > >> +break; > > > > With the recently posted kernel patch > > https://lore.kernel.org/all/20240410-ffa_npi_support-v1-3-1a5223391...@arm.com/ > > we need to decide which feature interrupt to return since the kernel > > will only install a handle for the first it finds. Right now I propose > > to to not report FFA_FEATURE_SCHEDULE_RECV_INTR. When the time comes > > to use a secondary scheduler we'll need to report the SRI instead. > > > We just had a meeting with Sudeep to discuss that matter and he agreed that > he would register the interrupt handler for all the interrupts available so > that > we can keep a model where we will generate SRIs only to a secondary scheduler > and NPI for notification interrupts (so that the VM does not do a INFO_GET > when > not required). > > We will have to report both as any VM could act as secondary scheduler for SPs > in theory (we might need at some point a parameter for that) but for now those > should only be generated to Dom0 if there are pending notifications for SPs. OK, thanks. I'll keep both then. Cheers, Jens
Re: [PATCH] xen/spinlock: Adjust LOCK_DEBUG_INITVAL to placate MISRA
On 2024-04-10 21:35, Andrew Cooper wrote:
Resolves an R7.2 violation.
Thanks, I was going to suggest the same change. This will resolve the
failure of the CI MISRA analysis on GitLab.
Reviewed-by: Nicola Vetrini
Fixes: c286bb93d20c ("xen/spinlock: support higher number of cpus")
Signed-off-by: Andrew Cooper
---
CC: George Dunlap
CC: Jan Beulich
CC: Stefano Stabellini
CC: Julien Grall
CC: Juergen Gross
CC: consult...@bugseng.com
CC: Roberto Bagnara
CC: Federico Serafini
CC: Nicola Vetrini
---
xen/include/xen/spinlock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index db00a24646bd..18793c5e29cb 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -13,7 +13,7 @@
#ifdef CONFIG_DEBUG_LOCKS
union lock_debug {
uint32_t val;
-#define LOCK_DEBUG_INITVAL 0x
+#define LOCK_DEBUG_INITVAL 0xU
struct {
unsigned int cpu:SPINLOCK_CPU_BITS;
#define LOCK_DEBUG_PAD_BITS (30 - SPINLOCK_CPU_BITS)
base-commit: 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9
prerequisite-patch-id: 8d06e56c5d8a52f1387e1f5a7fce6a94b8c4a1ed
prerequisite-patch-id: 13355d26254b979c79de456c9a6ea6a9c639ba63
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
[linux-linus test] 185290: tolerable FAIL - PUSHED
flight 185290 linux-linus real [real] flight 185305 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/185290/ http://logs.test-lab.xenproject.org/osstest/logs/185305/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-qemuu-nested-intel 20 debian-hvm-install/l1/l2 fail pass in 185305-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 185270 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 185270 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 185270 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 185270 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 185270 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 185270 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-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-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 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-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-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 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass version targeted for testing: linux2c71fdf02a95b3dd425b42f28fd47fb2b1d22702 baseline version: linuxfec50db7033ea478773b159e0e2efb135270e3b7 Last test of basis 185270 2024-04-07 20:42:12 Z3 days Failing since185275 2024-04-08 23:43:57 Z2 days3 attempts Testing same since 185290 2024-04-09 18:43:05 Z1 days1 attempts People who touched revisions under test: Boris Burkov Daniel Sneddon Dave Airlie Dave Airlie David Sterba Josh Poimboeuf Linus Torvalds Mike Rapoport (IBM) Pawan Gupta Thomas Gleixner Thorsten Blum Wei Yang jobs: build-amd64-xsm pass build-arm64-xsm
Re: Serious AMD-Vi(?) issue
On Thu, Mar 28, 2024 at 07:25:02AM +0100, Jan Beulich wrote: > On 27.03.2024 18:27, Elliott Mitchell wrote: > > On Mon, Mar 25, 2024 at 02:43:44PM -0700, Elliott Mitchell wrote: > >> On Mon, Mar 25, 2024 at 08:55:56AM +0100, Jan Beulich wrote: > >>> > >>> In fact when running into trouble, the usual course of action would be to > >>> increase verbosity in both hypervisor and kernel, just to make sure no > >>> potentially relevant message is missed. > >> > >> More/better information might have been obtained if I'd been engaged > >> earlier. > > > > This is still true, things are in full mitigation mode and I'll be > > quite unhappy to go back with experiments at this point. > > Well, it very likely won't work without further experimenting by someone > able to observe the bad behavior. Recall we're on xen-devel here; it is > kind of expected that without clear (and practical) repro instructions > experimenting as well as info collection will remain with the reporter. After looking at the situation and considering the issues, I /may/ be able to setup for doing more testing. I guess I should confirm, which of those criteria do you think currently provided information fails at? AMD-IOMMU + Linux MD RAID1 + dual Samsung SATA (or various NVMe) + dbench; seems a pretty specific setup. I could see this being criticised as impractical if /new/ devices were required, but the confirmed flash devices are several generations old. Difficulty is cheaper candidate devices are being recycled for their precious metal content, rather than resold as used. -- (\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/) \BS (| ehem+sig...@m5p.com PGP 87145445 |) / \_CS\ | _ -O #include O- _ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Re: [PATCH v1 0/2] Starting AMD SEV work
On Wed, Apr 10, 2024 at 05:36:34PM +0200, Andrei Semenov wrote: > This patch series initiate work on AMD SEV technology implementation in Xen. > SEV stands for "Secure Encrypted Virtualization" and allows the memory > contents > of a VM to be encrypted with a key unique to this VM. In this way the neither > other VMs nor hypervisor can't read the memory content of this "encrypted" > VM. > > In order to create and to run such a VM different layers of software must > interact (bascally Xen hypevisor, Xen toolstack in dom0 and the encrypted VM > itself). > > In this work we start with discovering and enabling SEV feature on the > platform. > The second patch ports AMD Secure Processor driver on Xen. This AMD Secure > Processor device (a.k.a PSP) is the way the different software layers interact > with AMD firmware/hardware to manage and run the encrypted VM. How will that interact with the PSP driver in dom0? AFAIK amdgpu driver uses PSP for loading the GPU firmware. Does it mean one need to choose either GPU in dom0 or encrypted VMs, or is it going to work somehow together? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
Re: [PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments
On Wed, 10 Apr 2024 at 19:47, Stefano Stabellini wrote: > xen_ulong_t is widely used in public headers. > > Signed-off-by: Stefano Stabellini > --- > > Given that xen_ulong_t is used in public headers there could be a better > place for documenting it but this was the most straightforward to add. > --- > docs/misra/C-language-toolchain.rst | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/docs/misra/C-language-toolchain.rst > b/docs/misra/C-language-toolchain.rst > index 5ddfe7bdbe..7a334260e6 100644 > --- a/docs/misra/C-language-toolchain.rst > +++ b/docs/misra/C-language-toolchain.rst > @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is > below: > - 64 bits > - x86_64, ARMv8-A AArch64, RV64, PPC64 > > + * - xen_ulong_t > + - 32 bits > + - 32 bits > + - x86_32 > + > + * - xen_ulong_t > + - 64 bits > + - 64 bits > + - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R > + AArch32, ARMv7-A We support neither ARMv8-R nor ARMv8-A Aarch32. I could possibly accept the latter because it works to. But the former is so far misleading. Cheers, > + > * - long long > - 64-bit > - 32-bit > -- > 2.25.1 > >
[PATCH v2 3/4] xen/public: s/unsigned long/xen_ulong_t
The goal is to use only fixed-size integers in public headers, such as
uint32_t and uint64_t.
However, there are cases where the ABI changes depending on the
architecture. In those cases, adding #ifdefs might be the clearest
solution but it is also cumbersome. We already define a xen_ulong_t type
which is widely used in public headers and it is defined differently by
architecture.
Instead of unsigned long, use xen_ulong_t in public headers:
- it makes it clearer that size might change by arch
- it gets us closer to the goal of no unfixed-size integers in public
headers
Note that unsigned long and xen_ulong_t are the same thing on x86 (both
32-bit and 64-bit) but they differ on all other arches. However, all
the interfaces with xen_ulong_t or unsigned long are x86-only interfaces
today. Thus, this patch doesn't introduce any ABI or semantic changes.
Signed-off-by: Stefano Stabellini
---
xen/include/public/kexec.h | 10 +-
xen/include/public/nmi.h | 4 ++--
xen/include/public/physdev.h | 2 +-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index 8d2a0ef697..89164094d9 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -78,10 +78,10 @@
typedef struct xen_kexec_image {
#if defined(__i386__) || defined(__x86_64__)
-unsigned long page_list[KEXEC_XEN_NO_PAGES];
+xen_ulong_t page_list[KEXEC_XEN_NO_PAGES];
#endif
-unsigned long indirection_page;
-unsigned long start_address;
+xen_ulong_t indirection_page;
+xen_ulong_t start_address;
} xen_kexec_image_t;
/*
@@ -145,8 +145,8 @@ typedef struct xen_kexec_load_v1 {
typedef struct xen_kexec_range {
int32_t range;
int32_t nr;
-unsigned long size;
-unsigned long start;
+xen_ulong_t size;
+xen_ulong_t start;
} xen_kexec_range_t;
#if __XEN_INTERFACE_VERSION__ >= 0x00040400
diff --git a/xen/include/public/nmi.h b/xen/include/public/nmi.h
index 5900703f5f..c8c0ddafc2 100644
--- a/xen/include/public/nmi.h
+++ b/xen/include/public/nmi.h
@@ -43,8 +43,8 @@
*/
#define XENNMI_register_callback 0
struct xennmi_callback {
-unsigned long handler_address;
-unsigned long pad;
+xen_ulong_t handler_address;
+xen_ulong_t pad;
};
typedef struct xennmi_callback xennmi_callback_t;
DEFINE_XEN_GUEST_HANDLE(xennmi_callback_t);
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index 03ccf86618..5bbb809fae 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -109,7 +109,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_set_iobitmap_t);
#define PHYSDEVOP_apic_write 9
struct physdev_apic {
/* IN */
-unsigned long apic_physbase;
+xen_ulong_t apic_physbase;
uint32_t reg;
/* IN or OUT */
uint32_t value;
--
2.25.1
[PATCH v2 4/4] xen/public: replace remaining int and long
Replace all int and long in few remaining x86-specific headers and
x86-specific hypercalls.
Signed-off-by: Stefano Stabellini
---
xen/include/public/arch-x86/hvm/save.h | 4 +-
xen/include/public/arch-x86/xen-x86_32.h | 10 ++---
xen/include/public/arch-x86/xen-x86_64.h | 10 ++---
xen/include/public/arch-x86/xen.h| 50
xen/include/public/xen.h | 24 ++--
5 files changed, 49 insertions(+), 49 deletions(-)
diff --git a/xen/include/public/arch-x86/hvm/save.h
b/xen/include/public/arch-x86/hvm/save.h
index 7ecacadde1..8e3dfcf439 100644
--- a/xen/include/public/arch-x86/hvm/save.h
+++ b/xen/include/public/arch-x86/hvm/save.h
@@ -415,7 +415,7 @@ struct hvm_hw_pci_irqs {
* Indexed by: device*4 + INTx#.
*/
union {
-unsigned long i[16 / sizeof (unsigned long)]; /* DECLARE_BITMAP(i,
32*4); */
+xen_ulong_t i[16 / sizeof (xen_ulong_t)]; /* DECLARE_BITMAP(i, 32*4);
*/
uint64_t pad[2];
};
};
@@ -428,7 +428,7 @@ struct hvm_hw_isa_irqs {
* Indexed by ISA IRQ (assumes no ISA-device IRQ sharing).
*/
union {
-unsigned long i[1]; /* DECLARE_BITMAP(i, 16); */
+xen_ulong_t i[1]; /* DECLARE_BITMAP(i, 16); */
uint64_t pad[1];
};
};
diff --git a/xen/include/public/arch-x86/xen-x86_32.h
b/xen/include/public/arch-x86/xen-x86_32.h
index 9e3bf06b12..0b2671ab65 100644
--- a/xen/include/public/arch-x86/xen-x86_32.h
+++ b/xen/include/public/arch-x86/xen-x86_32.h
@@ -68,7 +68,7 @@
#define MACH2PHYS_VIRT_ENDxen_mk_ulong(__MACH2PHYS_VIRT_END)
#define MACH2PHYS_NR_ENTRIES ((MACH2PHYS_VIRT_END-MACH2PHYS_VIRT_START)>>2)
#ifndef machine_to_phys_mapping
-#define machine_to_phys_mapping ((unsigned long *)MACH2PHYS_VIRT_START)
+#define machine_to_phys_mapping ((xen_ulong_t *)MACH2PHYS_VIRT_START)
#endif
/* 32-/64-bit invariability for control interfaces (domctl/sysctl). */
@@ -151,14 +151,14 @@ DEFINE_XEN_GUEST_HANDLE(cpu_user_regs_t);
#define xen_cr3_to_pfn(cr3) (((unsigned)(cr3) >> 12) | ((unsigned)(cr3) << 20))
struct arch_vcpu_info {
-unsigned long cr2;
-unsigned long pad[5]; /* sizeof(vcpu_info_t) == 64 */
+xen_ulong_t cr2;
+xen_ulong_t pad[5]; /* sizeof(vcpu_info_t) == 64 */
};
typedef struct arch_vcpu_info arch_vcpu_info_t;
struct xen_callback {
-unsigned long cs;
-unsigned long eip;
+xen_ulong_t cs;
+xen_ulong_t eip;
};
typedef struct xen_callback xen_callback_t;
diff --git a/xen/include/public/arch-x86/xen-x86_64.h
b/xen/include/public/arch-x86/xen-x86_64.h
index 43f6e3d220..bd27fc59fe 100644
--- a/xen/include/public/arch-x86/xen-x86_64.h
+++ b/xen/include/public/arch-x86/xen-x86_64.h
@@ -67,7 +67,7 @@
#define MACH2PHYS_VIRT_ENDxen_mk_ulong(__MACH2PHYS_VIRT_END)
#define MACH2PHYS_NR_ENTRIES ((MACH2PHYS_VIRT_END-MACH2PHYS_VIRT_START)>>3)
#ifndef machine_to_phys_mapping
-#define machine_to_phys_mapping ((unsigned long *)HYPERVISOR_VIRT_START)
+#define machine_to_phys_mapping ((xen_ulong_t *)HYPERVISOR_VIRT_START)
#endif
/*
@@ -198,12 +198,12 @@ DEFINE_XEN_GUEST_HANDLE(cpu_user_regs_t);
#undef __DECL_REG_LO16
#undef __DECL_REG_HI
-#define xen_pfn_to_cr3(pfn) ((unsigned long)(pfn) << 12)
-#define xen_cr3_to_pfn(cr3) ((unsigned long)(cr3) >> 12)
+#define xen_pfn_to_cr3(pfn) ((xen_ulong_t)(pfn) << 12)
+#define xen_cr3_to_pfn(cr3) ((xen_ulong_t)(cr3) >> 12)
struct arch_vcpu_info {
-unsigned long cr2;
-unsigned long pad; /* sizeof(vcpu_info_t) == 64 */
+xen_ulong_t cr2;
+xen_ulong_t pad; /* sizeof(vcpu_info_t) == 64 */
};
typedef struct arch_vcpu_info arch_vcpu_info_t;
diff --git a/xen/include/public/arch-x86/xen.h
b/xen/include/public/arch-x86/xen.h
index a9a87d9b50..90710c1230 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -141,7 +141,7 @@ struct trap_info {
uint8_t vector; /* exception vector */
uint8_t flags; /* 0-3: privilege level; 4: clear event enable? */
uint16_t cs; /* code selector */
-unsigned long address; /* code offset */
+xen_ulong_t address; /* code offset */
};
typedef struct trap_info trap_info_t;
DEFINE_XEN_GUEST_HANDLE(trap_info_t);
@@ -174,36 +174,36 @@ struct vcpu_guest_context {
#define VGCF_syscall_disables_events (1<<_VGCF_syscall_disables_events)
#define _VGCF_online 5
#define VGCF_online(1<<_VGCF_online)
-unsigned long flags;/* VGCF_* flags */
-struct cpu_user_regs user_regs; /* User-level CPU registers */
-struct trap_info trap_ctxt[256];/* Virtual IDT */
-unsigned long ldt_base, ldt_ents; /* LDT (linear address, # ents) */
-unsigned long gdt_frames[16], gdt_ents; /* GDT (machine frames, # ents)
[PATCH v2 1/4] docs: add xen_ulong_t to the documented integers sizes/alignments
xen_ulong_t is widely used in public headers. Signed-off-by: Stefano Stabellini --- Given that xen_ulong_t is used in public headers there could be a better place for documenting it but this was the most straightforward to add. --- docs/misra/C-language-toolchain.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/misra/C-language-toolchain.rst b/docs/misra/C-language-toolchain.rst index 5ddfe7bdbe..7a334260e6 100644 --- a/docs/misra/C-language-toolchain.rst +++ b/docs/misra/C-language-toolchain.rst @@ -531,6 +531,17 @@ A summary table of data types, sizes and alignment is below: - 64 bits - x86_64, ARMv8-A AArch64, RV64, PPC64 + * - xen_ulong_t + - 32 bits + - 32 bits + - x86_32 + + * - xen_ulong_t + - 64 bits + - 64 bits + - x86_64, ARMv8-A AArch64, RV64, PPC64, ARMv8-A AArch32, ARMv8-R + AArch32, ARMv7-A + * - long long - 64-bit - 32-bit -- 2.25.1
[PATCH v2 2/4] public: s/int/int32_t
Straightforward int -> int32_t and unsigned int -> uint32_t replacements
in public headers. No ABI or semantic changes intended.
Signed-off-by: Stefano Stabellini
---
Changes in v2:
- avoid changes to GUEST_HANDLEs for now (there was one GUEST_HANDLE
change in v1)
---
xen/include/public/kexec.h | 8
xen/include/public/memory.h | 22 +++---
xen/include/public/physdev.h | 18 +-
xen/include/public/sched.h | 6 +++---
xen/include/public/vcpu.h| 2 +-
5 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/xen/include/public/kexec.h b/xen/include/public/kexec.h
index 40d79e936b..8d2a0ef697 100644
--- a/xen/include/public/kexec.h
+++ b/xen/include/public/kexec.h
@@ -105,7 +105,7 @@ typedef struct xen_kexec_image {
*/
#define KEXEC_CMD_kexec 0
typedef struct xen_kexec_exec {
-int type;
+int32_t type;
} xen_kexec_exec_t;
/*
@@ -116,7 +116,7 @@ typedef struct xen_kexec_exec {
#define KEXEC_CMD_kexec_load_v1 1 /* obsolete since 0x00040400 */
#define KEXEC_CMD_kexec_unload_v1 2 /* obsolete since 0x00040400 */
typedef struct xen_kexec_load_v1 {
-int type;
+int32_t type;
xen_kexec_image_t image;
} xen_kexec_load_v1_t;
@@ -143,8 +143,8 @@ typedef struct xen_kexec_load_v1 {
*/
#define KEXEC_CMD_kexec_get_range 3
typedef struct xen_kexec_range {
-int range;
-int nr;
+int32_t range;
+int32_t nr;
unsigned long size;
unsigned long start;
} xen_kexec_range_t;
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 5e545ae9a4..ae4a71268f 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -61,13 +61,13 @@ struct xen_memory_reservation {
/* Number of extents, and size/alignment of each (2^extent_order pages). */
xen_ulong_tnr_extents;
-unsigned int extent_order;
+uint32_t extent_order;
#if __XEN_INTERFACE_VERSION__ >= 0x00030209
/* XENMEMF flags. */
-unsigned int mem_flags;
+uint32_t mem_flags;
#else
-unsigned int address_bits;
+uint32_t address_bits;
#endif
/*
@@ -163,7 +163,7 @@ struct xen_machphys_mfn_list {
* Size of the 'extent_start' array. Fewer entries will be filled if the
* machphys table is smaller than max_extents * 2MB.
*/
-unsigned int max_extents;
+uint32_t max_extents;
/*
* Pointer to buffer to fill with list of extent starts. If there are
@@ -176,7 +176,7 @@ struct xen_machphys_mfn_list {
* Number of extents written to the above array. This will be smaller
* than 'max_extents' if the machphys table is smaller than max_e * 2MB.
*/
-unsigned int nr_extents;
+uint32_t nr_extents;
};
typedef struct xen_machphys_mfn_list xen_machphys_mfn_list_t;
DEFINE_XEN_GUEST_HANDLE(xen_machphys_mfn_list_t);
@@ -232,7 +232,7 @@ struct xen_add_to_physmap {
/* Number of pages to go through for gmfn_range */
uint16_tsize;
-unsigned int space; /* => enum phys_map_space */
+uint32_tspace; /* => enum phys_map_space */
#define XENMAPIDX_grant_table_status 0x8000U
@@ -317,7 +317,7 @@ struct xen_memory_map {
* return the number of entries which have been stored in
* buffer.
*/
-unsigned int nr_entries;
+uint32_t nr_entries;
/*
* Entries in the buffer are in the same format as returned by the
@@ -591,7 +591,7 @@ struct xen_reserved_device_memory_map {
* Gets set to the required number of entries when too low,
* signaled by error code -ERANGE.
*/
-unsigned int nr_entries;
+uint32_t nr_entries;
/* OUT */
XEN_GUEST_HANDLE(xen_reserved_device_memory_t) buffer;
/* IN */
@@ -711,9 +711,9 @@ struct xen_vnuma_topology_info {
domid_t domid;
uint16_t pad;
/* IN/OUT */
-unsigned int nr_vnodes;
-unsigned int nr_vcpus;
-unsigned int nr_vmemranges;
+uint32_t nr_vnodes;
+uint32_t nr_vcpus;
+uint32_t nr_vmemranges;
/* OUT */
union {
XEN_GUEST_HANDLE(uint) h;
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
index f0c0d4727c..03ccf86618 100644
--- a/xen/include/public/physdev.h
+++ b/xen/include/public/physdev.h
@@ -142,17 +142,17 @@ DEFINE_XEN_GUEST_HANDLE(physdev_irq_t);
struct physdev_map_pirq {
domid_t domid;
/* IN */
-int type;
+int32_t type;
/* IN (ignored for ..._MULTI_MSI) */
-int index;
+int32_t index;
/* IN or OUT */
-int pirq;
+int32_t pirq;
/* IN - high 16 bits hold segment for ..._MSI_SEG and ..._MULTI_MSI */
-int bus;
+int32_t bus;
/* IN */
-int devfn;
+int32_t devfn;
/* IN (also OUT for ..._MULTI_MSI) */
-int entry_nr;
+int32_t entry_nr;
/* IN */
uint64_t table_base;
};
@@ -163,7 +163,7 @@ DEFINE_XEN_GUEST_HANDLE(physdev_map_pirq_t);
struct physdev_unmap_pirq {
domid_t domid;
/* IN */
[PATCH v2 0/4] xen/public: use fixed-size integers
Hi all, This short patch series replaces integers without a fixed size with fixed-size integers in public headers. Cheers, Stefano
Re: [PATCH] x86/pat: fix W^X violation false-positives when running as Xen PV guest
Hi Juergen, On Tue, Apr 9, 2024 at 5:47 AM Juergen Gross wrote: > > When running as Xen PV guest in some cases W^X violation WARN()s have > been observed. Those WARN()s are produced by verify_rwx(), which looks > into the PTE to verify that writable kernel pages have the NX bit set > in order to avoid code modifications of the kernel by rogue code. > > As the NX bits of all levels of translation entries are or-ed and the > RW bits of all levels are and-ed, looking just into the PTE isn't enough > for the decision that a writable page is executable, too. When running > as a Xen PV guest, kernel initialization will set the NX bit in PMD > entries of the initial page tables covering the .data segment. I think this is a more accurate description of what I investigated: "When running as a Xen PV guest, the direct map PMDs and kernel high map PMDs share the same set of PTEs. Kernel initialization will set the NX bit in the direct map PMD entries, and not the shared PTEs." The WARN()s I saw were with direct map addresses. Thanks, Jason
Re: [PATCH] x86/emul: Adjust X86EMUL_OPC_EXT_MASK to placate MISRA
On Wed, 10 Apr 2024, Andrew Cooper wrote: > Resolves 4740 MISRA R7.2 violations. LOL!! > Signed-off-by: Andrew Cooper Reviewed-by: Stefano Stabellini > --- > CC: Jan Beulich > CC: Roger Pau Monné > CC: Stefano Stabellini > CC: consult...@bugseng.com > CC: Roberto Bagnara > CC: Federico Serafini > CC: Nicola Vetrini > > of 4935, so 96% of them... > --- > xen/arch/x86/x86_emulate/x86_emulate.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h > b/xen/arch/x86/x86_emulate/x86_emulate.h > index 698750267a90..d92be69d84d9 100644 > --- a/xen/arch/x86/x86_emulate/x86_emulate.h > +++ b/xen/arch/x86/x86_emulate/x86_emulate.h > @@ -620,7 +620,7 @@ struct x86_emulate_ctxt > * below). > * Hence no separate #define-s get added. > */ > -#define X86EMUL_OPC_EXT_MASK 0x > +#define X86EMUL_OPC_EXT_MASK 0xU > #define X86EMUL_OPC(ext, byte) ((uint8_t)(byte) | \ >MASK_INSR((ext), X86EMUL_OPC_EXT_MASK)) > /* > > base-commit: 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9 > prerequisite-patch-id: 8d06e56c5d8a52f1387e1f5a7fce6a94b8c4a1ed > prerequisite-patch-id: 13355d26254b979c79de456c9a6ea6a9c639ba63 > prerequisite-patch-id: d1f308616490257685a49248e29f1b3516c2dde4 > -- > 2.30.2 >
Re: [PATCH] xen/spinlock: Adjust LOCK_DEBUG_INITVAL to placate MISRA
On Wed, 10 Apr 2024, Andrew Cooper wrote:
> Resolves an R7.2 violation.
>
> Fixes: c286bb93d20c ("xen/spinlock: support higher number of cpus")
> Signed-off-by: Andrew Cooper
Reviewed-by: Stefano Stabellini
Re: [PATCH] xen/vPCI: Remove shadowed variable
On Wed, 10 Apr 2024, Andrew Cooper wrote:
> Resolves a MISRA R5.3 violation.
>
> Fixes: 622bdd962822 ("vpci/header: handle p2m range sets per BAR")
> Signed-off-by: Andrew Cooper
Reviewed-by: Stefano Stabellini
Re: [PATCH v2 4/4] xen/virtual-region: Drop setup_virtual_regions()
> On 10 Apr 2024, at 19:42, Andrew Cooper wrote: > > All other actions it used to perform have been converted to build-time > initialisation. The extable setup can done at build time too. > > This is one fewer setup step required to get exceptions working. > > Take the opportunity to move 'core' into read_mostly, where it probably should > have lived all along. > > Signed-off-by: Andrew Cooper For the arm part: Reviewed-by: Luca Fancellu #arm I’ve also tested the serie on arm32 and arm64 on qemu Tested-by: Luca Fancellu
Re: [PATCH] xen/nospec: Remove unreachable code
On Wed, 10 Apr 2024, Andrew Cooper wrote:
> When CONFIG_SPECULATIVE_HARDEN_LOCK is active, this reads:
>
> static always_inline bool lock_evaluate_nospec(bool condition)
> {
> return arch_lock_evaluate_nospec(condition);
> return condition;
> }
>
> Insert an #else to take out the second return.
>
> Fixes: 7ef0084418e1 ("x86/spinlock: introduce support for blocking
> speculation into critical regions")
> Signed-off-by: Andrew Cooper
Reviewed-by: Stefano Stabellini
Re: [PATCH] x86/hvm: Fix Misra Rule 19.1 regression
On Wed, 10 Apr 2024, Andrew Cooper wrote:
> Despite noticing an impending Rule 19.1 violation, the adjustment made (the
> uint32_t cast) wasn't sufficient to avoid it. Try again.
>
> Fixes: 6a98383b0877 ("x86/HVM: clear upper halves of GPRs upon entry from
> 32-bit code")
> Signed-off-by: Andrew Cooper
Reviewed-by: Stefano Stabellini
> ---
> CC: Jan Beulich
> CC: Roger Pau Monné
> CC: Stefano Stabellini
> CC: consult...@bugseng.com
> CC: Roberto Bagnara
> CC: Federico Serafini
> CC: Nicola Vetrini
> ---
> xen/arch/x86/include/asm/hvm/hvm.h | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h
> b/xen/arch/x86/include/asm/hvm/hvm.h
> index 595253babeaf..899233fb257b 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -575,16 +575,16 @@ static inline void hvm_sanitize_regs_fields(struct
> cpu_user_regs *regs,
> if ( compat )
> {
> /* Clear GPR upper halves, to counteract guests playing games. */
> -regs->rbp = (uint32_t)regs->ebp;
> -regs->rbx = (uint32_t)regs->ebx;
> -regs->rax = (uint32_t)regs->eax;
> -regs->rcx = (uint32_t)regs->ecx;
> -regs->rdx = (uint32_t)regs->edx;
> -regs->rsi = (uint32_t)regs->esi;
> -regs->rdi = (uint32_t)regs->edi;
> -regs->rip = (uint32_t)regs->eip;
> -regs->rflags = (uint32_t)regs->eflags;
> -regs->rsp = (uint32_t)regs->esp;
> +regs->rbp = (uint32_t)regs->rbp;
> +regs->rbx = (uint32_t)regs->rbx;
> +regs->rax = (uint32_t)regs->rax;
> +regs->rcx = (uint32_t)regs->rcx;
> +regs->rdx = (uint32_t)regs->rdx;
> +regs->rsi = (uint32_t)regs->rsi;
> +regs->rdi = (uint32_t)regs->rdi;
> +regs->rip = (uint32_t)regs->rip;
> +regs->rflags = (uint32_t)regs->rflags;
> +regs->rsp = (uint32_t)regs->rsp;
> }
>
> #ifndef NDEBUG
>
> base-commit: f48299cad5c3c69fdc2c101517a6dab9c9827ea5
> --
> 2.30.2
>
[xen-unstable-smoke test] 185302: tolerable all pass - PUSHED
flight 185302 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/185302/ 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 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9 baseline version: xen f48299cad5c3c69fdc2c101517a6dab9c9827ea5 Last test of basis 185291 2024-04-09 22:02:16 Z0 days Testing same since 185302 2024-04-10 18:00:25 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Roger Pau Monné 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 f48299cad5..0e7ea8ca5f 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9 -> smoke
Re: [VirtIO] Support for various devices in Xen
Hi Stefano, Vikram, Viresh, Thank you for your answers and support, and sorry for my late reply. On 12/01/2024 02:56, Vikram Garhwal wrote: > Hi Andrei & Stefano, > > Actually, QEMU patches are already upstreamed for virtio-blk and virtio-net > devices available in v8.2.0. > For virtio with grants, the patches are WiP. > > On Xen side, we are yet to upstream xen-tools patches which basically generate > the right arguments when invoking QEMU. > Here are down stream patches if you want: > 1. > https://github.com/Xilinx/xen/commit/be35b46e907c7c78fd23888d837475eb28334638 > 2. For Virtio disk backend: > > https://github.com/Xilinx/xen/commit/947280803294bbb963f428423f679d074c60d632 > 3. For Virtio-net: > > https://github.com/Xilinx/xen/commit/32fcc702718591270e5c8928b7687d853249c882 > 4. For changing the machine name to Xenpvh(to align with QEMU changes): > > https://github.com/Xilinx/xen/commit/5f669949c9ffdb1947cb47038956b5fb8eeb072a >> The libxl changes are lagging behind a bit and you might have to use >> device_model_args to enable virtio backends in QEMU. > But QEMU 8.2.0 can still be used for virtio-net on ARM. > > @Andrei here is an example on how to use virtio-net with QEMU: > -device virtio-net-device,id=nic0,netdev=net0,mac=00:16:3e:4f:43:05 \ > -netdev > type=tap,id=net0,ifname=vif1.0-emu,br=xenbr0,script=no,downscript=no\ > -machine xenpvh > > Please make sure to use xenpvh as QEMU machine. > > Regards, > Vikram I've managed to successfully get a DomU up and running with the rootfs based on virtio-blk. I'm running QEMU 8.2.1, Xen 4.18 + Vikram's downstream patches, Linux 6.6.12-rt, built through yocto with some changes to xen-tools and QEMU recipes. However, when also enabling PV networking through virtio-net, it seems that DomU cannot successfully boot. The device model args passed by xen-tools when invoking QEMU look exactly like what Vikram said they should. While executing `xl -v create ..` I can see some error regarding the device model crashing: libxl: debug: libxl_exec.c:127:libxl_report_child_exitstatus: domain 1 device model (dying as expected) [300] died due to fatal signal Killed But the error is not fatal and the DomU spawn goes on, it boots but never reaches prompt. It seems that kernel crashes silently at some point. Though, the networking interface is present since udev tries to rename it right before boot hangs: [ 4.376715] vif vif-0 enX0: renamed from eth1 Why would the QEMU DM process be killed, though? Invalid memory access? Here are the full logs for the "xl create" command [0] and for DomU's dmesg [1]. Any ideas as to why that might happen, some debugging insights, or maybe some configuration details I could have overlooked? Thank you very much for your help once again. Regards, Andrei Cherechesu [0] https://privatebin.net/?0fc1db27433dbcb5#4twCBMayizr7x89pxPzNqQ198z92q8YxVheHvNDsVAtd [1] https://privatebin.net/?ec3cb13fe2a086a1#F1zynLYQJCUDfZiwikZtRBEPJTACR2GZX6jn2ShXxmae >> For SCMI, I'll let Bertrand (CCed) comment. >> >> Cheers, >> >> Stefano >> >> >> On Thu, 11 Jan 2024, Andrei Cherechesu (OSS) wrote: >>> Hello, >>> >>> As I've mentioned in previous discussion threads in the xen-devel >>> community, we are running Xen 4.17 (uprev to 4.18 in progress) on NXP >>> S32G automotive processors (Cortex-A53 cores) and we wanted to know more >>> about the support for various VirtIO device types in Xen. >>> >>> In the Xen 4.17 release notes, the VirtIO standalone backends mentioned >>> as supported and tested are: virtio-disk, virtio-net, virtio-i2c and >>> virtio-gpio. >>> >>> However, we've only managed to successfully set up and try some >>> use-cases with the virtio-disk standalone backend [0] (which Olexandr >>> provided) based on the virtio-mmio transport. >>> >>> As such, we have a few questions, which we haven't been able to figure >>> out from the mailing list discussions and/or code: >>> 1. Are there any plans for the virtio-disk repo to have a stable >>> version? Is it going to be long-term hosted and maintained in the >>> xen-troops github repo? Or was it just an one-time PoC implementation >>> >>> and the strategy for future VirtIO devices will be based on a more >>> generic >>> >>> approach (i.e., without need for a specific standalone app)? >>> >>> >>> 2. With regards to the other backends, we want to try out and provide PV >>> >>> networking to a DomU based on virtio-net, but we haven't found any >>> available >>> >>> resources for it (e.g., the standalone backend implementation if needed >>> for >>> >>> control plane, configuration examples, presentations, demos, docs). >>> Does it >>> >>> rely on the QEMU virtio-net or vhost implementation? Are there any >>> examples >>> >>> on how to set it up? Any required Xen/Linux Kernel/QEMU versions? >>> >>> >>> 3. What other VirtIO device types are there planned to be supported in >>> Xen? >>> >>>
Re: [PATCH] xen/nospec: Remove unreachable code
> On 10 Apr 2024, at 20:26, Andrew Cooper wrote:
>
> When CONFIG_SPECULATIVE_HARDEN_LOCK is active, this reads:
>
> static always_inline bool lock_evaluate_nospec(bool condition)
> {
> return arch_lock_evaluate_nospec(condition);
> return condition;
> }
>
> Insert an #else to take out the second return.
>
> Fixes: 7ef0084418e1 ("x86/spinlock: introduce support for blocking
> speculation into critical regions")
> Signed-off-by: Andrew Cooper
> ---
Reviewed-by: Luca Fancellu
Re: [PATCH] xen/vPCI: Remove shadowed variable
> On 10 Apr 2024, at 20:33, Andrew Cooper wrote:
>
> Resolves a MISRA R5.3 violation.
>
> Fixes: 622bdd962822 ("vpci/header: handle p2m range sets per BAR")
> Signed-off-by: Andrew Cooper
> ---
Reviewed-by: Luca Fancellu
Re: [PATCH] xen/spinlock: Adjust LOCK_DEBUG_INITVAL to placate MISRA
> On 10 Apr 2024, at 20:35, Andrew Cooper wrote:
>
> Resolves an R7.2 violation.
>
> Fixes: c286bb93d20c ("xen/spinlock: support higher number of cpus")
> Signed-off-by: Andrew Cooper
Yes makes sense
Reviewed-by: Luca Fancellu
Re: [PATCH 2/5] x86/pvh: Make PVH entrypoint PIC for x86-64
On Wed, Apr 10, 2024 at 3:50 PM Jason Andryuk wrote: > > The PVH entrypoint is 32bit non-PIC code running the uncompressed > vmlinux at its load address CONFIG_PHYSICAL_START - default 0x100 > (16MB). The kernel is loaded at that physical address inside the VM by > the VMM software (Xen/QEMU). > > When running a Xen PVH Dom0, the host reserved addresses are mapped 1-1 > into the PVH container. There exist system firmwares (Coreboot/EDK2) > with reserved memory at 16MB. This creates a conflict where the PVH > kernel cannot be loaded at that address. > > Modify the PVH entrypoint to be position-indepedent to allow flexibility > in load address. Only the 64bit entry path is converted. A 32bit > kernel is not PIC, so calling into other parts of the kernel, like > xen_prepare_pvh() and mk_pgtable_32(), don't work properly when > relocated. > > This makes the code PIC, but the page tables need to be updated as well > to handle running from the kernel high map. > > The UNWIND_HINT_END_OF_STACK is to silence: > vmlinux.o: warning: objtool: pvh_start_xen+0x7f: unreachable instruction > after the lret into 64bit code. > > Signed-off-by: Jason Andryuk > --- > --- > arch/x86/platform/pvh/head.S | 44 > 1 file changed, 34 insertions(+), 10 deletions(-) > > diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S > index f7235ef87bc3..bb1e582e32b1 100644 > --- a/arch/x86/platform/pvh/head.S > +++ b/arch/x86/platform/pvh/head.S > @@ -7,6 +7,7 @@ > .code32 > .text > #define _pa(x) ((x) - __START_KERNEL_map) > +#define rva(x) ((x) - pvh_start_xen) > > #include > #include > @@ -54,7 +55,25 @@ SYM_CODE_START_LOCAL(pvh_start_xen) > UNWIND_HINT_END_OF_STACK > cld > > - lgdt (_pa(gdt)) > + /* > +* See the comment for startup_32 for more details. We need to > +* execute a call to get the execution address to be position > +* independent, but we don't have a stack. Save and restore the > +* magic field of start_info in ebx, and use that as the stack. > +*/ > + mov (%ebx), %eax > + leal 4(%ebx), %esp > + ANNOTATE_INTRA_FUNCTION_CALL > + call 1f > +1: popl %ebp > + mov %eax, (%ebx) > + subl $rva(1b), %ebp > + movl $0, %esp > + > + leal rva(gdt)(%ebp), %eax > + leal rva(gdt_start)(%ebp), %ecx > + movl %ecx, 2(%eax) > + lgdt (%eax) > > mov $PVH_DS_SEL,%eax > mov %eax,%ds > @@ -62,14 +81,14 @@ SYM_CODE_START_LOCAL(pvh_start_xen) > mov %eax,%ss > > /* Stash hvm_start_info. */ > - mov $_pa(pvh_start_info), %edi > + leal rva(pvh_start_info)(%ebp), %edi > mov %ebx, %esi > - mov _pa(pvh_start_info_sz), %ecx > + movl rva(pvh_start_info_sz)(%ebp), %ecx > shr $2,%ecx > rep > movsl > > - mov $_pa(early_stack_end), %esp > + leal rva(early_stack_end)(%ebp), %esp > > /* Enable PAE mode. */ > mov %cr4, %eax > @@ -84,28 +103,33 @@ SYM_CODE_START_LOCAL(pvh_start_xen) > wrmsr > > /* Enable pre-constructed page tables. */ > - mov $_pa(init_top_pgt), %eax > + leal rva(init_top_pgt)(%ebp), %eax > mov %eax, %cr3 > mov $(X86_CR0_PG | X86_CR0_PE), %eax > mov %eax, %cr0 > > /* Jump to 64-bit mode. */ > - ljmp $PVH_CS_SEL, $_pa(1f) > + pushl $PVH_CS_SEL > + leal rva(1f)(%ebp), %eax > + pushl %eax > + lretl > > /* 64-bit entry point. */ > .code64 > 1: > + UNWIND_HINT_END_OF_STACK > + > /* Set base address in stack canary descriptor. */ > mov $MSR_GS_BASE,%ecx > - mov $_pa(canary), %eax > + leal rva(canary)(%ebp), %eax Since this is in 64-bit mode, RIP-relative addressing can be used. > xor %edx, %edx > wrmsr > > call xen_prepare_pvh > > /* startup_64 expects boot_params in %rsi. */ > - mov $_pa(pvh_bootparams), %rsi > - mov $_pa(startup_64), %rax > + lea rva(pvh_bootparams)(%ebp), %rsi > + lea rva(startup_64)(%ebp), %rax RIP-relative here too. > ANNOTATE_RETPOLINE_SAFE > jmp *%rax > > @@ -143,7 +167,7 @@ SYM_CODE_END(pvh_start_xen) > .balign 8 > SYM_DATA_START_LOCAL(gdt) > .word gdt_end - gdt_start > - .long _pa(gdt_start) > + .long _pa(gdt_start) /* x86-64 will overwrite if relocated. */ > .word 0 > SYM_DATA_END(gdt) > SYM_DATA_START_LOCAL(gdt_start) > -- > 2.44.0 > > Brian Gerst
[libvirt test] 185295: regressions - FAIL
flight 185295 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/185295/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvops 6 kernel-build fail REGR. vs. 185278 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 185278 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-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-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 01f2b614a25f60b9deab44e3efd05646958776ad baseline version: libvirt 4b5cc57ed35dc24d11673dd3f04bfb8073c0340d Last test of basis 185278 2024-04-09 04:18:45 Z1 days Testing same since 185295 2024-04-10 04:20:37 Z0 days1 attempts People who touched revisions under test: Michal Privoznik jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopsfail 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 blocked test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-amd64-libvirt-qcow2 pass test-arm64-arm64-libvirt-qcow2 blocked test-amd64-amd64-libvirt-raw pass test-arm64-arm64-libvirt-raw blocked 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 Not pushing. commit 01f2b614a25f60b9deab44e3efd05646958776ad Author: Michal Privoznik Date: Tue Apr 9 10:17:11 2024 +0200 qemusecuritytest: Call real virFileExists in mock When I suggested to Jim to call real virFileExists() I forgot to also suggest calling init_syms(). Without it, real_virFileExists pointer might be left unset. And indeed, that's what we were seeing on FreeBSD. This effectively reverts commit
[PATCH 4/5] x86/kernel: Move page table macros to new header
The PVH entry point will need an additional set of prebuild page tables. Move the macros and defines to a new header so they can be re-used. Signed-off-by: Jason Andryuk --- checkpatch.pl gives an error: "ERROR: Macros with multiple statements should be enclosed in a do - while loop" about the moved PMDS macro. But PMDS is an assembler macro, so its not applicable. --- arch/x86/kernel/head_64.S| 22 ++ arch/x86/kernel/pgtable_64_helpers.h | 28 2 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 arch/x86/kernel/pgtable_64_helpers.h diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S index d4918d03efb4..4b036f3220f2 100644 --- a/arch/x86/kernel/head_64.S +++ b/arch/x86/kernel/head_64.S @@ -27,17 +27,12 @@ #include #include +#include "pgtable_64_helpers.h" + /* * We are not able to switch in one step to the final KERNEL ADDRESS SPACE * because we need identity-mapped pages. */ -#define l4_index(x)(((x) >> 39) & 511) -#define pud_index(x) (((x) >> PUD_SHIFT) & (PTRS_PER_PUD-1)) - -L4_PAGE_OFFSET = l4_index(__PAGE_OFFSET_BASE_L4) -L4_START_KERNEL = l4_index(__START_KERNEL_map) - -L3_START_KERNEL = pud_index(__START_KERNEL_map) .text __HEAD @@ -619,9 +614,6 @@ SYM_CODE_START_NOALIGN(vc_no_ghcb) SYM_CODE_END(vc_no_ghcb) #endif -#define SYM_DATA_START_PAGE_ALIGNED(name) \ - SYM_START(name, SYM_L_GLOBAL, .balign PAGE_SIZE) - #ifdef CONFIG_PAGE_TABLE_ISOLATION /* * Each PGD needs to be 8k long and 8k aligned. We do not @@ -643,14 +635,6 @@ SYM_CODE_END(vc_no_ghcb) #define PTI_USER_PGD_FILL 0 #endif -/* Automate the creation of 1 to 1 mapping pmd entries */ -#define PMDS(START, PERM, COUNT) \ - i = 0 ; \ - .rept (COUNT) ; \ - .quad (START) + (i << PMD_SHIFT) + (PERM) ; \ - i = i + 1 ; \ - .endr - __INITDATA .balign 4 @@ -749,8 +733,6 @@ SYM_DATA_START_PAGE_ALIGNED(level1_fixmap_pgt) .endr SYM_DATA_END(level1_fixmap_pgt) -#undef PMDS - .data .align 16 diff --git a/arch/x86/kernel/pgtable_64_helpers.h b/arch/x86/kernel/pgtable_64_helpers.h new file mode 100644 index ..0ae87d768ce2 --- /dev/null +++ b/arch/x86/kernel/pgtable_64_helpers.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __PGTABLES_64_H__ +#define __PGTABLES_64_H__ + +#ifdef __ASSEMBLY__ + +#define l4_index(x)(((x) >> 39) & 511) +#define pud_index(x) (((x) >> PUD_SHIFT) & (PTRS_PER_PUD-1)) + +L4_PAGE_OFFSET = l4_index(__PAGE_OFFSET_BASE_L4) +L4_START_KERNEL = l4_index(__START_KERNEL_map) + +L3_START_KERNEL = pud_index(__START_KERNEL_map) + +#define SYM_DATA_START_PAGE_ALIGNED(name) \ + SYM_START(name, SYM_L_GLOBAL, .balign PAGE_SIZE) + +/* Automate the creation of 1 to 1 mapping pmd entries */ +#define PMDS(START, PERM, COUNT) \ + i = 0 ; \ + .rept (COUNT) ; \ + .quad (START) + (i << PMD_SHIFT) + (PERM) ; \ + i = i + 1 ; \ + .endr + +#endif /* __ASSEMBLY__ */ + +#endif /* __PGTABLES_64_H__ */ -- 2.44.0
[PATCH 5/5] x86/pvh: Add 64bit relocation page tables
The PVH entry point is 32bit. For a 64bit kernel, the entry point must switch to 64bit mode, which requires a set of page tables. In the past, PVH used init_top_pgt. This works fine when the kernel is loaded at LOAD_PHYSICAL_ADDR, as the page tables are prebuilt for this address. If the kernel is loaded at a different address, they need to be adjusted. __startup_64() adjusts the prebuilt page tables for the physical load address, but it is 64bit code. The 32bit PVH entry code can't call it to adjust the page tables, so it can't readily be re-used. 64bit PVH entry needs page tables set up for identity map, the kernel high map and the direct map. pvh_start_xen() enters identity mapped. Inside xen_prepare_pvh(), it jumps through a pv_ops function pointer into the highmap. The direct map is used for __va() on the initramfs and other guest physical addresses. Add a dedicated set of prebuild page tables for PVH entry. They are adjusted in assembly before loading. Add XEN_ELFNOTE_PHYS32_RELOC to indicate support for relocation along with the kernel's loading constraints. The maximum load address, KERNEL_IMAGE_SIZE - 1, is determined by a single pvh_level2_ident_pgt page. It could be larger with more pages. Signed-off-by: Jason Andryuk --- Instead of adding 5 pages of prebuilt page tables, they could be contructed dynamically in the .bss area. They are then only used for PVH entry and until transitioning to init_top_pgt. The .bss is later cleared. It's safer to add the dedicated pages, so that is done here. --- arch/x86/platform/pvh/head.S | 105 ++- 1 file changed, 104 insertions(+), 1 deletion(-) diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S index c08d08d8cc92..4af3cfbcf2f8 100644 --- a/arch/x86/platform/pvh/head.S +++ b/arch/x86/platform/pvh/head.S @@ -21,6 +21,8 @@ #include #include +#include "../kernel/pgtable_64_helpers.h" + __HEAD /* @@ -102,8 +104,47 @@ SYM_CODE_START_LOCAL(pvh_start_xen) btsl $_EFER_LME, %eax wrmsr + mov %ebp, %ebx + subl $LOAD_PHYSICAL_ADDR, %ebx /* offset */ + jz .Lpagetable_done + + /* Fixup page-tables for relocation. */ + leal rva(pvh_init_top_pgt)(%ebp), %edi + movl $512, %ecx +2: + testl $_PAGE_PRESENT, 0x00(%edi) + jz 1f + addl %ebx, 0x00(%edi) +1: + addl $8, %edi + decl %ecx + jnz 2b + + /* L3 ident has a single entry. */ + leal rva(pvh_level3_ident_pgt)(%ebp), %edi + addl %ebx, 0x00(%edi) + + leal rva(pvh_level3_kernel_pgt)(%ebp), %edi + addl %ebx, (4096 - 16)(%edi) + addl %ebx, (4096 - 8)(%edi) + + /* pvh_level2_ident_pgt is fine - large pages */ + + /* pvh_level2_kernel_pgt needs adjustment - large pages */ + leal rva(pvh_level2_kernel_pgt)(%ebp), %edi + movl $512, %ecx +2: + testl $_PAGE_PRESENT, 0x00(%edi) + jz 1f + addl %ebx, 0x00(%edi) +1: + addl $8, %edi + decl %ecx + jnz 2b + +.Lpagetable_done: /* Enable pre-constructed page tables. */ - leal rva(init_top_pgt)(%ebp), %eax + leal rva(pvh_init_top_pgt)(%ebp), %eax mov %eax, %cr3 mov $(X86_CR0_PG | X86_CR0_PE), %eax mov %eax, %cr0 @@ -197,5 +238,67 @@ SYM_DATA_START_LOCAL(early_stack) .fill BOOT_STACK_SIZE, 1, 0 SYM_DATA_END_LABEL(early_stack, SYM_L_LOCAL, early_stack_end) +#ifdef CONFIG_X86_64 +/* + * Xen PVH needs a set of identity mapped and kernel high mapping + * page tables. pvh_start_xen starts running on the identity mapped + * page tables, but xen_prepare_pvh calls into the high mapping. + * These page tables need to be relocatable and are only used until + * startup_64 transitions to init_top_pgt. + */ +SYM_DATA_START_PAGE_ALIGNED(pvh_init_top_pgt) + .quad pvh_level3_ident_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC + .orgpvh_init_top_pgt + L4_PAGE_OFFSET*8, 0 + .quad pvh_level3_ident_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC + .orgpvh_init_top_pgt + L4_START_KERNEL*8, 0 + /* (2^48-(2*1024*1024*1024))/(2^39) = 511 */ + .quad pvh_level3_kernel_pgt - __START_KERNEL_map + _PAGE_TABLE_NOENC +SYM_DATA_END(pvh_init_top_pgt) + +SYM_DATA_START_PAGE_ALIGNED(pvh_level3_ident_pgt) + .quad pvh_level2_ident_pgt - __START_KERNEL_map + _KERNPG_TABLE_NOENC + .fill 511, 8, 0 +SYM_DATA_END(pvh_level3_ident_pgt) +SYM_DATA_START_PAGE_ALIGNED(pvh_level2_ident_pgt) + /* +* Since I easily can, map the first 1G. +* Don't set NX because code runs from these pages. +* +* Note: This sets _PAGE_GLOBAL despite whether +* the CPU supports it or it is enabled. But, +* the CPU should ignore the bit. +*/ + PMDS(0, __PAGE_KERNEL_IDENT_LARGE_EXEC, PTRS_PER_PMD) +SYM_DATA_END(pvh_level2_ident_pgt) +SYM_DATA_START_PAGE_ALIGNED(pvh_level3_kernel_pgt) + .fill
[PATCH 2/5] x86/pvh: Make PVH entrypoint PIC for x86-64
The PVH entrypoint is 32bit non-PIC code running the uncompressed vmlinux at its load address CONFIG_PHYSICAL_START - default 0x100 (16MB). The kernel is loaded at that physical address inside the VM by the VMM software (Xen/QEMU). When running a Xen PVH Dom0, the host reserved addresses are mapped 1-1 into the PVH container. There exist system firmwares (Coreboot/EDK2) with reserved memory at 16MB. This creates a conflict where the PVH kernel cannot be loaded at that address. Modify the PVH entrypoint to be position-indepedent to allow flexibility in load address. Only the 64bit entry path is converted. A 32bit kernel is not PIC, so calling into other parts of the kernel, like xen_prepare_pvh() and mk_pgtable_32(), don't work properly when relocated. This makes the code PIC, but the page tables need to be updated as well to handle running from the kernel high map. The UNWIND_HINT_END_OF_STACK is to silence: vmlinux.o: warning: objtool: pvh_start_xen+0x7f: unreachable instruction after the lret into 64bit code. Signed-off-by: Jason Andryuk --- --- arch/x86/platform/pvh/head.S | 44 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S index f7235ef87bc3..bb1e582e32b1 100644 --- a/arch/x86/platform/pvh/head.S +++ b/arch/x86/platform/pvh/head.S @@ -7,6 +7,7 @@ .code32 .text #define _pa(x) ((x) - __START_KERNEL_map) +#define rva(x) ((x) - pvh_start_xen) #include #include @@ -54,7 +55,25 @@ SYM_CODE_START_LOCAL(pvh_start_xen) UNWIND_HINT_END_OF_STACK cld - lgdt (_pa(gdt)) + /* +* See the comment for startup_32 for more details. We need to +* execute a call to get the execution address to be position +* independent, but we don't have a stack. Save and restore the +* magic field of start_info in ebx, and use that as the stack. +*/ + mov (%ebx), %eax + leal 4(%ebx), %esp + ANNOTATE_INTRA_FUNCTION_CALL + call 1f +1: popl %ebp + mov %eax, (%ebx) + subl $rva(1b), %ebp + movl $0, %esp + + leal rva(gdt)(%ebp), %eax + leal rva(gdt_start)(%ebp), %ecx + movl %ecx, 2(%eax) + lgdt (%eax) mov $PVH_DS_SEL,%eax mov %eax,%ds @@ -62,14 +81,14 @@ SYM_CODE_START_LOCAL(pvh_start_xen) mov %eax,%ss /* Stash hvm_start_info. */ - mov $_pa(pvh_start_info), %edi + leal rva(pvh_start_info)(%ebp), %edi mov %ebx, %esi - mov _pa(pvh_start_info_sz), %ecx + movl rva(pvh_start_info_sz)(%ebp), %ecx shr $2,%ecx rep movsl - mov $_pa(early_stack_end), %esp + leal rva(early_stack_end)(%ebp), %esp /* Enable PAE mode. */ mov %cr4, %eax @@ -84,28 +103,33 @@ SYM_CODE_START_LOCAL(pvh_start_xen) wrmsr /* Enable pre-constructed page tables. */ - mov $_pa(init_top_pgt), %eax + leal rva(init_top_pgt)(%ebp), %eax mov %eax, %cr3 mov $(X86_CR0_PG | X86_CR0_PE), %eax mov %eax, %cr0 /* Jump to 64-bit mode. */ - ljmp $PVH_CS_SEL, $_pa(1f) + pushl $PVH_CS_SEL + leal rva(1f)(%ebp), %eax + pushl %eax + lretl /* 64-bit entry point. */ .code64 1: + UNWIND_HINT_END_OF_STACK + /* Set base address in stack canary descriptor. */ mov $MSR_GS_BASE,%ecx - mov $_pa(canary), %eax + leal rva(canary)(%ebp), %eax xor %edx, %edx wrmsr call xen_prepare_pvh /* startup_64 expects boot_params in %rsi. */ - mov $_pa(pvh_bootparams), %rsi - mov $_pa(startup_64), %rax + lea rva(pvh_bootparams)(%ebp), %rsi + lea rva(startup_64)(%ebp), %rax ANNOTATE_RETPOLINE_SAFE jmp *%rax @@ -143,7 +167,7 @@ SYM_CODE_END(pvh_start_xen) .balign 8 SYM_DATA_START_LOCAL(gdt) .word gdt_end - gdt_start - .long _pa(gdt_start) + .long _pa(gdt_start) /* x86-64 will overwrite if relocated. */ .word 0 SYM_DATA_END(gdt) SYM_DATA_START_LOCAL(gdt_start) -- 2.44.0
[PATCH 1/5] xen: sync elfnote.h from xen tree
Sync Xen's elfnote.h header from xen.git to pull in the
XEN_ELFNOTE_PHYS32_RELOC define.
xen commit dfc9fab00378 ("x86/PVH: Support relocatable dom0 kernels")
This is a copy except for the removal of the emacs editor config at the
end of the file.
Signed-off-by: Jason Andryuk
---
include/xen/interface/elfnote.h | 93 +++--
1 file changed, 88 insertions(+), 5 deletions(-)
diff --git a/include/xen/interface/elfnote.h b/include/xen/interface/elfnote.h
index 38deb1214613..918f47d87d7a 100644
--- a/include/xen/interface/elfnote.h
+++ b/include/xen/interface/elfnote.h
@@ -11,7 +11,9 @@
#define __XEN_PUBLIC_ELFNOTE_H__
/*
- * The notes should live in a SHT_NOTE segment and have "Xen" in the
+ * `incontents 200 elfnotes ELF notes
+ *
+ * The notes should live in a PT_NOTE segment and have "Xen" in the
* name field.
*
* Numeric types are either 4 or 8 bytes depending on the content of
@@ -22,6 +24,8 @@
*
* String values (for non-legacy) are NULL terminated ASCII, also known
* as ASCIZ type.
+ *
+ * Xen only uses ELF Notes contained in x86 binaries.
*/
/*
@@ -52,7 +56,7 @@
#define XEN_ELFNOTE_VIRT_BASE 3
/*
- * The offset of the ELF paddr field from the acutal required
+ * The offset of the ELF paddr field from the actual required
* pseudo-physical address (numeric).
*
* This is used to maintain backwards compatibility with older kernels
@@ -92,7 +96,12 @@
#define XEN_ELFNOTE_LOADER 8
/*
- * The kernel supports PAE (x86/32 only, string = "yes" or "no").
+ * The kernel supports PAE (x86/32 only, string = "yes", "no" or
+ * "bimodal").
+ *
+ * For compatibility with Xen 3.0.3 and earlier the "bimodal" setting
+ * may be given as "yes,bimodal" which will cause older Xen to treat
+ * this kernel as PAE.
*
* LEGACY: PAE (n.b. The legacy interface included a provision to
* indicate 'extended-cr3' support allowing L3 page tables to be
@@ -149,7 +158,9 @@
* The (non-default) location the initial phys-to-machine map should be
* placed at by the hypervisor (Dom0) or the tools (DomU).
* The kernel must be prepared for this mapping to be established using
- * large pages, despite such otherwise not being available to guests.
+ * large pages, despite such otherwise not being available to guests. Note
+ * that these large pages may be misaligned in PFN space (they'll obviously
+ * be aligned in MFN and virtual address spaces).
* The kernel must also be able to handle the page table pages used for
* this mapping not being accessible through the initial mapping.
* (Only x86-64 supports this at present.)
@@ -185,9 +196,81 @@
*/
#define XEN_ELFNOTE_PHYS32_ENTRY 18
+/*
+ * Physical loading constraints for PVH kernels
+ *
+ * The presence of this note indicates the kernel supports relocating itself.
+ *
+ * The note may include up to three 32bit values to place constraints on the
+ * guest physical loading addresses and alignment for a PVH kernel. Values
+ * are read in the following order:
+ * - a required start alignment (default 0x20)
+ * - a minimum address for the start of the image (default 0; see below)
+ * - a maximum address for the last byte of the image (default 0x)
+ *
+ * When this note specifies an alignment value, it is used. Otherwise the
+ * maximum p_align value from loadable ELF Program Headers is used, if it is
+ * greater than or equal to 4k (0x1000). Otherwise, the default is used.
+ */
+#define XEN_ELFNOTE_PHYS32_RELOC 19
+
/*
* The number of the highest elfnote defined.
*/
-#define XEN_ELFNOTE_MAX XEN_ELFNOTE_PHYS32_ENTRY
+#define XEN_ELFNOTE_MAX XEN_ELFNOTE_PHYS32_RELOC
+
+/*
+ * System information exported through crash notes.
+ *
+ * The kexec / kdump code will create one XEN_ELFNOTE_CRASH_INFO
+ * note in case of a system crash. This note will contain various
+ * information about the system, see xen/include/xen/elfcore.h.
+ */
+#define XEN_ELFNOTE_CRASH_INFO 0x101
+
+/*
+ * System registers exported through crash notes.
+ *
+ * The kexec / kdump code will create one XEN_ELFNOTE_CRASH_REGS
+ * note per cpu in case of a system crash. This note is architecture
+ * specific and will contain registers not saved in the "CORE" note.
+ * See xen/include/xen/elfcore.h for more information.
+ */
+#define XEN_ELFNOTE_CRASH_REGS 0x102
+
+
+/*
+ * xen dump-core none note.
+ * xm dump-core code will create one XEN_ELFNOTE_DUMPCORE_NONE
+ * in its dump file to indicate that the file is xen dump-core
+ * file. This note doesn't have any other information.
+ * See tools/libxc/xc_core.h for more information.
+ */
+#define XEN_ELFNOTE_DUMPCORE_NONE 0x200
+
+/*
+ * xen dump-core header note.
+ * xm dump-core code will create one XEN_ELFNOTE_DUMPCORE_HEADER
+ * in its dump file.
+ * See tools/libxc/xc_core.h for more information.
+ */
+#define XEN_ELFNOTE_DUMPCORE_HEADER 0x201
+
+/*
+ * xen dump-core xen version note.
+ * xm dump-core code will create one
[PATCH 0/5] x86/pvh: Make PVH entry relocatable
Using the PVH entry point, the uncompressed vmlinux is loaded at
LOAD_PHYSICAL_ADDR, and execution starts in 32bit mode at the
address in XEN_ELFNOTE_PHYS32_ENTRY, pvh_start_xen, with paging
disabled.
Loading at LOAD_PHYSICAL_ADDR has not been a problem in the past as
virtual machines don't have conflicting memory maps. But Xen now
supports a PVH dom0, which uses the host memory map, and there are
Coreboot/EDK2 firmwares that have reserved regions conflicting with
LOAD_PHYSICAL_ADDR. Xen recently added XEN_ELFNOTE_PHYS32_RELOC to
specify an alignment, minimum and maximum load address when
LOAD_PHYSICAL_ADDR cannot be used. This patch series makes the PVH
entry path PIC to support relocation.
Only x86-64 is converted. The 32bit entry path calling into vmlinux,
which is not PIC, will not support relocation.
The entry path needs pages tables to switch to 64bit mode. A new
pvh_init_top_pgt is added to make the transition into the startup_64
when the regular init_top_pgt pagetables are setup. This duplication is
unfortunate, but it keeps the changes simpler. __startup_64() can't be
used to setup init_top_pgt for PVH entry because it is 64bit code - the
32bit entry code doesn't have page tables to use.
This is the straight forward implementation to make it work. Other
approaches could be pursued.
checkpatch.pl gives an error: "ERROR: Macros with multiple statements
should be enclosed in a do - while loop" about the moved PMDS macro.
But PMDS is an assembler macro, so its not applicable. There are some
false positive warnings "WARNING: space prohibited between function name
and open parenthesis '('" about the macro, too.
Jason Andryuk (5):
xen: sync elfnote.h from xen tree
x86/pvh: Make PVH entrypoint PIC for x86-64
x86/pvh: Set phys_base when calling xen_prepare_pvh()
x86/kernel: Move page table macros to new header
x86/pvh: Add 64bit relocation page tables
arch/x86/kernel/head_64.S| 22 +---
arch/x86/kernel/pgtable_64_helpers.h | 28 +
arch/x86/platform/pvh/head.S | 157 +--
include/xen/interface/elfnote.h | 93 +++-
4 files changed, 265 insertions(+), 35 deletions(-)
create mode 100644 arch/x86/kernel/pgtable_64_helpers.h
--
2.44.0
[PATCH 3/5] x86/pvh: Set phys_base when calling xen_prepare_pvh()
phys_base needs to be set for __pa() to work in xen_pvh_init() when finding the hypercall page. Set it before calling into xen_prepare_pvh(), which calls xen_pvh_init(). Clear it afterward to avoid __startup_64() adding to it and creating an incorrect value. Signed-off-by: Jason Andryuk --- Instead of setting and clearing phys_base, a dedicated variable could be used just for the hypercall page. Having phys_base set properly may avoid further issues if the use of phys_base or __pa() grows. --- arch/x86/platform/pvh/head.S | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S index bb1e582e32b1..c08d08d8cc92 100644 --- a/arch/x86/platform/pvh/head.S +++ b/arch/x86/platform/pvh/head.S @@ -125,7 +125,17 @@ SYM_CODE_START_LOCAL(pvh_start_xen) xor %edx, %edx wrmsr + /* Calculate load offset from LOAD_PHYSICAL_ADDR and store in +* phys_base. __pa() needs phys_base set to calculate the +* hypercall page in xen_pvh_init(). */ + movq %rbp, %rbx + subq $LOAD_PHYSICAL_ADDR, %rbx + movq %rbx, phys_base(%rip) call xen_prepare_pvh + /* Clear phys_base. __startup_64 will *add* to its value, +* so reset to 0. */ + xor %rbx, %rbx + movq %rbx, phys_base(%rip) /* startup_64 expects boot_params in %rsi. */ lea rva(pvh_bootparams)(%ebp), %rsi -- 2.44.0
[PATCH] x86/emul: Adjust X86EMUL_OPC_EXT_MASK to placate MISRA
Resolves 4740 MISRA R7.2 violations. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Roger Pau Monné CC: Stefano Stabellini CC: consult...@bugseng.com CC: Roberto Bagnara CC: Federico Serafini CC: Nicola Vetrini of 4935, so 96% of them... --- xen/arch/x86/x86_emulate/x86_emulate.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h index 698750267a90..d92be69d84d9 100644 --- a/xen/arch/x86/x86_emulate/x86_emulate.h +++ b/xen/arch/x86/x86_emulate/x86_emulate.h @@ -620,7 +620,7 @@ struct x86_emulate_ctxt * below). * Hence no separate #define-s get added. */ -#define X86EMUL_OPC_EXT_MASK 0x +#define X86EMUL_OPC_EXT_MASK 0xU #define X86EMUL_OPC(ext, byte) ((uint8_t)(byte) | \ MASK_INSR((ext), X86EMUL_OPC_EXT_MASK)) /* base-commit: 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9 prerequisite-patch-id: 8d06e56c5d8a52f1387e1f5a7fce6a94b8c4a1ed prerequisite-patch-id: 13355d26254b979c79de456c9a6ea6a9c639ba63 prerequisite-patch-id: d1f308616490257685a49248e29f1b3516c2dde4 -- 2.30.2
[PATCH] xen/spinlock: Adjust LOCK_DEBUG_INITVAL to placate MISRA
Resolves an R7.2 violation.
Fixes: c286bb93d20c ("xen/spinlock: support higher number of cpus")
Signed-off-by: Andrew Cooper
---
CC: George Dunlap
CC: Jan Beulich
CC: Stefano Stabellini
CC: Julien Grall
CC: Juergen Gross
CC: consult...@bugseng.com
CC: Roberto Bagnara
CC: Federico Serafini
CC: Nicola Vetrini
---
xen/include/xen/spinlock.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index db00a24646bd..18793c5e29cb 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -13,7 +13,7 @@
#ifdef CONFIG_DEBUG_LOCKS
union lock_debug {
uint32_t val;
-#define LOCK_DEBUG_INITVAL 0x
+#define LOCK_DEBUG_INITVAL 0xU
struct {
unsigned int cpu:SPINLOCK_CPU_BITS;
#define LOCK_DEBUG_PAD_BITS (30 - SPINLOCK_CPU_BITS)
base-commit: 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9
prerequisite-patch-id: 8d06e56c5d8a52f1387e1f5a7fce6a94b8c4a1ed
prerequisite-patch-id: 13355d26254b979c79de456c9a6ea6a9c639ba63
--
2.30.2
[PATCH] xen/vPCI: Remove shadowed variable
Resolves a MISRA R5.3 violation.
Fixes: 622bdd962822 ("vpci/header: handle p2m range sets per BAR")
Signed-off-by: Andrew Cooper
---
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: consult...@bugseng.com
CC: Roberto Bagnara
CC: Federico Serafini
CC: Nicola Vetrini
---
xen/drivers/vpci/vpci.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 260b72875ee1..97e115dc5798 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -62,8 +62,6 @@ void vpci_deassign_device(struct pci_dev *pdev)
spin_unlock(>vpci->lock);
if ( pdev->vpci->msix )
{
-unsigned int i;
-
list_del(>vpci->msix->next);
for ( i = 0; i < ARRAY_SIZE(pdev->vpci->msix->table); i++ )
if ( pdev->vpci->msix->table[i] )
--
2.30.2
[PATCH] xen/nospec: Remove unreachable code
When CONFIG_SPECULATIVE_HARDEN_LOCK is active, this reads:
static always_inline bool lock_evaluate_nospec(bool condition)
{
return arch_lock_evaluate_nospec(condition);
return condition;
}
Insert an #else to take out the second return.
Fixes: 7ef0084418e1 ("x86/spinlock: introduce support for blocking speculation
into critical regions")
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: consult...@bugseng.com
CC: Roberto Bagnara
CC: Federico Serafini
CC: Nicola Vetrini
---
xen/include/xen/nospec.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/include/xen/nospec.h b/xen/include/xen/nospec.h
index 9fb15aa26aa9..828dbd4e0ad8 100644
--- a/xen/include/xen/nospec.h
+++ b/xen/include/xen/nospec.h
@@ -82,8 +82,9 @@ static always_inline bool lock_evaluate_nospec(bool condition)
{
#ifdef CONFIG_SPECULATIVE_HARDEN_LOCK
return arch_lock_evaluate_nospec(condition);
-#endif
+#else
return condition;
+#endif
}
#endif /* XEN_NOSPEC_H */
base-commit: 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9
--
2.30.2
[PATCH v2 3/4] xen/virtual-region: Link the list build time
Given 3 statically initialised objects, its easy to link the list at build
time. There's no need to do it during runtime at boot (and with IRQs-off,
even).
As a consequence, register_virtual_region() can now move inside ifdef
CONFIG_LIVEPATCH like unregister_virtual_region().
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
CC: Stefano Stabellini
CC: Julien Grall
CC: Volodymyr Babchuk
CC: Bertrand Marquis
CC: Michal Orzel
CC: Oleksii Kurochko
CC: Shawn Anastasio
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
v2:
* Collect the initialisers togoether too.
---
xen/common/virtual_region.c | 37 ++---
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
index 7d8bdeb61282..db3e0dc9fe74 100644
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -15,8 +15,18 @@ extern const struct bug_frame
__start_bug_frames_2[], __stop_bug_frames_2[],
__start_bug_frames_3[], __stop_bug_frames_3[];
+/*
+ * For the built-in regions, the double linked list can be constructed at
+ * build time. Forward-declare the elements and their initialisers.
+ */
+static struct list_head virtual_region_list;
+static struct virtual_region core, core_init;
+#define LIST_ENTRY_HEAD() { .next = , .prev =
_init.list }
+#define LIST_ENTRY_CORE() { .next = _init.list, .prev =
_region_list }
+#define LIST_ENTRY_INIT() { .next = _region_list, .prev = }
+
static struct virtual_region core = {
-.list = LIST_HEAD_INIT(core.list),
+.list = LIST_ENTRY_CORE(),
.text_start = _stext,
.text_end = _etext,
.rodata_start = _srodata,
@@ -32,7 +42,7 @@ static struct virtual_region core = {
/* Becomes irrelevant when __init sections are cleared. */
static struct virtual_region core_init __initdata = {
-.list = LIST_HEAD_INIT(core_init.list),
+.list = LIST_ENTRY_INIT(),
.text_start = _sinittext,
.text_end = _einittext,
@@ -50,7 +60,7 @@ static struct virtual_region core_init __initdata = {
*
* All readers of virtual_region_list MUST use list_for_each_entry_rcu.
*/
-static LIST_HEAD(virtual_region_list);
+static struct list_head virtual_region_list = LIST_ENTRY_HEAD();
static DEFINE_SPINLOCK(virtual_region_lock);
static DEFINE_RCU_READ_LOCK(rcu_virtual_region_lock);
@@ -73,15 +83,6 @@ const struct virtual_region *find_text_region(unsigned long
addr)
return region;
}
-void register_virtual_region(struct virtual_region *r)
-{
-unsigned long flags;
-
-spin_lock_irqsave(_region_lock, flags);
-list_add_tail_rcu(>list, _region_list);
-spin_unlock_irqrestore(_region_lock, flags);
-}
-
/*
* Suggest inline so when !CONFIG_LIVEPATCH the function is not left
* unreachable after init code is removed.
@@ -96,6 +97,15 @@ static void inline remove_virtual_region(struct
virtual_region *r)
}
#ifdef CONFIG_LIVEPATCH
+void register_virtual_region(struct virtual_region *r)
+{
+unsigned long flags;
+
+spin_lock_irqsave(_region_lock, flags);
+list_add_tail_rcu(>list, _region_list);
+spin_unlock_irqrestore(_region_lock, flags);
+}
+
void unregister_virtual_region(struct virtual_region *r)
{
remove_virtual_region(r);
@@ -155,9 +165,6 @@ void __init setup_virtual_regions(const struct
exception_table_entry *start,
{
core_init.ex = core.ex = start;
core_init.ex_end = core.ex_end = end;
-
-register_virtual_region(_init);
-register_virtual_region();
}
/*
--
2.30.2
[PATCH v2 1/4] xen/link: Introduce a common BUGFRAMES definition
Bugframe linkage is identical in all architectures. This is not surprising
given that it is (now) only consumed by common/virtual_region.c
Introduce a common BUGFRAMES define in xen.lds.h ahead of rearranging their
structure.
No functional change.
Signed-off-by: Andrew Cooper
Reviewed-by: Jan Beulich
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
CC: Stefano Stabellini
CC: Julien Grall
CC: Volodymyr Babchuk
CC: Bertrand Marquis
CC: Michal Orzel
CC: Oleksii Kurochko
CC: Shawn Anastasio
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
---
xen/arch/arm/xen.lds.S| 13 +++--
xen/arch/ppc/xen.lds.S| 13 +++--
xen/arch/riscv/xen.lds.S | 13 +++--
xen/arch/x86/xen.lds.S| 11 +--
xen/include/xen/xen.lds.h | 11 +++
5 files changed, 21 insertions(+), 40 deletions(-)
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 2266c9536f05..bd884664adf6 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -55,16 +55,9 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
.rodata : {
_srodata = .; /* Read-only data */
-/* Bug frames table */
- __start_bug_frames = .;
- *(.bug_frames.0)
- __stop_bug_frames_0 = .;
- *(.bug_frames.1)
- __stop_bug_frames_1 = .;
- *(.bug_frames.2)
- __stop_bug_frames_2 = .;
- *(.bug_frames.3)
- __stop_bug_frames_3 = .;
+
+BUGFRAMES
+
*(.rodata)
*(.rodata.*)
*(.data.rel.ro)
diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S
index 8840a0044666..38cd857187e2 100644
--- a/xen/arch/ppc/xen.lds.S
+++ b/xen/arch/ppc/xen.lds.S
@@ -45,16 +45,9 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
DECL_SECTION(.rodata) {
_srodata = .; /* Read-only data */
-/* Bug frames table */
-__start_bug_frames = .;
-*(.bug_frames.0)
-__stop_bug_frames_0 = .;
-*(.bug_frames.1)
-__stop_bug_frames_1 = .;
-*(.bug_frames.2)
-__stop_bug_frames_2 = .;
-*(.bug_frames.3)
-__stop_bug_frames_3 = .;
+
+BUGFRAMES
+
*(.rodata)
*(.rodata.*)
*(.data.rel.ro)
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index ace6f49c579c..070b19d91503 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -42,16 +42,9 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
.rodata : {
_srodata = .; /* Read-only data */
-/* Bug frames table */
-__start_bug_frames = .;
-*(.bug_frames.0)
-__stop_bug_frames_0 = .;
-*(.bug_frames.1)
-__stop_bug_frames_1 = .;
-*(.bug_frames.2)
-__stop_bug_frames_2 = .;
-*(.bug_frames.3)
-__stop_bug_frames_3 = .;
+
+BUGFRAMES
+
*(.rodata)
*(.rodata.*)
*(.data.rel.ro)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 1ef6645128b2..9a1dfe1b340a 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -127,16 +127,7 @@ SECTIONS
. = ALIGN(PAGE_SIZE);
__ro_after_init_end = .;
- /* Bug frames table */
- __start_bug_frames = .;
- *(.bug_frames.0)
- __stop_bug_frames_0 = .;
- *(.bug_frames.1)
- __stop_bug_frames_1 = .;
- *(.bug_frames.2)
- __stop_bug_frames_2 = .;
- *(.bug_frames.3)
- __stop_bug_frames_3 = .;
+ BUGFRAMES
*(.rodata)
*(.rodata.*)
diff --git a/xen/include/xen/xen.lds.h b/xen/include/xen/xen.lds.h
index be90f5ca0fdb..7ab7aa567872 100644
--- a/xen/include/xen/xen.lds.h
+++ b/xen/include/xen/xen.lds.h
@@ -114,6 +114,17 @@
/* List of constructs other than *_SECTIONS in alphabetical order. */
+#define BUGFRAMES \
+__start_bug_frames = .; \
+*(.bug_frames.0)\
+__stop_bug_frames_0 = .;\
+*(.bug_frames.1)\
+__stop_bug_frames_1 = .;\
+*(.bug_frames.2)\
+__stop_bug_frames_2 = .;\
+*(.bug_frames.3)\
+__stop_bug_frames_3 = .;
+
#ifdef CONFIG_HYPFS
#define HYPFS_PARAM \
. = ALIGN(POINTER_ALIGN); \
--
2.30.2
[PATCH v2 4/4] xen/virtual-region: Drop setup_virtual_regions()
All other actions it used to perform have been converted to build-time
initialisation. The extable setup can done at build time too.
This is one fewer setup step required to get exceptions working.
Take the opportunity to move 'core' into read_mostly, where it probably should
have lived all along.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
CC: Stefano Stabellini
CC: Julien Grall
CC: Volodymyr Babchuk
CC: Bertrand Marquis
CC: Michal Orzel
CC: Oleksii Kurochko
CC: Shawn Anastasio
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
v2:
* Use CONFIG_HAS_EX_TABLE
---
xen/arch/arm/setup.c | 1 -
xen/arch/x86/setup.c | 2 --
xen/common/virtual_region.c | 19 +++
xen/include/xen/virtual_region.h | 4 ++--
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 424744ad5e1a..b9a7f61f73ef 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -719,7 +719,6 @@ void asmlinkage __init start_xen(unsigned long
boot_phys_offset,
percpu_init_areas();
set_processor_id(0); /* needed early, for smp_processor_id() */
-setup_virtual_regions(NULL, NULL);
/* Initialize traps early allow us to get backtrace when an error occurred
*/
init_traps();
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ac983ddc6977..86cd8b999774 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1014,8 +1014,6 @@ void asmlinkage __init noreturn __start_xen(unsigned long
mbi_p)
smp_prepare_boot_cpu();
sort_exception_tables();
-setup_virtual_regions(__start___ex_table, __stop___ex_table);
-
/* Full exception support from here on in. */
rdmsrl(MSR_EFER, this_cpu(efer));
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
index db3e0dc9fe74..0d0a3df0b669 100644
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -25,7 +25,7 @@ static struct virtual_region core, core_init;
#define LIST_ENTRY_CORE() { .next = _init.list, .prev =
_region_list }
#define LIST_ENTRY_INIT() { .next = _region_list, .prev = }
-static struct virtual_region core = {
+static struct virtual_region core __read_mostly = {
.list = LIST_ENTRY_CORE(),
.text_start = _stext,
.text_end = _etext,
@@ -38,6 +38,11 @@ static struct virtual_region core = {
{ __start_bug_frames_2, __stop_bug_frames_2 },
{ __start_bug_frames_3, __stop_bug_frames_3 },
},
+
+#ifdef CONFIG_HAS_EX_TABLE
+.ex = __start___ex_table,
+.ex_end = __stop___ex_table,
+#endif
};
/* Becomes irrelevant when __init sections are cleared. */
@@ -52,6 +57,11 @@ static struct virtual_region core_init __initdata = {
{ __start_bug_frames_2, __stop_bug_frames_2 },
{ __start_bug_frames_3, __stop_bug_frames_3 },
},
+
+#ifdef CONFIG_HAS_EX_TABLE
+.ex = __start___ex_table,
+.ex_end = __stop___ex_table,
+#endif
};
/*
@@ -160,13 +170,6 @@ void __init unregister_init_virtual_region(void)
remove_virtual_region(_init);
}
-void __init setup_virtual_regions(const struct exception_table_entry *start,
- const struct exception_table_entry *end)
-{
-core_init.ex = core.ex = start;
-core_init.ex_end = core.ex_end = end;
-}
-
/*
* Local variables:
* mode: C
diff --git a/xen/include/xen/virtual_region.h b/xen/include/xen/virtual_region.h
index c8a90e3ef26d..a18dfb6fb05e 100644
--- a/xen/include/xen/virtual_region.h
+++ b/xen/include/xen/virtual_region.h
@@ -32,13 +32,13 @@ struct virtual_region
const struct bug_frame *start, *stop; /* Pointers to array of bug
frames. */
} frame[BUGFRAME_NR];
+#ifdef CONFIG_HAS_EX_TABLE
const struct exception_table_entry *ex;
const struct exception_table_entry *ex_end;
+#endif
};
const struct virtual_region *find_text_region(unsigned long addr);
-void setup_virtual_regions(const struct exception_table_entry *start,
- const struct exception_table_entry *end);
void unregister_init_virtual_region(void);
void register_virtual_region(struct virtual_region *r);
void unregister_virtual_region(struct virtual_region *r);
--
2.30.2
[PATCH v2 2/4] xen/virtual-region: Rework how bugframe linkage works
The start/stop1/etc linkage scheme predates struct virtual_region, and as
setup_virtual_regions() shows, it's awkward to express in the new scheme.
Change the linker to provide explicit start/stop symbols for each bugframe
type, and change virtual_region to have a stop pointer rather than a count.
This marginly simplifies both do_bug_frame()s and prepare_payload(), but it
massively simplifies setup_virtual_regions() by allowing the compiler to
initialise the .frame[] array at build time.
virtual_region.c is the only user of the linker symbols, and this is unlikely
to change given the purpose of struct virtual_region, so move their externs
out of bug.h
No functional change.
Signed-off-by: Andrew Cooper
Reviewed-by: Ross Lagerwall
Reviewed-by: Jan Beulich
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
CC: Stefano Stabellini
CC: Julien Grall
CC: Volodymyr Babchuk
CC: Bertrand Marquis
CC: Michal Orzel
CC: Oleksii Kurochko
CC: Shawn Anastasio
CC: Konrad Rzeszutek Wilk
CC: Ross Lagerwall
---
xen/arch/arm/traps.c | 5 ++--
xen/common/bug.c | 5 ++--
xen/common/livepatch.c | 7 +++--
xen/common/virtual_region.c | 45 ++--
xen/include/xen/bug.h| 6 -
xen/include/xen/virtual_region.h | 3 +--
xen/include/xen/xen.lds.h| 8 +-
7 files changed, 35 insertions(+), 44 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9cffe7f79005..a8039087c805 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1226,10 +1226,9 @@ int do_bug_frame(const struct cpu_user_regs *regs,
vaddr_t pc)
for ( id = 0; id < BUGFRAME_NR; id++ )
{
const struct bug_frame *b;
-unsigned int i;
-for ( i = 0, b = region->frame[id].bugs;
- i < region->frame[id].n_bugs; b++, i++ )
+for ( b = region->frame[id].start;
+ b < region->frame[id].stop; b++ )
{
if ( ((vaddr_t)bug_loc(b)) == pc )
{
diff --git a/xen/common/bug.c b/xen/common/bug.c
index c43e7c439735..b7c5d8fd4d4a 100644
--- a/xen/common/bug.c
+++ b/xen/common/bug.c
@@ -25,10 +25,9 @@ int do_bug_frame(const struct cpu_user_regs *regs, unsigned
long pc)
for ( id = 0; id < BUGFRAME_NR; id++ )
{
const struct bug_frame *b;
-size_t i;
-for ( i = 0, b = region->frame[id].bugs;
- i < region->frame[id].n_bugs; b++, i++ )
+for ( b = region->frame[id].start;
+ b < region->frame[id].stop; b++ )
{
if ( bug_loc(b) == pc )
{
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index cabfb6391117..351a3e0b9a60 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -804,12 +804,11 @@ static int prepare_payload(struct payload *payload,
if ( !sec )
continue;
-if ( !section_ok(elf, sec, sizeof(*region->frame[i].bugs)) )
+if ( !section_ok(elf, sec, sizeof(*region->frame[i].start)) )
return -EINVAL;
-region->frame[i].bugs = sec->load_addr;
-region->frame[i].n_bugs = sec->sec->sh_size /
- sizeof(*region->frame[i].bugs);
+region->frame[i].start = sec->load_addr;
+region->frame[i].stop = sec->load_addr + sec->sec->sh_size;
}
sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
index 142f21e18153..7d8bdeb61282 100644
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -9,12 +9,25 @@
#include
#include
+extern const struct bug_frame
+__start_bug_frames_0[], __stop_bug_frames_0[],
+__start_bug_frames_1[], __stop_bug_frames_1[],
+__start_bug_frames_2[], __stop_bug_frames_2[],
+__start_bug_frames_3[], __stop_bug_frames_3[];
+
static struct virtual_region core = {
.list = LIST_HEAD_INIT(core.list),
.text_start = _stext,
.text_end = _etext,
.rodata_start = _srodata,
.rodata_end = _erodata,
+
+.frame = {
+{ __start_bug_frames_0, __stop_bug_frames_0 },
+{ __start_bug_frames_1, __stop_bug_frames_1 },
+{ __start_bug_frames_2, __stop_bug_frames_2 },
+{ __start_bug_frames_3, __stop_bug_frames_3 },
+},
};
/* Becomes irrelevant when __init sections are cleared. */
@@ -22,6 +35,13 @@ static struct virtual_region core_init __initdata = {
.list = LIST_HEAD_INIT(core_init.list),
.text_start = _sinittext,
.text_end = _einittext,
+
+.frame = {
+{ __start_bug_frames_0, __stop_bug_frames_0 },
+{ __start_bug_frames_1, __stop_bug_frames_1 },
+{ __start_bug_frames_2, __stop_bug_frames_2 },
+{ __start_bug_frames_3, __stop_bug_frames_3 },
+},
};
/*
@@ -133,31 +153,6 @@ void __init unregister_init_virtual_region(void)
void __init
[PATCH v2 0/4] xen/arch: Simplify virtual_region setup
There is nothing that setup_virtual_regions() does which can't be done at build time. Make this happen. Importantly, this removes logic from needed prior to setting up exceptions. v2: * Only minor changes in patches 3 and 4. Andrew Cooper (4): xen/link: Introduce a common BUGFRAMES definition xen/virtual-region: Rework how bugframe linkage works xen/virtual-region: Link the list build time xen/virtual-region: Drop setup_virtual_regions() xen/arch/arm/setup.c | 1 - xen/arch/arm/traps.c | 5 +- xen/arch/arm/xen.lds.S | 13 +--- xen/arch/ppc/xen.lds.S | 13 +--- xen/arch/riscv/xen.lds.S | 13 +--- xen/arch/x86/setup.c | 2 - xen/arch/x86/xen.lds.S | 11 +--- xen/common/bug.c | 5 +- xen/common/livepatch.c | 7 +-- xen/common/virtual_region.c | 101 --- xen/include/xen/bug.h| 6 -- xen/include/xen/virtual_region.h | 7 +-- xen/include/xen/xen.lds.h| 17 ++ 13 files changed, 90 insertions(+), 111 deletions(-) base-commit: 0e7ea8ca5fc9bce9248414f6aaf2dc861abd45d9 -- 2.30.2
[xen-4.18-testing test] 185285: tolerable FAIL - PUSHED
flight 185285 xen-4.18-testing real [real] flight 185301 xen-4.18-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/185285/ http://logs.test-lab.xenproject.org/osstest/logs/185301/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-libvirt 8 xen-bootfail pass in 185301-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-check fail in 185301 like 185218 test-armhf-armhf-libvirt15 migrate-support-check fail in 185301 never pass test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 185218 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 185218 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 185218 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 185218 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 185218 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-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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 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-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 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-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-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-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-qcow214 migrate-support-checkfail never pass test-armhf-armhf-xl-qcow215 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 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: xen 1166467ed300d605529aaf7a7d26c8c92defe36a baseline version: xen 17cf285d87e28a9ee9ab1f192982fc6dfc9e4193 Last test of basis 185218 2024-04-02 14:38:50 Z8 days Testing same since 185285 2024-04-09 12:07:14 Z1 days1 attempts People who touched revisions under test: Bjoern Doebel Jan Beulich jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64
Re: [PATCH v2 2/2] x86/Kconfig: Introduce CONFIG_{AMD,INTEL} and conditionalise ucode
On Wed, 10 Apr 2024, Roger Pau Monné wrote: > On Wed, Apr 10, 2024 at 05:21:37PM +0100, Andrew Cooper wrote: > > On 10/04/2024 4:14 pm, Roger Pau Monné wrote: > > > On Thu, Oct 26, 2023 at 09:55:39PM +0100, Andrew Cooper wrote: > > >> + > > >> +config AMD > > >> +bool "AMD" > > >> +default y > > >> +help > > >> + Detection, tunings and quirks for AMD platforms. > > >> + > > >> + May be turned off in builds targetting other vendors. > > >> Otherwise, > > >> + must be enabled for Xen to work suitably on AMD platforms. > > >> + > > >> +config INTEL > > >> +bool "Intel" > > >> +default y > > >> +help > > >> + Detection, tunings and quirks for Intel platforms. > > >> + > > >> + May be turned off in builds targetting other vendors. > > >> Otherwise, > > >> + must be enabled for Xen to work suitably on Intel platforms. > > > There seems to be a weird mix between hard tabs and spaces above. > > > Naming is OK for me. > > > > Yeah. I already fixed those locally. > > With that fixed: > > Acked-by: Roger Pau Monné This is fine for me too Acked-by: Stefano Stabellini
Re: [PATCH v2 2/2] x86/Kconfig: Introduce CONFIG_{AMD,INTEL} and conditionalise ucode
On 10/04/2024 5:34 pm, Roger Pau Monné wrote:
> On Wed, Apr 10, 2024 at 05:21:37PM +0100, Andrew Cooper wrote:
>> On 10/04/2024 4:14 pm, Roger Pau Monné wrote:
>>> On Thu, Oct 26, 2023 at 09:55:39PM +0100, Andrew Cooper wrote:
+
+config AMD
+ bool "AMD"
After double checking what {menu,old}config looks like, I've extended
these prompts "Support $X CPU" so they make more sense in the context
they're asked in.
+default y
+help
+ Detection, tunings and quirks for AMD platforms.
+
+May be turned off in builds targetting other vendors. Otherwise,
+must be enabled for Xen to work suitably on AMD platforms.
+
+config INTEL
+ bool "Intel"
+default y
+help
+ Detection, tunings and quirks for Intel platforms.
+
+May be turned off in builds targetting other vendors. Otherwise,
+must be enabled for Xen to work suitably on Intel platforms.
>>> There seems to be a weird mix between hard tabs and spaces above.
>>> Naming is OK for me.
>> Yeah. I already fixed those locally.
> With that fixed:
>
> Acked-by: Roger Pau Monné
Thanks.
We can always tweak later if necessary.
~Andrew
Re: [PATCH RFC v3 for-6.8/block 09/17] btrfs: use bdev apis
On Thu, Jan 04, 2024 at 12:49:58PM +0100, Jan Kara wrote: > On Sat 23-12-23 17:31:55, Matthew Wilcox wrote: > > On Thu, Dec 21, 2023 at 04:57:04PM +0800, Yu Kuai wrote: > > > @@ -3674,16 +3670,17 @@ struct btrfs_super_block > > > *btrfs_read_dev_one_super(struct block_device *bdev, > > >* Drop the page of the primary superblock, so later read will > > >* always read from the device. > > >*/ > > > - invalidate_inode_pages2_range(mapping, > > > - bytenr >> PAGE_SHIFT, > > > + invalidate_bdev_range(bdev, bytenr >> PAGE_SHIFT, > > > (bytenr + BTRFS_SUPER_INFO_SIZE) >> PAGE_SHIFT); > > > } > > > > > > - page = read_cache_page_gfp(mapping, bytenr >> PAGE_SHIFT, GFP_NOFS); > > > - if (IS_ERR(page)) > > > - return ERR_CAST(page); > > > + nofs_flag = memalloc_nofs_save(); > > > + folio = bdev_read_folio(bdev, bytenr); > > > + memalloc_nofs_restore(nofs_flag); > > > > This is the wrong way to use memalloc_nofs_save/restore. They should be > > used at the point that the filesystem takes/releases whatever lock is > > also used during reclaim. I don't know btrfs well enough to suggest > > what lock is missing these annotations. > > In principle I agree with you but in this particular case I agree the ask > is just too big. I suspect it is one of btrfs btree locks or maybe > chunk_mutex but I doubt even btrfs developers know and maybe it is just a > cargo cult. And it is not like this would be the first occurence of this > anti-pattern in btrfs - see e.g. device_list_add(), add_missing_dev(), > btrfs_destroy_delalloc_inodes() (here the wrapping around > invalidate_inode_pages2() looks really weird), and many others... The pattern is intentional and a temporary solution before we could implement the scoped NOFS. Functions calling allocations get converted from GFP_NOFS to GFP_KERNEL but in case they're called from a context that either holds big locks or can recursively enter the filesystem then it's protected by the memalloc calls. This should not be surprising. What may not be obvious is which locks or kmalloc calling functions it could be, this depends on the analysis of the function call chain and usually there's enough evidence why it's needed.
Re: [PATCH v1 0/2] AMD SEV initial work
Also sent emacs ~ file. To ignore, sorry Le mercredi 10/04/2024 17:39, Andrei Semenov a écrit : ///HERE YOU GO Andrei Semenov (2): Implemented AMD SEV discovery and enabling. Implemented Amd Secure Processor device driver xen/arch/x86/cpu/amd.c | 53 ++ xen/arch/x86/hvm/svm/Makefile | 1 + xen/arch/x86/hvm/svm/sev.c | 4 + xen/arch/x86/include/asm/cpufeature.h | 3 + xen/arch/x86/include/asm/cpufeatures.h | 2 + xen/arch/x86/include/asm/msr-index.h | 1 + xen/arch/x86/include/asm/psp-sev.h | 655 xen/arch/x86/include/asm/sev.h | 11 + xen/drivers/Kconfig| 2 + xen/drivers/Makefile | 1 + xen/drivers/crypto/Kconfig | 10 + xen/drivers/crypto/Makefile| 1 + xen/drivers/crypto/asp.c | 808 + xen/include/xen/types.h| 2 +- 14 files changed, 1553 insertions(+), 1 deletion(-) create mode 100644 xen/arch/x86/hvm/svm/sev.c create mode 100644 xen/arch/x86/include/asm/psp-sev.h create mode 100644 xen/arch/x86/include/asm/sev.h create mode 100644 xen/drivers/crypto/Kconfig create mode 100644 xen/drivers/crypto/Makefile create mode 100644 xen/drivers/crypto/asp.c -- 2.35.3
Re: [PATCH v2 2/2] x86/Kconfig: Introduce CONFIG_{AMD,INTEL} and conditionalise ucode
On Wed, Apr 10, 2024 at 05:21:37PM +0100, Andrew Cooper wrote:
> On 10/04/2024 4:14 pm, Roger Pau Monné wrote:
> > On Thu, Oct 26, 2023 at 09:55:39PM +0100, Andrew Cooper wrote:
> >> +
> >> +config AMD
> >> + bool "AMD"
> >> +default y
> >> +help
> >> + Detection, tunings and quirks for AMD platforms.
> >> +
> >> +May be turned off in builds targetting other vendors. Otherwise,
> >> +must be enabled for Xen to work suitably on AMD platforms.
> >> +
> >> +config INTEL
> >> + bool "Intel"
> >> +default y
> >> +help
> >> + Detection, tunings and quirks for Intel platforms.
> >> +
> >> +May be turned off in builds targetting other vendors. Otherwise,
> >> +must be enabled for Xen to work suitably on Intel platforms.
> > There seems to be a weird mix between hard tabs and spaces above.
> > Naming is OK for me.
>
> Yeah. I already fixed those locally.
With that fixed:
Acked-by: Roger Pau Monné
> >> +
> >> +endmenu
> >> diff --git a/xen/arch/x86/cpu/microcode/Makefile
> >> b/xen/arch/x86/cpu/microcode/Makefile
> >> index aae235245b06..30d600544f45 100644
> >> --- a/xen/arch/x86/cpu/microcode/Makefile
> >> +++ b/xen/arch/x86/cpu/microcode/Makefile
> >> @@ -1,3 +1,3 @@
> >> -obj-y += amd.o
> >> +obj-$(CONFIG_AMD) += amd.o
> >> obj-y += core.o
> >> -obj-y += intel.o
> >> +obj-$(CONFIG_INTEL) += intel.o
> >> diff --git a/xen/arch/x86/cpu/microcode/private.h
> >> b/xen/arch/x86/cpu/microcode/private.h
> >> index b58611e908aa..da556fe5060a 100644
> >> --- a/xen/arch/x86/cpu/microcode/private.h
> >> +++ b/xen/arch/x86/cpu/microcode/private.h
> >> @@ -70,7 +70,16 @@ struct microcode_ops {
> >> * support available) and (not) ops->apply_microcode (i.e. read only).
> >> * Otherwise, all hooks must be filled in.
> >> */
> >> +#ifdef CONFIG_AMD
> >> void ucode_probe_amd(struct microcode_ops *ops);
> >> +#else
> >> +static inline void ucode_probe_amd(struct microcode_ops *ops) {}
> >> +#endif
> >> +
> >> +#ifdef CONFIG_INTEL
> >> void ucode_probe_intel(struct microcode_ops *ops);
> >> +#else
> >> +static inline void ucode_probe_intel(struct microcode_ops *ops) {}
> > This is stale now, and will need some updating to match what's in
> > private.h.
>
> There's nothing state I can see.
>
> Patch 1 does significantly edit this vs what's currently in staging.
Oh, sorry, I'm missed patch 1 then.
Thanks, Roger.
[xen-4.17-testing test] 185284: tolerable FAIL - PUSHED
flight 185284 xen-4.17-testing real [real] flight 185298 xen-4.17-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/185284/ http://logs.test-lab.xenproject.org/osstest/logs/185298/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-qcow2 8 xen-bootfail pass in 185298-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-qcow2 14 migrate-support-check fail in 185298 never pass test-armhf-armhf-xl-qcow2 15 saverestore-support-check fail in 185298 never pass test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 185217 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 185217 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 185217 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 185217 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 185217 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 185217 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 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-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-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-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-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 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass version targeted for testing: xen b8f39fd4d024ea72c586f1afd233f379c6f6230b baseline version: xen 9bc40dbcf9eafccc1923b2555286bf6a2af03b7a Last test of basis 185217 2024-04-02 14:38:50 Z8 days Testing same since 185284 2024-04-09 12:07:06 Z1 days1 attempts People who touched revisions under test: Andrew Cooper Bjoern Doebel Jan Beulich jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64
Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
Hi Jens,
> On 10 Apr 2024, at 17:45, Jens Wiklander wrote:
>
> On Tue, Apr 9, 2024 at 5:36 PM Jens Wiklander
> wrote:
>>
>> Add support for FF-A notifications, currently limited to an SP (Secure
>> Partition) sending an asynchronous notification to a guest.
>>
>> Guests and Xen itself are made aware of pending notifications with an
>> interrupt. The interrupt handler retrieves the notifications using the
>> FF-A ABI and deliver them to their destinations.
>>
>> Signed-off-by: Jens Wiklander
>> ---
>> xen/arch/arm/tee/Makefile | 1 +
>> xen/arch/arm/tee/ffa.c | 58 ++
>> xen/arch/arm/tee/ffa_notif.c | 319 +
>> xen/arch/arm/tee/ffa_private.h | 71
>> 4 files changed, 449 insertions(+)
>> create mode 100644 xen/arch/arm/tee/ffa_notif.c
>>
>> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
>> index f0112a2f922d..7c0f46f7f446 100644
>> --- a/xen/arch/arm/tee/Makefile
>> +++ b/xen/arch/arm/tee/Makefile
>> @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
>> obj-$(CONFIG_FFA) += ffa_shm.o
>> obj-$(CONFIG_FFA) += ffa_partinfo.o
>> obj-$(CONFIG_FFA) += ffa_rxtx.o
>> +obj-$(CONFIG_FFA) += ffa_notif.o
>> obj-y += tee.o
>> obj-$(CONFIG_OPTEE) += optee.o
>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>> index 5209612963e1..ce9757bfeed1 100644
>> --- a/xen/arch/arm/tee/ffa.c
>> +++ b/xen/arch/arm/tee/ffa.c
>> @@ -39,6 +39,9 @@
>> * - at most 32 shared memory regions per guest
>> * o FFA_MSG_SEND_DIRECT_REQ:
>> * - only supported from a VM to an SP
>> + * o FFA_NOTIFICATION_*:
>> + * - only supports global notifications, that is, per vCPU notifications
>> + * are not supported
>> *
>> * There are some large locked sections with ffa_tx_buffer_lock and
>> * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
>> @@ -194,6 +197,8 @@ out:
>>
>> static void handle_features(struct cpu_user_regs *regs)
>> {
>> +struct domain *d = current->domain;
>> +struct ffa_ctx *ctx = d->arch.tee;
>> uint32_t a1 = get_user_reg(regs, 1);
>> unsigned int n;
>>
>> @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs *regs)
>> BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
>> ffa_set_regs_success(regs, 0, 0);
>> break;
>> +case FFA_FEATURE_NOTIF_PEND_INTR:
>> +if ( ctx->notif.enabled )
>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>> +else
>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>> +break;
>> +case FFA_FEATURE_SCHEDULE_RECV_INTR:
>> +if ( ctx->notif.enabled )
>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>> +else
>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>> +break;
>
> With the recently posted kernel patch
> https://lore.kernel.org/all/20240410-ffa_npi_support-v1-3-1a5223391...@arm.com/
> we need to decide which feature interrupt to return since the kernel
> will only install a handle for the first it finds. Right now I propose
> to to not report FFA_FEATURE_SCHEDULE_RECV_INTR. When the time comes
> to use a secondary scheduler we'll need to report the SRI instead.
We just had a meeting with Sudeep to discuss that matter and he agreed that
he would register the interrupt handler for all the interrupts available so that
we can keep a model where we will generate SRIs only to a secondary scheduler
and NPI for notification interrupts (so that the VM does not do a INFO_GET when
not required).
We will have to report both as any VM could act as secondary scheduler for SPs
in theory (we might need at some point a parameter for that) but for now those
should only be generated to Dom0 if there are pending notifications for SPs.
Cheers
Bertrand
Re: [PATCH v2 2/2] x86/Kconfig: Introduce CONFIG_{AMD,INTEL} and conditionalise ucode
On 10/04/2024 4:14 pm, Roger Pau Monné wrote:
> On Thu, Oct 26, 2023 at 09:55:39PM +0100, Andrew Cooper wrote:
>> We eventually want to be able to build a stripped down Xen for a single
>> platform. Make a start with CONFIG_{AMD,INTEL} (hidden behind EXPERT, but
>> available to randconfig), and adjust the microcode logic.
>>
>> No practical change.
>>
>> Signed-off-by: Andrew Cooper
>> ---
>> CC: Jan Beulich
>> CC: Roger Pau Monné
>> CC: Wei Liu
>> CC: Alejandro Vallejo
>> CC: Stefano Stabellini
>> CC: Xenia Ragiadakou
>>
>> I've intentionally ignored the other vendors for now. They can be put into
>> Kconfig by whomever figures out the actual dependencies between their init
>> routines.
>>
>> v2:
>> * Tweak text
>> ---
>> xen/arch/x86/Kconfig | 2 ++
>> xen/arch/x86/Kconfig.cpu | 22 ++
>> xen/arch/x86/cpu/microcode/Makefile | 4 ++--
>> xen/arch/x86/cpu/microcode/private.h | 9 +
>> 4 files changed, 35 insertions(+), 2 deletions(-)
>> create mode 100644 xen/arch/x86/Kconfig.cpu
>>
>> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
>> index eac77573bd75..d9eacdd7e0fa 100644
>> --- a/xen/arch/x86/Kconfig
>> +++ b/xen/arch/x86/Kconfig
>> @@ -49,6 +49,8 @@ config HAS_CC_CET_IBT
>>
>> menu "Architecture Features"
>>
>> +source "arch/x86/Kconfig.cpu"
> Since we are not targeting at the CPU only, would this be better named
> as Kconfig.vendor? Or Kconfig.platform? (I'm OK if you prefer to
> leave as .cpu, just suggesting more neutral names.
Well - its based on ...
>
>> +
>> source "arch/Kconfig"
>>
>> config PV
>> diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
>> new file mode 100644
>> index ..3c5d88fdfd16
>> --- /dev/null
>> +++ b/xen/arch/x86/Kconfig.cpu
>> @@ -0,0 +1,22 @@
>> +menu "Supported CPU vendors"
>> +visible if EXPERT
... this, and I think "cpu" is the thing that is going to be most
meaningful to people. (Also because this is what Linux calls the file.)
Technically platform would be the right term, but "CPU vendors" is what
you call Intel / AMD / etc in the x86 world.
>> +
>> +config AMD
>> +bool "AMD"
>> +default y
>> +help
>> + Detection, tunings and quirks for AMD platforms.
>> +
>> + May be turned off in builds targetting other vendors. Otherwise,
>> + must be enabled for Xen to work suitably on AMD platforms.
>> +
>> +config INTEL
>> +bool "Intel"
>> +default y
>> +help
>> + Detection, tunings and quirks for Intel platforms.
>> +
>> + May be turned off in builds targetting other vendors. Otherwise,
>> + must be enabled for Xen to work suitably on Intel platforms.
> There seems to be a weird mix between hard tabs and spaces above.
> Naming is OK for me.
Yeah. I already fixed those locally.
>
>> +
>> +endmenu
>> diff --git a/xen/arch/x86/cpu/microcode/Makefile
>> b/xen/arch/x86/cpu/microcode/Makefile
>> index aae235245b06..30d600544f45 100644
>> --- a/xen/arch/x86/cpu/microcode/Makefile
>> +++ b/xen/arch/x86/cpu/microcode/Makefile
>> @@ -1,3 +1,3 @@
>> -obj-y += amd.o
>> +obj-$(CONFIG_AMD) += amd.o
>> obj-y += core.o
>> -obj-y += intel.o
>> +obj-$(CONFIG_INTEL) += intel.o
>> diff --git a/xen/arch/x86/cpu/microcode/private.h
>> b/xen/arch/x86/cpu/microcode/private.h
>> index b58611e908aa..da556fe5060a 100644
>> --- a/xen/arch/x86/cpu/microcode/private.h
>> +++ b/xen/arch/x86/cpu/microcode/private.h
>> @@ -70,7 +70,16 @@ struct microcode_ops {
>> * support available) and (not) ops->apply_microcode (i.e. read only).
>> * Otherwise, all hooks must be filled in.
>> */
>> +#ifdef CONFIG_AMD
>> void ucode_probe_amd(struct microcode_ops *ops);
>> +#else
>> +static inline void ucode_probe_amd(struct microcode_ops *ops) {}
>> +#endif
>> +
>> +#ifdef CONFIG_INTEL
>> void ucode_probe_intel(struct microcode_ops *ops);
>> +#else
>> +static inline void ucode_probe_intel(struct microcode_ops *ops) {}
> This is stale now, and will need some updating to match what's in
> private.h.
There's nothing state I can see.
Patch 1 does significantly edit this vs what's currently in staging.
~Andrew
Re: [RFC PATCH v1 0/2] convert LIBXL_DEVICE_MODEL_START_TIMEOUT to env var
On Wed, Apr 10, 2024 at 02:43:13PM +0300, Manos Pitsidianakis wrote: > This patch series proposes converting the compile-time define > LIBXL_DEVICE_MODEL_START_TIMEOUT value to an optionally overridden by > environment variable value, just like the current behavior for > LIBXL_BOOTLOADER_TIMEOUT is. > > Manos Pitsidianakis (2): > libs/light: add device model start timeout env var > libs/light: expand device model start timeout use > > docs/man/xl.1.pod.in | 11 +++ > tools/libs/light/libxl_9pfs.c| 2 +- > tools/libs/light/libxl_device.c | 2 +- > tools/libs/light/libxl_dm.c | 10 +- > tools/libs/light/libxl_dom_suspend.c | 2 +- > tools/libs/light/libxl_domain.c | 5 +++-- > tools/libs/light/libxl_internal.h| 6 ++ > tools/libs/light/libxl_pci.c | 10 +- > tools/libs/light/libxl_usb.c | 8 > 9 files changed, 37 insertions(+), 19 deletions(-) > > > base-commit: f48299cad5c3c69fdc2c101517a6dab9c9827ea5 > -- > γαῖα πυρί μιχθήτω > > Hi Manos, Did you know that you could run something like `git send-email --cc-cmd="scripts/get_maintainer.pl --no-l" ...` and git would CC the maintainers of the code? I've configure my xen repo to have that been automatic, with git config sendemail.cccmd 'cd `git rev-parse --show-toplevel` && scripts/get_maintainer.pl --no-l' There's other way to send patch, like using "scripts/add_maintainers.pl" described on this page: https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Sending_a_Patch_Series Not sure which one is better. Anyway, I've added the series to my list of patch to look at. But I might miss it next time if I'm not CCed. Cheers, -- Anthony PERARD
Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
Hi Bertrand,
On Wed, Apr 10, 2024 at 5:41 PM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 10 Apr 2024, at 16:27, Jens Wiklander wrote:
> >
> > On Wed, Apr 10, 2024 at 9:49 AM Bertrand Marquis
> > wrote:
> >>
> >> Hi Jens,
> >>
> >>> On 9 Apr 2024, at 17:36, Jens Wiklander wrote:
> >>>
> >>> Add support for FF-A notifications, currently limited to an SP (Secure
> >>> Partition) sending an asynchronous notification to a guest.
> >>>
> >>> Guests and Xen itself are made aware of pending notifications with an
> >>> interrupt. The interrupt handler retrieves the notifications using the
> >>> FF-A ABI and deliver them to their destinations.
> >>>
> >>> Signed-off-by: Jens Wiklander
> >>> ---
> >>> xen/arch/arm/tee/Makefile | 1 +
> >>> xen/arch/arm/tee/ffa.c | 58 ++
> >>> xen/arch/arm/tee/ffa_notif.c | 319 +
> >>> xen/arch/arm/tee/ffa_private.h | 71
> >>> 4 files changed, 449 insertions(+)
> >>> create mode 100644 xen/arch/arm/tee/ffa_notif.c
> >>>
> >>> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
> >>> index f0112a2f922d..7c0f46f7f446 100644
> >>> --- a/xen/arch/arm/tee/Makefile
> >>> +++ b/xen/arch/arm/tee/Makefile
> >>> @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
> >>> obj-$(CONFIG_FFA) += ffa_shm.o
> >>> obj-$(CONFIG_FFA) += ffa_partinfo.o
> >>> obj-$(CONFIG_FFA) += ffa_rxtx.o
> >>> +obj-$(CONFIG_FFA) += ffa_notif.o
> >>> obj-y += tee.o
> >>> obj-$(CONFIG_OPTEE) += optee.o
> >>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> >>> index 5209612963e1..ce9757bfeed1 100644
> >>> --- a/xen/arch/arm/tee/ffa.c
> >>> +++ b/xen/arch/arm/tee/ffa.c
> >>> @@ -39,6 +39,9 @@
> >>> * - at most 32 shared memory regions per guest
> >>> * o FFA_MSG_SEND_DIRECT_REQ:
> >>> * - only supported from a VM to an SP
> >>> + * o FFA_NOTIFICATION_*:
> >>> + * - only supports global notifications, that is, per vCPU
> >>> notifications
> >>> + * are not supported
> >>> *
> >>> * There are some large locked sections with ffa_tx_buffer_lock and
> >>> * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
> >>> @@ -194,6 +197,8 @@ out:
> >>>
> >>> static void handle_features(struct cpu_user_regs *regs)
> >>> {
> >>> +struct domain *d = current->domain;
> >>> +struct ffa_ctx *ctx = d->arch.tee;
> >>>uint32_t a1 = get_user_reg(regs, 1);
> >>>unsigned int n;
> >>>
> >>> @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs
> >>> *regs)
> >>>BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
> >>>ffa_set_regs_success(regs, 0, 0);
> >>>break;
> >>> +case FFA_FEATURE_NOTIF_PEND_INTR:
> >>> +if ( ctx->notif.enabled )
> >>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
> >>> +else
> >>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> >>> +break;
> >>> +case FFA_FEATURE_SCHEDULE_RECV_INTR:
> >>> +if ( ctx->notif.enabled )
> >>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
> >>
> >> This should return the RECV_INTR, not the PEND one.
> >
> > Thanks, I'll fix it.
> >
> >>
> >>> +else
> >>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> >>> +break;
> >>> +
> >>> +case FFA_NOTIFICATION_BIND:
> >>> +case FFA_NOTIFICATION_UNBIND:
> >>> +case FFA_NOTIFICATION_GET:
> >>> +case FFA_NOTIFICATION_SET:
> >>> +case FFA_NOTIFICATION_INFO_GET_32:
> >>> +case FFA_NOTIFICATION_INFO_GET_64:
> >>> +if ( ctx->notif.enabled )
> >>> +ffa_set_regs_success(regs, 0, 0);
> >>> +else
> >>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> >>> +break;
> >>>default:
> >>>ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> >>>break;
> >>> @@ -305,6 +334,30 @@ static bool ffa_handle_call(struct cpu_user_regs
> >>> *regs)
> >>> get_user_reg(regs,
> >>> 1)),
> >>> get_user_reg(regs, 3));
> >>>break;
> >>> +case FFA_NOTIFICATION_BIND:
> >>> +e = ffa_handle_notification_bind(get_user_reg(regs, 1),
> >>> + get_user_reg(regs, 2),
> >>> + get_user_reg(regs, 3),
> >>> + get_user_reg(regs, 4));
> >>
> >> I would suggest to pass regs and handle the get_user_regs in the function.
> >
> > OK
> >
> >>
> >>> +break;
> >>> +case FFA_NOTIFICATION_UNBIND:
> >>> +e = ffa_handle_notification_unbind(get_user_reg(regs, 1),
> >>> + get_user_reg(regs, 3),
> >>> + get_user_reg(regs, 4));
> >>
> >> same here
> >
> > OK
> >
> >>
> >>> +break;
> >>> +case FFA_NOTIFICATION_INFO_GET_32:
> >>> +case FFA_NOTIFICATION_INFO_GET_64:
> >>> +
Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
On Tue, Apr 9, 2024 at 5:36 PM Jens Wiklander wrote:
>
> Add support for FF-A notifications, currently limited to an SP (Secure
> Partition) sending an asynchronous notification to a guest.
>
> Guests and Xen itself are made aware of pending notifications with an
> interrupt. The interrupt handler retrieves the notifications using the
> FF-A ABI and deliver them to their destinations.
>
> Signed-off-by: Jens Wiklander
> ---
> xen/arch/arm/tee/Makefile | 1 +
> xen/arch/arm/tee/ffa.c | 58 ++
> xen/arch/arm/tee/ffa_notif.c | 319 +
> xen/arch/arm/tee/ffa_private.h | 71
> 4 files changed, 449 insertions(+)
> create mode 100644 xen/arch/arm/tee/ffa_notif.c
>
> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
> index f0112a2f922d..7c0f46f7f446 100644
> --- a/xen/arch/arm/tee/Makefile
> +++ b/xen/arch/arm/tee/Makefile
> @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
> obj-$(CONFIG_FFA) += ffa_shm.o
> obj-$(CONFIG_FFA) += ffa_partinfo.o
> obj-$(CONFIG_FFA) += ffa_rxtx.o
> +obj-$(CONFIG_FFA) += ffa_notif.o
> obj-y += tee.o
> obj-$(CONFIG_OPTEE) += optee.o
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 5209612963e1..ce9757bfeed1 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -39,6 +39,9 @@
> * - at most 32 shared memory regions per guest
> * o FFA_MSG_SEND_DIRECT_REQ:
> * - only supported from a VM to an SP
> + * o FFA_NOTIFICATION_*:
> + * - only supports global notifications, that is, per vCPU notifications
> + * are not supported
> *
> * There are some large locked sections with ffa_tx_buffer_lock and
> * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
> @@ -194,6 +197,8 @@ out:
>
> static void handle_features(struct cpu_user_regs *regs)
> {
> +struct domain *d = current->domain;
> +struct ffa_ctx *ctx = d->arch.tee;
> uint32_t a1 = get_user_reg(regs, 1);
> unsigned int n;
>
> @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs *regs)
> BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
> ffa_set_regs_success(regs, 0, 0);
> break;
> +case FFA_FEATURE_NOTIF_PEND_INTR:
> +if ( ctx->notif.enabled )
> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
> +else
> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +break;
> +case FFA_FEATURE_SCHEDULE_RECV_INTR:
> +if ( ctx->notif.enabled )
> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
> +else
> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +break;
With the recently posted kernel patch
https://lore.kernel.org/all/20240410-ffa_npi_support-v1-3-1a5223391...@arm.com/
we need to decide which feature interrupt to return since the kernel
will only install a handle for the first it finds. Right now I propose
to to not report FFA_FEATURE_SCHEDULE_RECV_INTR. When the time comes
to use a secondary scheduler we'll need to report the SRI instead.
Cheers,
Jens
> +
> +case FFA_NOTIFICATION_BIND:
> +case FFA_NOTIFICATION_UNBIND:
> +case FFA_NOTIFICATION_GET:
> +case FFA_NOTIFICATION_SET:
> +case FFA_NOTIFICATION_INFO_GET_32:
> +case FFA_NOTIFICATION_INFO_GET_64:
> +if ( ctx->notif.enabled )
> +ffa_set_regs_success(regs, 0, 0);
> +else
> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +break;
> default:
> ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> break;
> @@ -305,6 +334,30 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
> get_user_reg(regs, 1)),
> get_user_reg(regs, 3));
> break;
> +case FFA_NOTIFICATION_BIND:
> +e = ffa_handle_notification_bind(get_user_reg(regs, 1),
> + get_user_reg(regs, 2),
> + get_user_reg(regs, 3),
> + get_user_reg(regs, 4));
> +break;
> +case FFA_NOTIFICATION_UNBIND:
> +e = ffa_handle_notification_unbind(get_user_reg(regs, 1),
> + get_user_reg(regs, 3),
> + get_user_reg(regs, 4));
> +break;
> +case FFA_NOTIFICATION_INFO_GET_32:
> +case FFA_NOTIFICATION_INFO_GET_64:
> +ffa_handle_notification_info_get(regs);
> +return true;
> +case FFA_NOTIFICATION_GET:
> +ffa_handle_notification_get(re
Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
Hi Jens,
> On 10 Apr 2024, at 16:27, Jens Wiklander wrote:
>
> On Wed, Apr 10, 2024 at 9:49 AM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 9 Apr 2024, at 17:36, Jens Wiklander wrote:
>>>
>>> Add support for FF-A notifications, currently limited to an SP (Secure
>>> Partition) sending an asynchronous notification to a guest.
>>>
>>> Guests and Xen itself are made aware of pending notifications with an
>>> interrupt. The interrupt handler retrieves the notifications using the
>>> FF-A ABI and deliver them to their destinations.
>>>
>>> Signed-off-by: Jens Wiklander
>>> ---
>>> xen/arch/arm/tee/Makefile | 1 +
>>> xen/arch/arm/tee/ffa.c | 58 ++
>>> xen/arch/arm/tee/ffa_notif.c | 319 +
>>> xen/arch/arm/tee/ffa_private.h | 71
>>> 4 files changed, 449 insertions(+)
>>> create mode 100644 xen/arch/arm/tee/ffa_notif.c
>>>
>>> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
>>> index f0112a2f922d..7c0f46f7f446 100644
>>> --- a/xen/arch/arm/tee/Makefile
>>> +++ b/xen/arch/arm/tee/Makefile
>>> @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
>>> obj-$(CONFIG_FFA) += ffa_shm.o
>>> obj-$(CONFIG_FFA) += ffa_partinfo.o
>>> obj-$(CONFIG_FFA) += ffa_rxtx.o
>>> +obj-$(CONFIG_FFA) += ffa_notif.o
>>> obj-y += tee.o
>>> obj-$(CONFIG_OPTEE) += optee.o
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index 5209612963e1..ce9757bfeed1 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -39,6 +39,9 @@
>>> * - at most 32 shared memory regions per guest
>>> * o FFA_MSG_SEND_DIRECT_REQ:
>>> * - only supported from a VM to an SP
>>> + * o FFA_NOTIFICATION_*:
>>> + * - only supports global notifications, that is, per vCPU notifications
>>> + * are not supported
>>> *
>>> * There are some large locked sections with ffa_tx_buffer_lock and
>>> * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
>>> @@ -194,6 +197,8 @@ out:
>>>
>>> static void handle_features(struct cpu_user_regs *regs)
>>> {
>>> +struct domain *d = current->domain;
>>> +struct ffa_ctx *ctx = d->arch.tee;
>>>uint32_t a1 = get_user_reg(regs, 1);
>>>unsigned int n;
>>>
>>> @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs *regs)
>>>BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
>>>ffa_set_regs_success(regs, 0, 0);
>>>break;
>>> +case FFA_FEATURE_NOTIF_PEND_INTR:
>>> +if ( ctx->notif.enabled )
>>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>>> +else
>>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +break;
>>> +case FFA_FEATURE_SCHEDULE_RECV_INTR:
>>> +if ( ctx->notif.enabled )
>>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>>
>> This should return the RECV_INTR, not the PEND one.
>
> Thanks, I'll fix it.
>
>>
>>> +else
>>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +break;
>>> +
>>> +case FFA_NOTIFICATION_BIND:
>>> +case FFA_NOTIFICATION_UNBIND:
>>> +case FFA_NOTIFICATION_GET:
>>> +case FFA_NOTIFICATION_SET:
>>> +case FFA_NOTIFICATION_INFO_GET_32:
>>> +case FFA_NOTIFICATION_INFO_GET_64:
>>> +if ( ctx->notif.enabled )
>>> +ffa_set_regs_success(regs, 0, 0);
>>> +else
>>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +break;
>>>default:
>>>ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>>break;
>>> @@ -305,6 +334,30 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
>>> get_user_reg(regs, 1)),
>>> get_user_reg(regs, 3));
>>>break;
>>> +case FFA_NOTIFICATION_BIND:
>>> +e = ffa_handle_notification_bind(get_user_reg(regs, 1),
>>> + get_user_reg(regs, 2),
>>> + get_user_reg(regs, 3),
>>> + get_user_reg(regs, 4));
>>
>> I would suggest to pass regs and handle the get_user_regs in the function.
>
> OK
>
>>
>>> +break;
>>> +case FFA_NOTIFICATION_UNBIND:
>>> +e = ffa_handle_notification_unbind(get_user_reg(regs, 1),
>>> + get_user_reg(regs, 3),
>>> + get_user_reg(regs, 4));
>>
>> same here
>
> OK
>
>>
>>> +break;
>>> +case FFA_NOTIFICATION_INFO_GET_32:
>>> +case FFA_NOTIFICATION_INFO_GET_64:
>>> +ffa_handle_notification_info_get(regs);
>>> +return true;
>>> +case FFA_NOTIFICATION_GET:
>>> +ffa_handle_notification_get(regs);
>>> +return true;
>>> +case FFA_NOTIFICATION_SET:
>>> +e = ffa_handle_notification_set(get_user_reg(regs, 1),
>>> +get_user_reg(regs, 2),
>>> +
[PATCH v1 1/2] Implemented AMD SEV discovery and enabling.
Signed-off-by: Andrei Semenov
---
xen/arch/x86/cpu/amd.c | 53 ++
xen/arch/x86/hvm/svm/Makefile | 1 +
xen/arch/x86/hvm/svm/sev.c | 4 ++
xen/arch/x86/include/asm/cpufeature.h | 3 ++
xen/arch/x86/include/asm/cpufeatures.h | 2 +
xen/arch/x86/include/asm/msr-index.h | 1 +
xen/arch/x86/include/asm/sev.h | 11 ++
7 files changed, 75 insertions(+)
create mode 100644 xen/arch/x86/hvm/svm/sev.c
create mode 100644 xen/arch/x86/include/asm/sev.h
diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index ab92333673..a5903613f0 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include "cpu.h"
@@ -1030,6 +1031,54 @@ static void amd_check_erratum_1485(void)
wrmsrl(MSR_AMD64_BP_CFG, val | chickenbit);
}
+#ifdef CONFIG_HVM
+static void amd_enable_mem_encrypt(const struct cpuinfo_x86 *c)
+{
+ unsigned int eax, ebx, ecx, edx;
+ uint64_t syscfg;
+
+ if (!smp_processor_id()) {
+
+ cpuid_count(0x8000,0,, , , );
+
+ if (eax < 0x801f)
+ return;
+
+ cpuid_count(0x801f,0,, , , );
+
+ if (eax & 0x1)
+ setup_force_cpu_cap(X86_FEATURE_SME);
+
+ if (eax & 0x2) {
+ setup_force_cpu_cap(X86_FEATURE_SEV);
+ max_sev_asid = ecx;
+ min_sev_asid = edx;
+ }
+
+ if (eax & 0x3)
+ pte_c_bit_mask = 1UL << (ebx & 0x3f);
+ }
+
+ if (!(cpu_has_sme || cpu_has_sev))
+ return;
+
+ if (!smp_processor_id()) {
+ if (cpu_has_sev)
+ printk(XENLOG_INFO "SEV: ASID range [0x%x - 0x%x]\n",
+ min_sev_asid, max_sev_asid);
+ }
+
+ rdmsrl(MSR_K8_SYSCFG, syscfg);
+
+ if (syscfg & SYSCFG_MEM_ENCRYPT) {
+ return;
+ }
+
+ syscfg |= SYSCFG_MEM_ENCRYPT;
+ wrmsrl(MSR_K8_SYSCFG, syscfg);
+}
+#endif
+
static void cf_check init_amd(struct cpuinfo_x86 *c)
{
u32 l, h;
@@ -1305,6 +1354,10 @@ static void cf_check init_amd(struct cpuinfo_x86 *c)
check_syscfg_dram_mod_en();
amd_log_freq(c);
+
+#ifdef CONFIG_HVM
+ amd_enable_mem_encrypt(c);
+#endif
}
const struct cpu_dev __initconst_cf_clobber amd_cpu_dev = {
diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile
index 760d2954da..9773d539ef 100644
--- a/xen/arch/x86/hvm/svm/Makefile
+++ b/xen/arch/x86/hvm/svm/Makefile
@@ -6,3 +6,4 @@ obj-y += nestedsvm.o
obj-y += svm.o
obj-y += svmdebug.o
obj-y += vmcb.o
+obj-y += sev.o
diff --git a/xen/arch/x86/hvm/svm/sev.c b/xen/arch/x86/hvm/svm/sev.c
new file mode 100644
index 00..336fad25f5
--- /dev/null
+++ b/xen/arch/x86/hvm/svm/sev.c
@@ -0,0 +1,4 @@
+#include
+uint64_t __read_mostly pte_c_bit_mask;
+unsigned int __read_mostly min_sev_asid;
+unsigned int __read_mostly max_sev_asid;
diff --git a/xen/arch/x86/include/asm/cpufeature.h
b/xen/arch/x86/include/asm/cpufeature.h
index 743f11f989..a41374d0b7 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -231,6 +231,9 @@ static inline bool boot_cpu_has(unsigned int feat)
#define cpu_has_msr_tsc_aux (cpu_has_rdtscp || cpu_has_rdpid)
+#define cpu_has_sme boot_cpu_has(X86_FEATURE_SME)
+#define cpu_has_sev boot_cpu_has(X86_FEATURE_SEV)
+
/* Bugs. */
#define cpu_bug_fpu_ptrsboot_cpu_has(X86_BUG_FPU_PTRS)
#define cpu_bug_null_segboot_cpu_has(X86_BUG_NULL_SEG)
diff --git a/xen/arch/x86/include/asm/cpufeatures.h
b/xen/arch/x86/include/asm/cpufeatures.h
index ba3df174b7..9b67ea2427 100644
--- a/xen/arch/x86/include/asm/cpufeatures.h
+++ b/xen/arch/x86/include/asm/cpufeatures.h
@@ -42,6 +42,8 @@ XEN_CPUFEATURE(XEN_SHSTK, X86_SYNTH(26)) /* Xen uses
CET Shadow Stacks *
XEN_CPUFEATURE(XEN_IBT, X86_SYNTH(27)) /* Xen uses CET Indirect
Branch Tracking */
XEN_CPUFEATURE(IBPB_ENTRY_PV, X86_SYNTH(28)) /* MSR_PRED_CMD used by Xen
for PV */
XEN_CPUFEATURE(IBPB_ENTRY_HVM,X86_SYNTH(29)) /* MSR_PRED_CMD used by Xen
for HVM */
+XEN_CPUFEATURE(SME, X86_SYNTH(30)) /* AMD Secure Memory
Encrypion */
+XEN_CPUFEATURE(SEV, X86_SYNTH(31)) /* AMD Secure Encryption
Virtualization */
/* Bug words follow the synthetic words. */
#define X86_NR_BUG 1
diff --git a/xen/arch/x86/include/asm/msr-index.h
b/xen/arch/x86/include/asm/msr-index.h
index 92dd9fa496..318e8ca0c0 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -221,6 +221,7 @@
#define SYSCFG_MTRR_VAR_DRAM_EN(_AC(1, ULL) << 20)
#define SYSCFG_MTRR_TOM2_EN(_AC(1, ULL) << 21)
#define SYSCFG_TOM2_FORCE_WB (_AC(1, ULL) <<
[PATCH v1 2/2] Implemented Amd Secure Processor device driver
Signed-off-by: Andrei Semenov
---
xen/arch/x86/include/asm/psp-sev.h | 655 +++
xen/drivers/Kconfig| 2 +
xen/drivers/Makefile | 1 +
xen/drivers/crypto/Kconfig | 10 +
xen/drivers/crypto/Makefile| 1 +
xen/drivers/crypto/asp.c | 808 +
xen/include/xen/types.h| 2 +-
7 files changed, 1478 insertions(+), 1 deletion(-)
create mode 100644 xen/arch/x86/include/asm/psp-sev.h
create mode 100644 xen/drivers/crypto/Kconfig
create mode 100644 xen/drivers/crypto/Makefile
create mode 100644 xen/drivers/crypto/asp.c
diff --git a/xen/arch/x86/include/asm/psp-sev.h
b/xen/arch/x86/include/asm/psp-sev.h
new file mode 100644
index 00..3413de41a9
--- /dev/null
+++ b/xen/arch/x86/include/asm/psp-sev.h
@@ -0,0 +1,655 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * AMD Secure Encrypted Virtualization (SEV) driver interface
+ *
+ * Copyright (C) 2016-2017 Advanced Micro Devices, Inc.
+ *
+ * Author: Brijesh Singh
+ *
+ * SEV API spec is available at https://developer.amd.com/sev
+ */
+
+#ifndef __PSP_SEV_H__
+#define __PSP_SEV_H__
+
+#include
+
+/**
+ * SEV platform and guest management commands
+ */
+enum sev_cmd {
+ /* platform commands */
+ SEV_CMD_INIT= 0x001,
+ SEV_CMD_SHUTDOWN= 0x002,
+ SEV_CMD_FACTORY_RESET = 0x003,
+ SEV_CMD_PLATFORM_STATUS = 0x004,
+ SEV_CMD_PEK_GEN = 0x005,
+ SEV_CMD_PEK_CSR = 0x006,
+ SEV_CMD_PEK_CERT_IMPORT = 0x007,
+ SEV_CMD_PDH_CERT_EXPORT = 0x008,
+ SEV_CMD_PDH_GEN = 0x009,
+ SEV_CMD_DF_FLUSH= 0x00A,
+ SEV_CMD_DOWNLOAD_FIRMWARE = 0x00B,
+ SEV_CMD_GET_ID = 0x00C,
+ SEV_CMD_INIT_EX = 0x00D,
+
+ /* Guest commands */
+ SEV_CMD_DECOMMISSION= 0x020,
+ SEV_CMD_ACTIVATE= 0x021,
+ SEV_CMD_DEACTIVATE = 0x022,
+ SEV_CMD_GUEST_STATUS= 0x023,
+
+ /* Guest launch commands */
+ SEV_CMD_LAUNCH_START= 0x030,
+ SEV_CMD_LAUNCH_UPDATE_DATA = 0x031,
+ SEV_CMD_LAUNCH_UPDATE_VMSA = 0x032,
+ SEV_CMD_LAUNCH_MEASURE = 0x033,
+ SEV_CMD_LAUNCH_UPDATE_SECRET= 0x034,
+ SEV_CMD_LAUNCH_FINISH = 0x035,
+ SEV_CMD_ATTESTATION_REPORT = 0x036,
+
+ /* Guest migration commands (outgoing) */
+ SEV_CMD_SEND_START = 0x040,
+ SEV_CMD_SEND_UPDATE_DATA= 0x041,
+ SEV_CMD_SEND_UPDATE_VMSA= 0x042,
+ SEV_CMD_SEND_FINISH = 0x043,
+ SEV_CMD_SEND_CANCEL = 0x044,
+
+ /* Guest migration commands (incoming) */
+ SEV_CMD_RECEIVE_START = 0x050,
+ SEV_CMD_RECEIVE_UPDATE_DATA = 0x051,
+ SEV_CMD_RECEIVE_UPDATE_VMSA = 0x052,
+ SEV_CMD_RECEIVE_FINISH = 0x053,
+
+ /* Guest debug commands */
+ SEV_CMD_DBG_DECRYPT = 0x060,
+ SEV_CMD_DBG_ENCRYPT = 0x061,
+
+ SEV_CMD_MAX,
+};
+
+/**
+ * struct sev_data_init - INIT command parameters
+ *
+ * @flags: processing flags
+ * @tmr_address: system physical address used for SEV-ES
+ * @tmr_len: len of tmr_address
+ */
+struct sev_data_init {
+ u32 flags; /* In */
+ u32 reserved; /* In */
+ u64 tmr_address;/* In */
+ u32 tmr_len;/* In */
+} __packed;
+
+/**
+ * struct sev_data_init_ex - INIT_EX command parameters
+ *
+ * @length: len of the command buffer read by the PSP
+ * @flags: processing flags
+ * @tmr_address: system physical address used for SEV-ES
+ * @tmr_len: len of tmr_address
+ * @nv_address: system physical address used for PSP NV storage
+ * @nv_len: len of nv_address
+ */
+struct sev_data_init_ex {
+ u32 length; /* In */
+ u32 flags; /* In */
+ u64 tmr_address;/* In */
+ u32 tmr_len;/* In */
+ u32 reserved; /* In */
+ u64 nv_address; /* In/Out */
+ u32 nv_len; /* In */
+} __packed;
+
+#define SEV_INIT_FLAGS_SEV_ES 0x01
+
+/**
+ * struct sev_data_pek_csr - PEK_CSR command parameters
+ *
+ * @address: PEK certificate chain
+ * @len: len of certificate
+ */
+struct sev_data_pek_csr {
+ u64 address;/* In */
+ u32 len;/* In/Out */
+} __packed;
+
+/**
+ * struct sev_data_cert_import - PEK_CERT_IMPORT command parameters
+ *
+ * @pek_address: PEK certificate chain
+ * @pek_len: len of PEK certificate
+ * @oca_address: OCA certificate chain
+ * @oca_len: len of OCA certificate
+ */
+struct
[PATCH v1 0/2] AMD SEV initial work
///HERE YOU GO Andrei Semenov (2): Implemented AMD SEV discovery and enabling. Implemented Amd Secure Processor device driver xen/arch/x86/cpu/amd.c | 53 ++ xen/arch/x86/hvm/svm/Makefile | 1 + xen/arch/x86/hvm/svm/sev.c | 4 + xen/arch/x86/include/asm/cpufeature.h | 3 + xen/arch/x86/include/asm/cpufeatures.h | 2 + xen/arch/x86/include/asm/msr-index.h | 1 + xen/arch/x86/include/asm/psp-sev.h | 655 xen/arch/x86/include/asm/sev.h | 11 + xen/drivers/Kconfig| 2 + xen/drivers/Makefile | 1 + xen/drivers/crypto/Kconfig | 10 + xen/drivers/crypto/Makefile| 1 + xen/drivers/crypto/asp.c | 808 + xen/include/xen/types.h| 2 +- 14 files changed, 1553 insertions(+), 1 deletion(-) create mode 100644 xen/arch/x86/hvm/svm/sev.c create mode 100644 xen/arch/x86/include/asm/psp-sev.h create mode 100644 xen/arch/x86/include/asm/sev.h create mode 100644 xen/drivers/crypto/Kconfig create mode 100644 xen/drivers/crypto/Makefile create mode 100644 xen/drivers/crypto/asp.c -- 2.35.3
Re: [PATCH v2 2/2] x86/Kconfig: Introduce CONFIG_{AMD,INTEL} and conditionalise ucode
On Thu, Oct 26, 2023 at 09:55:39PM +0100, Andrew Cooper wrote:
> We eventually want to be able to build a stripped down Xen for a single
> platform. Make a start with CONFIG_{AMD,INTEL} (hidden behind EXPERT, but
> available to randconfig), and adjust the microcode logic.
>
> No practical change.
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Roger Pau Monné
> CC: Wei Liu
> CC: Alejandro Vallejo
> CC: Stefano Stabellini
> CC: Xenia Ragiadakou
>
> I've intentionally ignored the other vendors for now. They can be put into
> Kconfig by whomever figures out the actual dependencies between their init
> routines.
>
> v2:
> * Tweak text
> ---
> xen/arch/x86/Kconfig | 2 ++
> xen/arch/x86/Kconfig.cpu | 22 ++
> xen/arch/x86/cpu/microcode/Makefile | 4 ++--
> xen/arch/x86/cpu/microcode/private.h | 9 +
> 4 files changed, 35 insertions(+), 2 deletions(-)
> create mode 100644 xen/arch/x86/Kconfig.cpu
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index eac77573bd75..d9eacdd7e0fa 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -49,6 +49,8 @@ config HAS_CC_CET_IBT
>
> menu "Architecture Features"
>
> +source "arch/x86/Kconfig.cpu"
Since we are not targeting at the CPU only, would this be better named
as Kconfig.vendor? Or Kconfig.platform? (I'm OK if you prefer to
leave as .cpu, just suggesting more neutral names.
> +
> source "arch/Kconfig"
>
> config PV
> diff --git a/xen/arch/x86/Kconfig.cpu b/xen/arch/x86/Kconfig.cpu
> new file mode 100644
> index ..3c5d88fdfd16
> --- /dev/null
> +++ b/xen/arch/x86/Kconfig.cpu
> @@ -0,0 +1,22 @@
> +menu "Supported CPU vendors"
> + visible if EXPERT
> +
> +config AMD
> + bool "AMD"
> +default y
> +help
> + Detection, tunings and quirks for AMD platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on AMD platforms.
> +
> +config INTEL
> + bool "Intel"
> +default y
> +help
> + Detection, tunings and quirks for Intel platforms.
> +
> + May be turned off in builds targetting other vendors. Otherwise,
> + must be enabled for Xen to work suitably on Intel platforms.
There seems to be a weird mix between hard tabs and spaces above.
Naming is OK for me.
> +
> +endmenu
> diff --git a/xen/arch/x86/cpu/microcode/Makefile
> b/xen/arch/x86/cpu/microcode/Makefile
> index aae235245b06..30d600544f45 100644
> --- a/xen/arch/x86/cpu/microcode/Makefile
> +++ b/xen/arch/x86/cpu/microcode/Makefile
> @@ -1,3 +1,3 @@
> -obj-y += amd.o
> +obj-$(CONFIG_AMD) += amd.o
> obj-y += core.o
> -obj-y += intel.o
> +obj-$(CONFIG_INTEL) += intel.o
> diff --git a/xen/arch/x86/cpu/microcode/private.h
> b/xen/arch/x86/cpu/microcode/private.h
> index b58611e908aa..da556fe5060a 100644
> --- a/xen/arch/x86/cpu/microcode/private.h
> +++ b/xen/arch/x86/cpu/microcode/private.h
> @@ -70,7 +70,16 @@ struct microcode_ops {
> * support available) and (not) ops->apply_microcode (i.e. read only).
> * Otherwise, all hooks must be filled in.
> */
> +#ifdef CONFIG_AMD
> void ucode_probe_amd(struct microcode_ops *ops);
> +#else
> +static inline void ucode_probe_amd(struct microcode_ops *ops) {}
> +#endif
> +
> +#ifdef CONFIG_INTEL
> void ucode_probe_intel(struct microcode_ops *ops);
> +#else
> +static inline void ucode_probe_intel(struct microcode_ops *ops) {}
This is stale now, and will need some updating to match what's in
private.h.
Thanks, Roger.
Re: [PATCH v2 10/13] xen/arm: remove shm holes from extended regions
Hi Luca,
On 10/04/2024 16:08, Luca Fancellu wrote:
>
>
> Hi Michal,
>
>>>
>>> For direct-map domain with iommu on, after we get guest shm info from
>>> "kinfo",
>>> we use "remove_shm_from_rangeset" to remove static shm.
>>>
>>> For direct-map domain with iommu off, as static shm has already been taken
>>> care of through reserved memory banks, we do nothing.
>> Stale info given that shmem is no longer part of reserved memory banks. It's
>> been taken care
>> of by removing shmem regions in find_unallocated_memory()
>
> Sure, will amend for this:
>
>>>
>>> +int __init remove_shm_from_rangeset(const struct kernel_info *kinfo,
>>> +struct rangeset *rangeset)
>>> +{
>>> +const struct membanks *shm_mem = >shm_mem.common;
>>> +unsigned int i;
>>> +
>>> +/* Remove static shared memory regions */
>>> +for ( i = 0; i < shm_mem->nr_banks; i++ )
>>> +{
>>> +paddr_t start, end;
>>> +int res;
>>> +
>>> +start = shm_mem->bank[i].start;
>>> +end = shm_mem->bank[i].start + shm_mem->bank[i].size - 1;
>> If you look at other rangeset_remove_range use cases and error messages, 1
>> is subtracted
>> in PFN_DOWN() so that the error message contains end unchanged. Please
>> adhere to that so that
>> printed messages are consistent.
>
> Yes I will change it to have -1 inside PFN_DOWN(), here and in the other
> occurrences
>>>
>>> +/* Remove static shared memory regions */
>>> +res = remove_shm_from_rangeset(kinfo, guest_holes);
>>> +if ( res )
>>> +goto out;
>>> +
>> Could you please add a comment explaining here what's done below?
>
> Is it ok something like this:
I'm ok with your proposal in the other e-mail.
>
> /*
> * Take the interval of memory starting from the first extended region bank
> * start address and ending to the end of the last extended region bank.
I would stop here. The rest reads quite difficult.
> * The interval will be passed to rangeset_report_ranges to allow it to
> * create, by the add_ext_regions callback, a set of extended memory region
> * banks from the guest_holes rangeset, which contains the original extended
> * memory region ranges where the static shared memory ranges are carved
> * out.
> */
>
>>> +i = ext_regions->nr_banks - 1;
>>> +start = ext_regions->bank[0].start;
>>> +end = ext_regions->bank[i].start + ext_regions->bank[i].size - 1;
>>> +
>>> +/* Reset original extended regions to hold new value */
>>> +ext_regions->nr_banks = 0;
>>> +res = rangeset_report_ranges(guest_holes, PFN_DOWN(start),
>>> PFN_DOWN(end),
>>> + add_ext_regions, ext_regions);
>>> +if ( res )
>>> +ext_regions->nr_banks = 0;
>>> +else if ( !ext_regions->nr_banks )
>>> +res = -ENOENT;
>>> +
>>> + out:
>>> +rangeset_destroy(guest_holes);
>>> +
>>> +return res;
>>> +}
>>> +
>>> /*
>>> * Local variables:
>>> * mode: C
>>> --
>>> 2.34.1
>>>
>>
>> ~Michal
>>
>
> Cheers,
> Luca
~Michal
Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
Hi Jens,
> On 10 Apr 2024, at 16:27, Jens Wiklander wrote:
>
> On Wed, Apr 10, 2024 at 9:49 AM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 9 Apr 2024, at 17:36, Jens Wiklander wrote:
>>>
>>> Add support for FF-A notifications, currently limited to an SP (Secure
>>> Partition) sending an asynchronous notification to a guest.
>>>
>>> Guests and Xen itself are made aware of pending notifications with an
>>> interrupt. The interrupt handler retrieves the notifications using the
>>> FF-A ABI and deliver them to their destinations.
>>>
>>> Signed-off-by: Jens Wiklander
>>> ---
>>> xen/arch/arm/tee/Makefile | 1 +
>>> xen/arch/arm/tee/ffa.c | 58 ++
>>> xen/arch/arm/tee/ffa_notif.c | 319 +
>>> xen/arch/arm/tee/ffa_private.h | 71
>>> 4 files changed, 449 insertions(+)
>>> create mode 100644 xen/arch/arm/tee/ffa_notif.c
>>>
>>> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
>>> index f0112a2f922d..7c0f46f7f446 100644
>>> --- a/xen/arch/arm/tee/Makefile
>>> +++ b/xen/arch/arm/tee/Makefile
>>> @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
>>> obj-$(CONFIG_FFA) += ffa_shm.o
>>> obj-$(CONFIG_FFA) += ffa_partinfo.o
>>> obj-$(CONFIG_FFA) += ffa_rxtx.o
>>> +obj-$(CONFIG_FFA) += ffa_notif.o
>>> obj-y += tee.o
>>> obj-$(CONFIG_OPTEE) += optee.o
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index 5209612963e1..ce9757bfeed1 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -39,6 +39,9 @@
>>> * - at most 32 shared memory regions per guest
>>> * o FFA_MSG_SEND_DIRECT_REQ:
>>> * - only supported from a VM to an SP
>>> + * o FFA_NOTIFICATION_*:
>>> + * - only supports global notifications, that is, per vCPU notifications
>>> + * are not supported
>>> *
>>> * There are some large locked sections with ffa_tx_buffer_lock and
>>> * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
>>> @@ -194,6 +197,8 @@ out:
>>>
>>> static void handle_features(struct cpu_user_regs *regs)
>>> {
>>> +struct domain *d = current->domain;
>>> +struct ffa_ctx *ctx = d->arch.tee;
>>>uint32_t a1 = get_user_reg(regs, 1);
>>>unsigned int n;
>>>
>>> @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs *regs)
>>>BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
>>>ffa_set_regs_success(regs, 0, 0);
>>>break;
>>> +case FFA_FEATURE_NOTIF_PEND_INTR:
>>> +if ( ctx->notif.enabled )
>>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>>> +else
>>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +break;
>>> +case FFA_FEATURE_SCHEDULE_RECV_INTR:
>>> +if ( ctx->notif.enabled )
>>> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>>
>> This should return the RECV_INTR, not the PEND one.
>
> Thanks, I'll fix it.
>
>>
>>> +else
>>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +break;
>>> +
>>> +case FFA_NOTIFICATION_BIND:
>>> +case FFA_NOTIFICATION_UNBIND:
>>> +case FFA_NOTIFICATION_GET:
>>> +case FFA_NOTIFICATION_SET:
>>> +case FFA_NOTIFICATION_INFO_GET_32:
>>> +case FFA_NOTIFICATION_INFO_GET_64:
>>> +if ( ctx->notif.enabled )
>>> +ffa_set_regs_success(regs, 0, 0);
>>> +else
>>> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>> +break;
>>>default:
>>>ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
>>>break;
>>> @@ -305,6 +334,30 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
>>> get_user_reg(regs, 1)),
>>> get_user_reg(regs, 3));
>>>break;
>>> +case FFA_NOTIFICATION_BIND:
>>> +e = ffa_handle_notification_bind(get_user_reg(regs, 1),
>>> + get_user_reg(regs, 2),
>>> + get_user_reg(regs, 3),
>>> + get_user_reg(regs, 4));
>>
>> I would suggest to pass regs and handle the get_user_regs in the function.
>
> OK
>
>>
>>> +break;
>>> +case FFA_NOTIFICATION_UNBIND:
>>> +e = ffa_handle_notification_unbind(get_user_reg(regs, 1),
>>> + get_user_reg(regs, 3),
>>> + get_user_reg(regs, 4));
>>
>> same here
>
> OK
>
>>
>>> +break;
>>> +case FFA_NOTIFICATION_INFO_GET_32:
>>> +case FFA_NOTIFICATION_INFO_GET_64:
>>> +ffa_handle_notification_info_get(regs);
>>> +return true;
>>> +case FFA_NOTIFICATION_GET:
>>> +ffa_handle_notification_get(regs);
>>> +return true;
>>> +case FFA_NOTIFICATION_SET:
>>> +e = ffa_handle_notification_set(get_user_reg(regs, 1),
>>> +get_user_reg(regs, 2),
>>> +
Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
On Wed, Apr 10, 2024 at 9:49 AM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 9 Apr 2024, at 17:36, Jens Wiklander wrote:
> >
> > Add support for FF-A notifications, currently limited to an SP (Secure
> > Partition) sending an asynchronous notification to a guest.
> >
> > Guests and Xen itself are made aware of pending notifications with an
> > interrupt. The interrupt handler retrieves the notifications using the
> > FF-A ABI and deliver them to their destinations.
> >
> > Signed-off-by: Jens Wiklander
> > ---
> > xen/arch/arm/tee/Makefile | 1 +
> > xen/arch/arm/tee/ffa.c | 58 ++
> > xen/arch/arm/tee/ffa_notif.c | 319 +
> > xen/arch/arm/tee/ffa_private.h | 71
> > 4 files changed, 449 insertions(+)
> > create mode 100644 xen/arch/arm/tee/ffa_notif.c
> >
> > diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
> > index f0112a2f922d..7c0f46f7f446 100644
> > --- a/xen/arch/arm/tee/Makefile
> > +++ b/xen/arch/arm/tee/Makefile
> > @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
> > obj-$(CONFIG_FFA) += ffa_shm.o
> > obj-$(CONFIG_FFA) += ffa_partinfo.o
> > obj-$(CONFIG_FFA) += ffa_rxtx.o
> > +obj-$(CONFIG_FFA) += ffa_notif.o
> > obj-y += tee.o
> > obj-$(CONFIG_OPTEE) += optee.o
> > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> > index 5209612963e1..ce9757bfeed1 100644
> > --- a/xen/arch/arm/tee/ffa.c
> > +++ b/xen/arch/arm/tee/ffa.c
> > @@ -39,6 +39,9 @@
> > * - at most 32 shared memory regions per guest
> > * o FFA_MSG_SEND_DIRECT_REQ:
> > * - only supported from a VM to an SP
> > + * o FFA_NOTIFICATION_*:
> > + * - only supports global notifications, that is, per vCPU notifications
> > + * are not supported
> > *
> > * There are some large locked sections with ffa_tx_buffer_lock and
> > * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
> > @@ -194,6 +197,8 @@ out:
> >
> > static void handle_features(struct cpu_user_regs *regs)
> > {
> > +struct domain *d = current->domain;
> > +struct ffa_ctx *ctx = d->arch.tee;
> > uint32_t a1 = get_user_reg(regs, 1);
> > unsigned int n;
> >
> > @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs *regs)
> > BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
> > ffa_set_regs_success(regs, 0, 0);
> > break;
> > +case FFA_FEATURE_NOTIF_PEND_INTR:
> > +if ( ctx->notif.enabled )
> > +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
> > +else
> > +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> > +break;
> > +case FFA_FEATURE_SCHEDULE_RECV_INTR:
> > +if ( ctx->notif.enabled )
> > +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
>
> This should return the RECV_INTR, not the PEND one.
Thanks, I'll fix it.
>
> > +else
> > +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> > +break;
> > +
> > +case FFA_NOTIFICATION_BIND:
> > +case FFA_NOTIFICATION_UNBIND:
> > +case FFA_NOTIFICATION_GET:
> > +case FFA_NOTIFICATION_SET:
> > +case FFA_NOTIFICATION_INFO_GET_32:
> > +case FFA_NOTIFICATION_INFO_GET_64:
> > +if ( ctx->notif.enabled )
> > +ffa_set_regs_success(regs, 0, 0);
> > +else
> > +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> > +break;
> > default:
> > ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> > break;
> > @@ -305,6 +334,30 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
> > get_user_reg(regs, 1)),
> >get_user_reg(regs, 3));
> > break;
> > +case FFA_NOTIFICATION_BIND:
> > +e = ffa_handle_notification_bind(get_user_reg(regs, 1),
> > + get_user_reg(regs, 2),
> > + get_user_reg(regs, 3),
> > + get_user_reg(regs, 4));
>
> I would suggest to pass regs and handle the get_user_regs in the function.
OK
>
> > +break;
> > +case FFA_NOTIFICATION_UNBIND:
> > +e = ffa_handle_notification_unbind(get_user_reg(regs, 1),
> > + get_user_reg(regs, 3),
> > + get_user_reg(regs, 4));
>
> same here
OK
>
> > +break;
> > +case FFA_NOTIFICATION_INFO_GET_32:
> > +case FFA_NOTIFICATION_INFO_GET_64:
> > +ffa_handle_notification_info_get(regs);
> > +return true;
> > +case FFA_NOTIFICATION_GET:
> > +ffa_handle_notification_get(regs);
> > +return true;
> > +case FFA_NOTIFICATION_SET:
> > +e = ffa_handle_notification_set(get_user_reg(regs, 1),
> > +get_user_reg(regs, 2),
> > +get_user_reg(regs, 3),
> > +
Re: [PATCH 5/5] x86: Add --force option to xen-ucode to override microcode version check
On Fri, Apr 05, 2024 at 01:11:28PM +0100, Fouad Hilly wrote:
> diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c
> index 5ecdfa2c7934..edce45bc2a17 100644
> --- a/tools/misc/xen-ucode.c
> +++ b/tools/misc/xen-ucode.c
> @@ -21,10 +23,11 @@ static const char amd_id[] = "AuthenticAMD";
> static void usage(const char *name)
> {
> printf("%s: Xen microcode updating tool\n"
> -"Usage: %s [ | --show-cpu-info]\n"
> +"Usage: %s [[--force] | --show-cpu-info]\n"
How about "Usage: %s [OPTIONS..] [MICROCODE FILE]" ?
> "\n"
> " -h, --helpdisplay this help and exit\n"
> " -s, --show-cpu-info show CPU information and exit\n"
> +" -f, --force force to skip micorocde version check\n"
typo: microcode ;-)
> "\n"
> , name, name);
> }
> @@ -89,11 +92,13 @@ int main(int argc, char *argv[])
> char *filename = NULL, *buf;
> size_t len;
> struct stat st;
> +uint32_t ucode_flag = XENPF_UCODE_FLAG_FORCE_NOT_SET;
> int opt;
>
> const static struct option options[] = {
> {"help", no_argument, NULL, 'h'},
> {"show-cpu-info", no_argument, NULL, 's'},
> +{"force", required_argument, NULL, 'f'},
This is weird, could you do without the argument?
It is weird because sometime "microcode file" is an argument of
"--force", sometime it is part of the rests of the options.
> {NULL, no_argument, NULL, 0}
> };
>
> @@ -105,10 +110,10 @@ int main(int argc, char *argv[])
> exit(1);
> }
>
> -if ( argc != 2 )
> +if ( argc < 2 || argc > 3)
> goto ext_err;
>
> -while ( (opt = getopt_long(argc, argv, "hs:", options, NULL)) != -1 )
> +while ( (opt = getopt_long(argc, argv, "hsf:", options, NULL)) != -1 )
> {
> switch (opt)
> {
> @@ -120,12 +125,17 @@ int main(int argc, char *argv[])
> goto ext_err;
> show_curr_cpu(stdout);
> exit(EXIT_SUCCESS);
> +case 'f':
> +ucode_flag = XENPF_UCODE_FLAG_FORCE_SET;
> +filename = optarg;
> +break;
> default:
> goto ext_err;
> }
> }
>
> -filename = argv[1];
> +if ( argc == 2 )
> +filename = argv[1];
Sometime we take filename from argv[1], sometime we don't? The logic is
going to be very confusing, takeout of context, only set `filename` from a
single place please.
Thanks,
--
Anthony PERARD
Re: [PATCH] x86/pat: fix W^X violation false-positives when running as Xen PV guest
On 10.04.24 15:48, Ingo Molnar wrote:
* Juergen Gross wrote:
When running as Xen PV guest in some cases W^X violation WARN()s have
been observed. Those WARN()s are produced by verify_rwx(), which looks
into the PTE to verify that writable kernel pages have the NX bit set
in order to avoid code modifications of the kernel by rogue code.
As the NX bits of all levels of translation entries are or-ed and the
RW bits of all levels are and-ed, looking just into the PTE isn't enough
for the decision that a writable page is executable, too. When running
as a Xen PV guest, kernel initialization will set the NX bit in PMD
entries of the initial page tables covering the .data segment.
When finding the PTE to have set the RW bit but no NX bit, higher level
entries must be looked at. Only when all levels have the RW bit set and
no NX bit set, the W^X violation should be flagged.
Additionally show_fault_oops() has a similar problem: it will issue the
"kernel tried to execute NX-protected page" message only if it finds
the NX bit set in the leaf translation entry, while any NX bit in
non-leaf entries are being ignored for issuing the message.
Modify lookup_address_in_pgd() to return the effective NX and RW bit
values of the non-leaf translation entries and evaluate those as well
in verify_rwx() and show_fault_oops().
Ok, this fix makes sense, as that's how the hardware works and we interpret
the pagetables poorly.
Thanks for confirmation that my approach is sane.
Fixes: 652c5bf380ad ("x86/mm: Refuse W^X violations")
Reported-by: Jason Andryuk
Signed-off-by: Juergen Gross
---
arch/x86/include/asm/pgtable_types.h | 2 +-
arch/x86/kernel/sev.c| 3 +-
arch/x86/mm/fault.c | 7 ++--
arch/x86/mm/pat/set_memory.c | 56 +---
arch/x86/virt/svm/sev.c | 3 +-
5 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/arch/x86/include/asm/pgtable_types.h
b/arch/x86/include/asm/pgtable_types.h
index 0b748ee16b3d..91ab538d3872 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -565,7 +565,7 @@ static inline void update_page_count(int level, unsigned
long pages) { }
*/
extern pte_t *lookup_address(unsigned long address, unsigned int *level);
extern pte_t *lookup_address_in_pgd(pgd_t *pgd, unsigned long address,
- unsigned int *level);
+ unsigned int *level, bool *nx, bool *rw);
extern pmd_t *lookup_pmd_address(unsigned long address);
extern phys_addr_t slow_virt_to_phys(void *__address);
extern int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn,
Please introduce a new lookup_address_in_pgd_attr() function or so, which
is used by code intentionally.
This avoids changing the arch/x86/kernel/sev.c and arch/x86/virt/svm/sev.c
uses, that retrieve these attributes but don't do anything with them:
Okay.
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 38ad066179d8..adba581e999d 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -516,12 +516,13 @@ static enum es_result vc_slow_virt_to_phys(struct ghcb
*ghcb, struct es_em_ctxt
unsigned long va = (unsigned long)vaddr;
unsigned int level;
phys_addr_t pa;
+ bool nx, rw;
pgd_t *pgd;
pte_t *pte;
pgd = __va(read_cr3_pa());
pgd = [pgd_index(va)];
- pte = lookup_address_in_pgd(pgd, va, );
+ pte = lookup_address_in_pgd(pgd, va, , , );
if (!pte) {
ctxt->fi.vector = X86_TRAP_PF;
ctxt->fi.cr2= vaddr;
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 622d12ec7f08..eb8e897a5653 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -514,18 +514,19 @@ show_fault_oops(struct pt_regs *regs, unsigned long
error_code, unsigned long ad
if (error_code & X86_PF_INSTR) {
unsigned int level;
+ bool nx, rw;
pgd_t *pgd;
pte_t *pte;
pgd = __va(read_cr3_pa());
pgd += pgd_index(address);
- pte = lookup_address_in_pgd(pgd, address, );
+ pte = lookup_address_in_pgd(pgd, address, , , );
- if (pte && pte_present(*pte) && !pte_exec(*pte))
+ if (pte && pte_present(*pte) && (!pte_exec(*pte) || nx))
pr_crit("kernel tried to execute NX-protected page - exploit
attempt? (uid: %d)\n",
from_kuid(_user_ns, current_uid()));
- if (pte && pte_present(*pte) && pte_exec(*pte) &&
+ if (pte && pte_present(*pte) && pte_exec(*pte) && !nx &&
(pgd_flags(*pgd) & _PAGE_USER) &&
(__read_cr4() & X86_CR4_SMEP))
pr_crit("unable to execute userspace code (SMEP?) (uid:
%d)\n",
This should be a separate patch - as it might change
Re: [PATCH v2 10/13] xen/arm: remove shm holes from extended regions
Hi Michal, >> >> For direct-map domain with iommu off, as static shm has already been taken >> care of through reserved memory banks, we do nothing. > Stale info given that shmem is no longer part of reserved memory banks. It's > been taken care > of by removing shmem regions in find_unallocated_memory() Sorry, in the previous mail it didn’t paste my proposed fix: For direct-map domain with iommu off, as static shm has already been taken care of through find_unallocated_memory, we do nothing. Cheers, Luca
Re: [PATCH v2 10/13] xen/arm: remove shm holes from extended regions
Hi Michal,
>>
>> For direct-map domain with iommu on, after we get guest shm info from
>> "kinfo",
>> we use "remove_shm_from_rangeset" to remove static shm.
>>
>> For direct-map domain with iommu off, as static shm has already been taken
>> care of through reserved memory banks, we do nothing.
> Stale info given that shmem is no longer part of reserved memory banks. It's
> been taken care
> of by removing shmem regions in find_unallocated_memory()
Sure, will amend for this:
>>
>> +int __init remove_shm_from_rangeset(const struct kernel_info *kinfo,
>> +struct rangeset *rangeset)
>> +{
>> +const struct membanks *shm_mem = >shm_mem.common;
>> +unsigned int i;
>> +
>> +/* Remove static shared memory regions */
>> +for ( i = 0; i < shm_mem->nr_banks; i++ )
>> +{
>> +paddr_t start, end;
>> +int res;
>> +
>> +start = shm_mem->bank[i].start;
>> +end = shm_mem->bank[i].start + shm_mem->bank[i].size - 1;
> If you look at other rangeset_remove_range use cases and error messages, 1 is
> subtracted
> in PFN_DOWN() so that the error message contains end unchanged. Please adhere
> to that so that
> printed messages are consistent.
Yes I will change it to have -1 inside PFN_DOWN(), here and in the other
occurrences
>>
>> +/* Remove static shared memory regions */
>> +res = remove_shm_from_rangeset(kinfo, guest_holes);
>> +if ( res )
>> +goto out;
>> +
> Could you please add a comment explaining here what's done below?
Is it ok something like this:
/*
* Take the interval of memory starting from the first extended region bank
* start address and ending to the end of the last extended region bank.
* The interval will be passed to rangeset_report_ranges to allow it to
* create, by the add_ext_regions callback, a set of extended memory region
* banks from the guest_holes rangeset, which contains the original extended
* memory region ranges where the static shared memory ranges are carved
* out.
*/
>> +i = ext_regions->nr_banks - 1;
>> +start = ext_regions->bank[0].start;
>> +end = ext_regions->bank[i].start + ext_regions->bank[i].size - 1;
>> +
>> +/* Reset original extended regions to hold new value */
>> +ext_regions->nr_banks = 0;
>> +res = rangeset_report_ranges(guest_holes, PFN_DOWN(start),
>> PFN_DOWN(end),
>> + add_ext_regions, ext_regions);
>> +if ( res )
>> +ext_regions->nr_banks = 0;
>> +else if ( !ext_regions->nr_banks )
>> +res = -ENOENT;
>> +
>> + out:
>> +rangeset_destroy(guest_holes);
>> +
>> +return res;
>> +}
>> +
>> /*
>> * Local variables:
>> * mode: C
>> --
>> 2.34.1
>>
>
> ~Michal
>
Cheers,
Luca
Re: [PATCH] MAINTAINERS: Update livepatch maintainers
On Tue, Apr 09, 2024 at 12:13:23PM -0400, Konrad Rzeszutek Wilk wrote: > On Tue, Apr 09, 2024 at 11:32:07AM +0100, Ross Lagerwall wrote: > > Remove Konrad from the livepatch maintainers list as he hasn't been > > active for a few years. > > At the same time, add Roger as a new maintainer since he has been > > actively working on it for a while. > > > > Signed-off-by: Ross Lagerwall > > Acked-by: Konrad Rzeszutek Wilk > > Thank you for picking it up! Acked-by: Roger Pau Monné Thanks for your work on this Konrad. Regards, Roger.
Re: [PATCH 4/5] x86: Use getopt to handle command line args
On Fri, Apr 05, 2024 at 01:11:27PM +0100, Fouad Hilly wrote:
> diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c
> index 1edcebfb9f9c..9bde991c5df5 100644
> --- a/tools/misc/xen-ucode.c
> +++ b/tools/misc/xen-ucode.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
>
> static xc_interface *xch;
>
> @@ -20,7 +21,10 @@ static const char amd_id[] = "AuthenticAMD";
> static void usage(const char *name)
> {
> printf("%s: Xen microcode updating tool\n"
> -"Usage: %s [ | show-cpu-info]\n"
> +"Usage: %s [ | --show-cpu-info]\n"
This look like a change worth mentioning to users, can we add something
in the CHANGELOG to say "show-cpu-info" is no longer an option and
users/admin should use "--show-cpu-info" instead?
> +"\n"
> +" -h, --helpdisplay this help and exit\n"
> +" -s, --show-cpu-info show CPU information and exit\n"
> "\n"
> , name, name);
> }
> @@ -82,9 +86,16 @@ static void show_curr_cpu(FILE *f)
> int main(int argc, char *argv[])
> {
> int fd, ret;
> -char *filename, *buf;
> +char *filename = NULL, *buf;
> size_t len;
> struct stat st;
> +int opt;
> +
> +const static struct option options[] = {
> +{"help", no_argument, NULL, 'h'},
> +{"show-cpu-info", no_argument, NULL, 's'},
> +{NULL, no_argument, NULL, 0}
> +};
>
> xch = xc_interface_open(NULL, NULL, 0);
> if ( xch == NULL )
> @@ -94,20 +105,33 @@ int main(int argc, char *argv[])
> exit(1);
> }
>
> -if ( argc < 2 )
> +if ( argc != 2 )
This is overly restrictive, and doesn't need to be, especially when this
patch introduces the use of getopt_long().
> +goto ext_err;
> +
> +while ( (opt = getopt_long(argc, argv, "hs:", options, NULL)) != -1 )
`-s` requires an argument but `--show-cpu-info`, looks there's an extra
':' in the `optstring`, it should read "hs", not "hs:".
> {
> -usage(argv[0]);
> -show_curr_cpu(stderr);
> -exit(2);
> +switch (opt)
> +{
> +case 'h':
> +usage(argv[0]);
> +exit(EXIT_SUCCESS);
> +case 's':
> +if ( argc > 2 )
Why is `-s` only allowed alone? What if want to include some other
option like "--json" to print the cpu-info in a different format?
I think one way to deal with this would be to record the fact that we
want to display the cpu information, and after the getopt_long() loop,
check that they are no more arguments. (Check out `optind` in the man page)
> +goto ext_err;
> +show_curr_cpu(stdout);
> +exit(EXIT_SUCCESS);
> +default:
> +goto ext_err;
> +}
> }
>
> -if ( !strcmp(argv[1], "show-cpu-info") )
> +filename = argv[1];
> +if ( filename == NULL )
> {
> -show_curr_cpu(stdout);
> -return 0;
> +printf("File name error\n");
> +goto ext_err;
> }
>
> -filename = argv[1];
> fd = open(filename, O_RDONLY);
> if ( fd < 0 )
> {
> @@ -149,4 +173,9 @@ int main(int argc, char *argv[])
> close(fd);
>
> return 0;
> +
> +ext_err:
> +usage(argv[0]);
> +show_curr_cpu(stderr);
Why is show_curr_cpu() called on an error path?
> +exit(STDERR_FILENO);
STDERR_FILENO isn't an exit code, it's a file descriptor.
Thanks,
--
Anthony PERARD
Re: [PATCH] x86/pat: fix W^X violation false-positives when running as Xen PV guest
* Juergen Gross wrote:
> When running as Xen PV guest in some cases W^X violation WARN()s have
> been observed. Those WARN()s are produced by verify_rwx(), which looks
> into the PTE to verify that writable kernel pages have the NX bit set
> in order to avoid code modifications of the kernel by rogue code.
>
> As the NX bits of all levels of translation entries are or-ed and the
> RW bits of all levels are and-ed, looking just into the PTE isn't enough
> for the decision that a writable page is executable, too. When running
> as a Xen PV guest, kernel initialization will set the NX bit in PMD
> entries of the initial page tables covering the .data segment.
>
> When finding the PTE to have set the RW bit but no NX bit, higher level
> entries must be looked at. Only when all levels have the RW bit set and
> no NX bit set, the W^X violation should be flagged.
>
> Additionally show_fault_oops() has a similar problem: it will issue the
> "kernel tried to execute NX-protected page" message only if it finds
> the NX bit set in the leaf translation entry, while any NX bit in
> non-leaf entries are being ignored for issuing the message.
>
> Modify lookup_address_in_pgd() to return the effective NX and RW bit
> values of the non-leaf translation entries and evaluate those as well
> in verify_rwx() and show_fault_oops().
Ok, this fix makes sense, as that's how the hardware works and we interpret
the pagetables poorly.
> Fixes: 652c5bf380ad ("x86/mm: Refuse W^X violations")
> Reported-by: Jason Andryuk
> Signed-off-by: Juergen Gross
> ---
> arch/x86/include/asm/pgtable_types.h | 2 +-
> arch/x86/kernel/sev.c| 3 +-
> arch/x86/mm/fault.c | 7 ++--
> arch/x86/mm/pat/set_memory.c | 56 +---
> arch/x86/virt/svm/sev.c | 3 +-
> 5 files changed, 52 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/include/asm/pgtable_types.h
> b/arch/x86/include/asm/pgtable_types.h
> index 0b748ee16b3d..91ab538d3872 100644
> --- a/arch/x86/include/asm/pgtable_types.h
> +++ b/arch/x86/include/asm/pgtable_types.h
> @@ -565,7 +565,7 @@ static inline void update_page_count(int level, unsigned
> long pages) { }
> */
> extern pte_t *lookup_address(unsigned long address, unsigned int *level);
> extern pte_t *lookup_address_in_pgd(pgd_t *pgd, unsigned long address,
> - unsigned int *level);
> + unsigned int *level, bool *nx, bool *rw);
> extern pmd_t *lookup_pmd_address(unsigned long address);
> extern phys_addr_t slow_virt_to_phys(void *__address);
> extern int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn,
Please introduce a new lookup_address_in_pgd_attr() function or so, which
is used by code intentionally.
This avoids changing the arch/x86/kernel/sev.c and arch/x86/virt/svm/sev.c
uses, that retrieve these attributes but don't do anything with them:
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 38ad066179d8..adba581e999d 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -516,12 +516,13 @@ static enum es_result vc_slow_virt_to_phys(struct ghcb
> *ghcb, struct es_em_ctxt
> unsigned long va = (unsigned long)vaddr;
> unsigned int level;
> phys_addr_t pa;
> + bool nx, rw;
> pgd_t *pgd;
> pte_t *pte;
>
> pgd = __va(read_cr3_pa());
> pgd = [pgd_index(va)];
> - pte = lookup_address_in_pgd(pgd, va, );
> + pte = lookup_address_in_pgd(pgd, va, , , );
> if (!pte) {
> ctxt->fi.vector = X86_TRAP_PF;
> ctxt->fi.cr2= vaddr;
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 622d12ec7f08..eb8e897a5653 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -514,18 +514,19 @@ show_fault_oops(struct pt_regs *regs, unsigned long
> error_code, unsigned long ad
>
> if (error_code & X86_PF_INSTR) {
> unsigned int level;
> + bool nx, rw;
> pgd_t *pgd;
> pte_t *pte;
>
> pgd = __va(read_cr3_pa());
> pgd += pgd_index(address);
>
> - pte = lookup_address_in_pgd(pgd, address, );
> + pte = lookup_address_in_pgd(pgd, address, , , );
>
> - if (pte && pte_present(*pte) && !pte_exec(*pte))
> + if (pte && pte_present(*pte) && (!pte_exec(*pte) || nx))
> pr_crit("kernel tried to execute NX-protected page -
> exploit attempt? (uid: %d)\n",
> from_kuid(_user_ns, current_uid()));
> - if (pte && pte_present(*pte) && pte_exec(*pte) &&
> + if (pte && pte_present(*pte) && pte_exec(*pte) && !nx &&
> (pgd_flags(*pgd) & _PAGE_USER) &&
> (__read_cr4() & X86_CR4_SMEP))
> pr_crit("unable to execute userspace code (SMEP?) (uid:
> %d)\n",
Re: [XEN PATCH v1 4/5] xen/arm: allow dynamically assigned SGI handlers
Hi Jens, On 09/04/2024 17:36, Jens Wiklander wrote: > > > Updates so request_irq() can be used with a dynamically assigned SGI irq > as input. At this point it would be handy to mention the use case for which you need to add this support > > gic_route_irq_to_xen() don't gic_set_irq_type() for SGIs since they have > their type assigned earlier during boot Could you elaborate more on that? Do you mean that SGIs are always edge triggered and there's no need for setting the type? > > gic_interrupt() is updated to route the dynamically assigned SGIs to > do_IRQ() instead of do_sgi(). The latter still handles the statically > assigned SGI handlers like for instance GIC_SGI_CALL_FUNCTION. > > Signed-off-by: Jens Wiklander Other than that, it LGTM: Acked-by: Michal Orzel but I would like other maintainers (especially Julien) to cross check it. ~Michal
Re: [PATCH] x86/hvm: Fix Misra Rule 19.1 regression
On 10/04/2024 11:37 am, Andrew Cooper wrote:
> Despite noticing an impending Rule 19.1 violation, the adjustment made (the
> uint32_t cast) wasn't sufficient to avoid it. Try again.
>
> Fixes: 6a98383b0877 ("x86/HVM: clear upper halves of GPRs upon entry from
> 32-bit code")
> Signed-off-by: Andrew Cooper
Subsequently noticed by Coverity too. CIDs 15962{89..98}
[RFC PATCH v1 1/2] libs/light: add device model start timeout env var
When debugging QEMU, the need to run it under Valgrind and asan meant
the compile-time define LIBXL_DEVICE_MODEL_START_TIMEOUT must be changed
to allow for `xl` to wait longer while the instrumented QEMU
initializes.
This commit adds support for reading the environment variable
LIBXL_DEVICE_MODEL_START_TIMEOUT to configure the timeout value and
otherwise fall back to the default 60.
Signed-off-by: Manos Pitsidianakis
---
docs/man/xl.1.pod.in | 11 +++
tools/libs/light/libxl_9pfs.c| 2 +-
tools/libs/light/libxl_device.c | 2 +-
tools/libs/light/libxl_dm.c | 6 +++---
tools/libs/light/libxl_dom_suspend.c | 2 +-
tools/libs/light/libxl_internal.h| 6 ++
tools/libs/light/libxl_pci.c | 6 +++---
7 files changed, 26 insertions(+), 9 deletions(-)
diff --git docs/man/xl.1.pod.in docs/man/xl.1.pod.in
index bed8393473..c159877094 100644
--- docs/man/xl.1.pod.in
+++ docs/man/xl.1.pod.in
@@ -1993,6 +1993,17 @@ Otherwise the build time default in
LIBXL_BOOTLOADER_TIMEOUT will be used.
If defined the value must be an unsigned integer between 0 and INT_MAX,
otherwise behavior is undefined. Setting to 0 disables the timeout.
+=item LIBXL_DEVICE_MODEL_START_TIMEOUT
+
+Timeout in seconds for starting the device model process. Useful in case the
+device model takes an unusual amount of time to start— for example in case of
+very slow I/O, in case of slow performance due to memory sanitizer usage, etc.
+
+If undefined, the default hard-coded value of 60 seconds is used.
+
+If defined, the value must be an unsigned integer between 0 and INT_MAX,
+otherwise behaviour is undefined. Setting the value to 0 disables the timeout.
+
=back
=head1 SEE ALSO
diff --git tools/libs/light/libxl_9pfs.c tools/libs/light/libxl_9pfs.c
index 48f894f070..950a464b45 100644
--- tools/libs/light/libxl_9pfs.c
+++ tools/libs/light/libxl_9pfs.c
@@ -132,7 +132,7 @@ static int xen9pfsd_spawn(libxl__egc *egc, uint32_t domid,
libxl_device_p9 *p9,
aop9->spawn.ao = aodev->ao;
aop9->spawn.what = "xen-9pfs daemon";
aop9->spawn.xspath = GCSPRINTF("%s/state", path);
-aop9->spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+aop9->spawn.timeout_ms = __libxl_device_model_start_timeout() * 1000;
aop9->spawn.pidpath = GCSPRINTF("%s/pid", path);
aop9->spawn.midproc_cb = libxl__spawn_record_pid;
aop9->spawn.confirm_cb = xen9pfsd_confirm;
diff --git tools/libs/light/libxl_device.c tools/libs/light/libxl_device.c
index 6f0100d05e..452e55ba23 100644
--- tools/libs/light/libxl_device.c
+++ tools/libs/light/libxl_device.c
@@ -1436,7 +1436,7 @@ int libxl__wait_for_device_model_deprecated(libxl__gc *gc,
path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/state");
return libxl__xenstore_child_wait_deprecated(gc, domid,
- LIBXL_DEVICE_MODEL_START_TIMEOUT,
+ __libxl_device_model_start_timeout(),
"Device Model", path, state, spawning,
check_callback, check_callback_userdata);
}
diff --git tools/libs/light/libxl_dm.c tools/libs/light/libxl_dm.c
index 0b03a7c747..4369fef161 100644
--- tools/libs/light/libxl_dm.c
+++ tools/libs/light/libxl_dm.c
@@ -2629,7 +2629,7 @@ static void spawn_qmp_proxy(libxl__egc *egc,
sdss->qmp_proxy_spawn.pidpath = GCSPRINTF("%s/image/qmp-proxy-pid",
dom_path);
sdss->qmp_proxy_spawn.xspath = DEVICE_MODEL_XS_PATH(gc,
LIBXL_TOOLSTACK_DOMID,
dm_domid,
"/qmp-proxy-state");
-sdss->qmp_proxy_spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+sdss->qmp_proxy_spawn.timeout_ms = __libxl_device_model_start_timeout() *
1000;
sdss->qmp_proxy_spawn.midproc_cb = libxl__spawn_record_pid;
sdss->qmp_proxy_spawn.confirm_cb = qmp_proxy_confirm;
sdss->qmp_proxy_spawn.failure_cb = qmp_proxy_startup_failed;
@@ -3011,7 +3011,7 @@ retry_transaction:
spawn->what = GCSPRINTF("domain %d device model", domid);
spawn->xspath = DEVICE_MODEL_XS_PATH(gc, LIBXL_TOOLSTACK_DOMID, domid,
"/state");
-spawn->timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+spawn->timeout_ms = __libxl_device_model_start_timeout() * 1000;
spawn->pidpath = GCSPRINTF("%s/image/device-model-pid", dom_path);
spawn->midproc_cb = libxl__spawn_record_pid;
spawn->confirm_cb = device_model_confirm;
@@ -3435,7 +3435,7 @@ void libxl__spawn_qdisk_backend(libxl__egc *egc,
libxl__dm_spawn_state *dmss)
dmss->spawn.what = GCSPRINTF("domain %u Qdisk backend", domid);
dmss->spawn.xspath = GCSPRINTF("device-model/%u/state", domid);
-dmss->spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+dmss->spawn.timeout_ms = __libxl_device_model_start_timeout() * 1000;
/*
* We cannot save Qemu pid anywhere in the xenstore guest
[RFC PATCH v1 2/2] libs/light: expand device model start timeout use
Various timeout values that depend on the device model should also
respect the device model start timeout setting. This commit adds the
__libxl_device_model_start_timeout() value to those time outs without
changing their default values.
Signed-off-by: Manos Pitsidianakis
---
tools/libs/light/libxl_dm.c | 4 ++--
tools/libs/light/libxl_domain.c | 5 +++--
tools/libs/light/libxl_pci.c| 4 ++--
tools/libs/light/libxl_usb.c| 8
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git tools/libs/light/libxl_dm.c tools/libs/light/libxl_dm.c
index 4369fef161..9ffdd50c69 100644
--- tools/libs/light/libxl_dm.c
+++ tools/libs/light/libxl_dm.c
@@ -2807,7 +2807,7 @@ static void stubdom_pvqemu_unpaused(libxl__egc *egc,
dm_domid, sdss->dm.guest_domid);
sdss->xswait.path = DEVICE_MODEL_XS_PATH(gc, dm_domid,
sdss->dm.guest_domid,
"/state");
-sdss->xswait.timeout_ms = LIBXL_STUBDOM_START_TIMEOUT * 1000;
+sdss->xswait.timeout_ms = (__libxl_device_model_start_timeout() +
LIBXL_STUBDOM_START_TIMEOUT) * 1000;
sdss->xswait.callback = stubdom_xswait_cb;
rc = libxl__xswait_start(gc, >xswait);
if (rc) goto out;
@@ -3177,7 +3177,7 @@ static void device_model_spawn_outcome(libxl__egc *egc,
== LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
rc = libxl__ev_time_register_rel(ao, >timeout,
devise_model_postconfig_timeout,
- LIBXL_QMP_CMD_TIMEOUT * 1000);
+ (__libxl_device_model_start_timeout()
+ LIBXL_QMP_CMD_TIMEOUT) * 1000);
if (rc) goto out;
dmss->qmp.ao = ao;
dmss->qmp.domid = dmss->guest_domid;
diff --git tools/libs/light/libxl_domain.c tools/libs/light/libxl_domain.c
index 6751fc785f..2fc3481f78 100644
--- tools/libs/light/libxl_domain.c
+++ tools/libs/light/libxl_domain.c
@@ -1882,7 +1882,8 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t domid,
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
rc = libxl__ev_time_register_rel(ao, >timeout,
set_vcpuonline_timeout,
- LIBXL_QMP_CMD_TIMEOUT * 1000);
+
(__libxl_device_model_start_timeout()
+ + LIBXL_QMP_CMD_TIMEOUT) * 1000);
if (rc) goto out;
qmp->callback = set_vcpuonline_qmp_cpus_fast_queried;
rc = libxl__ev_qmp_send(egc, qmp, "query-cpus-fast", NULL);
@@ -2353,7 +2354,7 @@ static void retrieve_domain_configuration_lock_acquired(
*/
rc = libxl__ev_time_register_rel(ao, >timeout,
retrieve_domain_configuration_timeout,
-LIBXL_QMP_CMD_TIMEOUT * 1000);
+(__libxl_device_model_start_timeout() + LIBXL_QMP_CMD_TIMEOUT) *
1000);
if (rc) goto out;
libxl_bitmap_alloc(CTX, >qemuu_cpus,
d_config->b_info.max_vcpus);
diff --git tools/libs/light/libxl_pci.c tools/libs/light/libxl_pci.c
index 7bdd9f6c3b..3120649a8e 100644
--- tools/libs/light/libxl_pci.c
+++ tools/libs/light/libxl_pci.c
@@ -1165,7 +1165,7 @@ static void do_pci_add(libxl__egc *egc,
case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
rc = libxl__ev_time_register_rel(ao, >timeout,
pci_add_timeout,
- LIBXL_QMP_CMD_TIMEOUT * 1000);
+
(__libxl_device_model_start_timeout() + LIBXL_QMP_CMD_TIMEOUT) * 1000);
if (rc) goto out;
pci_add_qmp_device_add(egc, pas); /* must be last */
@@ -2030,7 +2030,7 @@ static void pci_remove_qmp_device_del(libxl__egc *egc,
rc = libxl__ev_time_register_rel(ao, >timeout,
pci_remove_timeout,
- LIBXL_QMP_CMD_TIMEOUT * 1000);
+ (__libxl_device_model_start_timeout() +
LIBXL_QMP_CMD_TIMEOUT) * 1000);
if (rc) goto out;
QMP_PARAMETERS_SPRINTF(, "id", PCI_PT_QDEV_ID,
diff --git tools/libs/light/libxl_usb.c tools/libs/light/libxl_usb.c
index c5ae59681c..59db8a6f64 100644
--- tools/libs/light/libxl_usb.c
+++ tools/libs/light/libxl_usb.c
@@ -487,7 +487,7 @@ static void libxl__device_usbctrl_add(libxl__egc *egc,
uint32_t domid,
rc = libxl__ev_time_register_rel(ao, >timeout,
device_usbctrl_add_timeout,
- LIBXL_QMP_CMD_TIMEOUT * 1000);
+ (__libxl_device_model_start_timeout()
+ LIBXL_QMP_CMD_TIMEOUT) * 1000);
if (rc) goto outrm;
qmp->ao = ao;
@@ -644,7 +644,7 @@ static void device_usbctrl_usbdevs_removed(libxl__egc
[RFC PATCH v1 0/2] convert LIBXL_DEVICE_MODEL_START_TIMEOUT to env var
This patch series proposes converting the compile-time define LIBXL_DEVICE_MODEL_START_TIMEOUT value to an optionally overridden by environment variable value, just like the current behavior for LIBXL_BOOTLOADER_TIMEOUT is. Manos Pitsidianakis (2): libs/light: add device model start timeout env var libs/light: expand device model start timeout use docs/man/xl.1.pod.in | 11 +++ tools/libs/light/libxl_9pfs.c| 2 +- tools/libs/light/libxl_device.c | 2 +- tools/libs/light/libxl_dm.c | 10 +- tools/libs/light/libxl_dom_suspend.c | 2 +- tools/libs/light/libxl_domain.c | 5 +++-- tools/libs/light/libxl_internal.h| 6 ++ tools/libs/light/libxl_pci.c | 10 +- tools/libs/light/libxl_usb.c | 8 9 files changed, 37 insertions(+), 19 deletions(-) base-commit: f48299cad5c3c69fdc2c101517a6dab9c9827ea5 -- γαῖα πυρί μιχθήτω
Re: [PATCH v2 09/13] xen/arm: Reduce struct membank size on static shared memory
On 10/04/2024 13:19, Luca Fancellu wrote: > > > > Afterwards, create a new structure 'struct shared_meminfo' which > has the same interface of 'struct meminfo', but requires less I would expect some justification for selecting 32 as the max number of shmem banks >>> >>> So I have to say I picked up a value I thought was ok for the amount of >>> shared memory >>> Banks, do you think it is too low? The real intention here was to decouple >>> the number >>> of shared memory banks from the number of generic memory banks, and I felt >>> 32 was enough, >>> but if you think it might be an issue I could bump it, or we could have a >>> Kconfig... >> No need for Kconfig. 32 is enough for now but I expect a paragraph in commit >> msg that you select >> 32 which should be enough for current use cases and can be bumped in the >> future in case there is a need. > > What do you think of this proposal: > > [...] > hence the 'struct membank' won't grow in size. > > Afterwards, create a new structure 'struct shared_meminfo' which > has the same interface of 'struct meminfo', but requires less > banks, defined by the number in NR_SHMEM_BANKS, which is 32 at the > moment and should be enough for the current use cases, the value > might be increased in te future if needed. > Finally, this structure hosts also the extra information for the > static shared memory banks. > The fields 'bank' and 'extra' of this structure are meant to be > [...] reads ok ~Michal
Re: [PATCH v2 09/13] xen/arm: Reduce struct membank size on static shared memory
Afterwards, create a new structure 'struct shared_meminfo' which has the same interface of 'struct meminfo', but requires less >>> I would expect some justification for selecting 32 as the max number of >>> shmem banks >> >> So I have to say I picked up a value I thought was ok for the amount of >> shared memory >> Banks, do you think it is too low? The real intention here was to decouple >> the number >> of shared memory banks from the number of generic memory banks, and I felt >> 32 was enough, >> but if you think it might be an issue I could bump it, or we could have a >> Kconfig... > No need for Kconfig. 32 is enough for now but I expect a paragraph in commit > msg that you select > 32 which should be enough for current use cases and can be bumped in the > future in case there is a need. What do you think of this proposal: [...] hence the 'struct membank' won't grow in size. Afterwards, create a new structure 'struct shared_meminfo' which has the same interface of 'struct meminfo', but requires less banks, defined by the number in NR_SHMEM_BANKS, which is 32 at the moment and should be enough for the current use cases, the value might be increased in te future if needed. Finally, this structure hosts also the extra information for the static shared memory banks. The fields 'bank' and 'extra' of this structure are meant to be [...] Cheers, Luca
Re: [PATCH v2 10/13] xen/arm: remove shm holes from extended regions
Hi Luca,
On 09/04/2024 13:45, Luca Fancellu wrote:
>
>
> From: Penny Zheng
>
> Static shared memory acts as reserved memory in guest, so it shall be
> excluded from extended regions.
>
> Extended regions are taken care of under three different scenarios:
> normal DomU, direct-map domain with iommu on, and direct-map domain
> with iommu off.
>
> For normal DomU, we create a new function "remove_shm_holes_for_domU",
> to firstly transfer original outputs into the format of
> "struct rangeset", then use "remove_shm_from_rangeset" to remove static
> shm from them.
>
> For direct-map domain with iommu on, after we get guest shm info from "kinfo",
> we use "remove_shm_from_rangeset" to remove static shm.
>
> For direct-map domain with iommu off, as static shm has already been taken
> care of through reserved memory banks, we do nothing.
Stale info given that shmem is no longer part of reserved memory banks. It's
been taken care
of by removing shmem regions in find_unallocated_memory()
>
> Signed-off-by: Penny Zheng
> Signed-off-by: Luca Fancellu
> ---
> v2:
> - Fixed commit title, fixed comment, moved call of remove_shm_from_rangeset
>after populating rangeset in find_memory_holes, print error code when
>possible
> - used PFN_DOWN where needed
> v1:
> - Rework of
> https://patchwork.kernel.org/project/xen-devel/patch/20231206090623.1932275-8-penny.zh...@arm.com/
> ---
> ---
> xen/arch/arm/domain_build.c | 16 -
> xen/arch/arm/include/asm/domain_build.h | 2 +
> xen/arch/arm/include/asm/static-shmem.h | 18 +
> xen/arch/arm/static-shmem.c | 88 +
> 4 files changed, 121 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 01d66fbde92b..9b36d6bb70c9 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -817,8 +817,8 @@ int __init make_memory_node(const struct domain *d,
> return res;
> }
>
> -static int __init add_ext_regions(unsigned long s_gfn, unsigned long e_gfn,
> - void *data)
> +int __init add_ext_regions(unsigned long s_gfn, unsigned long e_gfn,
> + void *data)
> {
> struct membanks *ext_regions = data;
> paddr_t start, size;
> @@ -978,6 +978,8 @@ static int __init handle_pci_range(const struct
> dt_device_node *dev,
> * - MMIO
> * - Host RAM
> * - PCI aperture
> + * - Static shared memory regions, which are described by special property
> + * "xen,shared-mem"
> */
> static int __init find_memory_holes(const struct kernel_info *kinfo,
> struct membanks *ext_regions)
> @@ -1005,6 +1007,11 @@ static int __init find_memory_holes(const struct
> kernel_info *kinfo,
> goto out;
> }
>
> +/* Remove static shared memory regions */
> +res = remove_shm_from_rangeset(kinfo, mem_holes);
> +if ( res )
> +goto out;
> +
> /*
> * Remove regions described by "reg" and "ranges" properties where
> * the memory is addressable (MMIO, RAM, PCI BAR, etc).
> @@ -1097,7 +1104,10 @@ static int __init find_domU_holes(const struct
> kernel_info *kinfo,
> res = 0;
> }
>
> -return res;
> +if ( res )
> +return res;
> +
> +return remove_shm_holes_for_domU(kinfo, ext_regions);
> }
>
> int __init make_hypervisor_node(struct domain *d,
> diff --git a/xen/arch/arm/include/asm/domain_build.h
> b/xen/arch/arm/include/asm/domain_build.h
> index a6f276cc4263..026d975da28e 100644
> --- a/xen/arch/arm/include/asm/domain_build.h
> +++ b/xen/arch/arm/include/asm/domain_build.h
> @@ -51,6 +51,8 @@ static inline int prepare_acpi(struct domain *d, struct
> kernel_info *kinfo)
> int prepare_acpi(struct domain *d, struct kernel_info *kinfo);
> #endif
>
> +int add_ext_regions(unsigned long s_gfn, unsigned long e_gfn, void *data);
> +
> #endif
>
> /*
> diff --git a/xen/arch/arm/include/asm/static-shmem.h
> b/xen/arch/arm/include/asm/static-shmem.h
> index 90aafc81e740..2e8b138eb989 100644
> --- a/xen/arch/arm/include/asm/static-shmem.h
> +++ b/xen/arch/arm/include/asm/static-shmem.h
> @@ -28,6 +28,12 @@ void early_print_info_shmem(void);
>
> void init_sharedmem_pages(void);
>
> +int remove_shm_from_rangeset(const struct kernel_info *kinfo,
> + struct rangeset *rangeset);
> +
> +int remove_shm_holes_for_domU(const struct kernel_info *kinfo,
> + struct membanks *ext_regions);
> +
> #else /* !CONFIG_STATIC_SHM */
>
> static inline int make_resv_memory_node(const struct kernel_info *kinfo,
> @@ -59,6 +65,18 @@ static inline void early_print_info_shmem(void) {};
>
> static inline void init_sharedmem_pages(void) {};
>
> +static inline int remove_shm_from_rangeset(const struct kernel_info *kinfo,
> + struct rangeset *rangeset)
> +{
> +return 0;
> +}
> +
>
Re: [QEMU][PATCH v3 4/7] xen: let xen_ram_addr_from_mapcache() return -1 in case of not found entry
On Fri, Mar 1, 2024 at 6:08 PM Alex Bennée wrote:
> Vikram Garhwal writes:
>
> > From: Juergen Gross
> >
> > Today xen_ram_addr_from_mapcache() will either abort() or return 0 in
> > case it can't find a matching entry for a pointer value. Both cases
> > are bad, so change that to return an invalid address instead.
> >
> > Signed-off-by: Juergen Gross
> > Reviewed-by: Stefano Stabellini
> > ---
> > hw/xen/xen-mapcache.c | 11 +++
> > 1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> > index dfc412d138..179b7e95b2 100644
> > --- a/hw/xen/xen-mapcache.c
> > +++ b/hw/xen/xen-mapcache.c
> > @@ -396,13 +396,8 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
> > }
> > }
> > if (!found) {
> > -trace_xen_ram_addr_from_mapcache_not_found(ptr);
> > -QTAILQ_FOREACH(reventry, >locked_entries, next) {
> > -
> trace_xen_ram_addr_from_mapcache_found(reventry->paddr_index,
> > - reventry->vaddr_req);
> > -}
>
> If these tracepoints aren't useful they need removing from trace-events.
> However I suspect it would be better to keep them in as they are fairly
> cheap.
>
> Otherwise:
>
> Reviewed-by: Alex Bennée
>
>
Reviewed-by: Edgar E. Iglesias
Re: [QEMU][PATCH v3 2/7] xen: add pseudo RAM region for grant mappings
On Fri, Mar 1, 2024 at 3:06 PM Alex Bennée wrote: > Vikram Garhwal writes: > > > From: Juergen Gross > > > > Add a memory region which can be used to automatically map granted > > memory. It is starting at 0x8000ULL in order to be able to > > distinguish it from normal RAM. > > Is the Xen memory map for HVM guests documented anywhere? I couldn't > find anything googling or on the Xen wiki. I'm guessing this is going to > be shared across all 64 bit HVM arches in Xen? > > Anyway: > > Reviewed-by: Alex Bennée > > Reviewed-by: Edgar E. Iglesias
Re: [QEMU][PATCH v3 6/7] xen: add map and unmap callbacks for grant region
On Fri, Mar 1, 2024 at 12:34 AM Stefano Stabellini wrote: > On Tue, 27 Feb 2024, Vikram Garhwal wrote: > > From: Juergen Gross > > > > Add the callbacks for mapping/unmapping guest memory via grants to the > > special grant memory region. > > > > Signed-off-by: Juergen Gross > > Signed-off-by: Vikram Garhwal > > Reviewed-by: Stefano Stabellini > > Reviewed-by: Edgar E. Iglesias
Re: [PATCH v2 09/13] xen/arm: Reduce struct membank size on static shared memory
On 10/04/2024 12:56, Luca Fancellu wrote: > > > Hi Michal, > >> On 10 Apr 2024, at 11:01, Michal Orzel wrote: >> >> Hi Luca, >> >> On 09/04/2024 13:45, Luca Fancellu wrote: >>> >>> >>> Currently the memory footprint of the static shared memory feature >>> is impacting all the struct meminfo instances with memory space >>> that is not going to be used. >>> >>> To solve this issue, rework the static shared memory extra >>> information linked to the memory bank to another structure, >>> struct shmem_membank_extra, and exploit the struct membank >>> padding to host a pointer to that structure in a union with the >>> enum membank_type, with this trick the 'struct membank' has the >>> same size with or without the static shared memory, given that >>> the 'type' and 'shmem_extra' are never used at the same time, >>> hence the 'struct membank' won't grow in size. >>> >>> Afterwards, create a new structure 'struct shared_meminfo' which >>> has the same interface of 'struct meminfo', but requires less >> I would expect some justification for selecting 32 as the max number of >> shmem banks > > So I have to say I picked up a value I thought was ok for the amount of > shared memory > Banks, do you think it is too low? The real intention here was to decouple > the number > of shared memory banks from the number of generic memory banks, and I felt 32 > was enough, > but if you think it might be an issue I could bump it, or we could have a > Kconfig... No need for Kconfig. 32 is enough for now but I expect a paragraph in commit msg that you select 32 which should be enough for current use cases and can be bumped in the future in case there is a need. > >>> >>> >>> Signed-off-by: Luca Fancellu >> With the find_unallocated_memory() issue fixed: >> Reviewed-by: Michal Orzel > > Thanks, I took the opportunity to improve the comment in that function in > this way, > adding “ (when the feature is enabled)": > > * 3) Remove static shared memory (when the feature is enabled) > > Please tell me if that works for you so I will keep your R-by You can retain Rb. ~Michal
Re: [PATCH v2 09/13] xen/arm: Reduce struct membank size on static shared memory
Hi Michal, > On 10 Apr 2024, at 11:01, Michal Orzel wrote: > > Hi Luca, > > On 09/04/2024 13:45, Luca Fancellu wrote: >> >> >> Currently the memory footprint of the static shared memory feature >> is impacting all the struct meminfo instances with memory space >> that is not going to be used. >> >> To solve this issue, rework the static shared memory extra >> information linked to the memory bank to another structure, >> struct shmem_membank_extra, and exploit the struct membank >> padding to host a pointer to that structure in a union with the >> enum membank_type, with this trick the 'struct membank' has the >> same size with or without the static shared memory, given that >> the 'type' and 'shmem_extra' are never used at the same time, >> hence the 'struct membank' won't grow in size. >> >> Afterwards, create a new structure 'struct shared_meminfo' which >> has the same interface of 'struct meminfo', but requires less > I would expect some justification for selecting 32 as the max number of shmem > banks So I have to say I picked up a value I thought was ok for the amount of shared memory Banks, do you think it is too low? The real intention here was to decouple the number of shared memory banks from the number of generic memory banks, and I felt 32 was enough, but if you think it might be an issue I could bump it, or we could have a Kconfig... >> >> >> Signed-off-by: Luca Fancellu > With the find_unallocated_memory() issue fixed: > Reviewed-by: Michal Orzel Thanks, I took the opportunity to improve the comment in that function in this way, adding “ (when the feature is enabled)": * 3) Remove static shared memory (when the feature is enabled) Please tell me if that works for you so I will keep your R-by Cheers, Luca > > ~Michal
[PATCH] x86/hvm: Fix Misra Rule 19.1 regression
Despite noticing an impending Rule 19.1 violation, the adjustment made (the
uint32_t cast) wasn't sufficient to avoid it. Try again.
Fixes: 6a98383b0877 ("x86/HVM: clear upper halves of GPRs upon entry from
32-bit code")
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Stefano Stabellini
CC: consult...@bugseng.com
CC: Roberto Bagnara
CC: Federico Serafini
CC: Nicola Vetrini
---
xen/arch/x86/include/asm/hvm/hvm.h | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/include/asm/hvm/hvm.h
b/xen/arch/x86/include/asm/hvm/hvm.h
index 595253babeaf..899233fb257b 100644
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -575,16 +575,16 @@ static inline void hvm_sanitize_regs_fields(struct
cpu_user_regs *regs,
if ( compat )
{
/* Clear GPR upper halves, to counteract guests playing games. */
-regs->rbp = (uint32_t)regs->ebp;
-regs->rbx = (uint32_t)regs->ebx;
-regs->rax = (uint32_t)regs->eax;
-regs->rcx = (uint32_t)regs->ecx;
-regs->rdx = (uint32_t)regs->edx;
-regs->rsi = (uint32_t)regs->esi;
-regs->rdi = (uint32_t)regs->edi;
-regs->rip = (uint32_t)regs->eip;
-regs->rflags = (uint32_t)regs->eflags;
-regs->rsp = (uint32_t)regs->esp;
+regs->rbp = (uint32_t)regs->rbp;
+regs->rbx = (uint32_t)regs->rbx;
+regs->rax = (uint32_t)regs->rax;
+regs->rcx = (uint32_t)regs->rcx;
+regs->rdx = (uint32_t)regs->rdx;
+regs->rsi = (uint32_t)regs->rsi;
+regs->rdi = (uint32_t)regs->rdi;
+regs->rip = (uint32_t)regs->rip;
+regs->rflags = (uint32_t)regs->rflags;
+regs->rsp = (uint32_t)regs->rsp;
}
#ifndef NDEBUG
base-commit: f48299cad5c3c69fdc2c101517a6dab9c9827ea5
--
2.30.2
Re: [RFC PATCH v1 1/2] xen/arm64: entry: Use xen/linkage.h to annotate symbols
On Wed, Apr 10, 2024 at 12:21 PM Andrew Cooper wrote: > On 10/04/2024 10:19 am, Edgar E. Iglesias wrote: > > From: "Edgar E. Iglesias" > > > > Use the generic xen/linkage.h macros when annotating symbols. > > > > Signed-off-by: Edgar E. Iglesias > > --- > > xen/arch/arm/arm64/entry.S | 12 ++-- > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S > > index f963c923bb..6188dd2416 100644 > > --- a/xen/arch/arm/arm64/entry.S > > +++ b/xen/arch/arm/arm64/entry.S > > @@ -480,9 +480,9 @@ guest_fiq_invalid_compat: > > guest_error_compat: > > guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror > > > > -ENTRY(return_to_new_vcpu32) > > +FUNC(return_to_new_vcpu32) > > exithyp=0, compat=1 > > In the new world, you want an END() too, which sets the size of the symbol. > > A good cross-check of this annotation stuff is: > > readelf -Wa xen-syms | grep return_to_new_vcpu32 > > which in this case will tell you that the symbol called > return_to_new_vcpu32 still has a size of 0. > Thanks Andrew, Patch 2/2 adds the END, I should probably have squashed them into one... Best regards, Edgar
Re: [RFC PATCH v1 1/2] xen/arm64: entry: Use xen/linkage.h to annotate symbols
On 10/04/2024 10:19 am, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" > > Use the generic xen/linkage.h macros when annotating symbols. > > Signed-off-by: Edgar E. Iglesias > --- > xen/arch/arm/arm64/entry.S | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S > index f963c923bb..6188dd2416 100644 > --- a/xen/arch/arm/arm64/entry.S > +++ b/xen/arch/arm/arm64/entry.S > @@ -480,9 +480,9 @@ guest_fiq_invalid_compat: > guest_error_compat: > guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror > > -ENTRY(return_to_new_vcpu32) > +FUNC(return_to_new_vcpu32) > exithyp=0, compat=1 In the new world, you want an END() too, which sets the size of the symbol. A good cross-check of this annotation stuff is: readelf -Wa xen-syms | grep return_to_new_vcpu32 which in this case will tell you that the symbol called return_to_new_vcpu32 still has a size of 0. ~Andrew
Re: [PATCH v2 09/13] xen/arm: Reduce struct membank size on static shared memory
Hi Luca, On 09/04/2024 13:45, Luca Fancellu wrote: > > > Currently the memory footprint of the static shared memory feature > is impacting all the struct meminfo instances with memory space > that is not going to be used. > > To solve this issue, rework the static shared memory extra > information linked to the memory bank to another structure, > struct shmem_membank_extra, and exploit the struct membank > padding to host a pointer to that structure in a union with the > enum membank_type, with this trick the 'struct membank' has the > same size with or without the static shared memory, given that > the 'type' and 'shmem_extra' are never used at the same time, > hence the 'struct membank' won't grow in size. > > Afterwards, create a new structure 'struct shared_meminfo' which > has the same interface of 'struct meminfo', but requires less I would expect some justification for selecting 32 as the max number of shmem banks > banks and hosts the extra information for the static shared memory. > The fields 'bank' and 'extra' of this structure are meant to be > linked by the index (e.g. extra[idx] will have the information for > the bank[idx], for i=0..NR_SHMEM_BANKS), the convinient pointer > 'shmem_extra' of 'struct membank' is then linked to the related > 'extra' bank to ease the fruition when a function has access only > to the 'struct membanks common' of 'struct shared_meminfo'. > > The last part of this work is to move the allocation of the > static shared memory banks from the 'reserved_mem' to a new > 'shmem' member of the 'struct bootinfo'. > Change also the 'shm_mem' member type to be 'struct shared_meminfo' > in order to match the above changes and allow a memory space > reduction also in 'struct kernel_info'. > > Signed-off-by: Luca Fancellu With the find_unallocated_memory() issue fixed: Reviewed-by: Michal Orzel ~Michal
Re: [PATCH v2 2/2] x86: Call Shim Verify in the multiboot2 path
On Mon, Apr 8, 2024 at 11:42 AM Jan Beulich wrote:
>
> On 28.03.2024 16:11, Ross Lagerwall wrote:
> > --- a/xen/arch/x86/efi/efi-boot.h
> > +++ b/xen/arch/x86/efi/efi-boot.h
> > @@ -3,6 +3,7 @@
> > * is intended to be included by common/efi/boot.c _only_, and
> > * therefore can define arch specific global variables.
> > */
> > +#include
> > #include
> > #include
> > #include
> > @@ -808,9 +809,69 @@ static const char *__init get_option(const char *cmd,
> > const char *opt)
> > return o;
> > }
> >
> > +#define ALIGN_UP(arg, align) \
> > +(((arg) + (align) - 1) & ~((typeof(arg))(align) - 1))
>
> Nit: I don't think aligning the opening parentheses is an appropriate
> criteria here. Imo either
>
> #define ALIGN_UP(arg, align) \
> (((arg) + (align) - 1) & ~((typeof(arg))(align) - 1))
>
> or
>
> #define ALIGN_UP(arg, align) \
> (((arg) + (align) - 1) & ~((typeof(arg))(align) - 1))
>
> or
>
> #define ALIGN_UP(arg, align) \
> (((arg) + (align) - 1) & ~((typeof(arg))(align) - 1))
>
> .
OK, will fix.
>
> > +static void __init efi_verify_dom0(uint64_t mbi_in)
> > +{
> > +uint64_t ptr;
> > +const multiboot2_tag_t *tag;
> > +EFI_SHIM_LOCK_PROTOCOL *shim_lock;
> > +EFI_STATUS status;
> > +const multiboot2_tag_module_t *kernel = NULL;
> > +const multiboot2_fixed_t *mbi_fix = _p(mbi_in);
> > +static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
> > +static EFI_GUID __initdata global_variable_guid = EFI_GLOBAL_VARIABLE;
> > +
> > +ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
> > +
> > +for ( tag = _p(ptr); (uint64_t)tag - mbi_in < mbi_fix->total_size;
> > + tag = _p(ALIGN_UP((uint64_t)tag + tag->size,
> > MULTIBOOT2_TAG_ALIGN)) )
> > +{
> > +if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
> > +{
> > +kernel = (const multiboot2_tag_module_t *)tag;
> > +break;
>
> This could do with a comment along the lines of what __start_xen() has
> ("Dom0 kernel is always first").
Will add.
>
> > +}
> > +else if ( tag->type == MULTIBOOT2_TAG_TYPE_END )
>
> Not need for "else" here (personally I find such irritating).
OK, I'll remove it.
>
> > +break;
> > +}
> > +
> > +if ( !kernel )
> > +return;
> > +
> > +if ( (status = efi_bs->LocateProtocol(_lock_guid, NULL,
> > + (void **)_lock)) !=
> > EFI_SUCCESS )
> > +{
> > +UINT32 attr;
> > +UINT8 data;
> > +UINTN size = sizeof(data);
> > +
> > +status = efi_rs->GetVariable((CHAR16 *)L"SecureBoot",
> > _variable_guid,
> > + , , );
> > +if ( status == EFI_NOT_FOUND )
> > +return;
> > +
> > +if ( EFI_ERROR(status) )
> > +PrintErrMesg(L"Could not get SecureBoot variable", status);
> > +
> > +if ( attr != (EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > EFI_VARIABLE_RUNTIME_ACCESS) )
> > +PrintErrMesg(L"Unexpected SecureBoot attributes", attr);
>
> This wants to be blexit(), not PrintErrMesg().
blexit() doesn't allow printing the attributes but I can call some
other function like DisplayUint() to do that before calling blexit().
>
> > +if ( size == 1 && data == 0 )
> > +return;
> > +
> > +blexit(L"Could not locate shim but Secure Boot is enabled");
> > +}
> > +
> > +if ( (status = shim_lock->Verify(_p(kernel->mod_start),
> > + kernel->mod_end - kernel->mod_start))
> > != EFI_SUCCESS )
> > +PrintErrMesg(L"Dom0 kernel image could not be verified", status);
> > +}
>
> Overall this is a superset of what efi_start() does. What I'm missing from
> the description is some discussion of why what's done there is not
> sufficient (beyond the env var check, which iirc there once was a patch to
> add it). One could only then judge whether it wouldn't make sense to make
> this function uniformly used by both paths (with mbi_in suitably dealt with
> for the other case).
>
Hmm, I wasn't really looking at efi_start() verification for the
purpose of this patch series. I can update the patch so that
both paths use a common verification function and also describe
how it differs from the simple call to shim verify that currently
exists in efi_start().
Thanks,
Ross
[xen-4.16-testing test] 185283: tolerable FAIL - PUSHED
flight 185283 xen-4.16-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/185283/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt-qcow2 19 guest-start/debian.repeat fail baseline untested test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 185007 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 185007 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 185007 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 185007 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 185007 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 185007 test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 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-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-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-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-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-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-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-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-armhf-armhf-xl-qcow214 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-qcow215 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 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: xen 9c7c50969fa6c7b1e2d24c2c9dfe528079d72df2 baseline version: xen ea1da2ba8ea7c76a8a3a08d11f7de5acb8b17801 Last test of basis 185007 2024-03-12 17:37:05 Z 28 days Testing same since 185283 2024-04-09 12:06:57 Z0 days1 attempts People who touched revisions under test: Bjoern Doebel Jan Beulich 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
Re: [PATCH v2 1/2] x86: Add support for building a multiboot2 PE binary
On Mon, Apr 8, 2024 at 11:25 AM Jan Beulich wrote: > > On 28.03.2024 16:11, Ross Lagerwall wrote: > > In addition to building xen.efi and xen.gz, build xen-mbi.exe. The > > latter is a PE binary that can be used with a multiboot2 loader that > > supports loading PE binaries. > > I have to admit I find .exe a strange extension outside of the Windows > world. Would it be an option to have no extension at all (xen-mbi), or > use xen.mbi? Sure, I have no strong preference on the name. I'll change it to xen-mbi. > > > Using this option allows the binary to be signed and verified by Shim. > > This means the same xen-mbi.exe binary can then be used for BIOS boot, > > UEFI Boot and UEFI boot with Secure Boot verification (all with the > > convenience of GRUB2 as a bootloader). > > With which "UEFI boot" really means "chainloader" from grub? That isn't > required though, is it? I.e. "UEFI boot" ought to work also without > involving grub? There are a few comments here related to terminology and purpose so let me try and clarify them in one place... These are the existing methods of booting Xen on x86 that I know of: * UEFI firmware boots xen.efi directly. This can be used with Secure Boot enabled. * UEFI GRUB2 chainloads xen.efi by calling the firmware's LoadImage/StartImage. This can be used with Secure Boot enabled. * BIOS GRUB2 boots xen.gz via multiboot1 protocol. * BIOS GRUB2 boots xen.gz via multiboot2 protocol. * UEFI GRUB2 boots xen.gz via multiboot2 protocol. This is much the same as the previous ,ethod but it does use a different entry point. This cannot be used with Secure Boot because Shim can only verify PE binaries. These methods _do not_ work: * BIOS GRUB2 boots xen.efi via multiboot2 protocol. * UEFI GRUB2 boots xen.efi via multiboot2 protocol (despite what is implied by https://xenbits.xen.org/docs/unstable/misc/efi.html). * Shim chainloads xen.efi. AFAICT this may have worked some time in the past but it doesn't currently work in my testing. * GRUB2 verifies xen.efi using Shim and then chainloads it using an internal PE loader. This functionality doesn't exist in upstream GRUB. There are some distro patches to implement this functionality but they did not work with Xen when I tested them (probably for the same reason as the previous method). This patch series adds 2 new methods of booting Xen: * BIOS GRUB2 boots xen-mbi via multiboot2 protocol. * UEFI GRUB2 boots xen-mbi via multiboot2 protocol. This supports Secure Boot by making it possible to call back to Shim to verify Xen. We want to add Secure Boot support to XenServer and to that end, the boot methods added by this patch are preferable to the existing boot methods for a few reasons: * We want a similar user experience regardless of whether BIOS or UEFI is used. * When using GRUB2/multiboot, the boot files (xen.gz, vmlinuz, initrd) can be stored outside of the ESP which is preferable since the ESP is less flexible and is often somewhat limited in size. * Using GRUB2/multiboot makes it easier to edit the Xen/Dom0 command-line while booting / recovering a host compared with the config files used by the direct EFI boot. * Using GRUB2 makes it easier to choose different boot options rather than having to use the firmware boot menu which is often quite unpleasant. Yes, we could use a UEFI bootloader like rEFInd to help with this but that then goes back to the first point about user experience. * For development it makes life easier to always have a single Xen binary that is used regardless of whether BIOS/UEFI is used. The terminology used in the patch description was not particularly precise. To clarify, "BIOS boot" means booting xen-mbi via the multiboot2 protocol with a BIOS-booted bootloader (like GRUB2). "(U)EFI boot" means booting xen-mbi via the multiboot2 protocol with a UEFI bootloader (like GRUB2). > > > The new binary is created by modifying xen.efi: > > * Relocations are stripped since they are not needed. > > Because of the changed entry point, aiui. What hasn't become clear to > me is what difference in functionality there's going to be between > booting this new image in "UEFI boot" mode vs using xen.efi. Clearly > xen.efi's own (EFI-level) command line options won't be available. But > it feels like there might be more. Indeed, relocations are not needed because we're using the multiboot2 entry point which handles everything without the need for relocations. Hopefully the long comment above addresses why this patch is useful. > > > * The image base address is set to 0 since it must necessarily be below > > 4 GiB and the loader will relocate it anyway. > > While technically okay, what is the reason for this adjustment? The multiboot2 spec generally uses 32 bit addresses for everything and says: "The bootloader must not load any part of the kernel, the modules, the Multiboot2 information structure, etc. higher than 4 GiB - 1." An image base address above 4
[RFC PATCH v1 0/2] xen/arm: Annotate code symbols
From: "Edgar E. Iglesias" On the way towards Xen safety certification we're evaluating the use of tools to collect code-coverage/profiling information from execution traces. Some tools rely on ELF symbols for code being declared with type FUNC and having a symbol size. We currently annotate some symbols but not all. Also, there seems to be different ways to do the annotation so I'm sending out this RFC to first figure out how we want to do things before I go ahead and edit more of the ARM port. In this first try I've followed the style from commit: b3a9037550 x86: annotate entry points with type and size IIUC, prefering to use macros from the generic framework in xen/linkage.h in favor of ENTRY/ENDPROC. But perhaps we would like to keep using ENTRY() for entry points into the hypervisor? Another way could be to add .type name, %function to the ENTRY macro and use END from xen/linkage.h. Or we can keep using ENTRY/GLOBAL/ENDPROC. Any thoughts or better ideas? Best regards, Edgar Edgar E. Iglesias (2): xen/arm64: entry: Use xen/linkage.h to annotate symbols xen/arm64: entry: Add missing code symbol annotations xen/arch/arm/arm64/entry.S | 72 +- 1 file changed, 48 insertions(+), 24 deletions(-) -- 2.40.1
[RFC PATCH v1 2/2] xen/arm64: entry: Add missing code symbol annotations
From: "Edgar E. Iglesias" Add missing code symbol annotations. Signed-off-by: Edgar E. Iglesias --- xen/arch/arm/arm64/entry.S | 60 ++ 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S index 6188dd2416..af9a592cae 100644 --- a/xen/arch/arm/arm64/entry.S +++ b/xen/arch/arm/arm64/entry.S @@ -289,21 +289,25 @@ b do_bad_mode .endm -hyp_sync_invalid: +FUNC_LOCAL(hyp_sync_invalid) entry hyp=1 invalid BAD_SYNC +END(hyp_sync_invalid) -hyp_irq_invalid: +FUNC_LOCAL(hyp_irq_invalid) entry hyp=1 invalid BAD_IRQ +END(hyp_irq_invalid) -hyp_fiq_invalid: +FUNC_LOCAL(hyp_fiq_invalid) entry hyp=1 invalid BAD_FIQ +END(hyp_fiq_invalid) -hyp_error_invalid: +FUNC_LOCAL(hyp_error_invalid) entry hyp=1 invalid BAD_ERROR +END(hyp_error_invalid) /* * SError received while running in the hypervisor mode. @@ -313,11 +317,12 @@ hyp_error_invalid: * simplicity, as SError should be rare and potentially fatal, * all interrupts are kept masked. */ -hyp_error: +FUNC_LOCAL(hyp_error) entry hyp=1 mov x0, sp bl do_trap_hyp_serror exithyp=1 +END(hyp_error) /* * Synchronous exception received while running in the hypervisor mode. @@ -327,7 +332,7 @@ hyp_error: * some of them. So we want to inherit the state from the interrupted * context. */ -hyp_sync: +FUNC_LOCAL(hyp_sync) entry hyp=1 /* Inherit interrupts */ @@ -338,6 +343,7 @@ hyp_sync: mov x0, sp bl do_trap_hyp_sync exithyp=1 +END(hyp_sync) /* * IRQ received while running in the hypervisor mode. @@ -352,7 +358,7 @@ hyp_sync: * would require some rework in some paths (e.g. panic, livepatch) to * ensure the ordering is enforced everywhere. */ -hyp_irq: +FUNC_LOCAL(hyp_irq) entry hyp=1 /* Inherit D, A, F interrupts and keep I masked */ @@ -365,8 +371,9 @@ hyp_irq: mov x0, sp bl do_trap_irq exithyp=1 +END(hyp_irq) -guest_sync: +FUNC_LOCAL(guest_sync) /* * Save x0, x1 in advance */ @@ -413,8 +420,9 @@ fastpath_out_workaround: mov x1, xzr eret sb +END(guest_sync) -wa2_ssbd: +FUNC_LOCAL(wa2_ssbd) #ifdef CONFIG_ARM_SSBD alternative_cb arm_enable_wa2_handling b wa2_end @@ -450,42 +458,55 @@ wa2_end: mov x0, xzr eret sb -guest_sync_slowpath: +END(wa2_ssbd) + +FUNC_LOCAL(guest_sync_slowpath) /* * x0/x1 may have been scratch by the fast path above, so avoid * to save them. */ guest_vector compat=0, iflags=IFLAGS__AI_, trap=guest_sync, save_x0_x1=0 +END(guest_sync_slowpath) -guest_irq: +FUNC_LOCAL(guest_irq) guest_vector compat=0, iflags=IFLAGS__A__, trap=irq +END(guest_irq) -guest_fiq_invalid: +FUNC_LOCAL(guest_fiq_invalid) entry hyp=0, compat=0 invalid BAD_FIQ +END(guest_fiq_invalid) -guest_error: +FUNC_LOCAL(guest_error) guest_vector compat=0, iflags=IFLAGS__AI_, trap=guest_serror +END(guest_error) -guest_sync_compat: +FUNC_LOCAL(guest_sync_compat) guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_sync +END(guest_sync_compat) -guest_irq_compat: +FUNC_LOCAL(guest_irq_compat) guest_vector compat=1, iflags=IFLAGS__A__, trap=irq +END(guest_irq_compat) -guest_fiq_invalid_compat: +FUNC_LOCAL(guest_fiq_invalid_compat) entry hyp=0, compat=1 invalid BAD_FIQ +END(guest_fiq_invalid_compat) -guest_error_compat: +FUNC_LOCAL(guest_error_compat) guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror +END(guest_error_compat) FUNC(return_to_new_vcpu32) exithyp=0, compat=1 +END(return_to_new_vcpu32) + FUNC(return_to_new_vcpu64) exithyp=0, compat=0 +END(return_to_new_vcpu64) -return_from_trap: +FUNC_LOCAL(return_from_trap) msr daifset, #IFLAGS___I_ /* Mask interrupts */ ldr x21, [sp, #UREGS_PC]/* load ELR */ @@ -524,6 +545,7 @@ return_from_trap: eret sb +END(return_from_trap) /* * Consume pending SError generated by the guest if any. @@ -617,6 +639,7 @@ FUNC(hyp_traps_vector) ventry guest_irq_compat/* IRQ 32-bit EL0/EL1 */ ventry guest_fiq_invalid_compat/* FIQ 32-bit EL0/EL1 */ ventry guest_error_compat /* Error 32-bit EL0/EL1 */ +END(hyp_traps_vector) /* * struct vcpu *__context_switch(struct vcpu *prev, struct vcpu *next) @@ -647,6 +670,7 @@ FUNC(__context_switch) ldr lr, [x8] mov sp, x9 ret +END(__context_switch) /* * Local variables: -- 2.40.1
[RFC PATCH v1 1/2] xen/arm64: entry: Use xen/linkage.h to annotate symbols
From: "Edgar E. Iglesias" Use the generic xen/linkage.h macros when annotating symbols. Signed-off-by: Edgar E. Iglesias --- xen/arch/arm/arm64/entry.S | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S index f963c923bb..6188dd2416 100644 --- a/xen/arch/arm/arm64/entry.S +++ b/xen/arch/arm/arm64/entry.S @@ -480,9 +480,9 @@ guest_fiq_invalid_compat: guest_error_compat: guest_vector compat=1, iflags=IFLAGS__AI_, trap=guest_serror -ENTRY(return_to_new_vcpu32) +FUNC(return_to_new_vcpu32) exithyp=0, compat=1 -ENTRY(return_to_new_vcpu64) +FUNC(return_to_new_vcpu64) exithyp=0, compat=0 return_from_trap: @@ -536,7 +536,7 @@ return_from_trap: * it. So the function will unmask SError exception for a small window and * then mask it again. */ -check_pending_guest_serror: +FUNC_LOCAL(check_pending_guest_serror) /* * Save elr_el2 to check whether the pending SError exception takes * place while we are doing this sync exception. @@ -586,7 +586,7 @@ abort_guest_exit_end: csetx19, ne ret -ENDPROC(check_pending_guest_serror) +END(check_pending_guest_serror) /* * Exception vectors. @@ -597,7 +597,7 @@ ENDPROC(check_pending_guest_serror) .endm .align 11 -ENTRY(hyp_traps_vector) +FUNC(hyp_traps_vector) ventry hyp_sync_invalid/* Synchronous EL2t */ ventry hyp_irq_invalid /* IRQ EL2t */ ventry hyp_fiq_invalid /* FIQ EL2t */ @@ -626,7 +626,7 @@ ENTRY(hyp_traps_vector) * * Returns prev in x0 */ -ENTRY(__context_switch) +FUNC(__context_switch) add x8, x0, #VCPU_arch_saved_context mov x9, sp stp x19, x20, [x8], #16 /* store callee-saved registers */ -- 2.40.1
Re: [XEN PATCH v1 5/5] xen/arm: ffa: support notification
Hi Jens,
> On 9 Apr 2024, at 17:36, Jens Wiklander wrote:
>
> Add support for FF-A notifications, currently limited to an SP (Secure
> Partition) sending an asynchronous notification to a guest.
>
> Guests and Xen itself are made aware of pending notifications with an
> interrupt. The interrupt handler retrieves the notifications using the
> FF-A ABI and deliver them to their destinations.
>
> Signed-off-by: Jens Wiklander
> ---
> xen/arch/arm/tee/Makefile | 1 +
> xen/arch/arm/tee/ffa.c | 58 ++
> xen/arch/arm/tee/ffa_notif.c | 319 +
> xen/arch/arm/tee/ffa_private.h | 71
> 4 files changed, 449 insertions(+)
> create mode 100644 xen/arch/arm/tee/ffa_notif.c
>
> diff --git a/xen/arch/arm/tee/Makefile b/xen/arch/arm/tee/Makefile
> index f0112a2f922d..7c0f46f7f446 100644
> --- a/xen/arch/arm/tee/Makefile
> +++ b/xen/arch/arm/tee/Makefile
> @@ -2,5 +2,6 @@ obj-$(CONFIG_FFA) += ffa.o
> obj-$(CONFIG_FFA) += ffa_shm.o
> obj-$(CONFIG_FFA) += ffa_partinfo.o
> obj-$(CONFIG_FFA) += ffa_rxtx.o
> +obj-$(CONFIG_FFA) += ffa_notif.o
> obj-y += tee.o
> obj-$(CONFIG_OPTEE) += optee.o
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 5209612963e1..ce9757bfeed1 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -39,6 +39,9 @@
> * - at most 32 shared memory regions per guest
> * o FFA_MSG_SEND_DIRECT_REQ:
> * - only supported from a VM to an SP
> + * o FFA_NOTIFICATION_*:
> + * - only supports global notifications, that is, per vCPU notifications
> + * are not supported
> *
> * There are some large locked sections with ffa_tx_buffer_lock and
> * ffa_rx_buffer_lock. Especially the ffa_tx_buffer_lock spinlock used
> @@ -194,6 +197,8 @@ out:
>
> static void handle_features(struct cpu_user_regs *regs)
> {
> +struct domain *d = current->domain;
> +struct ffa_ctx *ctx = d->arch.tee;
> uint32_t a1 = get_user_reg(regs, 1);
> unsigned int n;
>
> @@ -240,6 +245,30 @@ static void handle_features(struct cpu_user_regs *regs)
> BUILD_BUG_ON(PAGE_SIZE != FFA_PAGE_SIZE);
> ffa_set_regs_success(regs, 0, 0);
> break;
> +case FFA_FEATURE_NOTIF_PEND_INTR:
> +if ( ctx->notif.enabled )
> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
> +else
> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +break;
> +case FFA_FEATURE_SCHEDULE_RECV_INTR:
> +if ( ctx->notif.enabled )
> +ffa_set_regs_success(regs, FFA_NOTIF_PEND_INTR_ID, 0);
This should return the RECV_INTR, not the PEND one.
> +else
> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +break;
> +
> +case FFA_NOTIFICATION_BIND:
> +case FFA_NOTIFICATION_UNBIND:
> +case FFA_NOTIFICATION_GET:
> +case FFA_NOTIFICATION_SET:
> +case FFA_NOTIFICATION_INFO_GET_32:
> +case FFA_NOTIFICATION_INFO_GET_64:
> +if ( ctx->notif.enabled )
> +ffa_set_regs_success(regs, 0, 0);
> +else
> +ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> +break;
> default:
> ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED);
> break;
> @@ -305,6 +334,30 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
> get_user_reg(regs, 1)),
>get_user_reg(regs, 3));
> break;
> +case FFA_NOTIFICATION_BIND:
> +e = ffa_handle_notification_bind(get_user_reg(regs, 1),
> + get_user_reg(regs, 2),
> + get_user_reg(regs, 3),
> + get_user_reg(regs, 4));
I would suggest to pass regs and handle the get_user_regs in the function.
> +break;
> +case FFA_NOTIFICATION_UNBIND:
> +e = ffa_handle_notification_unbind(get_user_reg(regs, 1),
> + get_user_reg(regs, 3),
> + get_user_reg(regs, 4));
same here
> +break;
> +case FFA_NOTIFICATION_INFO_GET_32:
> +case FFA_NOTIFICATION_INFO_GET_64:
> +ffa_handle_notification_info_get(regs);
> +return true;
> +case FFA_NOTIFICATION_GET:
> +ffa_handle_notification_get(regs);
> +return true;
> +case FFA_NOTIFICATION_SET:
> +e = ffa_handle_notification_set(get_user_reg(regs, 1),
> +get_user_reg(regs, 2),
> +get_user_reg(regs, 3),
> +get_user_reg(regs, 4));
same here
> +break;
>
> default:
> gprintk(XENLOG_ERR, "ffa: unhandled fid 0x%x\n", fid);
> @@ -348,6 +401,9 @@ static int ffa_domain_init(struct domain *d)
> if ( !ffa_partinfo_domain_init(d) )
> return -EIO;
>
> +if ( !ffa_notif_domain_init(d) )
>
Re: [PATCH v2 07/13] xen/arm: Avoid code duplication in check_reserved_regions_overlap
Hi Luca, On 09/04/2024 13:45, Luca Fancellu wrote: > > > The function check_reserved_regions_overlap is calling > 'meminfo_overlap_check' on the same type of structure, this code > can be written in a way to avoid code duplication, so rework the > function to do that. > > Signed-off-by: Luca Fancellu Reviewed-by: Michal Orzel ~Michal
Re: [XEN PATCH v1 3/5] xen/arm: ffa: simplify ffa_handle_mem_share()
Hi Jens,
> On 9 Apr 2024, at 17:36, Jens Wiklander wrote:
>
> Simplify ffa_handle_mem_share() by removing the start_page_idx and
> last_page_idx parameters from get_shm_pages() and check that the number
> of pages matches expectations at the end of get_shm_pages().
>
> Signed-off-by: Jens Wiklander
Reviewed-by: Bertrand Marquis
Cheers
Bertrand
> ---
> xen/arch/arm/tee/ffa_shm.c | 18 ++
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c
> index 75a5b66aeb4c..370d83ec5cf8 100644
> --- a/xen/arch/arm/tee/ffa_shm.c
> +++ b/xen/arch/arm/tee/ffa_shm.c
> @@ -159,10 +159,9 @@ static int32_t ffa_mem_reclaim(uint32_t handle_lo,
> uint32_t handle_hi,
> */
> static int get_shm_pages(struct domain *d, struct ffa_shm_mem *shm,
> const struct ffa_address_range *range,
> - uint32_t range_count, unsigned int start_page_idx,
> - unsigned int *last_page_idx)
> + uint32_t range_count)
> {
> -unsigned int pg_idx = start_page_idx;
> +unsigned int pg_idx = 0;
> gfn_t gfn;
> unsigned int n;
> unsigned int m;
> @@ -191,7 +190,9 @@ static int get_shm_pages(struct domain *d, struct
> ffa_shm_mem *shm,
> }
> }
>
> -*last_page_idx = pg_idx;
> +/* The ranges must add up */
> +if ( pg_idx < shm->page_count )
> +return FFA_RET_INVALID_PARAMETERS;
>
> return FFA_RET_OK;
> }
> @@ -460,7 +461,6 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
> struct domain *d = current->domain;
> struct ffa_ctx *ctx = d->arch.tee;
> struct ffa_shm_mem *shm = NULL;
> -unsigned int last_page_idx = 0;
> register_t handle_hi = 0;
> register_t handle_lo = 0;
> int ret = FFA_RET_DENIED;
> @@ -570,15 +570,9 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
> goto out;
> }
>
> -ret = get_shm_pages(d, shm, region_descr->address_range_array,
> range_count,
> -0, _page_idx);
> +ret = get_shm_pages(d, shm, region_descr->address_range_array,
> range_count);
> if ( ret )
> goto out;
> -if ( last_page_idx != shm->page_count )
> -{
> -ret = FFA_RET_INVALID_PARAMETERS;
> -goto out;
> -}
>
> /* Note that share_shm() uses our tx buffer */
> spin_lock(_tx_buffer_lock);
> --
> 2.34.1
>
Re: [XEN PATCH v1 2/5] xen/arm: ffa: use ACCESS_ONCE()
Hi Jens,
> On 9 Apr 2024, at 17:36, Jens Wiklander wrote:
>
> Replace read_atomic() with ACCESS_ONCE() to match the intended use, that
> is, to prevent the compiler from (via optimization) reading shared
> memory more than once.
This definitely makes sense.
>
> Signed-off-by: Jens Wiklander
Reviewed-by: Bertrand Marquis
Cheers
Bertrand
> ---
> xen/arch/arm/tee/ffa_shm.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c
> index eed9ad2d2986..75a5b66aeb4c 100644
> --- a/xen/arch/arm/tee/ffa_shm.c
> +++ b/xen/arch/arm/tee/ffa_shm.c
> @@ -7,6 +7,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -171,8 +172,8 @@ static int get_shm_pages(struct domain *d, struct
> ffa_shm_mem *shm,
>
> for ( n = 0; n < range_count; n++ )
> {
> -page_count = read_atomic([n].page_count);
> -addr = read_atomic([n].address);
> +page_count = ACCESS_ONCE(range[n].page_count);
> +addr = ACCESS_ONCE(range[n].address);
> for ( m = 0; m < page_count; m++ )
> {
> if ( pg_idx >= shm->page_count )
> @@ -527,13 +528,13 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
> goto out_unlock;
>
> mem_access = ctx->tx + trans.mem_access_offs;
> -if ( read_atomic(_access->access_perm.perm) != FFA_MEM_ACC_RW )
> +if ( ACCESS_ONCE(mem_access->access_perm.perm) != FFA_MEM_ACC_RW )
> {
> ret = FFA_RET_NOT_SUPPORTED;
> goto out_unlock;
> }
>
> -region_offs = read_atomic(_access->region_offs);
> +region_offs = ACCESS_ONCE(mem_access->region_offs);
> if ( sizeof(*region_descr) + region_offs > frag_len )
> {
> ret = FFA_RET_NOT_SUPPORTED;
> @@ -541,8 +542,8 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
> }
>
> region_descr = ctx->tx + region_offs;
> -range_count = read_atomic(_descr->address_range_count);
> -page_count = read_atomic(_descr->total_page_count);
> +range_count = ACCESS_ONCE(region_descr->address_range_count);
> +page_count = ACCESS_ONCE(region_descr->total_page_count);
>
> if ( !page_count )
> {
> @@ -557,7 +558,7 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
> goto out_unlock;
> }
> shm->sender_id = trans.sender_id;
> -shm->ep_id = read_atomic(_access->access_perm.endpoint_id);
> +shm->ep_id = ACCESS_ONCE(mem_access->access_perm.endpoint_id);
>
> /*
> * Check that the Composite memory region descriptor fits.
> --
> 2.34.1
>
Re: [PATCH v4 1/3] xen/arm: Add imx8q{m,x} platform glue
Hi John,
On 10/04/2024 08:53, John Ernberg wrote:
>
>
> On 4/9/24 8:47 AM, Michal Orzel wrote:
>> Hi John,
>>
>> On 08/04/2024 18:11, John Ernberg wrote:
>>>
>>>
>>> When using Linux for dom0 there are a bunch of drivers that need to do SMC
>>> SIP calls into the firmware to enable certain hardware bits like the
>>> watchdog.
>>>
>>> Provide a basic platform glue that implements the needed SMC forwarding.
>>>
>>> The format of these calls are as follows:
>>> - reg 0: function ID
>>> - reg 1: subfunction ID (when there's a subfunction)
>>> remaining regs: args
>>>
>>> For now we only allow Dom0 to make these calls as they are all managing
>>> hardware. There is no specification for these SIP calls, the IDs and names
>>> have been extracted from the upstream linux kernel and the vendor kernel.
>>>
>>> Most of the SIP calls are only available for the iMX8M series of SoCs, so
>>> they are easy to reject and they need to be revisited when iMX8M series
>>> support is added.
>> Stale paragraph. Should be removed given that the driver targets only
>> Q{M,XP}.
>>
>>>
>>> From the other calls we can reject CPUFREQ because Dom0 cannot make an
>>> informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake
>>> up from suspend, which Xen doesn't support at this time.
>>>
>>> This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which
>>> for now are allowed to Dom0.
>> BUILDINFO, TEMP ALARM are leftovers from previous revision.
>>
>>>
>>> NOTE: This code is based on code found in NXP Xen tree located here:
>>> https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c
>>>
>>> Signed-off-by: Peng Fan
>>> [jernberg: Add SIP call filtering]
>>> Signed-off-by: John Ernberg
>> Reviewed-by: Michal Orzel
>>
>> The commit msg can be fixed on commit.
>>
>> ~Michal
>
>
> Apologies for forgetting to adjust that. Let me know if it's easier for
> you if I do a v5 with the fixed commit message.
>
> Thanks! // John Ernberg
The series is already committed and Stefano fixed my remarks on commit.
~Michal
Re: [XEN PATCH v1 1/5] xen/arm: ffa: refactor ffa_handle_call()
Hi Jens, > On 9 Apr 2024, at 17:36, Jens Wiklander wrote: > > Refactors the large switch block in ffa_handle_call() to use common code > for the simple case where it's either an error code or success with no > further parameters. > > Signed-off-by: Jens Wiklander Reviewed-by: Bertrand Marquis Cheers Bertrand > --- > xen/arch/arm/tee/ffa.c | 30 ++ > 1 file changed, 10 insertions(+), 20 deletions(-) > > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c > index 8665201e34a9..5209612963e1 100644 > --- a/xen/arch/arm/tee/ffa.c > +++ b/xen/arch/arm/tee/ffa.c > @@ -273,18 +273,10 @@ static bool ffa_handle_call(struct cpu_user_regs *regs) > case FFA_RXTX_MAP_64: > e = ffa_handle_rxtx_map(fid, get_user_reg(regs, 1), > get_user_reg(regs, 2), get_user_reg(regs, 3)); > -if ( e ) > -ffa_set_regs_error(regs, e); > -else > -ffa_set_regs_success(regs, 0, 0); > -return true; > +break; > case FFA_RXTX_UNMAP: > e = ffa_handle_rxtx_unmap(); > -if ( e ) > -ffa_set_regs_error(regs, e); > -else > -ffa_set_regs_success(regs, 0, 0); > -return true; > +break; > case FFA_PARTITION_INFO_GET: > e = ffa_handle_partition_info_get(get_user_reg(regs, 1), > get_user_reg(regs, 2), > @@ -299,11 +291,7 @@ static bool ffa_handle_call(struct cpu_user_regs *regs) > return true; > case FFA_RX_RELEASE: > e = ffa_handle_rx_release(); > -if ( e ) > -ffa_set_regs_error(regs, e); > -else > -ffa_set_regs_success(regs, 0, 0); > -return true; > +break; > case FFA_MSG_SEND_DIRECT_REQ_32: > case FFA_MSG_SEND_DIRECT_REQ_64: > handle_msg_send_direct_req(regs, fid); > @@ -316,17 +304,19 @@ static bool ffa_handle_call(struct cpu_user_regs *regs) > e = ffa_handle_mem_reclaim(regpair_to_uint64(get_user_reg(regs, 2), > get_user_reg(regs, 1)), >get_user_reg(regs, 3)); > -if ( e ) > -ffa_set_regs_error(regs, e); > -else > -ffa_set_regs_success(regs, 0, 0); > -return true; > +break; > > default: > gprintk(XENLOG_ERR, "ffa: unhandled fid 0x%x\n", fid); > ffa_set_regs_error(regs, FFA_RET_NOT_SUPPORTED); > return true; > } > + > +if ( e ) > +ffa_set_regs_error(regs, e); > +else > +ffa_set_regs_success(regs, 0, 0); > +return true; > } > > static int ffa_domain_init(struct domain *d) > -- > 2.34.1 >
Re: [PATCH v4 1/3] xen/arm: Add imx8q{m,x} platform glue
On 4/9/24 8:47 AM, Michal Orzel wrote:
> Hi John,
>
> On 08/04/2024 18:11, John Ernberg wrote:
>>
>>
>> When using Linux for dom0 there are a bunch of drivers that need to do SMC
>> SIP calls into the firmware to enable certain hardware bits like the
>> watchdog.
>>
>> Provide a basic platform glue that implements the needed SMC forwarding.
>>
>> The format of these calls are as follows:
>> - reg 0: function ID
>> - reg 1: subfunction ID (when there's a subfunction)
>> remaining regs: args
>>
>> For now we only allow Dom0 to make these calls as they are all managing
>> hardware. There is no specification for these SIP calls, the IDs and names
>> have been extracted from the upstream linux kernel and the vendor kernel.
>>
>> Most of the SIP calls are only available for the iMX8M series of SoCs, so
>> they are easy to reject and they need to be revisited when iMX8M series
>> support is added.
> Stale paragraph. Should be removed given that the driver targets only Q{M,XP}.
>
>>
>> From the other calls we can reject CPUFREQ because Dom0 cannot make an
>> informed decision regarding CPU frequency scaling, WAKEUP_SRC is to wake
>> up from suspend, which Xen doesn't support at this time.
>>
>> This leaves the TIME SIP, OTP SIPs, BUILDINFO SIP and TEMP ALARM SIP, which
>> for now are allowed to Dom0.
> BUILDINFO, TEMP ALARM are leftovers from previous revision.
>
>>
>> NOTE: This code is based on code found in NXP Xen tree located here:
>> https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c
>>
>> Signed-off-by: Peng Fan
>> [jernberg: Add SIP call filtering]
>> Signed-off-by: John Ernberg
> Reviewed-by: Michal Orzel
>
> The commit msg can be fixed on commit.
>
> ~Michal
Apologies for forgetting to adjust that. Let me know if it's easier for
you if I do a v5 with the fixed commit message.
Thanks! // John Ernberg
