[PATCH] x86/xen: fix percpu vcpu_info allocation
Today the percpu struct vcpu_info is allocated via DEFINE_PER_CPU(),
meaning that it could cross a page boundary. In this case registering
it with the hypervisor will fail, resulting in a panic().
This can easily be fixed by using DEFINE_PER_CPU_ALIGNED() instead,
as struct vcpu_info is guaranteed to have a size of 64 bytes, matching
the cache line size of x86 64-bit processors (Xen doesn't support
32-bit processors).
Fixes: 5ead97c84fa7 ("xen: Core Xen implementation")
Signed-off-by: Juergen Gross
---
arch/x86/xen/enlighten.c | 6 +-
arch/x86/xen/xen-ops.h | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 0337392a3121..3c61bb98c10e 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -33,9 +33,12 @@ EXPORT_SYMBOL_GPL(hypercall_page);
* and xen_vcpu_setup for details. By default it points to
share_info->vcpu_info
* but during boot it is switched to point to xen_vcpu_info.
* The pointer is used in xen_evtchn_do_upcall to acknowledge pending events.
+ * Make sure that xen_vcpu_info doesn't cross a page boundary by making it
+ * cache-line aligned (the struct is guaranteed to have a size of 64 bytes,
+ * which matches the cache line size of 64-bit x86 processors).
*/
DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
-DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
+DEFINE_PER_CPU_ALIGNED(struct vcpu_info, xen_vcpu_info);
/* Linux <-> Xen vCPU id mapping */
DEFINE_PER_CPU(uint32_t, xen_vcpu_id);
@@ -160,6 +163,7 @@ void xen_vcpu_setup(int cpu)
int err;
struct vcpu_info *vcpup;
+ BUILD_BUG_ON(sizeof(*vcpup) > SMP_CACHE_BYTES);
BUG_ON(HYPERVISOR_shared_info == _dummy_shared_info);
/*
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 408a2aa66c69..a87ab36889e7 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -21,7 +21,7 @@ extern void *xen_initial_gdt;
struct trap_info;
void xen_copy_trap_info(struct trap_info *traps);
-DECLARE_PER_CPU(struct vcpu_info, xen_vcpu_info);
+DECLARE_PER_CPU_ALIGNED(struct vcpu_info, xen_vcpu_info);
DECLARE_PER_CPU(unsigned long, xen_cr3);
DECLARE_PER_CPU(unsigned long, xen_current_cr3);
--
2.35.3
Re: [XEN PATCH v2] xen/sort: address violations of MISRA C:2012 Rule 8.2
On 23.11.2023 13:39, Federico Serafini wrote: > Add missing parameter names. No functional change. > > Signed-off-by: Federico Serafini Acked-by: Jan Beulich
[xen-4.17-testing test] 183842: tolerable FAIL - PUSHED
flight 183842 xen-4.17-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/183842/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 183760 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 183760 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 183760 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183760 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183760 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 183760 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 183760 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183760 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 183760 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 183760 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183760 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183760 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-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-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-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: xen e1f9cb16e2efbb202f2f8a9aa7c5ff1d392ece33 baseline version: xen 28f44b603fd86c233726bdc2a11b6325f102471a Last test of basis 183760 2023-11-15 04:51:48 Z9 days Testing same since 183842 2023-11-23 11:39:19 Z0 days1 attempts People who touched revisions under test: Juergen Gross jobs: build-amd64-xsm
[xen-4.18-testing test] 183843: tolerable FAIL - PUSHED
flight 183843 xen-4.18-testing real [real] flight 183849 xen-4.18-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/183843/ http://logs.test-lab.xenproject.org/osstest/logs/183849/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-pygrub 19 guest-localmigrate/x10 fail pass in 183849-retest test-amd64-i386-qemuu-rhel6hvm-amd 14 guest-start/redhat.repeat fail pass in 183849-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 183777 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183777 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183777 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183777 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 183777 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183777 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 183777 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 183777 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 183777 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183777 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 183777 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 183777 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-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-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-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-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-xsm 15 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-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt 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-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 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-qcow2 14 migrate-support-checkfail never pass version targeted for testing: xen 3f9390fea5c51a6d64596d295902d28931eeca4c baseline version: xen
[PATCH v2 0/2] Mini-OS: hide mini-os internal symbols
In order to avoid conflicts due to symbols with the same name when linking Mini-OS with an application, hide all Mini9-OS internal symbols from the application by linking the Mini-OS kernel individually and then removing all symbols which should be used internally only. Changes in V2: - added more symbols in patch 2 Juergen Gross (2): Mini-OS: link kernel separately Mini-OS: keep a positive list of externally visible symbols Makefile| 8 +- mini-os.map | 295 2 files changed, 301 insertions(+), 2 deletions(-) create mode 100644 mini-os.map -- 2.35.3
[PATCH] tools/xenstored: potentially split trace_io() out message
Today write_messages() will call trace_io() after having written the
complete message to the ring buffer or socket.
In case the message can't be written in one go, split it by writing
one trace entry when starting the write and one when finishing it.
Signed-off-by: Juergen Gross
---
This patch is meant to go on top of the patch "tools/xenstored: remove
\"-V\" command line option" in order to not lose any possible debug
information.
---
tools/xenstored/core.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index 9046b200bc..a14b240ed9 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -121,7 +121,7 @@ void trace(const char *fmt, ...)
static void trace_io(const struct connection *conn,
const struct buffered_data *data,
-int out)
+const char *type)
{
unsigned int i;
time_t now;
@@ -134,8 +134,7 @@ static void trace_io(const struct connection *conn,
tm = localtime();
trace("io: %s %p (d%u) %04d%02d%02d %02d:%02d:%02d %s (",
- out ? "OUT" : "IN", conn, conn->id,
- tm->tm_year + 1900, tm->tm_mon + 1,
+ type, conn, conn->id, tm->tm_year + 1900, tm->tm_mon + 1,
tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec,
sockmsg_string(data->hdr.msg.type));
@@ -321,43 +320,54 @@ static bool write_messages(struct connection *conn)
{
int ret;
struct buffered_data *out;
+ bool started = false;
out = list_top(>out_list, struct buffered_data, list);
if (out == NULL)
return true;
if (out->inhdr) {
+ started = !out->used;
ret = conn->funcs->write(conn, out->hdr.raw + out->used,
sizeof(out->hdr) - out->used);
if (ret < 0)
- return false;
+ goto err;
out->used += ret;
if (out->used < sizeof(out->hdr))
- return true;
+ goto start;
out->inhdr = false;
out->used = 0;
/* Second write might block if non-zero. */
if (out->hdr.msg.len && !conn->domain)
- return true;
+ goto start;
}
ret = conn->funcs->write(conn, out->buffer + out->used,
out->hdr.msg.len - out->used);
if (ret < 0)
- return false;
+ goto err;
out->used += ret;
if (out->used != out->hdr.msg.len)
- return true;
+ goto start;
- trace_io(conn, out, 1);
+ trace_io(conn, out, started ? "OUT" : "OUT(END)");
free_buffered_data(out, conn);
return true;
+
+ err:
+ trace_io(conn, out, "OUT(ERR)");
+ return false;
+
+ start:
+ if (started)
+ trace_io(conn, out, "OUT(START)");
+ return true;
}
static int undelay_request(void *_req)
@@ -2067,7 +2077,7 @@ static void process_message(struct connection *conn,
struct buffered_data *in)
/* At least send_error() and send_reply() expects conn->in == in */
assert(conn->in == in);
- trace_io(conn, in, 0);
+ trace_io(conn, in, "IN");
if ((unsigned int)type >= XS_TYPE_COUNT || !wire_funcs[type].func) {
eprintf("Client unknown operation %i", type);
--
2.35.3
[PATCH v2 0/2] Mini-OS: hide mini-os internal symbols
In order to avoid conflicts due to symbols with the same name when linking Mini-OS with an application, hide all Mini9-OS internal symbols from the application by linking the Mini-OS kernel individually and then removing all symbols which should be used internally only. Changes in V2: - added more symbols in patch 2 Juergen Gross (2): Mini-OS: link kernel separately Mini-OS: keep a positive list of externally visible symbols Makefile| 8 +- mini-os.map | 295 2 files changed, 301 insertions(+), 2 deletions(-) create mode 100644 mini-os.map -- 2.35.3
[PATCH v2 1/2] Mini-OS: link kernel separately
Add an additional link step with linking all Mini-OS kernel binaries into a single object file. This is done in preparation of hiding Mini-OS internal symbols before linking the kernel with libraries and an application. Signed-off-by: Juergen Gross Reviewed-by: Samuel Thibault --- Makefile | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7ee181a2..85c6db75 100644 --- a/Makefile +++ b/Makefile @@ -164,8 +164,11 @@ endif $(OBJ_DIR)/arch/x86/minios-x86%.lds: arch/x86/minios-x86.lds.S $(CPP) $(ASFLAGS) -P $< -o $@ -$(OBJ_DIR)/$(TARGET): $(OBJS) $(APP_O) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds - $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(APP_O) $(OBJS) $(LDARCHLIB) $(LDLIBS) -o $@.o +$(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds + $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@ + +$(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) + $(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o $@.o $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* -G _start $@.o $@.o $(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@-debug strip -s $@-debug -o $@ -- 2.35.3
[PATCH v2 2/2] Mini-OS: keep a positive list of externally visible symbols
Add a mini-os.map file containing all global symbols that are allowed to be referenced by an application or library. Hide all other symbols of Mini-OS from being visible externally. Signed-off-by: Juergen Gross --- V2: - added more symbols (Samuel Thibault) - sorted symbols in each section alphabetically --- Makefile| 3 +- mini-os.map | 295 2 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 mini-os.map diff --git a/Makefile b/Makefile index 85c6db75..d4768110 100644 --- a/Makefile +++ b/Makefile @@ -164,8 +164,9 @@ endif $(OBJ_DIR)/arch/x86/minios-x86%.lds: arch/x86/minios-x86.lds.S $(CPP) $(ASFLAGS) -P $< -o $@ -$(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds +$(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds mini-os.map $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@ + $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* --keep-global-symbols=mini-os.map $@ $@ $(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o $@.o diff --git a/mini-os.map b/mini-os.map new file mode 100644 index ..58a3a0ee --- /dev/null +++ b/mini-os.map @@ -0,0 +1,295 @@ +# Mini-OS symbols being externally visible +# entry point +_start +# Mini-OS service functions +alloc_fd +alloc_file_type +alloc_pages +bind_pirq +bind_virq +block +clear_evtchn +console_print +create_thread +do_map_frames +event_queue +evtchn_alloc_unbound +evtchn_bind_interdomain +evtchn_get_peercontext +exit_thread +free_pages +get_domid +get_file_from_fd +gntmap_fini +gntmap_init +gntmap_map_grant_refs +gntmap_munmap +gntmap_set_max_grants +gnttabop_error +gnttab_alloc_and_grant +gnttab_grant_access +gnttab_grant_transfer +gnttab_end_transfer +gnttab_end_access +hypercall_page +ioremap +ioremap_nocache +iounmap +map_frames_ex +map_frame_rw +map_frame_virt +mask_evtchn +msleep +need_pgt +printk +schedule +stop_kernel +unbind_evtchn +unmap_frames +unmask_evtchn +wake +xencons_ring_avail +xprintk +__local_irq_restore +__local_irq_save +# libgcc +__divdi3 +__moddi3 +__qdivrem +__udivdi3 +__udivmoddi4 +__umoddi3 +# libc +accept +bind +cfmakeraw +chdir +clock_gettime +close +closedir +closelog +connect +do_exit +dup +dup2 +err +errx +execv +fcntl +ffs +ffsl +ffsll +fork +free +fstat64 +fsync +ftruncate +getegid +geteuid +getgid +gethostname +getpagesize +getpeername +getpid +getsockname +getsockopt +gettimeofday +getuid +htonl +htons +inet_aton +inet_ntoa +ioctl +isatty +kill +link +listen +lockf +lseek64 +malloc +memcmp +memcpy +memset +mkdir +mmap64 +munmap +nanosleep +nice +ntohl +ntohs +open64 +opendir +openlog +pipe +poll +posix_openpt +read +readdir +realloc +recv +recvfrom +rmdir +sbrk +scnprintf +select +select_read_flag +send +sendto +setsid +setsockopt +shutdown +sigaction +sleep +snprintf +socket +sprintf +sscanf +stat +strcat +strchr +strcmp +strcpy +strdup +strlen +strncmp +strncpy +strnlen +strrchr +strstr +strtoq +strtoul +strtouq +sysconf +syslog +tcgetattr +tcsetattr +umask +unlink +usleep +verr +verrx +vscnprintf +vsnprintf +vsprintf +vsscanf +vsyslog +vwarn +vwarnx +waitpid +warn +warnx +write +_ctype +_exit +_fini +_init +___lock_acquire +___lock_acquire_recursive +___lock_init_recursive +___lock_release +___lock_release_recursive +# 9pfront driver +init_9pfront +shutdown_9pfront +# blkfront driver +blkfront_aio +blkfront_aio_poll +blkfront_aio_push_operation +blkfront_io +blkfront_open +blkfront_queue +blkfront_sync +init_blkfront +shutdown_blkfront +# fbfront driver +fbfront_open +fbfront_receive +fbfront_resize +fbfront_update +init_fbfront +shutdown_fbfront +# kbdfront driver +init_kbdfront +kbdfront_open +kbdfront_receive +shutdown_kbdfront +# netfront driver +init_netfront +netfront_get_gateway +netfront_get_netmask +netfront_receive +netfront_tap_open +netfront_xmit +networking_set_addr +resume_netfront +shutdown_netfront +start_networking +stop_networking +suspend_netfront +# pcifront driver +init_pcifront +pcifront_conf_read +pcifront_conf_write +pcifront_disable_msi +pcifront_disable_msix +pcifront_enable_msi +pcifront_enable_msix +pcifront_op +pcifront_scan +shutdown_pcifront +# tpmback driver +init_tpmback +shutdown_tpmback +tpmback_get_opaque +tpmback_get_peercontext +tpmback_get_uuid +tpmback_num_frontends +tpmback_req +tpmback_req_any +tpmback_resp +tpmback_set_opaque +tpmback_wait_for_frontend_connect +# tpmfront driver +init_tpmfront +shutdown_tpmfront +tpmfront_cmd +tpmfront_open +tpmfront_set_locality +# tpm_tis driver +init_tpm_tis +init_tpm2_tis +tpm_tis_cmd +tpm_tis_open +tpm_tis_request_locality +# xenbus driver +xenbus_get_perms +xenbus_get_self_id +xenbus_ls +xenbus_msg_reply +xenbus_printf +xenbus_read +xenbus_read_integer +xenbus_read_uuid +xenbus_rm +xenbus_set_perms +xenbus_transaction_end
[PATCH v2 2/2] Mini-OS: keep a positive list of externally visible symbols
Add a mini-os.map file containing all global symbols that are allowed to be referenced by an application or library. Hide all other symbols of Mini-OS from being visible externally. Signed-off-by: Juergen Gross --- V2: - added more symbols (Samuel Thibault) - sorted symbols in each section alphabetically --- Makefile| 3 +- mini-os.map | 295 2 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 mini-os.map diff --git a/Makefile b/Makefile index 85c6db75..d4768110 100644 --- a/Makefile +++ b/Makefile @@ -164,8 +164,9 @@ endif $(OBJ_DIR)/arch/x86/minios-x86%.lds: arch/x86/minios-x86.lds.S $(CPP) $(ASFLAGS) -P $< -o $@ -$(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds +$(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds mini-os.map $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@ + $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* --keep-global-symbols=mini-os.map $@ $@ $(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o $@.o diff --git a/mini-os.map b/mini-os.map new file mode 100644 index ..58a3a0ee --- /dev/null +++ b/mini-os.map @@ -0,0 +1,295 @@ +# Mini-OS symbols being externally visible +# entry point +_start +# Mini-OS service functions +alloc_fd +alloc_file_type +alloc_pages +bind_pirq +bind_virq +block +clear_evtchn +console_print +create_thread +do_map_frames +event_queue +evtchn_alloc_unbound +evtchn_bind_interdomain +evtchn_get_peercontext +exit_thread +free_pages +get_domid +get_file_from_fd +gntmap_fini +gntmap_init +gntmap_map_grant_refs +gntmap_munmap +gntmap_set_max_grants +gnttabop_error +gnttab_alloc_and_grant +gnttab_grant_access +gnttab_grant_transfer +gnttab_end_transfer +gnttab_end_access +hypercall_page +ioremap +ioremap_nocache +iounmap +map_frames_ex +map_frame_rw +map_frame_virt +mask_evtchn +msleep +need_pgt +printk +schedule +stop_kernel +unbind_evtchn +unmap_frames +unmask_evtchn +wake +xencons_ring_avail +xprintk +__local_irq_restore +__local_irq_save +# libgcc +__divdi3 +__moddi3 +__qdivrem +__udivdi3 +__udivmoddi4 +__umoddi3 +# libc +accept +bind +cfmakeraw +chdir +clock_gettime +close +closedir +closelog +connect +do_exit +dup +dup2 +err +errx +execv +fcntl +ffs +ffsl +ffsll +fork +free +fstat64 +fsync +ftruncate +getegid +geteuid +getgid +gethostname +getpagesize +getpeername +getpid +getsockname +getsockopt +gettimeofday +getuid +htonl +htons +inet_aton +inet_ntoa +ioctl +isatty +kill +link +listen +lockf +lseek64 +malloc +memcmp +memcpy +memset +mkdir +mmap64 +munmap +nanosleep +nice +ntohl +ntohs +open64 +opendir +openlog +pipe +poll +posix_openpt +read +readdir +realloc +recv +recvfrom +rmdir +sbrk +scnprintf +select +select_read_flag +send +sendto +setsid +setsockopt +shutdown +sigaction +sleep +snprintf +socket +sprintf +sscanf +stat +strcat +strchr +strcmp +strcpy +strdup +strlen +strncmp +strncpy +strnlen +strrchr +strstr +strtoq +strtoul +strtouq +sysconf +syslog +tcgetattr +tcsetattr +umask +unlink +usleep +verr +verrx +vscnprintf +vsnprintf +vsprintf +vsscanf +vsyslog +vwarn +vwarnx +waitpid +warn +warnx +write +_ctype +_exit +_fini +_init +___lock_acquire +___lock_acquire_recursive +___lock_init_recursive +___lock_release +___lock_release_recursive +# 9pfront driver +init_9pfront +shutdown_9pfront +# blkfront driver +blkfront_aio +blkfront_aio_poll +blkfront_aio_push_operation +blkfront_io +blkfront_open +blkfront_queue +blkfront_sync +init_blkfront +shutdown_blkfront +# fbfront driver +fbfront_open +fbfront_receive +fbfront_resize +fbfront_update +init_fbfront +shutdown_fbfront +# kbdfront driver +init_kbdfront +kbdfront_open +kbdfront_receive +shutdown_kbdfront +# netfront driver +init_netfront +netfront_get_gateway +netfront_get_netmask +netfront_receive +netfront_tap_open +netfront_xmit +networking_set_addr +resume_netfront +shutdown_netfront +start_networking +stop_networking +suspend_netfront +# pcifront driver +init_pcifront +pcifront_conf_read +pcifront_conf_write +pcifront_disable_msi +pcifront_disable_msix +pcifront_enable_msi +pcifront_enable_msix +pcifront_op +pcifront_scan +shutdown_pcifront +# tpmback driver +init_tpmback +shutdown_tpmback +tpmback_get_opaque +tpmback_get_peercontext +tpmback_get_uuid +tpmback_num_frontends +tpmback_req +tpmback_req_any +tpmback_resp +tpmback_set_opaque +tpmback_wait_for_frontend_connect +# tpmfront driver +init_tpmfront +shutdown_tpmfront +tpmfront_cmd +tpmfront_open +tpmfront_set_locality +# tpm_tis driver +init_tpm_tis +init_tpm2_tis +tpm_tis_cmd +tpm_tis_open +tpm_tis_request_locality +# xenbus driver +xenbus_get_perms +xenbus_get_self_id +xenbus_ls +xenbus_msg_reply +xenbus_printf +xenbus_read +xenbus_read_integer +xenbus_read_uuid +xenbus_rm +xenbus_set_perms +xenbus_transaction_end
[PATCH v2 1/2] Mini-OS: link kernel separately
Add an additional link step with linking all Mini-OS kernel binaries into a single object file. This is done in preparation of hiding Mini-OS internal symbols before linking the kernel with libraries and an application. Signed-off-by: Juergen Gross Reviewed-by: Samuel Thibault --- Makefile | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7ee181a2..85c6db75 100644 --- a/Makefile +++ b/Makefile @@ -164,8 +164,11 @@ endif $(OBJ_DIR)/arch/x86/minios-x86%.lds: arch/x86/minios-x86.lds.S $(CPP) $(ASFLAGS) -P $< -o $@ -$(OBJ_DIR)/$(TARGET): $(OBJS) $(APP_O) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds - $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(APP_O) $(OBJS) $(LDARCHLIB) $(LDLIBS) -o $@.o +$(OBJ_DIR)/$(TARGET)-kern.o: $(OBJS) arch_lib $(OBJ_DIR)/$(TARGET_ARCH_DIR)/minios-$(MINIOS_TARGET_ARCH).lds + $(LD) -r $(LDFLAGS) $(HEAD_OBJ) $(OBJS) $(LDARCHLIB) -o $@ + +$(OBJ_DIR)/$(TARGET): $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) + $(LD) -r $(LDFLAGS) $(OBJ_DIR)/$(TARGET)-kern.o $(APP_O) $(LDLIBS) -o $@.o $(OBJCOPY) -w -G $(GLOBAL_PREFIX)* -G _start $@.o $@.o $(LD) $(LDFLAGS) $(LDFLAGS_FINAL) $@.o $(EXTRA_OBJS) -o $@-debug strip -s $@-debug -o $@ -- 2.35.3
[PATCH v4 3/6] automation: prevent QEMU access to /dev/mem in PCI passthrough tests
/dev/mem access doesn't work in dom0 in lockdown and in stubdomain. Simulate this environment with removing /dev/mem device node. Full test for lockdown and stubdomain will come later, when all requirements will be in place. Signed-off-by: Marek Marczykowski-Górecki --- This can be applied only after QEMU change is committed. Otherwise the test will fail. --- automation/scripts/qubes-x86-64.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh index d81ed7b931cf..7eabc1bd6ad4 100755 --- a/automation/scripts/qubes-x86-64.sh +++ b/automation/scripts/qubes-x86-64.sh @@ -163,6 +163,8 @@ ifconfig eth0 up ifconfig xenbr0 up ifconfig xenbr0 192.168.0.1 +# ensure QEMU wont have access /dev/mem +rm -f /dev/mem # get domU console content into test log tail -F /var/log/xen/console/guest-domU.log 2>/dev/null | sed -e \"s/^/(domU) /\" & xl create /etc/xen/domU.cfg -- git-series 0.9.1
[PATCH v4 6/6] [DO NOT APPLY] switch to alternative artifact repo
For testing, switch to my containers registry that includes containers rebuilt with changes in this series. --- automation/gitlab-ci/build.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index 32af30ccedc9..52abb12bce48 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -324,7 +324,7 @@ qemu-system-ppc64-8.1.0-ppc64-export: alpine-3.18-rootfs-export: extends: .test-jobs-artifact-common - image: registry.gitlab.com/xen-project/xen/tests-artifacts/alpine:3.18 + image: registry.gitlab.com/xen-project/people/marmarek/xen/tests-artifacts/alpine:3.18 script: - mkdir binaries && cp /initrd.tar.gz binaries/initrd.tar.gz artifacts: @@ -335,7 +335,7 @@ alpine-3.18-rootfs-export: kernel-6.1.19-export: extends: .test-jobs-artifact-common - image: registry.gitlab.com/xen-project/xen/tests-artifacts/kernel:6.1.19 + image: registry.gitlab.com/xen-project/people/marmarek/xen/tests-artifacts/kernel:6.1.19 script: - mkdir binaries && cp /bzImage binaries/bzImage artifacts: -- git-series 0.9.1
[PATCH v4 0/6] MSI-X support with qemu in stubdomain, and other related changes
This series includes changes to make MSI-X working with Linux stubdomain and especially Intel Wifi 6 AX210 card. This takes care of remaining reasons for QEMU to access /dev/mem, but also the Intel Wifi card violating spec by putting some registers on the same page as the MSI-X table. See individual patches for details. This series include also tests for MSI-X using new approach (by preventing QEMU access to /dev/mem). But for it to work, it needs QEMU change that makes use of the changes introduced here. It can be seen at https://github.com/marmarek/qemu/commits/msix Here is the pipeline that used the QEMU fork above: https://gitlab.com/xen-project/people/marmarek/xen/-/pipelines/1083468508 Marek Marczykowski-Górecki (6): x86/msi: passthrough all MSI-X vector ctrl writes to device model x86/hvm: Allow access to registers on the same page as MSI-X table automation: prevent QEMU access to /dev/mem in PCI passthrough tests automation: switch to a wifi card on ADL system [DO NOT APPLY] switch to qemu fork [DO NOT APPLY] switch to alternative artifact repo Config.mk | 4 +- automation/gitlab-ci/build.yaml | 4 +- automation/gitlab-ci/test.yaml | 4 +- automation/scripts/qubes-x86-64.sh | 9 +- automation/tests-artifacts/alpine/3.18.dockerfile | 7 +- automation/tests-artifacts/kernel/6.1.19.dockerfile | 2 +- xen/arch/x86/hvm/vmsi.c | 206 - xen/arch/x86/include/asm/msi.h | 5 +- xen/arch/x86/msi.c | 40 +++- xen/common/kernel.c | 1 +- xen/include/public/features.h | 8 +- 11 files changed, 272 insertions(+), 18 deletions(-) base-commit: f96e2f64576cdbb147391c7cb399d393385719a9 -- git-series 0.9.1
[PATCH v4 2/6] x86/hvm: Allow access to registers on the same page as MSI-X table
Some devices (notably Intel Wifi 6 AX210 card) keep auxiliary registers
on the same page as MSI-X table. Device model (especially one in
stubdomain) cannot really handle those, as direct writes to that page is
refused (page is on the mmio_ro_ranges list). Instead, extend
msixtbl_mmio_ops to handle such accesses too.
Doing this, requires correlating read/write location with guest
of MSI-X table address. Since QEMU doesn't map MSI-X table to the guest,
it requires msixtbl_entry->gtable, which is HVM-only. Similar feature
for PV would need to be done separately.
This will be also used to read Pending Bit Array, if it lives on the same
page, making QEMU not needing /dev/mem access at all (especially helpful
with lockdown enabled in dom0). If PBA lives on another page, QEMU will
map it to the guest directly.
If PBA lives on the same page, discard writes and log a message.
Technically, writes outside of PBA could be allowed, but at this moment
the precise location of PBA isn't saved, and also no known device abuses
the spec in this way (at least yet).
To access those registers, msixtbl_mmio_ops need the relevant page
mapped. MSI handling already has infrastructure for that, using fixmap,
so try to map first/last page of the MSI-X table (if necessary) and save
their fixmap indexes. Note that msix_get_fixmap() does reference
counting and reuses existing mapping, so just call it directly, even if
the page was mapped before. Also, it uses a specific range of fixmap
indexes which doesn't include 0, so use 0 as default ("not mapped")
value - which simplifies code a bit.
GCC gets confused about 'desc' variable:
arch/x86/hvm/vmsi.c: In function ‘msixtbl_range’:
arch/x86/hvm/vmsi.c:553:8: error: ‘desc’ may be used uninitialized
[-Werror=maybe-uninitialized]
553 | if ( desc )
|^
arch/x86/hvm/vmsi.c:537:28: note: ‘desc’ was declared here
537 | const struct msi_desc *desc;
|^~~~
It's conditional initialization is actually correct (in the case where
it isn't initialized, function returns early), but to avoid
build failure initialize it explicitly to NULL anyway.
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v4:
- drop same_page parameter of msixtbl_find_entry(), distinguish two
cases in relevant callers
- rename adj_access_table_idx to adj_access_idx
- code style fixes
- drop alignment check in adjacent_{read,write}() - all callers already
have it earlier
- delay mapping first/last MSI-X pages until preparing device for a
passthrough
v3:
- merge handling into msixtbl_mmio_ops
- extend commit message
v2:
- adjust commit message
- pass struct domain to msixtbl_page_handler_get_hwaddr()
- reduce local variables used only once
- log a warning if write is forbidden if MSI-X and PBA lives on the same
page
- do not passthrough unaligned accesses
- handle accesses both before and after MSI-X table
---
xen/arch/x86/hvm/vmsi.c| 191 --
xen/arch/x86/include/asm/msi.h | 5 +-
xen/arch/x86/msi.c | 40 +++-
3 files changed, 225 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 2436154c40b6..d1e8cb1e30f6 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -180,6 +180,10 @@ static bool msixtbl_initialised(const struct domain *d)
return d->arch.hvm.msixtbl_list.next;
}
+/*
+ * Lookup an msixtbl_entry on the same page as given addr. It's up to the
+ * caller to check if address is strictly part of the table - if relevant.
+ */
static struct msixtbl_entry *msixtbl_find_entry(
struct vcpu *v, unsigned long addr)
{
@@ -187,8 +191,8 @@ static struct msixtbl_entry *msixtbl_find_entry(
struct domain *d = v->domain;
list_for_each_entry( entry, >arch.hvm.msixtbl_list, list )
-if ( addr >= entry->gtable &&
- addr < entry->gtable + entry->table_len )
+if ( PFN_DOWN(addr) >= PFN_DOWN(entry->gtable) &&
+ PFN_DOWN(addr) <= PFN_DOWN(entry->gtable + entry->table_len - 1) )
return entry;
return NULL;
@@ -213,6 +217,131 @@ static struct msi_desc *msixtbl_addr_to_desc(
return NULL;
}
+/*
+ * Returns:
+ * - UINT_MAX if no handling should be done
+ * - UINT_MAX-1 if write should be discarded
+ * - a fixmap idx to use for handling
+ */
+#define ADJACENT_DONT_HANDLE UINT_MAX
+#define ADJACENT_DISCARD_WRITE (UINT_MAX - 1)
+static unsigned int adjacent_handle(
+const struct msixtbl_entry *entry, unsigned long addr, bool write)
+{
+unsigned int adj_type;
+const struct arch_msix *msix;
+
+if ( !entry || !entry->pdev )
+return ADJACENT_DONT_HANDLE;
+
+if ( PFN_DOWN(addr) == PFN_DOWN(entry->gtable) && addr < entry->gtable )
+adj_type = ADJ_IDX_FIRST;
+else if ( PFN_DOWN(addr) == PFN_DOWN(entry->gtable + entry->table_len - 1)
&&
+ addr >= entry->gtable + entry->table_len
[PATCH v4 5/6] [DO NOT APPLY] switch to qemu fork
This makes tests to use patched QEMU, to actually test the new behavior. --- Config.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Config.mk b/Config.mk index 2c43702958eb..dd2687c0e9e7 100644 --- a/Config.mk +++ b/Config.mk @@ -222,8 +222,8 @@ endif OVMF_UPSTREAM_URL ?= https://xenbits.xen.org/git-http/ovmf.git OVMF_UPSTREAM_REVISION ?= ba91d0292e593df8528b66f99c1b0b14fadc8e16 -QEMU_UPSTREAM_URL ?= https://xenbits.xen.org/git-http/qemu-xen.git -QEMU_UPSTREAM_REVISION ?= master +QEMU_UPSTREAM_URL ?= https://github.com/marmarek/qemu +QEMU_UPSTREAM_REVISION ?= origin/msix MINIOS_UPSTREAM_URL ?= https://xenbits.xen.org/git-http/mini-os.git MINIOS_UPSTREAM_REVISION ?= b08019f0b2fbc30c75169a160acb9fd9af5d68f4 -- git-series 0.9.1
[PATCH v4 4/6] automation: switch to a wifi card on ADL system
Switch to a wifi card that has registers on a MSI-X page. This tests the
"x86/hvm: Allow writes to registers on the same page as MSI-X table"
feature. Switch it only for HVM test, because MSI-X adjacent write is
not supported on PV.
This requires also including drivers and firmware in system for tests.
Remove firmware unrelated to the test, to not increase initrd size too
much (all firmware takes over 100MB compressed).
And finally adjusts test script to handle not only eth0 as a test device,
but also wlan0 and connect it to the wifi network.
Signed-off-by: Marek Marczykowski-Górecki
---
This needs two new gitlab variables: WIFI_HW2_SSID and WIFI_HW2_PSK. I'll
provide them in private.
This change requires rebuilding test containers.
This can be applied only after QEMU change is committed. Otherwise the
test will fail.
---
automation/gitlab-ci/test.yaml | 4
automation/scripts/qubes-x86-64.sh | 7 +++
automation/tests-artifacts/alpine/3.18.dockerfile | 7 +++
automation/tests-artifacts/kernel/6.1.19.dockerfile | 2 ++
4 files changed, 20 insertions(+)
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 6aabdb9d156f..931a8fb28e1d 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -195,6 +195,10 @@ adl-pci-pv-x86-64-gcc-debug:
adl-pci-hvm-x86-64-gcc-debug:
extends: .adl-x86-64
+ variables:
+PCIDEV: "00:14.3"
+WIFI_SSID: "$WIFI_HW2_SSID"
+WIFI_PSK: "$WIFI_HW2_PSK"
script:
- ./automation/scripts/qubes-x86-64.sh pci-hvm 2>&1 | tee ${LOGFILE}
needs:
diff --git a/automation/scripts/qubes-x86-64.sh
b/automation/scripts/qubes-x86-64.sh
index 7eabc1bd6ad4..60498ef1e89a 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -94,6 +94,13 @@ on_reboot = "destroy"
domU_check="
set -x -e
interface=eth0
+if [ -e /sys/class/net/wlan0 ]; then
+interface=wlan0
+set +x
+wpa_passphrase "$WIFI_SSID" "$WIFI_PSK" > /etc/wpa_supplicant.conf
+set -x
+wpa_supplicant -B -iwlan0 -c /etc/wpa_supplicant.conf
+fi
ip link set \"\$interface\" up
timeout 30s udhcpc -i \"\$interface\"
pingip=\$(ip -o -4 r show default|cut -f 3 -d ' ')
diff --git a/automation/tests-artifacts/alpine/3.18.dockerfile
b/automation/tests-artifacts/alpine/3.18.dockerfile
index f1b4a8b7a191..b821a291fed3 100644
--- a/automation/tests-artifacts/alpine/3.18.dockerfile
+++ b/automation/tests-artifacts/alpine/3.18.dockerfile
@@ -34,6 +34,13 @@ RUN \
apk add curl && \
apk add udev && \
apk add pciutils && \
+ apk add wpa_supplicant && \
+ # Select firmware for hardware tests
+ apk add linux-firmware-other && \
+ mkdir /lib/firmware-preserve && \
+ mv /lib/firmware/iwlwifi-so-a0-gf-a0* /lib/firmware-preserve/ && \
+ rm -rf /lib/firmware && \
+ mv /lib/firmware-preserve /lib/firmware && \
\
# Xen
cd / && \
diff --git a/automation/tests-artifacts/kernel/6.1.19.dockerfile
b/automation/tests-artifacts/kernel/6.1.19.dockerfile
index 3a4096780d20..84ed5dff23ae 100644
--- a/automation/tests-artifacts/kernel/6.1.19.dockerfile
+++ b/automation/tests-artifacts/kernel/6.1.19.dockerfile
@@ -32,6 +32,8 @@ RUN curl -fsSLO
https://cdn.kernel.org/pub/linux/kernel/v6.x/linux-"$LINUX_VERSI
make xen.config && \
scripts/config --enable BRIDGE && \
scripts/config --enable IGC && \
+scripts/config --enable IWLWIFI && \
+scripts/config --enable IWLMVM && \
cp .config .config.orig && \
cat .config.orig | grep XEN | grep =m |sed 's/=m/=y/g' >> .config && \
make -j$(nproc) bzImage && \
--
git-series 0.9.1
[PATCH v4 1/6] x86/msi: passthrough all MSI-X vector ctrl writes to device model
QEMU needs to know whether clearing maskbit of a vector is really
clearing, or was already cleared before. Currently Xen sends only
clearing that bit to the device model, but not setting it, so QEMU
cannot detect it. Because of that, QEMU is working this around by
checking via /dev/mem, but that isn't the proper approach.
Give all necessary information to QEMU by passing all ctrl writes,
including masking a vector. Advertise the new behavior via
XENVER_get_features, so QEMU can know it doesn't need to access /dev/mem
anymore.
While this commit doesn't move the whole maskbit handling to QEMU (as
discussed on xen-devel as one of the possibilities), it is a necessary
first step anyway. Including telling QEMU it will get all the required
information to do so. The actual implementation would need to include:
- a hypercall for QEMU to control just maskbit (without (re)binding the
interrupt again
- a methor for QEMU to tell Xen it will actually do the work
Those are not part of this series.
Signed-off-by: Marek Marczykowski-Górecki
---
I did not added any control to enable/disable this new behavior (as
Roger have suggested for possible non-QEMU ioreqs). I don't see how the
new behavior could be problematic for some existing ioreq server (they
already received writes to those addresses, just not all of them),
but if that's really necessary, I can probably add a command line option
to restore previous behavior system-wide.
Changes in v4:
- ignore unaligned writes with X86EMUL_OKAY
- restructure the code to forward all writes in _msixtbl_write() instead
of manipulating return value of msixtbl_write() - this makes
WRITE_LEN4_COMPLETION special case unnecessary
- advertise the changed behavior via XENVER_get_features instead of DMOP
v3:
- advertise changed behavior in XEN_DMOP_get_ioreq_server_info - make
"flags" parameter IN/OUT
- move len check back to msixtbl_write() - will be needed there anyway
in a later patch
v2:
- passthrough quad writes to emulator too (Jan)
- (ab)use len==0 for write len=4 completion (Jan), but add descriptive
#define for this magic value
---
xen/arch/x86/hvm/vmsi.c | 19 ++-
xen/common/kernel.c | 1 +
xen/include/public/features.h | 8
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 128f23636279..2436154c40b6 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -283,8 +283,8 @@ static int msixtbl_write(struct vcpu *v, unsigned long
address,
unsigned long flags;
struct irq_desc *desc;
-if ( (len != 4 && len != 8) || (address & (len - 1)) )
-return r;
+if ( !IS_ALIGNED(address, len) )
+return X86EMUL_OKAY;
rcu_read_lock(_rcu_lock);
@@ -345,8 +345,7 @@ static int msixtbl_write(struct vcpu *v, unsigned long
address,
unlock:
spin_unlock_irqrestore(>lock, flags);
-if ( len == 4 )
-r = X86EMUL_OKAY;
+r = X86EMUL_OKAY;
out:
rcu_read_unlock(_rcu_lock);
@@ -357,7 +356,17 @@ static int cf_check _msixtbl_write(
const struct hvm_io_handler *handler, uint64_t address, uint32_t len,
uint64_t val)
{
-return msixtbl_write(current, address, len, val);
+/* ignore invalid length or unaligned writes */
+if ( len != 4 && len != 8 || !IS_ALIGNED(address, len) )
+return X86EMUL_OKAY;
+
+/*
+ * This function returns X86EMUL_UNHANDLEABLE even if write is properly
+ * handled, to propagate it to the device model (so it can keep its
+ * internal state in sync).
+ */
+msixtbl_write(current, address, len, val);
+return X86EMUL_UNHANDLEABLE;
}
static bool cf_check msixtbl_range(
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 08dbaa2a054c..229784c6ce52 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -642,6 +642,7 @@ long do_xen_version(int cmd, XEN_GUEST_HANDLE_PARAM(void)
arg)
fi.submap |= (1U << XENFEAT_direct_mapped);
else
fi.submap |= (1U << XENFEAT_not_direct_mapped);
+fi.submap |= (1U << XENFEAT_dm_msix_all_writes);
break;
default:
return -EINVAL;
diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index 36936f6a4ee0..634534827d43 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -120,6 +120,14 @@
#define XENFEAT_runstate_phys_area 18
#define XENFEAT_vcpu_time_phys_area 19
+/*
+ * If set, Xen will passthrough all MSI-X vector ctrl writes to device model,
+ * not only those unmasking an entry. This allows device model to properly keep
+ * track of the MSI-X table without having to read it from the device behind
+ * Xen's backs. This information is relevant only for device models.
+ */
+#define XENFEAT_dm_msix_all_writes20
+
#define XENFEAT_NR_SUBMAPS 1
#endif /* __XEN_PUBLIC_FEATURES_H__ */
--
git-series 0.9.1
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David,
David Woodhouse writes:
> On 23 November 2023 12:17:57 GMT, Volodymyr Babchuk
> wrote:
>>
>>Hi David,
>>
>>David Woodhouse writes:
>>> Which PV backends do you care about? We already have net, block and console
>>> converted.
>>
>>Well, this is all what we need, actually. Even console only will be
>>sufficient, as we are using QEMU to provide virtio-pci backends, so both
>>storage and networking should be provided by virtio. Are you proposing
>>to just drop this patch at all? I believe we can live without it, yes.
>
> Yeah, I think you can drop anything that's only needed for the legacy
> backends. I'm tempted to make a separate config option to compile those out.
Yep, we need this. Because without the patch ("xen_pvdev: Do not assume
Dom0 when creating a directory") I can't run QEMU in the driver domain:
root@spider-domd:~# qemu-system-aarch64 -machine xenpv -m 128M
xen be core: xs_mkdir device-model/0/backends/vkbd: failed
xen be core: xs_mkdir device-model/0/backends/vkbd: failed
xen be core: xs_mkdir device-model/0/backends/9pfs: failed
xen be core: xs_mkdir device-model/0/backends/9pfs: failed
So yeah, we need something like CONFIG_XEN_LEGACY_BACKENDS option if we
don't want to fix xenstore_mkdir()
--
WBR, Volodymyr
xen | Successful pipeline for staging | 03d6720a
Pipeline #1083211317 has passed! Project: xen ( https://gitlab.com/xen-project/xen ) Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging ) Commit: 03d6720a ( https://gitlab.com/xen-project/xen/-/commit/03d6720a4c62c283f9a9f09858eeccd24299b312 ) Commit Message: tools/pygrub: Restrict depriv operation with RL... Commit Author: Alejandro Vallejo Committed by: Andrew Cooper ( https://gitlab.com/andyhhp ) Pipeline #1083211317 ( https://gitlab.com/xen-project/xen/-/pipelines/1083211317 ) triggered by Ganis ( https://gitlab.com/ganis ) successfully completed 129 jobs in 3 stages. -- You're receiving this email because of your account on gitlab.com.
[xen-unstable-smoke test] 183846: tolerable all pass - PUSHED
flight 183846 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/183846/ 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 03d6720a4c62c283f9a9f09858eeccd24299b312 baseline version: xen f96e2f64576cdbb147391c7cb399d393385719a9 Last test of basis 183844 2023-11-23 15:02:19 Z0 days Testing same since 183846 2023-11-23 19:02:06 Z0 days1 attempts People who touched revisions under test: Alejandro Vallejo Andrew Cooper Juergen Gross Julien Grall Luca Fancellu 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 f96e2f6457..03d6720a4c 03d6720a4c62c283f9a9f09858eeccd24299b312 -> smoke
[xen-unstable test] 183839: tolerable FAIL - PUSHED
flight 183839 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/183839/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183807 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 183807 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183807 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 183807 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 183807 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 183807 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 183807 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183807 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 183807 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183807 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 183807 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183807 test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-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-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-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-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: xen c22fe7213c9b1f99cbc64c33e391afa223f9cd08 baseline version: xen fa2da5bce90b3777aa7a323e1cf201c97b56d278 Last test of basis 183807 2023-11-21 04:17:59 Z2 days Failing since183816 2023-11-21 21:39:08 Z1 days4 attempts Testing same since 183831 2023-11-22 23:38:57 Z0 days2 attempts People who touched
[xen-unstable-smoke test] 183844: tolerable all pass - PUSHED
flight 183844 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/183844/ 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 f96e2f64576cdbb147391c7cb399d393385719a9 baseline version: xen 8f45862580c962791e66df716389bb5b1dd704df Last test of basis 183840 2023-11-23 10:00:25 Z0 days Testing same since 183844 2023-11-23 15:02:19 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Henry Wang Luca Fancellu Stefano Stabellini Stewart Hildebrand 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 8f45862580..f96e2f6457 f96e2f64576cdbb147391c7cb399d393385719a9 -> smoke
[PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID
Both Intel and AMD manuals agree that on x2APIC mode, the APIC LDR and ID
registers are derivable from each other through a fixed formula.
Xen uses that formula, but applies it to vCPU IDs (which are sequential)
rather than x2APIC IDs (which are not, at the moment). As I understand it,
this is an attempt to tightly pack vCPUs into clusters so each cluster has
16 vCPUs rather than 8, but this is problematic for OSs that might read the
x2APIC ID and internally derive LDR (or the other way around)
This patch fixes the implementation so we follow the rules in the x2APIC
spec(s) and covers migrations from broken hypervisors, so LDRs are
preserved even for hotppluggable CPUs and across APIC resets.
While touching that area, I removed the existing printk statement in
vlapic_load_fixup() (as the checks it performed didn't make sense in x2APIC
mode and wouldn't affect the outcome) and put another printk as an else
branch so we get warnings trying to load nonsensical LDR values we don't
know about.
Fixes: f9e0cccf7b35 ("x86/HVM: fix ID handling of x2APIC emulation")
Signed-off-by: Alejandro Vallejo
Reviewed-by: Roger Pau Monné
---
v4:
* Removed "See ()" part of comment in set_x2apic_id()
* Removed _with_ in field name
* Trimmed down comments further
* Rephrased the Xen versions in the comments so it's implied not every
Xen 4.X is affected (as they won't be after this patch is backported)
* Replace Xen 4.18 reference with date+4.19 dev window
---
xen/arch/x86/hvm/vlapic.c | 66 ++-
xen/arch/x86/include/asm/hvm/domain.h | 3 ++
2 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
index 5cb87f8649..cd4701c5a2 100644
--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -1061,13 +1061,26 @@ static const struct hvm_mmio_ops vlapic_mmio_ops = {
.write = vlapic_mmio_write,
};
+static uint32_t x2apic_ldr_from_id(uint32_t id)
+{
+return ((id & ~0xf) << 12) | (1 << (id & 0xf));
+}
+
static void set_x2apic_id(struct vlapic *vlapic)
{
-u32 id = vlapic_vcpu(vlapic)->vcpu_id;
-u32 ldr = ((id & ~0xf) << 12) | (1 << (id & 0xf));
+uint32_t vcpu_id = vlapic_vcpu(vlapic)->vcpu_id;
+uint32_t apic_id = vcpu_id * 2;
+uint32_t apic_ldr = x2apic_ldr_from_id(apic_id);
+
+/*
+ * Workaround for migrated domains to derive LDRs as the source host
+ * would've.
+ */
+if ( vlapic_domain(vlapic)->arch.hvm.bug_x2apic_ldr_vcpu_id )
+apic_ldr = x2apic_ldr_from_id(vcpu_id);
-vlapic_set_reg(vlapic, APIC_ID, id * 2);
-vlapic_set_reg(vlapic, APIC_LDR, ldr);
+vlapic_set_reg(vlapic, APIC_ID, apic_id);
+vlapic_set_reg(vlapic, APIC_LDR, apic_ldr);
}
int guest_wrmsr_apic_base(struct vcpu *v, uint64_t val)
@@ -1498,27 +1511,36 @@ static int cf_check lapic_save_regs(struct vcpu *v,
hvm_domain_context_t *h)
*/
static void lapic_load_fixup(struct vlapic *vlapic)
{
-uint32_t id = vlapic->loaded.id;
+uint32_t good_ldr = x2apic_ldr_from_id(vlapic->loaded.id);
-if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
-{
+/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct */
+if ( !vlapic_x2apic_mode(vlapic) ||
+ (vlapic->loaded.ldr == good_ldr) )
+return;
+
+if ( vlapic->loaded.ldr == 1 )
+ /*
+* Xen <= 4.4 may have a bug by which all the APICs configured in
+* x2APIC mode got LDR = 1. We can't leave it as-is because it
+* assigned the same LDR to every CPU. We'll fix fix the bug now
+* and assign an LDR value consistent with the APIC ID.
+*/
+set_x2apic_id(vlapic);
+else if ( vlapic->loaded.ldr ==
+ x2apic_ldr_from_id(vlapic_vcpu(vlapic)->vcpu_id) )
/*
- * This is optional: ID != 0 contradicts LDR == 1. It's being added
- * to aid in eventual debugging of issues arising from the fixup done
- * here, but can be dropped as soon as it is found to conflict with
- * other (future) changes.
+ * Migrations from Xen 4.4 to date (4.19 dev window, Nov 2023) may
+ * show this bug. We must preserve LDRs so new vCPUs use consistent
+ * derivations and existing guests, which may have already read the
+ * LDR at the source host, aren't surprised when interrupts stop
+ * working the way they did at the other end.
*/
-if ( GET_xAPIC_ID(id) != vlapic_vcpu(vlapic)->vcpu_id * 2 ||
- id != SET_xAPIC_ID(GET_xAPIC_ID(id)) )
-printk(XENLOG_G_WARNING "%pv: bogus APIC ID %#x loaded\n",
- vlapic_vcpu(vlapic), id);
-set_x2apic_id(vlapic);
-}
-else /* Undo an eventual earlier fixup. */
-{
-vlapic_set_reg(vlapic, APIC_ID, id);
-vlapic_set_reg(vlapic, APIC_LDR, vlapic->loaded.ldr);
-}
+vlapic_domain(vlapic)->arch.hvm.bug_x2apic_ldr_vcpu_id =
Re: [PATCH 5/6] tools/pygrub: Expose libfsimage's fdopen() to python
On 22/11/2023 22:35, Andrew Cooper wrote:
On 06/11/2023 3:05 pm, Alejandro Vallejo wrote:
Create a wrapper for the new fdopen() function of libfsimage.
Signed-off-by: Alejandro Vallejo
I'd appreciate it if Marek would cast his eye (as python maintainer)
over it.
That said, ...
diff --git a/tools/pygrub/src/fsimage/fsimage.c
b/tools/pygrub/src/fsimage/fsimage.c
index 12dfcff6e3..216f265331 100644
--- a/tools/pygrub/src/fsimage/fsimage.c
+++ b/tools/pygrub/src/fsimage/fsimage.c
@@ -270,6 +270,30 @@ fsimage_open(PyObject *o, PyObject *args, PyObject *kwargs)
return (PyObject *)fs;
}
+static PyObject *
+fsimage_fdopen(PyObject *o, PyObject *args, PyObject *kwargs)
+{
+ static char *kwlist[] = { "fd", "offset", "options", NULL };
+ int fd;
+ char *options = NULL;
+ uint64_t offset = 0;
+ fsimage_fs_t *fs;
+
+ if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|Ls", kwlist,
+ , , ))
+ return (NULL);
+
+ if ((fs = PyObject_NEW(fsimage_fs_t, _fs_type)) == NULL)
+ return (NULL);
+
+ if ((fs->fs = fsi_fdopen_fsimage(fd, offset, options)) == NULL) {
+ PyErr_SetFromErrno(PyExc_IOError);
Don't we need a Py_DECREF(fs) here to avoid leaking it?
~Andrew
If so, there's a bug in fsimage_open() as well. The logic here identical
to the logic there.
Cheers,
Alejandro
Re: [PATCH 3/6] tools/pygrub: Restrict depriv operation with RLIMIT_AS
On 22/11/2023 20:16, Andrew Cooper wrote: On 06/11/2023 3:05 pm, Alejandro Vallejo wrote: diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub index 327cf51774..b96bdfd849 100755 --- a/tools/pygrub/src/pygrub +++ b/tools/pygrub/src/pygrub @@ -75,6 +80,11 @@ def downgrade_rlimits(): resource.setrlimit(resource.RLIMIT_CORE, (0, 0)) resource.setrlimit(resource.RLIMIT_MEMLOCK, (0, 0)) +max_ram_usage = LIMIT_AS +if "PYGRUB_MAX_RAM_USAGE_MB" in os.environ.keys(): With the .keys() dropped as per patch 2.5/6, Acked-by: Andrew Cooper Happy to do this on commit. Sure Cheers, Alejandro
Re: [PATCH v2.5/6] tools/pygrub: Fix expression before it's copied elsewhere
On 23/11/2023 16:50, Alejandro Vallejo wrote: On 22/11/2023 20:07, Andrew Cooper wrote: This has an identical meaning, and is the more pythonic way of writing it. Signed-off-by: Andrew Cooper --- CC: Wei Liu CC: Anthony PERARD CC: Alejandro Vallejo --- tools/pygrub/src/pygrub | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub index 327cf51774fc..2c06684d6532 100755 --- a/tools/pygrub/src/pygrub +++ b/tools/pygrub/src/pygrub @@ -88,7 +88,7 @@ def downgrade_rlimits(): # filesystem we set RLIMIT_FSIZE to a high bound, so that the file # write permissions are bound. fsize = LIMIT_FSIZE - if "PYGRUB_MAX_FILE_SIZE_MB" in os.environ.keys(): + if "PYGRUB_MAX_FILE_SIZE_MB" in os.environ: fsize = int(os.environ["PYGRUB_MAX_FILE_SIZE_MB"]) << 20 resource.setrlimit(resource.RLIMIT_FSIZE, (fsize, fsize)) LGTM. Cheers, Alejandro ... and Reviewed-by: Alejandro Vallejo Cheers, Alejandro
Re: [PATCH v2] livepatch: do not use .livepatch.funcs section to store internal state
On Thu, Nov 23, 2023 at 9:52 AM Roger Pau Monne wrote:
>
> Currently the livepatch logic inside of Xen will use fields of struct
> livepatch_func in order to cache internal state of patched functions. Note
> this is a field that is part of the payload, and is loaded as an ELF section
> (.livepatch.funcs), taking into account the SHF_* flags in the section
> header.
>
> The flags for the .livepatch.funcs section, as set by livepatch-build-tools,
> are SHF_ALLOC, which leads to its contents (the array of livepatch_func
> structures) being placed in read-only memory:
>
> Section Headers:
> [Nr] Name Type Address Offset
>Size EntSize Flags Link Info Align
> [...]
> [ 4] .livepatch.funcs PROGBITS 0080
>0068 A 0 0 8
>
> This previously went unnoticed, as all writes to the fields of livepatch_func
> happen in the critical region that had WP disabled in CR0. After 8676092a0f16
> however WP is no longer toggled in CR0 for patch application, and only the
> hypervisor .text mappings are made write-accessible. That leads to the
> following page fault when attempting to apply a livepatch:
>
> [ Xen-4.19-unstable x86_64 debug=y Tainted: C]
> CPU:4
> RIP:e008:[] common/livepatch.c#apply_payload+0x45/0x1e1
> [...]
> Xen call trace:
>[] R common/livepatch.c#apply_payload+0x45/0x1e1
>[] F check_for_livepatch_work+0x385/0xaa5
>[] F arch/x86/domain.c#idle_loop+0x92/0xee
>
> Pagetable walk from 82d040625079:
> L4[0x105] = 8c6c9063
> L3[0x141] = 8c6c6063
> L2[0x003] = 00086a1e7063
> L1[0x025] = 80086ca5d121
>
>
> Panic on CPU 4:
> FATAL PAGE FAULT
> [error_code=0003]
> Faulting linear address: 82d040625079
>
>
> Fix this by moving the internal Xen function patching state out of
> livepatch_func into an area not allocated as part of the ELF payload. While
> there also constify the array of livepatch_func structures in order to prevent
> further surprises.
>
> Note there's still one field (old_addr) that gets set during livepatch load.
> I
> consider this fine since the field is read-only after load, and at the point
> the field gets set the underlying mapping hasn't been made read-only yet.
>
> Fixes: 8676092a0f16 ('x86/livepatch: Fix livepatch application when CET is
> active')
> Signed-off-by: Roger Pau Monné
Reviewed-by: Ross Lagerwall
Thanks,
Ross
Re: [PATCH 4/6] tools/libfsimage: Add an fdopen() interface to libfsimage
On 22/11/2023 22:29, Andrew Cooper wrote:
On 06/11/2023 3:05 pm, Alejandro Vallejo wrote:
diff --git a/tools/libfsimage/common/fsimage_priv.h
b/tools/libfsimage/common/fsimage_priv.h
index 2274403557..779e433b37 100644
--- a/tools/libfsimage/common/fsimage_priv.h
+++ b/tools/libfsimage/common/fsimage_priv.h
@@ -29,6 +29,7 @@ extern C {
#endif
#include
+#include
#include "xenfsimage.h"
#include "xenfsimage_plugin.h"
@@ -54,7 +55,7 @@ struct fsi_file {
void *ff_data;
};
-int find_plugin(fsi_t *, const char *, const char *);
+int find_plugin(fsi_t *, const char *);
#ifdef __cplusplus
};
These are the only two hunks in this file. Is the stdbool include stale?
It seems to compile fine with it removed.
I think it's leftover from code I needed initially and then didn't
anymore. Safe to get rid of.
diff --git a/tools/libfsimage/ext2fs-lib/ext2fs-lib.c
b/tools/libfsimage/ext2fs-lib/ext2fs-lib.c
index 864a15b349..9f07ea288f 100644
--- a/tools/libfsimage/ext2fs-lib/ext2fs-lib.c
+++ b/tools/libfsimage/ext2fs-lib/ext2fs-lib.c
@@ -25,15 +25,25 @@
#include INCLUDE_EXTFS_H
#include
#include
+#include
static int
-ext2lib_mount(fsi_t *fsi, const char *name, const char *options)
+ext2lib_mount(fsi_t *fsi, const char *options)
{
int err;
char opts[30] = "";
ext2_filsys *fs;
uint64_t offset = fsip_fs_offset(fsi);
+ /*
+* We must choose unixfd_io_manager rather than unix_io_manager in
+* order for the library to accept fd strings instead of paths. It
+* still means we must pass a string representing an fd rather than
+* an int, but at least this way we don't need to pass paths around
+*/
+ char name[32] = {0};
For an int? 12 will do fine including a terminator, and practical
system limits leave it far smaller than that generally.
Maybe. I admit optimising that line went pretty low on the list of
things I cared terribly about. I'm happy to reduce it, but it is
inconsequential.
Given that it is guaranteed long enough, you don't need to zero it just
to have snprintf() write a well-formed string in.
~Andrew
For wellformed fd's that's true. Through bugs or malice that may not be
the case. I'm happier knowing at least the last zero remains.
Cheers,
Alejandro
Re: [PATCH v2.5/6] tools/pygrub: Fix expression before it's copied elsewhere
On 23/11/2023 4:50 pm, Alejandro Vallejo wrote: > On 22/11/2023 20:07, Andrew Cooper wrote: >> This has an identical meaning, and is the more pythonic way of >> writing it. >> >> Signed-off-by: Andrew Cooper >> --- >> CC: Wei Liu >> CC: Anthony PERARD >> CC: Alejandro Vallejo >> --- >> tools/pygrub/src/pygrub | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub >> index 327cf51774fc..2c06684d6532 100755 >> --- a/tools/pygrub/src/pygrub >> +++ b/tools/pygrub/src/pygrub >> @@ -88,7 +88,7 @@ def downgrade_rlimits(): >> # filesystem we set RLIMIT_FSIZE to a high bound, so that the file >> # write permissions are bound. >> fsize = LIMIT_FSIZE >> - if "PYGRUB_MAX_FILE_SIZE_MB" in os.environ.keys(): >> + if "PYGRUB_MAX_FILE_SIZE_MB" in os.environ: >> fsize = int(os.environ["PYGRUB_MAX_FILE_SIZE_MB"]) << 20 >> resource.setrlimit(resource.RLIMIT_FSIZE, (fsize, fsize)) > > LGTM. Can I take that as a R-by then? ~Andrew
Re: [PATCH 2/3] livepatch: add a dummy hypercall for testing purposes
On 23/11/2023 11:23 am, Roger Pau Monne wrote: > Introduce a dummy XEN_SYSCTL_LIVEPATCH_TEST hypercall to be used in order to > test livepatch functionality. The hypercall fills a value in the passed > structure, which is returned to the caller. > > The xen-livepatch utility is expanded to allow calling that hypercall, and > printing the returned value on stdout. > > Finally, add dummy patch that changes the returned value of the hypercall from > 1 to 2. Such patch can be used with livepatch-build-tools in order to > generate > a livepatch payload, that when applied to the hypervisor change the printed > value of `xen-livepatch test`. > > Signed-off-by: Roger Pau Monné > --- > The whole logic is very simple now. I think it's enough to have a skeleton we > can later expand. > > Unsure whether we should do some kind of test (with `patch -F0`) that the > patch > still applies cleanly as part of Xen build. Thanks for looking into this. I think one tweak towards the larger plan might help here. When I envisaged this originally, it was something along the lines of test_self_modify_code() which would be called on boot after alternatives were called, which would sanity check the result of certain simple cases. Then, for livepatching, I was thinking of something like this: obj-y += test_smc.o targets-y += test_smc_alt.o i.e. we have test_smc.c and test_smc_alt.c which are two slightly different copies of the same thing, and we compile both but don't link the second one in. Then, we can diff the two C files in order to make the patch to build as a livepatch. This way we're not maintaining a patch committed into the tree, which I suspect will make everyone's lives easier. I suspect in practice that we'll want test_smc_asm.S pairs too. Not necessarily for now, but I was also thinking we'd have a test stage where we know exactly what the livepatch ought to look like, so we audit the eventual livepatch elf that it has all the expected differences encoded, and no extraneous ones. Finally, I was thinking that the hypercall would (re)run test_self_modify_code(). I'm on the fence about making it part of the livepatch infrastructure, given that the nature of the test is wider, but I can't think of any case that we'd be wanting runtime self modifying code (e.g. rerun alternatives after ucode load) which isn't linked to a new livepatch to begin with. Thoughts? ~Andrew
Re: [PATCH v2.5/6] tools/pygrub: Fix expression before it's copied elsewhere
On 22/11/2023 20:07, Andrew Cooper wrote: This has an identical meaning, and is the more pythonic way of writing it. Signed-off-by: Andrew Cooper --- CC: Wei Liu CC: Anthony PERARD CC: Alejandro Vallejo --- tools/pygrub/src/pygrub | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub index 327cf51774fc..2c06684d6532 100755 --- a/tools/pygrub/src/pygrub +++ b/tools/pygrub/src/pygrub @@ -88,7 +88,7 @@ def downgrade_rlimits(): # filesystem we set RLIMIT_FSIZE to a high bound, so that the file # write permissions are bound. fsize = LIMIT_FSIZE -if "PYGRUB_MAX_FILE_SIZE_MB" in os.environ.keys(): +if "PYGRUB_MAX_FILE_SIZE_MB" in os.environ: fsize = int(os.environ["PYGRUB_MAX_FILE_SIZE_MB"]) << 20 resource.setrlimit(resource.RLIMIT_FSIZE, (fsize, fsize)) LGTM. Cheers, Alejandro
Re: [PATCH 1/6] tools/pygrub: Set mount propagation to private recursively
On 22/11/2023 19:48, Andrew Cooper wrote:
On 22/11/2023 7:46 pm, Andrew Cooper wrote:
On 06/11/2023 3:05 pm, Alejandro Vallejo wrote:
This is important in order for every mount done inside a mount namespace to
go away after the namespace itself goes away. The comment referring to
unreliability in Linux 4.19 was just wrong.
This patch sets the story straight and makes the depriv pygrub a bit more
confined should a layer of the onion be vulnerable.
Signed-off-by: Alejandro Vallejo
Acked-by: Andrew Cooper
Sorry, wants
Fixes: e0342ae5556f ("tools/pygrub: Deprivilege pygrub")
too. Will fix on commit.
~Andrew
Sounds good.
Cheers,
Alejandro
Re: [PATCH 6/6] tools/pygrub: Hook libfsimage's fdopen() to pygrub
On 06/11/2023 3:05 pm, Alejandro Vallejo wrote:
> This patch increases the security posture by removing the need to grant
> filesystem access to the depriv pygrub. Using libfsimage's fdopen(), the
> parent thread in the depriv procedure can simply ensure all the appropriate
> file descriptors are present before revoking permissions to the filesystem.
>
> A convenient usability side-effect is that it's no longer required for the
> restricted user to have access to the disk, because the depriv thread
> already has the file descriptor open by its parent.
>
> Signed-off-by: Alejandro Vallejo
> ---
> docs/man/xl.cfg.5.pod.in | 6 +-
> tools/pygrub/src/ExtLinuxConf.py | 20 ---
> tools/pygrub/src/GrubConf.py | 29 ++
> tools/pygrub/src/LiloConf.py | 20 ---
> tools/pygrub/src/pygrub | 95
> 5 files changed, 68 insertions(+), 102 deletions(-)
>
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index 2e234b450e..e3fd2e37eb 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1704,8 +1704,7 @@ See docs/features/qemu-deprivilege.pandoc for more
> information
> on how to setup the unprivileged users.
>
> Note that running the bootloader in restricted mode also implies using
> -non-interactive mode, and the disk image must be readable by the
> -restricted user.
> +non-interactive mode.
>
> =item B
>
> @@ -2768,8 +2767,7 @@ See docs/features/qemu-deprivilege.pandoc for more
> information
> on how to setup the unprivileged users.
>
> Note that running the bootloader in restricted mode also implies using
> -non-interactive mode, and the disk image must be readable by the
> -restricted user.
> +non-interactive mode.
>
> =item B
>
I'm leaning towards suggesting that this wants a note in the changelog,
as we're removing a limitation imposed by a security fix.
> diff --git a/tools/pygrub/src/ExtLinuxConf.py
> b/tools/pygrub/src/ExtLinuxConf.py
> index 4e990a9304..f2e9a81013 100644
> --- a/tools/pygrub/src/ExtLinuxConf.py
> +++ b/tools/pygrub/src/ExtLinuxConf.py
> @@ -123,6 +123,7 @@ class ExtLinuxImage(object):
> class ExtLinuxConfigFile(object):
> def __init__(self, fn = None):
> self.filename = fn
> +self.file = None
> self.images = []
> self.timeout = -1
> self._default = 0
> @@ -138,16 +139,21 @@ class ExtLinuxConfigFile(object):
>
> def parse(self, buf = None):
> if buf is None:
> -if self.filename is None:
> +if not self.filename and not self.file:
> raise ValueError("No config file defined to parse!")
>
> -f = open(self.filename, 'r')
> -lines = f.readlines()
> -f.close()
> -else:
> -lines = buf.split("\n")
> +if self.file:
> +buf = file.read()
> +path = self.file.name
> +else:
> +f = open(self.filename, 'r')
> +buf = f.read()
> +f.close()
> +lines = buf.split("\n")
> +
> +if not path:
> +path = os.path.dirname(self.filename)
>
> -path = os.path.dirname(self.filename)
> img = []
> for l in lines:
> l = l.strip()
[List context, Alejandro and I discussed this IRL]
This pattern is horrible and repeated in each object. I'm going to
experiment and see if there's a (limited) bit of cleanup which can be
done to reduce the invasiveness, and therefore the legibility of this patch.
~Andrew
xen | Successful pipeline for staging-4.17 | e1f9cb16
Pipeline #1082754385 has passed! Project: xen ( https://gitlab.com/xen-project/xen ) Branch: staging-4.17 ( https://gitlab.com/xen-project/xen/-/commits/staging-4.17 ) Commit: e1f9cb16 ( https://gitlab.com/xen-project/xen/-/commit/e1f9cb16e2efbb202f2f8a9aa7c5ff1d392ece33 ) Commit Message: xen/sched: fix sched_move_domain() When moving... Commit Author: Juergen Gross ( https://gitlab.com/jgross1 ) Committed by: Jan Beulich ( https://gitlab.com/jbeulich ) Pipeline #1082754385 ( https://gitlab.com/xen-project/xen/-/pipelines/1082754385 ) triggered by Ganis ( https://gitlab.com/ganis ) successfully completed 84 jobs in 2 stages. -- You're receiving this email because of your account on gitlab.com.
[PATCH] tools/pygrub: Drop compatibility symlink
This was declared deprecated in commit 10c88f1c18b7 ("tools: Install pv
bootloaders in libexec rather than /usr/bin") in 2012
Take it out fully now, 11 years later.
Signed-off-by: Andrew Cooper
---
CC: George Dunlap
CC: Jan Beulich
CC: Stefano Stabellini
CC: Wei Liu
CC: Julien Grall
---
CHANGELOG.md | 3 +++
tools/pygrub/Makefile | 6 --
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4ecebb9f686a..36a8ef89d8e4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,9 @@ The format is based on [Keep a
Changelog](https://keepachangelog.com/en/1.0.0/)
### Removed
- caml-stubdom. It hasn't built since 2014, was pinned to Ocaml 4.02, and has
been superseded by the MirageOS/SOLO5 projects.
+- /usr/bin/pygrub symlink. This was deprecated in Xen 4.2 (2012) but left for
+ compatibility reasons. VMs configured with bootloader="/usr/bin/pygrub"
+ should be updated to just bootloader="pygrub".
##
[4.18.0](https://xenbits.xenproject.org/gitweb/?p=xen.git;a=shortlog;h=RELEASE-4.18.0)
- 2023-11-16
diff --git a/tools/pygrub/Makefile b/tools/pygrub/Makefile
index 4963bc89c6ed..d5e291ea0619 100644
--- a/tools/pygrub/Makefile
+++ b/tools/pygrub/Makefile
@@ -22,15 +22,9 @@ install: all
$(setup.py) install --record $(INSTALL_LOG) $(PYTHON_PREFIX_ARG) \
--root="$(DESTDIR)" --force
$(INSTALL_PYTHON_PROG) src/pygrub $(DESTDIR)/$(LIBEXEC_BIN)/pygrub
- set -e; if [ $(bindir) != $(LIBEXEC_BIN) -a \
-"`readlink -f $(DESTDIR)/$(bindir)`" != \
-"`readlink -f $(LIBEXEC_BIN)`" ]; then \
- ln -sf $(LIBEXEC_BIN)/pygrub $(DESTDIR)/$(bindir); \
- fi
.PHONY: uninstall
uninstall:
- rm -f $(DESTDIR)/$(bindir)/pygrub
if [ -e $(INSTALL_LOG) ]; then \
cat $(INSTALL_LOG) | xargs -i rm -f $(DESTDIR)/{}; \
fi
base-commit: f96e2f64576cdbb147391c7cb399d393385719a9
--
2.30.2
xen | Successful pipeline for staging | f96e2f64
Pipeline #1082747345 has passed! Project: xen ( https://gitlab.com/xen-project/xen ) Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging ) Commit: f96e2f64 ( https://gitlab.com/xen-project/xen/-/commit/f96e2f64576cdbb147391c7cb399d393385719a9 ) Commit Message: xen/MISRA: Remove nonstandard inline keywords ... Commit Author: Andrew Cooper ( https://gitlab.com/andyhhp ) Pipeline #1082747345 ( https://gitlab.com/xen-project/xen/-/pipelines/1082747345 ) triggered by Ganis ( https://gitlab.com/ganis ) successfully completed 129 jobs in 3 stages. -- You're receiving this email because of your account on gitlab.com.
[PATCH 2/2] livepatch-build-tools: do not use readlink -m option
Busybox readlink implementation only supports the -f option to follow symlinks,
so adjust the logic in order to keep the same behaviour without using the -m
option.
Singed-off-by: Roger Pau Monné
---
livepatch-build | 29 -
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/livepatch-build b/livepatch-build
index 26c88a2ed2c3..2f35e98d30a3 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -336,8 +336,8 @@ while [[ $# -gt 0 ]]; do
;;
--xen-syms)
shift
-XENSYMS="$(readlink -m -- "$1")"
-[ -f "$XENSYMS" ] || die "xen-syms file does not exist"
+[ -f "$1" ] || die "xen-syms file does not exist"
+XENSYMS="$(readlink -f -- "$1")"
shift
;;
--depends)
@@ -366,22 +366,20 @@ while [[ $# -gt 0 ]]; do
done
[ -z "$srcarg" ] && die "Xen directory not given"
+[ -d "$srcarg" ] || die "Xen directory does not exist"
[ -z "$patcharg" ] && die "Patchfile not given"
+[ -f "$patcharg" ] || die "Patchfile does not exist"
[ -z "$configarg" ] && die ".config not given"
+[ -f "$configarg" ] || die ".config does not exist"
[ -z "$outputarg" ] && die "Output directory not given"
[ -z "$DEPENDS" ] && die "Build-id dependency not given"
[ -z "$XEN_DEPENDS" ] && die "Xen Build-id dependency not given"
-SRCDIR="$(readlink -m -- "$srcarg")"
+SRCDIR="$(readlink -f -- "$srcarg")"
# We need an absolute path because we move around, but we need to
# retain the name of the symlink (= realpath -s)
PATCHFILE="$(readlink -f "$(dirname "$patcharg")")/$(basename "$patcharg")"
-CONFIGFILE="$(readlink -m -- "$configarg")"
-OUTPUT="$(readlink -m -- "$outputarg")"
-
-[ -d "${SRCDIR}" ] || die "Xen directory does not exist"
-[ -f "${PATCHFILE}" ] || die "Patchfile does not exist"
-[ -f "${CONFIGFILE}" ] || die ".config does not exist"
+CONFIGFILE="$(readlink -f -- "$configarg")"
PATCHNAME=$(make_patch_name "${PATCHFILE}")
@@ -390,17 +388,20 @@ echo
echo "Xen directory: ${SRCDIR}"
echo "Patch file: ${PATCHFILE}"
echo ".config file: ${CONFIGFILE}"
-echo "Output directory: ${OUTPUT}"
+echo "Output directory: $outputarg"
echo ""
echo
if [ "${SKIP}" != "build" ]; then
-[ -e "${OUTPUT}" ] && die "Output directory exists"
+# Make sure output directory doesn't exist, and create it.
+[ -e "$outputarg" ] && die "Output directory exists"
+mkdir -p "$outputarg" || die
+OUTPUT="$(readlink -f -- "$outputarg")"
+
grep -q 'CONFIG_LIVEPATCH=y' "${CONFIGFILE}" || die "CONFIG_LIVEPATCH must
be enabled"
cd "$SRCDIR" || die
patch -s -N -p1 -f --fuzz=0 --dry-run < "$PATCHFILE" || die "Source patch
file failed to apply"
-mkdir -p "${OUTPUT}" || die
cp -f "${CONFIGFILE}" "${OUTPUT}/.config"
cp -f "${OUTPUT}/.config" "xen/.config"
@@ -453,7 +454,9 @@ if [ "${SKIP}" != "build" ]; then
fi
if [ "${SKIP}" != "diff" ]; then
-[ -d "${OUTPUT}" ] || die "Output directory does not exist"
+cd "${SCRIPTDIR}" || die
+[ -d "$outputarg" ] || die "Output directory does not exist"
+OUTPUT="$(readlink -f -- "$outputarg")"
cd "${OUTPUT}" || die
create_patch
--
2.43.0
[PATCH 1/2] livepatch-build-tools: remove usage of gawk
And instead use plain awk.
There's no need to use the --non-decimal-data option for gawk, since the
numbers that we want to print are already prefixed with '0x', and so plain awk
will do the conversion from hexadecimal to decimal just fine.
Signed-off-by: Roger Pau Monné
---
livepatch-build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/livepatch-build b/livepatch-build
index 91d203bda0eb..26c88a2ed2c3 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -422,7 +422,7 @@ if [ "${SKIP}" != "build" ]; then
echo "Reading special section data"
# Using xen-syms built in the previous step by build_full().
SPECIAL_VARS=$(readelf -wi "$OUTPUT/xen-syms" |
- gawk --non-decimal-data '
+ awk '
BEGIN { a = b = e = 0 }
a == 0 && /DW_AT_name.* alt_instr/ {a = 1; next}
b == 0 && /DW_AT_name.* bug_frame/ {b = 1; next}
base-commit: e588b7914e7afa3abb64b15a32fc2fdb57ded341
--
2.43.0
[PATCH 0/2] livepatch-build-tools: fixes for non GNU tools
Hello, A couple more fixes to allow the usage of livepatch-build-tools with non GNU utilities. Shouldn't introduce any functional change. Thanks, Roger. Roger Pau Monne (2): livepatch-build-tools: remove usage of gawk livepatch-build-tools: do not use readlink -m option livepatch-build | 31 +-- 1 file changed, 17 insertions(+), 14 deletions(-) base-commit: e588b7914e7afa3abb64b15a32fc2fdb57ded341 -- 2.43.0
xen | Successful pipeline for staging-4.18 | 3f9390fe
Pipeline #1082740589 has passed! Project: xen ( https://gitlab.com/xen-project/xen ) Branch: staging-4.18 ( https://gitlab.com/xen-project/xen/-/commits/staging-4.18 ) Commit: 3f9390fe ( https://gitlab.com/xen-project/xen/-/commit/3f9390fea5c51a6d64596d295902d28931eeca4c ) Commit Message: xen/sched: fix sched_move_domain() When moving... Commit Author: Juergen Gross ( https://gitlab.com/jgross1 ) Committed by: Jan Beulich ( https://gitlab.com/jbeulich ) Pipeline #1082740589 ( https://gitlab.com/xen-project/xen/-/pipelines/1082740589 ) triggered by Ganis ( https://gitlab.com/ganis ) successfully completed 129 jobs in 3 stages. -- You're receiving this email because of your account on gitlab.com.
Re: [Discussion]: Making "LIBXL_HOTPLUG_TIMEOUT" configurable through 'xl.conf'
On 11/16/23 9:53 AM, Olaf Hering wrote: Thu, 16 Nov 2023 09:21:06 + Luca Fancellu : I see your patch is handling this issue but maybe was not meant to be upstreamed, so I would like to ask if you are willing to make it upstream-able or if it’s not planned to do so. Right now I do not have the time to work on this. First there need to be an agreement about how the timeout needs to be configured. Olaf Hello everyone, Following our earlier discussion, we learned that there is already a solution to fix the LIBXL hotplug timeout issue. Currently, we have two options: 1. Xenstore Key/Value Approach: This is implemented in the patch "libxl.LIBXL_HOTPLUG_TIMEOUT.patch". This method does not require recompiling libxl. It involves setting up a key/value in Xenstore before domain creation. The patch can be viewed here: [https://build.opensuse.org/package/view_file/openSUSE:Factory/xen/libxl.LIBXL_HOTPLUG_TIMEOUT.patch?expand=1]. 2. Configuration via 'xl.conf': We can make "LIBXL_HOTPLUG_TIMEOUT" configurable through the 'xl.conf' file. I propose we have a discussion to determine the most suitable approach. Once we reach the best solution, we can move forward with the implementation/upstream. Your feedback, suggestions, or any improvements would be greatly appreciated. Thank you for your time and consideration. Regards, Divin
[XEN PATCH v2] automation/eclair: improve scheduled analyses
The scheduled analyses are intended to maintain an overall vision
of the MISRA complaince of the entire project. For this reason,
the file exclusions in "out_of_scope.ecl" should not be applied.
This patch amends ECLAIR settings to prevent exempting files for
scheduled analyses.
Signed-off-by: Simone Ballarin
Reviewed-by: Stefano Stabellini
---
Changes in v2:
- drop changes to inhibit test and build stages in scheduled pipelines.
---
automation/eclair_analysis/ECLAIR/action.settings | 2 +-
automation/eclair_analysis/ECLAIR/analysis.ecl| 12 ++--
automation/gitlab-ci/analyze.yaml | 2 ++
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/automation/eclair_analysis/ECLAIR/action.settings
b/automation/eclair_analysis/ECLAIR/action.settings
index f96368ffc7..3cba1a3afb 100644
--- a/automation/eclair_analysis/ECLAIR/action.settings
+++ b/automation/eclair_analysis/ECLAIR/action.settings
@@ -134,7 +134,7 @@ push)
badgeLabel="ECLAIR ${ANALYSIS_KIND} ${ref}${variantHeadline} #${jobId}"
;;
auto_pull_request)
-git remote remove autoPRRemote || true
+git remote remove autoPRRemote 2>/dev/null || true
git remote add autoPRRemote "${autoPRRemoteUrl}"
git fetch -q autoPRRemote
subDir="${ref}"
diff --git a/automation/eclair_analysis/ECLAIR/analysis.ecl
b/automation/eclair_analysis/ECLAIR/analysis.ecl
index fe418d6da1..2507a8e787 100644
--- a/automation/eclair_analysis/ECLAIR/analysis.ecl
+++ b/automation/eclair_analysis/ECLAIR/analysis.ecl
@@ -2,7 +2,13 @@
-project_name=getenv("ECLAIR_PROJECT_NAME")
-project_root=getenv("ECLAIR_PROJECT_ROOT")
--setq=data_dir,getenv("ECLAIR_DATA_DIR")
+setq(data_dir,getenv("ECLAIR_DATA_DIR"))
+setq(analysis_kind,getenv("ANALYSIS_KIND"))
+setq(scheduled_analysis,nil)
+
+strings_map("scheduled-analysis",500,"","^.*scheduled$",0,setq(scheduled_analysis,t))
+strings_map("scheduled-analysis",500,"","^.*$",0)
+map_strings("scheduled-analysis",analysis_kind)
-verbose
@@ -15,7 +21,9 @@
-eval_file=toolchain.ecl
-eval_file=public_APIs.ecl
--eval_file=out_of_scope.ecl
+if(scheduled_analysis,
+eval_file("out_of_scope.ecl")
+)
-eval_file=deviations.ecl
-eval_file=call_properties.ecl
-eval_file=tagging.ecl
diff --git a/automation/gitlab-ci/analyze.yaml
b/automation/gitlab-ci/analyze.yaml
index bd9a68de31..6631db53fa 100644
--- a/automation/gitlab-ci/analyze.yaml
+++ b/automation/gitlab-ci/analyze.yaml
@@ -28,6 +28,8 @@
extends: .eclair-analysis
allow_failure: true
rules:
+- if: $CI_PIPELINE_SOURCE == "schedule"
+ when: never
- if: $WTOKEN && $CI_PROJECT_PATH =~ /^xen-project\/people\/.*$/
when: manual
- !reference [.eclair-analysis, rules]
--
2.34.1
Re: [linux-linus test] 183794: regressions - FAIL
Hi Juergen, On 23/11/2023 05:57, Juergen Gross wrote: On 23.11.23 00:07, Stefano Stabellini wrote: On Wed, 22 Nov 2023, Juergen Gross wrote: On 22.11.23 04:07, Stefano Stabellini wrote: On Mon, 20 Nov 2023, Stefano Stabellini wrote: On Mon, 20 Nov 2023, Juergen Gross wrote: On 20.11.23 03:21, osstest service owner wrote: flight 183794 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/183794/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-examine 8 reboot fail REGR. vs. 183766 I'm seeing the following in the serial log: Nov 20 00:25:41.586712 [ 0.567318] kernel BUG at arch/arm64/xen/../../arm/xen/enlighten.c:164! Nov 20 00:25:41.598711 [ 0.574002] Internal error: Oops - BUG: f2000800 [#1] PREEMPT SMP The related source code lines in the kernel are: err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, xen_vcpu_nr(cpu), ); BUG_ON(err); I suspect commit 20f3b8eafe0ba to be the culprit. Stefano, could you please have a look? The good news and bad news is that I cannot repro this neither with nor without CONFIG_UNMAP_KERNEL_AT_EL0. I looked at commit 20f3b8eafe0ba but I cannot see anything wrong with it. Looking at the register dump, from: x0 : fffa I am guessing the error was -ENXIO which is returned from map_guest_area in Xen. Could it be that the struct is crossing a page boundary? Or that it is not 64-bit aligned? Do we need to do something like the following? diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 9afdc4c4a5dc..5326070c5dc0 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -484,7 +485,7 @@ static int __init xen_guest_init(void) * for secondary CPUs as they are brought up. * For uniformity we use VCPUOP_register_vcpu_info even on cpu0. */ - xen_vcpu_info = alloc_percpu(struct vcpu_info); + xen_vcpu_info = __alloc_percpu(struct vcpu_info, PAGE_SIZE); if (xen_vcpu_info == NULL) return -ENOMEM; May I suggest to use a smaller alignment? What about: 1 << fls(sizeof(struct vcpu_info) - 1) See below --- [PATCH] arm/xen: fix xen_vcpu_info allocation alignment Stefano, are you going to submit the patch formally? xen_vcpu_info is a percpu area than needs to be mapped by Xen. Currently, it could cross a page boundary resulting in Xen being unable to map it: [ 0.567318] kernel BUG at arch/arm64/xen/../../arm/xen/enlighten.c:164! [ 0.574002] Internal error: Oops - BUG: f2000800 [#1] PREEMPT SMP Fix the issue by using __alloc_percpu and requesting alignment for the memory allocation. Signed-off-by: Stefano Stabellini I am guessing we want to backport it. So should this contain a tag to indicate the intention? diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index 9afdc4c4a5dc..09eb74a07dfc 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -484,7 +484,8 @@ static int __init xen_guest_init(void) * for secondary CPUs as they are brought up. * For uniformity we use VCPUOP_register_vcpu_info even on cpu0. */ - xen_vcpu_info = alloc_percpu(struct vcpu_info); + xen_vcpu_info = __alloc_percpu(sizeof(struct vcpu_info), + 1 << fls(sizeof(struct vcpu_info) - 1)); Nit: one tab less, please (can be fixed while committing). if (xen_vcpu_info == NULL) return -ENOMEM; Reviewed-by: Juergen Gross Juergen, looking at the x86 code, you seem to use DEFINE_PER_CPU(). So what guarantees that this is not going to cross a page? Cheers, -- Julien Grall
[xen-unstable-smoke test] 183840: tolerable all pass - PUSHED
flight 183840 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/183840/ 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 8f45862580c962791e66df716389bb5b1dd704df baseline version: xen c22fe7213c9b1f99cbc64c33e391afa223f9cd08 Last test of basis 183826 2023-11-22 14:00:36 Z1 days Testing same since 183840 2023-11-23 10:00:25 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Federico Serafini Frediano Ziglio Jan Beulich Nicola Vetrini 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 c22fe7213c..8f45862580 8f45862580c962791e66df716389bb5b1dd704df -> smoke
Clang-format configuration discussion - pt 2
Hi all, Let’s continue the discussion about clang-format configuration, this is part 2, previous discussions are: - https://lists.xenproject.org/archives/html/xen-devel/2023-11/msg00498.html You can find the serie introducing clang-format here: https://patchwork.kernel.org/project/xen-devel/cover/20231031132304.2573924-1-luca.fance...@arm.com/ and there is also a patch linked to my gitlab account where you can find the output for the hypervisor code. For a full list of configurables and to find the possible values for them, please refer to this page: https://clang.llvm.org/docs/ClangFormatStyleOptions.html -- Our coding style doesn’t mention anything about alignment, shall we add a new section? I can send patches when we reach agreement on each of these rules. QualifierAlignment: Custom QualifierOrder: ['static', 'inline', 'const', 'volatile', 'type'] --- For “QualifierAlignment” I chose Custom in order to apply in “QualifierOrder” an order for the qualifiers that match the current codebase, we could specify also “Leave” in order to keep them as they are. Depending on how the discussion goes on this one, it could be an entry in our coding style -- AlignAfterOpenBracket: Align --- This one is to align function parameters that overflows the line length, I chose to align them to the open bracket to match the current codebase (hopefully) e.g.: someLongFunction(argument1, argument2); This one can be a candidate for an entry in our coding style -- AlignArrayOfStructures: Left --- “When using initialization for an array of structs aligns the fields into columns." It’s important to say that even if we specify “None”, it is going to format the data structure anyway, I choose left, but clearly I’m open to suggestions. I don’t know how to phrase this one in our coding style -- AlignConsecutiveAssignments: None --- This one is disabled because of feedbacks from Stefano and Alejandro about some weird behaviour on our codebase. This one could be phased along this line: “Consecutive assignments don't need to be aligned.”, the issue is that in this way it seems that it’s optional, but clang-format is going to remove the alignment anyway for assignment that are consecutive and aligned. -- AlignConsecutiveBitFields: None --- Same thing as AlignConsecutiveAssignments, but for bitfields. -- AlignConsecutiveDeclarations: None --- This aligns declarations names, same considerations as AlignConsecutiveAssignments. -- Ok this is it for now, let me know your thoughts about them, ideally if I don’t get any reply in two weeks (7th of December), I will consider that we have an agreement on these configuration. Cheers, Luca
Re: [PATCH v7 1/2] xen/vpci: header: status register handler
On Thu, Nov 23, 2023 at 07:57:21AM -0500, Stewart Hildebrand wrote: > On 11/23/23 03:14, Roger Pau Monné wrote: > > On Wed, Nov 22, 2023 at 03:16:29PM -0500, Stewart Hildebrand wrote: > >> On 11/17/23 07:40, Roger Pau Monné wrote: > >>> On Wed, Sep 13, 2023 at 10:35:46AM -0400, Stewart Hildebrand wrote: > r->write(pdev, r->offset, data & (0xU >> (32 - 8 * > r->size)), > r->private); > } > diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h > index 84b18736a85d..b72131729db6 100644 > --- a/xen/include/xen/pci_regs.h > +++ b/xen/include/xen/pci_regs.h > @@ -66,6 +66,15 @@ > #define PCI_STATUS_REC_MASTER_ABORT0x2000 /* Set on master abort */ > #define PCI_STATUS_SIG_SYSTEM_ERROR0x4000 /* Set when we drive > SERR */ > #define PCI_STATUS_DETECTED_PARITY 0x8000 /* Set on parity error */ > +#define PCI_STATUS_RSVDZ_MASK 0x0006 > >>> > >>> In my copy of the PCIe 6 spec bit 6 is also RsvdZ, so the mask should > >>> be 0x46. > >> > >> Right, mine too. It's probably safer to follow the newer version of the > >> spec, so I'll make the change. For completeness / archaeology purposes, I > >> just want to document some relevant data points here. > >> > >> In PCIe 4 spec, it says this about bit 6: > >> "These bits were used in previous versions of the programming model. > >> Careful consideration should be given to any attempt to repurpose them." > >> > >> Going further back, PCI (old school PCI, not Express) spec 3.0 says this > >> about bit 6: > >> "This bit is reserved. *" > >> "* In Revision 2.1 of this specification, this bit was used to indicate > >> whether or not a device supported User Definable Features." > >> > >> Just above in our pci_regs.h (and equally in Linux > >> include/uapi/linux/pci_regs.h) we have this definition for bit 6: > >> #define PCI_STATUS_UDF 0x40/* Support User Definable Features > >> [obsolete] */ > >> > >> Qemu hw/xen/xen_pt_config_init.c treats bit 6 as RO: > >> .ro_mask= 0x06F8, > > > > Right, given the implementation of ro_mask that would likely be fine. > > Reading unconditionally as 0 while preserving the value on writes > > seems the safest option. > > That would mean treating bit 6 as RsvdP, even though the PCIe 6 spec > says RsvdZ. I just want to confirm this is indeed the intent since > we both said RsvdZ just a moment ago? If so, I would add a comment > since it's deviating from spec. I would personally still vote in > favor of following PCIe 6 spec (RsvdZ). Hm, yes, lets go with the spec and use RsdvZ. I very much doubt we passthrough any device that actually uses the UDF bit. Thanks, Roger.
Re: [PATCH v3] xen/x86: On x2APIC mode, derive LDR from APIC ID
On 23/11/2023 2:03 pm, Roger Pau Monné wrote:
> On Thu, Nov 23, 2023 at 12:21:36PM +, Alejandro Vallejo wrote:
>> On Thu, Nov 23, 2023 at 10:03:12AM +0100, Roger Pau Monné wrote:
>>> On Wed, Nov 22, 2023 at 04:08:17PM +, Alejandro Vallejo wrote:
+if ( vlapic_domain(vlapic)->arch.hvm.x2apic_ldr_bug_with_vcpu_id )
+apic_ldr = x2apic_ldr_from_id(vcpu_id);
-vlapic_set_reg(vlapic, APIC_ID, id * 2);
-vlapic_set_reg(vlapic, APIC_LDR, ldr);
+vlapic_set_reg(vlapic, APIC_ID, apic_id);
+vlapic_set_reg(vlapic, APIC_LDR, apic_ldr);
}
int guest_wrmsr_apic_base(struct vcpu *v, uint64_t val)
@@ -1498,27 +1508,35 @@ static int cf_check lapic_save_regs(struct vcpu
*v, hvm_domain_context_t *h)
*/
static void lapic_load_fixup(struct vlapic *vlapic)
{
-uint32_t id = vlapic->loaded.id;
+/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct
*/
+if ( !vlapic_x2apic_mode(vlapic) ||
+ (vlapic->loaded.ldr == x2apic_ldr_from_id(vlapic->loaded.id)) )
+return;
-if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
-{
+if ( vlapic->loaded.ldr == 1 )
+ /*
+* Xen <= 4.4 had a bug by which all the APICs configured in x2APIC
+* mode got LDR = 1. We can't leave it as-is because it assigned
the
+* same LDR to every CPU. We'll fix fix the bug now and assign an
+* LDR value consistent with the APIC ID.
+*/
+set_x2apic_id(vlapic);
+else if ( vlapic->loaded.ldr ==
+ x2apic_ldr_from_id(vlapic_vcpu(vlapic)->vcpu_id) )
/*
- * This is optional: ID != 0 contradicts LDR == 1. It's being
added
- * to aid in eventual debugging of issues arising from the fixup
done
- * here, but can be dropped as soon as it is found to conflict
with
- * other (future) changes.
+ * This is a migration from a broken Xen between 4.4 and 4.18 and
+ * we must _PRESERVE_ LDRs so new vCPUs use consistent
derivations.
>>> Not sure if we should try to avoid mentioning specific versions in the
>>> comments, as I this fix will be backported to stable branches (I hope),
>>> and hence those will no longer be affected.
>> Hence the "broken Xen" part of the paragraphs. Not every 4.18 will have the
>> problem, but it shouldn't be seen in 4.19 onwards. I think there's value in
>> stating the versions that "may" exhibit problems, but this is all
>> subjective
> FE.
This patch has a high chance of being backported. Old version numbers
(4.4 in this case) are critical information.
For the new end, "to date (Nov 2023)" or similar tends to works better,
because it also doesn't go stale when backported.
If a version is necessary/preferred, then "4.19 dev window" also works well.
~Andrew
Re: [PATCH v3] xen/x86: On x2APIC mode, derive LDR from APIC ID
On Thu, Nov 23, 2023 at 12:21:36PM +, Alejandro Vallejo wrote:
> On Thu, Nov 23, 2023 at 10:03:12AM +0100, Roger Pau Monné wrote:
> > On Wed, Nov 22, 2023 at 04:08:17PM +, Alejandro Vallejo wrote:
> > > +static uint32_t x2apic_ldr_from_id(uint32_t id)
> > > +{
> > > +return ((id & ~0xf) << 12) | (1 << (id & 0xf));
> > > +}
> > > +
> > > static void set_x2apic_id(struct vlapic *vlapic)
> > > {
> > > -u32 id = vlapic_vcpu(vlapic)->vcpu_id;
> > > -u32 ldr = ((id & ~0xf) << 12) | (1 << (id & 0xf));
> > > +uint32_t vcpu_id = vlapic_vcpu(vlapic)->vcpu_id;
> > > +uint32_t apic_id = vcpu_id * 2;
> > > +uint32_t apic_ldr = x2apic_ldr_from_id(apic_id);
> > > +
> > > +/* This is a migration bug workaround. See wall of text in
> > > lapic_load_fixup() */
> >
> > Line length > 80 cols.
> >
> > I try to avoid referencing function names in comments, as they tend to
> > get out of sync without noticing. It's much easier to use cscope to
> > grep for x2apic_ldr_bug_with_vcpu_id and find the comment itself.
> In my experience that's less of a problem than it's usually made out to be,
> and helps new readers know about the real context something is in the place
> it is.
>
> But I do hold the atypical belief that an out of date pointer to context is
> preferrable to no context.
It's a question of taste TBH, I'm certainly not going to insist.
Since you have to wrap the line to fit in 80 cols anyway, I think I
would rather write: "This is a workaround for migrated domains. ...".
Current text reads to me as it's a migration bug, but that's not the
case, the bug is in the previous Xen versions. I'm not a native
speaker anyway, so maybe it's just me reading it wrong.
> >
> > > +if ( vlapic_domain(vlapic)->arch.hvm.x2apic_ldr_bug_with_vcpu_id )
> > > +apic_ldr = x2apic_ldr_from_id(vcpu_id);
> > >
> > > -vlapic_set_reg(vlapic, APIC_ID, id * 2);
> > > -vlapic_set_reg(vlapic, APIC_LDR, ldr);
> > > +vlapic_set_reg(vlapic, APIC_ID, apic_id);
> > > +vlapic_set_reg(vlapic, APIC_LDR, apic_ldr);
> > > }
> > >
> > > int guest_wrmsr_apic_base(struct vcpu *v, uint64_t val)
> > > @@ -1498,27 +1508,35 @@ static int cf_check lapic_save_regs(struct vcpu
> > > *v, hvm_domain_context_t *h)
> > > */
> > > static void lapic_load_fixup(struct vlapic *vlapic)
> > > {
> > > -uint32_t id = vlapic->loaded.id;
> > > +/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already
> > > correct */
> > > +if ( !vlapic_x2apic_mode(vlapic) ||
> > > + (vlapic->loaded.ldr == x2apic_ldr_from_id(vlapic->loaded.id)) )
> > > +return;
> > >
> > > -if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
> > > -{
> > > +if ( vlapic->loaded.ldr == 1 )
> > > + /*
> > > +* Xen <= 4.4 had a bug by which all the APICs configured in
> > > x2APIC
> > > +* mode got LDR = 1. We can't leave it as-is because it assigned
> > > the
> > > +* same LDR to every CPU. We'll fix fix the bug now and assign an
> > > +* LDR value consistent with the APIC ID.
> > > +*/
> > > +set_x2apic_id(vlapic);
> > > +else if ( vlapic->loaded.ldr ==
> > > + x2apic_ldr_from_id(vlapic_vcpu(vlapic)->vcpu_id) )
> > > /*
> > > - * This is optional: ID != 0 contradicts LDR == 1. It's being
> > > added
> > > - * to aid in eventual debugging of issues arising from the fixup
> > > done
> > > - * here, but can be dropped as soon as it is found to conflict
> > > with
> > > - * other (future) changes.
> > > + * This is a migration from a broken Xen between 4.4 and 4.18 and
> > > + * we must _PRESERVE_ LDRs so new vCPUs use consistent
> > > derivations.
> >
> > Not sure if we should try to avoid mentioning specific versions in the
> > comments, as I this fix will be backported to stable branches (I hope),
> > and hence those will no longer be affected.
> Hence the "broken Xen" part of the paragraphs. Not every 4.18 will have the
> problem, but it shouldn't be seen in 4.19 onwards. I think there's value in
> stating the versions that "may" exhibit problems, but this is all
> subjective
FE.
> >
> > > + * This is so existing running guests that may have already read
> > > + * the LDR at the source host aren't surprised when IPIs stop
> > > + * working as they did at the other end. To address this, we set
> > > + * this domain boolean so future CPU hotplugs derive an LDR
> > > + * consistent with the older Xen's broken idea of consistency.
> >
> > I think this is possibly too verbose, I would be fine with just the
> > first sentence TBH. If we want the full comment however, the wording
> > should be slightly addressed: it's not just IPIs that would possibly
> > fail to be delivered, but any interrupt attempting to target the APIC
> > using the previous LDR addressing (either an IPI or an external
> >
Re: [PATCH] xen/efi: Drop image_name from efi_arch_handle_cmdline()
> On 23 Nov 2023, at 11:37, Andrew Cooper wrote: > > With all architectures no longer wanting an image name in the command line > handling, drop the parameter and forgo making one up. > > 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 > --- > xen/arch/arm/efi/efi-boot.h | 3 +-- Tested on ARM board Tested-by: Luca Fancellu
The Xen Project’s 20th Anniversary - Upcoming Social Event!
*The Xen Project’s 20th Anniversary* *Let's get together for an informal social, likely to be pizza/drinks and getting involved with the community. I hope to run these in future locations to give everyone a chance to attend.* *Date placeholder: Wednesday 21st February 2024* *Location: Cambridge* *Details TBC - **If you're interested, please reply to me directly and I will add you to the list.* *Celebrating Two Decades of Innovation* It’s hard to believe that two decades have passed since the inception of the Xen Project, a trailblazing force in the world of open-source virtualization. As we raise our glasses to commemorate this momentous occasion, it’s not just a celebration of time but a reflection on the incredible journey that has defined the Xen Project’s legacy. *A Legacy of Innovation* In the year 2003, the Xen Project emerged as a pioneering open-source hypervisor, laying the groundwork for some of the most influential cloud infrastructures that shape our digital landscape today. Over the past 20 years, the Xen Project has not only endured but has thrived, continuously evolving to meet the dynamic demands of the ever-changing tech landscape. *Driving Technological Frontiers* >From data center and server virtualization to cloud computing, desktop virtualization, and fortifying desktop security and hardware appliances, the Xen Project has been at the forefront of driving technological innovation. With 20 years of relentless development, it has become synonymous with reliability, scalability, and adaptability. *Venturing into New Horizons* As we celebrate this milestone, we also look forward to the exciting new territories that the Xen Project is venturing into. From embedded virtualization to even making strides in the automotive industry, the Xen Project continues to push boundaries and redefine what’s possible in the world of open-source virtualization. *The Annual Event: Xen Project Developer and Design Summit* At the heart of this remarkable journey is the Xen Project Developer and Design Summit, an annual gathering of the community’s brilliant minds and power users. More than just a conference, it’s a celebration of idea exchange, a showcase of the latest advancements, a platform for sharing invaluable experiences, and a forum for strategic planning and collaborative efforts. Be sure to look out for our upcoming event in 2024. *A Vibrant Community Defining the Future* Beyond the code and technological achievements, the Xen Project’s strength lies in its vibrant community. It’s a community that has come together to celebrate successes, overcome challenges, and collectively shape the future of open-source virtualization technology. Even to this day, community contributions and reviews are still going! *Looking Ahead* As we commemorate 20 years of innovation, we also eagerly anticipate the next chapter in the Xen Project’s journey. With gratitude for the past and excitement for the future, we extend our deepest thanks to everyone who has contributed to this incredible legacy. Here’s to 20 years of pushing boundaries, fostering collaboration, and shaping the digital landscape. Happy anniversary, Xen Project! The best is yet to come and I can’t wait to see what we all achieve. Many thanks, Kelly Choi Open Source Community Manager XenServer, Cloud Software Group
Re: [PATCH v7 1/2] xen/vpci: header: status register handler
On 11/23/23 03:14, Roger Pau Monné wrote: > On Wed, Nov 22, 2023 at 03:16:29PM -0500, Stewart Hildebrand wrote: >> On 11/17/23 07:40, Roger Pau Monné wrote: >>> On Wed, Sep 13, 2023 at 10:35:46AM -0400, Stewart Hildebrand wrote: r->write(pdev, r->offset, data & (0xU >> (32 - 8 * r->size)), r->private); } diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h index 84b18736a85d..b72131729db6 100644 --- a/xen/include/xen/pci_regs.h +++ b/xen/include/xen/pci_regs.h @@ -66,6 +66,15 @@ #define PCI_STATUS_REC_MASTER_ABORT 0x2000 /* Set on master abort */ #define PCI_STATUS_SIG_SYSTEM_ERROR 0x4000 /* Set when we drive SERR */ #define PCI_STATUS_DETECTED_PARITY 0x8000 /* Set on parity error */ +#define PCI_STATUS_RSVDZ_MASK0x0006 >>> >>> In my copy of the PCIe 6 spec bit 6 is also RsvdZ, so the mask should >>> be 0x46. >> >> Right, mine too. It's probably safer to follow the newer version of the >> spec, so I'll make the change. For completeness / archaeology purposes, I >> just want to document some relevant data points here. >> >> In PCIe 4 spec, it says this about bit 6: >> "These bits were used in previous versions of the programming model. Careful >> consideration should be given to any attempt to repurpose them." >> >> Going further back, PCI (old school PCI, not Express) spec 3.0 says this >> about bit 6: >> "This bit is reserved. *" >> "* In Revision 2.1 of this specification, this bit was used to indicate >> whether or not a device supported User Definable Features." >> >> Just above in our pci_regs.h (and equally in Linux >> include/uapi/linux/pci_regs.h) we have this definition for bit 6: >> #define PCI_STATUS_UDF 0x40/* Support User Definable Features >> [obsolete] */ >> >> Qemu hw/xen/xen_pt_config_init.c treats bit 6 as RO: >> .ro_mask= 0x06F8, > > Right, given the implementation of ro_mask that would likely be fine. > Reading unconditionally as 0 while preserving the value on writes > seems the safest option. That would mean treating bit 6 as RsvdP, even though the PCIe 6 spec says RsvdZ. I just want to confirm this is indeed the intent since we both said RsvdZ just a moment ago? If so, I would add a comment since it's deviating from spec. I would personally still vote in favor of following PCIe 6 spec (RsvdZ).
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23/11/2023 12:27, David Woodhouse wrote: On 23 November 2023 12:17:57 GMT, Volodymyr Babchuk wrote: Hi David, David Woodhouse writes: Which PV backends do you care about? We already have net, block and console converted. Well, this is all what we need, actually. Even console only will be sufficient, as we are using QEMU to provide virtio-pci backends, so both storage and networking should be provided by virtio. Are you proposing to just drop this patch at all? I believe we can live without it, yes. Yeah, I think you can drop anything that's only needed for the legacy backends. I'm tempted to make a separate config option to compile those out. I think that would be a good idea. The other legacy bacckend that we may need to care about is xenfb... not so much the framebuffer itself, but the mouse and keyboard aspects. The XENVKBD and XENHID drivers expose PV mouse and keyboard to Windows instances so it's be nice if we can avoid the backend withering away. Paul
[XEN PATCH v2] xen/sort: address violations of MISRA C:2012 Rule 8.2
Add missing parameter names. No functional change.
Signed-off-by: Federico Serafini
---
Changes in v2:
- used "a" and "b" for cmp.
---
xen/include/xen/sort.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/include/xen/sort.h b/xen/include/xen/sort.h
index 2f52ff85b9..b953286284 100644
--- a/xen/include/xen/sort.h
+++ b/xen/include/xen/sort.h
@@ -23,8 +23,8 @@
extern gnu_inline
#endif
void sort(void *base, size_t num, size_t size,
- int (*cmp)(const void *, const void *),
- void (*swap)(void *, void *, size_t))
+ int (*cmp)(const void *a, const void *b),
+ void (*swap)(void *a, void *b, size_t size))
{
/* pre-scale counters for performance */
size_t i = (num / 2) * size, n = num * size, c, r;
--
2.34.1
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23 November 2023 12:17:57 GMT, Volodymyr Babchuk wrote: > >Hi David, > >David Woodhouse writes: >> Which PV backends do you care about? We already have net, block and console >> converted. > >Well, this is all what we need, actually. Even console only will be >sufficient, as we are using QEMU to provide virtio-pci backends, so both >storage and networking should be provided by virtio. Are you proposing >to just drop this patch at all? I believe we can live without it, yes. Yeah, I think you can drop anything that's only needed for the legacy backends. I'm tempted to make a separate config option to compile those out.
Re: [PATCH 1/3] automation/alpine: add elfutils-dev and coreutils for livepatch-build-tools
On Thu, Nov 23, 2023 at 11:51:33AM +, Andrew Cooper wrote: > On 23/11/2023 11:23 am, Roger Pau Monne wrote: > > In preparation for adding some livepatch-build-tools test update the Alpine > > container to also install elfutils-dev, coreutils and GNU awk. > > > > Signed-off-by: Roger Pau Monné > > --- > > I don't very much like to add coreutils and gawk, as it's also good to test > > that we can build Xen with Busybox, but I also got tired of adjusting > > livepatch-build-tools. > > Right now, the alpine environment is the main one which spots violations > of Xen's requirement for simply a POSIX-compliant awk, which I think > this would break? Likely, we also test on Cirrus using FreeBSD, so we would at least spot instances where the extensions are not implemented by BSD sed. > How much effort would it be fix livepatch-build-tools? I think that > would be preferable, and could be persuaded to do some simple busywork... I can give it a try. > What about coreutils? Presumably that's down to some differences from > busybox ? Yeah, it's for the usage of `readlink -m` by livepatch-build-tools. We will have to switch to using `readlink -f`, as that's the only canonicalize option supported by BusyBox readlink. Note that anyway `readlink` is not part of POSIX. Thanks, Roger.
Re: [PATCH v3] xen/x86: On x2APIC mode, derive LDR from APIC ID
On Thu, Nov 23, 2023 at 10:03:12AM +0100, Roger Pau Monné wrote:
> On Wed, Nov 22, 2023 at 04:08:17PM +, Alejandro Vallejo wrote:
> > Both Intel and AMD manuals agree that on x2APIC mode, the APIC LDR and ID
> > registers are derivable from each other through a fixed formula.
> >
> > Xen uses that formula, but applies it to vCPU IDs (which are sequential)
> > rather than x2APIC IDs (which are not, at the moment). As I understand it,
> > this is an attempt to tightly pack vCPUs into clusters so each cluster has
> > 16 vCPUs rather than 8, but this is problematic for OSs that might read the
> > x2APIC ID and internally derive LDR (or the other way around)
> >
> > This patch fixes the implementation so we follow the rules in the x2APIC
> > spec(s) and covers migrations from broken hypervisors, so LDRs are
> > preserved even for hotppluggable CPUs and across APIC resets.
> >
> > While touching that area, I removed the existing printk statement in
> > vlapic_load_fixup() (as the checks it performed didn't make sense in x2APIC
> > mode and wouldn't affect the outcome) and put another printk as an else
> > branch so we get warnings trying to load nonsensical LDR values we don't
> > know about.
> >
> > Fixes: f9e0cccf7b35 ("x86/HVM: fix ID handling of x2APIC emulation")
> > Signed-off-by: Alejandro Vallejo
>
> Mostly some style nits, and one comment adjustment.
>
> If you are OK with those:
>
> Reviewed-by: Roger Pau Monné
>
> > ---
> > v3:
> > * Removed underscores from (x2)APIC_ID in commit message
> > * Added commit msg explaining the removal of the vlapic_load_fixup()
> > printk
> > * Restored lowercase to hex "F"s
> > * Refactored a bit vlapic_load_fixup() so it has a trailing printk in
> > case of spotting a nonsensical LDR it doesn't understand.
> > * Moved field in domain.h with the other booleans
> > * Trimmed down field name in domain.h
> > * Trimmed down field comment in domain.h
> >
> > If the field name in domain.h still seems too long I'm happy for it to be
> > trimmed further down, but I do want to preserve the "_bug_" part of it.
>
> I think the _with_ part is redundant, but it's certainly shorter than
> previously :).
Sure
>
> > ---
> > xen/arch/x86/hvm/vlapic.c | 62 +--
> > xen/arch/x86/include/asm/hvm/domain.h | 3 ++
> > 2 files changed, 43 insertions(+), 22 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> > index 5cb87f8649..cd929c3716 100644
> > --- a/xen/arch/x86/hvm/vlapic.c
> > +++ b/xen/arch/x86/hvm/vlapic.c
> > @@ -1061,13 +1061,23 @@ static const struct hvm_mmio_ops vlapic_mmio_ops = {
> > .write = vlapic_mmio_write,
> > };
> >
> > +static uint32_t x2apic_ldr_from_id(uint32_t id)
> > +{
> > +return ((id & ~0xf) << 12) | (1 << (id & 0xf));
> > +}
> > +
> > static void set_x2apic_id(struct vlapic *vlapic)
> > {
> > -u32 id = vlapic_vcpu(vlapic)->vcpu_id;
> > -u32 ldr = ((id & ~0xf) << 12) | (1 << (id & 0xf));
> > +uint32_t vcpu_id = vlapic_vcpu(vlapic)->vcpu_id;
> > +uint32_t apic_id = vcpu_id * 2;
> > +uint32_t apic_ldr = x2apic_ldr_from_id(apic_id);
> > +
> > +/* This is a migration bug workaround. See wall of text in
> > lapic_load_fixup() */
>
> Line length > 80 cols.
>
> I try to avoid referencing function names in comments, as they tend to
> get out of sync without noticing. It's much easier to use cscope to
> grep for x2apic_ldr_bug_with_vcpu_id and find the comment itself.
In my experience that's less of a problem than it's usually made out to be,
and helps new readers know about the real context something is in the place
it is.
But I do hold the atypical belief that an out of date pointer to context is
preferrable to no context.
>
> > +if ( vlapic_domain(vlapic)->arch.hvm.x2apic_ldr_bug_with_vcpu_id )
> > +apic_ldr = x2apic_ldr_from_id(vcpu_id);
> >
> > -vlapic_set_reg(vlapic, APIC_ID, id * 2);
> > -vlapic_set_reg(vlapic, APIC_LDR, ldr);
> > +vlapic_set_reg(vlapic, APIC_ID, apic_id);
> > +vlapic_set_reg(vlapic, APIC_LDR, apic_ldr);
> > }
> >
> > int guest_wrmsr_apic_base(struct vcpu *v, uint64_t val)
> > @@ -1498,27 +1508,35 @@ static int cf_check lapic_save_regs(struct vcpu *v,
> > hvm_domain_context_t *h)
> > */
> > static void lapic_load_fixup(struct vlapic *vlapic)
> > {
> > -uint32_t id = vlapic->loaded.id;
> > +/* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct
> > */
> > +if ( !vlapic_x2apic_mode(vlapic) ||
> > + (vlapic->loaded.ldr == x2apic_ldr_from_id(vlapic->loaded.id)) )
> > +return;
> >
> > -if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
> > -{
> > +if ( vlapic->loaded.ldr == 1 )
> > + /*
> > +* Xen <= 4.4 had a bug by which all the APICs configured in x2APIC
> > +* mode got LDR = 1. We can't leave it as-is because it assigned the
> > +* same LDR to every
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David, David Woodhouse writes: > On 23 November 2023 11:54:01 GMT, Volodymyr Babchuk > wrote: >> >>Hi Paul, >> >>Paul Durrant writes: >> >>> On 23/11/2023 00:07, Volodymyr Babchuk wrote: Hi, Volodymyr Babchuk writes: > Hi Stefano, > > Stefano Stabellini writes: > >> On Wed, 22 Nov 2023, David Woodhouse wrote: >>> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> On Wed, 22 Nov 2023, Paul Durrant wrote: >>> On 21/11/2023 22:10, Volodymyr Babchuk wrote: From: Oleksandr Tyshchenko Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to inherit the owner of the directory. >>> >>> Ah... so that's why the previous patch is there. >>> >>> This is not the right way to fix it. The QEMU Xen support is >>> *assuming* that >>> QEMU is either running in, or emulating, dom0. In the emulation >>> case this is >>> probably fine, but the 'real Xen' case it should be using the >>> correct domid >>> for node creation. I guess this could either be supplied on the >>> command line >>> or discerned by reading the local domain 'domid' node. >> >> yes, it should be passed as command line option to QEMU > > I'm not sure I like the idea of a command line option for something > which QEMU could discover for itself. That's fine too. I meant to say "yes, as far as I know the toolstack passes the domid to QEMU as a command line option today". >>> >>> The -xen-domid argument on the QEMU command line today is the *guest* >>> domain ID, not the domain ID in which QEMU itself is running. >>> >>> Or were you thinking of something different? >> >> Ops, you are right and I understand your comment better now. The backend >> domid is not on the command line but it should be discoverable (on >> xenstore if I remember right). > > Yes, it is just "~/domid". I'll add a function that reads it. Just a quick question to QEMU folks: is it better to add a global variable where we will store own Domain ID or it will be okay to read domid from Xenstore every time we need it? If global variable variant is better, what is proffered place to define this variable? system/globals.c ? >>> >>> Actually... is it possible for QEMU just to use a relative path for >>> the backend nodes? That way it won't need to know it's own domid, will >>> it? >> >>Well, it is possible to use relative path, AFAIK, linux-based backends >>are doing exactly this. But problem is with xenstore_mkdir() function, >>which requires domain id to correctly set owner when it causes call to >>set_permissions(). >> >>As David said, architecturally it will be better to get rid of >>xenstore_mkdir() usage, because it is used by legacy backends only. But >>to do this, someone needs to convert legacy backends to use newer API. I >>have no capacity to do this right now, so I implemented a contained >>solution: >> >>static int xenstore_read_own_domid(unsigned int *domid) >> >>in xen_pvdev.c. I believe, this new function will be removed along with >>whole xen_pvdev.c when there will be no legacy backends left. > > Which PV backends do you care about? We already have net, block and console > converted. Well, this is all what we need, actually. Even console only will be sufficient, as we are using QEMU to provide virtio-pci backends, so both storage and networking should be provided by virtio. Are you proposing to just drop this patch at all? I believe we can live without it, yes. -- WBR, Volodymyr
Re: [PATCH] xen/efi: Drop image_name from efi_arch_handle_cmdline()
Hi Andrew, On 23/11/2023 11:37, Andrew Cooper wrote: With all architectures no longer wanting an image name in the command line handling, drop the parameter and forgo making one up. Signed-off-by: Andrew Cooper I am not sure if you need one Ack for the change in arm/efi/efi-boot.h. If you do: Acked-by: Julien Grall Cheers, -- Julien Grall
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23 November 2023 11:54:01 GMT, Volodymyr Babchuk wrote: > >Hi Paul, > >Paul Durrant writes: > >> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >>> Hi, >>> Volodymyr Babchuk writes: >>> Hi Stefano, Stefano Stabellini writes: > On Wed, 22 Nov 2023, David Woodhouse wrote: >> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>> On Wed, 22 Nov 2023, David Woodhouse wrote: On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > On Wed, 22 Nov 2023, Paul Durrant wrote: >> On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>> From: Oleksandr Tyshchenko >>> >>> Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >>> inherit the owner of the directory. >> >> Ah... so that's why the previous patch is there. >> >> This is not the right way to fix it. The QEMU Xen support is >> *assuming* that >> QEMU is either running in, or emulating, dom0. In the emulation case >> this is >> probably fine, but the 'real Xen' case it should be using the >> correct domid >> for node creation. I guess this could either be supplied on the >> command line >> or discerned by reading the local domain 'domid' node. > > yes, it should be passed as command line option to QEMU I'm not sure I like the idea of a command line option for something which QEMU could discover for itself. >>> >>> That's fine too. I meant to say "yes, as far as I know the toolstack >>> passes the domid to QEMU as a command line option today". >> >> The -xen-domid argument on the QEMU command line today is the *guest* >> domain ID, not the domain ID in which QEMU itself is running. >> >> Or were you thinking of something different? > > Ops, you are right and I understand your comment better now. The backend > domid is not on the command line but it should be discoverable (on > xenstore if I remember right). Yes, it is just "~/domid". I'll add a function that reads it. >>> Just a quick question to QEMU folks: is it better to add a global >>> variable where we will store own Domain ID or it will be okay to read >>> domid from Xenstore every time we need it? >>> If global variable variant is better, what is proffered place to >>> define >>> this variable? system/globals.c ? >>> >> >> Actually... is it possible for QEMU just to use a relative path for >> the backend nodes? That way it won't need to know it's own domid, will >> it? > >Well, it is possible to use relative path, AFAIK, linux-based backends >are doing exactly this. But problem is with xenstore_mkdir() function, >which requires domain id to correctly set owner when it causes call to >set_permissions(). > >As David said, architecturally it will be better to get rid of >xenstore_mkdir() usage, because it is used by legacy backends only. But >to do this, someone needs to convert legacy backends to use newer API. I >have no capacity to do this right now, so I implemented a contained >solution: > >static int xenstore_read_own_domid(unsigned int *domid) > >in xen_pvdev.c. I believe, this new function will be removed along with >whole xen_pvdev.c when there will be no legacy backends left. Which PV backends do you care about? We already have net, block and console converted.
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi Paul, Paul Durrant writes: > On 23/11/2023 00:07, Volodymyr Babchuk wrote: >> Hi, >> Volodymyr Babchuk writes: >> >>> Hi Stefano, >>> >>> Stefano Stabellini writes: >>> On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> On Wed, 22 Nov 2023, David Woodhouse wrote: >>> On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: On Wed, 22 Nov 2023, Paul Durrant wrote: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> From: Oleksandr Tyshchenko >> >> Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >> inherit the owner of the directory. > > Ah... so that's why the previous patch is there. > > This is not the right way to fix it. The QEMU Xen support is > *assuming* that > QEMU is either running in, or emulating, dom0. In the emulation case > this is > probably fine, but the 'real Xen' case it should be using the correct > domid > for node creation. I guess this could either be supplied on the > command line > or discerned by reading the local domain 'domid' node. yes, it should be passed as command line option to QEMU >>> >>> I'm not sure I like the idea of a command line option for something >>> which QEMU could discover for itself. >> >> That's fine too. I meant to say "yes, as far as I know the toolstack >> passes the domid to QEMU as a command line option today". > > The -xen-domid argument on the QEMU command line today is the *guest* > domain ID, not the domain ID in which QEMU itself is running. > > Or were you thinking of something different? Ops, you are right and I understand your comment better now. The backend domid is not on the command line but it should be discoverable (on xenstore if I remember right). >>> >>> Yes, it is just "~/domid". I'll add a function that reads it. >> Just a quick question to QEMU folks: is it better to add a global >> variable where we will store own Domain ID or it will be okay to read >> domid from Xenstore every time we need it? >> If global variable variant is better, what is proffered place to >> define >> this variable? system/globals.c ? >> > > Actually... is it possible for QEMU just to use a relative path for > the backend nodes? That way it won't need to know it's own domid, will > it? Well, it is possible to use relative path, AFAIK, linux-based backends are doing exactly this. But problem is with xenstore_mkdir() function, which requires domain id to correctly set owner when it causes call to set_permissions(). As David said, architecturally it will be better to get rid of xenstore_mkdir() usage, because it is used by legacy backends only. But to do this, someone needs to convert legacy backends to use newer API. I have no capacity to do this right now, so I implemented a contained solution: static int xenstore_read_own_domid(unsigned int *domid) in xen_pvdev.c. I believe, this new function will be removed along with whole xen_pvdev.c when there will be no legacy backends left. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 23 November 2023 11:43:35 GMT, Volodymyr Babchuk wrote: > >Hi David, > >David Woodhouse writes: > >> [[S/MIME Signed Part:Undecided]] >> On Thu, 2023-11-23 at 09:28 +, Paul Durrant wrote: >>> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >>> > >>> > Hi, >>> > >>> > Volodymyr Babchuk writes: >>> > >>> > > Hi Stefano, >>> > > >>> > > Stefano Stabellini writes: >>> > > >>> > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >>> > > > > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>> > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >>> > > > > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >>> > > > > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >>> > > > > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>> > > > > > > > > > From: Oleksandr Tyshchenko >>> > > > > > > > > > >>> > > > > > > > > > Instead of forcing the owner to domid 0, use >>> > > > > > > > > > XS_PRESERVE_OWNER to >>> > > > > > > > > > inherit the owner of the directory. >>> > > > > > > > > >>> > > > > > > > > Ah... so that's why the previous patch is there. >>> > > > > > > > > >>> > > > > > > > > This is not the right way to fix it. The QEMU Xen support >>> > > > > > > > > is *assuming* that >>> > > > > > > > > QEMU is either running in, or emulating, dom0. In the >>> > > > > > > > > emulation case this is >>> > > > > > > > > probably fine, but the 'real Xen' case it should be using >>> > > > > > > > > the correct domid >>> > > > > > > > > for node creation. I guess this could either be supplied on >>> > > > > > > > > the command line >>> > > > > > > > > or discerned by reading the local domain 'domid' node. >>> > > > > > > > >>> > > > > > > > yes, it should be passed as command line option to QEMU >>> > > > > > > >>> > > > > > > I'm not sure I like the idea of a command line option for >>> > > > > > > something >>> > > > > > > which QEMU could discover for itself. >>> > > > > > >>> > > > > > That's fine too. I meant to say "yes, as far as I know the >>> > > > > > toolstack >>> > > > > > passes the domid to QEMU as a command line option today". >>> > > > > >>> > > > > The -xen-domid argument on the QEMU command line today is the >>> > > > > *guest* >>> > > > > domain ID, not the domain ID in which QEMU itself is running. >>> > > > > >>> > > > > Or were you thinking of something different? >>> > > > >>> > > > Ops, you are right and I understand your comment better now. The >>> > > > backend >>> > > > domid is not on the command line but it should be discoverable (on >>> > > > xenstore if I remember right). >>> > > >>> > > Yes, it is just "~/domid". I'll add a function that reads it. >>> > >>> > Just a quick question to QEMU folks: is it better to add a global >>> > variable where we will store own Domain ID or it will be okay to read >>> > domid from Xenstore every time we need it? >>> > >>> > If global variable variant is better, what is proffered place to define >>> > this variable? system/globals.c ? >>> > >>> >>> Actually... is it possible for QEMU just to use a relative path for the >>> backend nodes? That way it won't need to know it's own domid, will it? >> >> That covers some of the use cases, but it may also need to know its own >> domid for other purposes. Including writing the *absolute* path of the >> backend, to a frontend node? > >Is this case possible? As I understand, QEMU writes frontend nodes only >when it emulates Xen, otherwise this done by Xen toolstack. And in case >of Xen emulation, QEMU always assumes role of Domain-0. No, you can hotplug and unplug devices in QEMU even under real Xen. And if QEMU in a driver domain is given sufficient permission to write to its guest's frontend nodes, it should not need to be in dom0.
Re: [PATCH 1/3] automation/alpine: add elfutils-dev and coreutils for livepatch-build-tools
On 23/11/2023 11:23 am, Roger Pau Monne wrote: > In preparation for adding some livepatch-build-tools test update the Alpine > container to also install elfutils-dev, coreutils and GNU awk. > > Signed-off-by: Roger Pau Monné > --- > I don't very much like to add coreutils and gawk, as it's also good to test > that we can build Xen with Busybox, but I also got tired of adjusting > livepatch-build-tools. Right now, the alpine environment is the main one which spots violations of Xen's requirement for simply a POSIX-compliant awk, which I think this would break? How much effort would it be fix livepatch-build-tools? I think that would be preferable, and could be persuaded to do some simple busywork... What about coreutils? Presumably that's down to some differences from busybox ? ~Andrew
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi David, David Woodhouse writes: > [[S/MIME Signed Part:Undecided]] > On Thu, 2023-11-23 at 09:28 +, Paul Durrant wrote: >> On 23/11/2023 00:07, Volodymyr Babchuk wrote: >> > >> > Hi, >> > >> > Volodymyr Babchuk writes: >> > >> > > Hi Stefano, >> > > >> > > Stefano Stabellini writes: >> > > >> > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > > > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > > > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> > > > > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >> > > > > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > > > > > > > > > From: Oleksandr Tyshchenko >> > > > > > > > > > >> > > > > > > > > > Instead of forcing the owner to domid 0, use >> > > > > > > > > > XS_PRESERVE_OWNER to >> > > > > > > > > > inherit the owner of the directory. >> > > > > > > > > >> > > > > > > > > Ah... so that's why the previous patch is there. >> > > > > > > > > >> > > > > > > > > This is not the right way to fix it. The QEMU Xen support is >> > > > > > > > > *assuming* that >> > > > > > > > > QEMU is either running in, or emulating, dom0. In the >> > > > > > > > > emulation case this is >> > > > > > > > > probably fine, but the 'real Xen' case it should be using >> > > > > > > > > the correct domid >> > > > > > > > > for node creation. I guess this could either be supplied on >> > > > > > > > > the command line >> > > > > > > > > or discerned by reading the local domain 'domid' node. >> > > > > > > > >> > > > > > > > yes, it should be passed as command line option to QEMU >> > > > > > > >> > > > > > > I'm not sure I like the idea of a command line option for >> > > > > > > something >> > > > > > > which QEMU could discover for itself. >> > > > > > >> > > > > > That's fine too. I meant to say "yes, as far as I know the >> > > > > > toolstack >> > > > > > passes the domid to QEMU as a command line option today". >> > > > > >> > > > > The -xen-domid argument on the QEMU command line today is the *guest* >> > > > > domain ID, not the domain ID in which QEMU itself is running. >> > > > > >> > > > > Or were you thinking of something different? >> > > > >> > > > Ops, you are right and I understand your comment better now. The >> > > > backend >> > > > domid is not on the command line but it should be discoverable (on >> > > > xenstore if I remember right). >> > > >> > > Yes, it is just "~/domid". I'll add a function that reads it. >> > >> > Just a quick question to QEMU folks: is it better to add a global >> > variable where we will store own Domain ID or it will be okay to read >> > domid from Xenstore every time we need it? >> > >> > If global variable variant is better, what is proffered place to define >> > this variable? system/globals.c ? >> > >> >> Actually... is it possible for QEMU just to use a relative path for the >> backend nodes? That way it won't need to know it's own domid, will it? > > That covers some of the use cases, but it may also need to know its own > domid for other purposes. Including writing the *absolute* path of the > backend, to a frontend node? Is this case possible? As I understand, QEMU writes frontend nodes only when it emulates Xen, otherwise this done by Xen toolstack. And in case of Xen emulation, QEMU always assumes role of Domain-0. -- WBR, Volodymyr
Re: [PATCH] xen/efi: Drop image_name from efi_arch_handle_cmdline()
On 23/11/2023 11:42 am, Jan Beulich wrote: > On 23.11.2023 12:37, Andrew Cooper wrote: >> With all architectures no longer wanting an image name in the command line >> handling, drop the parameter and forgo making one up. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Jan Beulich Thanks. I'm quite surprised at how much cleanup this particular investigation has yielded. Definitely preferable to the first plan of using the const alias. ~Andrew
Re: [PATCH] xen/efi: Drop image_name from efi_arch_handle_cmdline()
On 23.11.2023 12:37, Andrew Cooper wrote: > With all architectures no longer wanting an image name in the command line > handling, drop the parameter and forgo making one up. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich
Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
On 2023-11-23 12:36, Jan Beulich wrote: On 23.11.2023 12:30, Nicola Vetrini wrote: I guess this one as well should remain as is. Can you confirm? void asmlinkage __stdcall cmdline_parse_early(const char *cmdline, early_boot_opts_t *ebo) Indeed, I simply overlooked it. Jan Thanks, I should be able to resend shortly. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
[PATCH] xen/efi: Drop image_name from efi_arch_handle_cmdline()
With all architectures no longer wanting an image name in the command line
handling, drop the parameter and forgo making one up.
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
---
xen/arch/arm/efi/efi-boot.h | 3 +--
xen/arch/x86/efi/efi-boot.h | 3 +--
xen/common/efi/boot.c | 15 +--
3 files changed, 3 insertions(+), 18 deletions(-)
diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 59d217667ff3..6e6db2445566 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -454,8 +454,7 @@ static void __init efi_arch_memory_setup(void)
{
}
-static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
- CHAR16 *cmdline_options,
+static void __init efi_arch_handle_cmdline(CHAR16 *cmdline_options,
const char *cfgfile_options)
{
union string name;
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 1a2a2dd83c9b..86467da301e5 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -303,8 +303,7 @@ static void __init efi_arch_cfg_file_late(const
EFI_LOADED_IMAGE *image,
}
}
-static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
- CHAR16 *cmdline_options,
+static void __init efi_arch_handle_cmdline(CHAR16 *cmdline_options,
const char *cfgfile_options)
{
union string name;
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index e5e86f22b2b2..61108199188f 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1462,21 +1462,8 @@ void EFIAPI __init noreturn efi_start(EFI_HANDLE
ImageHandle,
}
}
-/*
- * EFI_LOAD_OPTION does not supply an image name as first component:
- * Make one up.
- */
-if ( argc && !*argv )
-{
-EFI_FILE_HANDLE handle = get_parent_handle(loaded_image,
- _name);
-
-handle->Close(handle);
-*argv = file_name;
-}
-
name.s = get_value(, section.s, "options");
-efi_arch_handle_cmdline(argc ? *argv : NULL, options, name.s);
+efi_arch_handle_cmdline(options, name.s);
if ( !base_video )
{
base-commit: f96e2f64576cdbb147391c7cb399d393385719a9
--
2.30.2
Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
On 23.11.2023 12:30, Nicola Vetrini wrote: > I guess this one as well should remain as is. Can you confirm? > > void asmlinkage __stdcall cmdline_parse_early(const char *cmdline, >early_boot_opts_t *ebo) > Indeed, I simply overlooked it. Jan
Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
I guess this one as well should remain as is. Can you confirm? void asmlinkage __stdcall cmdline_parse_early(const char *cmdline, early_boot_opts_t *ebo) -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
[PATCH 3/3] automation: add x86-64 livepatching test
Introduce a new gitlab tests for livepatching, using livepatch-build-tools,
which better reflects how downstreams build live patches rather than the
in-tree tests.
The tests applies the dummy in-tree patch example, checks that the patch is
applied correctly and then reverts and unloads it.
Signed-off-by: Roger Pau Monné
---
automation/gitlab-ci/build.yaml | 8 ++
automation/gitlab-ci/test.yaml| 8 ++
automation/scripts/build | 13 +++
.../scripts/qemu-alpine-x86_64-livepatch.sh | 79 +++
4 files changed, 108 insertions(+)
create mode 100755 automation/scripts/qemu-alpine-x86_64-livepatch.sh
diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 32af30ccedc9..22026df51b87 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -358,6 +358,14 @@ alpine-3.18-gcc-debug:
variables:
CONTAINER: alpine:3.18
+alpine-3.18-gcc-livepatch:
+ extends: .gcc-x86-64-build
+ variables:
+CONTAINER: alpine:3.18
+LIVEPATCH: y
+EXTRA_XEN_CONFIG: |
+ CONFIG_LIVEPATCH=y
+
debian-stretch-gcc-debug:
extends: .gcc-x86-64-build-debug
variables:
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 6aabdb9d156f..58a90be5ed0e 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -459,3 +459,11 @@ qemu-smoke-ppc64le-powernv9-gcc:
needs:
- qemu-system-ppc64-8.1.0-ppc64-export
- debian-bullseye-gcc-ppc64le-debug
+
+qemu-alpine-x86_64-gcc-livepatch:
+ extends: .qemu-x86-64
+ script:
+- ./automation/scripts/qemu-alpine-x86_64-livepatch.sh 2>&1 | tee
${LOGFILE}
+ needs:
+- *x86-64-test-needs
+- alpine-3.18-gcc-livepatch
diff --git a/automation/scripts/build b/automation/scripts/build
index b3c71fb6fb60..7ae735fc193e 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -103,3 +103,16 @@ else
cp -r dist binaries/
if [[ -f xen/xen ]] ; then cp xen/xen binaries/xen; fi
fi
+
+if [[ "$LIVEPATCH" == "y" ]]; then
+# Build a test livepatch using livepatch-build-tools.
+
+BUILDID=$(readelf -Wn xen/xen-syms | sed -n -e 's/^.*Build ID: //p')
+
+git clone https://xenbits.xen.org/git-http/livepatch-build-tools.git
+cd livepatch-build-tools
+make
+./livepatch-build -s ../ -p ../xen/test/livepatch/patches/test1.patch \
+-o out -c ../xen/.config --depends $BUILDID --xen-depends $BUILDID
+cp out/test1.livepatch ../binaries/test1.livepatch
+fi
diff --git a/automation/scripts/qemu-alpine-x86_64-livepatch.sh
b/automation/scripts/qemu-alpine-x86_64-livepatch.sh
new file mode 100755
index ..9b27a01b07f0
--- /dev/null
+++ b/automation/scripts/qemu-alpine-x86_64-livepatch.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+
+set -ex
+
+cd binaries
+# initrd.tar.gz is Dom0 rootfs
+mkdir -p rootfs
+cd rootfs
+tar xvzf ../initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../dist/install/* .
+cp ../test1.livepatch ./root/
+cat << "EOF" >> etc/local.d/xen.start
+#!/bin/bash
+
+set -ex
+
+trap poweroff EXIT
+
+export LD_LIBRARY_PATH=/usr/local/lib
+
+result=`xen-livepatch test`
+if [ "$result" != "1" ]; then
+echo "FAIL"
+exit 1
+fi
+
+xen-livepatch load /root/test1.livepatch
+
+result=`xen-livepatch test`
+if [ "$result" != "2" ]; then
+echo "FAIL"
+exit 1
+fi
+
+xen-livepatch revert test1
+xen-livepatch unload test1
+
+result=`xen-livepatch test`
+if [ "$result" != "1" ]; then
+echo "FAIL"
+exit 1
+fi
+
+echo "SUCCESS"
+EOF
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+# rebuild Dom0 rootfs
+find . |cpio -H newc -o|gzip > ../xen-rootfs.cpio.gz
+cd ../..
+
+cat >> binaries/pxelinux.0 << EOF
+#!ipxe
+
+kernel xen console=com1 console_timestamps=boot
+module bzImage console=hvc0
+module xen-rootfs.cpio.gz
+boot
+EOF
+
+# Run the test
+rm -f smoke.serial
+timeout -k 1 360 \
+qemu-system-x86_64 \
+-cpu qemu64,+svm \
+-m 2G -smp 2 \
+-monitor none -serial stdio \
+-nographic \
+-device virtio-net-pci,netdev=n0 \
+-netdev user,id=n0,tftp=binaries,bootfile=/pxelinux.0 |& \
+tee smoke.serial | sed 's/\r//'
+
+grep -q "SUCCESS" smoke.serial
+exit 0
--
2.43.0
[PATCH 1/3] automation/alpine: add elfutils-dev and coreutils for livepatch-build-tools
In preparation for adding some livepatch-build-tools test update the Alpine container to also install elfutils-dev, coreutils and GNU awk. Signed-off-by: Roger Pau Monné --- I don't very much like to add coreutils and gawk, as it's also good to test that we can build Xen with Busybox, but I also got tired of adjusting livepatch-build-tools. --- automation/build/alpine/3.18.dockerfile | 4 1 file changed, 4 insertions(+) diff --git a/automation/build/alpine/3.18.dockerfile b/automation/build/alpine/3.18.dockerfile index 4ae9cb5e9e30..fa6789347d87 100644 --- a/automation/build/alpine/3.18.dockerfile +++ b/automation/build/alpine/3.18.dockerfile @@ -47,3 +47,7 @@ RUN apk --no-cache add \ libcap-ng-dev \ ninja \ pixman-dev \ + # livepatch-tools deps + elfutils-dev \ + coreutils \ + gawk \ -- 2.43.0
[PATCH 2/3] livepatch: add a dummy hypercall for testing purposes
Introduce a dummy XEN_SYSCTL_LIVEPATCH_TEST hypercall to be used in order to
test livepatch functionality. The hypercall fills a value in the passed
structure, which is returned to the caller.
The xen-livepatch utility is expanded to allow calling that hypercall, and
printing the returned value on stdout.
Finally, add dummy patch that changes the returned value of the hypercall from
1 to 2. Such patch can be used with livepatch-build-tools in order to generate
a livepatch payload, that when applied to the hypervisor change the printed
value of `xen-livepatch test`.
Signed-off-by: Roger Pau Monné
---
The whole logic is very simple now. I think it's enough to have a skeleton we
can later expand.
Unsure whether we should do some kind of test (with `patch -F0`) that the patch
still applies cleanly as part of Xen build.
---
tools/include/xenctrl.h| 3 +++
tools/libs/ctrl/xc_misc.c | 14 ++
tools/misc/xen-livepatch.c | 25 +
xen/common/Makefile| 2 +-
xen/common/livepatch-test.c| 20
xen/common/livepatch.c | 4
xen/include/public/sysctl.h| 7 +++
xen/include/xen/livepatch.h| 4
xen/test/livepatch/patches/test1.patch | 13 +
9 files changed, 91 insertions(+), 1 deletion(-)
create mode 100644 xen/common/livepatch-test.c
create mode 100644 xen/test/livepatch/patches/test1.patch
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 2ef8b4e05422..83a00d4974dd 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2645,6 +2645,9 @@ int xc_livepatch_revert(xc_interface *xch, char *name,
uint32_t timeout, uint32_
int xc_livepatch_unload(xc_interface *xch, char *name, uint32_t timeout,
uint32_t flags);
int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout,
uint32_t flags);
+/* Dummy hypercall to test livepatch functionality. */
+int xc_livepatch_test(xc_interface *xch, uint32_t *result);
+
/*
* Ensure cache coherency after memory modifications. A call to this function
* is only required on ARM as the x86 architecture provides cache coherency
diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c
index 5ecdfa2c7934..0ca86a53d097 100644
--- a/tools/libs/ctrl/xc_misc.c
+++ b/tools/libs/ctrl/xc_misc.c
@@ -1021,6 +1021,20 @@ int xc_livepatch_replace(xc_interface *xch, char *name,
uint32_t timeout, uint32
return _xc_livepatch_action(xch, name, LIVEPATCH_ACTION_REPLACE, timeout,
flags);
}
+int xc_livepatch_test(xc_interface *xch, uint32_t *result)
+{
+struct xen_sysctl sysctl = {
+.cmd = XEN_SYSCTL_livepatch_op,
+.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_TEST,
+};
+int rc = do_sysctl(xch, );
+
+if ( !rc )
+*result = sysctl.u.livepatch.u.test.result;
+
+return rc;
+}
+
/*
* Local variables:
* mode: C
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 5bf9d9a32b65..5f6fd20d8814 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -37,6 +37,7 @@ void show_help(void)
" replace apply patch and revert all
others.\n"
" unload unload name patch.\n"
" load [flags]upload and apply with name as the
name\n"
+" test print the result of the test hypercall
(for testing purposes only)\n"
"Supported flags:\n"
" --nodeps Disable inter-module buildid dependency
check.\n"
" Check only against hypervisor
buildid.\n",
@@ -542,6 +543,29 @@ error:
return rc;
}
+static int test_func(int argc, char *argv[])
+{
+int rc;
+uint32_t result = 0;
+
+if ( argc != 0 )
+{
+show_help();
+return -1;
+}
+
+rc = xc_livepatch_test(xch, );
+if ( rc )
+{
+fprintf(stderr, "test operation failed: %s\n", strerror(errno));
+return -1;
+}
+
+printf("%u\n", result);
+
+return 0;
+}
+
/*
* These are also functions in action_options that are called in case
* none of the ones in main_options match.
@@ -554,6 +578,7 @@ struct {
{ "list", list_func },
{ "upload", upload_func },
{ "load", load_func },
+{ "test", test_func },
};
int main(int argc, char *argv[])
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 69d6aa626c7f..ab073d41f1d2 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -23,7 +23,7 @@ obj-y += kernel.o
obj-y += keyhandler.o
obj-$(CONFIG_KEXEC) += kexec.o
obj-$(CONFIG_KEXEC) += kimage.o
-obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o livepatch_elf.o livepatch-test.o
obj-$(CONFIG_MEM_ACCESS) += mem_access.o
obj-y += memory.o
obj-y += multicall.o
diff --git a/xen/common/livepatch-test.c
[PATCH 0/3] automation: add livepatch testing
Hello, The following series attempts to introduce some basic livepatch testing in the gitlab CI loop. Such testing will be more similar to what downstreams use, as the patch payload will be built using livepatch-build-tools. A sample gitlab run can be found at: https://gitlab.com/xen-project/people/royger/xen/-/pipelines/1082636267 The series introduces a new build and test steps. Thanks, Roger. Roger Pau Monne (3): automation/alpine: add elfutils-dev and coreutils for livepatch-build-tools livepatch: add a dummy hypercall for testing purposes automation: add x86-64 livepatching test automation/build/alpine/3.18.dockerfile | 4 + automation/gitlab-ci/build.yaml | 8 ++ automation/gitlab-ci/test.yaml| 8 ++ automation/scripts/build | 13 +++ .../scripts/qemu-alpine-x86_64-livepatch.sh | 79 +++ tools/include/xenctrl.h | 3 + tools/libs/ctrl/xc_misc.c | 14 tools/misc/xen-livepatch.c| 25 ++ xen/common/Makefile | 2 +- xen/common/livepatch-test.c | 20 + xen/common/livepatch.c| 4 + xen/include/public/sysctl.h | 7 ++ xen/include/xen/livepatch.h | 4 + xen/test/livepatch/patches/test1.patch| 13 +++ 14 files changed, 203 insertions(+), 1 deletion(-) create mode 100755 automation/scripts/qemu-alpine-x86_64-livepatch.sh create mode 100644 xen/common/livepatch-test.c create mode 100644 xen/test/livepatch/patches/test1.patch -- 2.43.0
Re: [PATCH] x86/HVM: limit upcall vector related verbosity
On 23/11/2023 11:01 am, Roger Pau Monné wrote: > On Thu, Nov 23, 2023 at 11:50:41AM +0100, Jan Beulich wrote: >> On 23.11.2023 11:47, Roger Pau Monné wrote: >>> On Thu, Nov 23, 2023 at 11:25:34AM +0100, Jan Beulich wrote: Avoid logging all-identical messages for every vCPU, but make sure to log unusual events like the vector differing from vCPU 0's (note that the respective condition also makes sure vCPU 0 itself will have the vector setting logged), or it changing after it was once set. (Arguably a downside is that some vCPU not having its vector set would no longer be recognizable from the logs. But I think that's tolerable as sufficiently unlikely outside of people actively fiddling with related code.) >>> Maybe we could consider printing unconditionally for debug builds? >> Indeed I was considering that, but it's primarily debug builds where the >> unnecessary redundancy is annoying me. (After all I work with debug builds >> much more than with release ones.) > I did find the message useful when doing guest bringup in the past, in > order to know the guest was correctly setting up the vector callbacks. > > I guess there are other ways to figure that out, or the message could > be added when people is doing bringup themselves. > > I find the save/restore messages during domain create much more > unhelpful and annoying that this. +1 I delete them in every debugging branch... As to this message, we ought to do something. However, the reason it's on every vCPU is because on Windows, there's no ability to request the same vector on every CPU, and it often does end up slightly different. I think the logic added is fine, although it could do with a small comment explaining why. Debugging of weird bugs by dmesg isn't helpful anyway - things like "did it have a vector upcall" should be answered by tools in dom0. ~Andrew
Re: [PATCH] x86/HVM: limit upcall vector related verbosity
On 23.11.2023 12:01, Roger Pau Monné wrote: > On Thu, Nov 23, 2023 at 11:50:41AM +0100, Jan Beulich wrote: >> On 23.11.2023 11:47, Roger Pau Monné wrote: >>> On Thu, Nov 23, 2023 at 11:25:34AM +0100, Jan Beulich wrote: Avoid logging all-identical messages for every vCPU, but make sure to log unusual events like the vector differing from vCPU 0's (note that the respective condition also makes sure vCPU 0 itself will have the vector setting logged), or it changing after it was once set. (Arguably a downside is that some vCPU not having its vector set would no longer be recognizable from the logs. But I think that's tolerable as sufficiently unlikely outside of people actively fiddling with related code.) >>> >>> Maybe we could consider printing unconditionally for debug builds? >> >> Indeed I was considering that, but it's primarily debug builds where the >> unnecessary redundancy is annoying me. (After all I work with debug builds >> much more than with release ones.) > > I did find the message useful when doing guest bringup in the past, in > order to know the guest was correctly setting up the vector callbacks. > > I guess there are other ways to figure that out, or the message could > be added when people is doing bringup themselves. > > I find the save/restore messages during domain create much more > unhelpful and annoying that this. Yeah, these too are ugly. But going through this save/restore cycle when starting a guest is supposed to go away anyway, according to Andrew. Plus the primary annoyance here is for PVH Dom0 with vCPU count not restricted, and there's no save/restore involved there. The typical HVM guest, otoh, wouldn't have as many vCPU-s ... Jan
Re: [PATCH] x86/HVM: limit upcall vector related verbosity
On Thu, Nov 23, 2023 at 11:50:41AM +0100, Jan Beulich wrote: > On 23.11.2023 11:47, Roger Pau Monné wrote: > > On Thu, Nov 23, 2023 at 11:25:34AM +0100, Jan Beulich wrote: > >> Avoid logging all-identical messages for every vCPU, but make sure to > >> log unusual events like the vector differing from vCPU 0's (note that > >> the respective condition also makes sure vCPU 0 itself will have the > >> vector setting logged), or it changing after it was once set. (Arguably > >> a downside is that some vCPU not having its vector set would no longer > >> be recognizable from the logs. But I think that's tolerable as > >> sufficiently unlikely outside of people actively fiddling with related > >> code.) > > > > Maybe we could consider printing unconditionally for debug builds? > > Indeed I was considering that, but it's primarily debug builds where the > unnecessary redundancy is annoying me. (After all I work with debug builds > much more than with release ones.) I did find the message useful when doing guest bringup in the past, in order to know the guest was correctly setting up the vector callbacks. I guess there are other ways to figure that out, or the message could be added when people is doing bringup themselves. I find the save/restore messages during domain create much more unhelpful and annoying that this. Thanks, Roger.
Re: [PATCH] xen/arm: gicv3: clean up GICD_CTRL write
On 23/11/2023 10:52, Julien Grall wrote: Hi Stewart, On 22/11/2023 14:46, Stewart Hildebrand wrote: GICD_CTL_ENABLE is a GICv2 bit. Remove it. The definitions of GICD_CTL_ENABLE and GICD_CTLR_ENABLE_G1 are identical, so the value written is unchanged. Thanks for spotting it. Maybe we should move GICv2 specific definitions out of gic.h. Anyway, this is a separate clean-up. So... Signed-off-by: Stewart Hildebrand ... Reviewed-by: Julien Grall And committed. Cheers, -- Julien Grall
Re: [PATCH] xen/arm: gicv3: clean up GICD_CTRL write
Hi Stewart, On 22/11/2023 14:46, Stewart Hildebrand wrote: GICD_CTL_ENABLE is a GICv2 bit. Remove it. The definitions of GICD_CTL_ENABLE and GICD_CTLR_ENABLE_G1 are identical, so the value written is unchanged. Thanks for spotting it. Maybe we should move GICv2 specific definitions out of gic.h. Anyway, this is a separate clean-up. So... Signed-off-by: Stewart Hildebrand ... Reviewed-by: Julien Grall Cheers, -- Julien Grall
Re: [PATCH] x86/HVM: limit upcall vector related verbosity
On 23.11.2023 11:47, Roger Pau Monné wrote: > On Thu, Nov 23, 2023 at 11:25:34AM +0100, Jan Beulich wrote: >> Avoid logging all-identical messages for every vCPU, but make sure to >> log unusual events like the vector differing from vCPU 0's (note that >> the respective condition also makes sure vCPU 0 itself will have the >> vector setting logged), or it changing after it was once set. (Arguably >> a downside is that some vCPU not having its vector set would no longer >> be recognizable from the logs. But I think that's tolerable as >> sufficiently unlikely outside of people actively fiddling with related >> code.) > > Maybe we could consider printing unconditionally for debug builds? Indeed I was considering that, but it's primarily debug builds where the unnecessary redundancy is annoying me. (After all I work with debug builds much more than with release ones.) Jan
xen | Successful pipeline for staging | 8f458625
Pipeline #1082563582 has passed! Project: xen ( https://gitlab.com/xen-project/xen ) Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging ) Commit: 8f458625 ( https://gitlab.com/xen-project/xen/-/commit/8f45862580c962791e66df716389bb5b1dd704df ) Commit Message: xen/xalloc: address violations of MISRA C:2012 ... Commit Author: Federico Serafini Committed by: Jan Beulich ( https://gitlab.com/jbeulich ) Pipeline #1082563582 ( https://gitlab.com/xen-project/xen/-/pipelines/1082563582 ) triggered by Ganis ( https://gitlab.com/ganis ) successfully completed 129 jobs in 3 stages. -- You're receiving this email because of your account on gitlab.com.
Re: [PATCH] x86/HVM: limit upcall vector related verbosity
On Thu, Nov 23, 2023 at 11:25:34AM +0100, Jan Beulich wrote: > Avoid logging all-identical messages for every vCPU, but make sure to > log unusual events like the vector differing from vCPU 0's (note that > the respective condition also makes sure vCPU 0 itself will have the > vector setting logged), or it changing after it was once set. (Arguably > a downside is that some vCPU not having its vector set would no longer > be recognizable from the logs. But I think that's tolerable as > sufficiently unlikely outside of people actively fiddling with related > code.) Maybe we could consider printing unconditionally for debug builds? Thanks, Roger.
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Thu, 2023-11-23 at 09:28 +, Paul Durrant wrote: > On 23/11/2023 00:07, Volodymyr Babchuk wrote: > > > > Hi, > > > > Volodymyr Babchuk writes: > > > > > Hi Stefano, > > > > > > Stefano Stabellini writes: > > > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: > > > > > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: > > > > > > On Wed, 22 Nov 2023, David Woodhouse wrote: > > > > > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > > > > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > > > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > > > > > > > From: Oleksandr Tyshchenko > > > > > > > > > > > > > > > > > > > > Instead of forcing the owner to domid 0, use > > > > > > > > > > XS_PRESERVE_OWNER to > > > > > > > > > > inherit the owner of the directory. > > > > > > > > > > > > > > > > > > Ah... so that's why the previous patch is there. > > > > > > > > > > > > > > > > > > This is not the right way to fix it. The QEMU Xen support is > > > > > > > > > *assuming* that > > > > > > > > > QEMU is either running in, or emulating, dom0. In the > > > > > > > > > emulation case this is > > > > > > > > > probably fine, but the 'real Xen' case it should be using the > > > > > > > > > correct domid > > > > > > > > > for node creation. I guess this could either be supplied on > > > > > > > > > the command line > > > > > > > > > or discerned by reading the local domain 'domid' node. > > > > > > > > > > > > > > > > yes, it should be passed as command line option to QEMU > > > > > > > > > > > > > > I'm not sure I like the idea of a command line option for > > > > > > > something > > > > > > > which QEMU could discover for itself. > > > > > > > > > > > > That's fine too. I meant to say "yes, as far as I know the toolstack > > > > > > passes the domid to QEMU as a command line option today". > > > > > > > > > > The -xen-domid argument on the QEMU command line today is the *guest* > > > > > domain ID, not the domain ID in which QEMU itself is running. > > > > > > > > > > Or were you thinking of something different? > > > > > > > > Ops, you are right and I understand your comment better now. The backend > > > > domid is not on the command line but it should be discoverable (on > > > > xenstore if I remember right). > > > > > > Yes, it is just "~/domid". I'll add a function that reads it. > > > > Just a quick question to QEMU folks: is it better to add a global > > variable where we will store own Domain ID or it will be okay to read > > domid from Xenstore every time we need it? > > > > If global variable variant is better, what is proffered place to define > > this variable? system/globals.c ? > > > > Actually... is it possible for QEMU just to use a relative path for the > backend nodes? That way it won't need to know it's own domid, will it? That covers some of the use cases, but it may also need to know its own domid for other purposes. Including writing the *absolute* path of the backend, to a frontend node? smime.p7s Description: S/MIME cryptographic signature
Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
Hi Nicola,
On 23/11/2023 10:40, Nicola Vetrini wrote:
On 2023-11-23 11:26, Julien Grall wrote:
Hi Nicola,
On 23/11/2023 09:25, Nicola Vetrini wrote:
On 2023-11-23 09:57, Jan Beulich wrote:
On 16.11.2023 10:08, Nicola Vetrini wrote:
The comment-based justifications for MISRA C:2012 Rule 8.4 are
replaced
by the asmlinkage pseudo-attribute, for the sake of uniformity.
Add missing 'xen/compiler.h' #include-s where needed.
The text in docs/misra/deviations.rst and docs/misra/safe.json
is modified to reflect this change.
Signed-off-by: Nicola Vetrini
---
This patch should be applied after patch 2 of this series.
The request made by Julien to update the wording is
contained in the present patch.
Along with this request he supplied you with an ack. Did you drop that
for a particular reason, or did you simply forget to record it here?
I forgot and added it in a reply.
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -28,6 +28,7 @@ asm (
#include "defs.h"
+#include
#include
#include
#include
@@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t
mbi_in, uint32_t video_out)
return mbi_out;
}
-/* SAF-1-safe */
-void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t
trampoline,
- uint32_t video_info)
+void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in,
+ uint32_t trampoline, uint32_t
video_info)
{
One purpose of asmlinkage is to possibly alter the default C calling
convention
to some more easy to use in assembly code. At least over a period of
time Linux
for example used that on ix86. If we decided to do something like
this, there
would be a collision with __stdcall. Hence I'm not convinced we can put
asmlinkage here. At which point the complete removal of SAF-1-safe
also would
need dropping.
Ok, so we can keep SAF-1 here and below and leave the wording in
deviations.rst and safe.sjon as is.
I guess you mean without this patch applied and not including my
proposed rewording (i.e. to deprecated SAF-1)?
If so, then we are back to the initial concern I raised. We would have
two ways to address the deviation. From a user perspective, it would
be unclear which one should be used.
The wording that is now on staging (committed by Stefano).
- Functions and variables used only by asm modules are marked with
the `asmlinkage` macro. Existing code may use a SAF-1-safe
textual deviation (see safe.json), but new code should not use
it.
I guess special cases can be dealt with as they arise. That may or may
not be mentioned in the deviation.
Ah so the wording has been updated! It is not 100% ideal if we decide to
keep some SAF-1 for good reason. But better than any ambiguity on when
to use one but not the other. We can reword once we have other cases.
Cheers,
--
Julien Grall
Re: [PATCH] x86/vPIC: correct vcpi_domain()
^ typo in the subject :).
On Thu, Nov 23, 2023 at 11:20:24AM +0100, Jan Beulich wrote:
> Make it use its parameter in both places.
>
> Fixes: 00a70f44a68c ("[HVM] Update VPIC device model for new interrupt
> delivery code")
> Signed-off-by: Jan Beulich
Acked-by: Roger Pau Monné
Thanks, Roger.
Re: [PATCH v2 4/5] arm/efi: Simplify efi_arch_handle_cmdline()
Hi Andrew, On 21/11/2023 20:15, Andrew Cooper wrote: -Wwrite-strings is unhappy with assigning "xen" to a mutable pointer, but this logic looks incorrect. It was inherited from the x86 side, where the logic was redundant and has now been removed. In the ARM case it inserts the image name into "xen,xen-bootargs" and there is no logic at all to strip this before parsing it as the command line. The absence of any logic to strip an image name suggests that it shouldn't exist there, or having a Xen image named e.g. "hmp-unsafe" in the filesystem is going to lead to some unexpected behaviour on boot. I have noticed this behavior a few years ago but never really looked why we added the name. So thanks for looking at it. No functional change. Signed-off-by: Andrew Cooper Reviewed-by: Julien Grall Cheers, -- Julien Grall
Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
On 2023-11-23 11:26, Julien Grall wrote:
Hi Nicola,
On 23/11/2023 09:25, Nicola Vetrini wrote:
On 2023-11-23 09:57, Jan Beulich wrote:
On 16.11.2023 10:08, Nicola Vetrini wrote:
The comment-based justifications for MISRA C:2012 Rule 8.4 are
replaced
by the asmlinkage pseudo-attribute, for the sake of uniformity.
Add missing 'xen/compiler.h' #include-s where needed.
The text in docs/misra/deviations.rst and docs/misra/safe.json
is modified to reflect this change.
Signed-off-by: Nicola Vetrini
---
This patch should be applied after patch 2 of this series.
The request made by Julien to update the wording is
contained in the present patch.
Along with this request he supplied you with an ack. Did you drop
that
for a particular reason, or did you simply forget to record it here?
I forgot and added it in a reply.
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -28,6 +28,7 @@ asm (
#include "defs.h"
+#include
#include
#include
#include
@@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t
mbi_in, uint32_t video_out)
return mbi_out;
}
-/* SAF-1-safe */
-void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t
trampoline,
- uint32_t video_info)
+void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in,
+ uint32_t trampoline, uint32_t
video_info)
{
One purpose of asmlinkage is to possibly alter the default C calling
convention
to some more easy to use in assembly code. At least over a period of
time Linux
for example used that on ix86. If we decided to do something like
this, there
would be a collision with __stdcall. Hence I'm not convinced we can
put
asmlinkage here. At which point the complete removal of SAF-1-safe
also would
need dropping.
Ok, so we can keep SAF-1 here and below and leave the wording in
deviations.rst and safe.sjon as is.
I guess you mean without this patch applied and not including my
proposed rewording (i.e. to deprecated SAF-1)?
If so, then we are back to the initial concern I raised. We would have
two ways to address the deviation. From a user perspective, it would be
unclear which one should be used.
The wording that is now on staging (committed by Stefano).
- Functions and variables used only by asm modules are marked with
the `asmlinkage` macro. Existing code may use a SAF-1-safe
textual deviation (see safe.json), but new code should not use
it.
I guess special cases can be dealt with as they arise. That may or may
not be mentioned in the deviation.
In particular, I would rather favor asmlinkage whenever it is possible
and only use SAF-1 when it is not possible to use it (like here).
Another possibility would be to deviate __stdcall like we do for
asmlinkage (I will let Jan confirm if this is desirable). With this
approach, there is less ambiguity when to use either of them.
Cheers,
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [PATCH v2 4/5] arm/efi: Simplify efi_arch_handle_cmdline()
On 23/11/2023 9:46 am, Luca Fancellu wrote: > >> On 22 Nov 2023, at 18:03, Andrew Cooper wrote: >> >> On 22/11/2023 3:49 pm, Luca Fancellu wrote: On 21 Nov 2023, at 20:41, Andrew Cooper wrote: On 21/11/2023 8:33 pm, Luca Fancellu wrote: > + CC henry > >> On 21 Nov 2023, at 20:15, Andrew Cooper >> wrote: >> >> -Wwrite-strings is unhappy with assigning "xen" to a mutable pointer, >> but this >> logic looks incorrect. It was inherited from the x86 side, where the >> logic >> was redundant and has now been removed. >> >> In the ARM case it inserts the image name into "xen,xen-bootargs" and >> there is >> no logic at all to strip this before parsing it as the command line. >> >> The absence of any logic to strip an image name suggests that it >> shouldn't >> exist there, or having a Xen image named e.g. "hmp-unsafe" in the >> filesystem >> is going to lead to some unexpected behaviour on boot. >> >> No functional change. >> >> 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: Roberto Bagnara >> CC: Nicola Vetrini >> >> v2: >> * New. >> >> I'm afraid that all of this reasoning is based on reading the source >> code. I >> don't have any way to try this out in a real ARM UEFI environment. > I will test this one tomorrow on an arm board >>> I confirm that booting though UEFI on an arm board works >>> >>> Reviewed-by: Luca Fancellu >>> Tested-by: Luca Fancellu >> Thanks, and you confirmed that the first cmdline parameter is still usable? > Yes, I’m using a xen.cfg like this: > > [global] > default=xen > > [xen] > options=noreboot dom0_mem=4096M bootscrub=0 iommu=on loglvl=error > guest_loglvl=error > kernel=Image console=hvc0 earlycon=xenboot rootwait > root=PARTUUID=6a60524d-061d-454a-bfd1-38989910eccd > > And in Xen boot I can see this: > > [...] > (XEN) Command line: noreboot dom0_mem=4096M bootscrub=0 iommu=on > loglvl=error guest_loglvl=error > [...] > > So I think it’s passing every parameter Thankyou all for the testing. ~Andrew
Re: [PATCH] x86/vPIC: correct vcpi_domain()
On 23/11/2023 10:20 am, Jan Beulich wrote:
> Make it use its parameter in both places.
>
> Fixes: 00a70f44a68c ("[HVM] Update VPIC device model for new interrupt
> delivery code")
> Signed-off-by: Jan Beulich
Reviewed-by: Andrew Cooper
Re: [PATCH] x86emul/test: fold AVX512VL scatter/gather test blobs with AVX512F ones
On 29/08/2023 11:53 am, Jan Beulich wrote: > Everywhere else the VL tests are grouped with the basic ones, > distinguished simply by the "form" specifiers. > > No change to the generated test blobs, and hence no functional change. > > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper
Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
On 23.11.2023 11:26, Julien Grall wrote: > Another possibility would be to deviate __stdcall like we do for > asmlinkage (I will let Jan confirm if this is desirable). With this > approach, there is less ambiguity when to use either of them. Attributes changing calling convention may be a sign of there being interfacing with assembly code, but e.g. EFIAPI generally doesn't (it only happens to be so in the specific case here). Jan
Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute
Hi Nicola,
On 23/11/2023 09:25, Nicola Vetrini wrote:
On 2023-11-23 09:57, Jan Beulich wrote:
On 16.11.2023 10:08, Nicola Vetrini wrote:
The comment-based justifications for MISRA C:2012 Rule 8.4 are replaced
by the asmlinkage pseudo-attribute, for the sake of uniformity.
Add missing 'xen/compiler.h' #include-s where needed.
The text in docs/misra/deviations.rst and docs/misra/safe.json
is modified to reflect this change.
Signed-off-by: Nicola Vetrini
---
This patch should be applied after patch 2 of this series.
The request made by Julien to update the wording is
contained in the present patch.
Along with this request he supplied you with an ack. Did you drop that
for a particular reason, or did you simply forget to record it here?
I forgot and added it in a reply.
--- a/xen/arch/x86/boot/reloc.c
+++ b/xen/arch/x86/boot/reloc.c
@@ -28,6 +28,7 @@ asm (
#include "defs.h"
+#include
#include
#include
#include
@@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t
mbi_in, uint32_t video_out)
return mbi_out;
}
-/* SAF-1-safe */
-void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t trampoline,
- uint32_t video_info)
+void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in,
+ uint32_t trampoline, uint32_t
video_info)
{
One purpose of asmlinkage is to possibly alter the default C calling
convention
to some more easy to use in assembly code. At least over a period of
time Linux
for example used that on ix86. If we decided to do something like
this, there
would be a collision with __stdcall. Hence I'm not convinced we can put
asmlinkage here. At which point the complete removal of SAF-1-safe
also would
need dropping.
Ok, so we can keep SAF-1 here and below and leave the wording in
deviations.rst and safe.sjon as is.
I guess you mean without this patch applied and not including my
proposed rewording (i.e. to deprecated SAF-1)?
If so, then we are back to the initial concern I raised. We would have
two ways to address the deviation. From a user perspective, it would be
unclear which one should be used.
In particular, I would rather favor asmlinkage whenever it is possible
and only use SAF-1 when it is not possible to use it (like here).
Another possibility would be to deviate __stdcall like we do for
asmlinkage (I will let Jan confirm if this is desirable). With this
approach, there is less ambiguity when to use either of them.
Cheers,
--
Julien Grall
[PATCH] x86/HVM: limit upcall vector related verbosity
Avoid logging all-identical messages for every vCPU, but make sure to log unusual events like the vector differing from vCPU 0's (note that the respective condition also makes sure vCPU 0 itself will have the vector setting logged), or it changing after it was once set. (Arguably a downside is that some vCPU not having its vector set would no longer be recognizable from the logs. But I think that's tolerable as sufficiently unlikely outside of people actively fiddling with related code.) Signed-off-by: Jan Beulich --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -4129,7 +4129,10 @@ static int hvmop_set_evtchn_upcall_vecto if ( (v = domain_vcpu(d, op.vcpu)) == NULL ) return -ENOENT; -printk(XENLOG_G_INFO "%pv: upcall vector %02x\n", v, op.vector); +if ( op.vector != d->vcpu[0]->arch.hvm.evtchn_upcall_vector || + (v->arch.hvm.evtchn_upcall_vector && + op.vector != v->arch.hvm.evtchn_upcall_vector) ) +printk(XENLOG_G_INFO "%pv: upcall vector %02x\n", v, op.vector); v->arch.hvm.evtchn_upcall_vector = op.vector; hvm_assert_evtchn_irq(v);
[PATCH] x86/vPIC: correct vcpi_domain()
Make it use its parameter in both places.
Fixes: 00a70f44a68c ("[HVM] Update VPIC device model for new interrupt delivery
code")
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/hvm/vpic.c
+++ b/xen/arch/x86/hvm/vpic.c
@@ -35,7 +35,7 @@
#include
#define vpic_domain(v) (container_of((v), struct domain, \
-arch.hvm.vpic[!vpic->is_master]))
+ arch.hvm.vpic[!(v)->is_master]))
#define __vpic_lock(v) _of((v), struct hvm_domain, \
vpic[!(v)->is_master])->irq_lock
#define vpic_lock(v) spin_lock(__vpic_lock(v))
Re: [PATCH] x86emul/test: fold AVX512VL scatter/gather test blobs with AVX512F ones
On 29.08.2023 12:53, Jan Beulich wrote:
> Everywhere else the VL tests are grouped with the basic ones,
> distinguished simply by the "form" specifiers.
>
> No change to the generated test blobs, and hence no functional change.
>
> Signed-off-by: Jan Beulich
Any chance of an ack for this purely mechanical tidying?
Thanks, Jan
> --- a/tools/tests/x86_emulator/Makefile
> +++ b/tools/tests/x86_emulator/Makefile
> @@ -18,7 +18,7 @@ CFLAGS += $(CFLAGS_xeninclude)
>
> SIMD := 3dnow sse sse2 sse4 avx avx2 xop avx512f avx512bw avx512dq avx512er
> avx512vbmi avx512fp16
> FMA := fma4 fma
> -SG := avx2-sg avx512f-sg avx512vl-sg
> +SG := avx2-sg avx512f-sg
> AES := ssse3-aes avx-aes avx2-vaes avx512bw-vaes
> CLMUL := ssse3-pclmul avx-pclmul avx2-vpclmulqdq avx512bw-vpclmulqdq
> avx512vbmi2-vpclmulqdq
> SHA := sse4-sha avx-sha avx512f-sha
> @@ -70,14 +70,10 @@ xop-flts := $(avx-flts)
> avx512f-vecs := 64 16 32
> avx512f-ints := 4 8
> avx512f-flts := 4 8
> -avx512f-sg-vecs := 64
> +avx512f-sg-vecs := $(avx512f-vecs)
> avx512f-sg-idxs := 4 8
> avx512f-sg-ints := $(avx512f-ints)
> avx512f-sg-flts := $(avx512f-flts)
> -avx512vl-sg-vecs := 16 32
> -avx512vl-sg-idxs := $(avx512f-sg-idxs)
> -avx512vl-sg-ints := $(avx512f-ints)
> -avx512vl-sg-flts := $(avx512f-flts)
> avx512bw-vecs := $(avx512f-vecs)
> avx512bw-ints := 1 2
> avx512bw-flts :=
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -34,7 +34,6 @@ asm ( ".pushsection .test, \"ax\", @prog
> #include "avx512f.h"
> #include "avx512f-sg.h"
> #include "avx512f-sha.h"
> -#include "avx512vl-sg.h"
> #include "avx512bw.h"
> #include "avx512bw-vaes.h"
> #include "avx512bw-vpclmulqdq.h"
> @@ -462,22 +461,22 @@ static const struct {
> AVX512VL(VL u64x2,avx512f, 16u8),
> AVX512VL(VL s64x4,avx512f, 32i8),
> AVX512VL(VL u64x4,avx512f, 32u8),
> -SIMD(AVX512VL S/G f32[4x32], avx512vl_sg, 16x4f4),
> -SIMD(AVX512VL S/G f64[2x32], avx512vl_sg, 16x4f8),
> -SIMD(AVX512VL S/G f32[2x64], avx512vl_sg, 16x8f4),
> -SIMD(AVX512VL S/G f64[2x64], avx512vl_sg, 16x8f8),
> -SIMD(AVX512VL S/G f32[8x32], avx512vl_sg, 32x4f4),
> -SIMD(AVX512VL S/G f64[4x32], avx512vl_sg, 32x4f8),
> -SIMD(AVX512VL S/G f32[4x64], avx512vl_sg, 32x8f4),
> -SIMD(AVX512VL S/G f64[4x64], avx512vl_sg, 32x8f8),
> -SIMD(AVX512VL S/G i32[4x32], avx512vl_sg, 16x4i4),
> -SIMD(AVX512VL S/G i64[2x32], avx512vl_sg, 16x4i8),
> -SIMD(AVX512VL S/G i32[2x64], avx512vl_sg, 16x8i4),
> -SIMD(AVX512VL S/G i64[2x64], avx512vl_sg, 16x8i8),
> -SIMD(AVX512VL S/G i32[8x32], avx512vl_sg, 32x4i4),
> -SIMD(AVX512VL S/G i64[4x32], avx512vl_sg, 32x4i8),
> -SIMD(AVX512VL S/G i32[4x64], avx512vl_sg, 32x8i4),
> -SIMD(AVX512VL S/G i64[4x64], avx512vl_sg, 32x8i8),
> +SIMD(AVX512VL S/G f32[4x32], avx512f_sg, 16x4f4),
> +SIMD(AVX512VL S/G f64[2x32], avx512f_sg, 16x4f8),
> +SIMD(AVX512VL S/G f32[2x64], avx512f_sg, 16x8f4),
> +SIMD(AVX512VL S/G f64[2x64], avx512f_sg, 16x8f8),
> +SIMD(AVX512VL S/G f32[8x32], avx512f_sg, 32x4f4),
> +SIMD(AVX512VL S/G f64[4x32], avx512f_sg, 32x4f8),
> +SIMD(AVX512VL S/G f32[4x64], avx512f_sg, 32x8f4),
> +SIMD(AVX512VL S/G f64[4x64], avx512f_sg, 32x8f8),
> +SIMD(AVX512VL S/G i32[4x32], avx512f_sg, 16x4i4),
> +SIMD(AVX512VL S/G i64[2x32], avx512f_sg, 16x4i8),
> +SIMD(AVX512VL S/G i32[2x64], avx512f_sg, 16x8i4),
> +SIMD(AVX512VL S/G i64[2x64], avx512f_sg, 16x8i8),
> +SIMD(AVX512VL S/G i32[8x32], avx512f_sg, 32x4i4),
> +SIMD(AVX512VL S/G i64[4x32], avx512f_sg, 32x4i8),
> +SIMD(AVX512VL S/G i32[4x64], avx512f_sg, 32x8i4),
> +SIMD(AVX512VL S/G i64[4x64], avx512f_sg, 32x8i8),
> SIMD(AVX512BW s8x64, avx512bw, 64i1),
> SIMD(AVX512BW u8x64, avx512bw, 64u1),
> SIMD(AVX512BW s16x32,avx512bw, 64i2),
>
[PATCH v2] livepatch: do not use .livepatch.funcs section to store internal state
Currently the livepatch logic inside of Xen will use fields of struct
livepatch_func in order to cache internal state of patched functions. Note
this is a field that is part of the payload, and is loaded as an ELF section
(.livepatch.funcs), taking into account the SHF_* flags in the section
header.
The flags for the .livepatch.funcs section, as set by livepatch-build-tools,
are SHF_ALLOC, which leads to its contents (the array of livepatch_func
structures) being placed in read-only memory:
Section Headers:
[Nr] Name Type Address Offset
Size EntSize Flags Link Info Align
[...]
[ 4] .livepatch.funcs PROGBITS 0080
0068 A 0 0 8
This previously went unnoticed, as all writes to the fields of livepatch_func
happen in the critical region that had WP disabled in CR0. After 8676092a0f16
however WP is no longer toggled in CR0 for patch application, and only the
hypervisor .text mappings are made write-accessible. That leads to the
following page fault when attempting to apply a livepatch:
[ Xen-4.19-unstable x86_64 debug=y Tainted: C]
CPU:4
RIP:e008:[] common/livepatch.c#apply_payload+0x45/0x1e1
[...]
Xen call trace:
[] R common/livepatch.c#apply_payload+0x45/0x1e1
[] F check_for_livepatch_work+0x385/0xaa5
[] F arch/x86/domain.c#idle_loop+0x92/0xee
Pagetable walk from 82d040625079:
L4[0x105] = 8c6c9063
L3[0x141] = 8c6c6063
L2[0x003] = 00086a1e7063
L1[0x025] = 80086ca5d121
Panic on CPU 4:
FATAL PAGE FAULT
[error_code=0003]
Faulting linear address: 82d040625079
Fix this by moving the internal Xen function patching state out of
livepatch_func into an area not allocated as part of the ELF payload. While
there also constify the array of livepatch_func structures in order to prevent
further surprises.
Note there's still one field (old_addr) that gets set during livepatch load. I
consider this fine since the field is read-only after load, and at the point
the field gets set the underlying mapping hasn't been made read-only yet.
Fixes: 8676092a0f16 ('x86/livepatch: Fix livepatch application when CET is
active')
Signed-off-by: Roger Pau Monné
---
Changes since v1:
- Rename opaque state field to insn_buffer.
---
xen/arch/arm/arm32/livepatch.c | 9 ---
xen/arch/arm/arm64/livepatch.c | 9 ---
xen/arch/arm/livepatch.c| 9 ---
xen/arch/x86/livepatch.c| 26 +++-
xen/common/livepatch.c | 25 +--
xen/include/public/sysctl.h | 5 +---
xen/include/xen/livepatch.h | 38 -
xen/include/xen/livepatch_payload.h | 3 ++-
8 files changed, 76 insertions(+), 48 deletions(-)
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index 3c50283b2ab7..80d2659b7861 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -11,23 +11,24 @@
#include
#include
-void arch_livepatch_apply(struct livepatch_func *func)
+void arch_livepatch_apply(const struct livepatch_func *func,
+ struct livepatch_fstate *state)
{
uint32_t insn;
uint32_t *new_ptr;
unsigned int i, len;
-BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE > sizeof(func->opaque));
+BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE > sizeof(state->insn_buffer));
BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != sizeof(insn));
ASSERT(vmap_of_xen_text);
-len = livepatch_insn_len(func);
+len = livepatch_insn_len(func, state);
if ( !len )
return;
/* Save old ones. */
-memcpy(func->opaque, func->old_addr, len);
+memcpy(state->insn_buffer, func->old_addr, len);
if ( func->new_addr )
{
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 62d2ef373a0e..df2cebeddefb 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -15,23 +15,24 @@
#include
#include
-void arch_livepatch_apply(struct livepatch_func *func)
+void arch_livepatch_apply(const struct livepatch_func *func,
+ struct livepatch_fstate *state)
{
uint32_t insn;
uint32_t *new_ptr;
unsigned int i, len;
-BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE > sizeof(func->opaque));
+BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE > sizeof(state->insn_buffer));
BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != sizeof(insn));
ASSERT(vmap_of_xen_text);
-len = livepatch_insn_len(func);
+len = livepatch_insn_len(func, state);
if ( !len )
return;
/* Save old ones. */
-memcpy(func->opaque, func->old_addr, len);
+memcpy(state->insn_buffer, func->old_addr, len);
if ( func->new_addr )
Re: [PATCH v2 4/5] arm/efi: Simplify efi_arch_handle_cmdline()
> On 22 Nov 2023, at 18:03, Andrew Cooper wrote: > > On 22/11/2023 3:49 pm, Luca Fancellu wrote: >> >>> On 21 Nov 2023, at 20:41, Andrew Cooper wrote: >>> >>> On 21/11/2023 8:33 pm, Luca Fancellu wrote: + CC henry > On 21 Nov 2023, at 20:15, Andrew Cooper wrote: > > -Wwrite-strings is unhappy with assigning "xen" to a mutable pointer, but > this > logic looks incorrect. It was inherited from the x86 side, where the > logic > was redundant and has now been removed. > > In the ARM case it inserts the image name into "xen,xen-bootargs" and > there is > no logic at all to strip this before parsing it as the command line. > > The absence of any logic to strip an image name suggests that it shouldn't > exist there, or having a Xen image named e.g. "hmp-unsafe" in the > filesystem > is going to lead to some unexpected behaviour on boot. > > No functional change. > > 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: Roberto Bagnara > CC: Nicola Vetrini > > v2: > * New. > > I'm afraid that all of this reasoning is based on reading the source > code. I > don't have any way to try this out in a real ARM UEFI environment. I will test this one tomorrow on an arm board >> I confirm that booting though UEFI on an arm board works >> >> Reviewed-by: Luca Fancellu >> Tested-by: Luca Fancellu > > Thanks, and you confirmed that the first cmdline parameter is still usable? Yes, I’m using a xen.cfg like this: [global] default=xen [xen] options=noreboot dom0_mem=4096M bootscrub=0 iommu=on loglvl=error guest_loglvl=error kernel=Image console=hvc0 earlycon=xenboot rootwait root=PARTUUID=6a60524d-061d-454a-bfd1-38989910eccd And in Xen boot I can see this: [...] (XEN) Command line: noreboot dom0_mem=4096M bootscrub=0 iommu=on loglvl=error guest_loglvl=error [...] So I think it’s passing every parameter > > ~Andrew
[xen-unstable test] 183831: regressions - FAIL
flight 183831 xen-unstable real [real] flight 183836 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/183831/ http://logs.test-lab.xenproject.org/osstest/logs/183836/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. vs. 183807 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183807 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 183807 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183807 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 183807 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 183807 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 183807 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 183807 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183807 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 183807 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183807 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 183807 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183807 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-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-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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-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-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass version targeted for testing: xen c22fe7213c9b1f99cbc64c33e391afa223f9cd08 baseline version: xen fa2da5bce90b3777aa7a323e1cf201c97b56d278 Last test of basis 183807
Re: [PATCH] x86/x2apic: introduce a mixed physical/cluster mode
On Tue, Nov 21, 2023 at 04:56:47PM -0800, Elliott Mitchell wrote: > It was insisted that full logs be sent to xen-devel. Perhaps I am > paranoid, but I doubt I would have been successful at scrubbing all > hardware serial numbers. As such, I was willing to post substantial > sections, but not everything. Can you please point out which hardware serial numbers are you referring to, and where are they printed in Xen dmesg? Thanks, Roger.
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
On 22/11/2023 23:04, David Woodhouse wrote: On Wed, 2023-11-22 at 22:56 +, Volodymyr Babchuk wrote: Paul Durrant writes: On 21/11/2023 22:10, Volodymyr Babchuk wrote: From: David Woodhouse This allows a XenDevice implementation to know whether it was created by QEMU, or merely discovered in XenStore after the toolstack created it. This will allow us to create frontend/backend nodes only when we should, rather than unconditionally attempting to overwrite them from a driver domain which doesn't have privileges to do so. As an added benefit, it also means we no longer have to call the xen_backend_set_device() function from the device models immediately after calling qdev_realize_and_unref(). Even though we could make the argument that it's safe to do so, and the pointer to the unreffed device *will* actually still be valid, it still made my skin itch to look at it. Signed-off-by: David Woodhouse --- hw/block/xen-block.c | 3 +-- hw/char/xen_console.c | 2 +- hw/net/xen_nic.c | 2 +- hw/xen/xen-bus.c | 4 include/hw/xen/xen-backend.h | 2 -- include/hw/xen/xen-bus.h | 2 ++ 6 files changed, 9 insertions(+), 6 deletions(-) Actually, I think you should probably update xen_backend_try_device_destroy() in this patch too. It currently uses xen_backend_list_find() to check whether the specified XenDevice has an associated XenBackendInstance. Sure. Looks like it is the only user of xen_backend_list_find(), so we can get rid of it as well. I'll drop your R-b tag, because of those additional changes in the new version. I think it's fine to keep. He told me to do it :) I confirm that :-)
