Re: [PATCH] automation: Add missing and drop obsoleted aliases from containerize
On 02/03/2023 01:23, Stefano Stabellini wrote: > > > On Wed, 1 Mar 2023, Michal Orzel wrote: >> Add missing aliases for: >> - debian:unstable-cppcheck >> - debian:unstable-arm64v8-arm32-gcc >> - ubuntu:bionic >> >> Remove aliases for no longer used containers: >> - centos:7.2 >> - debian:unstable-arm32-gcc >> >> Modify docs to refer to CentOS 7 instead of 7.2 not to create confusion. >> >> Signed-off-by: Michal Orzel > > Acked-by: Stefano Stabellini > > >> --- >> Open questions related to the CI cleanup (@Andrew, @Anthony): >> - Why do we keep suse:sles11sp4 dockerfile? > > please remove, it EOLed in 2016 ok will do > > >> - Why do we keep jessie dockefiles? > > please remove, it EOLed in 2020 For this I would like to check with Anthony and Andrew \wrt recent patches that first modified the Jessie containers to use EOL tag and then removed Jessie related jobs. I am confused why we keep dockerfiles in git tree while not having any jobs for them. Anthony, Andrew? ~Michal
Re: [XEN PATCH v7 12/20] xen/arm: ffa: send guest events to Secure Partitions
Hi Jens,
> On 1 Mar 2023, at 17:45, Jens Wiklander wrote:
>
> Hi,
>
> On Wed, Mar 1, 2023 at 1:58 PM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 1 Mar 2023, at 11:16, Jens Wiklander wrote:
>>>
>>> Hi Bertrand,
>>>
>>> On Tue, Feb 28, 2023 at 5:49 PM Bertrand Marquis
>>> wrote:
Hi Jens,
> On 22 Feb 2023, at 16:33, Jens Wiklander
> wrote:
>
> The FF-A specification defines framework messages sent as direct
> requests when certain events occurs. For instance when a VM (guest) is
> created or destroyed. Only SPs which have subscribed to these events
> will receive them. An SP can subscribe to these messages in its
> partition properties.
>
> Adds a check that the SP supports the needed FF-A features
> FFA_PARTITION_INFO_GET and FFA_RX_RELEASE.
>
> The partition properties of each SP is retrieved with
> FFA_PARTITION_INFO_GET which returns the information in our RX buffer.
> Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
> caller (us), so once we're done with the buffer it must be released
> using FFA_RX_RELEASE before another call can be made.
>
> Signed-off-by: Jens Wiklander
> ---
> xen/arch/arm/tee/ffa.c | 191 -
> 1 file changed, 190 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 07dd5c36d54b..f1b014b6c7f4 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -140,6 +140,14 @@
> #define FFA_MSG_SEND0x846EU
> #define FFA_MSG_POLL0x846AU
>
> +/* Partition information descriptor */
> +struct ffa_partition_info_1_1 {
> +uint16_t id;
> +uint16_t execution_context;
> +uint32_t partition_properties;
> +uint8_t uuid[16];
> +};
> +
> struct ffa_ctx {
> uint32_t guest_vers;
> bool interrupted;
> @@ -148,6 +156,12 @@ struct ffa_ctx {
> /* Negotiated FF-A version to use with the SPMC */
> static uint32_t ffa_version __ro_after_init;
>
> +/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
> +static uint16_t *subscr_vm_created __read_mostly;
> +static unsigned int subscr_vm_created_count __read_mostly;
In the spec the number is returned in w2 so you should utse uint32_t here.
>>>
>>> I don't understand. This value is increased for each SP which has the
>>> property set in the Partition information descriptor.
>>
>> Using generic types should be prevented when possible.
>
> I'm usually of the opposite opinion, fixed width integers should only
> be used when there's a good reason to do so. However, if this is the
> Xen philosophy I can replace all those unsigned int with uint32_t if
> that's preferable.
Safety standards usually enforce to use explicit size types to prevent
compiler dependencies.
>
>> Here this is a subset of the number of partition which is uint32_t (wX reg)
>> so
>> i think this would be the logical type for this.
>
> The maximum number is actually UINT16_MAX so an observant reader might
> wonder why the uint32_t type was used here.
Switching to uint16_t might make sense then but you will have to check that you
are not going over UINT16_MAX in the code as you get a uint32_t back from the
call.
Cheers
Bertrand
>
>>
>>>
> +static uint16_t *subscr_vm_destroyed __read_mostly;
> +static unsigned int subscr_vm_destroyed_count __read_mostly;
Same here
> +
> /*
> * Our rx/tx buffers shared with the SPMC.
> *
> @@ -237,6 +251,72 @@ static int32_t ffa_rxtx_map(register_t tx_addr,
> register_t rx_addr,
> return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
> }
>
> +static int32_t ffa_partition_info_get(uint32_t w1, uint32_t w2, uint32_t
> w3,
> + uint32_t w4, uint32_t w5,
> + uint32_t *count)
> +{
> +const struct arm_smccc_1_2_regs arg = {
> +.a0 = FFA_PARTITION_INFO_GET,
> +.a1 = w1,
> +.a2 = w2,
> +.a3 = w3,
> +.a4 = w4,
> +.a5 = w5,
> +};
> +struct arm_smccc_1_2_regs resp;
> +uint32_t ret;
> +
> +arm_smccc_1_2_smc(&arg, &resp);
> +
> +ret = get_ffa_ret_code(&resp);
> +if ( !ret )
> +*count = resp.a2;
> +
> +return ret;
> +}
> +
> +static int32_t ffa_rx_release(void)
> +{
> +return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
> +}
> +
> +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> + uint8_t msg)
This function is actually only useable to send framework created/destroyed
messages so the function
Re: [PATCH v1] xen/arm: align *(.proc.info) in the linker script
Hi Julien, > > > On Wed, 2023-03-01 at 16:21 +, Julien Grall wrote: > > > > Hi Oleksii, > > > > > > > > On 01/03/2023 16:14, Oleksii Kurochko wrote: > > > > > During testing of bug.h's macros generic implementation > > > > > yocto- > > > > > qemuarm > > > > > job crashed with data abort: > > > > > > > > The commit message is lacking some information. You are telling > > > > us > > > > that > > > > there is an error when building with your series, but this > > > > doesn't > > > > tell > > > > me why this is the correct fix. > > > > > > > > This is also why I asked to have the xen binary because I want > > > > to > > > > check > > > > whether this was a latent bug in Xen or your series effectively > > > > introduce a bug. > > > > > > > > Note that regardless what I just wrote this is a good idea to > > > > align > > > > __proc_info_start. I will try to have a closer look later and > > > > propose > > > > a > > > > commit message and/or any action for your other series. > > > Regarding binaries please take a look here: > > > https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.ca...@gmail.com/ > > > > > > I am not sure if you get my answer as I had the message from > > > delivery > > > server that it was blocked for some reason. > > > > I got the answer. The problem now is gitlab only keep the artifact > > for > > the latest build and it only provide a zImage (having the ELF would > > be > > easier). > > > > I will try to reproduce the error on my end. > > I managed to reproduce it. It looks like that after your bug patch, > *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, > all the messages will be in .rodata.str. Now they are in > .bug_frames.*, > so there some difference in .rodata.*. > > That said, it is not entirely clear why we never seen the issue > before > because my guessing there are no guarantee that .rodata.* will be > suitably aligned. > > Anyway, here a proposal for the commit message: > > " > xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned > > The entries in *(.proc.info) are expected to be 4-byte aligned and > the > compiler will access them using 4-byte load instructions. On Arm32, > the > alignment is strictly enforced by the processor and will result to a > data abort if it is not correct. > > However, the linker script doesn't encode this requirement. So we are > at > the mercy of the compiler/linker to have aligned the previous > sections > suitably. > > This was spotted when trying to use the upcoming generic bug > infrastructure with the compiler provided by Yocto. > > Link: > https://lore.kernel.org/xen-devel/6735859208c6dcb7320f89664ae298005f70827b.ca...@gmail.com/ > " > > If you are happy with the proposed commit message, then I can update > it > while committing. I am happy with the proposed commit message. Thanks a lot. ~ Oleksii
xl restore domu fails to load state for instance 0x0 for virtio-gpu
Hi All, i am verifying xl save/restore functionality. While restoring the ubuntu domu state , it fails with instance error for "virtio-gpu". Can someone help how to overcome this error. *Setup:* Dom0 : Ubuntu + Xen + qemu DomU: Ubuntu *Error:* qemu-system-i386: Failed to load virtio-gpu:virtio-gpu qemu-system-i386: error while loading state for instance 0x0 of device ':00:03.0/virtio-gpu' qemu-system-i386: load of migration failed: Invalid argument Thanks Sudheer
[linux-linus test] 178910: regressions - trouble: fail/pass/starved
flight 178910 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/178910/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-freebsd12-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-qemuu-nested-intel 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-ws16-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-xl-pvshim8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-vhd 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-intel 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-win7-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-win7-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-credit1 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-shadow8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-amd 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-qemuu-nested-amd 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-ws16-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-examine-uefi 8 reboot fail REGR. vs. 178042 test-amd64-amd64-freebsd11-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-raw 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-pygrub 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-pair12 xen-boot/src_hostfail REGR. vs. 178042 test-amd64-amd64-pair13 xen-boot/dst_hostfail REGR. vs. 178042 test-amd64-amd64-libvirt-qcow2 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-pair 12 xen-boot/src_host fail REGR. vs. 178042 test-amd64-amd64-libvirt-pair 13 xen-boot/dst_host fail REGR. vs. 178042 test-amd64-coresched-amd64-xl 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-amd64 8 xen-bootfail REGR. vs. 178042 test-arm64-arm64-xl-credit1 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl-xsm 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-libvirt-xsm 17 guest-stop fail REGR. vs. 178042 test-amd64-amd64-xl-credit2 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt 8 xen-boot fail REGR. vs. 178042 test-arm64-arm64-xl-thunderx 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-ovmf-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-examine 8 reboot fail REGR. vs. 178042 test-amd64-amd64-examine-bios 8 reboot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-multivcpu 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl-vhd 12 debian-di-installfail REGR. vs. 178042 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 178042 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 8 xen-boot fail REGR. vs. 178042 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1
[ImageBuilder][PATCH 2/2] uboot-script-gen: Add support for static shared memory
Introduce support for creating shared-mem node for dom0less domUs in
the device tree. Add the following options:
- DOMU_SHARED_MEM[number]="HPA GPA size"
if specified, indicates the host physical address HPA will get mapped
at guest address GPA in domU and the memory of size will be reserved
to be shared memory.
- DOMU_SHARED_MEM_ID[number]
An arbitrary string that represents the unique identifier of the shared
memory region, with a strict limit on the number of characters(\0
included)
The static shared memory is used between two dom0less domUs.
Below is an example:
NUM_DOMUS=2
DOMU_SHARED_MEM[0]="0x5000 0x600 0x1000"
DOMU_SHARED_MEM_ID[0]="my-shared-mem-0"
DOMU_SHARED_MEM[1]="0x5000 0x600 0x1000"
DOMU_SHARED_MEM_ID[1]="my-shared-mem-0"
This static shared memory region is identified as "my-shared-mem-0", host
physical address starting at 0x5000 of 256MB will be reserved to be
shared between two domUs. It will get mapped at 0x600 in both guest
physical address space. Both DomUs are the borrower domain, the owner
domain is the default owner domain DOMID_IO.
Signed-off-by: jiamei.xie
---
README.md| 18 ++
scripts/uboot-script-gen | 26 ++
2 files changed, 44 insertions(+)
diff --git a/README.md b/README.md
index 787f413..48044ee 100644
--- a/README.md
+++ b/README.md
@@ -192,6 +192,24 @@ Where:
if specified, indicates the host physical address regions
[baseaddr, baseaddr + size) to be reserved to the VM for static allocation.
+- DOMU_SHARED_MEM[number]="HPA GPA size" and DOMU_SHARED_MEM_ID[number]
+ if specified, indicate the host physical address HPA will get mapped at
+ guest address GPA in domU and the memory of size will be reserved to be
+ shared memory. The shared memory is used between two dom0less domUs.
+
+ Below is an example:
+ NUM_DOMUS=2
+ DOMU_SHARED_MEM[0]="0x5000 0x600 0x1000"
+ DOMU_SHARED_MEM_ID[0]="my-shared-mem-0"
+ DOMU_SHARED_MEM[1]="0x5000 0x600 0x1000"
+ DOMU_SHARED_MEM_ID[1]="my-shared-mem-0"
+
+ This static shared memory region is identified as "my-shared-mem-0", host
+ physical address starting at 0x5000 of 256MB will be reserved to be
+ shared between two domUs. It will get mapped at 0x600 in both guest
+ physical address space. Both DomUs are the borrower domain, the owner
+ domain is the default owner domain DOMID_IO.
+
- DOMU_DIRECT_MAP[number] can be set to 1 or 0.
If set to 1, the VM is direct mapped. The default is 1.
This is only applicable when DOMU_STATIC_MEM is specified.
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index 4775293..46215c8 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -204,6 +204,27 @@ function add_device_tree_static_heap()
dt_set "$path" "xen,static-heap" "hex" "${cells[*]}"
}
+function add_device_tree_static_shared_mem()
+{
+local path=$1
+local domid=$2
+local regions=$3
+local SHARED_MEM_ID=$4
+local cells=()
+local SHARED_MEM_HOST=${regions%% *}
+
+dt_mknode "${path}" "domU${domid}-shared-mem@${SHARED_MEM_HOST}"
+
+for val in ${regions[@]}
+do
+cells+=("$(printf "0x%x 0x%x" $(($val >> 32)) $(($val & ((1 << 32) -
1")
+done
+
+dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "compatible"
"str" "xen,domain-shared-memory-v1"
+dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}" "xen,shm-id"
"str" "${SHARED_MEM_ID}"
+dt_set "${path}/domU${domid}-shared-mem@${SHARED_MEM_HOST}"
"xen,shared-mem" "hex" "${cells[*]}"
+}
+
function add_device_tree_cpupools()
{
local cpu
@@ -329,6 +350,11 @@ function xen_device_tree_editing()
dt_set "/chosen/domU$i" "xen,enhanced" "str" "enabled"
fi
+if test -n "${DOMU_SHARED_MEM[i]}" -a -n "${DOMU_SHARED_MEM_ID[i]}"
+then
+add_device_tree_static_shared_mem "/chosen/domU${i}" "${i}"
"${DOMU_SHARED_MEM[i]}" "${DOMU_SHARED_MEM_ID[i]}"
+fi
+
if test "${DOMU_COLORS[$i]}"
then
local startcolor=$(echo "${DOMU_COLORS[$i]}" | cut -d "-" -f 1)
--
2.25.1
[ImageBuilder][PATCH 1/2] uboot-script-gen: Add support for static heap
From: jiamei Xie
Add a new config parameter to configure static heap.
STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
if specified, indicates the host physical address regions
[baseaddr, baseaddr + size) to be reserved as static heap.
For instance, STATIC_HEAP="0x5000 0x3000", if specified,
indicates the host memory region starting from paddr 0x5000
with a size of 0x3000 to be reserved as static heap.
Signed-off-by: jiamei Xie
---
README.md| 4
scripts/uboot-script-gen | 20
2 files changed, 24 insertions(+)
diff --git a/README.md b/README.md
index 814a004..787f413 100644
--- a/README.md
+++ b/README.md
@@ -256,6 +256,10 @@ Where:
- NUM_CPUPOOLS specifies the number of boot-time cpupools to create.
+- STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
+ if specified, indicates the host physical address regions
+ [baseaddr, baseaddr + size) to be reserved as static heap.
+
Then you can invoke uboot-script-gen as follows:
```
diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
index f07e334..4775293 100755
--- a/scripts/uboot-script-gen
+++ b/scripts/uboot-script-gen
@@ -189,6 +189,21 @@ function add_device_tree_static_mem()
dt_set "$path" "xen,static-mem" "hex" "${cells[*]}"
}
+function add_device_tree_static_heap()
+{
+local path=$1
+local regions=$2
+local cells=()
+local val
+
+for val in ${regions[@]}
+do
+cells+=("$(printf "0x%x 0x%x" $(($val >> 32)) $(($val & ((1 << 32) -
1")
+done
+
+dt_set "$path" "xen,static-heap" "hex" "${cells[*]}"
+}
+
function add_device_tree_cpupools()
{
local cpu
@@ -344,6 +359,11 @@ function xen_device_tree_editing()
then
add_device_tree_cpupools
fi
+
+if test "${STATIC_HEAP}"
+then
+add_device_tree_static_heap "/chosen" "${STATIC_HEAP}"
+fi
}
function linux_device_tree_editing()
--
2.25.1
[ImageBuilder][PATCH 0/2] uboot-script-gen: Add support for static heap and shared memory
Hi all This patch series is to enable to set the device tree for static heap and static shared memory by uboot-script-gen jiamei Xie (1): uboot-script-gen: Add support for static heap jiamei.xie (1): uboot-script-gen: Add support for static shared memory README.md| 22 +++ scripts/uboot-script-gen | 46 2 files changed, 68 insertions(+) -- 2.25.1
[PATCH 2/2] automation: arm64: Create a test job for testing static shared memory on qemu
Create a new test job, called qemu-smoke-dom0less-arm64-gcc-static-shared-mem.
Adjust qemu-smoke-dom0less-arm64.sh script to accomodate the static
shared memory test as a new test variant. The test variant is determined
based on the first argument passed to the script. For testing static
shared memory, the argument is 'static-shared-mem'.
The test configures two dom0less DOMUs with a static shared memory
region and adds a check in the init script.
The check consists in comparing the contents of the
/proc/device-tree/reserved-memory
xen-shmem entry with the static shared memory range and id with which
DOMUs were configured. If the memory layout is correct, a message gets
printed by DOMU.
At the end of the qemu run, the script searches for the specific message
in the logs and fails if not found.
Signed-off-by: jiamei.xie
---
automation/gitlab-ci/build.yaml | 18
automation/gitlab-ci/test.yaml| 16 ++
.../scripts/qemu-smoke-dom0less-arm64.sh | 29 +++
3 files changed, 63 insertions(+)
diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 38bb22d860..820cc0af83 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -623,6 +623,24 @@ alpine-3.12-gcc-debug-arm64-staticmem:
CONFIG_UNSUPPORTED=y
CONFIG_STATIC_MEMORY=y
+alpine-3.12-gcc-arm64-static-shared-mem:
+ extends: .gcc-arm64-build
+ variables:
+CONTAINER: alpine:3.12-arm64v8
+EXTRA_XEN_CONFIG: |
+ CONFIG_UNSUPPORTED=y
+ CONFIG_STATIC_MEMORY=y
+ CONFIG_STATIC_SHM=y
+
+alpine-3.12-gcc-debug-arm64-static-shared-mem:
+ extends: .gcc-arm64-build-debug
+ variables:
+CONTAINER: alpine:3.12-arm64v8
+EXTRA_XEN_CONFIG: |
+ CONFIG_UNSUPPORTED=y
+ CONFIG_STATIC_MEMORY=y
+ CONFIG_STATIC_SHM=y
+
alpine-3.12-gcc-arm64-boot-cpupools:
extends: .gcc-arm64-build
variables:
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 5a9b88477a..f4d36babda 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -149,6 +149,22 @@ qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
- *arm64-test-needs
- alpine-3.12-gcc-debug-arm64
+qemu-smoke-dom0less-arm64-gcc-static-shared-mem:
+ extends: .qemu-arm64
+ script:
+- ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-shared-mem 2>&1
| tee ${LOGFILE}
+ needs:
+- *arm64-test-needs
+- alpine-3.12-gcc-arm64-static-shared-mem
+
+qemu-smoke-dom0less-arm64-gcc-debug-static-shared-mem:
+ extends: .qemu-arm64
+ script:
+- ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-shared-mem 2>&1
| tee ${LOGFILE}
+ needs:
+- *arm64-test-needs
+- alpine-3.12-gcc-debug-arm64-static-shared-mem
+
qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
extends: .qemu-arm64
script:
diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh
b/automation/scripts/qemu-smoke-dom0less-arm64.sh
index 4e73857199..fe3a282726 100755
--- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
+++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
@@ -32,6 +32,25 @@ if [[ "${test_variant}" == "static-heap" ]]; then
domU_check="echo \"${passed}\""
fi
+
+if [[ "${test_variant}" == "static-shared-mem" ]]; then
+passed="${test_variant} test passed"
+SHARED_MEM_HOST="5000"
+SHARED_MEM_GUEST="400"
+SHARED_MEM_SIZE="1000"
+SHARED_MEM_ID="my-shared-mem-0"
+
+domU_check="
+current_id=\$(cat /proc/device-tree/reserved-memory/xen-shmem@400/xen,id
2>/dev/null)
+expected_id=\"\$(echo ${SHARED_MEM_ID})\"
+current_reg=\$(hexdump -e '16/1 \"%02x\"'
/proc/device-tree/reserved-memory/xen-shmem@400/reg 2>/dev/null)
+expected_reg=$(printf \"%016x%016x\" 0x${SHARED_MEM_GUEST}
0x${SHARED_MEM_SIZE})
+if [[ \"\${expected_reg}\" == \"\${current_reg}\" && \"\${current_id}\" ==
\"\${expected_id}\" ]]; then
+echo \"${passed}\"
+fi
+"
+fi
+
if [[ "${test_variant}" == "boot-cpupools" ]]; then
# Check if domU0 (id=1) is assigned to Pool-1 with null scheduler
passed="${test_variant} test passed"
@@ -133,6 +152,16 @@ if [[ "${test_variant}" == "static-mem" ]]; then
echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >>
binaries/config
fi
+if [[ "${test_variant}" == "static-shared-mem" ]]; then
+echo "NUM_DOMUS=2
+DOMU_SHARED_MEM[0]=\""0x${SHARED_MEM_HOST} 0x${SHARED_MEM_GUEST}
0x${SHARED_MEM_SIZE}"\"
+DOMU_SHARED_MEM_ID[0]="${SHARED_MEM_ID}"
+DOMU_KERNEL[1]=\"Image\"
+DOMU_RAMDISK[1]=\"initrd\"
+DOMU_SHARED_MEM[1]=\"0x${SHARED_MEM_HOST} 0x${SHARED_MEM_GUEST}
0x${SHARED_MEM_SIZE}\"
+DOMU_SHARED_MEM_ID[1]=\"${SHARED_MEM_ID}\"" >> binaries/config
+fi
+
if [[ "${test_variant}" == "static-heap" ]]; then
# ImageBuilder uses the config file to create the uboot script. Devicetree
# will be set via the generated uboot script.
--
2.25.1
[PATCH 1/2] automation: arm64: Create a test job for testing static heap on qemu
From: Jiamei Xie
Create a new test job, called qemu-smoke-dom0less-arm64-gcc-staticheap.
Add property "xen,static-heap" under /chosen node to enable static-heap.
If the domU can start successfully with static-heap enabled, then this
test pass.
Signed-off-by: Jiamei Xie
---
automation/gitlab-ci/test.yaml | 16
.../scripts/qemu-smoke-dom0less-arm64.sh | 18 ++
2 files changed, 34 insertions(+)
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 1c5f400b68..5a9b88477a 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -133,6 +133,22 @@ qemu-smoke-dom0less-arm64-gcc-debug-staticmem:
- *arm64-test-needs
- alpine-3.12-gcc-debug-arm64-staticmem
+qemu-smoke-dom0less-arm64-gcc-staticheap:
+ extends: .qemu-arm64
+ script:
+ - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap 2>&1 | tee
${LOGFILE}
+ needs:
+ - *arm64-test-needs
+ - alpine-3.12-gcc-arm64
+
+qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
+ extends: .qemu-arm64
+ script:
+ - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap 2>&1 | tee
${LOGFILE}
+ needs:
+ - *arm64-test-needs
+ - alpine-3.12-gcc-debug-arm64
+
qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
extends: .qemu-arm64
script:
diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh
b/automation/scripts/qemu-smoke-dom0less-arm64.sh
index 182a4b6c18..4e73857199 100755
--- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
+++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
@@ -27,6 +27,11 @@ fi
"
fi
+if [[ "${test_variant}" == "static-heap" ]]; then
+passed="${test_variant} test passed"
+domU_check="echo \"${passed}\""
+fi
+
if [[ "${test_variant}" == "boot-cpupools" ]]; then
# Check if domU0 (id=1) is assigned to Pool-1 with null scheduler
passed="${test_variant} test passed"
@@ -128,6 +133,19 @@ if [[ "${test_variant}" == "static-mem" ]]; then
echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >>
binaries/config
fi
+if [[ "${test_variant}" == "static-heap" ]]; then
+# ImageBuilder uses the config file to create the uboot script. Devicetree
+# will be set via the generated uboot script.
+# The valid memory range is 0x4000 to 0x8000 as defined before.
+# ImageBuillder sets the kernel and ramdisk range based on the file size.
+# It will use the memory range between 0x4560 to 0x47AED1E8, so set
+# memory range between 0x5000 and 0x8000 as static heap.
+echo '
+STATIC_HEAP="0x5000 0x3000"
+# The size of static heap should be greater than the guest memory
+DOMU_MEM[0]="128"' >> binaries/config
+fi
+
if [[ "${test_variant}" == "boot-cpupools" ]]; then
echo '
CPUPOOL[0]="cpu@1 null"
--
2.25.1
[PATCH 0/2] automation: introduce static heap and shared memory tests
Hi all, This patch series introduces two test jobs: Jiamei Xie (1): automation: arm64: Create a test job for testing static heap on qemu jiamei.xie (1): automation: arm64: Create a test job for testing static shared memory on qemu automation/gitlab-ci/build.yaml | 18 +++ automation/gitlab-ci/test.yaml| 32 + .../scripts/qemu-smoke-dom0less-arm64.sh | 47 +++ 3 files changed, 97 insertions(+) -- 2.25.1
Re: [PATCH 07/10] xen/physinfo: add arm SVE arch capability and vector length
On Thu, 23 Feb 2023, Bertrand Marquis wrote:
> Hi Jan,
>
> > On 13 Feb 2023, at 09:36, Jan Beulich wrote:
> >
> > On 10.02.2023 16:54, Luca Fancellu wrote:
> >>> On 2 Feb 2023, at 12:05, Jan Beulich wrote:
> >>> On 02.02.2023 12:08, Luca Fancellu wrote:
> (is hw_cap only for x86?)
> >>>
> >>> I suppose it is, but I also expect it would better go away than be moved.
> >>> It doesn't hold a complete set of information, and it has been superseded.
> >>>
> >>> Question is (and I think I did raise this before, perhaps in different
> >>> context) whether Arm wouldn't want to follow x86 in how hardware as well
> >>> as hypervisor derived / used ones are exposed to the tool stack
> >>> (XEN_SYSCTL_get_cpu_featureset). I guess there's nothing really precluding
> >>> that data to consist of more than just boolean fields.
> >>
> >> Yes I guess that infrastructure could work, however I don’t have the
> >> bandwidth to
> >> put it in place at the moment, so I would like the Arm maintainers to give
> >> me a
> >> suggestion on how I can expose the vector length to XL if putting its
> >> value here
> >> needs to be avoided
> >
> > Since you've got a reply from Andrew boiling down to the same suggestion
> > (or should I even say request), I guess it wants seriously considering
> > to introduce abstract base infrastructure first. As Andrew says, time not
> > invested now will very likely mean yet more time to be invested later.
> >
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -18,7 +18,7 @@
> #include "domctl.h"
> #include "physdev.h"
>
> -#define XEN_SYSCTL_INTERFACE_VERSION 0x0015
> +#define XEN_SYSCTL_INTERFACE_VERSION 0x0016
> >>>
> >>> Why? You ...
> >>>
> @@ -104,7 +110,8 @@ struct xen_sysctl_physinfo {
> uint32_t cpu_khz;
> uint32_t capabilities;/* XEN_SYSCTL_PHYSCAP_??? */
> uint32_t arch_capabilities;/* XEN_SYSCTL_PHYSCAP_{X86,ARM,...}_??? */
> -uint32_t pad;
> +uint16_t arm_sve_vl_bits;
> +uint16_t pad;
> uint64_aligned_t total_pages;
> uint64_aligned_t free_pages;
> uint64_aligned_t scrub_pages;
> >>>
> >>> ... add no new fields, and the only producer of the data zero-fills the
> >>> struct first thing.
> >>
> >> Yes that is right, I will wait to understand how I can proceed here:
> >>
> >> 1) rename arch_capabilities to arm_sve_vl_bits and put vector length there.
> >> 2) leave arch_capabilities untouched, no flag creation/setting, create
> >> uint32_t arm_sve_vl_bits field removing “pad”,
> >>Use its value to determine if SVE is present or not.
> >> 3) ??
> >
> > 3) Introduce suitable #define(s) to use part of arch_capabilities for your
> > purpose, without renaming the field. (But that's of course only if Arm
> > maintainers agree with you on _not_ going the proper feature handling route
> > right away.)
>
> As Luca said, he does not have the required bandwidth to do this so I think
> it is ok for him to go with your solution 3.
>
> @Julien/Stefano: any thoughts here ?
I am OK with that
Re: [PATCH] xen/arm: check max_init_domid validity
On Tue, 28 Feb 2023, Bertrand Marquis wrote:
> Before trying to create a dom0less guest, check that max_init_domid
> increment will generate a valid domain ID, lower than
> DOMID_FIRST_RESERVED.
>
> Signed-off-by: Bertrand Marquis
Reviewed-by: Stefano Stabellini
> ---
> xen/arch/arm/domain_build.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index edca23b986d2..9707eb7b1bb1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3879,6 +3879,9 @@ void __init create_domUs(void)
> if ( !dt_device_is_compatible(node, "xen,domain") )
> continue;
>
> +if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
> +panic("No more domain IDs available\n");
> +
> if ( dt_find_property(node, "xen,static-mem", NULL) )
> flags |= CDF_staticmem;
>
> --
> 2.25.1
>
[PATCH 2/2] automation: introduce a dom0less test run on Xilinx hardware
From: Stefano Stabellini
The test prepares dom0 and domU binaries and boot artifacts, similarly
to the existing QEMU test. (TBD: share preparation steps with the
regular QEMU tests.)
However, instead of running the test inside QEMU as usual, it copies
the binaries to the tftp server root, triggers a Xilinx ZCU102 board
reboot, and connects to the real serial of the board.
For now and due to its novelty, allow_failure on the Xilinx hardware
test.
Signed-off-by: Stefano Stabellini
---
automation/gitlab-ci/test.yaml| 19 +++
.../scripts/xilinx-smoke-dom0less-arm64.sh| 115 ++
2 files changed, 134 insertions(+)
create mode 100755 automation/scripts/xilinx-smoke-dom0less-arm64.sh
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 1c5f400b68..88bb47f26c 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -85,6 +85,25 @@ build-each-commit-gcc:
tags:
- x86_64
+xilinx-smoke-dom0less-arm64-gcc:
+ extends: .test-jobs-common
+ variables:
+CONTAINER: ubuntu:xenial-xilinx
+LOGFILE: qemu-smoke-xilinx.log
+ artifacts:
+paths:
+ - smoke.serial
+ - '*.log'
+when: always
+ tags:
+- xilinx
+ script:
+- ./automation/scripts/xilinx-smoke-dom0less-arm64.sh 2>&1 | tee ${LOGFILE}
+ needs:
+- *arm64-test-needs
+- alpine-3.12-gcc-arm64
+ allow_failure: true
+
qemu-smoke-dom0-arm64-gcc:
extends: .qemu-arm64
script:
diff --git a/automation/scripts/xilinx-smoke-dom0less-arm64.sh
b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
new file mode 100755
index 00..180c8b5f1c
--- /dev/null
+++ b/automation/scripts/xilinx-smoke-dom0less-arm64.sh
@@ -0,0 +1,115 @@
+#!/bin/bash
+
+set -ex
+
+test_variant=$1
+
+if [ -z "${test_variant}" ]; then
+passed="ping test passed"
+domU_check="
+until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do
+sleep 30
+done
+echo \"${passed}\"
+"
+fi
+
+# DomU
+mkdir -p rootfs
+cd rootfs
+tar xzf ../binaries/initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+echo "#!/bin/sh
+
+${domU_check}
+/bin/sh" > etc/local.d/xen.start
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+find . | cpio -H newc -o | gzip > ../binaries/domU-rootfs.cpio.gz
+cd ..
+rm -rf rootfs
+
+# DOM0 rootfs
+mkdir -p rootfs
+cd rootfs
+tar xzf ../binaries/initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../binaries/dist/install/* .
+
+echo "#!/bin/bash
+
+export LD_LIBRARY_PATH=/usr/local/lib
+bash /etc/init.d/xencommons start
+
+/usr/local/lib/xen/bin/init-dom0less
+
+brctl addbr xenbr0
+brctl addif xenbr0 eth0
+ifconfig eth0 up
+ifconfig xenbr0 up
+ifconfig xenbr0 192.168.0.1
+
+xl network-attach 1 type=vif
+${dom0_check}
+" > etc/local.d/xen.start
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+find . | cpio -H newc -o | gzip > ../binaries/dom0-rootfs.cpio.gz
+cd ..
+
+
+TFTP=/scratch/gitlab-runner/tftp
+START=`pwd`
+
+# ImageBuilder
+echo 'MEMORY_START="0"
+MEMORY_END="0xC000"
+
+DEVICE_TREE="mpsoc.dtb"
+XEN="xen"
+DOM0_KERNEL="Image"
+DOM0_RAMDISK="dom0-rootfs.cpio.gz"
+XEN_CMD="console=dtuart dom0_mem=1024M"
+
+NUM_DOMUS=1
+DOMU_KERNEL[0]="Image"
+DOMU_RAMDISK[0]="domU-rootfs.cpio.gz"
+DOMU_MEM[0]="1024"
+
+LOAD_CMD="tftpb"
+UBOOT_SOURCE="boot.source"
+UBOOT_SCRIPT="boot.scr"' > $TFTP/config
+
+cp -f binaries/xen $TFTP/
+cp -f binaries/Image $TFTP/
+cp -f binaries/dom0-rootfs.cpio.gz $TFTP/
+cp -f binaries/domU-rootfs.cpio.gz $TFTP/
+
+rm -rf imagebuilder
+git clone https://gitlab.com/ViryaOS/imagebuilder
+bash imagebuilder/scripts/uboot-script-gen -t tftp -d $TFTP/ -c $TFTP/config
+
+# restart the board
+cd /scratch/gitlab-runner
+bash zcu102.sh 2
+sleep 5
+bash zcu102.sh 1
+sleep 5
+cd $START
+
+# connect to serial
+set +e
+stty -F /dev/ttyUSB0 115200
+timeout -k 1 120 nohup sh -c "cat /dev/ttyUSB0 | tee smoke.serial"
+
+set -e
+(grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}"
smoke.serial) || exit 1
+exit 0
--
2.25.1
[PATCH 1/2] automation: add Ubuntu container for Xilinx hardware tests
From: Stefano Stabellini This container is only run on the controller PC (x86) to trigger the test on a connected Xilinx ZCU102 physical board. Signed-off-by: Stefano Stabellini --- .../build/ubuntu/xenial-xilinx.dockerfile | 25 +++ 1 file changed, 25 insertions(+) create mode 100644 automation/build/ubuntu/xenial-xilinx.dockerfile diff --git a/automation/build/ubuntu/xenial-xilinx.dockerfile b/automation/build/ubuntu/xenial-xilinx.dockerfile new file mode 100644 index 00..7e4f5d6605 --- /dev/null +++ b/automation/build/ubuntu/xenial-xilinx.dockerfile @@ -0,0 +1,25 @@ +FROM ubuntu:16.04 +LABEL maintainer.name="The Xen Project " \ + maintainer.email="xen-devel@lists.xenproject.org" + +ENV DEBIAN_FRONTEND=noninteractive +ENV USER root + +RUN mkdir /build +WORKDIR /build + +# build depends +RUN apt-get update && \ +apt-get --quiet --yes install \ +snmp \ +snmp-mibs-downloader \ +u-boot-tools \ +device-tree-compiler \ +cpio \ +git \ +gzip \ +file \ +&& \ +apt-get autoremove -y && \ +apt-get clean && \ +rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* -- 2.25.1
[PATCH 0/2] automation: introduce a Xilinx hardware test
Hi all, This short patch series introduces the first Xen gitlab-ci test run on real hardware: a physical Xilinx ZCU102 board. The gitlab container is run on a workstation physically connected to a ZCU102 board. The test script looks very similar to a regular QEMU test script, except that at the end it reboots the physical board and connects to the serial instead of launching QEMU. Cheers, Stefano
Re: [PATCH] automation: Add missing and drop obsoleted aliases from containerize
On Wed, 1 Mar 2023, Michal Orzel wrote:
> Add missing aliases for:
> - debian:unstable-cppcheck
> - debian:unstable-arm64v8-arm32-gcc
> - ubuntu:bionic
>
> Remove aliases for no longer used containers:
> - centos:7.2
> - debian:unstable-arm32-gcc
>
> Modify docs to refer to CentOS 7 instead of 7.2 not to create confusion.
>
> Signed-off-by: Michal Orzel
Acked-by: Stefano Stabellini
> ---
> Open questions related to the CI cleanup (@Andrew, @Anthony):
> - Why do we keep suse:sles11sp4 dockerfile?
please remove, it EOLed in 2016
> - Why do we keep jessie dockefiles?
please remove, it EOLed in 2020
> ---
> automation/build/README.md | 4 ++--
> automation/scripts/containerize | 5 +++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/automation/build/README.md b/automation/build/README.md
> index 4cc1acb6b41c..2d07cafe0eaa 100644
> --- a/automation/build/README.md
> +++ b/automation/build/README.md
> @@ -44,10 +44,10 @@ understands.
>DOCKER_CMD=podman ./automation/scripts/containerize make
>```
>
> -- CONTAINER: This overrides the container to use. For CentOS 7.2, use:
> +- CONTAINER: This overrides the container to use. For CentOS 7, use:
>
>```
> - CONTAINER=centos72 ./automation/scripts/containerize make
> + CONTAINER=centos7 ./automation/scripts/containerize make
>```
>
> - CONTAINER_PATH: This overrides the path that will be available under the
> diff --git a/automation/scripts/containerize b/automation/scripts/containerize
> index 9b1a302d0565..5476ff0ea10d 100755
> --- a/automation/scripts/containerize
> +++ b/automation/scripts/containerize
> @@ -29,7 +29,6 @@ case "_${CONTAINER}" in
> _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
> _riscv64) CONTAINER="${BASE}/archlinux:current-riscv64" ;;
> _centos7) CONTAINER="${BASE}/centos:7" ;;
> -_centos72) CONTAINER="${BASE}/centos:7.2" ;;
> _fedora) CONTAINER="${BASE}/fedora:29";;
> _focal) CONTAINER="${BASE}/ubuntu:focal" ;;
> _jessie) CONTAINER="${BASE}/debian:jessie" ;;
> @@ -39,8 +38,10 @@ case "_${CONTAINER}" in
> _buster-gcc-ibt) CONTAINER="${BASE}/debian:buster-gcc-ibt" ;;
> _unstable|_) CONTAINER="${BASE}/debian:unstable" ;;
> _unstable-i386) CONTAINER="${BASE}/debian:unstable-i386" ;;
> -_unstable-arm32-gcc) CONTAINER="${BASE}/debian:unstable-arm32-gcc" ;;
> +_unstable-arm64v8-arm32-gcc)
> CONTAINER="${BASE}/debian:unstable-arm64v8-arm32-gcc" ;;
> _unstable-arm64v8) CONTAINER="${BASE}/debian:unstable-arm64v8" ;;
> +_unstable-cppcheck) CONTAINER="${BASE}/debian:unstable-cppcheck" ;;
> +_bionic) CONTAINER="${BASE}/ubuntu:bionic" ;;
> _trusty) CONTAINER="${BASE}/ubuntu:trusty" ;;
> _xenial) CONTAINER="${BASE}/ubuntu:xenial" ;;
> _opensuse-leap|_leap) CONTAINER="${BASE}/suse:opensuse-leap" ;;
> --
> 2.25.1
>
Re: [PATCH v2 2/2] xen/misra: add entries to exclude-list.json
On Wed, 1 Mar 2023, Luca Fancellu wrote:
> Add entries to the exclude-list.json for those files that need to be
> excluded from the analysis scan.
>
> Signed-off-by: Luca Fancellu
> Signed-off-by: Michal Orzel
I checked the results both x86 and arm and they look much more
reasonable (with the exception of the way too many unusedStructMember
reports on x86).
Reviewed-by: Stefano Stabellini
> ---
> This list is originated from Michal's work here:
> https://patchwork.kernel.org/project/xen-devel/patch/20221116092032.4423-1-michal.or...@amd.com/#25123099
> and changed to adapt to this task.
> Changes from v1:
> - updated list with new files from Stefano
> - add comment field for every entry (Jan)
> ---
> ---
> docs/misra/exclude-list.json | 199 ++-
> 1 file changed, 198 insertions(+), 1 deletion(-)
>
> diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
> index 1fb0ac67747b..ca1e2dd678ff 100644
> --- a/docs/misra/exclude-list.json
> +++ b/docs/misra/exclude-list.json
> @@ -1,4 +1,201 @@
> {
> "version": "1.0",
> -"content": []
> +"content": [
> +{
> +"rel_path": "arch/arm/arm64/cpufeature.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/arm/arm64/insn.c",
> +"comment": "Imported on Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/arm/arm64/lib/find_next_bit.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/acpi/boot.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/acpi/cpu_idle.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/acpi/cpufreq/cpufreq.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/acpi/cpuidle_menu.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/acpi/lib.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/amd.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/centaur.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/common.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/hygon.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/intel.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/intel_cacheinfo.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/mcheck/non-fatal.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/mtrr/*",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/cpu/mwait-idle.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/delay.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/dmi_scan.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/mpparse.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/srat.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/time.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "common/bitmap.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "common/bunzip2.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "common/earlycpio.c",
> +"comment": "Imported from Linux, ignore for now"
> +},
> +{
> +"rel_path": "common/inflate.c",
> +"comment": "Imported from Linux, ignore for
Re: [PATCH v2 1/2] xen/cppcheck: add a way to exclude files from the scan
On Wed, 1 Mar 2023, Luca Fancellu wrote:
> Add a way to exclude files from the scan, in this way we can skip
> some findings from the report on those files that Xen doesn't own.
>
> To do that, introduce the exclude-list.json file under docs/misra,
> this file will be populated with relative path to the files/folder
> to be excluded.
> Introduce a new module, exclusion_file_list.py, to deal with the
> exclusion list file and use the new module in cppcheck_analysis.py
> to take a list of excluded paths to update the suppression list of
> cppcheck.
> Modified --suppress flag for cppcheck tool to remove
> unmatchedSuppression findings for those external file that are
> listed but for example not built for the current architecture.
>
> Add documentation for the file.
>
> Signed-off-by: Luca Fancellu
> ---
> Changes from v1:
> - Indentation fixed (Jan)
> ---
> ---
> docs/misra/exclude-list.json | 4 +
> docs/misra/exclude-list.rst | 44 +++
> xen/scripts/xen_analysis/cppcheck_analysis.py | 20 -
> .../xen_analysis/exclusion_file_list.py | 79 +++
> 4 files changed, 145 insertions(+), 2 deletions(-)
> create mode 100644 docs/misra/exclude-list.json
> create mode 100644 docs/misra/exclude-list.rst
> create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py
>
> diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
> new file mode 100644
> index ..1fb0ac67747b
> --- /dev/null
> +++ b/docs/misra/exclude-list.json
> @@ -0,0 +1,4 @@
> +{
> +"version": "1.0",
> +"content": []
> +}
> diff --git a/docs/misra/exclude-list.rst b/docs/misra/exclude-list.rst
> new file mode 100644
> index ..969539c46beb
> --- /dev/null
> +++ b/docs/misra/exclude-list.rst
> @@ -0,0 +1,44 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Exclude file list for xen-analysis script
> +=
> +
> +The code analysis is performed on the Xen codebase for both MISRA checkers
> and
> +static analysis checkers, there are some files however that needs to be
> removed
> +from the findings report because they are not owned by Xen and they must be
> kept
> +in sync with their origin (completely or even partially), hence we can't
> easily
> +fix findings or deviate from them.
I would stay more generic and say something like:
The code analysis is performed on the Xen codebase for both MISRA
checkers and static analysis checkers, there are some files however that
needs to be removed from the findings report for various reasons (e.g.
they are imported from external sources, they generate too many false
positive results, etc.).
But what you wrote is also OK.
> +For this reason the file docs/misra/exclude-list.json is used to exclude
> every
> +entry listed in that file from the final report.
> +Currently only the cppcheck analysis will use this file.
> +
> +Here is an example of the exclude-list.json file::
> +
> +|{
> +|"version": "1.0",
> +|"content": [
> +|{
> +|"rel_path": "relative/path/from/xen/file",
> +|"comment": "This file is originated from ..."
> +|},
> +|{
> +|"rel_path": "relative/path/from/xen/folder/*",
> +|"comment": "This folder is a library"
> +|},
> +|{
> +|"rel_path": "relative/path/from/xen/mem*.c",
> +|"comment": "memcpy.c, memory.c and memcmp.c are from the
> outside"
> +|}
> +|]
> +|}
> +
> +Here is an explanation of the fields inside an object of the "content" array:
> + - rel_path: it is the relative path from the Xen folder to the file/folder
> that
> + needs to be excluded from the analysis report, it can contain a wildcard
> to
> + match more than one file/folder at the time. This field is mandatory.
> + - comment: an optional comment to explain why the file is removed from the
> + analysis.
> +
> +To ease the review and the modifications of the entries, they shall be
> listed in
> +alphabetical order referring to the rel_path field.
> diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py
> b/xen/scripts/xen_analysis/cppcheck_analysis.py
> index cc1f403d315e..e385e2c8f54a 100644
> --- a/xen/scripts/xen_analysis/cppcheck_analysis.py
> +++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
> @@ -1,7 +1,8 @@
> #!/usr/bin/env python3
>
> import os, re, shutil
> -from . import settings, utils, cppcheck_report_utils
> +from . import settings, utils, cppcheck_report_utils, exclusion_file_list
> +from .exclusion_file_list import ExclusionFileListError
>
> class GetMakeVarsPhaseError(Exception):
> pass
> @@ -50,6 +51,21 @@ def __generate_suppression_list(out_file):
> # header for cppcheck
> supplist_file.write("*:*generated/compiler-def.h\n")
>
> +try:
> +exclusion_file = \
> +
> "{}/docs/misra/exclude-list.json".format(settings
[xen-unstable test] 178875: trouble: fail/pass/starved
flight 178875 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/178875/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-livepatchbroken in 178827 test-amd64-i386-libvirt-raw broken in 178827 Tests which are failing intermittently (not blocking): test-amd64-i386-libvirt-raw 5 host-install(5) broken in 178827 pass in 178875 test-amd64-i386-livepatch5 host-install(5) broken in 178827 pass in 178875 test-amd64-i386-freebsd10-i386 7 xen-install fail pass in 178827 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 178771 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 178771 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 178771 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 178771 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 178771 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 178771 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 178771 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 178771 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 178771 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-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-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 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-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen b84fdf521b306cce64388fe57ee6c7c00f9d3e76 baseline version: xen bfc3780f23ded229f42a2565783e21c32083bbfd Last test of basis 178771 2023-02-28 13:11:03 Z1 days Testing same since 178827 2023-03-01 01:55:55 Z0 days2 attempts People who touched revisions under test: Andrew Cooper Anthony PERARD Demi Marie Obenour Jan Beulich Sergey Dyasli Xenia Ragiadakou jobs: build-amd64-xsm
[linux-linus test] 178854: regressions - trouble: fail/pass/starved
flight 178854 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/178854/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-freebsd12-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-qemuu-nested-intel 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-ws16-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-xl-pvshim8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-vhd 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-intel 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-win7-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-win7-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-credit1 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-shadow8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-amd 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-qemuu-nested-amd 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-ws16-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-freebsd11-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-raw 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-pygrub 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-pair12 xen-boot/src_hostfail REGR. vs. 178042 test-amd64-amd64-pair13 xen-boot/dst_hostfail REGR. vs. 178042 test-amd64-amd64-examine-uefi 8 reboot fail REGR. vs. 178042 test-amd64-amd64-libvirt-qcow2 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-pair 12 xen-boot/src_host fail REGR. vs. 178042 test-amd64-amd64-libvirt-pair 13 xen-boot/dst_host fail REGR. vs. 178042 test-amd64-coresched-amd64-xl 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-amd64 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 178042 test-arm64-arm64-xl 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-xl-credit2 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt 8 xen-boot fail REGR. vs. 178042 test-arm64-arm64-xl-credit1 18 guest-start/debian.repeat fail REGR. vs. 178042 test-arm64-arm64-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-ovmf-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-examine 8 reboot fail REGR. vs. 178042 test-amd64-amd64-examine-bios 8 reboot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-multivcpu 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl-vhd 12 debian-di-installfail REGR. vs. 178042 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 178042 test-arm64-arm64-libvirt-xsm 17 guest-stop fail in 178822 REGR. vs. 178042 test-arm64-arm64-xl-thunderx 18 guest-start/debian.repeat fail in 178822 REGR. vs. 178042 test-arm64-arm64-xl-xsm 18 guest-start/debian.repeat fail in 178822 REGR. vs. 178042 Tests which are failing intermittently (not blocking): test-arm64-arm64-xl-credit1 14 guest-start fail in 178822 pass in 178854 test-arm64-arm64-xl-xsm 14 guest-startfail pass in 178822 test-arm64-arm64-libvirt-xsm 14 guest-startfail pass in 178822 test-arm64-arm64-xl-thunderx 14 guest-startfail pass in 178822 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 8 xen-boot fail REGR. vs. 178042 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-thunderx 15 migrate-support-check fail in 178822 never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-check fail in 178822 never pass test-arm64-arm64
Re: [PATCH v2] x86/SVM: restrict hardware SSBD update upon guest VIRT_SPEC_CTRL write
On 15/12/2022 8:20 am, Jan Beulich wrote:
> core_set_legacy_ssbd() counts the number of times SSBD is being enabled
> via LS_CFG on a core. This assumes that calls there only occur if the
> state actually changes. While svm_ctxt_switch_{to,from}() conform to
> this, guest_wrmsr() doesn't: It also calls the function when the bit
> doesn't actually change. Make core_set_legacy_ssbd() track per-thread
> enabled state by converting the "count" field to a bitmap, thus allowing
> to skip redundant enable/disable requests, constraining
> amd_setup_legacy_ssbd() accordingly.
>
> Fixes: b2030e6730a2 ("amd/virt_ssbd: set SSBD at vCPU context switch")
> Reported-by: Andrew Cooper
> Signed-off-by: Jan Beulich
> ---
> This wants properly testing on affected hardware. From Andrew's
> description it's also not clear whether this really is addressing that
> problem, or yet another one in this same area.
> ---
> v2: Change core_set_legacy_ssbd() itself rather than the problematic
> caller.
I think this patch will fix one of the errors. I've lost count of how
many others issues I've found now that I've looked at the code in detail
for the first time.
However...
> --- a/xen/arch/x86/cpu/amd.c
> +++ b/xen/arch/x86/cpu/amd.c
> @@ -744,7 +744,7 @@ void amd_init_ssbd(const struct cpuinfo_
>
> static struct ssbd_ls_cfg {
> spinlock_t lock;
> -unsigned int count;
> +unsigned long enabled;
> } __cacheline_aligned *ssbd_ls_cfg;
> static unsigned int __ro_after_init ssbd_max_cores;
> #define AMD_FAM17H_MAX_SOCKETS 2
> @@ -757,6 +757,11 @@ bool __init amd_setup_legacy_ssbd(void)
> boot_cpu_data.x86_num_siblings <= 1 || opt_ssbd)
> return true;
>
> + if (boot_cpu_data.x86_num_siblings > BITS_PER_LONG ||
> + (boot_cpu_data.x86_num_siblings &
> + (boot_cpu_data.x86_num_siblings - 1)))
> + return false;
... this is nonsense. There is no such thing as an Zen1 uarch with more
than 2 threads per core, and attempts cope with it (as opposed to
rejecting it outright) makes ...
> +
> /*
>* One could be forgiven for thinking that c->x86_max_cores is the
>* correct value to use here.
> @@ -800,10 +805,12 @@ static void core_set_legacy_ssbd(bool en
> c->cpu_core_id];
>
> spin_lock_irqsave(&status->lock, flags);
> - status->count += enable ? 1 : -1;
> - ASSERT(status->count <= c->x86_num_siblings);
> - if (enable ? status->count == 1 : !status->count)
> + if (!enable)
> + __clear_bit(c->apicid & (c->x86_num_siblings - 1),
> &status->enabled);
> + if (!status->enabled)
> BUG_ON(!set_legacy_ssbd(c, enable));
> + if (enable)
> + __set_bit(c->apicid & (c->x86_num_siblings - 1),
> &status->enabled);
... this mess even worse.
I am rewriting the legacy SSBD code so that it is fit for purpose.
~Andrew
Re: [PATCH v6 4/4] hw: replace most qemu_bh_new calls with qemu_bh_new_guarded
On Sat, Feb 04, 2023 at 11:07:37PM -0500, Alexander Bulekov wrote:
> This protects devices from bh->mmio reentrancy issues.
>
> Reviewed-by: Darren Kenny
> Reviewed-by: Stefan Hajnoczi
> Signed-off-by: Alexander Bulekov
Reviewed-by: Michael S. Tsirkin
> ---
> hw/9pfs/xen-9p-backend.c| 4 +++-
> hw/block/dataplane/virtio-blk.c | 3 ++-
> hw/block/dataplane/xen-block.c | 5 +++--
> hw/char/virtio-serial-bus.c | 3 ++-
> hw/display/qxl.c| 9 ++---
> hw/display/virtio-gpu.c | 6 --
> hw/ide/ahci.c | 3 ++-
> hw/ide/core.c | 3 ++-
> hw/misc/imx_rngc.c | 6 --
> hw/misc/macio/mac_dbdma.c | 2 +-
> hw/net/virtio-net.c | 3 ++-
> hw/nvme/ctrl.c | 6 --
> hw/scsi/mptsas.c| 3 ++-
> hw/scsi/scsi-bus.c | 3 ++-
> hw/scsi/vmw_pvscsi.c| 3 ++-
> hw/usb/dev-uas.c| 3 ++-
> hw/usb/hcd-dwc2.c | 3 ++-
> hw/usb/hcd-ehci.c | 3 ++-
> hw/usb/hcd-uhci.c | 2 +-
> hw/usb/host-libusb.c| 6 --
> hw/usb/redirect.c | 6 --
> hw/usb/xen-usb.c| 3 ++-
> hw/virtio/virtio-balloon.c | 5 +++--
> hw/virtio/virtio-crypto.c | 3 ++-
> 24 files changed, 63 insertions(+), 33 deletions(-)
>
> diff --git a/hw/9pfs/xen-9p-backend.c b/hw/9pfs/xen-9p-backend.c
> index 65c4979c3c..f077c1b255 100644
> --- a/hw/9pfs/xen-9p-backend.c
> +++ b/hw/9pfs/xen-9p-backend.c
> @@ -441,7 +441,9 @@ static int xen_9pfs_connect(struct XenLegacyDevice
> *xendev)
> xen_9pdev->rings[i].ring.out = xen_9pdev->rings[i].data +
> XEN_FLEX_RING_SIZE(ring_order);
>
> -xen_9pdev->rings[i].bh = qemu_bh_new(xen_9pfs_bh,
> &xen_9pdev->rings[i]);
> +xen_9pdev->rings[i].bh = qemu_bh_new_guarded(xen_9pfs_bh,
> + &xen_9pdev->rings[i],
> +
> &DEVICE(xen_9pdev)->mem_reentrancy_guard);
> xen_9pdev->rings[i].out_cons = 0;
> xen_9pdev->rings[i].out_size = 0;
> xen_9pdev->rings[i].inprogress = false;
> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
> index b28d81737e..a6202997ee 100644
> --- a/hw/block/dataplane/virtio-blk.c
> +++ b/hw/block/dataplane/virtio-blk.c
> @@ -127,7 +127,8 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev,
> VirtIOBlkConf *conf,
> } else {
> s->ctx = qemu_get_aio_context();
> }
> -s->bh = aio_bh_new(s->ctx, notify_guest_bh, s);
> +s->bh = aio_bh_new_guarded(s->ctx, notify_guest_bh, s,
> + &DEVICE(vdev)->mem_reentrancy_guard);
> s->batch_notify_vqs = bitmap_new(conf->num_queues);
>
> *dataplane = s;
> diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c
> index 2785b9e849..e31806b317 100644
> --- a/hw/block/dataplane/xen-block.c
> +++ b/hw/block/dataplane/xen-block.c
> @@ -632,8 +632,9 @@ XenBlockDataPlane *xen_block_dataplane_create(XenDevice
> *xendev,
> } else {
> dataplane->ctx = qemu_get_aio_context();
> }
> -dataplane->bh = aio_bh_new(dataplane->ctx, xen_block_dataplane_bh,
> - dataplane);
> +dataplane->bh = aio_bh_new_guarded(dataplane->ctx,
> xen_block_dataplane_bh,
> + dataplane,
> +
> &DEVICE(xendev)->mem_reentrancy_guard);
>
> return dataplane;
> }
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 7d4601cb5d..dd619f0731 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -985,7 +985,8 @@ static void virtser_port_device_realize(DeviceState *dev,
> Error **errp)
> return;
> }
>
> -port->bh = qemu_bh_new(flush_queued_data_bh, port);
> +port->bh = qemu_bh_new_guarded(flush_queued_data_bh, port,
> + &dev->mem_reentrancy_guard);
> port->elem = NULL;
> }
>
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index ec712d3ca2..c0460c4ef1 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -2201,11 +2201,14 @@ static void qxl_realize_common(PCIQXLDevice *qxl,
> Error **errp)
>
> qemu_add_vm_change_state_handler(qxl_vm_change_state_handler, qxl);
>
> -qxl->update_irq = qemu_bh_new(qxl_update_irq_bh, qxl);
> +qxl->update_irq = qemu_bh_new_guarded(qxl_update_irq_bh, qxl,
> +
> &DEVICE(qxl)->mem_reentrancy_guard);
> qxl_reset_state(qxl);
>
> -qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl);
> -qxl->ssd.cursor_bh = qemu_bh_new(qemu_spice_cursor_refresh_bh,
> &qxl->ssd);
> +qxl->update_area_bh = qemu_bh_new_guarded(qxl_render_update_area_bh, qxl,
> +
Re: [PATCH v1] xen/arm: align *(.proc.info) in the linker script
Hi, On 01/03/2023 17:50, Julien Grall wrote: On 01/03/2023 16:38, Oleksii wrote: Hi Julien, Hi, On Wed, 2023-03-01 at 16:21 +, Julien Grall wrote: Hi Oleksii, On 01/03/2023 16:14, Oleksii Kurochko wrote: During testing of bug.h's macros generic implementation yocto- qemuarm job crashed with data abort: The commit message is lacking some information. You are telling us that there is an error when building with your series, but this doesn't tell me why this is the correct fix. This is also why I asked to have the xen binary because I want to check whether this was a latent bug in Xen or your series effectively introduce a bug. Note that regardless what I just wrote this is a good idea to align __proc_info_start. I will try to have a closer look later and propose a commit message and/or any action for your other series. Regarding binaries please take a look here: https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.ca...@gmail.com/ I am not sure if you get my answer as I had the message from delivery server that it was blocked for some reason. I got the answer. The problem now is gitlab only keep the artifact for the latest build and it only provide a zImage (having the ELF would be easier). I will try to reproduce the error on my end. I managed to reproduce it. It looks like that after your bug patch, *(.rodata.*) will not be end on a 4-byte boundary. Before your patch, all the messages will be in .rodata.str. Now they are in .bug_frames.*, so there some difference in .rodata.*. That said, it is not entirely clear why we never seen the issue before because my guessing there are no guarantee that .rodata.* will be suitably aligned. Anyway, here a proposal for the commit message: " xen/arm: Ensure the start *(.proc.info) of is 4-byte aligned The entries in *(.proc.info) are expected to be 4-byte aligned and the compiler will access them using 4-byte load instructions. On Arm32, the alignment is strictly enforced by the processor and will result to a data abort if it is not correct. However, the linker script doesn't encode this requirement. So we are at the mercy of the compiler/linker to have aligned the previous sections suitably. This was spotted when trying to use the upcoming generic bug infrastructure with the compiler provided by Yocto. Link: https://lore.kernel.org/xen-devel/6735859208c6dcb7320f89664ae298005f70827b.ca...@gmail.com/ " If you are happy with the proposed commit message, then I can update it while committing. Cheers, -- Julien Grall
Re: [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode
On Wed, Mar 1, 2023 at 11:31 AM Jan Beulich wrote: > > On 28.02.2023 18:39, Sergey Dyasli wrote: > > Add an option to xen-ucode tool to print the currently loaded ucode > > version and also print it during usage info. Print CPU signature and > > processor flags as well. The raw data comes from cpuinfo directory in > > xenhypfs and from XENPF_get_cpu_version platform op. > > While I don't mind the use of the platform-op, I'm little puzzled by the > mix. If CPU information is to be exposed in hypfs, can't we expose there > everything that's needed here? > > Then again, perhaps in a different context, Andrew pointed out that hypfs > is an optional component, so relying on its presence in the underlying > hypervisor will need weighing against the alternative of adding a new > platform-op for the ucode-related data (as you had it in v1). Since I'm > unaware of a request to switch, are there specific reasons you did? Ideal situation would be microcode information in Dom0's /proc/cpuinfo updated after late load, since that file already has most of the information about the cpu. And the closest thing to /proc is xenhypfs. It allows the user to query information directly, e.g. # xenhypfs cat /cpuinfo/microcode-revision 33554509 Which could be used manually or in scripts, instead of relying on xen-ucode utility. Though printing the value in hex would be nicer. That was my motivation to go hypfs route. In general it feels like cpu information is a good fit for hypfs, but agreement on its format and exposed values is needed. I can always switch back to a platform op if that would be the preference. > > --- a/tools/misc/xen-ucode.c > > +++ b/tools/misc/xen-ucode.c > > @@ -11,6 +11,96 @@ > > #include > > #include > > #include > > +#include > > + > > +static const char intel_id[] = "GenuineIntel"; > > +static const char amd_id[] = "AuthenticAMD"; > > + > > +static const char sig_path[] = "/cpuinfo/cpu-signature"; > > +static const char rev_path[] = "/cpuinfo/microcode-revision"; > > +static const char pf_path[] = "/cpuinfo/processor-flags"; > > Together with the use below I conclude (without having looked at patch 1 > yet) that you only expose perhaps the BSP's data, rather than such for > all CPUs. (And I was actually going to put up the question whether data > like the one presented here might not also be of interest for parked > CPUs.) Yes, that comes from the BSP. Xen must make sure that all CPUs have the same ucode revision for the system to work correctly. Sergey
[ovmf test] 178885: all pass - PUSHED
flight 178885 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/178885/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf f80f052277c88a67c55e107b550f504eeea947d3 baseline version: ovmf 1eeca0750af5af2f0e78437bf791ac2de74bde74 Last test of basis 178254 2023-02-23 15:11:10 Z6 days Testing same since 178885 2023-03-01 15:12:19 Z0 days1 attempts People who touched revisions under test: Andrei Warkentin Sunil V L jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 1eeca0750a..f80f052277 f80f052277c88a67c55e107b550f504eeea947d3 -> xen-tested-master
Re: [PATCH v1] xen/arm: align *(.proc.info) in the linker script
On 01/03/2023 16:38, Oleksii wrote: Hi Julien, Hi, On Wed, 2023-03-01 at 16:21 +, Julien Grall wrote: Hi Oleksii, On 01/03/2023 16:14, Oleksii Kurochko wrote: During testing of bug.h's macros generic implementation yocto- qemuarm job crashed with data abort: The commit message is lacking some information. You are telling us that there is an error when building with your series, but this doesn't tell me why this is the correct fix. This is also why I asked to have the xen binary because I want to check whether this was a latent bug in Xen or your series effectively introduce a bug. Note that regardless what I just wrote this is a good idea to align __proc_info_start. I will try to have a closer look later and propose a commit message and/or any action for your other series. Regarding binaries please take a look here: https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.ca...@gmail.com/ I am not sure if you get my answer as I had the message from delivery server that it was blocked for some reason. I got the answer. The problem now is gitlab only keep the artifact for the latest build and it only provide a zImage (having the ELF would be easier). I will try to reproduce the error on my end. Cheers, -- Julien Grall
Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
On Wed, Mar 01, 2023 at 04:31:41PM +, Alex Bennée wrote: > > Stefan Hajnoczi writes: > > > [[PGP Signed Part:Undecided]] > > Resend - for some reason my email didn't make it out. > > > > From: Stefan Hajnoczi > > Subject: Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory > > mapping support > > To: Viresh Kumar > > Cc: qemu-de...@nongnu.org, virtio-...@lists.oasis-open.org, "Michael S. > > Tsirkin" , Vincent Guittot , > > Alex Bennée , > > stratos-...@op-lists.linaro.org, Oleksandr Tyshchenko > > , xen-de...@lists.xen.org, Andrew Cooper > > , Juergen Gross , Sebastien > > Boeuf > > , Liu Jiang , > > Mathieu > > Poirier > > Date: Tue, 21 Feb 2023 10:17:01 -0500 (1 week, 1 day, 1 hour ago) > > Flags: seen, signed, personal > > > > On Tue, Feb 21, 2023 at 03:20:41PM +0530, Viresh Kumar wrote: > >> The current model of memory mapping at the back-end works fine with > >> Qemu, where a standard call to mmap() for the respective file > >> descriptor, passed from front-end, is generally all we need to do before > >> the front-end can start accessing the guest memory. > >> > >> There are other complex cases though, where we need more information at > >> the backend and need to do more than just an mmap() call. For example, > >> Xen, a type-1 hypervisor, currently supports memory mapping via two > >> different methods, foreign-mapping (via /dev/privcmd) and grant-dev (via > >> /dev/gntdev). In both these cases, the back-end needs to call mmap() > >> followed by an ioctl() (or vice-versa), and need to pass extra > >> information via the ioctl(), like the Xen domain-id of the guest whose > >> memory we are trying to map. > >> > >> Add a new protocol feature, 'VHOST_USER_PROTOCOL_F_CUSTOM_MMAP', which > >> lets the back-end know about the additional memory mapping requirements. > >> When this feature is negotiated, the front-end can send the > >> 'VHOST_USER_CUSTOM_MMAP' message type to provide the additional > >> information to the back-end. > >> > >> Signed-off-by: Viresh Kumar > >> --- > >> docs/interop/vhost-user.rst | 32 > >> 1 file changed, 32 insertions(+) > > > > The alternative to an in-band approach is to configure these details > > out-of-band. For example, via command-line options to the vhost-user > > back-end: > > > > $ my-xen-device --mapping-type=foreign-mapping --domain-id=123 > > > > I was thinking about both approaches and don't see an obvious reason to > > choose one or the other. What do you think? > > In-band has the nice property of being dynamic and not having to have > some other thing construct command lines. We are also trying to keep the > daemons from being Xen specific and keep the type of mmap as an > implementation detail that is mostly elided by the rust-vmm memory > traits. Okay. > > > >> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst > >> index 3f18ab424eb0..f2b1d705593a 100644 > >> --- a/docs/interop/vhost-user.rst > >> +++ b/docs/interop/vhost-user.rst > >> @@ -258,6 +258,23 @@ Inflight description > >> > >> :queue size: a 16-bit size of virtqueues > >> > >> +Custom mmap description > >> +^^^ > >> + > >> ++---+---+ > >> +| flags | value | > >> ++---+---+ > >> + > >> +:flags: 64-bit bit field > >> + > >> +- Bit 0 is Xen foreign memory access flag - needs Xen foreign memory > >> mapping. > >> +- Bit 1 is Xen grant memory access flag - needs Xen grant memory mapping. > >> + > >> +:value: a 64-bit hypervisor specific value. > >> + > >> +- For Xen foreign or grant memory access, this is set with guest's xen > >> domain > >> + id. > > > > This is highly Xen-specific. How about naming the feature XEN_MMAP > > instead of CUSTOM_MMAP? If someone needs to add other mmap data later, > > they should define their own struct instead of trying to squeeze into > > the same fields as Xen. > > We hope to support additional mmap mechanisms in the future - one > proposal is to use the hypervisor specific interface to support an > ioctl() that creates a domain specific device which can then be treated > more generically. > > Could we not declare the message as flag + n bytes of domain specific > message as defined my msglen? What is the advantage over defining separate messages? Separate messages are cleaner and more typesafe. > > There is an assumption in this design that a single > > VHOST_USER_CUSTOM_MMAP message provides the information necessary for > > all mmaps. Are you sure the limitation that every mmap belongs to the > > same domain will be workable in the future? > > Like a daemon servicing multiple VMs? Won't those still be separate > vhost-user control channels though? I don't have a concrete example, but was thinking of a guest that shares memory with other guests (like the experimental virtio-vhost-user device). Maybe there would be a scenario where some memory belongs to one domain and some belongs to another (but has been mapped in
Re: [XEN PATCH v7 12/20] xen/arm: ffa: send guest events to Secure Partitions
Hi,
On Wed, Mar 1, 2023 at 1:58 PM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 1 Mar 2023, at 11:16, Jens Wiklander wrote:
> >
> > Hi Bertrand,
> >
> > On Tue, Feb 28, 2023 at 5:49 PM Bertrand Marquis
> > wrote:
> >>
> >> Hi Jens,
> >>
> >>> On 22 Feb 2023, at 16:33, Jens Wiklander
> >>> wrote:
> >>>
> >>> The FF-A specification defines framework messages sent as direct
> >>> requests when certain events occurs. For instance when a VM (guest) is
> >>> created or destroyed. Only SPs which have subscribed to these events
> >>> will receive them. An SP can subscribe to these messages in its
> >>> partition properties.
> >>>
> >>> Adds a check that the SP supports the needed FF-A features
> >>> FFA_PARTITION_INFO_GET and FFA_RX_RELEASE.
> >>>
> >>> The partition properties of each SP is retrieved with
> >>> FFA_PARTITION_INFO_GET which returns the information in our RX buffer.
> >>> Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
> >>> caller (us), so once we're done with the buffer it must be released
> >>> using FFA_RX_RELEASE before another call can be made.
> >>>
> >>> Signed-off-by: Jens Wiklander
> >>> ---
> >>> xen/arch/arm/tee/ffa.c | 191 -
> >>> 1 file changed, 190 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> >>> index 07dd5c36d54b..f1b014b6c7f4 100644
> >>> --- a/xen/arch/arm/tee/ffa.c
> >>> +++ b/xen/arch/arm/tee/ffa.c
> >>> @@ -140,6 +140,14 @@
> >>> #define FFA_MSG_SEND0x846EU
> >>> #define FFA_MSG_POLL0x846AU
> >>>
> >>> +/* Partition information descriptor */
> >>> +struct ffa_partition_info_1_1 {
> >>> +uint16_t id;
> >>> +uint16_t execution_context;
> >>> +uint32_t partition_properties;
> >>> +uint8_t uuid[16];
> >>> +};
> >>> +
> >>> struct ffa_ctx {
> >>>uint32_t guest_vers;
> >>>bool interrupted;
> >>> @@ -148,6 +156,12 @@ struct ffa_ctx {
> >>> /* Negotiated FF-A version to use with the SPMC */
> >>> static uint32_t ffa_version __ro_after_init;
> >>>
> >>> +/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
> >>> +static uint16_t *subscr_vm_created __read_mostly;
> >>> +static unsigned int subscr_vm_created_count __read_mostly;
> >>
> >> In the spec the number is returned in w2 so you should utse uint32_t here.
> >
> > I don't understand. This value is increased for each SP which has the
> > property set in the Partition information descriptor.
>
> Using generic types should be prevented when possible.
I'm usually of the opposite opinion, fixed width integers should only
be used when there's a good reason to do so. However, if this is the
Xen philosophy I can replace all those unsigned int with uint32_t if
that's preferable.
> Here this is a subset of the number of partition which is uint32_t (wX reg) so
> i think this would be the logical type for this.
The maximum number is actually UINT16_MAX so an observant reader might
wonder why the uint32_t type was used here.
>
> >
> >>
> >>> +static uint16_t *subscr_vm_destroyed __read_mostly;
> >>> +static unsigned int subscr_vm_destroyed_count __read_mostly;
> >>
> >> Same here
> >>
> >>> +
> >>> /*
> >>> * Our rx/tx buffers shared with the SPMC.
> >>> *
> >>> @@ -237,6 +251,72 @@ static int32_t ffa_rxtx_map(register_t tx_addr,
> >>> register_t rx_addr,
> >>>return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
> >>> }
> >>>
> >>> +static int32_t ffa_partition_info_get(uint32_t w1, uint32_t w2, uint32_t
> >>> w3,
> >>> + uint32_t w4, uint32_t w5,
> >>> + uint32_t *count)
> >>> +{
> >>> +const struct arm_smccc_1_2_regs arg = {
> >>> +.a0 = FFA_PARTITION_INFO_GET,
> >>> +.a1 = w1,
> >>> +.a2 = w2,
> >>> +.a3 = w3,
> >>> +.a4 = w4,
> >>> +.a5 = w5,
> >>> +};
> >>> +struct arm_smccc_1_2_regs resp;
> >>> +uint32_t ret;
> >>> +
> >>> +arm_smccc_1_2_smc(&arg, &resp);
> >>> +
> >>> +ret = get_ffa_ret_code(&resp);
> >>> +if ( !ret )
> >>> +*count = resp.a2;
> >>> +
> >>> +return ret;
> >>> +}
> >>> +
> >>> +static int32_t ffa_rx_release(void)
> >>> +{
> >>> +return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
> >>> +}
> >>> +
> >>> +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> >>> + uint8_t msg)
> >>
> >> This function is actually only useable to send framework created/destroyed
> >> messages so the function name should be adapted to be less generic.
> >>
> >> ffa_send_vm_events ?
> >>
> >> unless you want to modify it later to send more framework messages ?
> >
> > That was the plan, but that may never happen. I'll rename it to
> > ffa_send_vm_event() since we're only sending one event at a time.
> >
> >>
> >>> +{
> >>> +uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
> >>> +int32_t res;
> >>> +
> >
Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
Stefan Hajnoczi writes: > [[PGP Signed Part:Undecided]] > Resend - for some reason my email didn't make it out. > > From: Stefan Hajnoczi > Subject: Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory > mapping support > To: Viresh Kumar > Cc: qemu-de...@nongnu.org, virtio-...@lists.oasis-open.org, "Michael S. > Tsirkin" , Vincent Guittot , > Alex Bennée , > stratos-...@op-lists.linaro.org, Oleksandr Tyshchenko > , xen-de...@lists.xen.org, Andrew Cooper > , Juergen Gross , Sebastien > Boeuf > , Liu Jiang , > Mathieu > Poirier > Date: Tue, 21 Feb 2023 10:17:01 -0500 (1 week, 1 day, 1 hour ago) > Flags: seen, signed, personal > > On Tue, Feb 21, 2023 at 03:20:41PM +0530, Viresh Kumar wrote: >> The current model of memory mapping at the back-end works fine with >> Qemu, where a standard call to mmap() for the respective file >> descriptor, passed from front-end, is generally all we need to do before >> the front-end can start accessing the guest memory. >> >> There are other complex cases though, where we need more information at >> the backend and need to do more than just an mmap() call. For example, >> Xen, a type-1 hypervisor, currently supports memory mapping via two >> different methods, foreign-mapping (via /dev/privcmd) and grant-dev (via >> /dev/gntdev). In both these cases, the back-end needs to call mmap() >> followed by an ioctl() (or vice-versa), and need to pass extra >> information via the ioctl(), like the Xen domain-id of the guest whose >> memory we are trying to map. >> >> Add a new protocol feature, 'VHOST_USER_PROTOCOL_F_CUSTOM_MMAP', which >> lets the back-end know about the additional memory mapping requirements. >> When this feature is negotiated, the front-end can send the >> 'VHOST_USER_CUSTOM_MMAP' message type to provide the additional >> information to the back-end. >> >> Signed-off-by: Viresh Kumar >> --- >> docs/interop/vhost-user.rst | 32 >> 1 file changed, 32 insertions(+) > > The alternative to an in-band approach is to configure these details > out-of-band. For example, via command-line options to the vhost-user > back-end: > > $ my-xen-device --mapping-type=foreign-mapping --domain-id=123 > > I was thinking about both approaches and don't see an obvious reason to > choose one or the other. What do you think? In-band has the nice property of being dynamic and not having to have some other thing construct command lines. We are also trying to keep the daemons from being Xen specific and keep the type of mmap as an implementation detail that is mostly elided by the rust-vmm memory traits. > >> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst >> index 3f18ab424eb0..f2b1d705593a 100644 >> --- a/docs/interop/vhost-user.rst >> +++ b/docs/interop/vhost-user.rst >> @@ -258,6 +258,23 @@ Inflight description >> >> :queue size: a 16-bit size of virtqueues >> >> +Custom mmap description >> +^^^ >> + >> ++---+---+ >> +| flags | value | >> ++---+---+ >> + >> +:flags: 64-bit bit field >> + >> +- Bit 0 is Xen foreign memory access flag - needs Xen foreign memory >> mapping. >> +- Bit 1 is Xen grant memory access flag - needs Xen grant memory mapping. >> + >> +:value: a 64-bit hypervisor specific value. >> + >> +- For Xen foreign or grant memory access, this is set with guest's xen >> domain >> + id. > > This is highly Xen-specific. How about naming the feature XEN_MMAP > instead of CUSTOM_MMAP? If someone needs to add other mmap data later, > they should define their own struct instead of trying to squeeze into > the same fields as Xen. We hope to support additional mmap mechanisms in the future - one proposal is to use the hypervisor specific interface to support an ioctl() that creates a domain specific device which can then be treated more generically. Could we not declare the message as flag + n bytes of domain specific message as defined my msglen? > There is an assumption in this design that a single > VHOST_USER_CUSTOM_MMAP message provides the information necessary for > all mmaps. Are you sure the limitation that every mmap belongs to the > same domain will be workable in the future? Like a daemon servicing multiple VMs? Won't those still be separate vhost-user control channels though? > >> + >> C structure >> --- >> >> @@ -867,6 +884,7 @@ Protocol features >>#define VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS 14 >>#define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 >>#define VHOST_USER_PROTOCOL_F_STATUS 16 >> + #define VHOST_USER_PROTOCOL_F_CUSTOM_MMAP 17 >> >> Front-end message types >> --- >> @@ -1422,6 +1440,20 @@ Front-end message types >>query the back-end for its device status as defined in the Virtio >>specification. >> >> +``VHOST_USER_CUSTOM_MMAP`` > > Most vhost-user protocol messages have a verb like > get/set/close/add/listen/etc. I sugge
Re: [PATCH v1] xen/arm: align *(.proc.info) in the linker script
Hi Julien, On Wed, 2023-03-01 at 16:21 +, Julien Grall wrote: > Hi Oleksii, > > On 01/03/2023 16:14, Oleksii Kurochko wrote: > > During testing of bug.h's macros generic implementation yocto- > > qemuarm > > job crashed with data abort: > > The commit message is lacking some information. You are telling us > that > there is an error when building with your series, but this doesn't > tell > me why this is the correct fix. > > This is also why I asked to have the xen binary because I want to > check > whether this was a latent bug in Xen or your series effectively > introduce a bug. > > Note that regardless what I just wrote this is a good idea to align > __proc_info_start. I will try to have a closer look later and propose > a > commit message and/or any action for your other series. Regarding binaries please take a look here: https://lore.kernel.org/xen-devel/aa2862eacccfb0574859bf4cda8f4992baa5d2e1.ca...@gmail.com/ I am not sure if you get my answer as I had the message from delivery server that it was blocked for some reason. ~ Oleksii
Re: [PATCH v1] xen/arm: align *(.proc.info) in the linker script
Hi Oleksii, On 01/03/2023 16:14, Oleksii Kurochko wrote: During testing of bug.h's macros generic implementation yocto-qemuarm job crashed with data abort: The commit message is lacking some information. You are telling us that there is an error when building with your series, but this doesn't tell me why this is the correct fix. This is also why I asked to have the xen binary because I want to check whether this was a latent bug in Xen or your series effectively introduce a bug. Note that regardless what I just wrote this is a good idea to align __proc_info_start. I will try to have a closer look later and propose a commit message and/or any action for your other series. Cheers, -- Julien Grall
Re: [XEN PATCH v7 10/20] xen/arm: ffa: add direct request support
Hi,
On Wed, Mar 1, 2023 at 2:06 PM Bertrand Marquis
wrote:
>
> HI Jens,
>
> > On 1 Mar 2023, at 11:55, Jens Wiklander wrote:
> >
> > Hi Bertrand,
> >
> > On Mon, Feb 27, 2023 at 4:28 PM Bertrand Marquis
> > wrote:
> >>
> >> Hi Jens,
> >>
> >>> On 22 Feb 2023, at 16:33, Jens Wiklander
> >>> wrote:
> >>>
> >>> Adds support for sending a FF-A direct request. Checks that the SP also
> >>> supports handling a 32-bit direct request. 64-bit direct requests are
> >>> not used by the mediator itself so there is not need to check for that.
> >>>
> >>> Signed-off-by: Jens Wiklander
> >>> ---
> >>> xen/arch/arm/tee/ffa.c | 119 +
> >>> 1 file changed, 119 insertions(+)
> >>>
> >>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> >>> index 463fd7730573..a5d8a12635b6 100644
> >>> --- a/xen/arch/arm/tee/ffa.c
> >>> +++ b/xen/arch/arm/tee/ffa.c
> >>> @@ -142,6 +142,7 @@
> >>>
> >>> struct ffa_ctx {
> >>>uint32_t guest_vers;
> >>> +bool interrupted;
> >>
> >> This is added and set here for one special error code but is never used.
> >> I would suggest to introduce this when there will be an action based on it.
> >
> > I'm sorry, I forgot about completing this. I'll add code to deal with
> > FFA_INTERRUPT. This will be tricky to test though since we don't use
> > FFA_INTERRUPT like this with OP-TEE. The Hypervisor is required by the
> > FF-A standard to support it so I better add something.
>
> You can do that in a different patch then and just remove this from this
> patch ?
OK, I'll do that.
>
> >
> >>
> >>> };
> >>>
> >>> /* Negotiated FF-A version to use with the SPMC */
> >>> @@ -167,6 +168,55 @@ static bool ffa_get_version(uint32_t *vers)
> >>>return true;
> >>> }
> >>>
> >>> +static int32_t get_ffa_ret_code(const struct arm_smccc_1_2_regs *resp)
> >>> +{
> >>> +switch ( resp->a0 )
> >>> +{
> >>> +case FFA_ERROR:
> >>> +if ( resp->a2 )
> >>> +return resp->a2;
> >>> +else
> >>> +return FFA_RET_NOT_SUPPORTED;
> >>> +case FFA_SUCCESS_32:
> >>> +case FFA_SUCCESS_64:
> >>> +return FFA_RET_OK;
> >>> +default:
> >>> +return FFA_RET_NOT_SUPPORTED;
> >>> +}
> >>> +}
> >>> +
> >>> +static int32_t ffa_simple_call(uint32_t fid, register_t a1, register_t
> >>> a2,
> >>> + register_t a3, register_t a4)
> >>> +{
> >>> +const struct arm_smccc_1_2_regs arg = {
> >>> +.a0 = fid,
> >>> +.a1 = a1,
> >>> +.a2 = a2,
> >>> +.a3 = a3,
> >>> +.a4 = a4,
> >>> +};
> >>> +struct arm_smccc_1_2_regs resp;
> >>> +
> >>> +arm_smccc_1_2_smc(&arg, &resp);
> >>> +
> >>> +return get_ffa_ret_code(&resp);
> >>> +}
> >>> +
> >>> +static int32_t ffa_features(uint32_t id)
> >>> +{
> >>> +return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
> >>> +}
> >>> +
> >>> +static bool check_mandatory_feature(uint32_t id)
> >>> +{
> >>> +uint32_t ret = ffa_features(id);
> >>> +
> >>> +if (ret)
> >>> +printk(XENLOG_ERR "ffa: mandatory feature id %#x missing\n", id);
> >>
> >> It might be useful here to actually print the error code.
> >> Are we sure that all errors actually mean not supported ?
> >
> > Yes, that's what the standard says.
>
> The error code might still be useful in the print.
OK, I'll add it.
>
> >
> >>
> >>> +
> >>> +return !ret;
> >>> +}
> >>> +
> >>> static uint16_t get_vm_id(const struct domain *d)
> >>> {
> >>>/* +1 since 0 is reserved for the hypervisor in FF-A */
> >>> @@ -208,6 +258,66 @@ static void handle_version(struct cpu_user_regs
> >>> *regs)
> >>>set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
> >>> }
> >>>
> >>> +static void handle_msg_send_direct_req(struct cpu_user_regs *regs,
> >>> uint32_t fid)
> >>> +{
> >>> +struct arm_smccc_1_2_regs arg = { .a0 = fid, };
> >>> +struct arm_smccc_1_2_regs resp = { };
> >>> +struct domain *d = current->domain;
> >>> +struct ffa_ctx *ctx = d->arch.tee;
> >>> +uint32_t src_dst;
> >>> +uint64_t mask;
> >>> +
> >>> +if ( smccc_is_conv_64(fid) )
> >>> +mask = GENMASK_ULL(63, 0);
> >>> +else
> >>> +mask = GENMASK_ULL(31, 0);
> >>> +
> >>> +src_dst = get_user_reg(regs, 1);
> >>> +if ( (src_dst >> 16) != get_vm_id(d) )
> >>> +{
> >>> +resp.a0 = FFA_ERROR;
> >>> +resp.a2 = FFA_RET_INVALID_PARAMETERS;
> >>> +goto out;
> >>> +}
> >>> +
> >>> +arg.a1 = src_dst;
> >>> +arg.a2 = get_user_reg(regs, 2) & mask;
> >>> +arg.a3 = get_user_reg(regs, 3) & mask;
> >>> +arg.a4 = get_user_reg(regs, 4) & mask;
> >>> +arg.a5 = get_user_reg(regs, 5) & mask;
> >>> +arg.a6 = get_user_reg(regs, 6) & mask;
> >>> +arg.a7 = get_user_reg(regs, 7) & mask;
> >>> +
> >>> +while ( true )
> >>> +{
> >>> +arm_smccc_1_2_smc(&arg, &resp);
> >>> +
> >>> +switch ( resp.a0 )
> >>> +{
> >>> +case FFA_INTERRUPT:
> >>> +
[PATCH v1] xen/arm: align *(.proc.info) in the linker script
During testing of bug.h's macros generic implementation yocto-qemuarm job crashed with data abort: (XEN) Data Abort Trap. Syndrome=0x1830021 (XEN) Walking Hypervisor VA 0x2a5ca6 on CPU0 via TTBR 0x4014a000 (XEN) 1ST[0x000] = 0x40149f7f (XEN) 2ND[0x001] = 0x004040148f7f (XEN) 3RD[0x0a5] = 0x0040400b5fff (XEN) CPU0: Unexpected Trap: Data Abort (XEN) [ Xen-4.18-unstable arm32 debug=y Not tainted ] (XEN) CPU:0 (XEN) PC: 0020063c head.o#__lookup_processor_type+0x1c/0x44 (XEN) CPSR: 61da MODE:Hypervisor (XEN) R0: 412fc0f1 R1: 002a5ca2 R2: 002a5cd2 R3: 61da (XEN) R4: 002a7e9c R5: 0011 R6: R7: (XEN) R8: 48008f20 R9: R10: R11:002ffecc R12: (XEN) HYP: SP: 002ffeb8 LR: 00200618 (XEN) (XEN) VTCR_EL2: (XEN) VTTBR_EL2: (XEN) (XEN) SCTLR_EL2: 30cd187f (XEN)HCR_EL2: 0038 (XEN) TTBR0_EL2: 4014a000 (XEN) (XEN)ESR_EL2: 97830021 (XEN) HPFAR_EL2: (XEN) HDFAR: 002a5ca6 (XEN) HIFAR: (XEN) (XEN) Xen stack trace from sp=002ffeb8: (XEN)97830021 002a7e9c 00276a88 002fff54 002c8fc4 2131 10011142 (XEN) 002a5500 8f20 2000 4800 002e01f0 (XEN) 6000 4000 0001 002e0208 002a7e8c 002a7e88 (XEN)002b0ab4 002e31f0 6000 0003 002aa000 (XEN)00200060 4800 4001 3fe1 00200068 (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) Xen call trace: (XEN)[<0020063c>] head.o#__lookup_processor_type+0x1c/0x44 (PC) (XEN)[<00200618>] lookup_processor_type+0xc/0x14 (LR) (XEN)[<002c8fc4>] start_xen+0xb8c/0x1138 (XEN)[<00200068>] head.o#primary_switched+0x8/0x1c (XEN) (XEN) (XEN) (XEN) Panic on CPU 0: (XEN) CPU0: Unexpected Trap: Data Abort (XEN) (XEN) (XEN) Reboot in five seconds... (XEN) Xen: Platform reset did not work properly! (XEN) Xen: Platform reset did not work properly! Signed-off-by: Oleksii Kurochko --- xen/arch/arm/xen.lds.S | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 3f7ebd19f3..1b392345bc 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -67,6 +67,7 @@ SECTIONS *(.data.rel.ro) *(.data.rel.ro.*) + . = ALIGN(4); __proc_info_start = .; *(.proc.info) __proc_info_end = .; -- 2.39.0
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
On Wed, 2023-03-01 at 15:21 +, Julien Grall wrote: > Hi Oleksii, > > On 01/03/2023 15:16, Oleksii wrote: > > On Wed, 2023-03-01 at 13:58 +, Julien Grall wrote: > > > On 01/03/2023 12:31, Oleksii wrote: > > > Given this is an alignment issue (Arm32 is more sensible to this > > > than > > > the other architecture, but this is still a potential problem for > > > the > > > other archs), I would really like to understand whether this is > > > an > > > issue > > > in the common code or in the Arm linker script. > > I have a good news. > > > > Alignment of "*(.proc.info)" helps but I checked only yocto- > > qemuarm: > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792923264 > > > > I ran all tests here: > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792953524 > > > > Should I create a separate patch with ALIGN if the tests are > > passed? > > Yes please. But, to be honest, I am not entirely sure what is not > aligned before hand. Do you know if it is possible to download the > Xen > binary from gitlab? It is possible. Please go to the link of the job: https://gitlab.com/xen-project/people/olkur/xen/-/jobs/3856617252 And on the right you will find 'Job artificats' where you can click 'Download'. Or in case if you need a particular binary can click 'Browse' and go to Artifcats/Binaries/: https://gitlab.com/xen-project/people/olkur/xen/-/jobs/3856617252/artifacts/browse/binaries/ ~ Oleksii
Re: [XEN PATCH v7 10/20] xen/arm: ffa: add direct request support
On Wed, Mar 1, 2023 at 4:50 PM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 1 Mar 2023, at 11:55, Jens Wiklander wrote:
> >
> > Hi Bertrand,
> >
> > On Mon, Feb 27, 2023 at 4:28 PM Bertrand Marquis
> > wrote:
> >>
> >> Hi Jens,
> >>
> >>> On 22 Feb 2023, at 16:33, Jens Wiklander
> >>> wrote:
> >>>
> >>> Adds support for sending a FF-A direct request. Checks that the SP also
> >>> supports handling a 32-bit direct request. 64-bit direct requests are
> >>> not used by the mediator itself so there is not need to check for that.
> >>>
> >>> Signed-off-by: Jens Wiklander
> >>> ---
> >>> xen/arch/arm/tee/ffa.c | 119 +
> >>> 1 file changed, 119 insertions(+)
> >>>
> >>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> >>> index 463fd7730573..a5d8a12635b6 100644
> >>> --- a/xen/arch/arm/tee/ffa.c
> >>> +++ b/xen/arch/arm/tee/ffa.c
> >>> @@ -142,6 +142,7 @@
> >>>
> >>> struct ffa_ctx {
> >>>uint32_t guest_vers;
> >>> +bool interrupted;
> >>
> >> This is added and set here for one special error code but is never used.
> >> I would suggest to introduce this when there will be an action based on it.
> >
> > I'm sorry, I forgot about completing this. I'll add code to deal with
> > FFA_INTERRUPT. This will be tricky to test though since we don't use
> > FFA_INTERRUPT like this with OP-TEE. The Hypervisor is required by the
> > FF-A standard to support it so I better add something.
> >
> >>
> >>> };
> >>>
> >>> /* Negotiated FF-A version to use with the SPMC */
> >>> @@ -167,6 +168,55 @@ static bool ffa_get_version(uint32_t *vers)
> >>>return true;
> >>> }
> >>>
> >>> +static int32_t get_ffa_ret_code(const struct arm_smccc_1_2_regs *resp)
> >>> +{
> >>> +switch ( resp->a0 )
> >>> +{
> >>> +case FFA_ERROR:
> >>> +if ( resp->a2 )
> >>> +return resp->a2;
> >>> +else
> >>> +return FFA_RET_NOT_SUPPORTED;
> >>> +case FFA_SUCCESS_32:
> >>> +case FFA_SUCCESS_64:
> >>> +return FFA_RET_OK;
> >>> +default:
> >>> +return FFA_RET_NOT_SUPPORTED;
> >>> +}
> >>> +}
> >>> +
> >>> +static int32_t ffa_simple_call(uint32_t fid, register_t a1, register_t
> >>> a2,
> >>> + register_t a3, register_t a4)
> >>> +{
> >>> +const struct arm_smccc_1_2_regs arg = {
> >>> +.a0 = fid,
> >>> +.a1 = a1,
> >>> +.a2 = a2,
> >>> +.a3 = a3,
> >>> +.a4 = a4,
> >>> +};
> >>> +struct arm_smccc_1_2_regs resp;
> >>> +
> >>> +arm_smccc_1_2_smc(&arg, &resp);
> >>> +
> >>> +return get_ffa_ret_code(&resp);
> >>> +}
> >>> +
> >>> +static int32_t ffa_features(uint32_t id)
> >>> +{
> >>> +return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
> >>> +}
> >>> +
> >>> +static bool check_mandatory_feature(uint32_t id)
> >>> +{
> >>> +uint32_t ret = ffa_features(id);
> >>> +
> >>> +if (ret)
> >>> +printk(XENLOG_ERR "ffa: mandatory feature id %#x missing\n", id);
> >>
> >> It might be useful here to actually print the error code.
> >> Are we sure that all errors actually mean not supported ?
> >
> > Yes, that's what the standard says.
> >
> >>
> >>> +
> >>> +return !ret;
> >>> +}
> >>> +
> >>> static uint16_t get_vm_id(const struct domain *d)
> >>> {
> >>>/* +1 since 0 is reserved for the hypervisor in FF-A */
> >>> @@ -208,6 +258,66 @@ static void handle_version(struct cpu_user_regs
> >>> *regs)
> >>>set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
> >>> }
> >>>
> >>> +static void handle_msg_send_direct_req(struct cpu_user_regs *regs,
> >>> uint32_t fid)
> >>> +{
> >>> +struct arm_smccc_1_2_regs arg = { .a0 = fid, };
> >>> +struct arm_smccc_1_2_regs resp = { };
> >>> +struct domain *d = current->domain;
> >>> +struct ffa_ctx *ctx = d->arch.tee;
> >>> +uint32_t src_dst;
> >>> +uint64_t mask;
> >>> +
> >>> +if ( smccc_is_conv_64(fid) )
> >>> +mask = GENMASK_ULL(63, 0);
> >>> +else
> >>> +mask = GENMASK_ULL(31, 0);
> >>> +
> >>> +src_dst = get_user_reg(regs, 1);
> >>> +if ( (src_dst >> 16) != get_vm_id(d) )
> >>> +{
> >>> +resp.a0 = FFA_ERROR;
> >>> +resp.a2 = FFA_RET_INVALID_PARAMETERS;
> >>> +goto out;
> >>> +}
> >>> +
> >>> +arg.a1 = src_dst;
> >>> +arg.a2 = get_user_reg(regs, 2) & mask;
> >>> +arg.a3 = get_user_reg(regs, 3) & mask;
> >>> +arg.a4 = get_user_reg(regs, 4) & mask;
> >>> +arg.a5 = get_user_reg(regs, 5) & mask;
> >>> +arg.a6 = get_user_reg(regs, 6) & mask;
> >>> +arg.a7 = get_user_reg(regs, 7) & mask;
> >>> +
> >>> +while ( true )
> >>> +{
> >>> +arm_smccc_1_2_smc(&arg, &resp);
> >>> +
> >>> +switch ( resp.a0 )
> >>> +{
> >>> +case FFA_INTERRUPT:
> >>> +ctx->interrupted = true;
> >>> +goto out;
> >>> +case FFA_ERROR:
> >>> +case FFA_SUCCESS_32:
> >>> +case FFA_SUCCESS_64:
> >>> +case FFA_
Re: [XEN PATCH v7 12/20] xen/arm: ffa: send guest events to Secure Partitions
Hi Jens,
> On 1 Mar 2023, at 11:16, Jens Wiklander wrote:
>
> Hi Bertrand,
>
> On Tue, Feb 28, 2023 at 5:49 PM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
>>>
>>> The FF-A specification defines framework messages sent as direct
>>> requests when certain events occurs. For instance when a VM (guest) is
>>> created or destroyed. Only SPs which have subscribed to these events
>>> will receive them. An SP can subscribe to these messages in its
>>> partition properties.
>>>
>>> Adds a check that the SP supports the needed FF-A features
>>> FFA_PARTITION_INFO_GET and FFA_RX_RELEASE.
>>>
>>> The partition properties of each SP is retrieved with
>>> FFA_PARTITION_INFO_GET which returns the information in our RX buffer.
>>> Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
>>> caller (us), so once we're done with the buffer it must be released
>>> using FFA_RX_RELEASE before another call can be made.
>>>
>>> Signed-off-by: Jens Wiklander
>>> ---
>>> xen/arch/arm/tee/ffa.c | 191 -
>>> 1 file changed, 190 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index 07dd5c36d54b..f1b014b6c7f4 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -140,6 +140,14 @@
>>> #define FFA_MSG_SEND0x846EU
>>> #define FFA_MSG_POLL0x846AU
>>>
>>> +/* Partition information descriptor */
>>> +struct ffa_partition_info_1_1 {
>>> +uint16_t id;
>>> +uint16_t execution_context;
>>> +uint32_t partition_properties;
>>> +uint8_t uuid[16];
>>> +};
>>> +
>>> struct ffa_ctx {
>>>uint32_t guest_vers;
>>>bool interrupted;
>>> @@ -148,6 +156,12 @@ struct ffa_ctx {
>>> /* Negotiated FF-A version to use with the SPMC */
>>> static uint32_t ffa_version __ro_after_init;
>>>
>>> +/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
>>> +static uint16_t *subscr_vm_created __read_mostly;
>>> +static unsigned int subscr_vm_created_count __read_mostly;
>>
>> In the spec the number is returned in w2 so you should utse uint32_t here.
>
> I don't understand. This value is increased for each SP which has the
> property set in the Partition information descriptor.
>
>>
>>> +static uint16_t *subscr_vm_destroyed __read_mostly;
>>> +static unsigned int subscr_vm_destroyed_count __read_mostly;
>>
>> Same here
>>
>>> +
>>> /*
>>> * Our rx/tx buffers shared with the SPMC.
>>> *
>>> @@ -237,6 +251,72 @@ static int32_t ffa_rxtx_map(register_t tx_addr,
>>> register_t rx_addr,
>>>return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
>>> }
>>>
>>> +static int32_t ffa_partition_info_get(uint32_t w1, uint32_t w2, uint32_t
>>> w3,
>>> + uint32_t w4, uint32_t w5,
>>> + uint32_t *count)
>>> +{
>>> +const struct arm_smccc_1_2_regs arg = {
>>> +.a0 = FFA_PARTITION_INFO_GET,
>>> +.a1 = w1,
>>> +.a2 = w2,
>>> +.a3 = w3,
>>> +.a4 = w4,
>>> +.a5 = w5,
>>> +};
>>> +struct arm_smccc_1_2_regs resp;
>>> +uint32_t ret;
>>> +
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +
>>> +ret = get_ffa_ret_code(&resp);
>>> +if ( !ret )
>>> +*count = resp.a2;
>>> +
>>> +return ret;
>>> +}
>>> +
>>> +static int32_t ffa_rx_release(void)
>>> +{
>>> +return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
>>> +}
>>> +
>>> +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>>> + uint8_t msg)
>>
>> This function is actually only useable to send framework created/destroyed
>> messages so the function name should be adapted to be less generic.
>>
>> ffa_send_vm_events ?
>>
>> unless you want to modify it later to send more framework messages ?
>
> That was the plan, but that may never happen. I'll rename it to
> ffa_send_vm_event() since we're only sending one event at a time.
>
>>
>>> +{
>>> +uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
>>> +int32_t res;
>>> +
>>> +if ( msg == FFA_MSG_SEND_VM_CREATED )
>>> +exp_resp |= FFA_MSG_RESP_VM_CREATED;
>>> +else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
>>> +exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
>>> +else
>>> +return FFA_RET_INVALID_PARAMETERS;
>>> +
>>> +do {
>>> +const struct arm_smccc_1_2_regs arg = {
>>> +.a0 = FFA_MSG_SEND_DIRECT_REQ_32,
>>> +.a1 = sp_id,
>>> +.a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
>>> +.a5 = vm_id,
>>> +};
>>> +struct arm_smccc_1_2_regs resp;
>>> +
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp
>>> )
>>> +{
>>> +/*
>>> + * This is an invalid response, likely due to some error in the
>>> + * implem
Re: [XEN PATCH v7 10/20] xen/arm: ffa: add direct request support
Hi Jens,
> On 1 Mar 2023, at 11:55, Jens Wiklander wrote:
>
> Hi Bertrand,
>
> On Mon, Feb 27, 2023 at 4:28 PM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
>>>
>>> Adds support for sending a FF-A direct request. Checks that the SP also
>>> supports handling a 32-bit direct request. 64-bit direct requests are
>>> not used by the mediator itself so there is not need to check for that.
>>>
>>> Signed-off-by: Jens Wiklander
>>> ---
>>> xen/arch/arm/tee/ffa.c | 119 +
>>> 1 file changed, 119 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index 463fd7730573..a5d8a12635b6 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -142,6 +142,7 @@
>>>
>>> struct ffa_ctx {
>>>uint32_t guest_vers;
>>> +bool interrupted;
>>
>> This is added and set here for one special error code but is never used.
>> I would suggest to introduce this when there will be an action based on it.
>
> I'm sorry, I forgot about completing this. I'll add code to deal with
> FFA_INTERRUPT. This will be tricky to test though since we don't use
> FFA_INTERRUPT like this with OP-TEE. The Hypervisor is required by the
> FF-A standard to support it so I better add something.
>
>>
>>> };
>>>
>>> /* Negotiated FF-A version to use with the SPMC */
>>> @@ -167,6 +168,55 @@ static bool ffa_get_version(uint32_t *vers)
>>>return true;
>>> }
>>>
>>> +static int32_t get_ffa_ret_code(const struct arm_smccc_1_2_regs *resp)
>>> +{
>>> +switch ( resp->a0 )
>>> +{
>>> +case FFA_ERROR:
>>> +if ( resp->a2 )
>>> +return resp->a2;
>>> +else
>>> +return FFA_RET_NOT_SUPPORTED;
>>> +case FFA_SUCCESS_32:
>>> +case FFA_SUCCESS_64:
>>> +return FFA_RET_OK;
>>> +default:
>>> +return FFA_RET_NOT_SUPPORTED;
>>> +}
>>> +}
>>> +
>>> +static int32_t ffa_simple_call(uint32_t fid, register_t a1, register_t a2,
>>> + register_t a3, register_t a4)
>>> +{
>>> +const struct arm_smccc_1_2_regs arg = {
>>> +.a0 = fid,
>>> +.a1 = a1,
>>> +.a2 = a2,
>>> +.a3 = a3,
>>> +.a4 = a4,
>>> +};
>>> +struct arm_smccc_1_2_regs resp;
>>> +
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +
>>> +return get_ffa_ret_code(&resp);
>>> +}
>>> +
>>> +static int32_t ffa_features(uint32_t id)
>>> +{
>>> +return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
>>> +}
>>> +
>>> +static bool check_mandatory_feature(uint32_t id)
>>> +{
>>> +uint32_t ret = ffa_features(id);
>>> +
>>> +if (ret)
>>> +printk(XENLOG_ERR "ffa: mandatory feature id %#x missing\n", id);
>>
>> It might be useful here to actually print the error code.
>> Are we sure that all errors actually mean not supported ?
>
> Yes, that's what the standard says.
>
>>
>>> +
>>> +return !ret;
>>> +}
>>> +
>>> static uint16_t get_vm_id(const struct domain *d)
>>> {
>>>/* +1 since 0 is reserved for the hypervisor in FF-A */
>>> @@ -208,6 +258,66 @@ static void handle_version(struct cpu_user_regs *regs)
>>>set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
>>> }
>>>
>>> +static void handle_msg_send_direct_req(struct cpu_user_regs *regs,
>>> uint32_t fid)
>>> +{
>>> +struct arm_smccc_1_2_regs arg = { .a0 = fid, };
>>> +struct arm_smccc_1_2_regs resp = { };
>>> +struct domain *d = current->domain;
>>> +struct ffa_ctx *ctx = d->arch.tee;
>>> +uint32_t src_dst;
>>> +uint64_t mask;
>>> +
>>> +if ( smccc_is_conv_64(fid) )
>>> +mask = GENMASK_ULL(63, 0);
>>> +else
>>> +mask = GENMASK_ULL(31, 0);
>>> +
>>> +src_dst = get_user_reg(regs, 1);
>>> +if ( (src_dst >> 16) != get_vm_id(d) )
>>> +{
>>> +resp.a0 = FFA_ERROR;
>>> +resp.a2 = FFA_RET_INVALID_PARAMETERS;
>>> +goto out;
>>> +}
>>> +
>>> +arg.a1 = src_dst;
>>> +arg.a2 = get_user_reg(regs, 2) & mask;
>>> +arg.a3 = get_user_reg(regs, 3) & mask;
>>> +arg.a4 = get_user_reg(regs, 4) & mask;
>>> +arg.a5 = get_user_reg(regs, 5) & mask;
>>> +arg.a6 = get_user_reg(regs, 6) & mask;
>>> +arg.a7 = get_user_reg(regs, 7) & mask;
>>> +
>>> +while ( true )
>>> +{
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +
>>> +switch ( resp.a0 )
>>> +{
>>> +case FFA_INTERRUPT:
>>> +ctx->interrupted = true;
>>> +goto out;
>>> +case FFA_ERROR:
>>> +case FFA_SUCCESS_32:
>>> +case FFA_SUCCESS_64:
>>> +case FFA_MSG_SEND_DIRECT_RESP_32:
>>> +case FFA_MSG_SEND_DIRECT_RESP_64:
>>> +goto out;
>>> +default:
>>> +/* Bad fid, report back. */
>>> +memset(&arg, 0, sizeof(arg));
>>> +arg.a0 = FFA_ERROR;
>>> +arg.a1 = src_dst;
>>> +arg.a2 = FFA_RET_NOT_SUPPORTED;
>>> +continue;
>>
>> T
Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
Resend - for some reason my email didn't make it out. - Forwarded message from Stefan Hajnoczi - Date: Tue, 21 Feb 2023 10:17:01 -0500 From: Stefan Hajnoczi To: Viresh Kumar Cc: qemu-de...@nongnu.org, virtio-...@lists.oasis-open.org, "Michael S. Tsirkin" , Vincent Guittot , Alex Bennée , stratos-...@op-lists.linaro.org, Oleksandr Tyshchenko , xen-de...@lists.xen.org, Andrew Cooper , Juergen Gross , Sebastien Boeuf , Liu Jiang , Mathieu Poirier Subject: Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support Message-ID: On Tue, Feb 21, 2023 at 03:20:41PM +0530, Viresh Kumar wrote: > The current model of memory mapping at the back-end works fine with > Qemu, where a standard call to mmap() for the respective file > descriptor, passed from front-end, is generally all we need to do before > the front-end can start accessing the guest memory. > > There are other complex cases though, where we need more information at > the backend and need to do more than just an mmap() call. For example, > Xen, a type-1 hypervisor, currently supports memory mapping via two > different methods, foreign-mapping (via /dev/privcmd) and grant-dev (via > /dev/gntdev). In both these cases, the back-end needs to call mmap() > followed by an ioctl() (or vice-versa), and need to pass extra > information via the ioctl(), like the Xen domain-id of the guest whose > memory we are trying to map. > > Add a new protocol feature, 'VHOST_USER_PROTOCOL_F_CUSTOM_MMAP', which > lets the back-end know about the additional memory mapping requirements. > When this feature is negotiated, the front-end can send the > 'VHOST_USER_CUSTOM_MMAP' message type to provide the additional > information to the back-end. > > Signed-off-by: Viresh Kumar > --- > docs/interop/vhost-user.rst | 32 > 1 file changed, 32 insertions(+) The alternative to an in-band approach is to configure these details out-of-band. For example, via command-line options to the vhost-user back-end: $ my-xen-device --mapping-type=foreign-mapping --domain-id=123 I was thinking about both approaches and don't see an obvious reason to choose one or the other. What do you think? > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst > index 3f18ab424eb0..f2b1d705593a 100644 > --- a/docs/interop/vhost-user.rst > +++ b/docs/interop/vhost-user.rst > @@ -258,6 +258,23 @@ Inflight description > > :queue size: a 16-bit size of virtqueues > > +Custom mmap description > +^^^ > + > ++---+---+ > +| flags | value | > ++---+---+ > + > +:flags: 64-bit bit field > + > +- Bit 0 is Xen foreign memory access flag - needs Xen foreign memory mapping. > +- Bit 1 is Xen grant memory access flag - needs Xen grant memory mapping. > + > +:value: a 64-bit hypervisor specific value. > + > +- For Xen foreign or grant memory access, this is set with guest's xen domain > + id. This is highly Xen-specific. How about naming the feature XEN_MMAP instead of CUSTOM_MMAP? If someone needs to add other mmap data later, they should define their own struct instead of trying to squeeze into the same fields as Xen. There is an assumption in this design that a single VHOST_USER_CUSTOM_MMAP message provides the information necessary for all mmaps. Are you sure the limitation that every mmap belongs to the same domain will be workable in the future? > + > C structure > --- > > @@ -867,6 +884,7 @@ Protocol features >#define VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS 14 >#define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 >#define VHOST_USER_PROTOCOL_F_STATUS 16 > + #define VHOST_USER_PROTOCOL_F_CUSTOM_MMAP 17 > > Front-end message types > --- > @@ -1422,6 +1440,20 @@ Front-end message types >query the back-end for its device status as defined in the Virtio >specification. > > +``VHOST_USER_CUSTOM_MMAP`` Most vhost-user protocol messages have a verb like get/set/close/add/listen/etc. I suggest renaming this to VHOST_USER_SET_XEN_MMAP_INFO. > + :id: 41 > + :equivalent ioctl: N/A > + :request payload: Custom mmap description > + :reply payload: N/A > + > + When the ``VHOST_USER_PROTOCOL_F_CUSTOM_MMAP`` protocol feature has been > + successfully negotiated, this message is submitted by the front-end to > + notify the back-end of the special memory mapping requirements, that the > + back-end needs to take care of, while mapping any memory regions sent > + over by the front-end. The front-end must send this message before > + any memory-regions are sent to the back-end via > ``VHOST_USER_SET_MEM_TABLE`` > + or ``VHOST_USER_ADD_MEM_REG`` message types. > + > > Back-end message types > -- > -- > 2.31.1.272.g89b43f80a514 > > > - > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-o
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
Hi Julien, > On 01/03/2023 15:16, Oleksii wrote: > > On Wed, 2023-03-01 at 13:58 +, Julien Grall wrote: > > > On 01/03/2023 12:31, Oleksii wrote: > > > Given this is an alignment issue (Arm32 is more sensible to this > > > than > > > the other architecture, but this is still a potential problem for > > > the > > > other archs), I would really like to understand whether this is > > > an > > > issue > > > in the common code or in the Arm linker script. > > I have a good news. > > > > Alignment of "*(.proc.info)" helps but I checked only yocto- > > qemuarm: > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792923264 > > > > I ran all tests here: > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792953524 > > > > Should I create a separate patch with ALIGN if the tests are > > passed? > > Yes please. But, to be honest, I am not entirely sure what is not > aligned before hand. Do you know if it is possible to download the > Xen > binary from gitlab? It is possible. Please go to the link of the job: https://gitlab.com/xen-project/people/olkur/xen/-/jobs/3856617252 And on the right you will find 'Job artificats' where you can click 'Download'. Or in case if you need a particular binary can click 'Browse' and go to Artifcats/Binaries/: https://gitlab.com/xen-project/people/olkur/xen/-/jobs/3856617252/artifacts/browse/binaries/ ~ Oleksii
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
Hi Oleksii, On 01/03/2023 15:16, Oleksii wrote: On Wed, 2023-03-01 at 13:58 +, Julien Grall wrote: On 01/03/2023 12:31, Oleksii wrote: Given this is an alignment issue (Arm32 is more sensible to this than the other architecture, but this is still a potential problem for the other archs), I would really like to understand whether this is an issue in the common code or in the Arm linker script. I have a good news. Alignment of "*(.proc.info)" helps but I checked only yocto-qemuarm: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792923264 I ran all tests here: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792953524 Should I create a separate patch with ALIGN if the tests are passed? Yes please. But, to be honest, I am not entirely sure what is not aligned before hand. Do you know if it is possible to download the Xen binary from gitlab? Cheers, -- Julien Grall
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
On Wed, 2023-03-01 at 13:58 +, Julien Grall wrote: > On 01/03/2023 12:31, Oleksii wrote: > > Hi Julien, > > Hi, > > > > > > > > > > > > On 24/02/2023 11:31, Oleksii Kurochko wrote: > > > > > > > The following changes were made: > > > > > > > * make GENERIC_BUG_FRAME mandatory for ARM > > > > > > > > > > > > I have asked in patch #1 but will ask it again because I > > > > > > think > > > > > > this > > > > > > should be recorded in the commit message. Can you outline > > > > > > why > > > > > > it is > > > > > > not > > > > > > possible to completely switch to the generic version? > > > > > > > > > > I have just tried to remove it on arm64 and it seems to work. > > > > > This > > > > > was > > > > > only tested with GCC 10 though. But given the generic version > > > > > is > > > > > not > > > > > not > > > > > using the %c modifier, then I wouldn't expect any issue. > > > > > > > > > > Cheers, > > > > > > > > > > > > > I tried to switch ARM to generic implementation. > > > > > > > > Here is the patch: [1] > > > > > > This is similar to the patch I wrote to test with generic > > > implementation > > > on Arm (see my other reply). > > > > > > [...] > > > > > > > (it will be merged with patch 3 if it is OK ) > > > > > > > > And looks like we can switch ARM to generic implementation as > > > > all > > > > tests > > > > passed: > > > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396 > > > > > > Thanks for checking with the gitlab CI! > > > > > > > > > > > The only issue is with yocto-arm: > > > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396/failures > > > > But I am not sure that it is because of my patch > > > > > > This looks unrelated to me. This is happening because there is a > > > data > > > abort before PSCI (used to reboot the platform) is properly > > > setup. I > > > think we should consider to only print once the error rather than > > > every > > > few iterations (not a request for you). > > > > > > That said, I am a bit puzzled why this issue is only happening in > > > the > > > Yocto test (the Debian one pass). Do you know if the test is > > > passing > > > in > > > the normal CI? > > I checked several pipelines on the normal CI and it is fine. > > > > > > If yes, what other modifications did you do? > > It looks like the issue happens after switch ARM to generic > > implementation of bug.h: > > - > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792379063/failures > > [ failure ] > > - > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792381665/failures > > [ passed ] > > > > The difference between builds is only in the commit ' check if ARM > > can > > be fully switched to generic implementation '. > > For second one it is reverted so it looks like we can't switch ARM > > to > > generic implementation now as it is required addiotional > > investigation. > > Thanks. Looking at the error again, it looks like the data abort is > because we are accessing an unaligned address. > > From a brief look at arch/arm/xen.lds.S, I can at least see one case > of > misalignment (not clear why it would only happen now though). Can you > try: > > diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S > index 3f7ebd19f3ed..fb8155bd729f 100644 > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -67,6 +67,7 @@ SECTIONS > *(.data.rel.ro) > *(.data.rel.ro.*) > > + . = ALIGN(4); > __proc_info_start = .; > *(.proc.info) > __proc_info_end = .; > > > > > There is no other significant changes ( only the changes mentioned > > in > > the current mail thread ). > > > > Could we go ahead without switching ARM to generic implementation > > to > > not block other RISC-V patch series? > > Given this is an alignment issue (Arm32 is more sensible to this than > the other architecture, but this is still a potential problem for the > other archs), I would really like to understand whether this is an > issue > in the common code or in the Arm linker script. I have a good news. Alignment of "*(.proc.info)" helps but I checked only yocto-qemuarm: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792923264 I ran all tests here: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792953524 Should I create a separate patch with ALIGN if the tests are passed? ~ Oleksii
[libvirt test] 178838: tolerable trouble: pass/starved - PUSHED
flight 178838 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/178838/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: 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-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass 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-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: libvirt 541670dd5cc33f621f1199e5421efd2c79c25b1a baseline version: libvirt d427102fbd690cb2f43dbf33751cc5194a5b16ce Last test of basis 178733 2023-02-28 04:20:14 Z1 days Testing same since 178838 2023-03-01 04:20:25 Z0 days1 attempts People who touched revisions under test: Laine Stump Peter Krempa Sergey A Sergey A. 김인수 jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt starved test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-qcow2 starved test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw starved test-amd64-i386-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=
[PATCH v2 4/4] x86/pv-shim: suppress core-parking logic
This is all dead code in shim-exclusive mode, so there's no point in building it. Signed-off-by: Jan Beulich --- Depends on "core-parking: fix build with gcc12 and NR_CPUS=1". --- v2: Use UNCONSTRAINED. --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -10,7 +10,7 @@ config COMPAT config CORE_PARKING bool - depends on NR_CPUS > 1 + depends on NR_CPUS > 1 && UNCONSTRAINED config GRANT_TABLE bool "Grant table support" if EXPERT
[PATCH v2 3/4] x86/pv-shim: don't even allow enabling GRANT_TABLE
Grant table code is unused in shim mode, so there's no point in building it in the first place for shim-exclusive mode. Signed-off-by: Jan Beulich --- v2: Use UNCONSTRAINED. --- a/xen/arch/x86/configs/pvshim_defconfig +++ b/xen/arch/x86/configs/pvshim_defconfig @@ -9,7 +9,6 @@ CONFIG_EXPERT=y # Disable features not used by the PV shim # CONFIG_XEN_SHSTK is not set # CONFIG_XEN_IBT is not set -# CONFIG_GRANT_TABLE is not set # CONFIG_HYPFS is not set # CONFIG_BIGMEM is not set # CONFIG_KEXEC is not set --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -15,6 +15,7 @@ config CORE_PARKING config GRANT_TABLE bool "Grant table support" if EXPERT default y + depends on UNCONSTRAINED ---help--- Grant table provides a generic mechanism to memory sharing between domains. This shared memory interface underpins the
[PATCH v2 2/4] common: reduce PV shim footprint
Having CONFIG_PV_SHIM conditionals in common code isn't really nice.
Utilize that we're no longer invoking hypercall handlers via indirect
calls through a table of function vectors. With the use of direct calls
from the macros defined by hypercall-defs.h, we can simply define
overriding macros for event channel and grant table ops handling. All
this requires is arrangement for careful double inclusion of
asm/hypercall.h out of xen/hypercall.h. Such double inclusion is
required because hypercall-defs.h expects certain definitions to be in
place, while the new handling (placed in pv/shim.h, which is now
included from asm/hypercall.h despite the apparent cyclic dependency)
requires prototypes from hypercall-defs.h to be available already.
Note that this makes it necessary to further constrain the stubbing of
pv_shim from common/sched/core.c, and allows removing the inclusion of
asm/guest.h there as well. Since this is actually part of the overall
goal, leverage the mechanism to also get rid of the similar construct in
xsm/flask/hooks.c, including xen/hypercall.h instead.
Note further that kind of as a side effect this fixes grant table
handling for 32-bit shim guests when GRANT_TABLE=y, as the non-stub
compat_grant_table_op() did not redirect to pv_shim_grant_table_op().
A downside of this is that now do_{event_channel,grant_table}_op() are
built in full again when PV_SHIM_EXCLUSIVE=y, despite all the code
actually being dead in that case.
Signed-off-by: Jan Beulich
---
RFC: Sadly I had to restore the two "#define pv_shim false", for Arm to
continue to build. Originally I was hoping to get rid of that
#ifdef-ary altogether. Would it be acceptable to put a single,
central #define in e.g. xen/sched.h or xen/hypercall.h?
--- a/xen/arch/x86/include/asm/hypercall.h
+++ b/xen/arch/x86/include/asm/hypercall.h
@@ -6,14 +6,23 @@
#error "asm/hypercall.h should not be included directly - include
xen/hypercall.h instead"
#endif
-#ifndef __ASM_X86_HYPERCALL_H__
-#define __ASM_X86_HYPERCALL_H__
-
#include
+#include
#include
-#include
#include /* for do_mca */
+
+#ifdef CONFIG_COMPAT
+#include
+#include
+#include
+#endif
+
+#if !defined(__ASM_X86_HYPERCALL_H__) && \
+(!defined(CONFIG_PV_SHIM) || defined(hypercall_args_pv64))
+#define __ASM_X86_HYPERCALL_H__
+
#include
+#include
#define __HYPERVISOR_paging_domctl_cont __HYPERVISOR_arch_1
@@ -33,10 +42,6 @@ void pv_ring3_init_hypercall_page(void *
#ifdef CONFIG_COMPAT
-#include
-#include
-#include
-
extern int
compat_common_vcpu_op(
int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
--- a/xen/arch/x86/include/asm/pv/shim.h
+++ b/xen/arch/x86/include/asm/pv/shim.h
@@ -49,6 +49,22 @@ const struct platform_bad_page *pv_shim_
typeof(do_event_channel_op) pv_shim_event_channel_op;
typeof(do_grant_table_op) pv_shim_grant_table_op;
+#ifdef CONFIG_PV_SHIM_EXCLUSIVE
+#define REVECTOR(pfx, op, args...) pv_shim_ ## op(args)
+#else
+#define REVECTOR(pfx, op, args...) ({ \
+likely(!pv_shim) \
+? pfx ## _ ## op(args) \
+: pv_shim_ ## op(args); \
+})
+#endif
+
+#define do_event_channel_op(args...) REVECTOR(do, event_channel_op, args)
+#define do_grant_table_op(args...) REVECTOR(do, grant_table_op, args)
+#ifdef CONFIG_COMPAT
+#define compat_grant_table_op(args...) REVECTOR(compat, grant_table_op, args)
+#endif
+
#else
static inline void pv_shim_setup_dom(struct domain *d, l4_pgentry_t *l4start,
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -822,9 +822,9 @@ long pv_shim_grant_table_op(unsigned int
return rc;
}
-#ifndef CONFIG_GRANT_TABLE
+#if !defined(CONFIG_GRANT_TABLE) && !defined(CONFIG_PV_SHIM_EXCLUSIVE)
/* Thin wrapper(s) needed. */
-long do_grant_table_op(
+long (do_grant_table_op)(
unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
{
if ( !pv_shim )
@@ -834,7 +834,7 @@ long do_grant_table_op(
}
#ifdef CONFIG_PV32
-int compat_grant_table_op(
+int (compat_grant_table_op)(
unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) uop, unsigned int count)
{
if ( !pv_shim )
--- a/xen/common/compat/grant_table.c
+++ b/xen/common/compat/grant_table.c
@@ -56,7 +56,7 @@ CHECK_gnttab_swap_grant_ref;
CHECK_gnttab_cache_flush;
#undef xen_gnttab_cache_flush
-int compat_grant_table_op(
+int (compat_grant_table_op)(
unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) cmp_uop, unsigned int count)
{
int rc = 0;
--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -32,10 +32,6 @@
#include
#include
-#ifdef CONFIG_PV_SHIM
-#include
-#endif
-
#define ERROR_EXIT(_errno) \
do {\
gdprintk(XENLOG_WARNING,\
@@ -1222,15 +1218,10 @@ static int evtchn_set_priority(const str
return ret;
}
-long do_event_channel_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
+long (do_event_cha
[PATCH v2 1/4] x86: provide an inverted Kconfig control for shim-exclusive mode
While we want certain things turned off in shim-exclusive mode, doing so via "depends on !PV_SHIM_EXCLUSIVE" badly affects allyesconfig: Since that will turn on PV_SHIM_EXCLUSIVE, other options will be turned off as a result. Yet allyesconfig wants to enable as much of the functionality as possible. Retain PV_SHIM_EXCLUSIVE as a prompt-less option such that first of all C code using it can remain as is. This isn't just for less code churn, but also because I think that symbol is more logical to use in many (all?) places. Requested-by: Andrew Cooper Signed-off-by: Jan Beulich --- The new Kconfig control's name is up for improvement suggestions, but I think it's already better than the originally thought of FULL_HYPERVISOR. Secondary Kconfig changes could be omitted; in all of the cases I wasn't really sure whether do the adjustments. I think to avoid setting a bad precedent we want to avoid "depends on !..." (and hence also the functionally equivalent "if !..."), but any default settings or prompt controls could also be left as they were (or could be done the other way around in subsequent patches). The Requested-by: isn't for what exactly is done here, but for the underlying principle of avoiding the negative dependencies we've grown. Outside of Arm-specific code we have two more negative "depends on": COVERAGE requires !LIVEPATCH and SUPPRESS_DUPLICATE_SYMBOL_WARNINGS requires !ENFORCE_UNIQUE_SYMBOLS. The latter could apparently be switched to a choice (enforce, warn, don't warn), but then I'm not sure how well choices play with allyesconfig (I guess the default setting is used). --- v2: New. --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -103,7 +103,7 @@ config PV_LINEAR_PT config HVM bool "HVM support" - depends on !PV_SHIM_EXCLUSIVE + depends on UNCONSTRAINED default !PV_SHIM select COMPAT select IOREQ_SERVER @@ -145,7 +145,7 @@ config XEN_IBT config SHADOW_PAGING bool "Shadow Paging" - default !PV_SHIM_EXCLUSIVE + default UNCONSTRAINED depends on PV || HVM ---help--- @@ -196,7 +196,7 @@ config HVM_FEP config TBOOT bool "Xen tboot support (UNSUPPORTED)" depends on UNSUPPORTED - default !PV_SHIM_EXCLUSIVE + default UNCONSTRAINED select CRYPTO ---help--- Allows support for Trusted Boot using the Intel(R) Trusted Execution @@ -276,17 +276,19 @@ config PV_SHIM If unsure, say Y. -config PV_SHIM_EXCLUSIVE - bool "PV Shim Exclusive" - depends on PV_SHIM - ---help--- - Build Xen in a way which unconditionally assumes PV_SHIM mode. This - option is only intended for use when building a dedicated PV Shim - firmware, and will not function correctly in other scenarios. +config UNCONSTRAINED + bool "do NOT build a functionality restricted hypervisor" if PV_SHIM + default y + help + Do NOT build Xen in a way which unconditionally assumes PV_SHIM mode. - If unsure, say N. + If unsure, say Y. + +config PV_SHIM_EXCLUSIVE + def_bool y + depends on !UNCONSTRAINED -if !PV_SHIM_EXCLUSIVE +if UNCONSTRAINED config HYPERV_GUEST bool "Hyper-V Guest" --- a/xen/arch/x86/configs/pvshim_defconfig +++ b/xen/arch/x86/configs/pvshim_defconfig @@ -3,7 +3,7 @@ CONFIG_PV=y CONFIG_XEN_GUEST=y CONFIG_PVH_GUEST=y CONFIG_PV_SHIM=y -CONFIG_PV_SHIM_EXCLUSIVE=y +# CONFIG_UNCONSTRAINED is not set CONFIG_NR_CPUS=32 CONFIG_EXPERT=y # Disable features not used by the PV shim --- a/xen/drivers/video/Kconfig +++ b/xen/drivers/video/Kconfig @@ -3,10 +3,10 @@ config VIDEO bool config VGA - bool "VGA support" if !PV_SHIM_EXCLUSIVE + bool "VGA support" if UNCONSTRAINED select VIDEO depends on X86 - default y if !PV_SHIM_EXCLUSIVE + default y if UNCONSTRAINED ---help--- Enable VGA output for the Xen hypervisor.
[PATCH v2 0/4] x86/pv-shim: configuring / building re-arrangements
The 2nd patch, while originally having been the main piece here, and while fixing a bug kind of as a side effect, is partly RFC; see there. The last two changes are really minor adjustments for aspects noticed while putting together the main change. 1: provide an inverted Kconfig control for shim-exclusive mode 2: common: reduce PV shim footprint 2: don't even allow enabling GRANT_TABLE 3: suppress core-parking logic The main change in v2 is the addition of patch 1, as requested by Andrew. Jan
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
On 01/03/2023 12:31, Oleksii wrote: Hi Julien, Hi, On 24/02/2023 11:31, Oleksii Kurochko wrote: The following changes were made: * make GENERIC_BUG_FRAME mandatory for ARM I have asked in patch #1 but will ask it again because I think this should be recorded in the commit message. Can you outline why it is not possible to completely switch to the generic version? I have just tried to remove it on arm64 and it seems to work. This was only tested with GCC 10 though. But given the generic version is not not using the %c modifier, then I wouldn't expect any issue. Cheers, I tried to switch ARM to generic implementation. Here is the patch: [1] This is similar to the patch I wrote to test with generic implementation on Arm (see my other reply). [...] (it will be merged with patch 3 if it is OK ) And looks like we can switch ARM to generic implementation as all tests passed: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396 Thanks for checking with the gitlab CI! The only issue is with yocto-arm: https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396/failures But I am not sure that it is because of my patch This looks unrelated to me. This is happening because there is a data abort before PSCI (used to reboot the platform) is properly setup. I think we should consider to only print once the error rather than every few iterations (not a request for you). That said, I am a bit puzzled why this issue is only happening in the Yocto test (the Debian one pass). Do you know if the test is passing in the normal CI? I checked several pipelines on the normal CI and it is fine. If yes, what other modifications did you do? It looks like the issue happens after switch ARM to generic implementation of bug.h: - https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792379063/failures [ failure ] - https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792381665/failures [ passed ] The difference between builds is only in the commit ' check if ARM can be fully switched to generic implementation '. For second one it is reverted so it looks like we can't switch ARM to generic implementation now as it is required addiotional investigation. Thanks. Looking at the error again, it looks like the data abort is because we are accessing an unaligned address. From a brief look at arch/arm/xen.lds.S, I can at least see one case of misalignment (not clear why it would only happen now though). Can you try: diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 3f7ebd19f3ed..fb8155bd729f 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -67,6 +67,7 @@ SECTIONS *(.data.rel.ro) *(.data.rel.ro.*) + . = ALIGN(4); __proc_info_start = .; *(.proc.info) __proc_info_end = .; There is no other significant changes ( only the changes mentioned in the current mail thread ). Could we go ahead without switching ARM to generic implementation to not block other RISC-V patch series? Given this is an alignment issue (Arm32 is more sensible to this than the other architecture, but this is still a potential problem for the other archs), I would really like to understand whether this is an issue in the common code or in the Arm linker script. Cheers, -- Julien Grall
Re: [XEN PATCH v7 10/20] xen/arm: ffa: add direct request support
HI Jens,
> On 1 Mar 2023, at 11:55, Jens Wiklander wrote:
>
> Hi Bertrand,
>
> On Mon, Feb 27, 2023 at 4:28 PM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
>>>
>>> Adds support for sending a FF-A direct request. Checks that the SP also
>>> supports handling a 32-bit direct request. 64-bit direct requests are
>>> not used by the mediator itself so there is not need to check for that.
>>>
>>> Signed-off-by: Jens Wiklander
>>> ---
>>> xen/arch/arm/tee/ffa.c | 119 +
>>> 1 file changed, 119 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index 463fd7730573..a5d8a12635b6 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -142,6 +142,7 @@
>>>
>>> struct ffa_ctx {
>>>uint32_t guest_vers;
>>> +bool interrupted;
>>
>> This is added and set here for one special error code but is never used.
>> I would suggest to introduce this when there will be an action based on it.
>
> I'm sorry, I forgot about completing this. I'll add code to deal with
> FFA_INTERRUPT. This will be tricky to test though since we don't use
> FFA_INTERRUPT like this with OP-TEE. The Hypervisor is required by the
> FF-A standard to support it so I better add something.
You can do that in a different patch then and just remove this from this patch ?
>
>>
>>> };
>>>
>>> /* Negotiated FF-A version to use with the SPMC */
>>> @@ -167,6 +168,55 @@ static bool ffa_get_version(uint32_t *vers)
>>>return true;
>>> }
>>>
>>> +static int32_t get_ffa_ret_code(const struct arm_smccc_1_2_regs *resp)
>>> +{
>>> +switch ( resp->a0 )
>>> +{
>>> +case FFA_ERROR:
>>> +if ( resp->a2 )
>>> +return resp->a2;
>>> +else
>>> +return FFA_RET_NOT_SUPPORTED;
>>> +case FFA_SUCCESS_32:
>>> +case FFA_SUCCESS_64:
>>> +return FFA_RET_OK;
>>> +default:
>>> +return FFA_RET_NOT_SUPPORTED;
>>> +}
>>> +}
>>> +
>>> +static int32_t ffa_simple_call(uint32_t fid, register_t a1, register_t a2,
>>> + register_t a3, register_t a4)
>>> +{
>>> +const struct arm_smccc_1_2_regs arg = {
>>> +.a0 = fid,
>>> +.a1 = a1,
>>> +.a2 = a2,
>>> +.a3 = a3,
>>> +.a4 = a4,
>>> +};
>>> +struct arm_smccc_1_2_regs resp;
>>> +
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +
>>> +return get_ffa_ret_code(&resp);
>>> +}
>>> +
>>> +static int32_t ffa_features(uint32_t id)
>>> +{
>>> +return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
>>> +}
>>> +
>>> +static bool check_mandatory_feature(uint32_t id)
>>> +{
>>> +uint32_t ret = ffa_features(id);
>>> +
>>> +if (ret)
>>> +printk(XENLOG_ERR "ffa: mandatory feature id %#x missing\n", id);
>>
>> It might be useful here to actually print the error code.
>> Are we sure that all errors actually mean not supported ?
>
> Yes, that's what the standard says.
The error code might still be useful in the print.
>
>>
>>> +
>>> +return !ret;
>>> +}
>>> +
>>> static uint16_t get_vm_id(const struct domain *d)
>>> {
>>>/* +1 since 0 is reserved for the hypervisor in FF-A */
>>> @@ -208,6 +258,66 @@ static void handle_version(struct cpu_user_regs *regs)
>>>set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
>>> }
>>>
>>> +static void handle_msg_send_direct_req(struct cpu_user_regs *regs,
>>> uint32_t fid)
>>> +{
>>> +struct arm_smccc_1_2_regs arg = { .a0 = fid, };
>>> +struct arm_smccc_1_2_regs resp = { };
>>> +struct domain *d = current->domain;
>>> +struct ffa_ctx *ctx = d->arch.tee;
>>> +uint32_t src_dst;
>>> +uint64_t mask;
>>> +
>>> +if ( smccc_is_conv_64(fid) )
>>> +mask = GENMASK_ULL(63, 0);
>>> +else
>>> +mask = GENMASK_ULL(31, 0);
>>> +
>>> +src_dst = get_user_reg(regs, 1);
>>> +if ( (src_dst >> 16) != get_vm_id(d) )
>>> +{
>>> +resp.a0 = FFA_ERROR;
>>> +resp.a2 = FFA_RET_INVALID_PARAMETERS;
>>> +goto out;
>>> +}
>>> +
>>> +arg.a1 = src_dst;
>>> +arg.a2 = get_user_reg(regs, 2) & mask;
>>> +arg.a3 = get_user_reg(regs, 3) & mask;
>>> +arg.a4 = get_user_reg(regs, 4) & mask;
>>> +arg.a5 = get_user_reg(regs, 5) & mask;
>>> +arg.a6 = get_user_reg(regs, 6) & mask;
>>> +arg.a7 = get_user_reg(regs, 7) & mask;
>>> +
>>> +while ( true )
>>> +{
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +
>>> +switch ( resp.a0 )
>>> +{
>>> +case FFA_INTERRUPT:
>>> +ctx->interrupted = true;
>>> +goto out;
>>> +case FFA_ERROR:
>>> +case FFA_SUCCESS_32:
>>> +case FFA_SUCCESS_64:
>>> +case FFA_MSG_SEND_DIRECT_RESP_32:
>>> +case FFA_MSG_SEND_DIRECT_RESP_64:
>>> +goto out;
>>> +default:
>>> +/* Bad fid, report back. */
>>> +memset(&arg, 0, sizeof(arg));
>>> +arg
Re: [PATCH v3 00/14] x86/hvm: {svm,vmx} {c,h} cleanup
On 28/02/2023 7:11 am, Jan Beulich wrote: > On 28.02.2023 08:09, Xenia Ragiadakou wrote: >> On 2/27/23 14:17, Jan Beulich wrote: >>> On 27.02.2023 13:06, Andrew Cooper wrote: On 27/02/2023 11:33 am, Jan Beulich wrote: > On 27.02.2023 12:15, Andrew Cooper wrote: >> On 27/02/2023 10:46 am, Jan Beulich wrote: >>> On 24.02.2023 22:33, Andrew Cooper wrote: But I think we want to change tact slightly at this point, so I'm not going to do any further tweaking on commit. Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h, updating the non-SVM include paths as we go. Probably best to chain-include the other svm/hvm/svm/*.h headers temporarily, so we've only got one tree-wide cleanup of the external include paths. Quick tangent - I will be making all of that cpu_has_svm_* infrastructure disappear by moving it into the normal CPUID handling, but I've not had sufficient time to finish that yet. Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and disappear (after my decoupling patch has gone in). >>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h? >>> The latter doesn't use anything from the former, does it? >> It's about what else uses them. >> >> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always >> included in tandem. > Well, yes, that's how things are today. But can you explain to me why > hvm_vcpu has to know nestedsvm's layout? Because it's part of the same single memory allocation. >>> Which keeps growing, and sooner or later we'll need to find something >>> again to split off, so we won't exceed a page in size. The nested >>> structures would, to me, look to be prime candidates for such. >>> > If the field was a pointer, > a forward decl of that struct would suffice, and any entity in the > rest of Xen not caring about struct nestedsvm would no longer see it > (and hence also no longer be re-built if a change is made there). Yes, you could hide it as a pointer. The cost of doing so is an unnecessary extra memory allocation, and extra pointer handling on create/destroy, not to mention extra pointer chasing in memory during use. >> nestedsvm is literally just one struct now, and no subsystem ought to >> have multiple headers when one will do. > When one will do, yes. Removing build dependencies is a good reason > to have separate headers, though. Its not the only only option, and an #ifdef CONFIG_NESTED_VIRT inside the struct would be an equally acceptable way of doing this which wouldn't involve making an extra memory allocation. >>> That would make it a build-time decision, but then on NESTED_VIRT=y >>> hypervisors there might still be guests not meaning to use that >>> functionality, and for quite some time that may actually be a majority. >>> Everything you've posed here is way out of scope for Xenia's series. >>> There was never an intention to extend the scope of the work she's doing. >>> Instead I was trying to limit the scope by suggesting to avoid a piece >>> of rework which later may want undoing. >> Can I suggest to leave hvm/svm/svm.h and hvm/svm/nestedsvm.h separate >> for now? > As per before - that's my preference. It'll be Andrew who you would need > to convince, as he did suggest the folding. Please fold them. I have strong doubts that they would actually be unfolded, even if we did want to make nested virt a build time choice. (I'm not actually convinced that the existing nestedvcpu structure will survive first-pass design of a working nested virt solution.) ~Andrew
Re: [XEN PATCH v7 12/20] xen/arm: ffa: send guest events to Secure Partitions
Hi Jens,
> On 1 Mar 2023, at 11:16, Jens Wiklander wrote:
>
> Hi Bertrand,
>
> On Tue, Feb 28, 2023 at 5:49 PM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
>>>
>>> The FF-A specification defines framework messages sent as direct
>>> requests when certain events occurs. For instance when a VM (guest) is
>>> created or destroyed. Only SPs which have subscribed to these events
>>> will receive them. An SP can subscribe to these messages in its
>>> partition properties.
>>>
>>> Adds a check that the SP supports the needed FF-A features
>>> FFA_PARTITION_INFO_GET and FFA_RX_RELEASE.
>>>
>>> The partition properties of each SP is retrieved with
>>> FFA_PARTITION_INFO_GET which returns the information in our RX buffer.
>>> Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
>>> caller (us), so once we're done with the buffer it must be released
>>> using FFA_RX_RELEASE before another call can be made.
>>>
>>> Signed-off-by: Jens Wiklander
>>> ---
>>> xen/arch/arm/tee/ffa.c | 191 -
>>> 1 file changed, 190 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index 07dd5c36d54b..f1b014b6c7f4 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -140,6 +140,14 @@
>>> #define FFA_MSG_SEND0x846EU
>>> #define FFA_MSG_POLL0x846AU
>>>
>>> +/* Partition information descriptor */
>>> +struct ffa_partition_info_1_1 {
>>> +uint16_t id;
>>> +uint16_t execution_context;
>>> +uint32_t partition_properties;
>>> +uint8_t uuid[16];
>>> +};
>>> +
>>> struct ffa_ctx {
>>>uint32_t guest_vers;
>>>bool interrupted;
>>> @@ -148,6 +156,12 @@ struct ffa_ctx {
>>> /* Negotiated FF-A version to use with the SPMC */
>>> static uint32_t ffa_version __ro_after_init;
>>>
>>> +/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
>>> +static uint16_t *subscr_vm_created __read_mostly;
>>> +static unsigned int subscr_vm_created_count __read_mostly;
>>
>> In the spec the number is returned in w2 so you should utse uint32_t here.
>
> I don't understand. This value is increased for each SP which has the
> property set in the Partition information descriptor.
Using generic types should be prevented when possible.
Here this is a subset of the number of partition which is uint32_t (wX reg) so
i think this would be the logical type for this.
>
>>
>>> +static uint16_t *subscr_vm_destroyed __read_mostly;
>>> +static unsigned int subscr_vm_destroyed_count __read_mostly;
>>
>> Same here
>>
>>> +
>>> /*
>>> * Our rx/tx buffers shared with the SPMC.
>>> *
>>> @@ -237,6 +251,72 @@ static int32_t ffa_rxtx_map(register_t tx_addr,
>>> register_t rx_addr,
>>>return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
>>> }
>>>
>>> +static int32_t ffa_partition_info_get(uint32_t w1, uint32_t w2, uint32_t
>>> w3,
>>> + uint32_t w4, uint32_t w5,
>>> + uint32_t *count)
>>> +{
>>> +const struct arm_smccc_1_2_regs arg = {
>>> +.a0 = FFA_PARTITION_INFO_GET,
>>> +.a1 = w1,
>>> +.a2 = w2,
>>> +.a3 = w3,
>>> +.a4 = w4,
>>> +.a5 = w5,
>>> +};
>>> +struct arm_smccc_1_2_regs resp;
>>> +uint32_t ret;
>>> +
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +
>>> +ret = get_ffa_ret_code(&resp);
>>> +if ( !ret )
>>> +*count = resp.a2;
>>> +
>>> +return ret;
>>> +}
>>> +
>>> +static int32_t ffa_rx_release(void)
>>> +{
>>> +return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
>>> +}
>>> +
>>> +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>>> + uint8_t msg)
>>
>> This function is actually only useable to send framework created/destroyed
>> messages so the function name should be adapted to be less generic.
>>
>> ffa_send_vm_events ?
>>
>> unless you want to modify it later to send more framework messages ?
>
> That was the plan, but that may never happen. I'll rename it to
> ffa_send_vm_event() since we're only sending one event at a time.
>
>>
>>> +{
>>> +uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
>>> +int32_t res;
>>> +
>>> +if ( msg == FFA_MSG_SEND_VM_CREATED )
>>> +exp_resp |= FFA_MSG_RESP_VM_CREATED;
>>> +else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
>>> +exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
>>> +else
>>> +return FFA_RET_INVALID_PARAMETERS;
>>> +
>>> +do {
>>> +const struct arm_smccc_1_2_regs arg = {
>>> +.a0 = FFA_MSG_SEND_DIRECT_REQ_32,
>>> +.a1 = sp_id,
>>> +.a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
>>> +.a5 = vm_id,
>>> +};
>>> +struct arm_smccc_1_2_regs resp;
>>> +
>>> +arm_smccc_1_2_smc(&arg, &resp);
>>> +if ( resp.a0 != FFA_MSG_SEND_DI
[xen-unstable test] 178827: trouble: broken/fail/pass/starved
flight 178827 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/178827/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-raw broken test-amd64-i386-livepatchbroken test-amd64-i386-libvirt-raw 5 host-install(5)broken REGR. vs. 178771 test-amd64-i386-livepatch 5 host-install(5)broken REGR. vs. 178771 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 178771 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 178771 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 178771 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 178771 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 178771 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 178771 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 178771 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 178771 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 178771 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-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-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-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-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen b84fdf521b306cce64388fe57ee6c7c00f9d3e76 baseline version: xen bfc3780f23ded229f42a2565783e21c32083bbfd Last test of basis 178771 2023-02-28 13:11:03 Z0 days Testing same since 178827 2023-03-01 01:55:55 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Anthony PERARD Demi Marie Obenour Jan Beulich Sergey Dyasli Xenia Ragiadakou jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf
Re: [PATCH v4 3/5] xen/riscv: introduce an implementation of macros from
On 01.03.2023 12:51, Oleksii wrote:
> On Mon, 2023-02-27 at 13:59 +0100, Jan Beulich wrote:
>> On 24.02.2023 12:35, Oleksii Kurochko wrote:
>>> +{
>>> + if ( (insn & INSN_LENGTH_MASK) == INSN_LENGTH_32 )
>>> + return ( insn == BUG_INSN_32 );
>>> + else
>>> + return ( (insn & COMPRESSED_INSN_MASK) == BUG_INSN_16 );
>>
>> Nit (style): The kind-of-operand to return is an expression. Hence
>> you have stray blanks there immediately inside the parentheses.
>> (This is unlike e.g. if(), where you've formatted things correctly.)
> To be 100% sure, should it be:
> return ( ( insn & COMPRESSED_INSN_MASK ) == BUG_INSN_16 );
No, that's yet more spaces instead of fewer ones.
if ( (insn & INSN_LENGTH_MASK) == INSN_LENGTH_32 )
return insn == BUG_INSN_32;
else
return (insn & COMPRESSED_INSN_MASK) == BUG_INSN_16;
or, if you really want the extra parentheses:
if ( (insn & INSN_LENGTH_MASK) == INSN_LENGTH_32 )
return (insn == BUG_INSN_32);
else
return ((insn & COMPRESSED_INSN_MASK) == BUG_INSN_16);
(Personally I'd also omit the "else":
if ( (insn & INSN_LENGTH_MASK) == INSN_LENGTH_32 )
return insn == BUG_INSN_32;
return (insn & COMPRESSED_INSN_MASK) == BUG_INSN_16;
. Plus I don't think you really need to mask as much, i.e.
return insn == BUG_INSN_32 ||
(insn & COMPRESSED_INSN_MASK) == BUG_INSN_16;
would do as well afaict.)
Jan
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
On Wed, 2023-03-01 at 09:31 +, Julien Grall wrote:
> Hi,
>
> On 01/03/2023 08:58, Oleksii wrote:
> > On Tue, 2023-02-28 at 17:48 +, Julien Grall wrote:
> > > Hi Oleksii,
> > >
> > > On 28/02/2023 15:09, Oleksii wrote:
> > > > On Sat, 2023-02-25 at 16:49 +, Julien Grall wrote:
> > > > > Hi Oleksii,
> > > > >
> > > > > On 24/02/2023 11:31, Oleksii Kurochko wrote:
> > > > > > The following changes were made:
> > > > > > * make GENERIC_BUG_FRAME mandatory for ARM
> > > > >
> > > > > I have asked in patch #1 but will ask it again because I
> > > > > think
> > > > > this
> > > > > should be recorded in the commit message. Can you outline why
> > > > > it
> > > > > is
> > > > > not
> > > > > possible to completely switch to the generic version?
> > > > I haven't tried to switch ARM too because of comment regarding
> > > > 'i'
> > > > in
> > > > :
> > > > /*
> > > > * GCC will not allow to use "i" when PIE is enabled (Xen
> > > > doesn't
> > > > set
> > > > the
> > > > * flag but instead rely on the default value from the
> > > > compiler).
> > > > So
> > > > the
> > > > * easiest way to implement run_in_exception_handler() is to
> > > > pass
> > > > the
> > > > to
> > > > * be called function in a fixed register.
> > > > */
> > >
> > > I would expect this comment to be valid for any arch. So if there
> > > is
> > > a
> > > need to deal with PIE, then we would not be able to use "i" in
> > > the
> > > BUG
> > > frame.
> > >
> > > Note that we are now explicitly compiling Xen without PIE (see
> > > Config.mk).
> > Then it looks like some architectures isn't expected to be compiled
> > with PIE. I mean that x86's bug.h is used 'i' and there is no any
> > alternative version in case of PIE.
> >
> > If Xen should be compilable with PIE then we have to use ARM
> > implementation of bug.h everywhere. ( based on comment about 'i'
> > with
> > PIE ).
>
> The comment was added because until commit ecd6b9759919 ("Config.mk:
> correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS") we would let
> the compiler to decide whether PIE should be enabled.
>
> Now we are forcing -fno-pie for Xen on any architecture (the flag is
> added at the top-level in Config.mk).
>
> >
> > Now I am totally confused...
>
> My point was not about using the Arm implementation everywhere. My
> point
> was that we disable even for Arm and therefore it is fine to use the
> common version.
>
> If in the future we need to support PIE in Xen (I am not aware of any
> effort yet), then we could decide to update the common BUG framework.
> But for now, I don't think this is something you need to care in your
> series.
>
Thanks for clarification.
> Cheers,
>
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
Hi Julien, > > > > > > > On 24/02/2023 11:31, Oleksii Kurochko wrote: > > > > > The following changes were made: > > > > > * make GENERIC_BUG_FRAME mandatory for ARM > > > > > > > > I have asked in patch #1 but will ask it again because I think > > > > this > > > > should be recorded in the commit message. Can you outline why > > > > it is > > > > not > > > > possible to completely switch to the generic version? > > > > > > I have just tried to remove it on arm64 and it seems to work. > > > This > > > was > > > only tested with GCC 10 though. But given the generic version is > > > not > > > not > > > using the %c modifier, then I wouldn't expect any issue. > > > > > > Cheers, > > > > > > > I tried to switch ARM to generic implementation. > > > > Here is the patch: [1] > > This is similar to the patch I wrote to test with generic > implementation > on Arm (see my other reply). > > [...] > > > (it will be merged with patch 3 if it is OK ) > > > > And looks like we can switch ARM to generic implementation as all > > tests > > passed: > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396 > > Thanks for checking with the gitlab CI! > > > > > The only issue is with yocto-arm: > > https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/791549396/failures > > But I am not sure that it is because of my patch > > This looks unrelated to me. This is happening because there is a data > abort before PSCI (used to reboot the platform) is properly setup. I > think we should consider to only print once the error rather than > every > few iterations (not a request for you). > > That said, I am a bit puzzled why this issue is only happening in the > Yocto test (the Debian one pass). Do you know if the test is passing > in > the normal CI? I checked several pipelines on the normal CI and it is fine. > > If yes, what other modifications did you do? It looks like the issue happens after switch ARM to generic implementation of bug.h: - https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792379063/failures [ failure ] - https://gitlab.com/xen-project/people/olkur/xen/-/pipelines/792381665/failures [ passed ] The difference between builds is only in the commit ' check if ARM can be fully switched to generic implementation '. For second one it is reverted so it looks like we can't switch ARM to generic implementation now as it is required addiotional investigation. There is no other significant changes ( only the changes mentioned in the current mail thread ). Could we go ahead without switching ARM to generic implementation to not block other RISC-V patch series? ~ Oleksii
Re: [PATCH v4 3/5] xen/riscv: introduce an implementation of macros from
On Mon, 2023-02-27 at 13:59 +0100, Jan Beulich wrote:
> On 24.02.2023 12:35, Oleksii Kurochko wrote:
> > The patch introduces macros: BUG(), WARN(), run_in_exception(),
> > assert_failed.
> >
> > The implementation uses "ebreak" instruction in combination with
> > diffrent bug frame tables (for each type) which contains useful
> > information.
> >
> > Signed-off-by: Oleksii Kurochko
> > ---
> > Changes in V4:
> > - Updates in RISC-V's :
> > * Add explanatory comment about why there is only defined for
> > 32-bits length
> > instructions and 16/32-bits BUG_INSN_{16,32}.
> > * Change 'unsigned long' to 'unsigned int' inside
> > GET_INSN_LENGTH().
> > * Update declaration of is_valid_bugaddr(): switch return type
> > from int to bool
> > and the argument from 'unsigned int' to 'vaddr'.
> > - Updates in RISC-V's traps.c:
> > * replace /xen and /asm includes
> > * update definition of is_valid_bugaddr():switch return type
> > from int to bool
> > and the argument from 'unsigned int' to 'vaddr'. Code style
> > inside function
> > was updated too.
> > * do_bug_frame() refactoring:
> > * local variables start and bug became 'const struct
> > bug_frame'
> > * bug_frames[] array became 'static const struct bug_frame[]
> > = ...'
> > * remove all casts
> > * remove unneeded comments and add an explanatory comment
> > that the do_bug_frame()
> > will be switched to a generic one.
> > * do_trap() refactoring:
> > * read 16-bits value instead of 32-bits as compressed
> > instruction can
> > be used and it might happen than only 16-bits may be
> > accessible.
> > * code style updates
> > * re-use instr variable instead of re-reading instruction.
> > - Updates in setup.c:
> > * add blank line between xen/ and asm/ includes.
> > ---
> > Changes in V3:
> > - Rebase the patch "xen/riscv: introduce an implementation of
> > macros
> > from " on top of patch series [introduce generic
> > implementation
> > of macros from bug.h]
> > ---
> > Changes in V2:
> > - Remove __ in define namings
> > - Update run_in_exception_handler() with
> > register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn);
> > - Remove bug_instr_t type and change it's usage to uint32_t
> > ---
> > xen/arch/riscv/include/asm/bug.h | 48 ++
> > xen/arch/riscv/include/asm/processor.h | 2 +
> > xen/arch/riscv/setup.c | 1 +
> > xen/arch/riscv/traps.c | 125
> > +
> > xen/arch/riscv/xen.lds.S | 10 ++
> > 5 files changed, 186 insertions(+)
> > create mode 100644 xen/arch/riscv/include/asm/bug.h
> >
> > diff --git a/xen/arch/riscv/include/asm/bug.h
> > b/xen/arch/riscv/include/asm/bug.h
> > new file mode 100644
> > index 00..67ade6f895
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/bug.h
> > @@ -0,0 +1,48 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2012 Regents of the University of California
> > + * Copyright (C) 2021-2023 Vates
> > + *
> > + */
> > +#ifndef _ASM_RISCV_BUG_H
> > +#define _ASM_RISCV_BUG_H
> > +
> > +#include
> > +
> > +#ifndef __ASSEMBLY__
>
> This #ifndef contradicts the xen/types.h inclusion. Either the
> #include
> moves down below here, or the #ifndef goes away as pointless. This is
> because xen/types.h cannot be include in assembly files.
I will remove it. It looks like it leave when as generic implementation
was taken ARM.
>
> > +#define BUG_INSTR "ebreak"
> > +
> > +/*
> > + * The base instruction set has a fixed length of 32-bit naturally
> > aligned
> > + * instructions.
> > + *
> > + * There are extensions of variable length ( where each
> > instruction can be
> > + * any number of 16-bit parcels in length ) but they aren't used
> > in Xen
> > + * and Linux kernel ( where these definitions were taken from ).
> > + *
> > + * Compressed ISA is used now where the instruction length is 16
> > bit and
> > + * 'ebreak' instruction, in this case, can be either 16 or 32 bit
> > (
> > + * depending on if compressed ISA is used or not )
> > + */
> > +#define INSN_LENGTH_MASK _UL(0x3)
> > +#define INSN_LENGTH_32 _UL(0x3)
> > +
> > +#define BUG_INSN_32 _UL(0x00100073) /* ebreak */
> > +#define BUG_INSN_16 _UL(0x9002) /* c.ebreak */
> > +#define COMPRESSED_INSN_MASK _UL(0x)
> > +
> > +#define GET_INSN_LENGTH(insn) \
> > +({ \
> > + unsigned int len; \
> > + len = ((insn & INSN_LENGTH_MASK) == INSN_LENGTH_32) ? \
> > + 4UL : 2UL; \
>
> Why the UL suffixes?
>
> > + len; \
> > +})
>
> And anyway - can't the whole macro be written without using any
> extension:
>
> #define GET_INSN_LEN
Re: [PATCH v2 3/3] tools/xen-ucode: print information about currently loaded ucode
On 28.02.2023 18:39, Sergey Dyasli wrote:
> Add an option to xen-ucode tool to print the currently loaded ucode
> version and also print it during usage info. Print CPU signature and
> processor flags as well. The raw data comes from cpuinfo directory in
> xenhypfs and from XENPF_get_cpu_version platform op.
While I don't mind the use of the platform-op, I'm little puzzled by the
mix. If CPU information is to be exposed in hypfs, can't we expose there
everything that's needed here?
Then again, perhaps in a different context, Andrew pointed out that hypfs
is an optional component, so relying on its presence in the underlying
hypervisor will need weighing against the alternative of adding a new
platform-op for the ucode-related data (as you had it in v1). Since I'm
unaware of a request to switch, are there specific reasons you did?
> --- a/tools/misc/xen-ucode.c
> +++ b/tools/misc/xen-ucode.c
> @@ -11,6 +11,96 @@
> #include
> #include
> #include
> +#include
> +
> +static const char intel_id[] = "GenuineIntel";
> +static const char amd_id[] = "AuthenticAMD";
> +
> +static const char sig_path[] = "/cpuinfo/cpu-signature";
> +static const char rev_path[] = "/cpuinfo/microcode-revision";
> +static const char pf_path[] = "/cpuinfo/processor-flags";
Together with the use below I conclude (without having looked at patch 1
yet) that you only expose perhaps the BSP's data, rather than such for
all CPUs. (And I was actually going to put up the question whether data
like the one presented here might not also be of interest for parked
CPUs.)
> +static int hypfs_read_uint(struct xenhypfs_handle *hdl, const char *path,
> + unsigned int *var)
> +{
> +char *result;
> +result = xenhypfs_read(hdl, path);
> +if ( !result )
> +return -1;
> +
> +errno = 0;
> +*var = strtol(result, NULL, 10);
Better use strtoul() (as you're after an unsigned quantity), and perhaps
also better to not assume 10 as the radix (neither signature nor ucode
revision are very useful to expose as decimal numbers in hypfs)? Plus
perhaps deal with the result not fitting in "unsigned int"?
> +if ( errno )
> +return -1;
> +
> +return 0;
> +}
> +
> +static void show_curr_cpu(FILE *f)
> +{
> +int ret;
> +struct xenhypfs_handle *hdl;
> +xc_interface *xch;
> +struct xenpf_pcpu_version cpu_ver = {0};
> +bool intel = false, amd = false;
> +unsigned int cpu_signature, pf, ucode_revision;
> +
> +hdl = xenhypfs_open(NULL, 0);
> +if ( !hdl )
> +return;
> +
> +xch = xc_interface_open(0, 0, 0);
> +if ( xch == NULL )
> +return;
> +
> +ret = xc_get_cpu_version(xch, &cpu_ver);
> +if ( ret )
> +return;
> +
> +if ( memcmp(cpu_ver.vendor_id, intel_id,
> +sizeof(cpu_ver.vendor_id)) == 0 )
> +intel = true;
> +else if ( memcmp(cpu_ver.vendor_id, amd_id,
> + sizeof(cpu_ver.vendor_id)) == 0 )
> +amd = true;
> +
> +if ( hypfs_read_uint(hdl, sig_path, &cpu_signature) != 0 )
> +return;
> +
> +if ( hypfs_read_uint(hdl, rev_path, &ucode_revision) != 0 )
> +return;
You consume these two fields only when "intel || amd"; without having
looked at patch 1 yet I'd also assume the node might not be present for
other vendors, and hence no output would be produced at all then, due
to the failure here.
> +if ( intel && hypfs_read_uint(hdl, pf_path, &pf) != 0 )
Nit: Stray double blank before "&pf".
> +return;
> +
> +/*
> + * Print signature in a form that allows to quickly identify which ucode
> + * blob to load, e.g.:
> + *
> + * Intel: /lib/firmware/intel-ucode/06-55-04
> + * AMD: /lib/firmware/amd-ucode/microcode_amd_fam19h.bin
> + */
> +if ( intel )
> +{
> +fprintf(f, "Current CPU signature is: %02x-%02x-%02x (raw %#x)\n",
> + cpu_ver.family, cpu_ver.model, cpu_ver.stepping,
> + cpu_signature);
> +}
> +else if ( amd )
> +{
> +fprintf(f, "Current CPU signature is: fam%xh (raw %#x)\n",
> + cpu_ver.family, cpu_signature);
> +}
else
fprintf("...", cpu_ver.vendor_id,
cpu_ver.family, cpu_ver.model, cpu_ver.stepping);
? Otherwise some kind of error message would imo be needed, such that the
tool won't exit successfully without providing any output at all. Recall
that we consider Hygon an alive target CPU, just with (at present) no
ucode support.
> +if ( intel || amd )
> +fprintf(f, "Current CPU microcode revision is: %#x\n",
> ucode_revision);
> +
> +if ( intel )
> +fprintf(f, "Current CPU processor flags are: %#x\n", pf);
> +
> +xc_interface_close(xch);
> +xenhypfs_close(hdl);
Maybe do these earlier, as soon as you're done with each of the two items?
Jan
Re: [XEN PATCH v7 11/20] xen/arm: ffa: map SPMC rx/tx buffers
Hi,
On Wed, Mar 1, 2023 at 10:56 AM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 1 Mar 2023, at 10:30, Jens Wiklander wrote:
> >
> > Hi Bertrand,
> >
> > On Tue, Feb 28, 2023 at 1:57 PM Bertrand Marquis
> > wrote:
> >>
> >> Hi Jens,
> >>
> >>> On 22 Feb 2023, at 16:33, Jens Wiklander
> >>> wrote:
> >>>
> >>> When initializing the FF-A mediator map the RX and TX buffers shared with
> >>> the SPMC.
> >>>
> >>> These buffer are later used to to transmit data that cannot be passed in
> >>> registers only.
> >>>
> >>> Adds a check that the SP supports the needed FF-A features
> >>> FFA_RXTX_MAP_64 / FFA_RXTX_MAP_32 and FFA_RXTX_UNMAP. In 64-bit mode we
> >>> must use FFA_RXTX_MAP_64 since registers are used to transmit the
> >>> physical addresses of the RX/TX buffers.
> >>
> >> Right now, FFA on 32bit would only work correctly if LPAE is not used and
> >> only addresses
> >> under 4G are used by Xen and by guests as addresses are transferred
> >> through a single register.
> >>
> >> I think that we need for now to only enable FFA support on 64bit as the
> >> limitations we
> >> would need to enforce on 32bit are complex and the use case for FFA on
> >> 32bit platforms
> >> is not that obvious now.
> >
> > OK, I'll drop the #ifdef CONFIG_ARM_64 and #ifdef CONFIG_ARM_32 and
> > instead depend on ARM_64 in Kconfig.
> > If we ever want to use this on ARM_32 we'll have to go through
> > everything anyway.
>
> Yes this is the best solution for now.
> And support.md patch is already saying already arm64.
>
> >
> >>
> >>>
> >>> Signed-off-by: Jens Wiklander
> >>> ---
> >>> xen/arch/arm/tee/ffa.c | 57 +-
> >>> 1 file changed, 56 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> >>> index a5d8a12635b6..07dd5c36d54b 100644
> >>> --- a/xen/arch/arm/tee/ffa.c
> >>> +++ b/xen/arch/arm/tee/ffa.c
> >>> @@ -148,6 +148,15 @@ struct ffa_ctx {
> >>> /* Negotiated FF-A version to use with the SPMC */
> >>> static uint32_t ffa_version __ro_after_init;
> >>>
> >>> +/*
> >>> + * Our rx/tx buffers shared with the SPMC.
> >>> + *
> >>> + * ffa_page_count is the number of pages used in each of these buffers.
> >>> + */
> >>> +static void *ffa_rx __read_mostly;
> >>> +static void *ffa_tx __read_mostly;
> >>> +static unsigned int ffa_page_count __read_mostly;
> >>> +
> >>> static bool ffa_get_version(uint32_t *vers)
> >>> {
> >>>const struct arm_smccc_1_2_regs arg = {
> >>> @@ -217,6 +226,17 @@ static bool check_mandatory_feature(uint32_t id)
> >>>return !ret;
> >>> }
> >>>
> >>> +static int32_t ffa_rxtx_map(register_t tx_addr, register_t rx_addr,
> >>> +uint32_t page_count)
> >>
> >> Using register_t type here is doing an implicit cast when called and on
> >> 32bit this might later remove part of the address.
> >> This function must take paddr_t as parameters.
> >
> > I'll change to paddr_t for rx/tx.
> >
> >>
> >>> +{
> >>> +uint32_t fid = FFA_RXTX_MAP_32;
> >>> +
> >>> +if ( IS_ENABLED(CONFIG_ARM_64) )
> >>> +fid = FFA_RXTX_MAP_64;
> >>> +
> >>> +return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
> >>
> >> simple call might not be suitable on 32bits due to the conversion.
> >> As said earlier, it would make more sense to disable FFA on 32bit and
> >> put some comments/build_bug_on in the code in places where there
> >> would be something to fix.
> >
> > I'm dropping the 32-bit support.
> >
> >>
> >>> +}
> >>> +
> >>> static uint16_t get_vm_id(const struct domain *d)
> >>> {
> >>>/* +1 since 0 is reserved for the hypervisor in FF-A */
> >>> @@ -384,6 +404,7 @@ static int ffa_relinquish_resources(struct domain *d)
> >>> static bool ffa_probe(void)
> >>> {
> >>>uint32_t vers;
> >>> +int e;
> >>>unsigned int major_vers;
> >>>unsigned int minor_vers;
> >>>
> >>> @@ -426,12 +447,46 @@ static bool ffa_probe(void)
> >>>printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
> >>> major_vers, minor_vers);
> >>>
> >>> -if ( !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
> >>> +if (
> >>> +#ifdef CONFIG_ARM_64
> >>> + !check_mandatory_feature(FFA_RXTX_MAP_64) ||
> >>> +#endif
> >>> +#ifdef CONFIG_ARM_32
> >>> + !check_mandatory_feature(FFA_RXTX_MAP_32) ||
> >>> +#endif
> >>> + !check_mandatory_feature(FFA_RXTX_UNMAP) ||
> >>> + !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
> >>>return false;
> >>>
> >>> +ffa_rx = alloc_xenheap_pages(0, 0);
> >>> +if ( !ffa_rx )
> >>> +return false;
> >>> +
> >>> +ffa_tx = alloc_xenheap_pages(0, 0);
> >>> +if ( !ffa_tx )
> >>> +goto err_free_ffa_rx;
> >>> +
> >>> +e = ffa_rxtx_map(__pa(ffa_tx), __pa(ffa_rx), 1);
> >>> +if ( e )
> >>> +{
> >>> +printk(XENLOG_ERR "ffa: Failed to map rxtx: error %d\n", e);
> >>> +goto err_free_ffa_tx;
> >>> +}
> >>> +ffa_page_count = 1;
> >>
> >> ffa
Re: [XEN PATCH v7 10/20] xen/arm: ffa: add direct request support
Hi Bertrand,
On Mon, Feb 27, 2023 at 4:28 PM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
> >
> > Adds support for sending a FF-A direct request. Checks that the SP also
> > supports handling a 32-bit direct request. 64-bit direct requests are
> > not used by the mediator itself so there is not need to check for that.
> >
> > Signed-off-by: Jens Wiklander
> > ---
> > xen/arch/arm/tee/ffa.c | 119 +
> > 1 file changed, 119 insertions(+)
> >
> > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> > index 463fd7730573..a5d8a12635b6 100644
> > --- a/xen/arch/arm/tee/ffa.c
> > +++ b/xen/arch/arm/tee/ffa.c
> > @@ -142,6 +142,7 @@
> >
> > struct ffa_ctx {
> > uint32_t guest_vers;
> > +bool interrupted;
>
> This is added and set here for one special error code but is never used.
> I would suggest to introduce this when there will be an action based on it.
I'm sorry, I forgot about completing this. I'll add code to deal with
FFA_INTERRUPT. This will be tricky to test though since we don't use
FFA_INTERRUPT like this with OP-TEE. The Hypervisor is required by the
FF-A standard to support it so I better add something.
>
> > };
> >
> > /* Negotiated FF-A version to use with the SPMC */
> > @@ -167,6 +168,55 @@ static bool ffa_get_version(uint32_t *vers)
> > return true;
> > }
> >
> > +static int32_t get_ffa_ret_code(const struct arm_smccc_1_2_regs *resp)
> > +{
> > +switch ( resp->a0 )
> > +{
> > +case FFA_ERROR:
> > +if ( resp->a2 )
> > +return resp->a2;
> > +else
> > +return FFA_RET_NOT_SUPPORTED;
> > +case FFA_SUCCESS_32:
> > +case FFA_SUCCESS_64:
> > +return FFA_RET_OK;
> > +default:
> > +return FFA_RET_NOT_SUPPORTED;
> > +}
> > +}
> > +
> > +static int32_t ffa_simple_call(uint32_t fid, register_t a1, register_t a2,
> > + register_t a3, register_t a4)
> > +{
> > +const struct arm_smccc_1_2_regs arg = {
> > +.a0 = fid,
> > +.a1 = a1,
> > +.a2 = a2,
> > +.a3 = a3,
> > +.a4 = a4,
> > +};
> > +struct arm_smccc_1_2_regs resp;
> > +
> > +arm_smccc_1_2_smc(&arg, &resp);
> > +
> > +return get_ffa_ret_code(&resp);
> > +}
> > +
> > +static int32_t ffa_features(uint32_t id)
> > +{
> > +return ffa_simple_call(FFA_FEATURES, id, 0, 0, 0);
> > +}
> > +
> > +static bool check_mandatory_feature(uint32_t id)
> > +{
> > +uint32_t ret = ffa_features(id);
> > +
> > +if (ret)
> > +printk(XENLOG_ERR "ffa: mandatory feature id %#x missing\n", id);
>
> It might be useful here to actually print the error code.
> Are we sure that all errors actually mean not supported ?
Yes, that's what the standard says.
>
> > +
> > +return !ret;
> > +}
> > +
> > static uint16_t get_vm_id(const struct domain *d)
> > {
> > /* +1 since 0 is reserved for the hypervisor in FF-A */
> > @@ -208,6 +258,66 @@ static void handle_version(struct cpu_user_regs *regs)
> > set_regs(regs, vers, 0, 0, 0, 0, 0, 0, 0);
> > }
> >
> > +static void handle_msg_send_direct_req(struct cpu_user_regs *regs,
> > uint32_t fid)
> > +{
> > +struct arm_smccc_1_2_regs arg = { .a0 = fid, };
> > +struct arm_smccc_1_2_regs resp = { };
> > +struct domain *d = current->domain;
> > +struct ffa_ctx *ctx = d->arch.tee;
> > +uint32_t src_dst;
> > +uint64_t mask;
> > +
> > +if ( smccc_is_conv_64(fid) )
> > +mask = GENMASK_ULL(63, 0);
> > +else
> > +mask = GENMASK_ULL(31, 0);
> > +
> > +src_dst = get_user_reg(regs, 1);
> > +if ( (src_dst >> 16) != get_vm_id(d) )
> > +{
> > +resp.a0 = FFA_ERROR;
> > +resp.a2 = FFA_RET_INVALID_PARAMETERS;
> > +goto out;
> > +}
> > +
> > +arg.a1 = src_dst;
> > +arg.a2 = get_user_reg(regs, 2) & mask;
> > +arg.a3 = get_user_reg(regs, 3) & mask;
> > +arg.a4 = get_user_reg(regs, 4) & mask;
> > +arg.a5 = get_user_reg(regs, 5) & mask;
> > +arg.a6 = get_user_reg(regs, 6) & mask;
> > +arg.a7 = get_user_reg(regs, 7) & mask;
> > +
> > +while ( true )
> > +{
> > +arm_smccc_1_2_smc(&arg, &resp);
> > +
> > +switch ( resp.a0 )
> > +{
> > +case FFA_INTERRUPT:
> > +ctx->interrupted = true;
> > +goto out;
> > +case FFA_ERROR:
> > +case FFA_SUCCESS_32:
> > +case FFA_SUCCESS_64:
> > +case FFA_MSG_SEND_DIRECT_RESP_32:
> > +case FFA_MSG_SEND_DIRECT_RESP_64:
> > +goto out;
> > +default:
> > +/* Bad fid, report back. */
> > +memset(&arg, 0, sizeof(arg));
> > +arg.a0 = FFA_ERROR;
> > +arg.a1 = src_dst;
> > +arg.a2 = FFA_RET_NOT_SUPPORTED;
> > +continue;
>
> There is a potential infinite loop here and i do not understand
> why this needs to be done.
> Here if something
Re: [PATCH v4 2/5] xen/riscv: introduce trap_init()
On 01.03.2023 11:34, Oleksii wrote:
> On Mon, 2023-02-27 at 13:50 +0100, Jan Beulich wrote:
>> On 24.02.2023 12:35, Oleksii Kurochko wrote:
>>> @@ -11,6 +13,8 @@ void __init noreturn start_xen(void)
>>> {
>>> early_printk("Hello from C env\n");
>>>
>>> + trap_init();
>>> +
>>> for ( ;; )
>>> asm volatile ("wfi");
>>
>> Along the lines of what Andrew has said there - it's hard to see
>> how this can come (both code flow and patch ordering wise) ahead
>> of clearing .bss.
> So should I add the patch with initializatin of bss as a part of this
> patch series?
That or make clear in the cover letter that there is a dependency.
Jan
Re: [PATCH v4 2/5] xen/riscv: introduce trap_init()
On Mon, 2023-02-27 at 13:50 +0100, Jan Beulich wrote:
> On 24.02.2023 12:35, Oleksii Kurochko wrote:
> > @@ -11,6 +13,8 @@ void __init noreturn start_xen(void)
> > {
> > early_printk("Hello from C env\n");
> >
> > + trap_init();
> > +
> > for ( ;; )
> > asm volatile ("wfi");
>
> Along the lines of what Andrew has said there - it's hard to see
> how this can come (both code flow and patch ordering wise) ahead
> of clearing .bss.
So should I add the patch with initializatin of bss as a part of this
patch series?
~ Oleksii
Re: [XEN PATCH v7 12/20] xen/arm: ffa: send guest events to Secure Partitions
Hi Bertrand,
On Tue, Feb 28, 2023 at 5:49 PM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
> >
> > The FF-A specification defines framework messages sent as direct
> > requests when certain events occurs. For instance when a VM (guest) is
> > created or destroyed. Only SPs which have subscribed to these events
> > will receive them. An SP can subscribe to these messages in its
> > partition properties.
> >
> > Adds a check that the SP supports the needed FF-A features
> > FFA_PARTITION_INFO_GET and FFA_RX_RELEASE.
> >
> > The partition properties of each SP is retrieved with
> > FFA_PARTITION_INFO_GET which returns the information in our RX buffer.
> > Using FFA_PARTITION_INFO_GET changes the owner of the RX buffer to the
> > caller (us), so once we're done with the buffer it must be released
> > using FFA_RX_RELEASE before another call can be made.
> >
> > Signed-off-by: Jens Wiklander
> > ---
> > xen/arch/arm/tee/ffa.c | 191 -
> > 1 file changed, 190 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> > index 07dd5c36d54b..f1b014b6c7f4 100644
> > --- a/xen/arch/arm/tee/ffa.c
> > +++ b/xen/arch/arm/tee/ffa.c
> > @@ -140,6 +140,14 @@
> > #define FFA_MSG_SEND0x846EU
> > #define FFA_MSG_POLL0x846AU
> >
> > +/* Partition information descriptor */
> > +struct ffa_partition_info_1_1 {
> > +uint16_t id;
> > +uint16_t execution_context;
> > +uint32_t partition_properties;
> > +uint8_t uuid[16];
> > +};
> > +
> > struct ffa_ctx {
> > uint32_t guest_vers;
> > bool interrupted;
> > @@ -148,6 +156,12 @@ struct ffa_ctx {
> > /* Negotiated FF-A version to use with the SPMC */
> > static uint32_t ffa_version __ro_after_init;
> >
> > +/* SPs subscribing to VM_CREATE and VM_DESTROYED events */
> > +static uint16_t *subscr_vm_created __read_mostly;
> > +static unsigned int subscr_vm_created_count __read_mostly;
>
> In the spec the number is returned in w2 so you should utse uint32_t here.
I don't understand. This value is increased for each SP which has the
property set in the Partition information descriptor.
>
> > +static uint16_t *subscr_vm_destroyed __read_mostly;
> > +static unsigned int subscr_vm_destroyed_count __read_mostly;
>
> Same here
>
> > +
> > /*
> > * Our rx/tx buffers shared with the SPMC.
> > *
> > @@ -237,6 +251,72 @@ static int32_t ffa_rxtx_map(register_t tx_addr,
> > register_t rx_addr,
> > return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
> > }
> >
> > +static int32_t ffa_partition_info_get(uint32_t w1, uint32_t w2, uint32_t
> > w3,
> > + uint32_t w4, uint32_t w5,
> > + uint32_t *count)
> > +{
> > +const struct arm_smccc_1_2_regs arg = {
> > +.a0 = FFA_PARTITION_INFO_GET,
> > +.a1 = w1,
> > +.a2 = w2,
> > +.a3 = w3,
> > +.a4 = w4,
> > +.a5 = w5,
> > +};
> > +struct arm_smccc_1_2_regs resp;
> > +uint32_t ret;
> > +
> > +arm_smccc_1_2_smc(&arg, &resp);
> > +
> > +ret = get_ffa_ret_code(&resp);
> > +if ( !ret )
> > +*count = resp.a2;
> > +
> > +return ret;
> > +}
> > +
> > +static int32_t ffa_rx_release(void)
> > +{
> > +return ffa_simple_call(FFA_RX_RELEASE, 0, 0, 0, 0);
> > +}
> > +
> > +static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
> > + uint8_t msg)
>
> This function is actually only useable to send framework created/destroyed
> messages so the function name should be adapted to be less generic.
>
> ffa_send_vm_events ?
>
> unless you want to modify it later to send more framework messages ?
That was the plan, but that may never happen. I'll rename it to
ffa_send_vm_event() since we're only sending one event at a time.
>
> > +{
> > +uint32_t exp_resp = FFA_MSG_FLAG_FRAMEWORK;
> > +int32_t res;
> > +
> > +if ( msg == FFA_MSG_SEND_VM_CREATED )
> > +exp_resp |= FFA_MSG_RESP_VM_CREATED;
> > +else if ( msg == FFA_MSG_SEND_VM_DESTROYED )
> > +exp_resp |= FFA_MSG_RESP_VM_DESTROYED;
> > +else
> > +return FFA_RET_INVALID_PARAMETERS;
> > +
> > +do {
> > +const struct arm_smccc_1_2_regs arg = {
> > +.a0 = FFA_MSG_SEND_DIRECT_REQ_32,
> > +.a1 = sp_id,
> > +.a2 = FFA_MSG_FLAG_FRAMEWORK | msg,
> > +.a5 = vm_id,
> > +};
> > +struct arm_smccc_1_2_regs resp;
> > +
> > +arm_smccc_1_2_smc(&arg, &resp);
> > +if ( resp.a0 != FFA_MSG_SEND_DIRECT_RESP_32 || resp.a2 != exp_resp
> > )
> > +{
> > +/*
> > + * This is an invalid response, likely due to some error in the
> > + * implementation of the ABI.
> > + */
> > +return FFA_RET_INVALID_PARAMETERS;
> > +}
> > +
Re: [XEN PATCH v7 11/20] xen/arm: ffa: map SPMC rx/tx buffers
Hi Jens,
> On 1 Mar 2023, at 10:30, Jens Wiklander wrote:
>
> Hi Bertrand,
>
> On Tue, Feb 28, 2023 at 1:57 PM Bertrand Marquis
> wrote:
>>
>> Hi Jens,
>>
>>> On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
>>>
>>> When initializing the FF-A mediator map the RX and TX buffers shared with
>>> the SPMC.
>>>
>>> These buffer are later used to to transmit data that cannot be passed in
>>> registers only.
>>>
>>> Adds a check that the SP supports the needed FF-A features
>>> FFA_RXTX_MAP_64 / FFA_RXTX_MAP_32 and FFA_RXTX_UNMAP. In 64-bit mode we
>>> must use FFA_RXTX_MAP_64 since registers are used to transmit the
>>> physical addresses of the RX/TX buffers.
>>
>> Right now, FFA on 32bit would only work correctly if LPAE is not used and
>> only addresses
>> under 4G are used by Xen and by guests as addresses are transferred through
>> a single register.
>>
>> I think that we need for now to only enable FFA support on 64bit as the
>> limitations we
>> would need to enforce on 32bit are complex and the use case for FFA on 32bit
>> platforms
>> is not that obvious now.
>
> OK, I'll drop the #ifdef CONFIG_ARM_64 and #ifdef CONFIG_ARM_32 and
> instead depend on ARM_64 in Kconfig.
> If we ever want to use this on ARM_32 we'll have to go through
> everything anyway.
Yes this is the best solution for now.
And support.md patch is already saying already arm64.
>
>>
>>>
>>> Signed-off-by: Jens Wiklander
>>> ---
>>> xen/arch/arm/tee/ffa.c | 57 +-
>>> 1 file changed, 56 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>>> index a5d8a12635b6..07dd5c36d54b 100644
>>> --- a/xen/arch/arm/tee/ffa.c
>>> +++ b/xen/arch/arm/tee/ffa.c
>>> @@ -148,6 +148,15 @@ struct ffa_ctx {
>>> /* Negotiated FF-A version to use with the SPMC */
>>> static uint32_t ffa_version __ro_after_init;
>>>
>>> +/*
>>> + * Our rx/tx buffers shared with the SPMC.
>>> + *
>>> + * ffa_page_count is the number of pages used in each of these buffers.
>>> + */
>>> +static void *ffa_rx __read_mostly;
>>> +static void *ffa_tx __read_mostly;
>>> +static unsigned int ffa_page_count __read_mostly;
>>> +
>>> static bool ffa_get_version(uint32_t *vers)
>>> {
>>>const struct arm_smccc_1_2_regs arg = {
>>> @@ -217,6 +226,17 @@ static bool check_mandatory_feature(uint32_t id)
>>>return !ret;
>>> }
>>>
>>> +static int32_t ffa_rxtx_map(register_t tx_addr, register_t rx_addr,
>>> +uint32_t page_count)
>>
>> Using register_t type here is doing an implicit cast when called and on
>> 32bit this might later remove part of the address.
>> This function must take paddr_t as parameters.
>
> I'll change to paddr_t for rx/tx.
>
>>
>>> +{
>>> +uint32_t fid = FFA_RXTX_MAP_32;
>>> +
>>> +if ( IS_ENABLED(CONFIG_ARM_64) )
>>> +fid = FFA_RXTX_MAP_64;
>>> +
>>> +return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
>>
>> simple call might not be suitable on 32bits due to the conversion.
>> As said earlier, it would make more sense to disable FFA on 32bit and
>> put some comments/build_bug_on in the code in places where there
>> would be something to fix.
>
> I'm dropping the 32-bit support.
>
>>
>>> +}
>>> +
>>> static uint16_t get_vm_id(const struct domain *d)
>>> {
>>>/* +1 since 0 is reserved for the hypervisor in FF-A */
>>> @@ -384,6 +404,7 @@ static int ffa_relinquish_resources(struct domain *d)
>>> static bool ffa_probe(void)
>>> {
>>>uint32_t vers;
>>> +int e;
>>>unsigned int major_vers;
>>>unsigned int minor_vers;
>>>
>>> @@ -426,12 +447,46 @@ static bool ffa_probe(void)
>>>printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
>>> major_vers, minor_vers);
>>>
>>> -if ( !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
>>> +if (
>>> +#ifdef CONFIG_ARM_64
>>> + !check_mandatory_feature(FFA_RXTX_MAP_64) ||
>>> +#endif
>>> +#ifdef CONFIG_ARM_32
>>> + !check_mandatory_feature(FFA_RXTX_MAP_32) ||
>>> +#endif
>>> + !check_mandatory_feature(FFA_RXTX_UNMAP) ||
>>> + !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
>>>return false;
>>>
>>> +ffa_rx = alloc_xenheap_pages(0, 0);
>>> +if ( !ffa_rx )
>>> +return false;
>>> +
>>> +ffa_tx = alloc_xenheap_pages(0, 0);
>>> +if ( !ffa_tx )
>>> +goto err_free_ffa_rx;
>>> +
>>> +e = ffa_rxtx_map(__pa(ffa_tx), __pa(ffa_rx), 1);
>>> +if ( e )
>>> +{
>>> +printk(XENLOG_ERR "ffa: Failed to map rxtx: error %d\n", e);
>>> +goto err_free_ffa_tx;
>>> +}
>>> +ffa_page_count = 1;
>>
>> ffa_page_count is a constant here and is not used to do the allocation or
>> passed as parameter to ffa_rxtx_map.
>>
>> Do you expect this value to be modified ? how ?
>
> I expect this value to match how many FFA_PAGE_SIZE pages have been
> mapped for the RX/TX buffers. Currently, this is only 1 but will have
> to be changed later i
[PATCH v2 2/2] xen/misra: add entries to exclude-list.json
Add entries to the exclude-list.json for those files that need to be
excluded from the analysis scan.
Signed-off-by: Luca Fancellu
Signed-off-by: Michal Orzel
---
This list is originated from Michal's work here:
https://patchwork.kernel.org/project/xen-devel/patch/20221116092032.4423-1-michal.or...@amd.com/#25123099
and changed to adapt to this task.
Changes from v1:
- updated list with new files from Stefano
- add comment field for every entry (Jan)
---
---
docs/misra/exclude-list.json | 199 ++-
1 file changed, 198 insertions(+), 1 deletion(-)
diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
index 1fb0ac67747b..ca1e2dd678ff 100644
--- a/docs/misra/exclude-list.json
+++ b/docs/misra/exclude-list.json
@@ -1,4 +1,201 @@
{
"version": "1.0",
-"content": []
+"content": [
+{
+"rel_path": "arch/arm/arm64/cpufeature.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/arm/arm64/insn.c",
+"comment": "Imported on Linux, ignore for now"
+},
+{
+"rel_path": "arch/arm/arm64/lib/find_next_bit.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/acpi/boot.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/acpi/cpu_idle.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/acpi/cpufreq/cpufreq.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/acpi/cpuidle_menu.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/acpi/lib.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/amd.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/centaur.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/common.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/hygon.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/intel.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/intel_cacheinfo.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/mcheck/non-fatal.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/mtrr/*",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/cpu/mwait-idle.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/delay.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/dmi_scan.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/mpparse.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/srat.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/time.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "arch/x86/x86_64/mmconf-fam10h.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "common/bitmap.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "common/bunzip2.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "common/earlycpio.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "common/inflate.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "common/libfdt/*",
+"comment": "External library"
+},
+{
+"rel_path": "common/livepatch_elf.c",
+"comment": "Not in scope initially as it generates many violations
and it is not enabled in safety configurations"
+},
+{
+"rel_path": "common/lzo.c",
+"comment": "Imported from Linux, ignore for now"
+},
+{
+"rel_path": "com
[PATCH v2 0/2] xen/misra: create exclusion file list
This serie is introducing an exclusion list for the misra analysis, at the moment only cppcheck can benefit from it because it's the tool where we control every step and configuration. Exclude a file from the analysis is the last step we should do, but sometime it is unavoidable because we can't do direct changes to it to fix/deviate from findings. So we declare the file(s) here and we leave the burden of the misra compliance to the final user. Luca Fancellu (2): xen/cppcheck: add a way to exclude files from the scan xen/misra: add entries to exclude-list.json docs/misra/exclude-list.json | 201 ++ docs/misra/exclude-list.rst | 44 xen/scripts/xen_analysis/cppcheck_analysis.py | 20 +- .../xen_analysis/exclusion_file_list.py | 79 +++ 4 files changed, 342 insertions(+), 2 deletions(-) create mode 100644 docs/misra/exclude-list.json create mode 100644 docs/misra/exclude-list.rst create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py -- 2.34.1
[PATCH v2 1/2] xen/cppcheck: add a way to exclude files from the scan
Add a way to exclude files from the scan, in this way we can skip
some findings from the report on those files that Xen doesn't own.
To do that, introduce the exclude-list.json file under docs/misra,
this file will be populated with relative path to the files/folder
to be excluded.
Introduce a new module, exclusion_file_list.py, to deal with the
exclusion list file and use the new module in cppcheck_analysis.py
to take a list of excluded paths to update the suppression list of
cppcheck.
Modified --suppress flag for cppcheck tool to remove
unmatchedSuppression findings for those external file that are
listed but for example not built for the current architecture.
Add documentation for the file.
Signed-off-by: Luca Fancellu
---
Changes from v1:
- Indentation fixed (Jan)
---
---
docs/misra/exclude-list.json | 4 +
docs/misra/exclude-list.rst | 44 +++
xen/scripts/xen_analysis/cppcheck_analysis.py | 20 -
.../xen_analysis/exclusion_file_list.py | 79 +++
4 files changed, 145 insertions(+), 2 deletions(-)
create mode 100644 docs/misra/exclude-list.json
create mode 100644 docs/misra/exclude-list.rst
create mode 100644 xen/scripts/xen_analysis/exclusion_file_list.py
diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json
new file mode 100644
index ..1fb0ac67747b
--- /dev/null
+++ b/docs/misra/exclude-list.json
@@ -0,0 +1,4 @@
+{
+"version": "1.0",
+"content": []
+}
diff --git a/docs/misra/exclude-list.rst b/docs/misra/exclude-list.rst
new file mode 100644
index ..969539c46beb
--- /dev/null
+++ b/docs/misra/exclude-list.rst
@@ -0,0 +1,44 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Exclude file list for xen-analysis script
+=
+
+The code analysis is performed on the Xen codebase for both MISRA checkers and
+static analysis checkers, there are some files however that needs to be removed
+from the findings report because they are not owned by Xen and they must be
kept
+in sync with their origin (completely or even partially), hence we can't easily
+fix findings or deviate from them.
+
+For this reason the file docs/misra/exclude-list.json is used to exclude every
+entry listed in that file from the final report.
+Currently only the cppcheck analysis will use this file.
+
+Here is an example of the exclude-list.json file::
+
+|{
+|"version": "1.0",
+|"content": [
+|{
+|"rel_path": "relative/path/from/xen/file",
+|"comment": "This file is originated from ..."
+|},
+|{
+|"rel_path": "relative/path/from/xen/folder/*",
+|"comment": "This folder is a library"
+|},
+|{
+|"rel_path": "relative/path/from/xen/mem*.c",
+|"comment": "memcpy.c, memory.c and memcmp.c are from the outside"
+|}
+|]
+|}
+
+Here is an explanation of the fields inside an object of the "content" array:
+ - rel_path: it is the relative path from the Xen folder to the file/folder
that
+ needs to be excluded from the analysis report, it can contain a wildcard to
+ match more than one file/folder at the time. This field is mandatory.
+ - comment: an optional comment to explain why the file is removed from the
+ analysis.
+
+To ease the review and the modifications of the entries, they shall be listed
in
+alphabetical order referring to the rel_path field.
diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py
b/xen/scripts/xen_analysis/cppcheck_analysis.py
index cc1f403d315e..e385e2c8f54a 100644
--- a/xen/scripts/xen_analysis/cppcheck_analysis.py
+++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
@@ -1,7 +1,8 @@
#!/usr/bin/env python3
import os, re, shutil
-from . import settings, utils, cppcheck_report_utils
+from . import settings, utils, cppcheck_report_utils, exclusion_file_list
+from .exclusion_file_list import ExclusionFileListError
class GetMakeVarsPhaseError(Exception):
pass
@@ -50,6 +51,21 @@ def __generate_suppression_list(out_file):
# header for cppcheck
supplist_file.write("*:*generated/compiler-def.h\n")
+try:
+exclusion_file = \
+"{}/docs/misra/exclude-list.json".format(settings.repo_dir)
+exclusion_list = \
+
exclusion_file_list.load_exclusion_file_list(exclusion_file)
+except ExclusionFileListError as e:
+raise CppcheckDepsPhaseError(
+"Issue with reading file {}: {}".format(exclusion_file, e)
+)
+
+# Append excluded files to the suppression list, using * as error
id
+# to suppress every findings on that file
+for path in exclusion_list:
+supplist_file.write("*:{}\n".format(path))
+
for entry in headers:
filename = entry["file"]
try:
@@ -128,7 +144
Re: [PATCH v4 1/5] xen/riscv: introduce decode_cause() stuff
On Wed, 2023-03-01 at 11:41 +0200, Oleksii wrote: > On Mon, 2023-02-27 at 13:48 +0100, Jan Beulich wrote: > > On 24.02.2023 12:35, Oleksii Kurochko wrote: > > > --- a/xen/arch/riscv/traps.c > > > +++ b/xen/arch/riscv/traps.c > > > @@ -4,10 +4,95 @@ > > > * > > > * RISC-V Trap handlers > > > */ > > > + > > > +#include > > > > I couldn't spot anything in the file which would require this > > inclusion. > -EINVAL, -ENOENT are used in do_bug_frame() I missed that when wrote > previous answer on e-mail. Sorry for confusion. I resolved merge conflicts after rebase so should be remove as anything is used in traps.c from it ~ Oleksii
Re: [PATCH v4 1/5] xen/riscv: introduce decode_cause() stuff
On Mon, 2023-02-27 at 13:48 +0100, Jan Beulich wrote: > On 24.02.2023 12:35, Oleksii Kurochko wrote: > > --- a/xen/arch/riscv/traps.c > > +++ b/xen/arch/riscv/traps.c > > @@ -4,10 +4,95 @@ > > * > > * RISC-V Trap handlers > > */ > > + > > +#include > > I couldn't spot anything in the file which would require this > inclusion. -EINVAL, -ENOENT are used in do_bug_frame() I missed that when wrote previous answer on e-mail.
Re: [XEN PATCH] automation: Rework archlinux container
On Tue, Feb 28, 2023 at 06:48:26PM +, Andrew Cooper wrote: > On 28/02/2023 6:16 pm, Anthony PERARD wrote: > > Base image "archlinux/base" isn't available anymore, > > > > https://lists.archlinux.org/pipermail/arch-dev-public/2020-November/030181.html > > > > But instead of switching to archlinux/archlinux, we will use the > > official image from Docker. Main difference is that the first one is > > updated daily while the second is updated weekly. > > > > Also, as we will install the packages from "base-devel" anyway, switch > > to the "base-devel" tag. > > > > "dev86" package is now available from the main repo, no need for > > multilib repo anymore. > > > > It is recommended to initialise local signing key used by pacman, so > > let's do that. > > > > Replace "markdown" by "discount" as the former isn't available anymore > > and has been replaced by the later. > > > > Also, clean pacman's cache. > > > > Signed-off-by: Anthony PERARD > > Acked-by: Andrew Cooper Thanks, I've rebuild the container with my prototype: https://gitlab.com/xen-project/people/anthonyper/xen/-/pipelines/791711467 I'll send a patch series for this prototype soon. Cheers, -- Anthony PERARD
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
Hi,
On 01/03/2023 08:58, Oleksii wrote:
On Tue, 2023-02-28 at 17:48 +, Julien Grall wrote:
Hi Oleksii,
On 28/02/2023 15:09, Oleksii wrote:
On Sat, 2023-02-25 at 16:49 +, Julien Grall wrote:
Hi Oleksii,
On 24/02/2023 11:31, Oleksii Kurochko wrote:
The following changes were made:
* make GENERIC_BUG_FRAME mandatory for ARM
I have asked in patch #1 but will ask it again because I think
this
should be recorded in the commit message. Can you outline why it
is
not
possible to completely switch to the generic version?
I haven't tried to switch ARM too because of comment regarding 'i'
in
:
/*
* GCC will not allow to use "i" when PIE is enabled (Xen doesn't
set
the
* flag but instead rely on the default value from the compiler).
So
the
* easiest way to implement run_in_exception_handler() is to pass
the
to
* be called function in a fixed register.
*/
I would expect this comment to be valid for any arch. So if there is
a
need to deal with PIE, then we would not be able to use "i" in the
BUG
frame.
Note that we are now explicitly compiling Xen without PIE (see
Config.mk).
Then it looks like some architectures isn't expected to be compiled
with PIE. I mean that x86's bug.h is used 'i' and there is no any
alternative version in case of PIE.
If Xen should be compilable with PIE then we have to use ARM
implementation of bug.h everywhere. ( based on comment about 'i' with
PIE ).
The comment was added because until commit ecd6b9759919 ("Config.mk:
correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS") we would let
the compiler to decide whether PIE should be enabled.
Now we are forcing -fno-pie for Xen on any architecture (the flag is
added at the top-level in Config.mk).
Now I am totally confused...
My point was not about using the Arm implementation everywhere. My point
was that we disable even for Arm and therefore it is fine to use the
common version.
If in the future we need to support PIE in Xen (I am not aware of any
effort yet), then we could decide to update the common BUG framework.
But for now, I don't think this is something you need to care in your
series.
Cheers,
--
Julien Grall
Re: [XEN PATCH v7 11/20] xen/arm: ffa: map SPMC rx/tx buffers
Hi Bertrand,
On Tue, Feb 28, 2023 at 1:57 PM Bertrand Marquis
wrote:
>
> Hi Jens,
>
> > On 22 Feb 2023, at 16:33, Jens Wiklander wrote:
> >
> > When initializing the FF-A mediator map the RX and TX buffers shared with
> > the SPMC.
> >
> > These buffer are later used to to transmit data that cannot be passed in
> > registers only.
> >
> > Adds a check that the SP supports the needed FF-A features
> > FFA_RXTX_MAP_64 / FFA_RXTX_MAP_32 and FFA_RXTX_UNMAP. In 64-bit mode we
> > must use FFA_RXTX_MAP_64 since registers are used to transmit the
> > physical addresses of the RX/TX buffers.
>
> Right now, FFA on 32bit would only work correctly if LPAE is not used and
> only addresses
> under 4G are used by Xen and by guests as addresses are transferred through a
> single register.
>
> I think that we need for now to only enable FFA support on 64bit as the
> limitations we
> would need to enforce on 32bit are complex and the use case for FFA on 32bit
> platforms
> is not that obvious now.
OK, I'll drop the #ifdef CONFIG_ARM_64 and #ifdef CONFIG_ARM_32 and
instead depend on ARM_64 in Kconfig.
If we ever want to use this on ARM_32 we'll have to go through
everything anyway.
>
> >
> > Signed-off-by: Jens Wiklander
> > ---
> > xen/arch/arm/tee/ffa.c | 57 +-
> > 1 file changed, 56 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> > index a5d8a12635b6..07dd5c36d54b 100644
> > --- a/xen/arch/arm/tee/ffa.c
> > +++ b/xen/arch/arm/tee/ffa.c
> > @@ -148,6 +148,15 @@ struct ffa_ctx {
> > /* Negotiated FF-A version to use with the SPMC */
> > static uint32_t ffa_version __ro_after_init;
> >
> > +/*
> > + * Our rx/tx buffers shared with the SPMC.
> > + *
> > + * ffa_page_count is the number of pages used in each of these buffers.
> > + */
> > +static void *ffa_rx __read_mostly;
> > +static void *ffa_tx __read_mostly;
> > +static unsigned int ffa_page_count __read_mostly;
> > +
> > static bool ffa_get_version(uint32_t *vers)
> > {
> > const struct arm_smccc_1_2_regs arg = {
> > @@ -217,6 +226,17 @@ static bool check_mandatory_feature(uint32_t id)
> > return !ret;
> > }
> >
> > +static int32_t ffa_rxtx_map(register_t tx_addr, register_t rx_addr,
> > +uint32_t page_count)
>
> Using register_t type here is doing an implicit cast when called and on
> 32bit this might later remove part of the address.
> This function must take paddr_t as parameters.
I'll change to paddr_t for rx/tx.
>
> > +{
> > +uint32_t fid = FFA_RXTX_MAP_32;
> > +
> > +if ( IS_ENABLED(CONFIG_ARM_64) )
> > +fid = FFA_RXTX_MAP_64;
> > +
> > +return ffa_simple_call(fid, tx_addr, rx_addr, page_count, 0);
>
> simple call might not be suitable on 32bits due to the conversion.
> As said earlier, it would make more sense to disable FFA on 32bit and
> put some comments/build_bug_on in the code in places where there
> would be something to fix.
I'm dropping the 32-bit support.
>
> > +}
> > +
> > static uint16_t get_vm_id(const struct domain *d)
> > {
> > /* +1 since 0 is reserved for the hypervisor in FF-A */
> > @@ -384,6 +404,7 @@ static int ffa_relinquish_resources(struct domain *d)
> > static bool ffa_probe(void)
> > {
> > uint32_t vers;
> > +int e;
> > unsigned int major_vers;
> > unsigned int minor_vers;
> >
> > @@ -426,12 +447,46 @@ static bool ffa_probe(void)
> > printk(XENLOG_INFO "ARM FF-A Firmware version %u.%u\n",
> >major_vers, minor_vers);
> >
> > -if ( !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
> > +if (
> > +#ifdef CONFIG_ARM_64
> > + !check_mandatory_feature(FFA_RXTX_MAP_64) ||
> > +#endif
> > +#ifdef CONFIG_ARM_32
> > + !check_mandatory_feature(FFA_RXTX_MAP_32) ||
> > +#endif
> > + !check_mandatory_feature(FFA_RXTX_UNMAP) ||
> > + !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
> > return false;
> >
> > +ffa_rx = alloc_xenheap_pages(0, 0);
> > +if ( !ffa_rx )
> > +return false;
> > +
> > +ffa_tx = alloc_xenheap_pages(0, 0);
> > +if ( !ffa_tx )
> > +goto err_free_ffa_rx;
> > +
> > +e = ffa_rxtx_map(__pa(ffa_tx), __pa(ffa_rx), 1);
> > +if ( e )
> > +{
> > +printk(XENLOG_ERR "ffa: Failed to map rxtx: error %d\n", e);
> > +goto err_free_ffa_tx;
> > +}
> > +ffa_page_count = 1;
>
> ffa_page_count is a constant here and is not used to do the allocation or
> passed as parameter to ffa_rxtx_map.
>
> Do you expect this value to be modified ? how ?
I expect this value to match how many FFA_PAGE_SIZE pages have been
mapped for the RX/TX buffers. Currently, this is only 1 but will have
to be changed later if PAGE_SIZE in Xen or in the secure world is
larger than FFA_PAGE_SIZE. We may also later add support
configurations where RX/TX buffers aren't mapped.
>
> Please set it first and use it for allocation and as parameter to rxtx_map so
> that
Re: [PATCH 4/4] x86/hvm: create hvm_funcs for {svm,vmx}_{set,clear}_msr_intercept()
On 28.02.2023 21:14, Xenia Ragiadakou wrote:
>
> On 2/28/23 16:58, Jan Beulich wrote:
>> On 27.02.2023 08:56, Xenia Ragiadakou wrote:
>>> Add hvm_funcs hooks for {set,clear}_msr_intercept() for controlling the msr
>>> intercept in common vpmu code.
>>
>> What is this going to buy us? All calls ...
>>
>>> --- a/xen/arch/x86/cpu/vpmu_amd.c
>>> +++ b/xen/arch/x86/cpu/vpmu_amd.c
>>> @@ -165,9 +165,9 @@ static void amd_vpmu_set_msr_bitmap(struct vcpu *v)
>>>
>>> for ( i = 0; i < num_counters; i++ )
>>> {
>>> -svm_clear_msr_intercept(v, counters[i], MSR_RW);
>>> -svm_set_msr_intercept(v, ctrls[i], MSR_W);
>>> -svm_clear_msr_intercept(v, ctrls[i], MSR_R);
>>> +hvm_clear_msr_intercept(v, counters[i], MSR_RW);
>>> +hvm_set_msr_intercept(v, ctrls[i], MSR_W);
>>> +hvm_clear_msr_intercept(v, ctrls[i], MSR_R);
>>> }
>>>
>>> msr_bitmap_on(vpmu);
>>> @@ -180,8 +180,8 @@ static void amd_vpmu_unset_msr_bitmap(struct vcpu *v)
>>>
>>> for ( i = 0; i < num_counters; i++ )
>>> {
>>> -svm_set_msr_intercept(v, counters[i], MSR_RW);
>>> -svm_set_msr_intercept(v, ctrls[i], MSR_RW);
>>> +hvm_set_msr_intercept(v, counters[i], MSR_RW);
>>> +hvm_set_msr_intercept(v, ctrls[i], MSR_RW);
>>> }
>>>
>>> msr_bitmap_off(vpmu);
>>
>> ... here will got to the SVM functions anyway, while ...
>>
>>> --- a/xen/arch/x86/cpu/vpmu_intel.c
>>> +++ b/xen/arch/x86/cpu/vpmu_intel.c
>>> @@ -230,22 +230,22 @@ static void core2_vpmu_set_msr_bitmap(struct vcpu *v)
>>>
>>> /* Allow Read/Write PMU Counters MSR Directly. */
>>> for ( i = 0; i < fixed_pmc_cnt; i++ )
>>> -vmx_clear_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR0 + i, MSR_RW);
>>> +hvm_clear_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR0 + i, MSR_RW);
>>>
>>> for ( i = 0; i < arch_pmc_cnt; i++ )
>>> {
>>> -vmx_clear_msr_intercept(v, MSR_IA32_PERFCTR0 + i, MSR_RW);
>>> +hvm_clear_msr_intercept(v, MSR_IA32_PERFCTR0 + i, MSR_RW);
>>>
>>> if ( full_width_write )
>>> -vmx_clear_msr_intercept(v, MSR_IA32_A_PERFCTR0 + i, MSR_RW);
>>> +hvm_clear_msr_intercept(v, MSR_IA32_A_PERFCTR0 + i, MSR_RW);
>>> }
>>>
>>> /* Allow Read PMU Non-global Controls Directly. */
>>> for ( i = 0; i < arch_pmc_cnt; i++ )
>>> -vmx_clear_msr_intercept(v, MSR_P6_EVNTSEL(i), MSR_R);
>>> +hvm_clear_msr_intercept(v, MSR_P6_EVNTSEL(i), MSR_R);
>>>
>>> -vmx_clear_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR_CTRL, MSR_R);
>>> -vmx_clear_msr_intercept(v, MSR_IA32_DS_AREA, MSR_R);
>>> +hvm_clear_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR_CTRL, MSR_R);
>>> +hvm_clear_msr_intercept(v, MSR_IA32_DS_AREA, MSR_R);
>>> }
>>>
>>> static void core2_vpmu_unset_msr_bitmap(struct vcpu *v)
>>> @@ -253,21 +253,21 @@ static void core2_vpmu_unset_msr_bitmap(struct vcpu
>>> *v)
>>> unsigned int i;
>>>
>>> for ( i = 0; i < fixed_pmc_cnt; i++ )
>>> -vmx_set_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR0 + i, MSR_RW);
>>> +hvm_set_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR0 + i, MSR_RW);
>>>
>>> for ( i = 0; i < arch_pmc_cnt; i++ )
>>> {
>>> -vmx_set_msr_intercept(v, MSR_IA32_PERFCTR0 + i, MSR_RW);
>>> +hvm_set_msr_intercept(v, MSR_IA32_PERFCTR0 + i, MSR_RW);
>>>
>>> if ( full_width_write )
>>> -vmx_set_msr_intercept(v, MSR_IA32_A_PERFCTR0 + i, MSR_RW);
>>> +hvm_set_msr_intercept(v, MSR_IA32_A_PERFCTR0 + i, MSR_RW);
>>> }
>>>
>>> for ( i = 0; i < arch_pmc_cnt; i++ )
>>> -vmx_set_msr_intercept(v, MSR_P6_EVNTSEL(i), MSR_R);
>>> +hvm_set_msr_intercept(v, MSR_P6_EVNTSEL(i), MSR_R);
>>>
>>> -vmx_set_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR_CTRL, MSR_R);
>>> -vmx_set_msr_intercept(v, MSR_IA32_DS_AREA, MSR_R);
>>> +hvm_set_msr_intercept(v, MSR_CORE_PERF_FIXED_CTR_CTRL, MSR_R);
>>> +hvm_set_msr_intercept(v, MSR_IA32_DS_AREA, MSR_R);
>>> }
>>>
>>> static inline void __core2_vpmu_save(struct vcpu *v)
>>
>> ... all calls here will go to VMX'es. For making either vpmu_.c
>> build without that vendor's virtualization enabled, isn't it all it
>> takes to have ...
>>
>>> @@ -916,6 +932,18 @@ static inline void hvm_set_reg(struct vcpu *v,
>>> unsigned int reg, uint64_t val)
>>> ASSERT_UNREACHABLE();
>>> }
>>>
>>> +static inline void hvm_set_msr_intercept(struct vcpu *v, uint32_t msr,
>>> + int flags)
>>> +{
>>> +ASSERT_UNREACHABLE();
>>> +}
>>> +
>>> +static inline void hvm_clear_msr_intercept(struct vcpu *v, uint32_t msr,
>>> + int flags)
>>> +{
>>> +ASSERT_UNREACHABLE();
>>> +}
>>
>> ... respective SVM and VMX stubs in place instead?
>
> IMO it is more readable and they looked very good candidates for being
> abstracted because they are doi
Re: [ANNOUNCE] Call for agenda items for [EDIT] 2 March Community Call @ 1600 UTC
On Tue, Feb 28, 2023 at 7:16 AM Jan Beulich wrote: > On 27.02.2023 22:41, George Dunlap wrote: > > Note the following administrative conventions for the call: > > * Unless, agreed in the previous meeting otherwise, the call is on the > 1st > > Thursday of each month at 1600 British Time (either GMT or BST) > > Since nothing else was said, I suppose the title is off by one and it's > Thursday March 2nd? > Indeed, that should be Thursday, 2 March. (The timeanddate link, as well as the title of the cryptpad agenda, both say 2 March.). Sorry for the confusion. -George
Re: [PATCH v4 1/5] xen/riscv: introduce decode_cause() stuff
On Mon, 2023-02-27 at 13:48 +0100, Jan Beulich wrote:
> On 24.02.2023 12:35, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/traps.c
> > +++ b/xen/arch/riscv/traps.c
> > @@ -4,10 +4,95 @@
> > *
> > * RISC-V Trap handlers
> > */
> > +
> > +#include
>
> I couldn't spot anything in the file which would require this
> inclusion.
Sure. It can be removed.
>
> > +#include
> > +
> > +#include
> > +#include
> > #include
> > #include
> >
> > -void do_trap(struct cpu_user_regs *cpu_regs)
> > +static const char *decode_trap_cause(unsigned long cause)
> > +{
> > + static const char *const trap_causes[] = {
> > + [CAUSE_MISALIGNED_FETCH] = "Instruction Address
> > Misaligned",
> > + [CAUSE_FETCH_ACCESS] = "Instruction Access Fault",
> > + [CAUSE_ILLEGAL_INSTRUCTION] = "Illegal Instruction",
> > + [CAUSE_BREAKPOINT] = "Breakpoint",
> > + [CAUSE_MISALIGNED_LOAD] = "Load Address Misaligned",
> > + [CAUSE_LOAD_ACCESS] = "Load Access Fault",
> > + [CAUSE_MISALIGNED_STORE] = "Store/AMO Address Misaligned",
> > + [CAUSE_STORE_ACCESS] = "Store/AMO Access Fault",
> > + [CAUSE_USER_ECALL] = "Environment Call from U-Mode",
> > + [CAUSE_SUPERVISOR_ECALL] = "Environment Call from S-Mode",
> > + [CAUSE_MACHINE_ECALL] = "Environment Call from M-Mode",
> > + [CAUSE_FETCH_PAGE_FAULT] = "Instruction Page Fault",
> > + [CAUSE_LOAD_PAGE_FAULT] = "Load Page Fault",
> > + [CAUSE_STORE_PAGE_FAULT] = "Store/AMO Page Fault",
> > + [CAUSE_FETCH_GUEST_PAGE_FAULT] = "Instruction Guest Page
> > Fault",
> > + [CAUSE_LOAD_GUEST_PAGE_FAULT] = "Load Guest Page Fault",
> > + [CAUSE_VIRTUAL_INST_FAULT] = "Virtualized Instruction
> > Fault",
> > + [CAUSE_STORE_GUEST_PAGE_FAULT] = "Guest Store/AMO Page
> > Fault",
> > + };
> > +
> > + if ( cause < ARRAY_SIZE(trap_causes) && trap_causes[cause] )
> > + return trap_causes[cause];
> > + return "UNKNOWN";
> > +}
> > +
> > +const char *decode_reserved_interrupt_cause(unsigned long
> > irq_cause)
>
> For any non-static function that you add you will need a declaration
> in a header, which the defining C file then includes. I understand
> that during initial bringup functions without (external) callers may
> want to (temporarily) exist, but briefly clarifying what the future
> expectation regarding external uses might help. Any function that's
> not expected to gain external callers should be static.
Thanks. For explanation. I'll make them static.
~ Oleksii
[PATCH] automation: Add missing and drop obsoleted aliases from containerize
Add missing aliases for:
- debian:unstable-cppcheck
- debian:unstable-arm64v8-arm32-gcc
- ubuntu:bionic
Remove aliases for no longer used containers:
- centos:7.2
- debian:unstable-arm32-gcc
Modify docs to refer to CentOS 7 instead of 7.2 not to create confusion.
Signed-off-by: Michal Orzel
---
Open questions related to the CI cleanup (@Andrew, @Anthony):
- Why do we keep suse:sles11sp4 dockerfile?
- Why do we keep jessie dockefiles?
---
automation/build/README.md | 4 ++--
automation/scripts/containerize | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/automation/build/README.md b/automation/build/README.md
index 4cc1acb6b41c..2d07cafe0eaa 100644
--- a/automation/build/README.md
+++ b/automation/build/README.md
@@ -44,10 +44,10 @@ understands.
DOCKER_CMD=podman ./automation/scripts/containerize make
```
-- CONTAINER: This overrides the container to use. For CentOS 7.2, use:
+- CONTAINER: This overrides the container to use. For CentOS 7, use:
```
- CONTAINER=centos72 ./automation/scripts/containerize make
+ CONTAINER=centos7 ./automation/scripts/containerize make
```
- CONTAINER_PATH: This overrides the path that will be available under the
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index 9b1a302d0565..5476ff0ea10d 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -29,7 +29,6 @@ case "_${CONTAINER}" in
_archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
_riscv64) CONTAINER="${BASE}/archlinux:current-riscv64" ;;
_centos7) CONTAINER="${BASE}/centos:7" ;;
-_centos72) CONTAINER="${BASE}/centos:7.2" ;;
_fedora) CONTAINER="${BASE}/fedora:29";;
_focal) CONTAINER="${BASE}/ubuntu:focal" ;;
_jessie) CONTAINER="${BASE}/debian:jessie" ;;
@@ -39,8 +38,10 @@ case "_${CONTAINER}" in
_buster-gcc-ibt) CONTAINER="${BASE}/debian:buster-gcc-ibt" ;;
_unstable|_) CONTAINER="${BASE}/debian:unstable" ;;
_unstable-i386) CONTAINER="${BASE}/debian:unstable-i386" ;;
-_unstable-arm32-gcc) CONTAINER="${BASE}/debian:unstable-arm32-gcc" ;;
+_unstable-arm64v8-arm32-gcc)
CONTAINER="${BASE}/debian:unstable-arm64v8-arm32-gcc" ;;
_unstable-arm64v8) CONTAINER="${BASE}/debian:unstable-arm64v8" ;;
+_unstable-cppcheck) CONTAINER="${BASE}/debian:unstable-cppcheck" ;;
+_bionic) CONTAINER="${BASE}/ubuntu:bionic" ;;
_trusty) CONTAINER="${BASE}/ubuntu:trusty" ;;
_xenial) CONTAINER="${BASE}/ubuntu:xenial" ;;
_opensuse-leap|_leap) CONTAINER="${BASE}/suse:opensuse-leap" ;;
--
2.25.1
Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h
On Tue, 2023-02-28 at 17:48 +, Julien Grall wrote: > Hi Oleksii, > > On 28/02/2023 15:09, Oleksii wrote: > > On Sat, 2023-02-25 at 16:49 +, Julien Grall wrote: > > > Hi Oleksii, > > > > > > On 24/02/2023 11:31, Oleksii Kurochko wrote: > > > > The following changes were made: > > > > * make GENERIC_BUG_FRAME mandatory for ARM > > > > > > I have asked in patch #1 but will ask it again because I think > > > this > > > should be recorded in the commit message. Can you outline why it > > > is > > > not > > > possible to completely switch to the generic version? > > I haven't tried to switch ARM too because of comment regarding 'i' > > in > > : > > /* > > * GCC will not allow to use "i" when PIE is enabled (Xen doesn't > > set > > the > > * flag but instead rely on the default value from the compiler). > > So > > the > > * easiest way to implement run_in_exception_handler() is to pass > > the > > to > > * be called function in a fixed register. > > */ > > I would expect this comment to be valid for any arch. So if there is > a > need to deal with PIE, then we would not be able to use "i" in the > BUG > frame. > > Note that we are now explicitly compiling Xen without PIE (see > Config.mk). Then it looks like some architectures isn't expected to be compiled with PIE. I mean that x86's bug.h is used 'i' and there is no any alternative version in case of PIE. If Xen should be compilable with PIE then we have to use ARM implementation of bug.h everywhere. ( based on comment about 'i' with PIE ). Now I am totally confused... ~ Oleksii
Re: [PATCH v7 13/41] mm: Make pte_mkwrite() take a VMA
On 01.03.23 08:03, Christophe Leroy wrote:
Le 27/02/2023 à 23:29, Rick Edgecombe a écrit :
The x86 Control-flow Enforcement Technology (CET) feature includes a new
type of memory called shadow stack. This shadow stack memory has some
unusual properties, which requires some core mm changes to function
properly.
One of these unusual properties is that shadow stack memory is writable,
but only in limited ways. These limits are applied via a specific PTE
bit combination. Nevertheless, the memory is writable, and core mm code
will need to apply the writable permissions in the typical paths that
call pte_mkwrite().
In addition to VM_WRITE, the shadow stack VMA's will have a flag denoting
that they are special shadow stack flavor of writable memory. So make
pte_mkwrite() take a VMA, so that the x86 implementation of it can know to
create regular writable memory or shadow stack memory.
Apply the same changes for pmd_mkwrite() and huge_pte_mkwrite().
I'm not sure it is a good idea to add a second argument to
pte_mkwrite(). All pte_mk() only take a pte and nothing else.
We touched on this in previous revisions and so far there was no strong
push back. This turned out to be cleaner and easier than the
alternatives we evaluated.
pte_modify(), for example, takes another argument. Sure, we could try
thinking about passing something else than a VMA to identify the
writability type, but I am not convinced that will look particularly better.
I think you should do the same as commit d9ed9faac283 ("mm: add new
arch_make_huge_pte() method for tile support")
We already have 3 architectures intending to support shadow stacks in
one way or the other. Replacing all pte_mkwrite() with
arch_pte_mkwrite() doesn't sound particularly appealing to me.
--
Thanks,
David / dhildenb
[linux-linus test] 178822: regressions - trouble: fail/pass/starved
flight 178822 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/178822/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-freebsd12-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-qemuu-nested-intel 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-ws16-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start fail REGR. vs. 178042 test-amd64-amd64-xl-pvshim8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-vhd 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-intel 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-win7-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-win7-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-credit1 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-amd64 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-shadow8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-pvhv2-amd 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-qemuu-nested-amd 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-ws16-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-freebsd11-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-raw 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-pygrub 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-pair12 xen-boot/src_hostfail REGR. vs. 178042 test-amd64-amd64-pair13 xen-boot/dst_hostfail REGR. vs. 178042 test-amd64-amd64-examine-uefi 8 reboot fail REGR. vs. 178042 test-amd64-amd64-libvirt-qcow2 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt-pair 12 xen-boot/src_host fail REGR. vs. 178042 test-amd64-amd64-libvirt-pair 13 xen-boot/dst_host fail REGR. vs. 178042 test-amd64-amd64-libvirt-xsm 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-xl-qemut-debianhvm-amd64 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 178042 test-arm64-arm64-xl-credit1 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-libvirt-xsm 17 guest-stop fail REGR. vs. 178042 test-amd64-amd64-xl-credit2 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-libvirt 8 xen-boot fail REGR. vs. 178042 test-amd64-coresched-amd64-xl 8 xen-bootfail REGR. vs. 178042 test-arm64-arm64-xl-thunderx 18 guest-start/debian.repeat fail REGR. vs. 178042 test-arm64-arm64-xl-xsm 18 guest-start/debian.repeat fail REGR. vs. 178042 test-arm64-arm64-xl-credit2 18 guest-start/debian.repeat fail REGR. vs. 178042 test-amd64-amd64-xl-qemuu-ovmf-amd64 8 xen-boot fail REGR. vs. 178042 test-amd64-amd64-examine 8 reboot fail REGR. vs. 178042 test-amd64-amd64-examine-bios 8 reboot fail REGR. vs. 178042 test-amd64-amd64-xl-multivcpu 8 xen-bootfail REGR. vs. 178042 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 178042 test-arm64-arm64-xl-vhd 12 debian-di-installfail REGR. vs. 178042 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 178042 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 8 xen-boot fail REGR. vs. 178042 Tests which did not succeed, but are not blocking: 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-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-
