[xen-4.17-testing test] 182095: tolerable FAIL - PUSHED
flight 182095 xen-4.17-testing real [real] flight 182109 xen-4.17-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/182095/ http://logs.test-lab.xenproject.org/osstest/logs/182109/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-qemut-rhel6hvm-amd 14 guest-start/redhat.repeat fail pass in 182109-retest Tests which did not succeed, but are not blocking: test-amd64-i386-migrupgrade 10 xen-install/src_host fail like 182016 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 182016 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 182016 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 182016 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 182016 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 182016 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 182016 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 182016 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 182016 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 182016 test-amd64-i386-xl-pvshim14 guest-start fail 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-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-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-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 16 saverestore-support-check fail starved in 182016 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail starved in 182016 test-armhf-armhf-libvirt-raw 15 saverestore-support-check fail starved in 182016 version targeted for testing: xen c18b2dd93f396eadd116d0016cf7285523a13505 baseline version: xen
Re: [PATCH v2] arm32: Avoid using solaris syntax for .section directive
Hi, On 01/08/2023 19:49, Khem Raj wrote: > > > Assembler from binutils 2.41 rejects [1] this syntax > > .section "name"[, flags...] > > where flags could be #alloc, #write, #execinstr, #exclude, and #tls [2] > > It is almost like a regression compared to 2.40 or older release, > It likely went unnoticed so far because Linux kernel changed > to GNU syntax already in 5.5, to allow building with Clang's > integrated assembler. > > Switch to using GNU syntax > > .section name[, "flags"[, @type]] > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=11601 > [2] https://sourceware.org/binutils/docs-2.41/as.html#Section > > Signed-off-by: Khem Raj With the changes suggested by Julien: Reviewed-by: Michal Orzel ~Michal
RE: [PATCH 2/2] fdt: make fdt handling reusable across arch
Hi Daniel,
> -Original Message-
> Subject: [PATCH 2/2] fdt: make fdt handling reusable across arch
>
> This refactors reusable code from Arm's bootfdt.c and device-tree.h that is
> general fdt handling code. The Kconfig parameter CORE_DEVICE_TREE is
> introduced for when the ability of parsing DTB files is needed by a capability
> such as hyperlaunch.
>
> Signed-off-by: Daniel P. Smith
> ---
> MAINTAINERS | 2 +
> xen/arch/arm/bootfdt.c| 141 +--
> xen/common/Kconfig| 4 +
> xen/common/Makefile | 3 +-
> xen/common/fdt.c | 153 ++
> xen/include/xen/device_tree.h | 50 +--
> xen/include/xen/fdt.h | 79 ++
> 7 files changed, 242 insertions(+), 190 deletions(-)
> create mode 100644 xen/common/fdt.c
> create mode 100644 xen/include/xen/fdt.h
>
> +void __init device_tree_get_reg(
> +const __be32 **cell, uint32_t address_cells, uint32_t size_cells,
> +paddr_t *start, paddr_t *size)
> +{
> +uint64_t dt_start, dt_size;
> +
> +/*
> + * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
> + * Thus, there is an implicit cast from uint64_t to paddr_t.
> + */
> +dt_start = dt_next_cell(address_cells, cell);
> +dt_size = dt_next_cell(size_cells, cell);
> +
> +if ( dt_start != (paddr_t)dt_start )
> +{
> +printk("Physical address greater than max width supported\n");
> +WARN();
> +}
> +
> +if ( dt_size != (paddr_t)dt_size )
> +{
> +printk("Physical size greater than max width supported\n");
> +WARN();
> +}
> +
> +/*
> + * Xen will truncate the address/size if it is greater than the maximum
> + * supported width and it will give an appropriate warning.
> + */
> +*start = dt_start;
> +*size = dt_size;
> +}
> +
> +u32 __init device_tree_get_u32(
> +const void *fdt, int node, const char *prop_name, u32 dflt)
> +{
> +const struct fdt_property *prop;
> +
> +prop = fdt_get_property(fdt, node, prop_name, NULL);
> +if ( !prop || prop->len < sizeof(u32) )
> +return dflt;
> +
> +return fdt32_to_cpu(*(uint32_t*)prop->data);
> +}
When I tested this patch by our internal CI (I applied this patch on top
of today's staging:
c2026b88b5 xen/arm/IRQ: uniform irq_set_affinity() with x86 version),
there are some build errors as below:
aarch64-linux-gnu-gcc -MMD -MP -MF ./.asm-offsets.s.d -DBUILD_ID
-fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes
-Wdeclaration-after-statement -Wno-unused-but-set-variable
-Wno-unused-local-typedefs -O2 -fomit-frame-pointer -nostdinc -fno-builtin
-fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__
-include /jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/config.h
-mcpu=generic -mgeneral-regs-only -mno-outline-atomics -I./include
-I./arch/arm/include
-I/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include
-I/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include -fno-pie
-fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables
-Wnested-externs -S -g0 -o asm-offsets.s.new -MQ asm-offsets.s
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c
In file included from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c:13:
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include/asm/setup.h:162:6:
error: redundant redeclaration of 'device_tree_get_reg'
[-Werror=redundant-decls]
162 | void device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
| ^~~
In file included from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/device_tree.h:17,
from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include/asm/smp.h:6,
from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/smp.h:4,
from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/rwlock.h:6,
from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/sched.h:7,
from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c:9:
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/include/xen/fdt.h:64:13:
note: previous declaration of 'device_tree_get_reg' with type 'void(const
__be32 **, u32, u32, u64 *, u64 *)' {aka 'void(const unsigned int **,
unsigned int, unsigned int, long unsigned int *, long unsigned int *)'}
64 | void __init device_tree_get_reg(
| ^~~
In file included from
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/arm64/asm-offsets.c:13:
/jenkins/workspace/ais-xenbits-xen/layers/xen/xen/arch/arm/include/asm/setup.h:165:5:
error: redundant redeclaration of 'device_tree_get_u32'
[-Werror=redundant-decls]
165 | u32 devi
Re: [PATCH v3 21/25] tools/xenstore: introduce read_node_nocopy()
On 02.08.23 00:00, Julien Grall wrote:
Hi Juergen,
Title: I can't find read_node_nocopy(). I found a read_node_const(). Which name
did you intend to use?
Oh, sorry for that. I think read_node_const() is the better choice.
On 24/07/2023 12:02, Juergen Gross wrote:
+static int read_node_helper(struct connection *conn, struct node *node)
+{
/* Data is binary blob (usually ascii, no nul). */
- node->data = node->perms + hdr->num_perms;
+ node->data = node->perms + node->hdr.num_perms;
/* Children is strings, nul separated. */
node->children = node->data + node->hdr.datalen;
if (domain_adjust_node_perms(node))
- goto error;
+ return -1;
You are either returning 0 or -1 which is then only used in if ( ... ). Can we
simply return a boolean (true for success, false for a failure)?
Fine with me.
The rest LGTM.
Thanks,
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v3 20/25] tools/xenstore: alloc new memory in domain_adjust_node_perms()
On 01.08.23 23:46, Julien Grall wrote:
Hi Juergen,
On 24/07/2023 12:02, Juergen Gross wrote:
In order to avoid modifying the node data in the data base in case a
domain is gone, let domain_adjust_node_perms() allocate new memory for
the permissions in case they need to be modified. As this should
happen only in very rare cases, it is fine to do this even when having
copied the node data already.
Signed-off-by: Juergen Gross
---
V3:
- new patch
---
tools/xenstore/xenstored_core.c | 10 +-
tools/xenstore/xenstored_domain.c | 19 +++
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 404ecd0c62..ea3d20a372 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -751,6 +751,11 @@ struct node *read_node(struct connection *conn, const
void *ctx,
goto error;
}
+ /* Data is binary blob (usually ascii, no nul). */
+ node->data = node->perms + hdr->num_perms;
+ /* Children is strings, nul separated. */
+ node->children = node->data + node->hdr.datalen;
+
It took me a while to understand why you move the lines above. I tihnk it would
be worth documenting in the code (possibly on top of the declaration
domain_adjust_node_perms()) that domain_adjust_node_perms() may re-allocate the
permissions.
Okay.
if (domain_adjust_node_perms(node))
goto error;
@@ -758,11 +763,6 @@ struct node *read_node(struct connection *conn, const
void *ctx,
if (node->acc.domid != get_node_owner(node))
node->acc.memory = 0;
- /* Data is binary blob (usually ascii, no nul). */
- node->data = node->perms + hdr->num_perms;
- /* Children is strings, nul separated. */
- node->children = node->data + node->hdr.datalen;
-
if (access_node(conn, node, NODE_ACCESS_READ, NULL))
goto error;
diff --git a/tools/xenstore/xenstored_domain.c
b/tools/xenstore/xenstored_domain.c
index fdf1095acb..cdef6efef4 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1334,13 +1334,24 @@ int domain_alloc_permrefs(struct node_perms *perms)
int domain_adjust_node_perms(struct node *node)
{
unsigned int i;
+ struct xs_permissions *perms = node->perms;
+ bool copied = false;
for (i = 1; i < node->hdr.num_perms; i++) {
- if (node->perms[i].perms & XS_PERM_IGNORE)
+ if ((perms[i].perms & XS_PERM_IGNORE) ||
+ chk_domain_generation(perms[i].id, node->hdr.generation))
continue;
- if (!chk_domain_generation(node->perms[i].id,
- node->hdr.generation))
- node->perms[i].perms |= XS_PERM_IGNORE;
+
+ if (!copied) {
This wants a coment explain why you need to copy it.
Okay.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v3 19/25] tools/xenstore: use struct node_hdr in struct node
On 01.08.23 23:34, Julien Grall wrote:
Hi Juergen,
On 24/07/2023 12:02, Juergen Gross wrote:
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c72fc0c725..404ecd0c62 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -555,6 +555,12 @@ static void initialize_fds(int *p_sock_pollfd_idx, int
*ptimeout)
}
}
+static size_t calc_node_acc_size(struct node_hdr *hdr)
The parameter can be const.
I've changed this already in my pending V4. :-)
+{
+ return sizeof(*hdr) + hdr->num_perms * sizeof(struct xs_permissions) +
+ hdr->datalen + hdr->childlen;
+}
+
[...]
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 9cb4c2f3eb..adf8a785fc 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -181,6 +181,7 @@ extern struct list_head connections;
*/
struct node_hdr {
uint64_t generation;
+#define NO_GENERATION ~((uint64_t)0)
uint16_t num_perms;
uint16_t datalen;
uint32_t childlen;
@@ -197,6 +198,10 @@ struct node_account_data {
};
struct node {
+ /* Data direct for data base. */
I can't parse it. Did you mean 'from' rather than 'for'?
I'll change it to:
/* Copied to/from data base. */
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v3 18/25] tools/xenstore: don't use struct node_perms in struct node
On 01.08.23 23:29, Julien Grall wrote:
Hi Juergen,
On 24/07/2023 12:02, Juergen Gross wrote:
Open code struct node_perms in struct node in order to prepare using
struct node_hdr in struct node.
Add two helpers to transfer permissions between struct node and struct
node_perms.
Signed-off-by: Juergen Gross
---
V2:
- new patch
---
tools/xenstore/xenstored_core.c | 76 ++
tools/xenstore/xenstored_core.h | 21 ++-
tools/xenstore/xenstored_domain.c | 13 ++---
tools/xenstore/xenstored_transaction.c | 8 +--
tools/xenstore/xenstored_watch.c | 7 ++-
5 files changed, 75 insertions(+), 50 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 86b7c9bf36..c72fc0c725 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -735,7 +735,7 @@ struct node *read_node(struct connection *conn, const void
*ctx,
/* Datalen, childlen, number of permissions */
node->generation = hdr->generation;
- node->perms.num = hdr->num_perms;
+ node->num_perms = hdr->num_perms;
node->datalen = hdr->datalen;
node->childlen = hdr->childlen;
node->acc.domid = perms_from_node_hdr(hdr)->id;
@@ -743,8 +743,8 @@ struct node *read_node(struct connection *conn, const void
*ctx,
/* Copy node data to new memory area, starting with permissions. */
size -= sizeof(*hdr);
- node->perms.p = talloc_memdup(node, perms_from_node_hdr(hdr), size);
- if (node->perms.p == NULL) {
+ node->perms = talloc_memdup(node, perms_from_node_hdr(hdr), size);
+ if (node->perms == NULL) {
errno = ENOMEM;
goto error;
}
@@ -757,7 +757,7 @@ struct node *read_node(struct connection *conn, const void
*ctx,
node->acc.memory = 0;
/* Data is binary blob (usually ascii, no nul). */
- node->data = node->perms.p + hdr->num_perms;
+ node->data = node->perms + hdr->num_perms;
/* Children is strings, nul separated. */
node->children = node->data + node->datalen;
@@ -796,7 +796,7 @@ int write_node_raw(struct connection *conn, const char
*db_name,
return errno;
size = sizeof(*hdr)
- + node->perms.num * sizeof(node->perms.p[0])
+ + node->num_perms * sizeof(node->perms[0])
+ node->datalen + node->childlen;
/* Call domain_max_chk() in any case in order to record max values. */
@@ -813,13 +813,13 @@ int write_node_raw(struct connection *conn, const char
*db_name,
hdr = data;
hdr->generation = node->generation;
- hdr->num_perms = node->perms.num;
+ hdr->num_perms = node->num_perms;
hdr->datalen = node->datalen;
hdr->childlen = node->childlen;
p = perms_from_node_hdr(hdr);
- memcpy(p, node->perms.p, node->perms.num * sizeof(*node->perms.p));
- p += node->perms.num * sizeof(*node->perms.p);
+ memcpy(p, node->perms, node->num_perms * sizeof(*node->perms));
+ p += node->num_perms * sizeof(*node->perms);
memcpy(p, node->data, node->datalen);
p += node->datalen;
memcpy(p, node->children, node->childlen);
@@ -900,6 +900,7 @@ static int ask_parents(struct connection *conn, const void
*ctx,
const char *name, unsigned int *perm)
{
struct node *node;
+ struct node_perms perms;
do {
name = get_parent(ctx, name);
@@ -919,7 +920,8 @@ static int ask_parents(struct connection *conn, const void
*ctx,
return 0;
}
- *perm = perm_for_conn(conn, &node->perms);
+ node_to_node_perms(node, &perms);
+ *perm = perm_for_conn(conn, &perms);
This seems to be a common pattern. Can you introduce a wrapper?
Okay.
return 0;
}
@@ -956,11 +958,13 @@ static struct node *get_node(struct connection *conn,
unsigned int perm)
{
struct node *node;
+ struct node_perms perms;
node = read_node(conn, ctx, name);
/* If we don't have permission, we don't have node. */
if (node) {
- if ((perm_for_conn(conn, &node->perms) & perm) != perm) {
+ node_to_node_perms(node, &perms);
+ if ((perm_for_conn(conn, &perms) & perm) != perm) {
errno = EACCES;
node = NULL;
}
@@ -1434,14 +1438,14 @@ static struct node *construct_node(struct connection
*conn, const void *ctx,
node->name = talloc_steal(node, names[levels - 1]);
/* Inherit permissions, unpriv domains own what they create. */
- node->perms.num = parent->perms.num;
- node->perms.p = talloc_memdup(node, parent->perms.p,
- node->perms.num *
- sizeof(*node->perms.p));
- if (!node->perms.p)
+ node->num_perms = parent->num_perms;
+ node->perms = talloc_memdup(node, parent->perms,
+ node->num_perms *
+ sizeof(*node->perms));
+ if (!node->perms)
goto nomem;
RE: [PATCH v4 00/13] xen/arm: Split MMU code as the prepration of MPU work
Hi Julien, Thanks for your prompt response and your time on the review! > -Original Message- > From: Julien Grall > Subject: Re: [PATCH v4 00/13] xen/arm: Split MMU code as the prepration of > MPU work > > Hi, > > On 01/08/2023 04:44, Henry Wang wrote: > > Based on the discussion in the Xen Summit [1], sending this series out after > > addressing the comments in v3 [2] as the preparation work to add MPU > support. > > > > Mostly code movement, with some of Kconfig and build system (mainly > Makefiles) > > adjustment. No functional change expected. > > I can't really review this series without knowing how this will > integrate with the rest of the MPU work. Sorry about it, I should have asked before sending the series. > Can you at least provide a tree > with all the patches applied (including the MPU one)? See [1] for the full single core MPU implementation which I've verified locally about the MMU and MPU single core Linux boot on FVP, and looks like Gitlab is also not complaining [2]. Note that I've addressed comments from you and Ayan in v3 about the MMU/MPU helpers duplication and the arm32/arm64 split work. I didn't address the comment in [3] about the RES0 stuff, but I think this is not related to the MMU split series I sent yesterday to mailing list and the formal v4 MPU work sent to the mailing list will contain the fix for this comment. [1] https://gitlab.com/xen-project/people/henryw/xen/-/commits/mpu_v4 [2] https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/952742025 [3] https://patchwork.kernel.org/project/xen-devel/patch/20230626033443.2943270-32-penny.zh...@arm.com/ Kind regards, Henry > > Cheers, > > -- > Julien Grall
[xen-4.16-testing test] 182094: tolerable FAIL - PUSHED
flight 182094 xen-4.16-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/182094/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 181998 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 181998 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 181998 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 181998 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 181998 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 181998 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 181998 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 181998 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 181998 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 181998 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 181998 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 181998 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-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-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail 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-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-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-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass version targeted for testing: xen a910e3f2a4d128bc045ac6be374459e73ddf9fde baseline version: xen 82c5ab6be04a1f5544e38ed5198e79b91cecac45 Last test of basis 181998 2023-07-24 16:37:44 Z8 days Testing same since 182094 2023-07-31 13:07:24 Z1 days1 attempts People who touched revisions under test: Andrew Cooper jobs: build-amd64-xsm
Re: [RFC 6/6] capabilities: convert attach debugger into a capability
On Tue, 1 Aug 2023, Daniel P. Smith wrote:
> Expresses the ability to attach a debugger as a capability that a domain can
> be
> provisioned.
>
> Signed-off-by: Daniel P. Smith
> ---
> xen/arch/x86/hvm/svm/svm.c | 8
> xen/arch/x86/hvm/vmx/realmode.c | 2 +-
> xen/arch/x86/hvm/vmx/vmcs.c | 2 +-
> xen/arch/x86/hvm/vmx/vmx.c | 10 +-
> xen/arch/x86/traps.c| 6 --
> xen/common/domctl.c | 6 --
> xen/include/xen/sched.h | 9 ++---
> 7 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 27170213ae..9872804d39 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -999,7 +999,7 @@ static void noreturn cf_check svm_do_resume(void)
> {
> struct vcpu *v = current;
> struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
> -bool debug_state = (v->domain->debugger_attached ||
> +bool debug_state = (domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) ||
> v->domain->arch.monitor.software_breakpoint_enabled
> ||
> v->domain->arch.monitor.debug_exception_enabled);
> bool_t vcpu_guestmode = 0;
> @@ -1335,7 +1335,7 @@ static void cf_check svm_inject_event(const struct
> x86_event *event)
> }
> /* fall through */
> case X86_EXC_BP:
> -if ( curr->domain->debugger_attached )
> +if ( domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH) )
> {
> /* Debug/Int3: Trap to debugger. */
> domain_pause_for_debugger();
> @@ -2732,7 +2732,7 @@ void svm_vmexit_handler(void)
>
> case VMEXIT_ICEBP:
> case VMEXIT_EXCEPTION_DB:
> -if ( !v->domain->debugger_attached )
> +if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
> {
> unsigned int trap_type;
>
> @@ -2769,7 +2769,7 @@ void svm_vmexit_handler(void)
> if ( insn_len == 0 )
> break;
>
> -if ( v->domain->debugger_attached )
> +if ( domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
> {
> /* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update
> RIP. */
> __update_guest_eip(regs, insn_len);
> diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
> index ff44ddcfa6..f761026a9d 100644
> --- a/xen/arch/x86/hvm/vmx/realmode.c
> +++ b/xen/arch/x86/hvm/vmx/realmode.c
> @@ -121,7 +121,7 @@ void vmx_realmode_emulate_one(struct hvm_emulate_ctxt
> *hvmemul_ctxt)
>
> if ( rc == X86EMUL_EXCEPTION )
> {
> -if ( unlikely(curr->domain->debugger_attached) &&
> +if ( unlikely(domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH)) &&
> ((hvmemul_ctxt->ctxt.event.vector == X86_EXC_DB) ||
>(hvmemul_ctxt->ctxt.event.vector == X86_EXC_BP)) )
> {
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 13719cc923..9474869018 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1912,7 +1912,7 @@ void cf_check vmx_do_resume(void)
> hvm_asid_flush_vcpu(v);
> }
>
> -debug_state = v->domain->debugger_attached
> +debug_state = domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH)
>|| v->domain->arch.monitor.software_breakpoint_enabled
>|| v->domain->arch.monitor.singlestep_enabled;
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 7ec44018d4..5069e3cbf3 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2041,7 +2041,7 @@ static void cf_check vmx_inject_event(const struct
> x86_event *event)
> break;
> /* fall through */
> case X86_EXC_BP:
> -if ( curr->domain->debugger_attached )
> +if ( domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH) )
> {
> /* Debug/Int3: Trap to debugger. */
> domain_pause_for_debugger();
> @@ -2121,7 +2121,7 @@ static void cf_check vmx_set_info_guest(struct vcpu *v)
> * immediately vmexit and hence make no progress.
> */
> __vmread(GUEST_INTERRUPTIBILITY_INFO, &intr_shadow);
> -if ( v->domain->debugger_attached &&
> +if ( domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) &&
> (v->arch.user_regs.eflags & X86_EFLAGS_TF) &&
> (intr_shadow & VMX_INTR_SHADOW_STI) )
> {
> @@ -4283,7 +4283,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> }
> }
>
> -if ( !v->domain->debugger_attached )
> +if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
> {
> unsigned long insn_len = 0;
> int rc;
> @@ -4307,7 +4307,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
> break;
> case X86_EXC_BP:
> HVMTRACE_1D(TRAP, vector);
Re: [RFC 4/6] capabilities: introduce console io as a domain capability
On Tue, 1 Aug 2023, Daniel P. Smith wrote:
> The field `is_console` suggests that the field represents a state of being or
> posession, not that it reflects the privilege to access the console. In this
^ possession
> patch the field is renamed to capabilities to encapsulate the capabilities a
> domain has been granted. The first capability being the ability to read/write
> the Xen console.
>
> Signed-off-by: Daniel P. Smith
Patch looks fine to me aside the two minor nits. I am not sure I
understand 100% the difference between capabilities and roles but I am
OK with the patch. I'd like to hear Julien's feedback on this as well.
> ---
> xen/arch/arm/domain_build.c | 4 +++-
> xen/include/xen/sched.h | 25 +++--
> xen/include/xsm/dummy.h | 2 +-
> 3 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 51b4daefe1..ad7432b029 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -4076,7 +4076,9 @@ void __init create_domUs(void)
> panic("Error creating domain %s (rc = %ld)\n",
>dt_node_name(node), PTR_ERR(d));
>
> -d->is_console = true;
> +if ( ! domain_set_cap(d, CAP_CONSOLE_IO) )
code style
> +printk("failed setting console_io on %pd\n", d);
> +
> dt_device_set_used_by(node, d->domain_id);
>
> rc = construct_domU(d, node);
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index ec0f9baff6..b04fbe0565 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -472,8 +472,8 @@ struct domain
> #define ROLE_HARDWARE_DOMAIN (1U<<2)
> #define ROLE_XENSTORE_DOMAIN (1U<<3)
> uint8_t role;
> -/* Can this guest access the Xen console? */
> -bool is_console;
> +#define CAP_CONSOLE_IO (1U<<0)
> +uint8_t capabilities;
> /* Is this guest being debugged by dom0? */
> bool debugger_attached;
> /*
> @@ -1146,6 +1146,27 @@ static always_inline bool is_hvm_vcpu(const struct
> vcpu *v)
> return is_hvm_domain(v->domain);
> }
>
> +static always_inline bool domain_has_cap(
> +const struct domain *d, uint8_t cap)
> +{
> +return d->capabilities & cap;
> +}
> +
> +static always_inline bool domain_set_cap(
> +struct domain *d, uint8_t cap)
> +{
> +switch ( cap )
> +{
> +case CAP_CONSOLE_IO:
> +d->capabilities |= cap;
> +break;
> +default:
> +return false;
> +}
> +
> +return domain_has_cap(d, cap);
> +}
> +
> static always_inline bool hap_enabled(const struct domain *d)
> {
> /* sanitise_domain_config() rejects HAP && !HVM */
> diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
> index 18f1ddd127..067ff1d111 100644
> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -268,7 +268,7 @@ static XSM_INLINE int cf_check xsm_console_io(
> XSM_DEFAULT_ARG struct domain *d, int cmd)
> {
> XSM_ASSERT_ACTION(XSM_OTHER);
> -if ( d->is_console )
> +if ( domain_has_cap(d, CAP_CONSOLE_IO) )
> return xsm_default_action(XSM_HOOK, d, NULL);
> #ifdef CONFIG_VERBOSE_DEBUG
> if ( cmd == CONSOLEIO_write )
> --
> 2.20.1
>
Re: [RFC 3/6] roles: add a role for xenstore domain
On Tue, 1 Aug 2023, Daniel P. Smith wrote:
> Expand the possible roles for a domain to include a role for the Xenstore
> domain.
>
> Signed-off-by: Daniel P. Smith
Reviewed-by: Stefano Stabellini
> ---
> xen/common/domain.c | 3 +++
> xen/include/xen/sched.h | 3 ++-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 0ff1d52e3d..dbf055c559 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -633,6 +633,9 @@ struct domain *domain_create(domid_t domid,
> d->role |= ROLE_HARDWARE_DOMAIN;
> }
>
> +if ( d->options & XEN_DOMCTL_CDF_xs_domain )
> +d->role |= ROLE_XENSTORE_DOMAIN;
> +
> TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
>
> lock_profile_register_struct(LOCKPROF_TYPE_PERDOM, d, domid);
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 695f240326..ec0f9baff6 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -470,6 +470,7 @@ struct domain
> #define ROLE_UNBOUNDED_DOMAIN (1U<<0)
> #define ROLE_CONTROL_DOMAIN(1U<<1)
> #define ROLE_HARDWARE_DOMAIN (1U<<2)
> +#define ROLE_XENSTORE_DOMAIN (1U<<3)
> uint8_t role;
> /* Can this guest access the Xen console? */
> bool is_console;
> @@ -1165,7 +1166,7 @@ static inline bool is_vcpu_online(const struct vcpu *v)
>
> static inline bool is_xenstore_domain(const struct domain *d)
> {
> -return d->options & XEN_DOMCTL_CDF_xs_domain;
> +return d->role & ROLE_XENSTORE_DOMAIN;
> }
>
> static always_inline bool is_iommu_enabled(const struct domain *d)
> --
> 2.20.1
>
[xen-4.15-testing test] 182093: tolerable FAIL - PUSHED
flight 182093 xen-4.15-testing real [real] flight 182105 xen-4.15-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/182093/ http://logs.test-lab.xenproject.org/osstest/logs/182105/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-vhd 13 guest-start fail pass in 182105-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-vhd 14 migrate-support-check fail in 182105 never pass test-armhf-armhf-xl-vhd 15 saverestore-support-check fail in 182105 never pass test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 181995 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 181995 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 181995 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 181995 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 181995 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 181995 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 181995 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 181995 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 181995 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 181995 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 181995 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 181995 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-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-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-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail 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-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass version targeted for testing: xen e05a0e65b09d8512fdf4881c7fae713045c7d83d baseline version: xen faa4e2b1cf8022497a7b60c5b50a3bd280a5fc65 Last test of basis 181995 2023-07-24 16:37:0
Re: [RFC 2/6] roles: provide abstraction for the possible domain roles
On Tue, 1 Aug 2023, Daniel P. Smith wrote:
> The existing concepts such as unbounded domain, ie. all powerful, control
> domain and hardware domain are, effectively, roles the domains provide for the
> system. Currently, these are represented with booleans within `struct domain`
> or global domid variables that are compared against. This patch begins to
> formalize these roles by replacing the `is_control` and `is_console`, along
> with expanding the check against the global `hardware_domain` with a single
> encapsulating role attribute in `struct domain`.
>
> Signed-off-by: Daniel P. Smith
This is definitely heading in the right direction
> ---
> xen/arch/arm/domain_build.c | 2 ++
> xen/arch/x86/setup.c| 2 ++
> xen/common/domain.c | 14 +-
> xen/include/xen/sched.h | 16 +---
> xen/include/xsm/dummy.h | 4 ++--
> xen/xsm/flask/hooks.c | 12 ++--
> 6 files changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 39b4ee03a5..51b4daefe1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -4201,6 +4201,8 @@ void __init create_dom0(void)
> if ( IS_ERR(dom0) )
> panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
>
> +dom0->role |= ROLE_UNBOUNDED_DOMAIN;
I am not a fan of "UNBOUNDED". Maybe "PRIMARY"? "PRIVILEGED"? "SUPER"?
"ROOT"?
I also recognize I am not good at naming things so I'll stop here and
let other provide better feedback :-)
Also, do we actually need unbounded given that it gets replaced with
control_domain pretty soon?
I am asking because I think that at least from a safety perspective it
would be a problem to run a domain as "unbounded". In a safety system,
we wouldn't want anything to be unbounded, not even temporarily at boot.
If "unbounded" is removed before running dom0, then of course it is no
problem because actually dom0 never gets to run with "unbounded" set.
(I am currently thinking of solving the privilege issue by using XSM and
removing most privileges from Dom0.)
> if ( alloc_dom0_vcpu0(dom0) == NULL )
> panic("Error creating domain 0 vcpu0\n");
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 2dbe9857aa..4e20edc3bf 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -905,6 +905,8 @@ static struct domain *__init create_dom0(const module_t
> *image,
> if ( IS_ERR(d) )
> panic("Error creating d%u: %ld\n", domid, PTR_ERR(d));
>
> +d->role |= ROLE_UNBOUNDED_DOMAIN;
> +
> init_dom0_cpuid_policy(d);
>
> if ( alloc_dom0_vcpu0(d) == NULL )
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 8fb3c052f5..0ff1d52e3d 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -340,6 +340,14 @@ static int late_hwdom_init(struct domain *d)
> setup_io_bitmap(dom0);
> #endif
>
> +/*
> + * "dom0" may have been created under the unbounded role, demote it from
> + * that role, reducing it to the control domain role and any other roles
> it
> + * may have been given.
> + */
> +dom0->role &= ~(ROLE_UNBOUNDED_DOMAIN & ROLE_HARDWARE_DOMAIN);
> +dom0->role |= ROLE_CONTROL_DOMAIN;
I think we need a better definition of the three roles to understand
what this mean.
> rcu_unlock_domain(dom0);
>
> iommu_hwdom_init(d);
> @@ -609,7 +617,10 @@ struct domain *domain_create(domid_t domid,
> }
>
> /* Sort out our idea of is_control_domain(). */
> -d->is_privileged = flags & CDF_privileged;
> +if ( flags & CDF_privileged )
> +d->role |= ROLE_CONTROL_DOMAIN;
> +else
> +d->role &= ~ROLE_CONTROL_DOMAIN; /*ensure not set */
>
> /* Sort out our idea of is_hardware_domain(). */
> if ( is_initial_domain(d) || domid == hardware_domid )
> @@ -619,6 +630,7 @@ struct domain *domain_create(domid_t domid,
>
> old_hwdom = hardware_domain;
> hardware_domain = d;
> +d->role |= ROLE_HARDWARE_DOMAIN;
> }
>
> TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index a9276a7bed..695f240326 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -467,8 +467,10 @@ struct domain
> #endif
> /* is node-affinity automatically computed? */
> bool auto_node_affinity;
> -/* Is this guest fully privileged (aka dom0)? */
> -bool is_privileged;
> +#define ROLE_UNBOUNDED_DOMAIN (1U<<0)
> +#define ROLE_CONTROL_DOMAIN(1U<<1)
> +#define ROLE_HARDWARE_DOMAIN (1U<<2)
This is a great step in the right direction but I think at least a short
in-code comment to explain the difference between the three
> +uint8_t role;
> /* Can this guest access the Xen console? */
> bool is_console;
> /* Is this guest being debugged by dom0? */
> @@ -10
Re: [RFC 1/6] dom0: replace explict zero checks
On Tue, 1 Aug 2023, Daniel P. Smith wrote:
> A legacy concept is that the initial domain will have a domain id of zero. As
> a
> result there are places where a check that a domain is the inital domain is
> determined by an explicit check that the domid is zero.
>
> This commit seeks to abstract this check into a function call and replace all
> check locations with the function call.
>
> Signed-off-by: Daniel P. Smith
Thanks for the patch, this is a good cleanup!
> ---
> xen/common/domain.c | 4 ++--
> xen/common/sched/arinc653.c | 2 +-
> xen/common/sched/core.c | 4 ++--
> xen/include/xen/sched.h | 7 +++
> 4 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 304aa04fa6..8fb3c052f5 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -309,7 +309,7 @@ static int late_hwdom_init(struct domain *d)
> struct domain *dom0;
> int rv;
>
> -if ( d != hardware_domain || d->domain_id == 0 )
> +if ( d != hardware_domain || is_initial_domain(d) )
> return 0;
>
> rv = xsm_init_hardware_domain(XSM_HOOK, d);
> @@ -612,7 +612,7 @@ struct domain *domain_create(domid_t domid,
> d->is_privileged = flags & CDF_privileged;
>
> /* Sort out our idea of is_hardware_domain(). */
> -if ( domid == 0 || domid == hardware_domid )
> +if ( is_initial_domain(d) || domid == hardware_domid )
> {
> if ( hardware_domid < 0 || hardware_domid >= DOMID_FIRST_RESERVED )
> panic("The value of hardware_dom must be a valid domain ID\n");
> diff --git a/xen/common/sched/arinc653.c b/xen/common/sched/arinc653.c
> index a82c0d7314..31e8270af3 100644
> --- a/xen/common/sched/arinc653.c
> +++ b/xen/common/sched/arinc653.c
> @@ -404,7 +404,7 @@ a653sched_alloc_udata(const struct scheduler *ops, struct
> sched_unit *unit,
> * Add every one of dom0's units to the schedule, as long as there are
> * slots available.
> */
> -if ( unit->domain->domain_id == 0 )
> +if ( is_initial_domain(unit->domain) )
> {
> entry = sched_priv->num_schedule_entries;
>
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index 022f548652..210ad30f94 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -585,7 +585,7 @@ int sched_init_vcpu(struct vcpu *v)
> */
> sched_set_affinity(unit, cpumask_of(0), cpumask_of(0));
> }
> -else if ( d->domain_id == 0 && opt_dom0_vcpus_pin )
> +else if ( is_initial_domain(d) && opt_dom0_vcpus_pin )
> {
> /*
> * If dom0_vcpus_pin is specified, dom0 vCPUs are pinned 1:1 to
> @@ -594,7 +594,7 @@ int sched_init_vcpu(struct vcpu *v)
> sched_set_affinity(unit, cpumask_of(processor), &cpumask_all);
> }
> #ifdef CONFIG_X86
> -else if ( d->domain_id == 0 )
> +else if ( is_initial_domain(d) )
> {
> /*
> * In absence of dom0_vcpus_pin instead, the hard and soft affinity
> of
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 854f3e32c0..a9276a7bed 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -1058,6 +1058,13 @@ void scheduler_disable(void);
> void watchdog_domain_init(struct domain *d);
> void watchdog_domain_destroy(struct domain *d);
>
> +static always_inline bool is_initial_domain(const struct domain *d)
I know you used always_inline only because is_hardware_domain just below
also uses it, but I wonder if we need it.
Also, Robero, it looks like always_inline is missing from
docs/misra/C-language-toolchain.rst? Or do we plan to get rid of it?
> +{
> +static int init_domain_id = 0;
I take it is done this way because you plan to make it configurable?
> +return d->domain_id == init_domain_id;
> +}
> +
> /*
> * Use this check when the following are both true:
> * - Using this feature or interface requires full access to the hardware
> --
> 2.20.1
>
Re: [PATCH 0/4] xen/ppc: Add PowerNV bare metal support
On 8/1/23 7:11 PM, Shawn Anastasio wrote: > Hello all, Apologies, the subject should indicate that this is a v2 but I forgot to pass the appropriate argument to git-send-email. Thanks, Shawn
[PATCH 1/4] xen/ppc: Switch to medium PIC code model
Switch Xen to the medium PIC code model on Power. Among other things,
this allows us to be load address agnostic and will open the door to
booting on bare metal PowerNV systems that don't use OpenFirmware.
Also update XEN_VIRT_START to 0xc000, which is equivalent to
address 0x0 when the MMU is off. This prevents Open Firmware from
loading Xen at an offset from its base load address, so the DECL_SECTION
hack in xen.lds.S is no longer required.
Signed-off-by: Shawn Anastasio
---
Changed in v2:
- Remove stray newline
- Use label instead of .+4 in TOC setup branch
xen/arch/ppc/arch.mk | 2 +-
xen/arch/ppc/include/asm/asm-defns.h | 7 +
xen/arch/ppc/include/asm/config.h| 2 +-
xen/arch/ppc/ppc64/head.S| 12 +---
xen/arch/ppc/xen.lds.S | 44 ++--
5 files changed, 33 insertions(+), 34 deletions(-)
diff --git a/xen/arch/ppc/arch.mk b/xen/arch/ppc/arch.mk
index 7eec22c283..0183b9ac6a 100644
--- a/xen/arch/ppc/arch.mk
+++ b/xen/arch/ppc/arch.mk
@@ -5,7 +5,7 @@ ppc-march-$(CONFIG_POWER_ISA_2_07B) := power8
ppc-march-$(CONFIG_POWER_ISA_3_00) := power9
CFLAGS += -m64 -mlittle-endian -mcpu=$(ppc-march-y)
-CFLAGS += -mstrict-align -mcmodel=large -mabi=elfv2 -mno-altivec -mno-vsx
+CFLAGS += -mstrict-align -mcmodel=medium -mabi=elfv2 -fPIC -mno-altivec
-mno-vsx -msoft-float
LDFLAGS += -m elf64lppc
diff --git a/xen/arch/ppc/include/asm/asm-defns.h
b/xen/arch/ppc/include/asm/asm-defns.h
index 35b1c89d4e..5821f9024d 100644
--- a/xen/arch/ppc/include/asm/asm-defns.h
+++ b/xen/arch/ppc/include/asm/asm-defns.h
@@ -16,6 +16,13 @@
lis reg, (val) @h; \
ori reg, reg, (val) @l; \
+/*
+ * Load the address of a symbol from the TOC into the specified GPR.
+ */
+#define LOAD_REG_ADDR(reg,name) \
+addis reg,%r2,name@toc@ha; \
+addi reg,reg,name@toc@l
+
/*
* Depending on how we were booted, the CPU could be running in either
* Little Endian or Big Endian mode. The following trampoline from Linux
diff --git a/xen/arch/ppc/include/asm/config.h
b/xen/arch/ppc/include/asm/config.h
index cb27d2781e..d060f0dca7 100644
--- a/xen/arch/ppc/include/asm/config.h
+++ b/xen/arch/ppc/include/asm/config.h
@@ -39,7 +39,7 @@
name:
#endif
-#define XEN_VIRT_START _AT(UL, 0x40)
+#define XEN_VIRT_START _AT(UL, 0xc000)
#define SMP_CACHE_BYTES (1 << 6)
diff --git a/xen/arch/ppc/ppc64/head.S b/xen/arch/ppc/ppc64/head.S
index 02ff520458..4172f1b8ec 100644
--- a/xen/arch/ppc/ppc64/head.S
+++ b/xen/arch/ppc/ppc64/head.S
@@ -1,6 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0-or-later */
#include
+#include
.section .text.header, "ax", %progbits
@@ -11,16 +12,19 @@ ENTRY(start)
FIXUP_ENDIAN
/* set up the TOC pointer */
-LOAD_IMM32(%r2, .TOC.)
+bcl20, 31, 1f
+1: mflr%r12
+addis %r2, %r12, .TOC.-1b@ha
+addi%r2, %r2, .TOC.-1b@l
/* set up the initial stack */
-LOAD_IMM32(%r1, cpu0_boot_stack)
+LOAD_REG_ADDR(%r1, cpu0_boot_stack)
li %r11, 0
stdu%r11, -STACK_FRAME_OVERHEAD(%r1)
/* clear .bss */
-LOAD_IMM32(%r14, __bss_start)
-LOAD_IMM32(%r15, __bss_end)
+LOAD_REG_ADDR(%r14, __bss_start)
+LOAD_REG_ADDR(%r15, __bss_end)
1:
std %r11, 0(%r14)
addi%r14, %r14, 8
diff --git a/xen/arch/ppc/xen.lds.S b/xen/arch/ppc/xen.lds.S
index c628cc0e5c..2fa81d5a83 100644
--- a/xen/arch/ppc/xen.lds.S
+++ b/xen/arch/ppc/xen.lds.S
@@ -15,25 +15,12 @@ PHDRS
#endif
}
-/**
- * OpenFirmware's base load address is 0x40 (XEN_VIRT_START).
- * By defining sections this way, we can keep our virtual address base at
0x40
- * while keeping the physical base at 0x0.
- *
- * Otherwise, OpenFirmware incorrectly loads .text at 0x40 + 0x40 =
0x80.
- * Taken from x86/xen.lds.S
- */
-#ifdef CONFIG_LD_IS_GNU
-# define DECL_SECTION(x) x : AT(ADDR(#x) - XEN_VIRT_START)
-#else
-# define DECL_SECTION(x) x : AT(ADDR(x) - XEN_VIRT_START)
-#endif
-
SECTIONS
{
. = XEN_VIRT_START;
+_start = .;
-DECL_SECTION(.text) {
+.text : {
_stext = .;/* Text section */
*(.text.header)
@@ -52,7 +39,7 @@ SECTIONS
} :text
. = ALIGN(PAGE_SIZE);
-DECL_SECTION(.rodata) {
+.rodata : {
_srodata = .; /* Read-only data */
*(.rodata)
*(.rodata.*)
@@ -67,7 +54,7 @@ SECTIONS
#if defined(BUILD_ID)
. = ALIGN(4);
-DECL_SECTION(.note.gnu.build-id) {
+.note.gnu.build-id : {
__note_gnu_build_id_start = .;
*(.note.gnu.build-id)
__note_gnu_build_id_end = .;
@@ -76,19 +63,19 @@ SECTIONS
_erodata = .;/* End of read-only data */
. = ALIGN(PAGE_SIZE);
-DECL_SECTION(.data.ro_afte
[PATCH 2/4] xen/ppc: Add OPAL API definition header file
OPAL (OpenPower Abstraction Layer) is the interface exposed by firmware on PowerNV (bare metal) systems. Import Linux's header definining the API and related information. >From Linux commit 5321d1b1afb9a17302c6cec79f0cbf823eb0d3fc Signed-off-by: Shawn Anastasio --- Changed in v2: - Add Linux commit reference to commit message xen/arch/ppc/include/asm/opal-api.h | 1190 +++ 1 file changed, 1190 insertions(+) create mode 100644 xen/arch/ppc/include/asm/opal-api.h diff --git a/xen/arch/ppc/include/asm/opal-api.h b/xen/arch/ppc/include/asm/opal-api.h new file mode 100644 index 00..75100eda83 --- /dev/null +++ b/xen/arch/ppc/include/asm/opal-api.h @@ -0,0 +1,1190 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * OPAL API definitions. + * + * Copyright 2011-2015 IBM Corp. + */ + +#ifndef __OPAL_API_H +#define __OPAL_API_H + +/** OPAL APIs **/ + +/* Return codes */ +#define OPAL_SUCCESS 0 +#define OPAL_PARAMETER -1 +#define OPAL_BUSY -2 +#define OPAL_PARTIAL -3 +#define OPAL_CONSTRAINED -4 +#define OPAL_CLOSED-5 +#define OPAL_HARDWARE -6 +#define OPAL_UNSUPPORTED -7 +#define OPAL_PERMISSION-8 +#define OPAL_NO_MEM-9 +#define OPAL_RESOURCE -10 +#define OPAL_INTERNAL_ERROR-11 +#define OPAL_BUSY_EVENT-12 +#define OPAL_HARDWARE_FROZEN -13 +#define OPAL_WRONG_STATE -14 +#define OPAL_ASYNC_COMPLETION -15 +#define OPAL_EMPTY -16 +#define OPAL_I2C_TIMEOUT -17 +#define OPAL_I2C_INVALID_CMD -18 +#define OPAL_I2C_LBUS_PARITY -19 +#define OPAL_I2C_BKEND_OVERRUN -20 +#define OPAL_I2C_BKEND_ACCESS -21 +#define OPAL_I2C_ARBT_LOST -22 +#define OPAL_I2C_NACK_RCVD -23 +#define OPAL_I2C_STOP_ERR -24 +#define OPAL_XIVE_PROVISIONING -31 +#define OPAL_XIVE_FREE_ACTIVE -32 +#define OPAL_TIMEOUT -33 + +/* API Tokens (in r0) */ +#define OPAL_INVALID_CALL -1 +#define OPAL_TEST 0 +#define OPAL_CONSOLE_WRITE 1 +#define OPAL_CONSOLE_READ 2 +#define OPAL_RTC_READ 3 +#define OPAL_RTC_WRITE 4 +#define OPAL_CEC_POWER_DOWN5 +#define OPAL_CEC_REBOOT6 +#define OPAL_READ_NVRAM7 +#define OPAL_WRITE_NVRAM 8 +#define OPAL_HANDLE_INTERRUPT 9 +#define OPAL_POLL_EVENTS 10 +#define OPAL_PCI_SET_HUB_TCE_MEMORY11 +#define OPAL_PCI_SET_PHB_TCE_MEMORY12 +#define OPAL_PCI_CONFIG_READ_BYTE 13 +#define OPAL_PCI_CONFIG_READ_HALF_WORD 14 +#define OPAL_PCI_CONFIG_READ_WORD 15 +#define OPAL_PCI_CONFIG_WRITE_BYTE 16 +#define OPAL_PCI_CONFIG_WRITE_HALF_WORD17 +#define OPAL_PCI_CONFIG_WRITE_WORD 18 +#define OPAL_SET_XIVE 19 +#define OPAL_GET_XIVE 20 +#define OPAL_GET_COMPLETION_TOKEN_STATUS 21 /* obsolete */ +#define OPAL_REGISTER_OPAL_EXCEPTION_HANDLER 22 +#define OPAL_PCI_EEH_FREEZE_STATUS 23 +#define OPAL_PCI_SHPC 24 +#define OPAL_CONSOLE_WRITE_BUFFER_SPACE25 +#define OPAL_PCI_EEH_FREEZE_CLEAR 26 +#define OPAL_PCI_PHB_MMIO_ENABLE 27 +#define OPAL_PCI_SET_PHB_MEM_WINDOW28 +#define OPAL_PCI_MAP_PE_MMIO_WINDOW29 +#define OPAL_PCI_SET_PHB_TABLE_MEMORY 30 +#define OPAL_PCI_SET_PE31 +#define OPAL_PCI_SET_PELTV 32 +#define OPAL_PCI_SET_MVE 33 +#define OPAL_PCI_SET_MVE_ENABLE34 +#define OPAL_PCI_GET_XIVE_REISSUE 35 +#define OPAL_PCI_SET_XIVE_REISSUE 36 +#define OPAL_PCI_SET_XIVE_PE 37 +#define OPAL_GET_XIVE_SOURCE 38 +#define OPAL_GET_MSI_3239 +#define OPAL_GET_MSI_6440 +#define OPAL_START_CPU 41 +#define OPAL_QUERY_CPU_STATUS 42 +#define OPAL_WRITE_OPPANEL 43 /* unimplemented */ +#define OPAL_PCI_MAP_PE_DMA_WINDOW 44 +#define OPAL_PCI_MAP_PE_DMA_WINDOW_REAL45 +#define OPAL_PCI_RESET 49 +#define OPAL_PCI_GET_HUB_DIAG_DATA 50 +#define OPAL_PCI_GET_PHB_DIAG_DATA 51 +#define OPAL_PCI_FENCE_PHB 52 +#define OPAL_PCI_REINIT53 +#define OPAL_PCI_MASK_PE_ERROR 54 +#define OPAL_SET_SLOT_LED_STATUS 55 +#define OPAL_GET_EPOW_STATUS 56 +#define OPAL_SET_SYSTEM_ATTENTION_LED 57 +#define OPAL_RESERVED1 58 +#define OPAL_RE
[PATCH 4/4] xen/ppc: Implement early serial console on PowerNV
Implement the OPAL firmware calls required to write to the serial
console on PowerNV systems. Unlike pseries/Open Firmware, the OPAL
firmware interface can be used past early boot and as such the relevant
functions are not marked as __init.
Signed-off-by: Shawn Anastasio
---
Changed in v2:
- Include asm-offsets in asm-defns.h
- Clean up assembly formatting in asm-defns.h
- Clean up formatting of opal.c
- Clean up stray semicolon in OPAL_CALL macro in opal-calls.S
xen/arch/ppc/include/asm/asm-defns.h | 18 ++-
xen/arch/ppc/opal.c | 28 +-
xen/arch/ppc/ppc64/Makefile | 1 +
xen/arch/ppc/ppc64/asm-offsets.c | 4 ++
xen/arch/ppc/ppc64/opal-calls.S | 81
5 files changed, 129 insertions(+), 3 deletions(-)
create mode 100644 xen/arch/ppc/ppc64/opal-calls.S
diff --git a/xen/arch/ppc/include/asm/asm-defns.h
b/xen/arch/ppc/include/asm/asm-defns.h
index 5821f9024d..f1c49808bd 100644
--- a/xen/arch/ppc/include/asm/asm-defns.h
+++ b/xen/arch/ppc/include/asm/asm-defns.h
@@ -2,6 +2,8 @@
#ifndef _ASM_PPC_ASM_DEFNS_H
#define _ASM_PPC_ASM_DEFNS_H
+#include
+
/*
* Load a 64-bit immediate value into the specified GPR.
*/
@@ -20,8 +22,20 @@
* Load the address of a symbol from the TOC into the specified GPR.
*/
#define LOAD_REG_ADDR(reg,name) \
-addis reg,%r2,name@toc@ha; \
-addi reg,reg,name@toc@l
+addis reg, %r2, name@toc@ha; \
+addi reg, reg, name@toc@l
+
+/*
+ * Declare a global assembly function with a proper TOC setup prologue
+ */
+#define _GLOBAL_TOC(name) \
+.balign 4; \
+.type name, @function; \
+.globl name;\
+name: \
+0: addis %r2, %r12, (.TOC.-0b)@ha; \
+addi %r2, %r2, (.TOC.-0b)@l; \
+.localentry name, .-name
/*
* Depending on how we were booted, the CPU could be running in either
diff --git a/xen/arch/ppc/opal.c b/xen/arch/ppc/opal.c
index 251de8ac23..1183b7d5ef 100644
--- a/xen/arch/ppc/opal.c
+++ b/xen/arch/ppc/opal.c
@@ -8,9 +8,29 @@
#include
#include
-/* Global OPAL struct containing entrypoint and base */
+/* Global OPAL struct containing entrypoint and base used by opal-calls.S */
struct opal opal;
+int64_t opal_console_write(int64_t term_number, uint64_t *length,
+ const void *buffer);
+int64_t opal_console_flush(int64_t term_number);
+int64_t opal_reinit_cpus(uint64_t flags);
+
+static void opal_putchar(char c)
+{
+uint64_t len;
+
+if ( c == '\n' )
+{
+char buf = '\r';
+len = cpu_to_be64(1);
+opal_console_write(0, &len, &buf);
+}
+len = cpu_to_be64(1);
+opal_console_write(0, &len, &c);
+opal_console_flush(0);
+}
+
void __init boot_opal_init(const void *fdt)
{
int opal_node;
@@ -45,4 +65,10 @@ void __init boot_opal_init(const void *fdt)
opal.base = be64_to_cpu(*opal_base);
opal.entry = be64_to_cpu(*opal_entry);
+
+early_printk_init(opal_putchar);
+
+/* Ask OPAL to set HID0 for Little Endian interrupts + Radix TLB support */
+opal_reinit_cpus(OPAL_REINIT_CPUS_HILE_LE | OPAL_REINIT_CPUS_MMU_RADIX |
+ OPAL_REINIT_CPUS_MMU_HASH);
}
diff --git a/xen/arch/ppc/ppc64/Makefile b/xen/arch/ppc/ppc64/Makefile
index f4956daaa9..b9a91dc15f 100644
--- a/xen/arch/ppc/ppc64/Makefile
+++ b/xen/arch/ppc/ppc64/Makefile
@@ -1,2 +1,3 @@
obj-y += head.o
obj-y += of-call.o
+obj-y += opal-calls.o
diff --git a/xen/arch/ppc/ppc64/asm-offsets.c b/xen/arch/ppc/ppc64/asm-offsets.c
index e1129cb0f4..c15c1bf136 100644
--- a/xen/arch/ppc/ppc64/asm-offsets.c
+++ b/xen/arch/ppc/ppc64/asm-offsets.c
@@ -6,6 +6,7 @@
#include
#include
+#include
#define DEFINE(_sym, _val) \
asm volatile ( "\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
@@ -46,6 +47,9 @@ void __dummy__(void)
OFFSET(UREGS_cr, struct cpu_user_regs, cr);
OFFSET(UREGS_fpscr, struct cpu_user_regs, fpscr);
DEFINE(UREGS_sizeof, sizeof(struct cpu_user_regs));
+
+OFFSET(OPAL_base, struct opal, base);
+OFFSET(OPAL_entry, struct opal, entry);
}
/*
diff --git a/xen/arch/ppc/ppc64/opal-calls.S b/xen/arch/ppc/ppc64/opal-calls.S
new file mode 100644
index 00..cc5de75c8a
--- /dev/null
+++ b/xen/arch/ppc/ppc64/opal-calls.S
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Adapted from Linux's arch/powerpc/boot/opal-calls.S
+ *
+ * Copyright (c) 2016 IBM Corporation.
+ * Copyright Raptor
[PATCH 0/4] xen/ppc: Add PowerNV bare metal support
Hello all,
This series adds support for booting and using the serial console on
bare metal PowerNV/OpenPOWER systems. Up until now, Xen could only be
booted on the QEMU pseries model, which was initially targetted for ease
of development, but which differs significantly from the bare metal
systems where this port is likely to be the most useful.
In addition to adding support for the PowerNV boot protocol and firmware
interface, changes required to get libfdt from xen/libfdt building are
included. This is required to obtain the firmware entrypoint address on
PowerNV.
This series is based on Jan's "common: move simple_strto{,u}l{,l}() to lib/"
https://lists.xenproject.org/archives/html/xen-devel/2023-08/msg00095.html
Thanks,
Shawn
--
Changed in v2:
- Drop simple_strtoul patch, base on Jan's patch instead
- Remove ALL_LIBS-y override
- Clean up formatting of opal.c, opal-calls.S, asm-defns.h
- Consistently use label in TOC address calculation in head.S
Shawn Anastasio (4):
xen/ppc: Switch to medium PIC code model
xen/ppc: Add OPAL API definition header file
xen/ppc: Parse device tree for OPAL node on PowerNV
xen/ppc: Implement early serial console on PowerNV
xen/arch/ppc/Kconfig |1 +
xen/arch/ppc/Makefile|1 +
xen/arch/ppc/arch.mk |5 +-
xen/arch/ppc/include/asm/asm-defns.h | 21 +
xen/arch/ppc/include/asm/boot.h | 16 +-
xen/arch/ppc/include/asm/bug.h | 18 +
xen/arch/ppc/include/asm/cache.h |6 +
xen/arch/ppc/include/asm/config.h|2 +-
xen/arch/ppc/include/asm/opal-api.h | 1190 ++
xen/arch/ppc/include/asm/processor.h | 13 +-
xen/arch/ppc/include/asm/string.h|6 +
xen/arch/ppc/include/asm/system.h|6 +
xen/arch/ppc/opal.c | 74 ++
xen/arch/ppc/ppc64/Makefile |1 +
xen/arch/ppc/ppc64/asm-offsets.c |4 +
xen/arch/ppc/ppc64/head.S| 12 +-
xen/arch/ppc/ppc64/opal-calls.S | 81 ++
xen/arch/ppc/setup.c |9 +-
xen/arch/ppc/xen.lds.S | 44 +-
19 files changed, 1470 insertions(+), 40 deletions(-)
create mode 100644 xen/arch/ppc/include/asm/bug.h
create mode 100644 xen/arch/ppc/include/asm/cache.h
create mode 100644 xen/arch/ppc/include/asm/opal-api.h
create mode 100644 xen/arch/ppc/include/asm/string.h
create mode 100644 xen/arch/ppc/include/asm/system.h
create mode 100644 xen/arch/ppc/opal.c
create mode 100644 xen/arch/ppc/ppc64/opal-calls.S
--
2.30.2
[PATCH 3/4] xen/ppc: Parse device tree for OPAL node on PowerNV
Communication with firmware boot services on PowerNV requires parsing
the fdt blob passed by the bootloader in order to obtain the firmware
entrypoint. Use Xen's libfdt to do this and store the information
required for firmware calls, to be implemented in a future patch.
The full xen/common build doesn't yet work, but libfdt and its xen/lib
dependency can be made to build by defining a few stub headers.
Signed-off-by: Shawn Anastasio
---
Changed in v2:
- Remove ALL_LIBS-y override
xen/arch/ppc/Kconfig | 1 +
xen/arch/ppc/Makefile| 1 +
xen/arch/ppc/arch.mk | 3 +-
xen/arch/ppc/include/asm/boot.h | 16 +-
xen/arch/ppc/include/asm/bug.h | 18 +++
xen/arch/ppc/include/asm/cache.h | 6
xen/arch/ppc/include/asm/processor.h | 13 +++-
xen/arch/ppc/include/asm/string.h| 6
xen/arch/ppc/include/asm/system.h| 6
xen/arch/ppc/opal.c | 48
xen/arch/ppc/setup.c | 9 --
11 files changed, 121 insertions(+), 6 deletions(-)
create mode 100644 xen/arch/ppc/include/asm/bug.h
create mode 100644 xen/arch/ppc/include/asm/cache.h
create mode 100644 xen/arch/ppc/include/asm/string.h
create mode 100644 xen/arch/ppc/include/asm/system.h
create mode 100644 xen/arch/ppc/opal.c
diff --git a/xen/arch/ppc/Kconfig b/xen/arch/ppc/Kconfig
index a2ade2ecf4..b32dce39b8 100644
--- a/xen/arch/ppc/Kconfig
+++ b/xen/arch/ppc/Kconfig
@@ -1,5 +1,6 @@
config PPC
def_bool y
+ select HAS_DEVICE_TREE
config PPC64
def_bool y
diff --git a/xen/arch/ppc/Makefile b/xen/arch/ppc/Makefile
index 098a4dd0a9..0c0a7884a1 100644
--- a/xen/arch/ppc/Makefile
+++ b/xen/arch/ppc/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_PPC64) += ppc64/
obj-y += boot-of.init.o
obj-$(CONFIG_EARLY_PRINTK) += early_printk.init.o
+obj-y += opal.o
obj-y += setup.o
$(TARGET): $(TARGET)-syms
diff --git a/xen/arch/ppc/arch.mk b/xen/arch/ppc/arch.mk
index 0183b9ac6a..3bf79bac37 100644
--- a/xen/arch/ppc/arch.mk
+++ b/xen/arch/ppc/arch.mk
@@ -10,5 +10,4 @@ CFLAGS += -mstrict-align -mcmodel=medium -mabi=elfv2 -fPIC
-mno-altivec -mno-vsx
LDFLAGS += -m elf64lppc
# TODO: Drop override when more of the build is working
-override ALL_OBJS-y = arch/$(SRCARCH)/built_in.o
-override ALL_LIBS-y =
+override ALL_OBJS-y = arch/$(SRCARCH)/built_in.o common/libfdt/built_in.o
lib/built_in.o
diff --git a/xen/arch/ppc/include/asm/boot.h b/xen/arch/ppc/include/asm/boot.h
index 9b8a7c43c2..296539cd9e 100644
--- a/xen/arch/ppc/include/asm/boot.h
+++ b/xen/arch/ppc/include/asm/boot.h
@@ -4,7 +4,10 @@
#include
-/* a collection of interfaces used during boot. */
+/*
+ * OpenFirmware boot interfaces
+ */
+
enum {
OF_FAILURE = -1,
OF_SUCCESS = 0,
@@ -20,4 +23,15 @@ struct of_service {
int enter_of(struct of_service *args, unsigned long entry);
void boot_of_init(unsigned long vec);
+/*
+ * OPAL boot interfaces
+ */
+
+struct opal {
+uint64_t base;
+uint64_t entry;
+};
+
+void boot_opal_init(const void *fdt);
+
#endif /* _ASM_PPC_BOOT_H */
diff --git a/xen/arch/ppc/include/asm/bug.h b/xen/arch/ppc/include/asm/bug.h
new file mode 100644
index 00..e5e874b31c
--- /dev/null
+++ b/xen/arch/ppc/include/asm/bug.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ASM_PPC_BUG_H
+#define _ASM_PPC_BUG_H
+
+#include
+
+/*
+ * Power ISA guarantees that an instruction consisting of all zeroes is
+ * illegal.
+ */
+#define BUG_OPCODE 0x
+
+#define BUG_INSTR ".long " __stringify(BUG_OPCODE)
+
+#define BUG_FN_REG r0
+
+#endif /* _ASM_PPC_BUG_H */
diff --git a/xen/arch/ppc/include/asm/cache.h b/xen/arch/ppc/include/asm/cache.h
new file mode 100644
index 00..8a0a6b7b17
--- /dev/null
+++ b/xen/arch/ppc/include/asm/cache.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ASM_PPC_CACHE_H
+#define _ASM_PPC_CACHE_H
+
+#endif /* _ASM_PPC_CACHE_H */
diff --git a/xen/arch/ppc/include/asm/processor.h
b/xen/arch/ppc/include/asm/processor.h
index 838d279508..2300640787 100644
--- a/xen/arch/ppc/include/asm/processor.h
+++ b/xen/arch/ppc/include/asm/processor.h
@@ -133,6 +133,17 @@ struct cpu_user_regs
uint32_t entry_vector;
};
-#endif
+/*
+ * panic() isn't available at the moment so an infinite loop will be
+ * used temporarily.
+ * TODO: change it to panic()
+ */
+static inline void noreturn die(void)
+{
+for ( ; ; )
+HMT_very_low();
+}
+
+#endif /* __ASSEMBLY__ */
#endif /* _ASM_PPC_PROCESSOR_H */
diff --git a/xen/arch/ppc/include/asm/string.h
b/xen/arch/ppc/include/asm/string.h
new file mode 100644
index 00..7a420e05e4
--- /dev/null
+++ b/xen/arch/ppc/include/asm/string.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ASM_PPC_STRING_H
+#define _ASM_PPC_STRING_H
+
+#endif /* _ASM_PPC_STRING_H */
diff --git a/xen/arch/ppc/include/asm/system.h
b/xen/arch/ppc/include/asm/syst
Re: [XEN PATCH 3/4] automation/eclair: add scheduled pipelines
On Wed, 2 Aug 2023, Marek Marczykowski-Górecki wrote: > On Tue, Aug 01, 2023 at 03:55:20PM -0700, Stefano Stabellini wrote: > > On Tue, 1 Aug 2023, Simone Ballarin wrote: > > > This patch introduces six new ECLAIR jobs that run only > > > when triggered by a GitLab scheduled pipeline. > > > > > > Signed-off-by: Simone Ballarin > > > --- > > > +.eclair-analysis:on-schedule: > > > + extends: .eclair-analysis > > > + rules: > > > +- if: $CI_PIPELINE_SOURCE == "schedule" > > > > If I understand this right, the idea is that someone would schedule a > > pipeline (Build -> "Pipeline Schedules") and as part of that, they would > > also define the variable "CI_PIPELINE_SOURCE" to schedule. > > No, this is pre-defined variable in gitlab: > https://docs.gitlab.com/ee/ci/variables/predefined_variables.html Even better! Thanks! Then no need for a comment and the patch is OK as is. Reviewed-by: Stefano Stabellini
Re: [XEN PATCH 3/4] automation/eclair: add scheduled pipelines
On Tue, Aug 01, 2023 at 03:55:20PM -0700, Stefano Stabellini wrote: > On Tue, 1 Aug 2023, Simone Ballarin wrote: > > This patch introduces six new ECLAIR jobs that run only > > when triggered by a GitLab scheduled pipeline. > > > > Signed-off-by: Simone Ballarin > > --- > > +.eclair-analysis:on-schedule: > > + extends: .eclair-analysis > > + rules: > > +- if: $CI_PIPELINE_SOURCE == "schedule" > > If I understand this right, the idea is that someone would schedule a > pipeline (Build -> "Pipeline Schedules") and as part of that, they would > also define the variable "CI_PIPELINE_SOURCE" to schedule. No, this is pre-defined variable in gitlab: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html > > Is that correct? > > If so, please add a good in-code comments here on top of > .eclair-analysis:on-schedule to explain it. So that someone reading this > might know how what to do with the Gitlab CI settings. > -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
RE: [PATCH RESEND v9 33/36] KVM: VMX: Add VMX_DO_FRED_EVENT_IRQOFF for IRQ/NMI handling
> > +#include "../../entry/calling.h"
>
> Rather than do the low level PUSH_REGS and POP_REGS, I vote to have core code
> expose a FRED-specific wrapper for invoking external_interrupt(). More below.
Nice idea!
> > + /*
> > +* A FRED stack frame has extra 16 bytes of information pushed at the
> > +* regular stack top compared to an IDT stack frame.
>
> There is pretty much no chance that anyone remembers the layout of an IDT
> stack
> frame off the top of their head. I.e. saying "FRED has 16 bytes more" isn't
> all
> that useful. It also fails to capture the fact that FRED stuff a hell of a
> lot
> more information in those "common" 48 bytes.
>
> It'll be hard/impossible to capture all of the overload info in a comment, but
> showing the actual layout of the frame would be super helpful, e.g. something
> like
> this
>
> /*
>* FRED stack frames are always 64 bytes:
>*
>* --
>* | Bytes | Usage |
>* -|
>* | 63:56 | Reserved |
>* | 55:48 | Event Data|
> * | 47:40 | SS + Event Info |
> * | 39:32 | RSP |
>* | 31:24 | RFLAGS|
> * | 23:16 | CS + Aux Info |
> * | 15:8 | RIP |
> * | 7:0 | Error Code|
> * --
>*/
>*
>* Use LEA to get the return RIP and push it, then push an error code.
>* Note, only NMI handling does an ERETS to the target! IRQ handling
>* doesn't need to unmask NMIs and so simply uses CALL+RET, i.e. the
>* RIP pushed here is only truly consumed for NMIs!
I take these as ASM code does need more love, i.e., nice comments :/
Otherwise only more confusion.
>
> Jumping way back to providing a wrapper for FRED, if we do that, then there's
> no
> need to include calling.h, and the weird wrinkle about the ERET target kinda
> goes
> away too. E.g. provide this in arch/x86/entry/entry_64_fred.S
>
> .section .text, "ax"
>
> /* Note, this is instrumentation safe, and returns via RET, not ERETS! */
> #if IS_ENABLED(CONFIG_KVM_INTEL)
> SYM_CODE_START(fred_irq_entry_from_kvm)
> FRED_ENTER
> call external_interrupt
> FRED_EXIT
> RET
> SYM_CODE_END(fred_irq_entry_from_kvm)
> EXPORT_SYMBOL_GPL(fred_irq_entry_from_kvm);
> #endif
>
> and then the KVM side for this particular chunk is more simply:
>
> lea 1f(%rip), %rax
> push %rax
> push $0 /* FRED error code, 0 for NMI and external
> interrupts */
>
> \branch_insn \branch_target
> 1:
> VMX_DO_EVENT_FRAME_END
> RET
>
>
> Alternatively, the whole thing could be shoved into
> arch/x86/entry/entry_64_fred.S,
> but at a glance I don't think that would be a net positive due to the need to
> handle
> IRQs vs. NMIs.
>
> > + \branch_insn \branch_target
> > +
> > + .if \nmi == 0
> > + POP_REGS
> > + .endif
> > +
> > +1:
> > + /*
> > +* "Restore" RSP from RBP, even though IRET has already unwound RSP to
>
> As mentioned above, this is incorrect on two fronts.
>
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 0ecf4be2c6af..4e90c69a92bf 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -6890,6 +6890,14 @@ static void vmx_apicv_post_state_restore(struct
> kvm_vcpu *vcpu)
> > memset(vmx->pi_desc.pir, 0, sizeof(vmx->pi_desc.pir));
> > }
> >
> > +#ifdef CONFIG_X86_FRED
> > +void vmx_do_fred_interrupt_irqoff(unsigned int vector);
> > +void vmx_do_fred_nmi_irqoff(unsigned int vector);
> > +#else
> > +#define vmx_do_fred_interrupt_irqoff(x) BUG()
> > +#define vmx_do_fred_nmi_irqoff(x) BUG()
> > +#endif
>
> My slight preference is to open code the BUG() as a ud2 in assembly, purely to
> avoid more #ifdefs.
>
> > +
> > void vmx_do_interrupt_irqoff(unsigned long entry);
> > void vmx_do_nmi_irqoff(void);
> >
> > @@ -6932,14 +6940,16 @@ static void handle_external_interrupt_irqoff(struct
> kvm_vcpu *vcpu)
> > {
> > u32 intr_info = vmx_get_intr_info(vcpu);
> > unsigned int vector = intr_info & INTR_INFO_VECTOR_MASK;
> > - gate_desc *desc = (gate_desc *)host_idt_base + vector;
> >
> > if (KVM_BUG(!is_external_intr(intr_info), vcpu->kvm,
> > "unexpected VM-Exit interrupt info: 0x%x", intr_info))
> > return;
> >
> > kvm_before_interrupt(vcpu, KVM_HANDLING_IRQ);
> > - vmx_do_interrupt_irqoff(gate_offset(desc));
> > + if (cpu_feature_enabled(X86_FEATURE_FRED))
> > + vmx_do_fred_interrupt_irqoff(vector); /* Event type is 0 */
>
> I strongly prefer to use code to document what's going on. E.g. the tail
> comment
> just left me wondering, what event type is 0? Whereas this makes it quite
> clear
> that KVM is signaling a hardware interrupt. The fact that it's a nop as far
> as
> code generation goes is
Re: [XEN PATCH 4/4] automation/eclair: avoid failure in case of missing merge point
On Tue, 1 Aug 2023, Simone Ballarin wrote:
> In the context of an auto pull request, when a common merge point
> is not found the integration will continue the analysis without
> failing.
>
> Signed-off-by: Simone Ballarin
Acked-by: Stefano Stabellini
> ---
> automation/eclair_analysis/ECLAIR/action.settings | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/automation/eclair_analysis/ECLAIR/action.settings
> b/automation/eclair_analysis/ECLAIR/action.settings
> index 528bc24c72..f96368ffc7 100644
> --- a/automation/eclair_analysis/ECLAIR/action.settings
> +++ b/automation/eclair_analysis/ECLAIR/action.settings
> @@ -138,7 +138,9 @@ auto_pull_request)
> git remote add autoPRRemote "${autoPRRemoteUrl}"
> git fetch -q autoPRRemote
> subDir="${ref}"
> -baseCommitId=$(git merge-base "autoPRRemote/${autoPRBranch}" HEAD)
> +if ! baseCommitId=$(git merge-base "autoPRRemote/${autoPRBranch}" HEAD);
> then
> +baseCommitId=no_merge_point
> +fi
> jobHeadline="ECLAIR ${ANALYSIS_KIND} on repository ${repository}:
> ${pushUser} wants to merge ${repository}:${ref} (${headCommitId}) into
> ${autoPRRepository}/${autoPRBranch} (${baseCommitId})"
> ;;
> *)
> --
> 2.34.1
>
Re: [XEN PATCH 3/4] automation/eclair: add scheduled pipelines
On Tue, 1 Aug 2023, Simone Ballarin wrote:
> This patch introduces six new ECLAIR jobs that run only
> when triggered by a GitLab scheduled pipeline.
>
> Signed-off-by: Simone Ballarin
> ---
> .../eclair_analysis/ECLAIR/action.settings| 2 +-
> automation/gitlab-ci/analyze.yaml | 65 +--
> 2 files changed, 62 insertions(+), 5 deletions(-)
>
> diff --git a/automation/eclair_analysis/ECLAIR/action.settings
> b/automation/eclair_analysis/ECLAIR/action.settings
> index 71c10d5141..528bc24c72 100644
> --- a/automation/eclair_analysis/ECLAIR/action.settings
> +++ b/automation/eclair_analysis/ECLAIR/action.settings
> @@ -73,7 +73,7 @@ gitlab)
> headCommitId="${CI_COMMIT_SHA}"
> baseCommitId="${CI_MERGE_REQUEST_DIFF_BASE_SHA}"
> ;;
> -push | pipeline | web)
> +push | pipeline | web | schedule)
> event=push
> if [ -n "${CI_COMMIT_BRANCH:-}" ]; then
> ref_kind=branch
> diff --git a/automation/gitlab-ci/analyze.yaml
> b/automation/gitlab-ci/analyze.yaml
> index 3d8166572b..3325ef9d9a 100644
> --- a/automation/gitlab-ci/analyze.yaml
> +++ b/automation/gitlab-ci/analyze.yaml
> @@ -8,6 +8,8 @@
> ENABLE_ECLAIR_BOT: "n"
> AUTO_PR_BRANCH: "staging"
> AUTO_PR_REPOSITORY: "xen-project/xen"
> + script:
> +- ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}"
>artifacts:
> when: always
> paths:
> @@ -23,8 +25,6 @@ eclair-x86_64:
> LOGFILE: "eclair-x86_64.log"
> VARIANT: "X86_64"
> RULESET: "Set1"
> - script:
> -- ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}"
>allow_failure: true
>
> eclair-ARM64:
> @@ -33,6 +33,63 @@ eclair-ARM64:
> LOGFILE: "eclair-ARM64.log"
> VARIANT: "ARM64"
> RULESET: "Set1"
> - script:
> -- ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}"
> + allow_failure: true
> +
> +.eclair-analysis:on-schedule:
> + extends: .eclair-analysis
> + rules:
> +- if: $CI_PIPELINE_SOURCE == "schedule"
If I understand this right, the idea is that someone would schedule a
pipeline (Build -> "Pipeline Schedules") and as part of that, they would
also define the variable "CI_PIPELINE_SOURCE" to schedule.
Is that correct?
If so, please add a good in-code comments here on top of
.eclair-analysis:on-schedule to explain it. So that someone reading this
might know how what to do with the Gitlab CI settings.
> +eclair-x86_64-Set1:on-schedule:
> + extends: .eclair-analysis:on-schedule
> + variables:
> +VARIANT: "X86_64"
> +RULESET: "Set1"
> +ANALYSIS_KIND: "${RULESET}-scheduled"
> +LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
> + allow_failure: true
> +
> +eclair-x86_64-Set2:on-schedule:
> + extends: .eclair-analysis:on-schedule
> + variables:
> +VARIANT: "X86_64"
> +RULESET: "Set2"
> +ANALYSIS_KIND: "${RULESET}-scheduled"
> +LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
> + allow_failure: true
> +
> +eclair-x86_64-Set3:on-schedule:
> + extends: .eclair-analysis:on-schedule
> + variables:
> +VARIANT: "X86_64"
> +RULESET: "Set3"
> +ANALYSIS_KIND: "${RULESET}-scheduled"
> +LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
> + allow_failure: true
> +
> +eclair-ARM64-Set1:on-schedule:
> + extends: .eclair-analysis:on-schedule
> + variables:
> +VARIANT: "ARM64"
> +RULESET: "Set1"
> +ANALYSIS_KIND: "${RULESET}-scheduled"
> +LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
> + allow_failure: true
> +
> +eclair-ARM64-Set2:on-schedule:
> + extends: .eclair-analysis:on-schedule
> + variables:
> +VARIANT: "ARM64"
> +RULESET: "Set2"
> +ANALYSIS_KIND: "${RULESET}-scheduled"
> +LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
> + allow_failure: true
> +
> +eclair-ARM64-Set3:on-schedule:
> + extends: .eclair-analysis:on-schedule
> + variables:
> +VARIANT: "ARM64"
> +RULESET: "Set3"
> +ANALYSIS_KIND: "${RULESET}-scheduled"
> +LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
>allow_failure: true
> --
> 2.34.1
>
Re: [XEN PATCH 2/4] automation/eclair: add direct link to reports
On Tue, 1 Aug 2023, Simone Ballarin wrote:
> This patch adds direct links to the analysis findings in the
> console log.
>
> Signed-off-by: Simone Ballarin
Acked-by: Stefano Stabellini
> ---
> .../eclair_analysis/ECLAIR/action.helpers | 84 ++-
> 1 file changed, 65 insertions(+), 19 deletions(-)
>
> diff --git a/automation/eclair_analysis/ECLAIR/action.helpers
> b/automation/eclair_analysis/ECLAIR/action.helpers
> index 2ad6428eaa..df9bf2bd11 100644
> --- a/automation/eclair_analysis/ECLAIR/action.helpers
> +++ b/automation/eclair_analysis/ECLAIR/action.helpers
> @@ -1,17 +1,26 @@
> +esc=$(printf '\e')
> +cr=$(printf '\r')
> +
> if [ -n "${GITLAB_CI:-}" ]; then
> ci=gitlab
> +eol=
> +link_start="${esc}[33m"
> +link_end="${esc}[m"
> elif [ -n "${GITHUB_ACTION:-}" ]; then
> ci=github
> +eol="\\"
> +link_start=
> +link_end=
> elif [ -n "${JENKINS_HOME:-}" ]; then
> ci=jenkins
> +eol=""
> +link_start=
> +link_end=
> else
> echo "Unexpected CI/CD context" >&2
> exit 1
> fi
>
> -esc=$(printf '\e')
> -cr=$(printf '\r')
> -
> open_section() {
> id=$1
> title=$2
> @@ -36,54 +45,86 @@ summary() {
>
> case "${ci}" in
> github)
> -nl="\\"
> +eol="\\"
> ;;
> gitlab)
> -nl=
> +eol=
> ;;
> jenkins)
> -nl=""
> +eol=""
> ;;
> *)
> -nl=
> +eol=
> ;;
> esac
>
> +
> currentDbReportsUrl="${eclairReportUrlPrefix}/fs${jobDir}/PROJECT.ecd;/by_service.html#service&kind"
> if [ -z "${newReports}" ]; then
> -fixedMsg=
> +fixedMsg="No fixed reports as there is no baseline"
> unfixedMsg="Unfixed reports: ${unfixedReports}"
> -countsMsg="${unfixedMsg}"
> +referenceReportsMsgTxt=
> +referenceReportsMsgLog=
> else
> fixedMsg="Fixed reports: ${fixedReports}"
> unfixedMsg="Unfixed reports: ${unfixedReports} [new: ${newReports}]"
> -countsMsg="${fixedMsg}${nl}
> -${unfixedMsg}"
> +case "${event}" in
> +pull_request | auto_pull_request)
> +
> referenceDbReportsUrl="${eclairReportUrlPrefix}/fs${jobDir}/base/PROJECT.ecd;/by_service.html#service&kind"
> +reference_kind=base
> +;;
> +push)
> +
> referenceDbReportsUrl="${eclairReportUrlPrefix}/fs${jobDir}/prev/PROJECT.ecd;/by_service.html#service&kind"
> +reference_kind=previous
> +;;
> +*)
> +echo "Unexpected event ${event}" >&2
> +exit 1
> +;;
> +esac
> fi
> +
> case "${ci}" in
> jenkins)
> +if [ -n "${newReports}" ]; then
> +referenceReportsMsgTxt=" href="${referenceDbReportsUrl}">Browse ${reference_kind} reports"
> +fi
> cat <"${summaryTxt}"
> -${countsMsg}
> ${nl}
> +${fixedMsg}${eol}
> +${unfixedMsg}
> ${eol}
> https://www.bugseng.com/eclair";>
>
>
> ${jobHeadline}
> -Browse analysis results
> +Browse analysis summary
> +Browse current reports
> +${referenceReportsMsgTxt}
> EOF
> ;;
> *)
> +if [ -n "${newReports}" ]; then
> +referenceReportsMsgTxt="Browse ${reference_kind} reports:
> ${referenceDbReportsUrl}"
> +fi
> cat <"${summaryTxt}"
> https://www.bugseng.com/eclair";>
>
>
> Analysis Summary
>
> -${jobHeadline}${nl}
> -${countsMsg}${nl}
> -[Browse analysis](${indexHtmlUrl})
> +${jobHeadline}${eol}
> +${fixedMsg}${eol}
> +${unfixedMsg}${eol}
> +Browse analysis summary: ${indexHtmlUrl}
> +Browse current reports: ${currentDbReportsUrl}
> +${referenceReportsMsgTxt}
> EOF
> ;;
> esac
>
> +analysisSummaryMsgLog="Browse analysis summary:
> ${link_start}${indexHtmlUrl}${link_end}"
> +currentReportsMsgLog="Browse current reports:
> ${link_start}${currentDbReportsUrl}${link_end}"
> +if [ -n "${newReports}" ]; then
> +referenceReportsMsgLog="Browse ${reference_kind} reports:
> ${link_start}${referenceDbReportsUrl}${link_end}"
> +fi
> case ${ci} in
> github)
> cat "${summaryTxt}" >"${GITHUB_STEP_SUMMARY}"
> @@ -93,8 +134,11 @@ EOF
> # Generate summary and print it (GitLab-specific)
> cat < ${jobHeadline}
> -${countsMsg}
> -Browse analysis: ${esc}[33m${indexHtmlUrl}${esc}[m
> +${fixedMsg}
> +${unfixedMsg}
> +${analysisSummaryMsgLog}
> +${currentReportsMsgLog}
> +${referenceReportsMsgLog}
> EOF
> close_section ECLAIR_summary
> ;;
> @@ -103,7 +147,9 @@ EOF
> ${jobHeadline}
> ${fixedMsg}
> ${unfixedMsg}
> -Browse analysis: ${indexHtmlUrl}
> +${analysisSummaryMsgLog}
> +${currentReportsMsgLog}
> +${referenceReportsMsgLog}
> EOF
> ;;
> *)
> --
Re: [XEN PATCH 1/4] automation/eclair: add support for tag pipelines
On Tue, 1 Aug 2023, Simone Ballarin wrote:
> The ECLAIR jobs fail when triggered by tag pipelines (e.g.
> xen-project/patchew/xen).
>
> This patch extends the integration to support such pipelines.
>
> Signed-off-by: Simone Ballarin
Acked-by: Stefano Stabellini
Thanks for the fix! One good suggestion from Andrew would be to check
for required variables at the beginning, rather than at the end. As it
is now we are doing all the work just to fail at the end. It would have
been better to fail immediately saving resources. It could be another
patch :-)
> ---
> .../eclair_analysis/ECLAIR/action.settings| 24 ---
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/automation/eclair_analysis/ECLAIR/action.settings
> b/automation/eclair_analysis/ECLAIR/action.settings
> index 96426811a8..71c10d5141 100644
> --- a/automation/eclair_analysis/ECLAIR/action.settings
> +++ b/automation/eclair_analysis/ECLAIR/action.settings
> @@ -41,7 +41,7 @@ github)
> push | workflow_dispatch)
> event=push
> # Extract the branch name from "refs/heads/"
> -branch="${GITHUB_REF#refs/heads/}"
> +ref="${GITHUB_REF#refs/heads/}"
> headCommitId="${GITHUB_SHA}"
> pushUser="${GITHUB_ACTOR}"
> ;;
> @@ -75,7 +75,13 @@ gitlab)
> ;;
> push | pipeline | web)
> event=push
> -branch="${CI_COMMIT_BRANCH}"
> +if [ -n "${CI_COMMIT_BRANCH:-}" ]; then
> +ref_kind=branch
> +ref="${CI_COMMIT_BRANCH}"
> +else
> +ref_kind=tag
> +ref="${CI_COMMIT_TAG}"
> +fi
> headCommitId="${CI_COMMIT_SHA}"
> pushUser="${GITLAB_USER_NAME}"
> ;;
> @@ -99,7 +105,7 @@ jenkins)
> jenkinsBotToken="${ECLAIR_BOT_TOKEN:-}"
>
> event=push
> -branch="${GIT_BRANCH}"
> +ref="${GIT_BRANCH}"
> headCommitId="${GIT_COMMIT}"
> pushUser=$(git show --pretty='format:%aN' -s)
> ;;
> @@ -111,7 +117,7 @@ esac
>
> if [ "${event}" = "push" ] && [ -n "${autoPRBranch:-}" ]; then
> # AUTO PR Feature enabled
> -if ! [ "${branch}" = "${autoPRBranch}" ] ||
> +if ! [ "${ref}" = "${autoPRBranch}" ] ||
> ! [ "${repository}" = "${autoPRRepository}" ]; then
> event=auto_pull_request
> fi
> @@ -123,17 +129,17 @@ pull_request)
> jobHeadline="ECLAIR ${ANALYSIS_KIND} on repository ${repository}:
> ${pullRequestUser} wants to merge
> ${pullRequestHeadRepo}:${pullRequestHeadRef} (${headCommitId}) into
> ${pullRequestBaseRef} (${baseCommitId})"
> ;;
> push)
> -subDir="${branch}"
> -jobHeadline="ECLAIR ${ANALYSIS_KIND} on repository ${repository}: branch
> ${branch} (${headCommitId})"
> -badgeLabel="ECLAIR ${ANALYSIS_KIND} ${branch}${variantHeadline}
> #${jobId}"
> +subDir="${ref}"
> +jobHeadline="ECLAIR ${ANALYSIS_KIND} on repository ${repository}:
> ${ref_kind} ${ref} (${headCommitId})"
> +badgeLabel="ECLAIR ${ANALYSIS_KIND} ${ref}${variantHeadline} #${jobId}"
> ;;
> auto_pull_request)
> git remote remove autoPRRemote || true
> git remote add autoPRRemote "${autoPRRemoteUrl}"
> git fetch -q autoPRRemote
> -subDir="${branch}"
> +subDir="${ref}"
> baseCommitId=$(git merge-base "autoPRRemote/${autoPRBranch}" HEAD)
> -jobHeadline="ECLAIR ${ANALYSIS_KIND} on repository ${repository}:
> ${pushUser} wants to merge ${repository}:${branch} (${headCommitId}) into
> ${autoPRRepository}/${autoPRBranch} (${baseCommitId})"
> +jobHeadline="ECLAIR ${ANALYSIS_KIND} on repository ${repository}:
> ${pushUser} wants to merge ${repository}:${ref} (${headCommitId}) into
> ${autoPRRepository}/${autoPRBranch} (${baseCommitId})"
> ;;
> *)
> echo "Unexpected event ${event}" >&2
> --
> 2.34.1
>
Re: [PATCH 5/5] xen/ppc: Implement early serial console on PowerNV
On 8/1/23 6:19 AM, Jan Beulich wrote:
> On 28.07.2023 23:35, Shawn Anastasio wrote:
>> --- a/xen/arch/ppc/include/asm/asm-defns.h
>> +++ b/xen/arch/ppc/include/asm/asm-defns.h
>> @@ -23,6 +23,18 @@
>> addis reg,%r2,name@toc@ha;
>> \
>> addi reg,reg,name@toc@l
>
> Noticing only now, because of the issue ...
>
>> +/*
>> + * Declare a global assembly function with a proper TOC setup prologue
>> + */
>> +#define _GLOBAL_TOC(name)
>> \
>> +.balign 4;
>> \
>> +.type name,@function;
>> \
>> +.globl name;
>> \
>> +name:
>> \
>> +0: addis %r2,%r12,(.TOC.-0b)@ha;
>> \
>> +addi %r2,%r2,(.TOC.-0b)@l;
>> \
>> +.localentry name,.-name
>
> ... being widened - could we gain blanks after the commas? Down here
> (to match the code in context) another padding blank after "addi"
> would also be nice.
Sure, will do in v2.
>> --- a/xen/arch/ppc/opal.c
>> +++ b/xen/arch/ppc/opal.c
>> @@ -8,9 +8,28 @@
>> #include
>> #include
>>
>> -/* Global OPAL struct containing entrypoint and base */
>> +/* Global OPAL struct containing entrypoint and base used by opal-calls.S */
>> struct opal opal;
>>
>> +int64_t opal_console_write(int64_t term_number, uint64_t *length,
>> + uint8_t *buffer);
>
> Would this perhaps better be void *, eliminating the need for casting
> in calls of this function?
I made it uint8_t to match the official OPAL API documentation (though I
now see I missed a `const`):
https://open-power.github.io/skiboot/doc/opal-api/opal-console-read-write-1-2.html#opal-console-write
In this case though, the type information of this parameter might not be
that important and changing it to void* to avoid the cast is fine with
me.
>> +int64_t opal_console_flush(int64_t term_number);
>> +int64_t opal_reinit_cpus(uint64_t flags);
>> +
>> +static void opal_putchar(char c)
>
> Can't this be __init?
Unlike OpenFirmware, OPAL calls are expected to be used by the OS during
its entire lifecycle, not just during early boot, so the full
(non-early) serial console driver would likely want to use these
functions as well.
>
>> +{
>> +uint64_t len;
>> +if (c == '\n')
>
> Nit: Blank line please between declaration(s) and statement(s). (At
> least one more instance below.)
Will fix.
> Also please add the missing blanks in the if(), seeing that otherwise
> the file is aiming at being Xen style.
Ditto.
>> +{
>> +char buf = '\r';
>> +len = cpu_to_be64(1);
>> +opal_console_write(0, &len, (uint8_t *) &buf);
>> +}
>> +len = cpu_to_be64(1);
>> +opal_console_write(0, &len, (uint8_t *) &c);
>> +opal_console_flush(0);
>> +}
>> +
>> void __init boot_opal_init(const void *fdt)
>> {
>> int opal_node;
>> @@ -45,4 +64,10 @@ void __init boot_opal_init(const void *fdt)
>>
>> opal.base = be64_to_cpu(*opal_base);
>> opal.entry = be64_to_cpu(*opal_entry);
>> +
>> +early_printk_init(opal_putchar);
>> +
>> +/* Ask OPAL to set HID0 for Little Endian interrupts + Radix TLB
>> support */
>> +opal_reinit_cpus(OPAL_REINIT_CPUS_HILE_LE | OPAL_REINIT_CPUS_MMU_RADIX
>> + | OPAL_REINIT_CPUS_MMU_HASH);
>
> Nit: operators on continued lines go at the end of the earlier line.
Will fix.
>> --- /dev/null
>> +++ b/xen/arch/ppc/ppc64/opal-calls.S
>> @@ -0,0 +1,82 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Adapted from Linux's arch/powerpc/boot/opal-calls.S
>> + *
>> + * Copyright (c) 2016 IBM Corporation.
>> + * Copyright Raptor Engineering, LLC
>> + */
>> +
>> +#include
>> +#include
>
> Would it make sense to have asm-defns.h include asm-offsets.h, like
> x86 and Arm do?
Sure, I'll make this change along with the formatting updates in
asm-defns.h
>> +#include
>> +#include
>> +
>> +.text
>
> Is any of this code still needed post-init?
Yes, see above.
>> +#define OPAL_CALL(name, token) \
>> +.globl name;\
>> +name: \
>> +li %r0, token; \
>> +b opal_call;
>
> I think the trailing semicolon wants omitting.
Will fix.
> Jan
Thanks,
Shawn
Re: [XEN PATCH v2 3/3] arm/efi: address MISRA C:2012 Rule 5.3
On Tue, 1 Aug 2023, Nicola Vetrini wrote:
> Rule 5.3 has the following headline:
> "An identifier declared in an inner scope shall not hide an
> identifier declared in an outer scope"
>
> The file-scope variable 'fdt' is shadowed by function parameters,
> and thus violates the rule, hence it's renamed to 'fdt_efi'
>
> No functional changes.
>
> Signed-off-by: Nicola Vetrini
Reviewed-by: Stefano Stabellini
> ---
> Changes in v2:
> - Renamed the file-scope variable instead of removing function parameters.
> ---
> xen/arch/arm/efi/efi-boot.h | 84 ++---
> 1 file changed, 42 insertions(+), 42 deletions(-)
>
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index 6126a71400..f24df2abb9 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -49,7 +49,7 @@ static void PrintMessage(const CHAR16 *s);
> {0xb1b621d5U, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa,
> 0xe0}}
>
> static struct file __initdata dtbfile;
> -static void __initdata *fdt;
> +static void __initdata *fdt_efi;
> static void __initdata *memmap;
>
> static int __init setup_chosen_node(void *fdt, int *addr_cells, int
> *size_cells)
> @@ -383,7 +383,7 @@ static void __init
> efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
> if ( EFI_ERROR(status) )
> blexit(L"EFI memory map processing failed");
>
> -status = fdt_add_uefi_nodes(SystemTable, fdt, map, map_size, desc_size,
> +status = fdt_add_uefi_nodes(SystemTable, fdt_efi, map, map_size,
> desc_size,
> desc_ver);
> if ( EFI_ERROR(status) )
> PrintErrMesg(L"Updating FDT failed", status);
> @@ -395,7 +395,7 @@ static void __init efi_arch_pre_exit_boot(void)
>
> static void __init noreturn efi_arch_post_exit_boot(void)
> {
> -efi_xen_start(fdt, fdt_totalsize(fdt));
> +efi_xen_start(fdt_efi, fdt_totalsize(fdt_efi));
> }
>
> static void __init efi_arch_cfg_file_early(const EFI_LOADED_IMAGE *image,
> @@ -420,8 +420,8 @@ static void __init efi_arch_cfg_file_early(const
> EFI_LOADED_IMAGE *image,
> efi_bs->FreePool(name.w);
> }
> }
> -fdt = fdt_increase_size(&dtbfile, cfg.size + EFI_PAGE_SIZE);
> -if ( !fdt )
> +fdt_efi = fdt_increase_size(&dtbfile, cfg.size + EFI_PAGE_SIZE);
> +if ( !fdt_efi )
> blexit(L"Unable to create new FDT");
> }
>
> @@ -465,7 +465,7 @@ static void __init efi_arch_handle_cmdline(CHAR16
> *image_name,
> int chosen;
>
> /* locate chosen node, which is where we add Xen module info. */
> -chosen = fdt_subnode_offset(fdt, 0, "chosen");
> +chosen = fdt_subnode_offset(fdt_efi, 0, "chosen");
> if ( chosen < 0 )
> blexit(L"Unable to find chosen node");
>
> @@ -498,7 +498,7 @@ static void __init efi_arch_handle_cmdline(CHAR16
> *image_name,
> else
> {
> /* Get xen,xen-bootargs in /chosen if it is specified */
> -const char *dt_bootargs_prop = fdt_getprop(fdt, chosen,
> +const char *dt_bootargs_prop = fdt_getprop(fdt_efi, chosen,
> "xen,xen-bootargs", NULL);
> if ( dt_bootargs_prop )
> {
> @@ -526,7 +526,7 @@ static void __init efi_arch_handle_cmdline(CHAR16
> *image_name,
> blexit(L"FDT string overflow");
> }
>
> -if ( fdt_setprop_string(fdt, chosen, "xen,xen-bootargs", buf) < 0 )
> +if ( fdt_setprop_string(fdt_efi, chosen, "xen,xen-bootargs", buf) < 0 )
> blexit(L"Unable to set xen,xen-bootargs property.");
>
> efi_bs->FreePool(buf);
> @@ -542,7 +542,7 @@ static void __init efi_arch_handle_module(const struct
> file *file,
>
> if ( file == &dtbfile )
> return;
> -chosen = setup_chosen_node(fdt, &addr_len, &size_len);
> +chosen = setup_chosen_node(fdt_efi, &addr_len, &size_len);
> if ( chosen < 0 )
> blexit(L"Unable to setup chosen node");
>
> @@ -551,13 +551,13 @@ static void __init efi_arch_handle_module(const struct
> file *file,
> static const char __initconst ramdisk_compat[] =
> "multiboot,ramdisk\0"
> "multiboot,module";
>
> -node = fdt_add_subnode(fdt, chosen, "ramdisk");
> +node = fdt_add_subnode(fdt_efi, chosen, "ramdisk");
> if ( node < 0 )
> blexit(L"Unable to add ramdisk FDT node.");
> -if ( fdt_setprop(fdt, node, "compatible", ramdisk_compat,
> +if ( fdt_setprop(fdt_efi, node, "compatible", ramdisk_compat,
> sizeof(ramdisk_compat)) < 0 )
> blexit(L"Unable to set compatible property.");
> -if ( fdt_set_reg(fdt, node, addr_len, size_len, ramdisk.addr,
> +if ( fdt_set_reg(fdt_efi, node, addr_len, size_len, ramdisk.addr,
> ramdisk.size) < 0 )
> blexit(L"Unable to set reg property.
Re: [PATCH v3 21/25] tools/xenstore: introduce read_node_nocopy()
Hi Juergen,
Title: I can't find read_node_nocopy(). I found a read_node_const().
Which name did you intend to use?
On 24/07/2023 12:02, Juergen Gross wrote:
+static int read_node_helper(struct connection *conn, struct node *node)
+{
/* Data is binary blob (usually ascii, no nul). */
- node->data = node->perms + hdr->num_perms;
+ node->data = node->perms + node->hdr.num_perms;
/* Children is strings, nul separated. */
node->children = node->data + node->hdr.datalen;
if (domain_adjust_node_perms(node))
- goto error;
+ return -1;
You are either returning 0 or -1 which is then only used in if ( ... ).
Can we simply return a boolean (true for success, false for a failure)?
The rest LGTM.
/* If owner is gone reset currently accounted memory size. */
if (node->acc.domid != get_node_owner(node))
node->acc.memory = 0;
if (access_node(conn, node, NODE_ACCESS_READ, NULL))
+ return -1;
+
+ return 0;
+}
Cheers,
--
Julien Grall
Re: [PATCH v2] arm32: Avoid using solaris syntax for .section directive
On Tue, Aug 1, 2023 at 2:03 PM Julien Grall wrote: > > Hi, > > Title: This patch is not arm32 specific anymore. So I would replace > 'arm32' with 'arm'. This can be done on commit. > > On 01/08/2023 18:49, Khem Raj wrote: > > Assembler from binutils 2.41 rejects [1] this syntax > > > > .section "name"[, flags...] > > > > where flags could be #alloc, #write, #execinstr, #exclude, and #tls [2] > > > > It is almost like a regression compared to 2.40 or older release, > > The next word after ',' start with an uppercase. Did you intend to use > '.' rather than ','? > > That said, the documentation has the following: > > For SPARC ELF targets, the assembler supports another type of .section > directive for compatibility with the Solaris assembler:" > > This leads me to think this is not a regression and instead an intended > behavior (even though it breaks older build) even it breaks build. > > I would suggest to reword the commit message to: > > " > Assembler from binutiles 2.41 will rejects ([1], [2]) the following syntax > > .section "name", #alloc > > for any other any target other than ELF SPARC. This means we can't use > it in the Arm code. > > So switch to the GNU syntax > > .section name [, "flags"[, @type]] > > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=11601 > [2] https://sourceware.org/binutils/docs-2.41/as.html#Section > > If you agree with the commit message, I can update it while committing. LGTM, go ahead. > > We should also consider to backport. > > Cheers, > > -- > Julien Grall
Re: [PATCH v3 20/25] tools/xenstore: alloc new memory in domain_adjust_node_perms()
Hi Juergen,
On 24/07/2023 12:02, Juergen Gross wrote:
In order to avoid modifying the node data in the data base in case a
domain is gone, let domain_adjust_node_perms() allocate new memory for
the permissions in case they need to be modified. As this should
happen only in very rare cases, it is fine to do this even when having
copied the node data already.
Signed-off-by: Juergen Gross
---
V3:
- new patch
---
tools/xenstore/xenstored_core.c | 10 +-
tools/xenstore/xenstored_domain.c | 19 +++
2 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 404ecd0c62..ea3d20a372 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -751,6 +751,11 @@ struct node *read_node(struct connection *conn, const void
*ctx,
goto error;
}
+ /* Data is binary blob (usually ascii, no nul). */
+ node->data = node->perms + hdr->num_perms;
+ /* Children is strings, nul separated. */
+ node->children = node->data + node->hdr.datalen;
+
It took me a while to understand why you move the lines above. I tihnk
it would be worth documenting in the code (possibly on top of the
declaration domain_adjust_node_perms()) that domain_adjust_node_perms()
may re-allocate the permissions.
if (domain_adjust_node_perms(node))
goto error;
@@ -758,11 +763,6 @@ struct node *read_node(struct connection *conn, const void *ctx,
if (node->acc.domid != get_node_owner(node))
node->acc.memory = 0;
- /* Data is binary blob (usually ascii, no nul). */
- node->data = node->perms + hdr->num_perms;
- /* Children is strings, nul separated. */
- node->children = node->data + node->hdr.datalen;
-
if (access_node(conn, node, NODE_ACCESS_READ, NULL))
goto error;
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index fdf1095acb..cdef6efef4 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1334,13 +1334,24 @@ int domain_alloc_permrefs(struct node_perms *perms)
int domain_adjust_node_perms(struct node *node)
{
unsigned int i;
+ struct xs_permissions *perms = node->perms;
+ bool copied = false;
for (i = 1; i < node->hdr.num_perms; i++) {
- if (node->perms[i].perms & XS_PERM_IGNORE)
+ if ((perms[i].perms & XS_PERM_IGNORE) ||
+ chk_domain_generation(perms[i].id, node->hdr.generation))
continue;
- if (!chk_domain_generation(node->perms[i].id,
- node->hdr.generation))
- node->perms[i].perms |= XS_PERM_IGNORE;
+
+ if (!copied) {
This wants a coment explain why you need to copy it.
+ perms = talloc_memdup(node, node->perms,
+ node->hdr.num_perms * sizeof(*perms));
+ if (!perms)
+ return ENOMEM;
+ node->perms = perms;
+ copied = true;
+ }
+
+ perms[i].perms |= XS_PERM_IGNORE;
}
return 0;
Cheers,
--
Julien Grall
Re: [PATCH v3 19/25] tools/xenstore: use struct node_hdr in struct node
Hi Juergen,
On 24/07/2023 12:02, Juergen Gross wrote:
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c72fc0c725..404ecd0c62 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -555,6 +555,12 @@ static void initialize_fds(int *p_sock_pollfd_idx, int
*ptimeout)
}
}
+static size_t calc_node_acc_size(struct node_hdr *hdr)
The parameter can be const.
+{
+ return sizeof(*hdr) + hdr->num_perms * sizeof(struct xs_permissions) +
+ hdr->datalen + hdr->childlen;
+}
+
[...]
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 9cb4c2f3eb..adf8a785fc 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -181,6 +181,7 @@ extern struct list_head connections;
*/
struct node_hdr {
uint64_t generation;
+#define NO_GENERATION ~((uint64_t)0)
uint16_t num_perms;
uint16_t datalen;
uint32_t childlen;
@@ -197,6 +198,10 @@ struct node_account_data {
};
struct node {
+ /* Data direct for data base. */
I can't parse it. Did you mean 'from' rather than 'for'?
+ struct node_hdr hdr;
+
+ /* Xenstore path. */
const char *name;
/* Name used to access data base. */
const char *db_name;
@@ -204,20 +209,13 @@ struct node {
/* Parent (optional) */
struct node *parent;
Cheers,
--
Julien Grall
Re: [PATCH v3 18/25] tools/xenstore: don't use struct node_perms in struct node
Hi Juergen,
On 24/07/2023 12:02, Juergen Gross wrote:
Open code struct node_perms in struct node in order to prepare using
struct node_hdr in struct node.
Add two helpers to transfer permissions between struct node and struct
node_perms.
Signed-off-by: Juergen Gross
---
V2:
- new patch
---
tools/xenstore/xenstored_core.c| 76 ++
tools/xenstore/xenstored_core.h| 21 ++-
tools/xenstore/xenstored_domain.c | 13 ++---
tools/xenstore/xenstored_transaction.c | 8 +--
tools/xenstore/xenstored_watch.c | 7 ++-
5 files changed, 75 insertions(+), 50 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 86b7c9bf36..c72fc0c725 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -735,7 +735,7 @@ struct node *read_node(struct connection *conn, const void
*ctx,
/* Datalen, childlen, number of permissions */
node->generation = hdr->generation;
- node->perms.num = hdr->num_perms;
+ node->num_perms = hdr->num_perms;
node->datalen = hdr->datalen;
node->childlen = hdr->childlen;
node->acc.domid = perms_from_node_hdr(hdr)->id;
@@ -743,8 +743,8 @@ struct node *read_node(struct connection *conn, const void
*ctx,
/* Copy node data to new memory area, starting with permissions. */
size -= sizeof(*hdr);
- node->perms.p = talloc_memdup(node, perms_from_node_hdr(hdr), size);
- if (node->perms.p == NULL) {
+ node->perms = talloc_memdup(node, perms_from_node_hdr(hdr), size);
+ if (node->perms == NULL) {
errno = ENOMEM;
goto error;
}
@@ -757,7 +757,7 @@ struct node *read_node(struct connection *conn, const void
*ctx,
node->acc.memory = 0;
/* Data is binary blob (usually ascii, no nul). */
- node->data = node->perms.p + hdr->num_perms;
+ node->data = node->perms + hdr->num_perms;
/* Children is strings, nul separated. */
node->children = node->data + node->datalen;
@@ -796,7 +796,7 @@ int write_node_raw(struct connection *conn, const char *db_name,
return errno;
size = sizeof(*hdr)
- + node->perms.num * sizeof(node->perms.p[0])
+ + node->num_perms * sizeof(node->perms[0])
+ node->datalen + node->childlen;
/* Call domain_max_chk() in any case in order to record max values. */
@@ -813,13 +813,13 @@ int write_node_raw(struct connection *conn, const char
*db_name,
hdr = data;
hdr->generation = node->generation;
- hdr->num_perms = node->perms.num;
+ hdr->num_perms = node->num_perms;
hdr->datalen = node->datalen;
hdr->childlen = node->childlen;
p = perms_from_node_hdr(hdr);
- memcpy(p, node->perms.p, node->perms.num * sizeof(*node->perms.p));
- p += node->perms.num * sizeof(*node->perms.p);
+ memcpy(p, node->perms, node->num_perms * sizeof(*node->perms));
+ p += node->num_perms * sizeof(*node->perms);
memcpy(p, node->data, node->datalen);
p += node->datalen;
memcpy(p, node->children, node->childlen);
@@ -900,6 +900,7 @@ static int ask_parents(struct connection *conn, const void
*ctx,
const char *name, unsigned int *perm)
{
struct node *node;
+ struct node_perms perms;
do {
name = get_parent(ctx, name);
@@ -919,7 +920,8 @@ static int ask_parents(struct connection *conn, const void
*ctx,
return 0;
}
- *perm = perm_for_conn(conn, &node->perms);
+ node_to_node_perms(node, &perms);
+ *perm = perm_for_conn(conn, &perms);
This seems to be a common pattern. Can you introduce a wrapper?
return 0;
}
@@ -956,11 +958,13 @@ static struct node *get_node(struct connection *conn,
unsigned int perm)
{
struct node *node;
+ struct node_perms perms;
node = read_node(conn, ctx, name);
/* If we don't have permission, we don't have node. */
if (node) {
- if ((perm_for_conn(conn, &node->perms) & perm) != perm) {
+ node_to_node_perms(node, &perms);
+ if ((perm_for_conn(conn, &perms) & perm) != perm) {
errno = EACCES;
node = NULL;
}
@@ -1434,14 +1438,14 @@ static struct node *construct_node(struct connection
*conn, const void *ctx,
node->name = talloc_steal(node, names[levels - 1]);
/* Inherit permissions, unpriv domains own what they create. */
- node->perms.num = parent->perms.num;
- node->perms.p = talloc_memdup(node, parent->perms.p,
- node->perms.num *
- sizeof(*node->perms.p));
- if (!node->perms.p)
+ node-
[xen-4.14-testing test] 182092: tolerable FAIL - PUSHED
flight 182092 xen-4.14-testing real [real] flight 182104 xen-4.14-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/182092/ http://logs.test-lab.xenproject.org/osstest/logs/182104/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-qemuu-freebsd11-amd64 19 guest-localmigrate/x10 fail pass in 182104-retest test-armhf-armhf-xl-vhd 13 guest-start fail pass in 182104-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-vhd 14 migrate-support-check fail in 182104 never pass test-armhf-armhf-xl-vhd 15 saverestore-support-check fail in 182104 never pass test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 181996 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 181996 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 181996 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 181996 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 181996 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 181996 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 181996 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 181996 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 181996 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 181996 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 181996 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 181996 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-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-amd64-amd64-libvirt-xsm 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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 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-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-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-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen 602ee4c2954ed3f9f8552e08043146bee60ec012 baseline version: xen
Re: [PATCH v2] arm32: Avoid using solaris syntax for .section directive
Hi, Title: This patch is not arm32 specific anymore. So I would replace 'arm32' with 'arm'. This can be done on commit. On 01/08/2023 18:49, Khem Raj wrote: Assembler from binutils 2.41 rejects [1] this syntax .section "name"[, flags...] where flags could be #alloc, #write, #execinstr, #exclude, and #tls [2] It is almost like a regression compared to 2.40 or older release, The next word after ',' start with an uppercase. Did you intend to use '.' rather than ','? That said, the documentation has the following: For SPARC ELF targets, the assembler supports another type of .section directive for compatibility with the Solaris assembler:" This leads me to think this is not a regression and instead an intended behavior (even though it breaks older build) even it breaks build. I would suggest to reword the commit message to: " Assembler from binutiles 2.41 will rejects ([1], [2]) the following syntax .section "name", #alloc for any other any target other than ELF SPARC. This means we can't use it in the Arm code. So switch to the GNU syntax .section name [, "flags"[, @type]] [1] https://sourceware.org/bugzilla/show_bug.cgi?id=11601 [2] https://sourceware.org/binutils/docs-2.41/as.html#Section If you agree with the commit message, I can update it while committing. We should also consider to backport. Cheers, -- Julien Grall
[RFC 6/6] capabilities: convert attach debugger into a capability
Expresses the ability to attach a debugger as a capability that a domain can be
provisioned.
Signed-off-by: Daniel P. Smith
---
xen/arch/x86/hvm/svm/svm.c | 8
xen/arch/x86/hvm/vmx/realmode.c | 2 +-
xen/arch/x86/hvm/vmx/vmcs.c | 2 +-
xen/arch/x86/hvm/vmx/vmx.c | 10 +-
xen/arch/x86/traps.c| 6 --
xen/common/domctl.c | 6 --
xen/include/xen/sched.h | 9 ++---
7 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 27170213ae..9872804d39 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -999,7 +999,7 @@ static void noreturn cf_check svm_do_resume(void)
{
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
-bool debug_state = (v->domain->debugger_attached ||
+bool debug_state = (domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) ||
v->domain->arch.monitor.software_breakpoint_enabled ||
v->domain->arch.monitor.debug_exception_enabled);
bool_t vcpu_guestmode = 0;
@@ -1335,7 +1335,7 @@ static void cf_check svm_inject_event(const struct
x86_event *event)
}
/* fall through */
case X86_EXC_BP:
-if ( curr->domain->debugger_attached )
+if ( domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH) )
{
/* Debug/Int3: Trap to debugger. */
domain_pause_for_debugger();
@@ -2732,7 +2732,7 @@ void svm_vmexit_handler(void)
case VMEXIT_ICEBP:
case VMEXIT_EXCEPTION_DB:
-if ( !v->domain->debugger_attached )
+if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
{
unsigned int trap_type;
@@ -2769,7 +2769,7 @@ void svm_vmexit_handler(void)
if ( insn_len == 0 )
break;
-if ( v->domain->debugger_attached )
+if ( domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
{
/* AMD Vol2, 15.11: INT3, INTO, BOUND intercepts do not update
RIP. */
__update_guest_eip(regs, insn_len);
diff --git a/xen/arch/x86/hvm/vmx/realmode.c b/xen/arch/x86/hvm/vmx/realmode.c
index ff44ddcfa6..f761026a9d 100644
--- a/xen/arch/x86/hvm/vmx/realmode.c
+++ b/xen/arch/x86/hvm/vmx/realmode.c
@@ -121,7 +121,7 @@ void vmx_realmode_emulate_one(struct hvm_emulate_ctxt
*hvmemul_ctxt)
if ( rc == X86EMUL_EXCEPTION )
{
-if ( unlikely(curr->domain->debugger_attached) &&
+if ( unlikely(domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH)) &&
((hvmemul_ctxt->ctxt.event.vector == X86_EXC_DB) ||
(hvmemul_ctxt->ctxt.event.vector == X86_EXC_BP)) )
{
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 13719cc923..9474869018 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1912,7 +1912,7 @@ void cf_check vmx_do_resume(void)
hvm_asid_flush_vcpu(v);
}
-debug_state = v->domain->debugger_attached
+debug_state = domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH)
|| v->domain->arch.monitor.software_breakpoint_enabled
|| v->domain->arch.monitor.singlestep_enabled;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 7ec44018d4..5069e3cbf3 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2041,7 +2041,7 @@ static void cf_check vmx_inject_event(const struct
x86_event *event)
break;
/* fall through */
case X86_EXC_BP:
-if ( curr->domain->debugger_attached )
+if ( domain_has_cap(curr->domain, CAP_DEBUGGER_ATTACH) )
{
/* Debug/Int3: Trap to debugger. */
domain_pause_for_debugger();
@@ -2121,7 +2121,7 @@ static void cf_check vmx_set_info_guest(struct vcpu *v)
* immediately vmexit and hence make no progress.
*/
__vmread(GUEST_INTERRUPTIBILITY_INFO, &intr_shadow);
-if ( v->domain->debugger_attached &&
+if ( domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) &&
(v->arch.user_regs.eflags & X86_EFLAGS_TF) &&
(intr_shadow & VMX_INTR_SHADOW_STI) )
{
@@ -4283,7 +4283,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
}
}
-if ( !v->domain->debugger_attached )
+if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
{
unsigned long insn_len = 0;
int rc;
@@ -4307,7 +4307,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
break;
case X86_EXC_BP:
HVMTRACE_1D(TRAP, vector);
-if ( !v->domain->debugger_attached )
+if ( !domain_has_cap(v->domain, CAP_DEBUGGER_ATTACH) )
{
unsigned long insn_len;
int rc;
@@ -4647,7 +4647,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
[RFC 5/6] capabilities: add dom0 cpu faulting disable
This encapsulates disableing cpu faulting for PV dom0 as a capability.
Signed-off-by: Daniel P. Smith
---
xen/arch/x86/cpu-policy.c | 2 +-
xen/arch/x86/cpu/common.c | 82 +++
xen/arch/x86/setup.c | 4 ++
xen/include/xen/sched.h | 8 +++-
4 files changed, 52 insertions(+), 44 deletions(-)
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 1f954d4e59..42c3193938 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -912,7 +912,7 @@ void __init init_dom0_cpuid_policy(struct domain *d)
* If the domain is getting unfiltered CPUID, don't let the guest kernel
* play with CPUID faulting either, as Xen's CPUID path won't cope.
*/
-if ( !opt_dom0_cpuid_faulting && is_control_domain(d) && is_pv_domain(d) )
+if ( domain_has_cap(d, CAP_DISABLE_CPU_FAULT) )
p->platform_info.cpuid_faulting = false;
recalculate_cpuid_policy(d);
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index cfcdaace12..937581e353 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -164,48 +164,46 @@ static void set_cpuid_faulting(bool enable)
void ctxt_switch_levelling(const struct vcpu *next)
{
- const struct domain *nextd = next ? next->domain : NULL;
- bool enable_cpuid_faulting;
-
- if (cpu_has_cpuid_faulting ||
- boot_cpu_has(X86_FEATURE_CPUID_USER_DIS)) {
- /*
-* No need to alter the faulting setting if we are switching
-* to idle; it won't affect any code running in idle context.
-*/
- if (nextd && is_idle_domain(nextd))
- return;
- /*
-* We *should* be enabling faulting for PV control domains.
-*
-* The domain builder has now been updated to not depend on
-* seeing host CPUID values. This makes it compatible with
-* PVH toolstack domains, and lets us enable faulting by
-* default for all PV domains.
-*
-* However, as PV control domains have never had faulting
-* enforced on them before, there might plausibly be other
-* dependenices on host CPUID data. Therefore, we have left
-* an interim escape hatch in the form of
-* `dom0=no-cpuid-faulting` to restore the older behaviour.
-*/
- enable_cpuid_faulting = nextd && (opt_dom0_cpuid_faulting ||
- !is_control_domain(nextd) ||
- !is_pv_domain(nextd)) &&
- (is_pv_domain(nextd) ||
-next->arch.msrs->
-misc_features_enables.cpuid_faulting);
-
- if (cpu_has_cpuid_faulting)
- set_cpuid_faulting(enable_cpuid_faulting);
- else
- amd_set_cpuid_user_dis(enable_cpuid_faulting);
-
- return;
- }
-
- if (ctxt_switch_masking)
- alternative_vcall(ctxt_switch_masking, next);
+const struct domain *nextd = next ? next->domain : NULL;
+bool enable_cpuid_faulting;
+
+if ( cpu_has_cpuid_faulting ||
+ boot_cpu_has(X86_FEATURE_CPUID_USER_DIS) ) {
+/*
+* No need to alter the faulting setting if we are switching
+* to idle; it won't affect any code running in idle context.
+*/
+if (nextd && is_idle_domain(nextd))
+return;
+/*
+* We *should* be enabling faulting for PV control domains.
+*
+* The domain builder has now been updated to not depend on
+* seeing host CPUID values. This makes it compatible with
+* PVH toolstack domains, and lets us enable faulting by
+* default for all PV domains.
+*
+* However, as PV control domains have never had faulting
+* enforced on them before, there might plausibly be other
+* dependenices on host CPUID data. Therefore, we have left
+* an interim escape hatch in the form of
+* `dom0=no-cpuid-faulting` to restore the older behaviour.
+*/
+enable_cpuid_faulting = nextd &&
+domain_has_cap(nextd, CAP_DISABLE_CPU_FAULT) &&
+(is_pv_domain(nextd) ||
+next->arch.msrs->misc_features_enables.cpuid_faulting);
+
+if (cpu_has_cpuid_faulting)
+set_cpuid_faulting(enable_cpuid_faulting);
+else
+amd_set_cpuid_user_dis(enable_cpuid_faulting);
+
+return;
+}
+
+if (ctxt_switch_masking)
+alternative_vcall(ctxt_switch_masking, next);
}
bool_t opt_cpu_info;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 4e20edc3bf..d65144da01 100644
--- a/xen/arch/
[RFC 4/6] capabilities: introduce console io as a domain capability
The field `is_console` suggests that the field represents a state of being or
posession, not that it reflects the privilege to access the console. In this
patch the field is renamed to capabilities to encapsulate the capabilities a
domain has been granted. The first capability being the ability to read/write
the Xen console.
Signed-off-by: Daniel P. Smith
---
xen/arch/arm/domain_build.c | 4 +++-
xen/include/xen/sched.h | 25 +++--
xen/include/xsm/dummy.h | 2 +-
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 51b4daefe1..ad7432b029 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -4076,7 +4076,9 @@ void __init create_domUs(void)
panic("Error creating domain %s (rc = %ld)\n",
dt_node_name(node), PTR_ERR(d));
-d->is_console = true;
+if ( ! domain_set_cap(d, CAP_CONSOLE_IO) )
+printk("failed setting console_io on %pd\n", d);
+
dt_device_set_used_by(node, d->domain_id);
rc = construct_domU(d, node);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index ec0f9baff6..b04fbe0565 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -472,8 +472,8 @@ struct domain
#define ROLE_HARDWARE_DOMAIN (1U<<2)
#define ROLE_XENSTORE_DOMAIN (1U<<3)
uint8_t role;
-/* Can this guest access the Xen console? */
-bool is_console;
+#define CAP_CONSOLE_IO (1U<<0)
+uint8_t capabilities;
/* Is this guest being debugged by dom0? */
bool debugger_attached;
/*
@@ -1146,6 +1146,27 @@ static always_inline bool is_hvm_vcpu(const struct vcpu
*v)
return is_hvm_domain(v->domain);
}
+static always_inline bool domain_has_cap(
+const struct domain *d, uint8_t cap)
+{
+return d->capabilities & cap;
+}
+
+static always_inline bool domain_set_cap(
+struct domain *d, uint8_t cap)
+{
+switch ( cap )
+{
+case CAP_CONSOLE_IO:
+d->capabilities |= cap;
+break;
+default:
+return false;
+}
+
+return domain_has_cap(d, cap);
+}
+
static always_inline bool hap_enabled(const struct domain *d)
{
/* sanitise_domain_config() rejects HAP && !HVM */
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 18f1ddd127..067ff1d111 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -268,7 +268,7 @@ static XSM_INLINE int cf_check xsm_console_io(
XSM_DEFAULT_ARG struct domain *d, int cmd)
{
XSM_ASSERT_ACTION(XSM_OTHER);
-if ( d->is_console )
+if ( domain_has_cap(d, CAP_CONSOLE_IO) )
return xsm_default_action(XSM_HOOK, d, NULL);
#ifdef CONFIG_VERBOSE_DEBUG
if ( cmd == CONSOLEIO_write )
--
2.20.1
[RFC 3/6] roles: add a role for xenstore domain
Expand the possible roles for a domain to include a role for the Xenstore
domain.
Signed-off-by: Daniel P. Smith
---
xen/common/domain.c | 3 +++
xen/include/xen/sched.h | 3 ++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 0ff1d52e3d..dbf055c559 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -633,6 +633,9 @@ struct domain *domain_create(domid_t domid,
d->role |= ROLE_HARDWARE_DOMAIN;
}
+if ( d->options & XEN_DOMCTL_CDF_xs_domain )
+d->role |= ROLE_XENSTORE_DOMAIN;
+
TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
lock_profile_register_struct(LOCKPROF_TYPE_PERDOM, d, domid);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 695f240326..ec0f9baff6 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -470,6 +470,7 @@ struct domain
#define ROLE_UNBOUNDED_DOMAIN (1U<<0)
#define ROLE_CONTROL_DOMAIN(1U<<1)
#define ROLE_HARDWARE_DOMAIN (1U<<2)
+#define ROLE_XENSTORE_DOMAIN (1U<<3)
uint8_t role;
/* Can this guest access the Xen console? */
bool is_console;
@@ -1165,7 +1166,7 @@ static inline bool is_vcpu_online(const struct vcpu *v)
static inline bool is_xenstore_domain(const struct domain *d)
{
-return d->options & XEN_DOMCTL_CDF_xs_domain;
+return d->role & ROLE_XENSTORE_DOMAIN;
}
static always_inline bool is_iommu_enabled(const struct domain *d)
--
2.20.1
[RFC 2/6] roles: provide abstraction for the possible domain roles
The existing concepts such as unbounded domain, ie. all powerful, control
domain and hardware domain are, effectively, roles the domains provide for the
system. Currently, these are represented with booleans within `struct domain`
or global domid variables that are compared against. This patch begins to
formalize these roles by replacing the `is_control` and `is_console`, along
with expanding the check against the global `hardware_domain` with a single
encapsulating role attribute in `struct domain`.
Signed-off-by: Daniel P. Smith
---
xen/arch/arm/domain_build.c | 2 ++
xen/arch/x86/setup.c| 2 ++
xen/common/domain.c | 14 +-
xen/include/xen/sched.h | 16 +---
xen/include/xsm/dummy.h | 4 ++--
xen/xsm/flask/hooks.c | 12 ++--
6 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 39b4ee03a5..51b4daefe1 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -4201,6 +4201,8 @@ void __init create_dom0(void)
if ( IS_ERR(dom0) )
panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
+dom0->role |= ROLE_UNBOUNDED_DOMAIN;
+
if ( alloc_dom0_vcpu0(dom0) == NULL )
panic("Error creating domain 0 vcpu0\n");
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2dbe9857aa..4e20edc3bf 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -905,6 +905,8 @@ static struct domain *__init create_dom0(const module_t
*image,
if ( IS_ERR(d) )
panic("Error creating d%u: %ld\n", domid, PTR_ERR(d));
+d->role |= ROLE_UNBOUNDED_DOMAIN;
+
init_dom0_cpuid_policy(d);
if ( alloc_dom0_vcpu0(d) == NULL )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8fb3c052f5..0ff1d52e3d 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -340,6 +340,14 @@ static int late_hwdom_init(struct domain *d)
setup_io_bitmap(dom0);
#endif
+/*
+ * "dom0" may have been created under the unbounded role, demote it from
+ * that role, reducing it to the control domain role and any other roles it
+ * may have been given.
+ */
+dom0->role &= ~(ROLE_UNBOUNDED_DOMAIN & ROLE_HARDWARE_DOMAIN);
+dom0->role |= ROLE_CONTROL_DOMAIN;
+
rcu_unlock_domain(dom0);
iommu_hwdom_init(d);
@@ -609,7 +617,10 @@ struct domain *domain_create(domid_t domid,
}
/* Sort out our idea of is_control_domain(). */
-d->is_privileged = flags & CDF_privileged;
+if ( flags & CDF_privileged )
+d->role |= ROLE_CONTROL_DOMAIN;
+else
+d->role &= ~ROLE_CONTROL_DOMAIN; /*ensure not set */
/* Sort out our idea of is_hardware_domain(). */
if ( is_initial_domain(d) || domid == hardware_domid )
@@ -619,6 +630,7 @@ struct domain *domain_create(domid_t domid,
old_hwdom = hardware_domain;
hardware_domain = d;
+d->role |= ROLE_HARDWARE_DOMAIN;
}
TRACE_1D(TRC_DOM0_DOM_ADD, d->domain_id);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index a9276a7bed..695f240326 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -467,8 +467,10 @@ struct domain
#endif
/* is node-affinity automatically computed? */
bool auto_node_affinity;
-/* Is this guest fully privileged (aka dom0)? */
-bool is_privileged;
+#define ROLE_UNBOUNDED_DOMAIN (1U<<0)
+#define ROLE_CONTROL_DOMAIN(1U<<1)
+#define ROLE_HARDWARE_DOMAIN (1U<<2)
+uint8_t role;
/* Can this guest access the Xen console? */
bool is_console;
/* Is this guest being debugged by dom0? */
@@ -1060,9 +1062,7 @@ void watchdog_domain_destroy(struct domain *d);
static always_inline bool is_initial_domain(const struct domain *d)
{
-static int init_domain_id = 0;
-
-return d->domain_id == init_domain_id;
+return d->role & ROLE_UNBOUNDED_DOMAIN;
}
/*
@@ -1076,7 +1076,8 @@ static always_inline bool is_hardware_domain(const struct
domain *d)
if ( IS_ENABLED(CONFIG_PV_SHIM_EXCLUSIVE) )
return false;
-return evaluate_nospec(d == hardware_domain);
+return evaluate_nospec(((d->role & ROLE_HARDWARE_DOMAIN) ||
+is_initial_domain(d)) && (d == hardware_domain));
}
/* This check is for functionality specific to a control domain */
@@ -1085,7 +1086,8 @@ static always_inline bool is_control_domain(const struct
domain *d)
if ( IS_ENABLED(CONFIG_PV_SHIM_EXCLUSIVE) )
return false;
-return evaluate_nospec(d->is_privileged);
+return evaluate_nospec((d->role & ROLE_CONTROL_DOMAIN) ||
+is_initial_domain(d));
}
#define VM_ASSIST(d, t) (test_bit(VMASST_TYPE_ ## t, &(d)->vm_assist))
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index 8671af1ba4..18f1ddd127 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -108,7 +108,7 @@ static XSM_INLINE int cf_ch
[RFC 1/6] dom0: replace explict zero checks
A legacy concept is that the initial domain will have a domain id of zero. As a
result there are places where a check that a domain is the inital domain is
determined by an explicit check that the domid is zero.
This commit seeks to abstract this check into a function call and replace all
check locations with the function call.
Signed-off-by: Daniel P. Smith
---
xen/common/domain.c | 4 ++--
xen/common/sched/arinc653.c | 2 +-
xen/common/sched/core.c | 4 ++--
xen/include/xen/sched.h | 7 +++
4 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 304aa04fa6..8fb3c052f5 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -309,7 +309,7 @@ static int late_hwdom_init(struct domain *d)
struct domain *dom0;
int rv;
-if ( d != hardware_domain || d->domain_id == 0 )
+if ( d != hardware_domain || is_initial_domain(d) )
return 0;
rv = xsm_init_hardware_domain(XSM_HOOK, d);
@@ -612,7 +612,7 @@ struct domain *domain_create(domid_t domid,
d->is_privileged = flags & CDF_privileged;
/* Sort out our idea of is_hardware_domain(). */
-if ( domid == 0 || domid == hardware_domid )
+if ( is_initial_domain(d) || domid == hardware_domid )
{
if ( hardware_domid < 0 || hardware_domid >= DOMID_FIRST_RESERVED )
panic("The value of hardware_dom must be a valid domain ID\n");
diff --git a/xen/common/sched/arinc653.c b/xen/common/sched/arinc653.c
index a82c0d7314..31e8270af3 100644
--- a/xen/common/sched/arinc653.c
+++ b/xen/common/sched/arinc653.c
@@ -404,7 +404,7 @@ a653sched_alloc_udata(const struct scheduler *ops, struct
sched_unit *unit,
* Add every one of dom0's units to the schedule, as long as there are
* slots available.
*/
-if ( unit->domain->domain_id == 0 )
+if ( is_initial_domain(unit->domain) )
{
entry = sched_priv->num_schedule_entries;
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
index 022f548652..210ad30f94 100644
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -585,7 +585,7 @@ int sched_init_vcpu(struct vcpu *v)
*/
sched_set_affinity(unit, cpumask_of(0), cpumask_of(0));
}
-else if ( d->domain_id == 0 && opt_dom0_vcpus_pin )
+else if ( is_initial_domain(d) && opt_dom0_vcpus_pin )
{
/*
* If dom0_vcpus_pin is specified, dom0 vCPUs are pinned 1:1 to
@@ -594,7 +594,7 @@ int sched_init_vcpu(struct vcpu *v)
sched_set_affinity(unit, cpumask_of(processor), &cpumask_all);
}
#ifdef CONFIG_X86
-else if ( d->domain_id == 0 )
+else if ( is_initial_domain(d) )
{
/*
* In absence of dom0_vcpus_pin instead, the hard and soft affinity of
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 854f3e32c0..a9276a7bed 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -1058,6 +1058,13 @@ void scheduler_disable(void);
void watchdog_domain_init(struct domain *d);
void watchdog_domain_destroy(struct domain *d);
+static always_inline bool is_initial_domain(const struct domain *d)
+{
+static int init_domain_id = 0;
+
+return d->domain_id == init_domain_id;
+}
+
/*
* Use this check when the following are both true:
* - Using this feature or interface requires full access to the hardware
--
2.20.1
[RFC 0/6] Hyperlaunch domain roles and capabilities
A goal of the hyperlaunch effort was to solidify the concept of the different types of domains the hypervisor has some notion around. The initial approach was to formalize these types as roles enforced through the XSM framework. In this RFC, a simpler approach is taken to lay a foundation of domain roles and assignable capabilities. The approach in this series is to collapse the relevant bools in struct domain into a pair of bit flag entries that represent roles and capabilities that a domain is assigned. Daniel P. Smith (6): dom0: replace explict zero checks roles: provide abstraction for the possible domain roles roles: add a role for xenstore domain capabilities: introduce console io as a domain capability capabilities: add dom0 cpu faulting disable capabilities: convert attach debugger into a capability xen/arch/arm/domain_build.c | 6 ++- xen/arch/x86/cpu-policy.c | 2 +- xen/arch/x86/cpu/common.c | 82 - xen/arch/x86/hvm/svm/svm.c | 8 ++-- xen/arch/x86/hvm/vmx/realmode.c | 2 +- xen/arch/x86/hvm/vmx/vmcs.c | 2 +- xen/arch/x86/hvm/vmx/vmx.c | 10 ++-- xen/arch/x86/setup.c| 6 +++ xen/arch/x86/traps.c| 6 ++- xen/common/domain.c | 21 +++-- xen/common/domctl.c | 6 ++- xen/common/sched/arinc653.c | 2 +- xen/common/sched/core.c | 4 +- xen/include/xen/sched.h | 58 +++ xen/include/xsm/dummy.h | 6 +-- xen/xsm/flask/hooks.c | 12 ++--- 16 files changed, 150 insertions(+), 83 deletions(-) -- 2.20.1
Re: [PATCH RESEND v9 33/36] KVM: VMX: Add VMX_DO_FRED_EVENT_IRQOFF for IRQ/NMI handling
On Tue, Aug 01, 2023, Peter Zijlstra wrote: > On Tue, Aug 01, 2023 at 07:01:15PM +, Sean Christopherson wrote: > > The spec I have from May 2022 says the NMI bit colocated with CS, not SS. > > And > > the cover letter's suggestion to use a search engine to find the spec ain't > > exactly helpful, that just gives me the same "May 2022 Revision 3.0" spec. > > So > > either y'all have a spec that I can't find, or this is wrong. > > https://intel.com/sdm > > is a useful shorthand I've recently been told about. Hallelujah! > On that page is also "Flexible Return and Event Delivery Specification", when > clicked it will gift you a FRED v5.0 PDF. Worked for me, too. Thanks!
Re: [PATCH RESEND v9 33/36] KVM: VMX: Add VMX_DO_FRED_EVENT_IRQOFF for IRQ/NMI handling
On Tue, Aug 01, 2023 at 07:01:15PM +, Sean Christopherson wrote: > The spec I have from May 2022 says the NMI bit colocated with CS, not SS. And > the cover letter's suggestion to use a search engine to find the spec ain't > exactly helpful, that just gives me the same "May 2022 Revision 3.0" spec. So > either y'all have a spec that I can't find, or this is wrong. https://intel.com/sdm is a useful shorthand I've recently been told about. On that page is also "Flexible Return and Event Delivery Specification", when clicked it will gift you a FRED v5.0 PDF.
Re: [PATCH RESEND v9 33/36] KVM: VMX: Add VMX_DO_FRED_EVENT_IRQOFF for IRQ/NMI handling
On Tue, Aug 01, 2023, Xin Li wrote:
>
> diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
> index 07e927d4d099..5ee6a57b59a5 100644
> --- a/arch/x86/kvm/vmx/vmenter.S
> +++ b/arch/x86/kvm/vmx/vmenter.S
> @@ -2,12 +2,14 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> #include "kvm-asm-offsets.h"
> #include "run_flags.h"
> +#include "../../entry/calling.h"
Rather than do the low level PUSH_REGS and POP_REGS, I vote to have core code
expose a FRED-specific wrapper for invoking external_interrupt(). More below.
>
> #define WORD_SIZE (BITS_PER_LONG / 8)
>
> @@ -31,6 +33,80 @@
> #define VCPU_R15 __VCPU_REGS_R15 * WORD_SIZE
> #endif
>
> +#ifdef CONFIG_X86_FRED
> +.macro VMX_DO_FRED_EVENT_IRQOFF branch_insn branch_target nmi=0
objtool isn't happy.
arch/x86/kvm/vmx/vmenter.o: warning: objtool:
vmx_do_fred_interrupt_irqoff+0x6c: return with modified stack frame
arch/x86/kvm/vmx/vmenter.o: warning: objtool: vmx_do_fred_nmi_irqoff+0x37:
sibling call from callable instruction with modified stack frame
The "return with modified stack frame" goes away with my suggested changes, but
the sibling call remains for the NMI case due to the JMP instead of a call.
> + /*
> + * Unconditionally create a stack frame, getting the correct RSP on the
> + * stack (for x86-64) would take two instructions anyways, and RBP can
> + * be used to restore RSP to make objtool happy (see below).
> + */
> + push %_ASM_BP
> + mov %_ASM_SP, %_ASM_BP
The frame stuff is worth throwing in a macro, if only to avoid a copy+pasted
comment, which by the by, is wrong. (a) it's ERETS, not IRET. (b) the IRQ does
a vanilla RET, not ERETS. E.g. like so:
.macro VMX_DO_EVENT_FRAME_BEGIN
/*
* Unconditionally create a stack frame, getting the correct RSP on the
* stack (for x86-64) would take two instructions anyways, and RBP can
* be used to restore RSP to make objtool happy (see below).
*/
push %_ASM_BP
mov %_ASM_SP, %_ASM_BP
.endm
.macro VMX_DO_EVENT_FRAME_END
/*
* "Restore" RSP from RBP, even though {E,I}RET has already unwound RSP
* to the correct value *in most cases*. KVM's IRQ handling with FRED
* doesn't do ERETS, and objtool doesn't know the callee will IRET/ERET
* and, without the explicit restore, thinks the stack is getting
walloped.
* Using an unwind hint is problematic due to x86-64's dynamic
alignment.
*/
mov %_ASM_BP, %_ASM_SP
pop %_ASM_BP
.endm
> +
> + /*
> + * Don't check the FRED stack level, the call stack leading to this
> + * helper is effectively constant and shallow (relatively speaking).
> + *
> + * Emulate the FRED-defined redzone and stack alignment.
> + */
> + sub $(FRED_CONFIG_REDZONE_AMOUNT << 6), %rsp
> + and $FRED_STACK_FRAME_RSP_MASK, %rsp
> +
> + /*
> + * A FRED stack frame has extra 16 bytes of information pushed at the
> + * regular stack top compared to an IDT stack frame.
There is pretty much no chance that anyone remembers the layout of an IDT stack
frame off the top of their head. I.e. saying "FRED has 16 bytes more" isn't all
that useful. It also fails to capture the fact that FRED stuff a hell of a lot
more information in those "common" 48 bytes.
It'll be hard/impossible to capture all of the overload info in a comment, but
showing the actual layout of the frame would be super helpful, e.g. something
like
this
/*
* FRED stack frames are always 64 bytes:
*
* --
* | Bytes | Usage |
* -|
* | 63:56 | Reserved |
* | 55:48 | Event Data|
* | 47:40 | SS + Event Info |
* | 39:32 | RSP |
* | 31:24 | RFLAGS|
* | 23:16 | CS + Aux Info |
* | 15:8 | RIP |
* | 7:0 | Error Code|
* --
*/
> + */
> + push $0 /* Reserved by FRED, must be 0 */
> + push $0 /* FRED event data, 0 for NMI and external interrupts */
> +
> + shl $32, %rdi /* FRED event type and vector */
> + .if \nmi
> + bts $FRED_SSX_NMI_BIT, %rdi /* Set the NMI bit */
The spec I have from May 2022 says the NMI bit colocated with CS, not SS. And
the cover letter's suggestion to use a search engine to find the spec ain't
exactly helpful, that just gives me the same "May 2022 Revision 3.0" spec. So
either y'all have a spec that I can't find, or this is wrong.
> + .endif
> + bts $FRED_SSX_64_BIT_MODE_BIT, %rdi /* Set the 64-bit mode */
> + or $__KERNEL_DS, %rdi
> + push %rdi
> + push %rbp
> + pushf
> +
Re: [PATCH 4/5] xen/ppc: Parse device tree for OPAL node on PowerNV
On 7/31/23 11:06 AM, Jan Beulich wrote: > On 28.07.2023 23:35, Shawn Anastasio wrote: >> --- a/xen/arch/ppc/arch.mk >> +++ b/xen/arch/ppc/arch.mk >> @@ -10,5 +10,5 @@ CFLAGS += -mstrict-align -mcmodel=medium -mabi=elfv2 -fPIC >> -mno-altivec -mno-vsx >> LDFLAGS += -m elf64lppc >> >> # TODO: Drop override when more of the build is working >> -override ALL_OBJS-y = arch/$(SRCARCH)/built_in.o >> -override ALL_LIBS-y = >> +override ALL_OBJS-y = arch/$(SRCARCH)/built_in.o common/libfdt/built_in.o >> lib/built_in.o >> +override ALL_LIBS-y = lib/lib.a > > Can't you drop the ALL_LIBS-y override right here? Just tested this and it does indeed work. I'll drop the override in v2. > Jan Thanks, Shawn
Re: [PATCH 2/5] xen/ppc: Switch to medium PIC code model
On 8/1/23 7:20 AM, Jan Beulich wrote: > On 28.07.2023 23:35, Shawn Anastasio wrote: >> @@ -11,16 +13,19 @@ ENTRY(start) >> FIXUP_ENDIAN >> >> /* set up the TOC pointer */ >> -LOAD_IMM32(%r2, .TOC.) >> +bcl 20, 31, .+4 >> +1: mflr%r12 >> +addis %r2, %r12, .TOC.-1b@ha >> +addi%r2, %r2, .TOC.-1b@l >> >> /* set up the initial stack */ >> -LOAD_IMM32(%r1, cpu0_boot_stack) >> +LOAD_REG_ADDR(%r1, cpu0_boot_stack) > > Question: Would perhaps make sense to use %sp and %rtoc in place of > %r1 and %r2 (not just here of course)? In my opinion, usage of the aliased register names ends up making the code less clear.
Re: [PATCH 00/24] ALSA: Generic PCM copy ops using sockptr_t
On Tue, Aug 01, 2023 at 02:54:45PM +0200, Takashi Iwai wrote:
> On Mon, 31 Jul 2023 21:40:20 +0200,
> Mark Brown wrote:
> > On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
> > > Mark Brown wrote:
> >
> > > > It really feels like we ought to rename, or add an alias for, the type
> > > > if we're going to start using it more widely - it's not helping to make
> > > > the code clearer.
> >
> > > That was my very first impression, too, but I changed my mind after
> > > seeing the already used code. An alias might work, either typedef or
> > > define genptr_t or such as sockptr_t. But we'll need to copy the
> > > bunch of helper functions, too...
> >
> > I would predict that if the type becomes more widely used that'll happen
> > eventually and the longer it's left the more work it'll be.
>
> That's true. The question is how more widely it'll be used, then.
>
> Is something like below what you had in mind, too?
I agree with your proposal (uniptr_t also works for me), but see below.
...
> +#include
> +#include
But let make the list of the headers right this time.
It seems to me that
err.h
minmax.h // maybe not, see a remark at the bottom
string.h
types.h
are missing.
More below.
...
> + void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN);
> +
> + if (!p)
> + return ERR_PTR(-ENOMEM);
This can use cleanup.h.
> + if (copy_from_uniptr(p, src, len)) {
> + kfree(p);
> + return ERR_PTR(-EFAULT);
> + }
> + return p;
> +}
> +
> +static inline void *memdup_uniptr_nul(uniptr_t src, size_t len)
> +{
> + char *p = kmalloc_track_caller(len + 1, GFP_KERNEL);
Ditto.
> + if (!p)
> + return ERR_PTR(-ENOMEM);
> + if (copy_from_uniptr(p, src, len)) {
> + kfree(p);
> + return ERR_PTR(-EFAULT);
> + }
> + p[len] = '\0';
> + return p;
> +}
...
> +static inline long strncpy_from_uniptr(char *dst, uniptr_t src, size_t count)
> +{
> + if (uniptr_is_kernel(src)) {
> + size_t len = min(strnlen(src.kernel, count - 1) + 1, count);
I didn't get why do we need min()? To check the count == 0 case?
Wouldn't
size_t len;
len = strnlen(src.kernel, count);
if (len == 0)
return 0;
/* Copy a trailing NUL if found */
if (len < count)
len++;
be a good equivalent?
> + memcpy(dst, src.kernel, len);
> + return len;
> + }
> + return strncpy_from_user(dst, src.user, count);
> +}
...
> +static inline int check_zeroed_uniptr(uniptr_t src, size_t offset, size_t
> size)
> +{
> + if (!uniptr_is_kernel(src))
Why not to align all the functions to use same conditional (either always
positive or negative)?
> + return check_zeroed_user(src.user + offset, size);
> + return memchr_inv(src.kernel + offset, 0, size) == NULL;
> +}
...
Taking all remarks into account I would rather go with sockptr.h being
untouched for now, just a big
/* DO NOT USE, it's obsolete, use uniptr.h instead! */
to be added.
--
With Best Regards,
Andy Shevchenko
[PATCH v2] arm32: Avoid using solaris syntax for .section directive
Assembler from binutils 2.41 rejects [1] this syntax .section "name"[, flags...] where flags could be #alloc, #write, #execinstr, #exclude, and #tls [2] It is almost like a regression compared to 2.40 or older release, It likely went unnoticed so far because Linux kernel changed to GNU syntax already in 5.5, to allow building with Clang's integrated assembler. Switch to using GNU syntax .section name[, "flags"[, @type]] [1] https://sourceware.org/bugzilla/show_bug.cgi?id=11601 [2] https://sourceware.org/binutils/docs-2.41/as.html#Section Signed-off-by: Khem Raj --- v1 -> v2: - Improvise on commit message - Make similar change in xen/arch/arm/dtb.S xen/arch/arm/arm32/proc-v7.S | 6 +++--- xen/arch/arm/dtb.S | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S index c90a31d80f..6d3d19b873 100644 --- a/xen/arch/arm/arm32/proc-v7.S +++ b/xen/arch/arm/arm32/proc-v7.S @@ -29,7 +29,7 @@ brahma15mp_init: mcr CP32(r0, ACTLR) mov pc, lr -.section ".proc.info", #alloc +.section .proc.info, "a" .type __v7_ca15mp_proc_info, #object __v7_ca15mp_proc_info: .long 0x410FC0F0 /* Cortex-A15 */ @@ -38,7 +38,7 @@ __v7_ca15mp_proc_info: .long caxx_processor .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info -.section ".proc.info", #alloc +.section .proc.info, "a" .type __v7_ca7mp_proc_info, #object __v7_ca7mp_proc_info: .long 0x410FC070 /* Cortex-A7 */ @@ -47,7 +47,7 @@ __v7_ca7mp_proc_info: .long caxx_processor .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info -.section ".proc.info", #alloc +.section .proc.info, "a" .type __v7_brahma15mp_proc_info, #object __v7_brahma15mp_proc_info: .long 0x420F00F0 /* Broadcom Brahma-B15 */ diff --git a/xen/arch/arm/dtb.S b/xen/arch/arm/dtb.S index c39f3a095c..386f83ba64 100644 --- a/xen/arch/arm/dtb.S +++ b/xen/arch/arm/dtb.S @@ -1,3 +1,3 @@ -.section .dtb,#alloc +.section .dtb, "a" GLOBAL(_sdtb) .incbin CONFIG_DTB_FILE -- 2.41.0
RE: [PATCH RESEND v9 00/36] x86: enable FRED for x86-64
> > I also believe there is a kernel.org service for sending patch series, > > but i'm not sure I remember the details. > > https://b4.docs.kernel.org/en/latest/contributor/send.html It says: The kernel.org endpoint can only be used for kernel.org-hosted projects. If there are no recognized mailing lists in the to/cc headers, then the submission will be rejected. If I want to test the email sending service, how could I test it with sending just to myself? Maybe it allows only sending to the sender.
RE: [PATCH RESEND v9 00/36] x86: enable FRED for x86-64
> > Resend because the mail system failed to deliver some messages yesterday. > > The one from yesterday came in 6 thread groups: 0-25, 26, 27, 28, 29, 30-36, > while the one from today comes in 2 thread groups: 0-26, 27-36. Which I > suppose one can count as an improvement :/ Sigh, sorry for the chaos. > > Seriously, it should not be hard to send 36 patches in a single thread. No, but it worked fine before thus I didn't realize there an email service policy which prevents sending email to too many recipients in a short period (I have a long CC list in this v9 patch set). > I see you're trying to send through the regular corporate email > trainwreck; do you have a linux.intel.com account? Or really anything > else besides intel.com? You can try sending the series to yourself to > see if it arrives correctly as a whole before sending it out to the list > again. I did try sending to myself before to LKML, and it worked fine. But now you know why it happened. As mentioned, I should avoid "the regular corporate email trainwreck" before doing it again. Working on it...
Re: [PATCH v7 0/9] Allow dynamic allocation of software IO TLB bounce buffers
On 8/1/2023 6:03 PM, Christoph Hellwig wrote: > Thanks, > > I've applied this to a new swiotlb-dynamic branch that I'll pull into > the dma-mapping for-next tree. Thank you. I guess I can prepare some follow-up series now. ;-) Petr T
Re: [PATCH] common: move simple_strto{,u}l{,l}() to lib/
On 8/1/23 5:34 AM, Jan Beulich wrote:
> Convert style from a Xen/Linux mix to pure Xen while doing the move. No
> other changes, despite having been heavily tempted to do some - at the
> very least to make simple_strtoul() and simple_strtoull() the same in
> how they deal with non-numeric digits.
>
> Requested-by: Shawn Anastasio
> Signed-off-by: Jan Beulich
> ---
> Further changes I was considering:
> - "value" doesn't need to be unsigned long, and even less so unsigned
> long long,
> - strtoull.c could simply include strtoul.c, after #define-ing "long"
> to "long long" and "simple_strtoul" to "simple_strtoull",
> - "else if ( base == 16 )" wants folding with its inner if().
>
> --- a/xen/common/vsprintf.c
> +++ b/xen/common/vsprintf.c
> @@ -24,108 +24,6 @@
> #include
> #include
>
> -/**
> - * simple_strtoul - convert a string to an unsigned long
> - * @cp: The start of the string
> - * @endp: A pointer to the end of the parsed string will be placed here
> - * @base: The number base to use
> - */
> -unsigned long simple_strtoul(
> -const char *cp, const char **endp, unsigned int base)
> -{
> -unsigned long result = 0,value;
> -
> -if (!base) {
> -base = 10;
> -if (*cp == '0') {
> -base = 8;
> -cp++;
> -if ((toupper(*cp) == 'X') && isxdigit(cp[1])) {
> -cp++;
> -base = 16;
> -}
> -}
> -} else if (base == 16) {
> -if (cp[0] == '0' && toupper(cp[1]) == 'X')
> -cp += 2;
> -}
> -while (isxdigit(*cp) &&
> - (value = isdigit(*cp) ? *cp-'0' : toupper(*cp)-'A'+10) < base) {
> -result = result*base + value;
> -cp++;
> -}
> -if (endp)
> -*endp = cp;
> -return result;
> -}
> -
> -EXPORT_SYMBOL(simple_strtoul);
> -
> -/**
> - * simple_strtol - convert a string to a signed long
> - * @cp: The start of the string
> - * @endp: A pointer to the end of the parsed string will be placed here
> - * @base: The number base to use
> - */
> -long simple_strtol(const char *cp, const char **endp, unsigned int base)
> -{
> -if(*cp=='-')
> -return -simple_strtoul(cp+1,endp,base);
> -return simple_strtoul(cp,endp,base);
> -}
> -
> -EXPORT_SYMBOL(simple_strtol);
> -
> -/**
> - * simple_strtoull - convert a string to an unsigned long long
> - * @cp: The start of the string
> - * @endp: A pointer to the end of the parsed string will be placed here
> - * @base: The number base to use
> - */
> -unsigned long long simple_strtoull(
> -const char *cp, const char **endp, unsigned int base)
> -{
> -unsigned long long result = 0,value;
> -
> -if (!base) {
> -base = 10;
> -if (*cp == '0') {
> -base = 8;
> -cp++;
> -if ((toupper(*cp) == 'X') && isxdigit(cp[1])) {
> -cp++;
> -base = 16;
> -}
> -}
> -} else if (base == 16) {
> -if (cp[0] == '0' && toupper(cp[1]) == 'X')
> -cp += 2;
> -}
> -while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp-'0' : (islower(*cp)
> - ?
> toupper(*cp) : *cp)-'A'+10) < base) {
> -result = result*base + value;
> -cp++;
> -}
> -if (endp)
> -*endp = cp;
> -return result;
> -}
> -
> -EXPORT_SYMBOL(simple_strtoull);
> -
> -/**
> - * simple_strtoll - convert a string to a signed long long
> - * @cp: The start of the string
> - * @endp: A pointer to the end of the parsed string will be placed here
> - * @base: The number base to use
> - */
> -long long simple_strtoll(const char *cp,const char **endp,unsigned int base)
> -{
> -if(*cp=='-')
> -return -simple_strtoull(cp+1,endp,base);
> -return simple_strtoull(cp,endp,base);
> -}
> -
> static int skip_atoi(const char **s)
> {
> int i=0;
> --- a/xen/lib/Makefile
> +++ b/xen/lib/Makefile
> @@ -28,6 +28,10 @@ lib-y += strrchr.o
> lib-y += strsep.o
> lib-y += strspn.o
> lib-y += strstr.o
> +lib-y += strtol.o
> +lib-y += strtoll.o
> +lib-y += strtoul.o
> +lib-y += strtoull.o
> lib-$(CONFIG_X86) += xxhash32.o
> lib-$(CONFIG_X86) += xxhash64.o
>
> --- /dev/null
> +++ b/xen/lib/strtol.c
> @@ -0,0 +1,28 @@
> +/*
> + * Copyright (C) 1991, 1992 Linus Torvalds
> + */
> +
> +#include
> +
> +/**
> + * simple_strtol - convert a string to a signed long
> + * @cp: The start of the string
> + * @endp: A pointer to the end of the parsed string will be placed here
> + * @base: The number base to use
> + */
> +long simple_strtol(const char *cp, const char **endp, unsigned int base)
> +{
> +if ( *cp == '-' )
> +return -simple_strtoul(cp + 1, endp, base);
> +return simple_strtoul(cp, endp, base);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> --- /dev/null
> +++ b/xen/lib/s
[PATCH 2/2] fdt: make fdt handling reusable across arch
This refactors reusable code from Arm's bootfdt.c and device-tree.h that is
general fdt handling code. The Kconfig parameter CORE_DEVICE_TREE is
introduced for when the ability of parsing DTB files is needed by a capability
such as hyperlaunch.
Signed-off-by: Daniel P. Smith
---
MAINTAINERS | 2 +
xen/arch/arm/bootfdt.c| 141 +--
xen/common/Kconfig| 4 +
xen/common/Makefile | 3 +-
xen/common/fdt.c | 153 ++
xen/include/xen/device_tree.h | 50 +--
xen/include/xen/fdt.h | 79 ++
7 files changed, 242 insertions(+), 190 deletions(-)
create mode 100644 xen/common/fdt.c
create mode 100644 xen/include/xen/fdt.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 694412a961..b7fc3ed805 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -303,7 +303,9 @@ F: xen/common/libfdt/
F: xen/common/device_tree.c
F: xen/include/xen/libfdt/
F: xen/include/xen/device_tree.h
+F: include/xen/fdt.h
F: xen/drivers/passthrough/device_tree.c
+F: common/fdt.c
ECLAIR
R: Simone Ballarin
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 2673ad17a1..cdf4f17789 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -12,79 +12,11 @@
#include
#include
#include
+#include
#include
#include
#include
-static bool __init device_tree_node_matches(const void *fdt, int node,
-const char *match)
-{
-const char *name;
-size_t match_len;
-
-name = fdt_get_name(fdt, node, NULL);
-match_len = strlen(match);
-
-/* Match both "match" and "match@..." patterns but not
- "match-foo". */
-return strncmp(name, match, match_len) == 0
-&& (name[match_len] == '@' || name[match_len] == '\0');
-}
-
-static bool __init device_tree_node_compatible(const void *fdt, int node,
- const char *match)
-{
-int len, l;
-const void *prop;
-
-prop = fdt_getprop(fdt, node, "compatible", &len);
-if ( prop == NULL )
-return false;
-
-while ( len > 0 ) {
-if ( !dt_compat_cmp(prop, match) )
-return true;
-l = strlen(prop) + 1;
-prop += l;
-len -= l;
-}
-
-return false;
-}
-
-void __init device_tree_get_reg(const __be32 **cell, uint32_t address_cells,
-uint32_t size_cells, paddr_t *start,
-paddr_t *size)
-{
-uint64_t dt_start, dt_size;
-
-/*
- * dt_next_cell will return uint64_t whereas paddr_t may not be 64-bit.
- * Thus, there is an implicit cast from uint64_t to paddr_t.
- */
-dt_start = dt_next_cell(address_cells, cell);
-dt_size = dt_next_cell(size_cells, cell);
-
-if ( dt_start != (paddr_t)dt_start )
-{
-printk("Physical address greater than max width supported\n");
-WARN();
-}
-
-if ( dt_size != (paddr_t)dt_size )
-{
-printk("Physical size greater than max width supported\n");
-WARN();
-}
-
-/*
- * Xen will truncate the address/size if it is greater than the maximum
- * supported width and it will give an appropriate warning.
- */
-*start = dt_start;
-*size = dt_size;
-}
-
static int __init device_tree_get_meminfo(const void *fdt, int node,
const char *prop_name,
u32 address_cells, u32 size_cells,
@@ -135,77 +67,6 @@ static int __init device_tree_get_meminfo(const void *fdt,
int node,
return 0;
}
-u32 __init device_tree_get_u32(const void *fdt, int node,
- const char *prop_name, u32 dflt)
-{
-const struct fdt_property *prop;
-
-prop = fdt_get_property(fdt, node, prop_name, NULL);
-if ( !prop || prop->len < sizeof(u32) )
-return dflt;
-
-return fdt32_to_cpu(*(uint32_t*)prop->data);
-}
-
-/**
- * device_tree_for_each_node - iterate over all device tree sub-nodes
- * @fdt: flat device tree.
- * @node: parent node to start the search from
- * @func: function to call for each sub-node.
- * @data: data to pass to @func.
- *
- * Any nodes nested at DEVICE_TREE_MAX_DEPTH or deeper are ignored.
- *
- * Returns 0 if all nodes were iterated over successfully. If @func
- * returns a value different from 0, that value is returned immediately.
- */
-int __init device_tree_for_each_node(const void *fdt, int node,
- device_tree_node_func func,
- void *data)
-{
-/*
- * We only care about relative depth increments, assume depth of
- * node is 0 for simplicity.
- */
-int depth = 0;
-const int first_node = node;
-u32 address_cells[DEVICE_TREE_MAX_DEPTH];
-u32 size_cells[DEVICE_TREE_MAX_DEPTH];
-int ret;
-
-do {
-const char
[PATCH 1/2] docs: update hyperlaunch device tree
With on going development of hyperlaunch, changes to the device tree definitions
has been necessary. This commit updates the specification for all current
changes
along with changes expected to be made in finalizing the capability.
This commit also adds a HYPERLAUNCH section to the MAINTAINERS file and places
this documentation under its purview. It also reserves the path
`xen/common/domain-builder` for the hyperlaunch domain builder code base.
Signed-off-by: Daniel P. Smith
---
MAINTAINERS | 9 +
.../designs/launch/hyperlaunch-devicetree.rst | 566 ++
2 files changed, 309 insertions(+), 266 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index d8a02a6c19..694412a961 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -332,6 +332,15 @@ M: Nick Rosbrook
S: Maintained
F: tools/golang
+HYPERLAUNCH
+M: Daniel P. Smith
+M: Christopher Clark
+W: https://wiki.xenproject.org/wiki/Hyperlaunch
+S: Supported
+F: docs/design/launch/hyperlaunch.rst
+F: docs/design/launch/hyperlaunch-devicetree.rst
+F: xen/common/domain-builder/
+
HYPFS
M: Juergen Gross
S: Supported
diff --git a/docs/designs/launch/hyperlaunch-devicetree.rst
b/docs/designs/launch/hyperlaunch-devicetree.rst
index b49c98cfbd..0bc719e4ae 100644
--- a/docs/designs/launch/hyperlaunch-devicetree.rst
+++ b/docs/designs/launch/hyperlaunch-devicetree.rst
@@ -2,10 +2,11 @@
Xen Hyperlaunch Device Tree Bindings
-
-The Xen Hyperlaunch device tree adopts the dom0less device tree structure and
-extends it to meet the requirements for the Hyperlaunch capability. The primary
-difference is the introduction of the ``hypervisor`` node that is under the
-``/chosen`` node. The move to a dedicated node was driven by:
+The Xen Hyperlaunch device tree is informed by the dom0less device tree
+structure with extensions to meet the requirements for the Hyperlaunch
+capability. A major depature from the dom0less device tree is the introduction
+of the ``hypervisor`` node that is under the ``/chosen`` node. The move to a
+dedicated node was driven by:
1. Reduces the need to walk over nodes that are not of interest, e.g. only
nodes of interest should be in ``/chosen/hypervisor``
@@ -13,331 +14,364 @@ difference is the introduction of the ``hypervisor`` node
that is under the
2. Allows for the domain construction information to easily be sanitized by
simple removing the ``/chosen/hypervisor`` node.
-Example Configuration
--
-
-Below are two example device tree definitions for the hypervisor node. The
-first is an example of a multiboot-based configuration for x86 and the second
-is a module-based configuration for Arm.
-
-Multiboot x86 Configuration:
-
-
-::
-
-hypervisor {
-#address-cells = <1>;
-#size-cells = <0>;
-compatible = “hypervisor,xen”
-
-// Configuration container
-config {
-compatible = "xen,config";
-
-module {
-compatible = "module,microcode", "multiboot,module";
-mb-index = <1>;
-};
-
-module {
-compatible = "module,xsm-policy", "multiboot,module";
-mb-index = <2>;
-};
-};
-
-// Boot Domain definition
-domain {
-compatible = "xen,domain";
-
-domid = <0x7FF5>;
-
-// FUNCTION_NONE(0)
-// FUNCTION_BOOT(1 << 0)
-// FUNCTION_CRASH (1 << 1)
-// FUNCTION_CONSOLE (1 << 2)
-// FUNCTION_XENSTORE(1 << 30)
-// FUNCTION_LEGACY_DOM0 (1 << 31)
-functions = <0x0001>;
-
-memory = <0x0 0x2>;
-cpus = <1>;
-module {
-compatible = "module,kernel", "multiboot,module";
-mb-index = <3>;
-};
-
-module {
-compatible = "module,ramdisk", "multiboot,module";
-mb-index = <4>;
-};
-module {
-compatible = "module,config", "multiboot,module";
-mb-index = <5>;
-};
-
-// Classic Dom0 definition
-domain {
-compatible = "xen,domain";
-
-domid = <0>;
-
-// PERMISSION_NONE (0)
-// PERMISSION_CONTROL (1 << 0)
-// PERMISSION_HARDWARE (1 << 1)
-permissions = <3>;
-
-// FUNCTION_NONE(0)
-// FUNCTION_BOOT(1 << 0)
-// FUNCTION_CRASH (1 << 1)
-// FUNCTION_CONSOLE (1 << 2)
-// FUNCTION_XENSTORE(1 << 30)
-// FUNCTION_LEGACY_DOM0 (1 << 31)
-functions = <0xC006>;
-
-// MODE_PARAVIRTUALIZED (1 << 0) /*
[PATCH 0/2] Rebranding dom0less to hyperlaunch part 1
This is the first series of the proposal put forth on moving to have dom0less folded under and thus rebranded as a part of hyperlaunch. As laid out in the proposal, the series updates the hyperlaunch device tree documentation and applies the general refactoring of FDT parsing to make core logic common. Daniel P. Smith (2): docs: update hyperlaunch device tree fdt: make fdt handling reusable across arch MAINTAINERS | 11 + .../designs/launch/hyperlaunch-devicetree.rst | 566 ++ xen/arch/arm/bootfdt.c| 141 + xen/common/Kconfig| 4 + xen/common/Makefile | 3 +- xen/common/fdt.c | 153 + xen/include/xen/device_tree.h | 50 +- xen/include/xen/fdt.h | 79 +++ 8 files changed, 551 insertions(+), 456 deletions(-) create mode 100644 xen/common/fdt.c create mode 100644 xen/include/xen/fdt.h -- 2.20.1
[PATCH] console: generalize the ability for domU access
This patch reworks the console rotation logic to provide a general mechanism to
incorporate domU in to the rotation. It does so by walking the domain list
using the XSM console privlege check to determine if the domain is given access.
In reworking the rotation logic, the assumption that the hardware domain is the
first domain created is removed and is changed to explicitly locate the
hardware domain at boot. As part of this removal, the reliance on the
`max_init_domid` global is eliminated, allowing the removal of a compatibility
`#define` for the x86 arch.
Suggested-by: Stefano Stabellini
Signed-off-by: Daniel P. Smith
---
This patch was developed as part of enabling PVH domain construction for
hyperlaunch. There will be a corresponding Linux patch to enable the kernel to
make use of console_io when running as a DomU. For x86, this will only be
usable by leveraging FLASK to assign the privilege to a label that is applied
to domains desired to be able to use the Xen console. When hyperlaunch is
merged, it will become possible to assign the privilege without FLASK. This
should work for Arm environments with or without vp1011 backend.
xen/arch/x86/include/asm/setup.h | 2 -
xen/drivers/char/console.c | 134 +++
2 files changed, 99 insertions(+), 37 deletions(-)
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index 51fce66607..5242dfcf93 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -64,6 +64,4 @@ extern bool opt_dom0_verbose;
extern bool opt_dom0_cpuid_faulting;
extern bool opt_dom0_msr_relaxed;
-#define max_init_domid (0)
-
#endif
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 0e410fa086..f5b759898e 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -473,45 +473,102 @@ static void cf_check dump_console_ring_key(unsigned char
key)
*/
static unsigned int __read_mostly console_rx = 0;
-#define max_console_rx (max_init_domid + 1)
+#define CON_RX_DOMID (console_rx - 1)
/* Make sure to rcu_unlock_domain after use */
struct domain *console_input_domain(void)
{
if ( console_rx == 0 )
return NULL;
-return rcu_lock_domain_by_id(console_rx - 1);
+return rcu_lock_domain_by_id(CON_RX_DOMID);
}
static void switch_serial_input(void)
{
-unsigned int next_rx = console_rx;
+struct domain *next, *d = console_input_domain();
-/*
- * Rotate among Xen, dom0 and boot-time created domUs while skipping
- * switching serial input to non existing domains.
- */
-for ( ; ; )
+if ( d == NULL )
{
-struct domain *d;
+if ( hardware_domain )
+{
+console_rx = hardware_domain->domain_id + 1;
+printk("*** Serial input to DOM%d", CON_RX_DOMID);
+goto out; //print switch_code statement & newline
+}
+else
+{
+for_each_domain(next)
+{
+if ( xsm_console_io(XSM_OTHER, next, CONSOLEIO_read) == 0 )
+{
+console_rx = next->domain_id + 1;
+printk("*** Serial input to DOM%d", CON_RX_DOMID);
+goto out; //print switch_code statement & newline
+}
+}
-if ( next_rx++ >= max_console_rx )
+console_rx = 0;
+printk("*** Serial input to Xen");
+goto out;
+}
+}
+
+for ( next = rcu_dereference(d->next_in_list); next != NULL;
+ next = rcu_dereference(next->next_in_list) )
+{
+if ( hardware_domain && next == hardware_domain )
{
console_rx = 0;
printk("*** Serial input to Xen");
-break;
+goto out;
}
-d = rcu_lock_domain_by_id(next_rx - 1);
-if ( d )
+if ( xsm_console_io(XSM_OTHER, next, CONSOLEIO_read) == 0 )
{
-rcu_unlock_domain(d);
-console_rx = next_rx;
-printk("*** Serial input to DOM%u", next_rx - 1);
-break;
+console_rx = next->domain_id + 1;
+printk("*** Serial input to DOM%d", CON_RX_DOMID);
+goto out;
+}
+}
+
+/*
+ * Hit the end of the domain list and instead of assuming that the
+ * hardware domain is the first in the list, get the first domain
+ * in the domain list and then if it is not the hardware domain or
+ * does not have console privilege, iterate the list until we find
+ * the hardware domain or a domain with console privilege.
+ */
+if ( next == NULL )
+{
+for_each_domain(next)
+{
+if ( hardware_domain && next == hardware_domain )
+{
+console_rx = 0;
+printk("*** Serial input to Xen");
+goto out;
+}
+
+if ( xsm_console_io(XSM_OTHER, next, CONSOLEIO_re
Re: [PATCH v7 0/9] Allow dynamic allocation of software IO TLB bounce buffers
Thanks, I've applied this to a new swiotlb-dynamic branch that I'll pull into the dma-mapping for-next tree.
Re: [PATCH v6 2/2] xen/riscv: introduce identity mapping
On Tue, 2023-08-01 at 16:50 +0200, Jan Beulich wrote: > On 01.08.2023 16:30, Oleksii Kurochko wrote: > > @@ -54,3 +70,17 @@ ENTRY(reset_stack) > > > > ret > > > > + .section .text.ident, "ax", %progbits > > + > > +ENTRY(turn_on_mmu) > > + sfence.vma > > + > > + li t0, RV_STAGE1_MODE > > + slli t0, t0, SATP_MODE_SHIFT > > + > > + la t1, stage1_pgtbl_root > > + srl t1, t1, PAGE_SHIFT > > I think it would be good to be consistent in the use of pseudo insns: > Above you use slli, so here it would want to be srli (or the other > way around). Oops, I overlooked that. When I examined the disassembler, it automatically transformed it to 'srli', so I forgot to change it. It would be more appropriate to use 'srli'. I'll wait for any additional comments, and if there are none, I'll send a new patch series version. Thanks. ~ Oleksii > > > + or t1, t1, t0 > > + csrw CSR_SATP, t1 > > + > > + jr a0 >
[PATCH -next] xen/evtchn: Remove unused function declaration xen_set_affinity_evtchn()
Commit 67473b8194bc ("xen/events: Remove disfunct affinity spreading")
leave this unused declaration.
Signed-off-by: Yue Haibing
---
include/xen/events.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/include/xen/events.h b/include/xen/events.h
index 95970a2f7695..95d5e28de324 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -75,7 +75,6 @@ void evtchn_put(evtchn_port_t evtchn);
void xen_send_IPI_one(unsigned int cpu, enum ipi_vector vector);
void rebind_evtchn_irq(evtchn_port_t evtchn, int irq);
-int xen_set_affinity_evtchn(struct irq_desc *desc, unsigned int tcpu);
static inline void notify_remote_via_evtchn(evtchn_port_t port)
{
--
2.34.1
Re: [PATCH] arm32: Avoid using solaris syntax for .section directive
On Tue, Aug 1, 2023 at 12:39 AM Michal Orzel wrote: > > Hi, > > On 01/08/2023 02:28, Khem Raj wrote: > > > > > > Assembler from binutils 2.41 rejects this syntax > > > > .section "name"[, flags...] > > > > where flags could be #alloc, #write, #execstr > s/execstr/execinstr + there is also #exclude and #tls if you want to list > them all > > > Switch to using ELF syntax > > > > .section name[, "flags"[, @type]] > > > > [1] > > https://ftp.gnu.org/old-gnu/Manuals/gas-2.9.1/html_chapter/as_7.html#SEC119 > I think it would be better to add a link to 2.41 docs instead or to refer to > the following commit > of binutils: > 4cb88cfae843 "PR11601, Solaris assembler compatibility doesn't work" > > > > > > Signed-off-by: Khem Raj > > --- > > xen/arch/arm/arm32/proc-v7.S | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/xen/arch/arm/arm32/proc-v7.S b/xen/arch/arm/arm32/proc-v7.S > > index c90a31d80f..6d3d19b873 100644 > > --- a/xen/arch/arm/arm32/proc-v7.S > > +++ b/xen/arch/arm/arm32/proc-v7.S > > @@ -29,7 +29,7 @@ brahma15mp_init: > > mcr CP32(r0, ACTLR) > > mov pc, lr > > > > -.section ".proc.info", #alloc > > +.section .proc.info, "a" > > .type __v7_ca15mp_proc_info, #object > > __v7_ca15mp_proc_info: > > .long 0x410FC0F0 /* Cortex-A15 */ > > @@ -38,7 +38,7 @@ __v7_ca15mp_proc_info: > > .long caxx_processor > > .size __v7_ca15mp_proc_info, . - __v7_ca15mp_proc_info > > > > -.section ".proc.info", #alloc > > +.section .proc.info, "a" > > .type __v7_ca7mp_proc_info, #object > > __v7_ca7mp_proc_info: > > .long 0x410FC070 /* Cortex-A7 */ > > @@ -47,7 +47,7 @@ __v7_ca7mp_proc_info: > > .long caxx_processor > > .size __v7_ca7mp_proc_info, . - __v7_ca7mp_proc_info > > > > -.section ".proc.info", #alloc > > +.section .proc.info, "a" > > .type __v7_brahma15mp_proc_info, #object > > __v7_brahma15mp_proc_info: > > .long 0x420F00F0 /* Broadcom Brahma-B15 */ > > -- > > 2.41.0 > > > > > > The patch looks good but a fast grep shows that ".section .dtb,#alloc" in > arch/arm/dtb.S would also want > to be changed (I do not have gas 2.41, so you can check it by specifying dtb > to be included in Xen image through > "menuconfig->Common Features->Absolute path to device tree blob") thanks will add it to v2 as well > > ~Michal
Re: [PATCH] arm32: Avoid using solaris syntax for .section directive
On Tue, Aug 1, 2023 at 12:33 AM Jan Beulich wrote: > > On 01.08.2023 02:12, Khem Raj wrote: > > Assembler from binutils 2.41 rejects this syntax > > > > .section "name"[, flags...] > > > > where flags could be #alloc, #write, #execstr > > Switch to using ELF syntax > > You mean GNU, not ELF (ELF is describing the object format, not the > syntax used). Feels almost like a regression (I'll mention that to > Alan), which likely went unnoticed so far because Linux had changed > to GNU syntax already in 5.5, to allow building with Clang's > integrated assembler. This aspect may be worth mentioning here as > well. OK will send v2 > > > .section name[, "flags"[, @type]] > > > > [1] > > https://ftp.gnu.org/old-gnu/Manuals/gas-2.9.1/html_chapter/as_7.html#SEC119 > > > > Signed-off-by: Khem Raj > > Out of curiosity - why were there four instances of the patch? > They all look largely identical; if there are differences, I haven't > spotted them. there should not have been I was a not a subscriber to mailing list, did not realize why it was not appearing on ml :) > > Jan
Re: [PATCH v6 2/2] xen/riscv: introduce identity mapping
On 01.08.2023 16:30, Oleksii Kurochko wrote: > @@ -54,3 +70,17 @@ ENTRY(reset_stack) > > ret > > +.section .text.ident, "ax", %progbits > + > +ENTRY(turn_on_mmu) > +sfence.vma > + > +li t0, RV_STAGE1_MODE > +sllit0, t0, SATP_MODE_SHIFT > + > +la t1, stage1_pgtbl_root > +srl t1, t1, PAGE_SHIFT I think it would be good to be consistent in the use of pseudo insns: Above you use slli, so here it would want to be srli (or the other way around). Jan > +or t1, t1, t0 > +csrwCSR_SATP, t1 > + > +jr a0
Xen Security Advisory 436 v1 (CVE-2023-34320) - arm: Guests can trigger a deadlock on Cortex-A77
-BEGIN PGP SIGNED MESSAGE- Hash: SHA256 Xen Security Advisory CVE-2023-34320 / XSA-436 arm: Guests can trigger a deadlock on Cortex-A77 ISSUE DESCRIPTION = Cortex-A77 cores (r0p0 and r1p0) are affected by erratum 1508412 where software, under certain circumstances, could deadlock a core due to the execution of either a load to device or non-cacheable memory, and either a store exclusive or register read of the Physical Address Register (PAR_EL1) in close proximity. IMPACT == A (malicious) guest that doesn't include the workaround for erratum 1508412 could deadlock the core. This will ultimately result to a deadlock of the system. VULNERABLE SYSTEMS == Systems running all version of Xen are affected. This bug is specific to Arm Cortex-A77 cores r0p0 and r1p0. MITIGATION == There are no known mitigations. NOTE REGARDING LACK OF EMBARGO == This issue has been publicly documented. RESOLUTION == To handle properly the erratum, it is necessary to have an updated firmware and that both the hypervisor and guest OSes have the workaround. This means it is not possible to security support Xen on the Cortex-A77, even on systems which have the workaround enabled. Applying the attached patches will document the situation and also add the workaround in Xen if someone wish to run on Cortex-A77 with only trusted guests. Note that patches for released versions are generally prepared to apply to the stable branches, and may not apply cleanly to the most recent release tarball. Downstreams are encouraged to update to the tip of the stable branch before applying these patches. xsa436/xsa436.patch xen-unstable - Xen 4.17.x xsa436/xsa436-4.16.patch Xen 4.16.x xsa436/xsa436-4.15.patch Xen 4.15.x $ sha256sum xsa436* xsa436*/* 64d34753cdbbcfec2c80db2daad98529bf900935419d0214057e962098b38160 xsa436.meta cc0f1303d4ad4c4750bd555622b87a9721e0253759b07915e6ba5216c24e8f8d xsa436/xsa436.patch 97d1bd7716637efce1fa5d7f608d7f26b2b396fa20b966c8c0cd22ef61dc07d4 xsa436/xsa436-4.15.patch e1264a44df39d56a2c6246d8f9f511d0371a5f416c364ef766ea5a59e7b46f92 xsa436/xsa436-4.16.patch $ -BEGIN PGP SIGNATURE- iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmTJGVoMHHBncEB4ZW4u b3JnAAoJEIP+FMlX6CvZIpMIAJJ/58V/2+aEQfc0Fd+UDegr+69PsgRVRKofbX5o M8r0hCLoowsEvI8vxloaOCTtgEwzFq2zCYsUED1nn0iLk0MqK6t9njkuVD3cmuqt WaVXiW7uJU8ph2pwscv2tVPBBYblT7+Y3fuHsbXEjEW40yQkStkD5NMgwH5Z0bhq 61zCZm+/xK66VBKnrWFdlTaueOLT11/lGPskISquWrYjz7Vr873k89fXdGURn6+9 N7gdl3eIDqkpGTXvUPFdPwwE+z1ESxGig24RYNQmt3UpLbIQO2wGp0HXbsJ8e1cj r4KNhSFm/h6tsjOYxm5Jmi4an4gAOlVxCSNds2/+oZQVHpQ= =GNOw -END PGP SIGNATURE- xsa436.meta Description: Binary data xsa436/xsa436.patch Description: Binary data xsa436/xsa436-4.15.patch Description: Binary data xsa436/xsa436-4.16.patch Description: Binary data
[PATCH v6 1/2] xen/riscv: introduce function for physical offset calculation
The function was introduced to calculate and save physical
offset before MMU is enabled because access to start() is
PC-relative and in case of linker_addr != load_addr it will
result in incorrect value in phys_offset.
Signed-off-by: Oleksii Kurochko
---
Changes in V6:
- After 9380f06fe8("xen: Drop the (almost) unused extern start[]")
an update of calc_phys_offset() was needed:
- change start to _start.
- add local volatile variable load_start to get properly Xens' load address.
Otherwise compile thinks that (start - XEN_VIRT_START) is equal to 0.
---
Changes in V5:
- update prototype of calc_phys_offset(). now it returns phys_offset.
- declare phys_offset as static.
- save returned value of calc_phys_offset to register s2.
---
Changes in V4:
- update the comment messages in head.S related to save/restore of a0/a1 regs.
---
Changes in V3:
- save/restore of a0/a1 registers before C first function call.
---
Changes in V2:
- add __ro_after_init for phys_offset variable.
- remove double blank lines.
- add __init for calc_phys_offset().
- update the commit message.
- declaring variable phys_off as non static as it will be used in head.S.
---
xen/arch/riscv/include/asm/mm.h | 2 ++
xen/arch/riscv/mm.c | 21 ++---
xen/arch/riscv/riscv64/head.S | 14 ++
3 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 5e3ac5cde3..7b94cbadd7 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -15,4 +15,6 @@ void setup_initial_pagetables(void);
void enable_mmu(void);
void cont_after_mmu_is_enabled(void);
+unsigned long calc_phys_offset(void);
+
#endif /* _ASM_RISCV_MM_H */
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index 99ed5e9623..a73f135a3c 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
+#include
#include
#include
#include
@@ -19,9 +20,10 @@ struct mmu_desc {
pte_t *pgtbl_base;
};
-#define PHYS_OFFSET ((unsigned long)_start - XEN_VIRT_START)
-#define LOAD_TO_LINK(addr) ((addr) - PHYS_OFFSET)
-#define LINK_TO_LOAD(addr) ((addr) + PHYS_OFFSET)
+static unsigned long __ro_after_init phys_offset;
+
+#define LOAD_TO_LINK(addr) ((unsigned long)(addr) - phys_offset)
+#define LINK_TO_LOAD(addr) ((unsigned long)(addr) + phys_offset)
/*
* It is expected that Xen won't be more then 2 MB.
@@ -273,3 +275,16 @@ void __init noreturn noinline enable_mmu()
switch_stack_and_jump((unsigned long)cpu0_boot_stack + STACK_SIZE,
cont_after_mmu_is_enabled);
}
+
+/*
+ * calc_phys_offset() should be used before MMU is enabled because access to
+ * start() is PC-relative and in case when load_addr != linker_addr phys_offset
+ * will have an incorrect value
+ */
+unsigned long __init calc_phys_offset(void)
+{
+volatile unsigned long load_start = (unsigned long)_start;
+
+phys_offset = load_start - XEN_VIRT_START;
+return phys_offset;
+}
diff --git a/xen/arch/riscv/riscv64/head.S b/xen/arch/riscv/riscv64/head.S
index 2c0304646a..ae194bb099 100644
--- a/xen/arch/riscv/riscv64/head.S
+++ b/xen/arch/riscv/riscv64/head.S
@@ -29,6 +29,20 @@ ENTRY(start)
jal reset_stack
+/*
+ * save hart_id ( bootcpu_id ) and dtb_base as a0 and a1 register can
+ * be used by C code
+ */
+mv s0, a0
+mv s1, a1
+
+jal calc_phys_offset
+mv s2, a0
+
+/* restore hart_id ( bootcpu_id ) and dtb address */
+mv a0, s0
+mv a1, s1
+
tailstart_xen
.section .text, "ax", %progbits
--
2.41.0
[PATCH v6 2/2] xen/riscv: introduce identity mapping
The way how switch to virtual address was implemented in the
commit e66003e7be ("xen/riscv: introduce setup_initial_pages")
isn't safe enough as:
* enable_mmu() depends on hooking all exceptions
and pagefault.
* Any exception other than pagefault, or not taking a pagefault
causes it to malfunction, which means you will fail to boot
depending on where Xen was loaded into memory.
Instead of the proposed way of switching to virtual addresses was
decided to use identity mapping for area which constains needed code
to switch from identity mapping and after switching to virtual addresses,
identity mapping is removed from page-tables in the following way:
search for top-most page table entry and remove it.
Fixes: e66003e7be ("xen/riscv: introduce setup_initial_pages")
Signed-off-by: Oleksii Kurochko
Suggested-by: Andrew Cooper
---
Changes in V6:
- t2 register was renamed to t1 as t1 isn't used anymore in
turn_on_mmu().
- fold li, sll instructions to slli.
- use a0 directly to calculate an argument of turn_on_mmu() instead of t0.
---
Changes in V5:
- update the algo of identity mapping removing.
- introduce IDENT_AREA_SIZE.
- introduce turn_on_mmu() function to enable and switch from 1:1 mapping.
- fix typo in PGTBL_INITIAL_COUNT define.
- update the comment above PGTBL_INITIAL_COUNT.
- update the commit message.
---
Changes in V4:
- remove definition of ARRAY_SIZE and ROUNDUP as was introduced
where these macros are located now.
- update definition of PGTBL_INITIAL_COUNT
- update the commit message
- update the algo of identity mapping removing
---
Changes in V3:
- remove unrelated to the patch changes ( SPDX tags in config.h ).
- update definition of PGTBL_INITIAL_COUNT taking into account identity
mapping.
- refactor remove_identity_mapping() function.
- add explanatory comments in xen.lds.S and mm.c.
- update commit message.
- move save/restore of a0/a1 registers to [PATCH v2 2/3] xen/riscv: introduce
function for physical offset calculation.
---
Changes in V2:
- update definition of PGTBL_INITIAL_COUNT and the comment above.
- code style fixes.
- 1:1 mapping for entire Xen.
- remove id_addrs array becase entire Xen is mapped.
- reverse condition for cycle inside remove_identity_mapping().
- fix page table walk in remove_identity_mapping().
- update the commit message.
- add Suggested-by: Andrew Cooper
- save hart_id and dtb_addr before call MMU related C functions.
- use phys_offset variable instead of doing calcultations to get phys offset
in head.S file. ( it can be easily done as entire Xen is 1:1 mapped )
- declare enable_muu() as __init.
---
xen/arch/riscv/include/asm/config.h | 2 +
xen/arch/riscv/include/asm/mm.h | 5 +-
xen/arch/riscv/mm.c | 90 +++--
xen/arch/riscv/riscv64/head.S | 30 ++
xen/arch/riscv/setup.c | 14 +
xen/arch/riscv/xen.lds.S| 11
6 files changed, 97 insertions(+), 55 deletions(-)
diff --git a/xen/arch/riscv/include/asm/config.h
b/xen/arch/riscv/include/asm/config.h
index fa90ae0898..f0544c6a20 100644
--- a/xen/arch/riscv/include/asm/config.h
+++ b/xen/arch/riscv/include/asm/config.h
@@ -95,6 +95,8 @@
#define RV_STAGE1_MODE SATP_MODE_SV32
#endif
+#define IDENT_AREA_SIZE 64
+
#endif /* __RISCV_CONFIG_H__ */
/*
* Local variables:
diff --git a/xen/arch/riscv/include/asm/mm.h b/xen/arch/riscv/include/asm/mm.h
index 7b94cbadd7..07c7a0abba 100644
--- a/xen/arch/riscv/include/asm/mm.h
+++ b/xen/arch/riscv/include/asm/mm.h
@@ -13,8 +13,11 @@ extern unsigned char cpu0_boot_stack[];
void setup_initial_pagetables(void);
void enable_mmu(void);
-void cont_after_mmu_is_enabled(void);
+
+void remove_identity_mapping(void);
unsigned long calc_phys_offset(void);
+void turn_on_mmu(unsigned long ra);
+
#endif /* _ASM_RISCV_MM_H */
diff --git a/xen/arch/riscv/mm.c b/xen/arch/riscv/mm.c
index a73f135a3c..053f043a3d 100644
--- a/xen/arch/riscv/mm.c
+++ b/xen/arch/riscv/mm.c
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#include
#include
@@ -35,8 +36,11 @@ static unsigned long __ro_after_init phys_offset;
*
* It might be needed one more page table in case when Xen load address
* isn't 2 MB aligned.
+ *
+ * CONFIG_PAGING_LEVELS page tables are needed for the identity mapping,
+ * except that the root page table is shared with the initial mapping
*/
-#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) + 1)
+#define PGTBL_INITIAL_COUNT ((CONFIG_PAGING_LEVELS - 1) * 2 + 1)
pte_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
stage1_pgtbl_root[PAGETABLE_ENTRIES];
@@ -75,6 +79,7 @@ static void __init setup_initial_mapping(struct mmu_desc
*mmu_desc,
unsigned int index;
pte_t *pgtbl;
unsigned long page_addr;
+bool is_identity_mapping = map_start == pa_start;
if ( (unsigned long)_start % XEN_PT_LEVEL_SIZE(0) )
{
@@ -108,16 +113,18 @@ static void __init setup_initial
[PATCH v6 0/2] xen/riscv: introduce identity mapping
The patch series introduces things necessary to implement identity mapping: 1. Make identity mapping for the entire Xen. 2. Enable MMU. 3. Jump to the virtual address world 4. Remove identity mapping. Also current patch series introduces the calculation of physical offset before MMU is enabled as access to physical offset will be calculated wrong after MMU will be enabled because access to phys_off variable is PC-relative and in the case when linker address != load address, it will cause MMU fault. The reason for this patch series can be found here: https://lore.kernel.org/xen-devel/4e336121-fc0c-b007-bf7b-430352563...@citrix.com/ --- Changes in V6: - Update calc_phys_offset() after rebase. - Refactor turn_on_mmu() and a way how an argument of turn_on_mmu() is calculated. --- Changes in V5: - update the algo of identity mapping removing. - introduce IDENT_AREA_SIZE. - introduce turn_on_mmu() function to enable and switch from 1:1 mapping. - fix typo in PGTBL_INITIAL_COUNT define. - update the comment above PGTBL_INITIAL_COUNT. - update prototype of calc_phys_offset(). now it returns phys_offset. - declare phys_offset as static. - save returned value of calc_phys_offset to register s2. --- Changes in V4: - drop patch [PATCH v3 1/3] xen/riscv: add SPDX tag to config.h as it was merged to staging - remove definition of ARRAY_SIZE and ROUNDUP as was introduced where these macros are located now. - update definition of PGTBL_INITIAL_COUNT - update the commit message for patch 'xen/riscv: introduce identity mapping' - update the comments in head.S - update the algo of identity mapping removing --- Changes in V3: - Update the patch series message. - The following patches were merged to staging so droped from the patch series: * xen/riscv: add .sbss section to .bss * xen/riscv: introduce reset_stack() function * xen/riscv: move extern of cpu0_boot_stack to header * xen/riscv: add SPDX tags - move save/restore of a0/a1 registers from patch 4 to patch 2 ( numbers are from the previous patch series version ) - add SPDX tag in config.h - update definition of PGTBL_INITIAL_COUNT taking into account identity mapping. - refactor remove_identity_mapping() function. - add explanatory comments in xen.lds.S and mm.c. --- Changes in V2: - update the patch series message. - drop patches from the previous version of the patch series: * xen/riscv: add __ASSEMBLY__ guards". ( merged ) * xen/riscv: make sure that identity mapping isn't bigger then page size ( entire Xen is 1:1 mapped so there is no need for the checks from the patch ) - add .sbss.* and put it befor .bss* . - move out reset_stack() to .text section. - add '__ro_after_init' for phys_offset variable. - add '__init' for calc_phys_offset(). - declaring variable phys_off as non static as it will be used in head.S. - update definition of PGTBL_INITIAL_COUNT and the comment above. - code style fixes. - remove id_addrs array becase entire Xen is mapped. - reverse condition for cycle inside remove_identity_mapping(). - fix page table walk in remove_identity_mapping(). - save hart_id and dtb_addr before call MMU related C functions - use phys_offset variable instead of doing calcultations to get phys offset in head.S file. ( it can be easily done as entire Xen is 1:1 mapped now ) - declare enable_muu() as __init. - Update SPDX tags. - Add Review-By/Suggested-By for some patches. - code style fixes. Oleksii Kurochko (2): xen/riscv: introduce function for physical offset calculation xen/riscv: introduce identity mapping xen/arch/riscv/include/asm/config.h | 2 + xen/arch/riscv/include/asm/mm.h | 7 +- xen/arch/riscv/mm.c | 109 +--- xen/arch/riscv/riscv64/head.S | 44 +++ xen/arch/riscv/setup.c | 14 +--- xen/arch/riscv/xen.lds.S| 11 +++ 6 files changed, 130 insertions(+), 57 deletions(-) -- 2.41.0
Re: [PATCH 00/24] ALSA: Generic PCM copy ops using sockptr_t
On Tue, Aug 01, 2023 at 02:54:45PM +0200, Takashi Iwai wrote: > That's true. The question is how more widely it'll be used, then. Indeed. > Is something like below what you had in mind, too? Yes, or Andy's suggestion of just copying without trying to put a redirect in works for me too. I imagine there might be some bikeshedding of the name, your proposal seems fine to me. signature.asc Description: PGP signature
Re: [PATCH 00/24] ALSA: Generic PCM copy ops using sockptr_t
On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote: > On Mon, 31 Jul 2023 19:20:54 +0200, > Mark Brown wrote: > > > > On Mon, Jul 31, 2023 at 05:46:54PM +0200, Takashi Iwai wrote: > > > > > this is a patch set to clean up the PCM copy ops using sockptr_t as a > > > "universal" pointer, inspired by the recent patch from Andy > > > Shevchenko: > > > > > > https://lore.kernel.org/r/20230721100146.67293-1-andriy.shevche...@linux.intel.com > > > > > Even though it sounds a bit weird, sockptr_t is a generic type that is > > > used already in wide ranges, and it can fit our purpose, too. With > > > sockptr_t, the former split of copy_user and copy_kernel PCM ops can > > > be unified again gracefully. > > > > It really feels like we ought to rename, or add an alias for, the type > > if we're going to start using it more widely - it's not helping to make > > the code clearer. > > That was my very first impression, too, but I changed my mind after > seeing the already used code. An alias might work, either typedef or > define genptr_t or such as sockptr_t. But we'll need to copy the > bunch of helper functions, too... Maybe we should define a genptr_t (in genptr.h) and convert sockptr infra to use it (in sockptr.h)? This will leave network and other existing users to convert to it step-by-step. Another approach is to simply copy sockptr.h to genptr.h with changed naming scheme and add a deprecation note to the former. Thank you, Takashi, for doing this! -- With Best Regards, Andy Shevchenko
Re: [XEN PATCH] x86/cpu-policy: justify a violation of MISRA C:2012 Rule 1.3
On 01/08/2023 15:40, Jan Beulich wrote: On 01.08.2023 15:06, Nicola Vetrini wrote: The empty feature set 'str_7c1' in 'tools/misc/xen-cpuid.c' causes the struct declaration to have no named members, hence violating Rule 1.3: "There shall be no occurrence of undefined or critical unspecified behaviour" because it is forbidden by ISO/IEC 9899:1999(E), Section 6.7.2.1.7: "If the struct-declaration-list contains no named members, the behavior is undefined." It has been assessed that the feature set declaration is intentionally empty, and that no risk of undesired behaviour stems from it, hence the struct declaration is marked safe. No functional changes. Signed-off-by: Nicola Vetrini --- As agreed during the MISRA C group meetings, this violation is dealt with by means of a comment deviation, as future changes may eliminate the root cause, which is the empty feature set. My justification for the claim and the commit message may need some adjusting. A reference to the compiler extension would be nice; the use of extensions (which generally are well-defined, even if not always well-documented) should eliminate the UB that the standard specifies. It sure is a good idea to specify this. Since the use of this compiler extension is already documented in 'docs/misra/C-language-toolchain.rst' I can just add a reference to that in the justification. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH] xen/spinlock: address violations of MISRA C:2012 Rules 8.2 and 8.3
On 01.08.2023 11:46, Federico Serafini wrote:
> --- a/xen/include/xen/spinlock.h
> +++ b/xen/include/xen/spinlock.h
> @@ -125,8 +125,9 @@ struct lock_profile_qhead {
> } while(0)
>
> void _lock_profile_register_struct(
> -int32_t, struct lock_profile_qhead *, int32_t);
> -void _lock_profile_deregister_struct(int32_t, struct lock_profile_qhead *);
> +int32_t type, struct lock_profile_qhead *qhead, int32_t idx);
> +void _lock_profile_deregister_struct(int32_t type,
> + struct lock_profile_qhead *qhead);
In adjacent declaration it would be nice if the same line wrapping
style was used. Can surely be taken care of while committing. With
the adjustment:
Acked-by: Jan Beulich
Jan
Re: [XEN PATCH] x86: mechanically rename to address MISRA C:2012 Rule 5.3
On 01.08.2023 10:58, Nicola Vetrini wrote: > Rule 5.3 has the following headline: > "An identifier declared in an inner scope shall not hide an > identifier declared in an outer scope" > > The renames done by this patch avoid shadowing from happening. > They are as follows: > - s/socket_info/sock_info/ in 'do_write_psr_msrs' > > The parameter 'cpu_khz' that causes a violation in 'pit_init' > is unused, and hence can be removed. > > Parameter 'debug_stack_lines' in 'compat_show_guest_stack' is removed, > since the shadowed static variable has the same purpose. > > The declaration of 'init_e820' is renamed to be consistent with its > definition. > > Signed-off-by: Nicola Vetrini Acked-by: Jan Beulich > This patch is the non-controversial part of patch 1/4 from series > https://lore.kernel.org/xen-devel/cover.1690788513.git.nicola.vetr...@bugseng.com/ > The other part has been dealt with with a patch from Jan Beulich > (not yet committed at the time of writing) > https://lore.kernel.org/xen-devel/e9035197-b329-af2e-65ed-af31cd037...@suse.com/ (Just) naming this v2 (or whichever version would have come next) would have been nice. Jan
Re: [XEN PATCH] xen/lib: address violations of MISRA C:2012 Rules 8.2 and 8.3
On 01.08.2023 09:05, Federico Serafini wrote:
> Give a name to unnamed parameters thus addressing violations of
> MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with
> named parameters").
> Keep consistency between parameter names and types used in function
> declarations and the ones used in the corresponding function
> definitions, thus addressing violations of MISRA C:2012 Rule 8.3
> ("All declarations of an object or function shall use the same names
> and type qualifiers").
>
> No functional changes.
>
> Signed-off-by: Federico Serafini
Acked-by: Jan Beulich
Re: xen | Failed pipeline for staging | c2026b88
On 01.08.2023 15:35, GitLab wrote: > > > Pipeline #951705012 has failed! > > Project: xen ( https://gitlab.com/xen-project/xen ) > Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging ) > > Commit: c2026b88 ( > https://gitlab.com/xen-project/xen/-/commit/c2026b88b58cbb6a84e2885c320a8cfe08f8ffc8 > ) > Commit Message: xen/arm/IRQ: uniform irq_set_affinity() with x8... > Commit Author: Federico Serafini > Committed by: Jan Beulich ( https://gitlab.com/jbeulich ) > > > Pipeline #951705012 ( > https://gitlab.com/xen-project/xen/-/pipelines/951705012 ) triggered by Ganis > ( https://gitlab.com/ganis ) > had 11 failed jobs. > > Job #4777864648 ( https://gitlab.com/xen-project/xen/-/jobs/4777864648/raw ) > > Stage: build > Name: opensuse-leap-gcc > Job #4777864742 ( https://gitlab.com/xen-project/xen/-/jobs/4777864742/raw ) > > Stage: test > Name: qemu-alpine-x86_64-gcc > Job #4777864646 ( https://gitlab.com/xen-project/xen/-/jobs/4777864646/raw ) > > Stage: build > Name: opensuse-leap-clang-debug > Job #4777864652 ( https://gitlab.com/xen-project/xen/-/jobs/4777864652/raw ) > > Stage: build > Name: opensuse-tumbleweed-clang > Job #4777864633 ( https://gitlab.com/xen-project/xen/-/jobs/4777864633/raw ) > > Stage: build > Name: ubuntu-bionic-gcc > Job #4777864636 ( https://gitlab.com/xen-project/xen/-/jobs/4777864636/raw ) > > Stage: build > Name: ubuntu-focal-gcc At the example of this (the first one I picked; I didn't look further): ../qemu-xen-dir-remote/hw/core/qdev-properties.c:974:1: fatal error: error writing to /tmp/ccxkeJPr.s: No space left on device Jan
Re: [XEN PATCH] x86/cpu-policy: justify a violation of MISRA C:2012 Rule 1.3
On 01.08.2023 15:06, Nicola Vetrini wrote: > The empty feature set 'str_7c1' in 'tools/misc/xen-cpuid.c' causes the > struct declaration to have no named members, hence violating > Rule 1.3: > "There shall be no occurrence of undefined or critical unspecified behaviour" > because it is forbidden by ISO/IEC 9899:1999(E), Section 6.7.2.1.7: > "If the struct-declaration-list contains no named > members, the behavior is undefined." > > It has been assessed that the feature set declaration is intentionally empty, > and that no risk of undesired behaviour stems from it, hence the struct > declaration is marked safe. > > No functional changes. > > Signed-off-by: Nicola Vetrini > --- > As agreed during the MISRA C group meetings, this violation is dealt > with by means of a comment deviation, as future changes may eliminate the > root cause, which is the empty feature set. > My justification for the claim and the commit message may need some adjusting. A reference to the compiler extension would be nice; the use of extensions (which generally are well-defined, even if not always well-documented) should eliminate the UB that the standard specifies. Jan
Re: [XEN PATCH v2 1/3] xen/common: address MISRA C:2012 Rule 5.3
On 01/08/2023 15:33, Jan Beulich wrote:
On 01.08.2023 14:47, Nicola Vetrini wrote:
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -81,6 +81,7 @@ int compat_memory_op(unsigned int cmd,
XEN_GUEST_HANDLE_PARAM(void) compat)
struct compat_memory_exchange xchg;
struct compat_add_to_physmap atp;
struct compat_add_to_physmap_batch atpb;
+struct compat_remove_from_physmap rmfp;
struct compat_vnuma_topology_info vnuma;
struct compat_mem_access_op mao;
struct compat_mem_acquire_resource mar;
@@ -321,12 +322,10 @@ int compat_memory_op(unsigned int cmd,
XEN_GUEST_HANDLE_PARAM(void) compat)
case XENMEM_remove_from_physmap:
{
-struct compat_remove_from_physmap cmp;
-
-if ( copy_from_guest(&cmp, compat, 1) )
+if ( copy_from_guest(&cmp.rmfp, compat, 1) )
return -EFAULT;
-XLAT_remove_from_physmap(nat.xrfp, &cmp);
+XLAT_remove_from_physmap(nat.xrfp, &cmp.rmfp);
Is there a reason not to use the same name in the compat union as is
used in the native one, like all other members do?
No, I just didn't notice there was a native remove_from_physmap field
when choosing the name.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
Re: [XEN PATCH v2 2/3] drivers/char: address MISRA C:2012 Rule 5.3
On 01.08.2023 14:47, Nicola Vetrini wrote: > The following strategies are adopted to deal with violations > of MISRA C:2012 Rule 5.3: > "An identifier declared in an inner scope shall not hide an > identifier declared in an outer scope". > > Local variable 'ctrl' shadows a variable defined in an outer scope. > Since the innermost variable is used only once after being set, it is safe > to remove it entirely. > > The enum constant 'baud' is shadowed by local a local variable at line > 1476, and renaming the enum constant avoid such conflicts. > > Signed-off-by: Nicola Vetrini Reviewed-by: Jan Beulich
Re: [XEN PATCH v2 1/3] xen/common: address MISRA C:2012 Rule 5.3
On 01.08.2023 14:47, Nicola Vetrini wrote:
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -81,6 +81,7 @@ int compat_memory_op(unsigned int cmd,
> XEN_GUEST_HANDLE_PARAM(void) compat)
> struct compat_memory_exchange xchg;
> struct compat_add_to_physmap atp;
> struct compat_add_to_physmap_batch atpb;
> +struct compat_remove_from_physmap rmfp;
> struct compat_vnuma_topology_info vnuma;
> struct compat_mem_access_op mao;
> struct compat_mem_acquire_resource mar;
> @@ -321,12 +322,10 @@ int compat_memory_op(unsigned int cmd,
> XEN_GUEST_HANDLE_PARAM(void) compat)
>
> case XENMEM_remove_from_physmap:
> {
> -struct compat_remove_from_physmap cmp;
> -
> -if ( copy_from_guest(&cmp, compat, 1) )
> +if ( copy_from_guest(&cmp.rmfp, compat, 1) )
> return -EFAULT;
>
> -XLAT_remove_from_physmap(nat.xrfp, &cmp);
> +XLAT_remove_from_physmap(nat.xrfp, &cmp.rmfp);
Is there a reason not to use the same name in the compat union as is
used in the native one, like all other members do?
Preferably with consistent naming (which I'd be okay switching to
while committing)
Acked-by: Jan Beulich
Jan
[linux-linus test] 182087: regressions - FAIL
flight 182087 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/182087/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-libvirt-raw 13 guest-start fail REGR. vs. 180278 test-arm64-arm64-xl-vhd 13 guest-start fail REGR. vs. 180278 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-credit1 20 guest-localmigrate/x10 fail in 182083 pass in 182087 test-amd64-amd64-xl-credit2 14 guest-startfail pass in 182083 test-amd64-amd64-xl-qemuu-ovmf-amd64 21 guest-start.2 fail pass in 182083 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-check fail blocked in 180278 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail blocked in 180278 test-armhf-armhf-libvirt-raw 15 saverestore-support-check fail blocked in 180278 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: linux5d0c230f1de8c7515b6567d9afba1f196fb4e2f4 baseline version: linux6c538e1adbfc696ac4747fb10d63e704344f763d Last test of basis 180278 2023-04-16 19:41:46 Z 106 days Failing since180281 2023-04-17 06:24:36 Z 106 days 196 attempts Testing same since 182083 2023-07-31 00:41:50 Z1 days2 attempts 3874 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf
Re: [PATCH 3/3] xen/ppc: Implement initial Radix MMU support
On 29.07.2023 00:21, Shawn Anastasio wrote: > --- /dev/null > +++ b/xen/arch/ppc/include/asm/bitops.h > @@ -0,0 +1,11 @@ > +#ifndef _ASM_PPC_BITOPS_H > +#define _ASM_PPC_BITOPS_H > + > +#include Not a big deal, but ... > +/* PPC bit number conversion */ > +#define PPC_BITLSHIFT(be)(BITS_PER_LONG - 1 - (be)) > +#define PPC_BIT(bit) (1UL << PPC_BITLSHIFT(bit)) > +#define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs)) > + > +#endif /* _ASM_PPC_BITOPS_H */ ... nothing here looks to require that #include. > --- /dev/null > +++ b/xen/arch/ppc/include/asm/mm.h > @@ -0,0 +1,19 @@ > +#ifndef _ASM_PPC_MM_H > +#define _ASM_PPC_MM_H > + > +#include This is included by xen/config.h, which in turn is included from the compiler command line. > --- /dev/null > +++ b/xen/arch/ppc/include/asm/page.h > @@ -0,0 +1,178 @@ > +#ifndef _ASM_PPC_PAGE_H > +#define _ASM_PPC_PAGE_H > + > +#include > + > +#include > +#include > + > +#define PDE_VALID PPC_BIT(0) > +#define PDE_NLB_MASK 0xfUL > +#define PDE_NLB_SHIFT 5UL > +#define PDE_NLS_MASK 0x1f > + > +#define PTE_VALID PPC_BIT(0) > +#define PTE_LEAF PPC_BIT(1) > +#define PTE_REFERENCE PPC_BIT(55) > +#define PTE_CHANGEPPC_BIT(56) > + > +/* PTE Attributes */ > +#define PTE_ATT_SAOPPC_BIT(59) /* Strong Access Ordering */ > +#define PTE_ATT_NON_IDEMPOTENT PPC_BIT(58) > +#define PTE_ATT_TOLERANT (PPC_BIT(58) | PPC_BIT(59)) > + > +/* PTE Encoded Access Authority*/ > +#define PTE_EAA_PRIVILEGED PPC_BIT(60) > +#define PTE_EAA_READ PPC_BIT(61) > +#define PTE_EAA_WRITE PPC_BIT(62) > +#define PTE_EAA_EXECUTEPPC_BIT(63) > + > +/* Field shifts/masks */ > +#define PTE_RPN_MASK 0x1fffUL > +#define PTE_RPN_SHIFT 12UL > +#define PTE_ATT_MASK 0x3UL > +#define PTE_ATT_SHIFT 4UL > +#define PTE_EAA_MASK 0xfUL Did you consider introducing just *_MASK values, utilizing MASK_INSR() and MASK_EXTR() instead of open-coded shifting/masking? > +#define PTE_XEN_BASE (PTE_VALID | PTE_EAA_PRIVILEGED | PTE_REFERENCE) > +#define PTE_XEN_RW (PTE_XEN_BASE | PTE_EAA_READ | PTE_EAA_WRITE | > PTE_CHANGE) > +#define PTE_XEN_RO (PTE_XEN_BASE | PTE_EAA_READ) > +#define PTE_XEN_RX (PTE_XEN_BASE | PTE_EAA_READ | PTE_EAA_EXECUTE) > + > +/* > + * Radix Tree layout for 64KB pages: > + * > + * [ L1 (ROOT) PAGE DIRECTORY (8192 * sizeof(pde_t)) ] > + * | > + * | > + * v > + *[ L2 PAGE DIRECTORY (512 * sizeof(pde_t)) ] > + * | > + * | > + * v > + *[ L3 PAGE DIRECTORY (512 * sizeof(pde_t)) ] > + * | > + * | > + * v > + * [ L4 PAGE TABLE (32 * sizeof(pte_t)) ] > + * | > + * | > + * v > + *[ PAGE TABLE ENTRY ] > + */ > + > +#define XEN_PT_ENTRIES_LOG2_LVL_1 13 /* 2**13 entries, maps 2**13 * 512GB = > 4PB */ > +#define XEN_PT_ENTRIES_LOG2_LVL_2 9 /* 2**9 entries, maps 2**9 * 1GB = > 512GB */ > +#define XEN_PT_ENTRIES_LOG2_LVL_3 9 /* 2**9 entries, maps 2**9 * 1GB = > 512GB */ > +#define XEN_PT_ENTRIES_LOG2_LVL_4 5 /* 2**5 entries, maps 2**5 * 64K = > 2MB */ > + > +#define XEN_PT_SHIFT_LVL_1(XEN_PT_SHIFT_LVL_2 + > XEN_PT_ENTRIES_LOG2_LVL_2) > +#define XEN_PT_SHIFT_LVL_2(XEN_PT_SHIFT_LVL_3 + > XEN_PT_ENTRIES_LOG2_LVL_3) > +#define XEN_PT_SHIFT_LVL_3(XEN_PT_SHIFT_LVL_4 + > XEN_PT_ENTRIES_LOG2_LVL_4) > +#define XEN_PT_SHIFT_LVL_4PAGE_SHIFT > + > +#define XEN_PT_ENTRIES_LOG2_LVL(lvl) (XEN_PT_ENTRIES_LOG2_LVL_##lvl) > +#define XEN_PT_SHIFT_LVL(lvl)(XEN_PT_SHIFT_LVL_##lvl) > +#define XEN_PT_ENTRIES_LVL(lvl) (1UL << XEN_PT_ENTRIES_LOG2_LVL(lvl)) > +#define XEN_PT_MASK_LVL(lvl) (XEN_PT_ENTRIES_LVL(lvl) - 1) > +#define XEN_PT_INDEX_LVL(lvl, va)((va >> XEN_PT_SHIFT_LVL(lvl)) & > XEN_PT_MASK_LVL(lvl)) > + > +/* > + * Calculate the index of the provided virtual address in the provided > + * page table struct > + */ > +#define pt_index(pt, va) _Generic((pt), \ Earlier on I did ask about the minimum compiler version you require for building the PPC hypervisor. Iirc you said about any, but here you're using another feature where I'm not sure how far back it is available. As indicated back then, it would be nice if ./README could gain respective information. > +struct lvl1_pd * : XEN_PT_INDEX_LVL(1, (va)), \ > +struct lvl2_pd * : XEN_PT_INDEX_LVL(2, (va)), \ > +struct lvl3_pd * : XEN_PT_INDEX_LVL(3, (va)), \ > +struct lvl4_pt * : XEN_PT_INDEX_LVL(4, (va))) > + > +#define pt_entry(pt, va) (&((pt)->entries[pt_index((pt), (va))])) As to style: Here (and elsewhere) you put unnecessary parentheses around two of the three uses of the macro arguments. If you think you want to keep it like that, feel free; in general we're recommending to omit ones not really needed, to help readability. Otoh a little h
[XEN PATCH] x86/cpu-policy: justify a violation of MISRA C:2012 Rule 1.3
The empty feature set 'str_7c1' in 'tools/misc/xen-cpuid.c' causes the
struct declaration to have no named members, hence violating
Rule 1.3:
"There shall be no occurrence of undefined or critical unspecified behaviour"
because it is forbidden by ISO/IEC 9899:1999(E), Section 6.7.2.1.7:
"If the struct-declaration-list contains no named
members, the behavior is undefined."
It has been assessed that the feature set declaration is intentionally empty,
and that no risk of undesired behaviour stems from it, hence the struct
declaration is marked safe.
No functional changes.
Signed-off-by: Nicola Vetrini
---
As agreed during the MISRA C group meetings, this violation is dealt
with by means of a comment deviation, as future changes may eliminate the
root cause, which is the empty feature set.
My justification for the claim and the commit message may need some adjusting.
---
docs/misra/safe.json | 9 +
xen/include/xen/lib/x86/cpu-policy.h | 1 +
2 files changed, 10 insertions(+)
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index e3c8a1d8eb..9239460e63 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -12,6 +12,15 @@
},
{
"id": "SAF-1-safe",
+"analyser": {
+"eclair": "MC3R1.R1.3",
+"text": "The following declaration of a struct with no named
members results is deliberate and it has been assessed that no unintended
behaviour arises from it."
+},
+"name": "Sentinel",
+"text": "Next ID to be used"
+},
+{
+"id": "SAF-2-safe",
"analyser": {},
"name": "Sentinel",
"text": "Next ID to be used"
diff --git a/xen/include/xen/lib/x86/cpu-policy.h
b/xen/include/xen/lib/x86/cpu-policy.h
index bab3eecda6..6b52f080c9 100644
--- a/xen/include/xen/lib/x86/cpu-policy.h
+++ b/xen/include/xen/lib/x86/cpu-policy.h
@@ -203,6 +203,7 @@ struct cpu_policy
};
union {
uint32_t _7c1;
+/* SAF-1-safe */
struct { DECL_BITFIELD(7c1); };
};
union {
--
2.34.1
Re: [PATCH RESEND v9 00/36] x86: enable FRED for x86-64
On Tue, Aug 01, 2023 at 12:52:36PM +0200, Peter Zijlstra wrote: > I also believe there is a kernel.org service for sending patch series, > but i'm not sure I remember the details. https://b4.docs.kernel.org/en/latest/contributor/send.html
Re: [PATCH 00/24] ALSA: Generic PCM copy ops using sockptr_t
On Mon, 31 Jul 2023 21:40:20 +0200,
Mark Brown wrote:
>
> On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
>
> > > It really feels like we ought to rename, or add an alias for, the type
> > > if we're going to start using it more widely - it's not helping to make
> > > the code clearer.
>
> > That was my very first impression, too, but I changed my mind after
> > seeing the already used code. An alias might work, either typedef or
> > define genptr_t or such as sockptr_t. But we'll need to copy the
> > bunch of helper functions, too...
>
> I would predict that if the type becomes more widely used that'll happen
> eventually and the longer it's left the more work it'll be.
That's true. The question is how more widely it'll be used, then.
Is something like below what you had in mind, too?
Takashi
-- 8< --
From: Takashi Iwai
Subject: [PATCH] Introduce uniptr_t as replacement of sockptr_t
Although sockptr_t is used already in several places as a "universal"
pointer, it's still too confusing as if were related with network
stuff.
Make a more generic type, uniptr_t, that does exactly as same as
sockptr_t for a wider use. sockptr_t becomes now alias to uniptr_t.
Signed-off-by: Takashi Iwai
---
include/linux/sockptr.h | 124 +---
include/linux/uniptr.h | 117 +
2 files changed, 132 insertions(+), 109 deletions(-)
create mode 100644 include/linux/uniptr.h
diff --git a/include/linux/sockptr.h b/include/linux/sockptr.h
index bae5e2369b4f..dc803989a4d6 100644
--- a/include/linux/sockptr.h
+++ b/include/linux/sockptr.h
@@ -1,118 +1,24 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Copyright (c) 2020 Christoph Hellwig.
- *
- * Support for "universal" pointers that can point to either kernel or
userspace
- * memory.
+ * Aliases for the old sockptr_t and its helpers for the new uniptr_t
*/
#ifndef _LINUX_SOCKPTR_H
#define _LINUX_SOCKPTR_H
-#include
-#include
+#include
-typedef struct {
- union {
- void*kernel;
- void __user *user;
- };
- boolis_kernel : 1;
-} sockptr_t;
-
-static inline bool sockptr_is_kernel(sockptr_t sockptr)
-{
- return sockptr.is_kernel;
-}
-
-static inline sockptr_t KERNEL_SOCKPTR(void *p)
-{
- return (sockptr_t) { .kernel = p, .is_kernel = true };
-}
-
-static inline sockptr_t USER_SOCKPTR(void __user *p)
-{
- return (sockptr_t) { .user = p };
-}
-
-static inline bool sockptr_is_null(sockptr_t sockptr)
-{
- if (sockptr_is_kernel(sockptr))
- return !sockptr.kernel;
- return !sockptr.user;
-}
-
-static inline int copy_from_sockptr_offset(void *dst, sockptr_t src,
- size_t offset, size_t size)
-{
- if (!sockptr_is_kernel(src))
- return copy_from_user(dst, src.user + offset, size);
- memcpy(dst, src.kernel + offset, size);
- return 0;
-}
-
-static inline int copy_from_sockptr(void *dst, sockptr_t src, size_t size)
-{
- return copy_from_sockptr_offset(dst, src, 0, size);
-}
-
-static inline int copy_to_sockptr_offset(sockptr_t dst, size_t offset,
- const void *src, size_t size)
-{
- if (!sockptr_is_kernel(dst))
- return copy_to_user(dst.user + offset, src, size);
- memcpy(dst.kernel + offset, src, size);
- return 0;
-}
-
-static inline int copy_to_sockptr(sockptr_t dst, const void *src, size_t size)
-{
- return copy_to_sockptr_offset(dst, 0, src, size);
-}
-
-static inline void *memdup_sockptr(sockptr_t src, size_t len)
-{
- void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN);
-
- if (!p)
- return ERR_PTR(-ENOMEM);
- if (copy_from_sockptr(p, src, len)) {
- kfree(p);
- return ERR_PTR(-EFAULT);
- }
- return p;
-}
-
-static inline void *memdup_sockptr_nul(sockptr_t src, size_t len)
-{
- char *p = kmalloc_track_caller(len + 1, GFP_KERNEL);
-
- if (!p)
- return ERR_PTR(-ENOMEM);
- if (copy_from_sockptr(p, src, len)) {
- kfree(p);
- return ERR_PTR(-EFAULT);
- }
- p[len] = '\0';
- return p;
-}
-
-static inline long strncpy_from_sockptr(char *dst, sockptr_t src, size_t count)
-{
- if (sockptr_is_kernel(src)) {
- size_t len = min(strnlen(src.kernel, count - 1) + 1, count);
-
- memcpy(dst, src.kernel, len);
- return len;
- }
- return strncpy_from_user(dst, src.user, count);
-}
-
-static inline int check_zeroed_sockptr(sockptr_t src, size_t offset,
- size_t size)
-{
- if (!sockptr_is_kernel(src))
- return check_zeroed_user(src.user + offset, size);
- return memchr_inv(src.kernel + offset, 0, size) == NULL;
-}
+#define sockptr_t uniptr_t
+#define sockptr_is_kern
[XEN PATCH v2 3/3] arm/efi: address MISRA C:2012 Rule 5.3
Rule 5.3 has the following headline:
"An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope"
The file-scope variable 'fdt' is shadowed by function parameters,
and thus violates the rule, hence it's renamed to 'fdt_efi'
No functional changes.
Signed-off-by: Nicola Vetrini
---
Changes in v2:
- Renamed the file-scope variable instead of removing function parameters.
---
xen/arch/arm/efi/efi-boot.h | 84 ++---
1 file changed, 42 insertions(+), 42 deletions(-)
diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index 6126a71400..f24df2abb9 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -49,7 +49,7 @@ static void PrintMessage(const CHAR16 *s);
{0xb1b621d5U, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0}}
static struct file __initdata dtbfile;
-static void __initdata *fdt;
+static void __initdata *fdt_efi;
static void __initdata *memmap;
static int __init setup_chosen_node(void *fdt, int *addr_cells, int
*size_cells)
@@ -383,7 +383,7 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
if ( EFI_ERROR(status) )
blexit(L"EFI memory map processing failed");
-status = fdt_add_uefi_nodes(SystemTable, fdt, map, map_size, desc_size,
+status = fdt_add_uefi_nodes(SystemTable, fdt_efi, map, map_size, desc_size,
desc_ver);
if ( EFI_ERROR(status) )
PrintErrMesg(L"Updating FDT failed", status);
@@ -395,7 +395,7 @@ static void __init efi_arch_pre_exit_boot(void)
static void __init noreturn efi_arch_post_exit_boot(void)
{
-efi_xen_start(fdt, fdt_totalsize(fdt));
+efi_xen_start(fdt_efi, fdt_totalsize(fdt_efi));
}
static void __init efi_arch_cfg_file_early(const EFI_LOADED_IMAGE *image,
@@ -420,8 +420,8 @@ static void __init efi_arch_cfg_file_early(const
EFI_LOADED_IMAGE *image,
efi_bs->FreePool(name.w);
}
}
-fdt = fdt_increase_size(&dtbfile, cfg.size + EFI_PAGE_SIZE);
-if ( !fdt )
+fdt_efi = fdt_increase_size(&dtbfile, cfg.size + EFI_PAGE_SIZE);
+if ( !fdt_efi )
blexit(L"Unable to create new FDT");
}
@@ -465,7 +465,7 @@ static void __init efi_arch_handle_cmdline(CHAR16
*image_name,
int chosen;
/* locate chosen node, which is where we add Xen module info. */
-chosen = fdt_subnode_offset(fdt, 0, "chosen");
+chosen = fdt_subnode_offset(fdt_efi, 0, "chosen");
if ( chosen < 0 )
blexit(L"Unable to find chosen node");
@@ -498,7 +498,7 @@ static void __init efi_arch_handle_cmdline(CHAR16
*image_name,
else
{
/* Get xen,xen-bootargs in /chosen if it is specified */
-const char *dt_bootargs_prop = fdt_getprop(fdt, chosen,
+const char *dt_bootargs_prop = fdt_getprop(fdt_efi, chosen,
"xen,xen-bootargs", NULL);
if ( dt_bootargs_prop )
{
@@ -526,7 +526,7 @@ static void __init efi_arch_handle_cmdline(CHAR16
*image_name,
blexit(L"FDT string overflow");
}
-if ( fdt_setprop_string(fdt, chosen, "xen,xen-bootargs", buf) < 0 )
+if ( fdt_setprop_string(fdt_efi, chosen, "xen,xen-bootargs", buf) < 0 )
blexit(L"Unable to set xen,xen-bootargs property.");
efi_bs->FreePool(buf);
@@ -542,7 +542,7 @@ static void __init efi_arch_handle_module(const struct file
*file,
if ( file == &dtbfile )
return;
-chosen = setup_chosen_node(fdt, &addr_len, &size_len);
+chosen = setup_chosen_node(fdt_efi, &addr_len, &size_len);
if ( chosen < 0 )
blexit(L"Unable to setup chosen node");
@@ -551,13 +551,13 @@ static void __init efi_arch_handle_module(const struct
file *file,
static const char __initconst ramdisk_compat[] = "multiboot,ramdisk\0"
"multiboot,module";
-node = fdt_add_subnode(fdt, chosen, "ramdisk");
+node = fdt_add_subnode(fdt_efi, chosen, "ramdisk");
if ( node < 0 )
blexit(L"Unable to add ramdisk FDT node.");
-if ( fdt_setprop(fdt, node, "compatible", ramdisk_compat,
+if ( fdt_setprop(fdt_efi, node, "compatible", ramdisk_compat,
sizeof(ramdisk_compat)) < 0 )
blexit(L"Unable to set compatible property.");
-if ( fdt_set_reg(fdt, node, addr_len, size_len, ramdisk.addr,
+if ( fdt_set_reg(fdt_efi, node, addr_len, size_len, ramdisk.addr,
ramdisk.size) < 0 )
blexit(L"Unable to set reg property.");
}
@@ -566,13 +566,13 @@ static void __init efi_arch_handle_module(const struct
file *file,
static const char __initconst xsm_compat[] = "xen,xsm-policy\0"
"multiboot,module";
-node = fdt_add_subnode(fdt, chosen, "xsm");
+node = fdt_add
[XEN PATCH v2 0/3] xen: address MISRA C:2012 Rule 5.3
Rule 5.3 has the following headline: "An identifier declared in an inner scope shall not hide an identifier declared in an outer scope". The following two strategies are adopted to deal with some violations of this rule: - renaming of local variables, functions or parameters; - removal of unnecessary declarations. No functional changes. Changes in v2: - Patches 1/4 and 2/4 from the previous version of this patch (https://lore.kernel.org/xen-devel/cover.1690810346.git.nicola.vetr...@bugseng.com/) are already committed to staging, therefore are excluded from this series. Nicola Vetrini (3): xen/common: address MISRA C:2012 Rule 5.3 drivers/char: address MISRA C:2012 Rule 5.3 arm/efi: address MISRA C:2012 Rule 5.3 xen/arch/arm/efi/efi-boot.h | 84 ++-- xen/common/compat/memory.c | 7 ++- xen/common/numa.c| 20 - xen/drivers/char/ehci-dbgp.c | 4 +- xen/drivers/char/ns16550.c | 6 +-- 5 files changed, 59 insertions(+), 62 deletions(-) -- 2.34.1
[XEN PATCH v2 2/3] drivers/char: address MISRA C:2012 Rule 5.3
The following strategies are adopted to deal with violations
of MISRA C:2012 Rule 5.3:
"An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope".
Local variable 'ctrl' shadows a variable defined in an outer scope.
Since the innermost variable is used only once after being set, it is safe
to remove it entirely.
The enum constant 'baud' is shadowed by local a local variable at line
1476, and renaming the enum constant avoid such conflicts.
Signed-off-by: Nicola Vetrini
---
Changes in v2:
- Renamed the enum constant instead of the local variable 'baud'.
- Removed the inner variable entirely, as it serves no purpose.
---
xen/drivers/char/ehci-dbgp.c | 4 +---
xen/drivers/char/ns16550.c | 6 +++---
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index 4d8d765122..72be4d9cc9 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -424,9 +424,7 @@ static void dbgp_issue_command(struct ehci_dbgp *dbgp, u32
ctrl,
* checks to see if ACPI or some other initialization also
* reset the EHCI debug port.
*/
-u32 ctrl = readl(&dbgp->ehci_debug->control);
-
-if ( ctrl & DBGP_ENABLED )
+if ( readl(&dbgp->ehci_debug->control) & DBGP_ENABLED )
{
cmd |= CMD_RUN;
writel(cmd, &dbgp->ehci_regs->command);
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 212a9c49ae..b75e7f8fa0 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -1388,7 +1388,7 @@ string_param("com1", opt_com1);
string_param("com2", opt_com2);
enum serial_param_type {
-baud,
+baud_rate,
clock_hz,
data_bits,
io_base,
@@ -1416,7 +1416,7 @@ struct serial_param_var {
* com_console_options for serial port com1 and com2.
*/
static const struct serial_param_var __initconst sp_vars[] = {
-{"baud", baud},
+{"baud", baud_rate},
{"clock-hz", clock_hz},
{"data-bits", data_bits},
{"io-base", io_base},
@@ -1596,7 +1596,7 @@ static bool __init parse_namevalue_pairs(char *str,
struct ns16550 *uart)
switch ( get_token(token, ¶m_value) )
{
-case baud:
+case baud_rate:
uart->baud = simple_strtoul(param_value, NULL, 0);
break;
--
2.34.1
[XEN PATCH v2 1/3] xen/common: address MISRA C:2012 Rule 5.3
The following strategies are adopted to deal with violations
of MISRA C:2012 Rule 5.3:
"An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope".
- s/nodes/numa_nodes/ for the file-scope variable in 'common/numa.c';
- move the variable 'struct compat_remove_from_physmap cmp' inside
the outer union variable 'cmp' to avoid shadowing it.
Signed-off-by: Nicola Vetrini
---
Changes in v2:
- Split the patch in common/ and drivers/.
- Moved the local cmp into the union as a member.
- Renamed the file-scope variable instead of parameters/local variables.
---
xen/common/compat/memory.c | 7 +++
xen/common/numa.c | 20 ++--
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c
index 8ca63ceda6..d000a5d93d 100644
--- a/xen/common/compat/memory.c
+++ b/xen/common/compat/memory.c
@@ -81,6 +81,7 @@ int compat_memory_op(unsigned int cmd,
XEN_GUEST_HANDLE_PARAM(void) compat)
struct compat_memory_exchange xchg;
struct compat_add_to_physmap atp;
struct compat_add_to_physmap_batch atpb;
+struct compat_remove_from_physmap rmfp;
struct compat_vnuma_topology_info vnuma;
struct compat_mem_access_op mao;
struct compat_mem_acquire_resource mar;
@@ -321,12 +322,10 @@ int compat_memory_op(unsigned int cmd,
XEN_GUEST_HANDLE_PARAM(void) compat)
case XENMEM_remove_from_physmap:
{
-struct compat_remove_from_physmap cmp;
-
-if ( copy_from_guest(&cmp, compat, 1) )
+if ( copy_from_guest(&cmp.rmfp, compat, 1) )
return -EFAULT;
-XLAT_remove_from_physmap(nat.xrfp, &cmp);
+XLAT_remove_from_physmap(nat.xrfp, &cmp.rmfp);
break;
}
diff --git a/xen/common/numa.c b/xen/common/numa.c
index fc1f7f665b..f454c4d894 100644
--- a/xen/common/numa.c
+++ b/xen/common/numa.c
@@ -15,7 +15,7 @@
static nodemask_t __initdata processor_nodes_parsed;
static nodemask_t __initdata memory_nodes_parsed;
-static struct node __initdata nodes[MAX_NUMNODES];
+static struct node __initdata numa_nodes[MAX_NUMNODES];
static unsigned int __ro_after_init num_node_memblks;
static struct node __ro_after_init node_memblk_range[NR_NODE_MEMBLKS];
@@ -117,7 +117,7 @@ static enum conflicts __init conflicting_memblks(
static void __init cutoff_node(nodeid_t i, paddr_t start, paddr_t end)
{
-struct node *nd = &nodes[i];
+struct node *nd = &numa_nodes[i];
if ( nd->start < start )
{
@@ -157,7 +157,7 @@ bool __init numa_update_node_memblks(nodeid_t node,
unsigned int arch_nid,
paddr_t end = start + size;
paddr_t nd_start = start;
paddr_t nd_end = end;
-struct node *nd = &nodes[node];
+struct node *nd = &numa_nodes[node];
/*
* For the node that already has some memory blocks, we will
@@ -292,17 +292,17 @@ static bool __init nodes_cover_memory(void)
do {
found = false;
for_each_node_mask ( j, memory_nodes_parsed )
-if ( start < nodes[j].end && end > nodes[j].start )
+if ( start < numa_nodes[j].end && end > numa_nodes[j].start )
{
-if ( start >= nodes[j].start )
+if ( start >= numa_nodes[j].start )
{
-start = nodes[j].end;
+start = numa_nodes[j].end;
found = true;
}
-if ( end <= nodes[j].end )
+if ( end <= numa_nodes[j].end )
{
-end = nodes[j].start;
+end = numa_nodes[j].start;
found = true;
}
}
@@ -356,10 +356,10 @@ static bool __init numa_process_nodes(paddr_t start,
paddr_t end)
/* Finally register nodes */
for_each_node_mask ( i, all_nodes_parsed )
{
-if ( nodes[i].end == nodes[i].start )
+if ( numa_nodes[i].end == numa_nodes[i].start )
printk(KERN_INFO "NUMA: node %u has no memory\n", i);
-setup_node_bootmem(i, nodes[i].start, nodes[i].end);
+setup_node_bootmem(i, numa_nodes[i].start, numa_nodes[i].end);
}
for ( i = 0; i < nr_cpu_ids; i++ )
--
2.34.1
[xen-unstable-smoke test] 182100: tolerable all pass - PUSHED
flight 182100 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/182100/ 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 c2026b88b58cbb6a84e2885c320a8cfe08f8ffc8 baseline version: xen 70eb862b01023c45b943e8ff92ef4f6a7e9e8950 Last test of basis 182098 2023-07-31 20:00:37 Z0 days Testing same since 182100 2023-08-01 10:03:51 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Federico Serafini 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 70eb862b01..c2026b88b5 c2026b88b58cbb6a84e2885c320a8cfe08f8ffc8 -> smoke
Re: [PATCH 2/5] xen/ppc: Switch to medium PIC code model
On 28.07.2023 23:35, Shawn Anastasio wrote: > @@ -11,16 +13,19 @@ ENTRY(start) > FIXUP_ENDIAN > > /* set up the TOC pointer */ > -LOAD_IMM32(%r2, .TOC.) > +bcl 20, 31, .+4 > +1: mflr%r12 > +addis %r2, %r12, .TOC.-1b@ha > +addi%r2, %r2, .TOC.-1b@l > > /* set up the initial stack */ > -LOAD_IMM32(%r1, cpu0_boot_stack) > +LOAD_REG_ADDR(%r1, cpu0_boot_stack) Question: Would perhaps make sense to use %sp and %rtoc in place of %r1 and %r2 (not just here of course)? Jan
Re: [PATCH v1 7/7] xenalyze: handle more potential exit reason values from vmx.h
Fri, 28 Jul 2023 21:35:54 +0100 George Dunlap : > Everything looks good (including adding the missing strings), except for > the removal of the fixed array size. Call me paranoid, but if we define it > as REASON_MAX length, then there will never be any way that a value less > than REASON_MAX causes a segfault (and if we assign a value higher than > REASON_MAX, the compiler will complain); whereas if we make it variable, we > leave open the possibility that someone won't update REASON_MAX properly, > resulting in either segfaults (if REASON_MAX is too high) or missing > strings (if REASON_MAX is too low). I think the code needs to be rearranged to work with "array_size" instead. Olaf pgpkqdwQ_lR5u.pgp Description: Digitale Signatur von OpenPGP
Re: [PATCH 5/5] xen/ppc: Implement early serial console on PowerNV
On 28.07.2023 23:35, Shawn Anastasio wrote:
> --- a/xen/arch/ppc/include/asm/asm-defns.h
> +++ b/xen/arch/ppc/include/asm/asm-defns.h
> @@ -23,6 +23,18 @@
> addis reg,%r2,name@toc@ha;
> \
> addi reg,reg,name@toc@l
Noticing only now, because of the issue ...
> +/*
> + * Declare a global assembly function with a proper TOC setup prologue
> + */
> +#define _GLOBAL_TOC(name) \
> +.balign 4; \
> +.type name,@function; \
> +.globl name;\
> +name: \
> +0: addis %r2,%r12,(.TOC.-0b)@ha; \
> +addi %r2,%r2,(.TOC.-0b)@l; \
> +.localentry name,.-name
... being widened - could we gain blanks after the commas? Down here
(to match the code in context) another padding blank after "addi"
would also be nice.
> --- a/xen/arch/ppc/opal.c
> +++ b/xen/arch/ppc/opal.c
> @@ -8,9 +8,28 @@
> #include
> #include
>
> -/* Global OPAL struct containing entrypoint and base */
> +/* Global OPAL struct containing entrypoint and base used by opal-calls.S */
> struct opal opal;
>
> +int64_t opal_console_write(int64_t term_number, uint64_t *length,
> + uint8_t *buffer);
Would this perhaps better be void *, eliminating the need for casting
in calls of this function?
> +int64_t opal_console_flush(int64_t term_number);
> +int64_t opal_reinit_cpus(uint64_t flags);
> +
> +static void opal_putchar(char c)
Can't this be __init?
> +{
> +uint64_t len;
> +if (c == '\n')
Nit: Blank line please between declaration(s) and statement(s). (At
least one more instance below.)
Also please add the missing blanks in the if(), seeing that otherwise
the file is aiming at being Xen style.
> +{
> +char buf = '\r';
> +len = cpu_to_be64(1);
> +opal_console_write(0, &len, (uint8_t *) &buf);
> +}
> +len = cpu_to_be64(1);
> +opal_console_write(0, &len, (uint8_t *) &c);
> +opal_console_flush(0);
> +}
> +
> void __init boot_opal_init(const void *fdt)
> {
> int opal_node;
> @@ -45,4 +64,10 @@ void __init boot_opal_init(const void *fdt)
>
> opal.base = be64_to_cpu(*opal_base);
> opal.entry = be64_to_cpu(*opal_entry);
> +
> +early_printk_init(opal_putchar);
> +
> +/* Ask OPAL to set HID0 for Little Endian interrupts + Radix TLB support
> */
> +opal_reinit_cpus(OPAL_REINIT_CPUS_HILE_LE | OPAL_REINIT_CPUS_MMU_RADIX
> + | OPAL_REINIT_CPUS_MMU_HASH);
Nit: operators on continued lines go at the end of the earlier line.
> --- /dev/null
> +++ b/xen/arch/ppc/ppc64/opal-calls.S
> @@ -0,0 +1,82 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Adapted from Linux's arch/powerpc/boot/opal-calls.S
> + *
> + * Copyright (c) 2016 IBM Corporation.
> + * Copyright Raptor Engineering, LLC
> + */
> +
> +#include
> +#include
Would it make sense to have asm-defns.h include asm-offsets.h, like
x86 and Arm do?
> +#include
> +#include
> +
> +.text
Is any of this code still needed post-init?
> +#define OPAL_CALL(name, token) \
> +.globl name;\
> +name: \
> +li %r0, token; \
> +b opal_call;
I think the trailing semicolon wants omitting.
Jan
Re: [PATCH RESEND v9 00/36] x86: enable FRED for x86-64
On Tue, Aug 01, 2023 at 01:32:42AM -0700, Xin Li wrote: > Resend because the mail system failed to deliver some messages yesterday. Well, you need to figure out how to send patches, because both yesterday and today are screwy. The one from yesterday came in 6 thread groups: 0-25, 26, 27, 28, 29, 30-36, while the one from today comes in 2 thread groups: 0-26, 27-36. Which I suppose one can count as an improvement :/ Seriously, it should not be hard to send 36 patches in a single thread. I see you're trying to send through the regular corporate email trainwreck; do you have a linux.intel.com account? Or really anything else besides intel.com? You can try sending the series to yourself to see if it arrives correctly as a whole before sending it out to the list again. I also believe there is a kernel.org service for sending patch series, but i'm not sure I remember the details.
[PATCH] common: move simple_strto{,u}l{,l}() to lib/
Convert style from a Xen/Linux mix to pure Xen while doing the move. No
other changes, despite having been heavily tempted to do some - at the
very least to make simple_strtoul() and simple_strtoull() the same in
how they deal with non-numeric digits.
Requested-by: Shawn Anastasio
Signed-off-by: Jan Beulich
---
Further changes I was considering:
- "value" doesn't need to be unsigned long, and even less so unsigned
long long,
- strtoull.c could simply include strtoul.c, after #define-ing "long"
to "long long" and "simple_strtoul" to "simple_strtoull",
- "else if ( base == 16 )" wants folding with its inner if().
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -24,108 +24,6 @@
#include
#include
-/**
- * simple_strtoul - convert a string to an unsigned long
- * @cp: The start of the string
- * @endp: A pointer to the end of the parsed string will be placed here
- * @base: The number base to use
- */
-unsigned long simple_strtoul(
-const char *cp, const char **endp, unsigned int base)
-{
-unsigned long result = 0,value;
-
-if (!base) {
-base = 10;
-if (*cp == '0') {
-base = 8;
-cp++;
-if ((toupper(*cp) == 'X') && isxdigit(cp[1])) {
-cp++;
-base = 16;
-}
-}
-} else if (base == 16) {
-if (cp[0] == '0' && toupper(cp[1]) == 'X')
-cp += 2;
-}
-while (isxdigit(*cp) &&
- (value = isdigit(*cp) ? *cp-'0' : toupper(*cp)-'A'+10) < base) {
-result = result*base + value;
-cp++;
-}
-if (endp)
-*endp = cp;
-return result;
-}
-
-EXPORT_SYMBOL(simple_strtoul);
-
-/**
- * simple_strtol - convert a string to a signed long
- * @cp: The start of the string
- * @endp: A pointer to the end of the parsed string will be placed here
- * @base: The number base to use
- */
-long simple_strtol(const char *cp, const char **endp, unsigned int base)
-{
-if(*cp=='-')
-return -simple_strtoul(cp+1,endp,base);
-return simple_strtoul(cp,endp,base);
-}
-
-EXPORT_SYMBOL(simple_strtol);
-
-/**
- * simple_strtoull - convert a string to an unsigned long long
- * @cp: The start of the string
- * @endp: A pointer to the end of the parsed string will be placed here
- * @base: The number base to use
- */
-unsigned long long simple_strtoull(
-const char *cp, const char **endp, unsigned int base)
-{
-unsigned long long result = 0,value;
-
-if (!base) {
-base = 10;
-if (*cp == '0') {
-base = 8;
-cp++;
-if ((toupper(*cp) == 'X') && isxdigit(cp[1])) {
-cp++;
-base = 16;
-}
-}
-} else if (base == 16) {
-if (cp[0] == '0' && toupper(cp[1]) == 'X')
-cp += 2;
-}
-while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp-'0' : (islower(*cp)
- ? toupper(*cp)
: *cp)-'A'+10) < base) {
-result = result*base + value;
-cp++;
-}
-if (endp)
-*endp = cp;
-return result;
-}
-
-EXPORT_SYMBOL(simple_strtoull);
-
-/**
- * simple_strtoll - convert a string to a signed long long
- * @cp: The start of the string
- * @endp: A pointer to the end of the parsed string will be placed here
- * @base: The number base to use
- */
-long long simple_strtoll(const char *cp,const char **endp,unsigned int base)
-{
-if(*cp=='-')
-return -simple_strtoull(cp+1,endp,base);
-return simple_strtoull(cp,endp,base);
-}
-
static int skip_atoi(const char **s)
{
int i=0;
--- a/xen/lib/Makefile
+++ b/xen/lib/Makefile
@@ -28,6 +28,10 @@ lib-y += strrchr.o
lib-y += strsep.o
lib-y += strspn.o
lib-y += strstr.o
+lib-y += strtol.o
+lib-y += strtoll.o
+lib-y += strtoul.o
+lib-y += strtoull.o
lib-$(CONFIG_X86) += xxhash32.o
lib-$(CONFIG_X86) += xxhash64.o
--- /dev/null
+++ b/xen/lib/strtol.c
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 1991, 1992 Linus Torvalds
+ */
+
+#include
+
+/**
+ * simple_strtol - convert a string to a signed long
+ * @cp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ */
+long simple_strtol(const char *cp, const char **endp, unsigned int base)
+{
+if ( *cp == '-' )
+return -simple_strtoul(cp + 1, endp, base);
+return simple_strtoul(cp, endp, base);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--- /dev/null
+++ b/xen/lib/strtoll.c
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 1991, 1992 Linus Torvalds
+ */
+
+#include
+
+/**
+ * simple_strtoll - convert a string to a signed long long
+ * @cp: The start of the string
+ * @endp: A pointer to the end of the parsed string will be placed here
+ * @base: The number base to use
+ */
+long long simple_strtoll(const char *cp, const char **endp, unsigned int base
Re: [PATCH 0/3] x86: Some MISRA Rule 5.3 fixes
On 28.07.2023 21:43, Andrew Cooper wrote: > 'debug' and 'str' account for an awefully large number of shadowed variable > violations. > > Andrew Cooper (3): > x86/traps: Move do_general_protection() earlier > x86/entry: Rename the exception entrypoints > x86: Delete str() Series Acked-by: Jan Beulich with a slightly amended description in patch 3, and preferably also with at least mentioning the Misra angle in patch 2. Jan
Re: [PATCH v4 1/1] ns16550: add support for polling mode when device tree is used
On 01.08.2023 11:46, Oleksii wrote: > On Mon, 2023-07-31 at 15:24 +0200, Jan Beulich wrote: >> On 27.07.2023 18:45, Oleksii Kurochko wrote: >>> @@ -654,6 +674,9 @@ static void ns16550_init_common(struct ns16550 >>> *uart) >>> >>> /* Default lsr_mask = UART_LSR_THRE */ >>> uart->lsr_mask = UART_LSR_THRE; >>> + >>> + if ( strstr(opt_com1, "poll") || strstr(opt_com2, "poll") ) >>> + uart->intr_works = polling; >>> } >> >> A non-__init function may not reference __initdata objects. But >> strstr() >> is too lax anyway, and you also shouldn't check the wrong port's >> options. >> You want to recognize "poll" _only_ where all other command line >> options >> are processed. > Just to confirm, do you mean that I should use parse_positional() ( or > something similar ) for the device-tree method of initializing ns16550? > > I checked the polling behavior as described above because > parse_positional() is utilized solely for X86. > > It appears that command line options are parsed only in the case of > x86. Hmm, looks like you're right. Arm folks will want to clarify how they got away without any command line overrides so far, and how they think now necessary ones should be suitably added. I recall I had reservations when the file was massaged to compile out supposedly x86-only code. Jan
[XEN PATCH 4/4] automation/eclair: avoid failure in case of missing merge point
In the context of an auto pull request, when a common merge point
is not found the integration will continue the analysis without
failing.
Signed-off-by: Simone Ballarin
---
automation/eclair_analysis/ECLAIR/action.settings | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/automation/eclair_analysis/ECLAIR/action.settings
b/automation/eclair_analysis/ECLAIR/action.settings
index 528bc24c72..f96368ffc7 100644
--- a/automation/eclair_analysis/ECLAIR/action.settings
+++ b/automation/eclair_analysis/ECLAIR/action.settings
@@ -138,7 +138,9 @@ auto_pull_request)
git remote add autoPRRemote "${autoPRRemoteUrl}"
git fetch -q autoPRRemote
subDir="${ref}"
-baseCommitId=$(git merge-base "autoPRRemote/${autoPRBranch}" HEAD)
+if ! baseCommitId=$(git merge-base "autoPRRemote/${autoPRBranch}" HEAD);
then
+baseCommitId=no_merge_point
+fi
jobHeadline="ECLAIR ${ANALYSIS_KIND} on repository ${repository}:
${pushUser} wants to merge ${repository}:${ref} (${headCommitId}) into
${autoPRRepository}/${autoPRBranch} (${baseCommitId})"
;;
*)
--
2.34.1
[XEN PATCH 2/4] automation/eclair: add direct link to reports
This patch adds direct links to the analysis findings in the
console log.
Signed-off-by: Simone Ballarin
---
.../eclair_analysis/ECLAIR/action.helpers | 84 ++-
1 file changed, 65 insertions(+), 19 deletions(-)
diff --git a/automation/eclair_analysis/ECLAIR/action.helpers
b/automation/eclair_analysis/ECLAIR/action.helpers
index 2ad6428eaa..df9bf2bd11 100644
--- a/automation/eclair_analysis/ECLAIR/action.helpers
+++ b/automation/eclair_analysis/ECLAIR/action.helpers
@@ -1,17 +1,26 @@
+esc=$(printf '\e')
+cr=$(printf '\r')
+
if [ -n "${GITLAB_CI:-}" ]; then
ci=gitlab
+eol=
+link_start="${esc}[33m"
+link_end="${esc}[m"
elif [ -n "${GITHUB_ACTION:-}" ]; then
ci=github
+eol="\\"
+link_start=
+link_end=
elif [ -n "${JENKINS_HOME:-}" ]; then
ci=jenkins
+eol=""
+link_start=
+link_end=
else
echo "Unexpected CI/CD context" >&2
exit 1
fi
-esc=$(printf '\e')
-cr=$(printf '\r')
-
open_section() {
id=$1
title=$2
@@ -36,54 +45,86 @@ summary() {
case "${ci}" in
github)
-nl="\\"
+eol="\\"
;;
gitlab)
-nl=
+eol=
;;
jenkins)
-nl=""
+eol=""
;;
*)
-nl=
+eol=
;;
esac
+
currentDbReportsUrl="${eclairReportUrlPrefix}/fs${jobDir}/PROJECT.ecd;/by_service.html#service&kind"
if [ -z "${newReports}" ]; then
-fixedMsg=
+fixedMsg="No fixed reports as there is no baseline"
unfixedMsg="Unfixed reports: ${unfixedReports}"
-countsMsg="${unfixedMsg}"
+referenceReportsMsgTxt=
+referenceReportsMsgLog=
else
fixedMsg="Fixed reports: ${fixedReports}"
unfixedMsg="Unfixed reports: ${unfixedReports} [new: ${newReports}]"
-countsMsg="${fixedMsg}${nl}
-${unfixedMsg}"
+case "${event}" in
+pull_request | auto_pull_request)
+
referenceDbReportsUrl="${eclairReportUrlPrefix}/fs${jobDir}/base/PROJECT.ecd;/by_service.html#service&kind"
+reference_kind=base
+;;
+push)
+
referenceDbReportsUrl="${eclairReportUrlPrefix}/fs${jobDir}/prev/PROJECT.ecd;/by_service.html#service&kind"
+reference_kind=previous
+;;
+*)
+echo "Unexpected event ${event}" >&2
+exit 1
+;;
+esac
fi
+
case "${ci}" in
jenkins)
+if [ -n "${newReports}" ]; then
+referenceReportsMsgTxt="Browse
${reference_kind} reports"
+fi
cat <"${summaryTxt}"
-${countsMsg}
${nl}
+${fixedMsg}${eol}
+${unfixedMsg}
${eol}
https://www.bugseng.com/eclair";>
${jobHeadline}
-Browse analysis results
+Browse analysis summary
+Browse current reports
+${referenceReportsMsgTxt}
EOF
;;
*)
+if [ -n "${newReports}" ]; then
+referenceReportsMsgTxt="Browse ${reference_kind} reports:
${referenceDbReportsUrl}"
+fi
cat <"${summaryTxt}"
https://www.bugseng.com/eclair";>
Analysis Summary
-${jobHeadline}${nl}
-${countsMsg}${nl}
-[Browse analysis](${indexHtmlUrl})
+${jobHeadline}${eol}
+${fixedMsg}${eol}
+${unfixedMsg}${eol}
+Browse analysis summary: ${indexHtmlUrl}
+Browse current reports: ${currentDbReportsUrl}
+${referenceReportsMsgTxt}
EOF
;;
esac
+analysisSummaryMsgLog="Browse analysis summary:
${link_start}${indexHtmlUrl}${link_end}"
+currentReportsMsgLog="Browse current reports:
${link_start}${currentDbReportsUrl}${link_end}"
+if [ -n "${newReports}" ]; then
+referenceReportsMsgLog="Browse ${reference_kind} reports:
${link_start}${referenceDbReportsUrl}${link_end}"
+fi
case ${ci} in
github)
cat "${summaryTxt}" >"${GITHUB_STEP_SUMMARY}"
@@ -93,8 +134,11 @@ EOF
# Generate summary and print it (GitLab-specific)
cat <
[XEN PATCH 3/4] automation/eclair: add scheduled pipelines
This patch introduces six new ECLAIR jobs that run only
when triggered by a GitLab scheduled pipeline.
Signed-off-by: Simone Ballarin
---
.../eclair_analysis/ECLAIR/action.settings| 2 +-
automation/gitlab-ci/analyze.yaml | 65 +--
2 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/automation/eclair_analysis/ECLAIR/action.settings
b/automation/eclair_analysis/ECLAIR/action.settings
index 71c10d5141..528bc24c72 100644
--- a/automation/eclair_analysis/ECLAIR/action.settings
+++ b/automation/eclair_analysis/ECLAIR/action.settings
@@ -73,7 +73,7 @@ gitlab)
headCommitId="${CI_COMMIT_SHA}"
baseCommitId="${CI_MERGE_REQUEST_DIFF_BASE_SHA}"
;;
-push | pipeline | web)
+push | pipeline | web | schedule)
event=push
if [ -n "${CI_COMMIT_BRANCH:-}" ]; then
ref_kind=branch
diff --git a/automation/gitlab-ci/analyze.yaml
b/automation/gitlab-ci/analyze.yaml
index 3d8166572b..3325ef9d9a 100644
--- a/automation/gitlab-ci/analyze.yaml
+++ b/automation/gitlab-ci/analyze.yaml
@@ -8,6 +8,8 @@
ENABLE_ECLAIR_BOT: "n"
AUTO_PR_BRANCH: "staging"
AUTO_PR_REPOSITORY: "xen-project/xen"
+ script:
+- ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}"
artifacts:
when: always
paths:
@@ -23,8 +25,6 @@ eclair-x86_64:
LOGFILE: "eclair-x86_64.log"
VARIANT: "X86_64"
RULESET: "Set1"
- script:
-- ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}"
allow_failure: true
eclair-ARM64:
@@ -33,6 +33,63 @@ eclair-ARM64:
LOGFILE: "eclair-ARM64.log"
VARIANT: "ARM64"
RULESET: "Set1"
- script:
-- ./automation/scripts/eclair 2>&1 | tee "${LOGFILE}"
+ allow_failure: true
+
+.eclair-analysis:on-schedule:
+ extends: .eclair-analysis
+ rules:
+- if: $CI_PIPELINE_SOURCE == "schedule"
+
+eclair-x86_64-Set1:on-schedule:
+ extends: .eclair-analysis:on-schedule
+ variables:
+VARIANT: "X86_64"
+RULESET: "Set1"
+ANALYSIS_KIND: "${RULESET}-scheduled"
+LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
+ allow_failure: true
+
+eclair-x86_64-Set2:on-schedule:
+ extends: .eclair-analysis:on-schedule
+ variables:
+VARIANT: "X86_64"
+RULESET: "Set2"
+ANALYSIS_KIND: "${RULESET}-scheduled"
+LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
+ allow_failure: true
+
+eclair-x86_64-Set3:on-schedule:
+ extends: .eclair-analysis:on-schedule
+ variables:
+VARIANT: "X86_64"
+RULESET: "Set3"
+ANALYSIS_KIND: "${RULESET}-scheduled"
+LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
+ allow_failure: true
+
+eclair-ARM64-Set1:on-schedule:
+ extends: .eclair-analysis:on-schedule
+ variables:
+VARIANT: "ARM64"
+RULESET: "Set1"
+ANALYSIS_KIND: "${RULESET}-scheduled"
+LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
+ allow_failure: true
+
+eclair-ARM64-Set2:on-schedule:
+ extends: .eclair-analysis:on-schedule
+ variables:
+VARIANT: "ARM64"
+RULESET: "Set2"
+ANALYSIS_KIND: "${RULESET}-scheduled"
+LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
+ allow_failure: true
+
+eclair-ARM64-Set3:on-schedule:
+ extends: .eclair-analysis:on-schedule
+ variables:
+VARIANT: "ARM64"
+RULESET: "Set3"
+ANALYSIS_KIND: "${RULESET}-scheduled"
+LOGFILE: "eclair-${VARIANT}-${RULESET}.log"
allow_failure: true
--
2.34.1
