Re: [PATCH 1/4] amd-vi: fix IVMD memory type checks
On Thursday, February 1st, 2024 at 18:01, Roger Pau Monne
wrote:
> The current code that parses the IVMD blocks is relaxed with regard to the
> restriction that such unity regions should always fall into memory ranges
> marked as reserved in the memory map.
>
> However the type checks for the IVMD addresses are inverted, and as a result
> IVMD ranges falling into RAM areas are accepted. Note that having such ranges
> in the first place is a firmware bug, as IVMD should always fall into reserved
> ranges.
>
> Fixes: ed6c77ebf0c1 ('AMD/IOMMU: check / convert IVMD ranges for being / to
> be reserved')
> Signed-off-by: Roger Pau Monné roger@citrix.com
>
> ---
> Cc: o...@proton.me
> ---
> xen/drivers/passthrough/amd/iommu_acpi.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c
> b/xen/drivers/passthrough/amd/iommu_acpi.c
> index 2e3b83014beb..ca70f4f3ae2c 100644
> --- a/xen/drivers/passthrough/amd/iommu_acpi.c
> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c
> @@ -426,9 +426,14 @@ static int __init parse_ivmd_block(const struct
> acpi_ivrs_memory ivmd_block)
> return -EIO;
> }
>
> - / Types which won't be handed out are considered good enough. /
> - if ( !(type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI |
> - RAM_TYPE_UNUSABLE)) )
> + /
> + * Types which aren't RAM are considered good enough.
> + * Note that a page being partially RESERVED, ACPI or UNUSABLE will
> + * force Xen into assuming the whole page as having that type in
> + * practice.
> + */
> + if ( type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI |
> + RAM_TYPE_UNUSABLE) )
> continue;
>
> AMD_IOMMU_ERROR("IVMD: page at %lx can't be converted\n", addr);
I tested the patch and it resolves the issue.
It eliminates the boot IVMD error message.
AMD-Vi is enabled and pci passthrough works.
Tested-by: oxjo
[xen-4.17-testing test] 184564: tolerable trouble: fail/pass/starved - PUSHED
flight 184564 xen-4.17-testing real [real] flight 184574 xen-4.17-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/184564/ http://logs.test-lab.xenproject.org/osstest/logs/184574/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl-shadow 22 guest-start/debian.repeat fail pass in 184574-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184531 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 184531 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 184531 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184531 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184531 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184531 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 184531 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184531 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 184531 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184531 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184531 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184531 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-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-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 3 hosts-allocate starved n/a version targeted for testing: xen 091466ba55d1e2e75738f751818ace2e3ed08ccf baseline version: xen 6b1864afc14d484cdbc9754ce3172ac3dc189846 Last test of basis 184531 2024-01-30 14:07:13 Z3 days Testing same since 184564 2024-02-02 07:07:04 Z0 days1
[linux-6.1 test] 184563: regressions - FAIL
flight 184563 linux-6.1 real [real] flight 184569 linux-6.1 real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/184563/ http://logs.test-lab.xenproject.org/osstest/logs/184569/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail REGR. vs. 184549 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184549 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184549 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184549 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184549 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184549 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184549 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184549 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184549 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass version targeted for testing: linuxe5c3b988b827c76f52d0f62343e863b9133a0cd2 baseline version: linux8fd7f44624538675abadc73f5a44e95016964d22 Last test of basis 184549 2024-02-01 11:13:03 Z1 days Testing same since 184563 2024-02-02 05:26:08 Z0 days1 attempts 486 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass
Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
On Fri, 2 Feb 2024, Julien Grall wrote: > On 01/02/2024 16:17, John Ernberg wrote: > > On 2/1/24 13:20, Julien Grall wrote: > > > > > > > > > On 31/01/2024 15:32, John Ernberg wrote: > > > > Hi Julien, > > > > > > Hi John, > > > > > > > On 1/31/24 13:22, Julien Grall wrote: > > > > > Hi, > > > > > > > > > > On 31/01/2024 11:50, John Ernberg wrote: > > > > > > When using Linux for dom0 there are a bunch of drivers that need to > > > > > > do > > > > > > SMC > > > > > > SIP calls into the PSCI provider to enable certain hardware bits > > > > > > like the > > > > > > watchdog. > > > > > > > > > > Do you know which protocol this is under the hood. Is this SCMI? > > > > > > > > I think I confused myself here when I wrote the commit log. > > > > > > > > The EL3 code in our case is ATF, and it does not appear to be SCMI, nor > > > > PSCI. The register usage of these SMC SIP calls are as follows: > > > > a0 - service > > > > a1 - function > > > > a2-a7 - args > > > > > > > > In ATF the handler is declared as a runtime service. > > > > > > > > Would the appropriate commmit message here be something along the lines > > > > of below? > > > > """ > > > > When using Linux for dom0 there are a bunch of drivers that need to > > > > do SMC > > > > SIP calls into the firmware to enable certain hardware bits like the > > > > watchdog. > > > > """ > > > > > > It reads better thanks. > > > > > > [...] > > > > > > > > But even if we restrict to dom0, have you checked that none of the > > > > > SMCs > > > > > use buffers? > > > > I haven't found any such instances in the Linux kernel where a buffer is > > > > used. Adding a call filtering like suggested below additions of such > > > > functions can be discovered and adapted for if they would show up later. > > > > > > > > > > Rather than providing a blanket forward, to me it sounds more like you > > > > > want to provide an allowlist of the SMCs. This is more futureproof and > > > > > avoid the risk to expose unsafe SMCs to any domain. > > > > > > > > > > For an example, you can have a look at the EEMI mediator for Xilinx. > > > > > > > > Ack. Do you prefer to see only on SMCCC service level or also on > > > > function level? (a1 register, per description earlier) > > > > > > I am not sure. It will depend on whether it is correct to expose *all* > > > the functions within a service level and they have the same format. > > > > > > If you can't guarantee that, then you will most likely need to allowlist > > > at the function level. > > > > > > Also, do you have a spec in hand that would help to understand which > > > service/function is implemented via those SMCs? > > > > I don't have the spec unfortunately, but I will add a filter on both > > service and function for V2 and we'll take it from there. > > @Peng, do you have any specification you could share? How stable is the > interface? Just to add some context to make the reason for the question clearer, if we have a specification we could check the patch for correctness. Without it, it is difficult to know if it is doing the right thing. The other aspect is about expectation of forward and backward compatibility. Can we guarantee that the next version of Xen and the one after it will still work against this interface? If not, can we check for the version of the interface before continuing? If not, can we at least document that the interface is only known-to-work with specific firmware versions? This is basically just to provide the right expectations to users and ideally to prevent a future version of Xen to break on boot silently without information. If we don't have a spec and we don't know if the interface is stable, I think we should try to detect the version of the interface and print a warning in Xen if it not a known version.
[PATCH v13 11/14] vpci: add initial support for virtual PCI bus topology
From: Oleksandr Andrushchenko
Assign SBDF to the PCI devices being passed through with bus 0.
The resulting topology is where PCIe devices reside on the bus 0 of the
root complex itself (embedded endpoints).
This implementation is limited to 32 devices which are allowed on
a single PCI bus.
Please note, that at the moment only function 0 of a multifunction
device can be passed through.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
In v13:
- s/depends on/select/ in Kconfig
- check pdev->sbdf.fn instead of two booleans in add_virtual_device()
- comment #endifs in sched.h
- clarify comment about limits in vpci.h with seg/bus limit
In v11:
- Fixed code formatting
- Removed bogus write_unlock() call
- Fixed type for new_dev_number
In v10:
- Removed ASSERT(pcidevs_locked())
- Removed redundant code (local sbdf variable, clearing sbdf during
device removal, etc)
- Added __maybe_unused attribute to "out:" label
- Introduced HAS_VPCI_GUEST_SUPPORT Kconfig option, as this is the
first patch where it is used (previously was in "vpci: add hooks for
PCI device assign/de-assign")
In v9:
- Lock in add_virtual_device() replaced with ASSERT (thanks, Stewart)
In v8:
- Added write lock in add_virtual_device
Since v6:
- re-work wrt new locking scheme
- OT: add ASSERT(pcidevs_write_locked()); to add_virtual_device()
Since v5:
- s/vpci_add_virtual_device/add_virtual_device and make it static
- call add_virtual_device from vpci_assign_device and do not use
REGISTER_VPCI_INIT machinery
- add pcidevs_locked ASSERT
- use DECLARE_BITMAP for vpci_dev_assigned_map
Since v4:
- moved and re-worked guest sbdf initializers
- s/set_bit/__set_bit
- s/clear_bit/__clear_bit
- minor comment fix s/Virtual/Guest/
- added VPCI_MAX_VIRT_DEV constant (PCI_SLOT(~0) + 1) which will be used
later for counting the number of MMIO handlers required for a guest
(Julien)
Since v3:
- make use of VPCI_INIT
- moved all new code to vpci.c which belongs to it
- changed open-coded 31 to PCI_SLOT(~0)
- added comments and code to reject multifunction devices with
functions other than 0
- updated comment about vpci_dev_next and made it unsigned int
- implement roll back in case of error while assigning/deassigning devices
- s/dom%pd/%pd
Since v2:
- remove casts that are (a) malformed and (b) unnecessary
- add new line for better readability
- remove CONFIG_HAS_VPCI_GUEST_SUPPORT ifdef's as the relevant vPCI
functions are now completely gated with this config
- gate common code with CONFIG_HAS_VPCI_GUEST_SUPPORT
New in v2
---
xen/drivers/Kconfig | 4 +++
xen/drivers/vpci/vpci.c | 57 +
xen/include/xen/sched.h | 10 +++-
xen/include/xen/vpci.h | 12 +
4 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/Kconfig b/xen/drivers/Kconfig
index db94393f47a6..20050e9bb8b3 100644
--- a/xen/drivers/Kconfig
+++ b/xen/drivers/Kconfig
@@ -15,4 +15,8 @@ source "drivers/video/Kconfig"
config HAS_VPCI
bool
+config HAS_VPCI_GUEST_SUPPORT
+ bool
+ select HAS_VPCI
+
endmenu
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 260b72875ee1..3cd142068f4e 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -40,6 +40,49 @@ extern vpci_register_init_t *const __start_vpci_array[];
extern vpci_register_init_t *const __end_vpci_array[];
#define NUM_VPCI_INIT (__end_vpci_array - __start_vpci_array)
+#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+static int add_virtual_device(struct pci_dev *pdev)
+{
+struct domain *d = pdev->domain;
+unsigned int new_dev_number;
+
+if ( is_hardware_domain(d) )
+return 0;
+
+ASSERT(rw_is_write_locked(>domain->pci_lock));
+
+/*
+ * Each PCI bus supports 32 devices/slots at max or up to 256 when
+ * there are multi-function ones which are not yet supported.
+ */
+if ( pdev->sbdf.fn )
+{
+gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
+ >sbdf);
+return -EOPNOTSUPP;
+}
+new_dev_number = find_first_zero_bit(d->vpci_dev_assigned_map,
+ VPCI_MAX_VIRT_DEV);
+if ( new_dev_number == VPCI_MAX_VIRT_DEV )
+return -ENOSPC;
+
+__set_bit(new_dev_number, >vpci_dev_assigned_map);
+
+/*
+ * Both segment and bus number are 0:
+ * - we emulate a single host bridge for the guest, e.g. segment 0
+ * - with bus 0 the virtual devices are seen as embedded
+ *endpoints behind the root complex
+ *
+ * TODO: add support for multi-function devices.
+ */
+pdev->vpci->guest_sbdf = PCI_SBDF(0, 0, new_dev_number, 0);
+
+return 0;
+}
+
+#endif /* CONFIG_HAS_VPCI_GUEST_SUPPORT */
+
void vpci_deassign_device(struct pci_dev *pdev)
{
unsigned int i;
@@ -49,6 +92,12 @@ void vpci_deassign_device(struct pci_dev *pdev)
if ( !has_vpci(pdev->domain) ||
[PATCH v13 07/14] rangeset: add rangeset_purge() function
From: Volodymyr Babchuk
This function can be used when user wants to remove all rangeset
entries but do not want to destroy rangeset itself.
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
Acked-by: Jan Beulich
---
Changes in v13:
- Added Jan's A-b
Changes in v12:
- s/rangeset_empty/rangeset_purge/
Changes in v11:
- Now the function only empties rangeset, without removing it from
domain's list
Changes in v10:
- New in v10. The function is used in "vpci/header: handle p2m range sets per
BAR"
---
xen/common/rangeset.c | 16
xen/include/xen/rangeset.h | 3 ++-
2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index 0ccd53caac52..b75590f90744 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -448,11 +448,20 @@ struct rangeset *rangeset_new(
return r;
}
-void rangeset_destroy(
-struct rangeset *r)
+void rangeset_purge(struct rangeset *r)
{
struct range *x;
+if ( r == NULL )
+return;
+
+while ( (x = first_range(r)) != NULL )
+destroy_range(r, x);
+}
+
+void rangeset_destroy(
+struct rangeset *r)
+{
if ( r == NULL )
return;
@@ -463,8 +472,7 @@ void rangeset_destroy(
spin_unlock(>domain->rangesets_lock);
}
-while ( (x = first_range(r)) != NULL )
-destroy_range(r, x);
+rangeset_purge(r);
xfree(r);
}
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index 87bd956962b5..96c918082501 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -56,7 +56,7 @@ void rangeset_limit(
bool __must_check rangeset_is_empty(
const struct rangeset *r);
-/* Add/claim/remove/query a numeric range. */
+/* Add/claim/remove/query/purge a numeric range. */
int __must_check rangeset_add_range(
struct rangeset *r, unsigned long s, unsigned long e);
int __must_check rangeset_claim_range(struct rangeset *r, unsigned long size,
@@ -70,6 +70,7 @@ bool __must_check rangeset_overlaps_range(
int rangeset_report_ranges(
struct rangeset *r, unsigned long s, unsigned long e,
int (*cb)(unsigned long s, unsigned long e, void *data), void *ctxt);
+void rangeset_purge(struct rangeset *r);
/*
* Note that the consume function can return an error value apart from
--
2.43.0
[PATCH v13 12/14] xen/arm: translate virtual PCI bus topology for guests
From: Oleksandr Andrushchenko
There are three originators for the PCI configuration space access:
1. The domain that owns physical host bridge: MMIO handlers are
there so we can update vPCI register handlers with the values
written by the hardware domain, e.g. physical view of the registers
vs guest's view on the configuration space.
2. Guest access to the passed through PCI devices: we need to properly
map virtual bus topology to the physical one, e.g. pass the configuration
space access to the corresponding physical devices.
3. Emulated host PCI bridge access. It doesn't exist in the physical
topology, e.g. it can't be mapped to some physical host bridge.
So, all access to the host bridge itself needs to be trapped and
emulated.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
In v11:
- Fixed format issues
- Added ASSERT_UNREACHABLE() to the dummy implementation of
vpci_translate_virtual_device()
- Moved variable in vpci_sbdf_from_gpa(), now it is easier to follow
the logic in the function
Since v9:
- Commend about required lock replaced with ASSERT()
- Style fixes
- call to vpci_translate_virtual_device folded into vpci_sbdf_from_gpa
Since v8:
- locks moved out of vpci_translate_virtual_device()
Since v6:
- add pcidevs locking to vpci_translate_virtual_device
- update wrt to the new locking scheme
Since v5:
- add vpci_translate_virtual_device for #ifndef CONFIG_HAS_VPCI_GUEST_SUPPORT
case to simplify ifdefery
- add ASSERT(!is_hardware_domain(d)); to vpci_translate_virtual_device
- reset output register on failed virtual SBDF translation
Since v4:
- indentation fixes
- constify struct domain
- updated commit message
- updates to the new locking scheme (pdev->vpci_lock)
Since v3:
- revisit locking
- move code to vpci.c
Since v2:
- pass struct domain instead of struct vcpu
- constify arguments where possible
- gate relevant code with CONFIG_HAS_VPCI_GUEST_SUPPORT
New in v2
---
xen/arch/arm/vpci.c | 47 +++--
xen/drivers/vpci/vpci.c | 24 +
xen/include/xen/vpci.h | 12 +++
3 files changed, 72 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 3bc4bb55082a..7a6a0017d132 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -7,31 +7,51 @@
#include
-static pci_sbdf_t vpci_sbdf_from_gpa(const struct pci_host_bridge *bridge,
- paddr_t gpa)
+static bool vpci_sbdf_from_gpa(struct domain *d,
+ const struct pci_host_bridge *bridge,
+ paddr_t gpa, pci_sbdf_t *sbdf)
{
-pci_sbdf_t sbdf;
+bool translated = true;
+
+ASSERT(sbdf);
if ( bridge )
{
-sbdf.sbdf = VPCI_ECAM_BDF(gpa - bridge->cfg->phys_addr);
-sbdf.seg = bridge->segment;
-sbdf.bus += bridge->cfg->busn_start;
+sbdf->sbdf = VPCI_ECAM_BDF(gpa - bridge->cfg->phys_addr);
+sbdf->seg = bridge->segment;
+sbdf->bus += bridge->cfg->busn_start;
}
else
-sbdf.sbdf = VPCI_ECAM_BDF(gpa - GUEST_VPCI_ECAM_BASE);
+{
+/*
+ * For the passed through devices we need to map their virtual SBDF
+ * to the physical PCI device being passed through.
+ */
+sbdf->sbdf = VPCI_ECAM_BDF(gpa - GUEST_VPCI_ECAM_BASE);
+read_lock(>pci_lock);
+translated = vpci_translate_virtual_device(d, sbdf);
+read_unlock(>pci_lock);
+}
-return sbdf;
+return translated;
}
static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
register_t *r, void *p)
{
struct pci_host_bridge *bridge = p;
-pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
+pci_sbdf_t sbdf;
/* data is needed to prevent a pointer cast on 32bit */
unsigned long data;
+ASSERT(!bridge == !is_hardware_domain(v->domain));
+
+if ( !vpci_sbdf_from_gpa(v->domain, bridge, info->gpa, ) )
+{
+*r = ~0UL;
+return 1;
+}
+
if ( vpci_ecam_read(sbdf, ECAM_REG_OFFSET(info->gpa),
1U << info->dabt.size, ) )
{
@@ -39,7 +59,7 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
return 1;
}
-*r = ~0ul;
+*r = ~0UL;
return 0;
}
@@ -48,7 +68,12 @@ static int vpci_mmio_write(struct vcpu *v, mmio_info_t *info,
register_t r, void *p)
{
struct pci_host_bridge *bridge = p;
-pci_sbdf_t sbdf = vpci_sbdf_from_gpa(bridge, info->gpa);
+pci_sbdf_t sbdf;
+
+ASSERT(!bridge == !is_hardware_domain(v->domain));
+
+if ( !vpci_sbdf_from_gpa(v->domain, bridge, info->gpa, ) )
+return 1;
return vpci_ecam_write(sbdf, ECAM_REG_OFFSET(info->gpa),
1U << info->dabt.size, r);
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 3cd142068f4e..6b732bb2358e
[PATCH v13 10/14] vpci/header: emulate PCI_COMMAND register for guests
From: Oleksandr Andrushchenko Xen and/or Dom0 may have put values in PCI_COMMAND which they expect to remain unaltered. PCI_COMMAND_SERR bit is a good example: while the guest's (domU) view of this will want to be zero (for now), the host having set it to 1 should be preserved, or else we'd effectively be giving the domU control of the bit. Thus, PCI_COMMAND register needs proper emulation in order to honor host's settings. According to "PCI LOCAL BUS SPECIFICATION, REV. 3.0", section "6.2.2 Device Control" the reset state of the command register is typically 0, so when assigning a PCI device use 0 as the initial state for the guest's (domU) view of the command register. Here is the full list of command register bits with notes about PCI/PCIe specification, and how Xen handles the bit. QEMU's behavior is also documented here since that is our current reference implementation for PCI passthrough. PCI_COMMAND_IO (bit 0) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. QEMU sets this bit to 1 in hardware if an I/O BAR is exposed to the guest. Xen domU: (rsvdp_mask) We treat this bit as RsvdP for now since we don't yet support I/O BARs for domUs. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_MEMORY (bit 1) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. QEMU sets this bit to 1 in hardware if a Memory BAR is exposed to the guest. Xen domU/dom0: We handle writes to this bit by mapping/unmapping BAR regions. Xen domU: For devices assigned to DomUs, memory decoding will be disabled at the time of initialization. PCI_COMMAND_MASTER (bit 2) PCIe 6.1: RW PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_SPECIAL (bit 3) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_INVALIDATE (bit 4) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_VGA_PALETTE (bit 5) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: Pass through writes to hardware. Xen domU/dom0: Pass through writes to hardware. PCI_COMMAND_PARITY (bit 6) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_WAIT (bit 7) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: hardwire to 0 QEMU: res_mask Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_SERR (bit 8) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_FAST_BACK (bit 9) PCIe 6.1: RO, hardwire to 0 PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. Xen domU: (rsvdp_mask) We treat this bit as RsvdP. Xen dom0: We allow dom0 to control this bit freely. PCI_COMMAND_INTX_DISABLE (bit 10) PCIe 6.1: RW PCI LB 3.0: RW QEMU: (emu_mask) QEMU provides an emulated view of this bit. Guest writes do not propagate to hardware. QEMU checks if INTx was mapped for a device. If it is not, then guest can't control PCI_COMMAND_INTX_DISABLE bit. Xen domU: We prohibit a guest from enabling INTx if MSI(X) is enabled. Xen dom0: We allow dom0 to control this bit freely. Bits 11-15 PCIe 6.1: RsvdP PCI LB 3.0: Reserved QEMU: res_mask Xen domU/dom0: rsvdp_mask Signed-off-by: Oleksandr Andrushchenko Signed-off-by: Volodymyr Babchuk Signed-off-by: Stewart Hildebrand --- RFC: There is an unaddressed question for Roger: should we update the guest view of the PCI_COMMAND_INTX_DISABLE bit in msi.c/msix.c:control_write()? See prior discussion at [1]. [1] https://lore.kernel.org/xen-devel/86b25777-788c-4b9a-8166-a6f8174be...@suse.com/ In v13: - Update right away (don't defer) PCI_COMMAND_MEMORY bit in guest_cmd variable in cmd_write() - Make comment single line in xen/drivers/vpci/msi.c:control_write() - Rearrange memory decoding disabling snippet in init_header() In v12: - Rework patch using vpci_add_register_mask() - Add bitmask #define in pci_regs.h according to PCIe 6.1 spec, except don't add the RO bits because they were RW in PCI LB 3.0 spec. - Move and expand TODO comment about properly emulating bits - Update guest_cmd in msi.c/msix.c:control_write() - Simplify cmd_write(), thanks to rsvdp_mask - Update commit
[PATCH v13 13/14] xen/arm: account IO handlers for emulated PCI MSI-X
From: Oleksandr Andrushchenko
At the moment, we always allocate an extra 16 slots for IO handlers
(see MAX_IO_HANDLER). So while adding IO trap handlers for the emulated
MSI-X registers we need to explicitly tell that we have additional IO
handlers, so those are accounted.
Signed-off-by: Oleksandr Andrushchenko
Acked-by: Julien Grall
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
This actually moved here from the part 2 of the prep work for PCI
passthrough on Arm as it seems to be the proper place for it.
Since v5:
- optimize with IS_ENABLED(CONFIG_HAS_PCI_MSI) since VPCI_MAX_VIRT_DEV is
defined unconditionally
New in v5
---
xen/arch/arm/vpci.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 7a6a0017d132..348ba0fbc860 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -130,6 +130,8 @@ static int vpci_get_num_handlers_cb(struct domain *d,
unsigned int domain_vpci_get_num_mmio_handlers(struct domain *d)
{
+unsigned int count;
+
if ( !has_vpci(d) )
return 0;
@@ -150,7 +152,17 @@ unsigned int domain_vpci_get_num_mmio_handlers(struct
domain *d)
* For guests each host bridge requires one region to cover the
* configuration space. At the moment, we only expose a single host bridge.
*/
-return 1;
+count = 1;
+
+/*
+ * There's a single MSI-X MMIO handler that deals with both PBA
+ * and MSI-X tables per each PCI device being passed through.
+ * Maximum number of emulated virtual devices is VPCI_MAX_VIRT_DEV.
+ */
+if ( IS_ENABLED(CONFIG_HAS_PCI_MSI) )
+count += VPCI_MAX_VIRT_DEV;
+
+return count;
}
/*
--
2.43.0
[PATCH v13 09/14] vpci/header: program p2m with guest BAR view
From: Oleksandr Andrushchenko
Take into account guest's BAR view and program its p2m accordingly:
gfn is guest's view of the BAR and mfn is the physical BAR value.
This way hardware domain sees physical BAR values and guest sees
emulated ones.
Hardware domain continues getting the BARs identity mapped, while for
domUs the BARs are mapped at the requested guest address without
modifying the BAR address in the device PCI config space.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
Reviewed-by: Roger Pau Monné
---
In v12.3:
- Update arguments passed to permission error prints in map_range()
In v12.2:
- Slightly tweak print format in modify_bars()
In v12.1:
- ASSERT(rangeset_is_empty()) in modify_bars()
- Fixup print format in modify_bars()
- Make comment single line in bar_write()
- Add Roger's R-b
In v12:
- Update guest_addr in rom_write()
- Use unsigned long for start_mfn and map_mfn to reduce mfn_x() calls
- Use existing vmsix_table_*() functions
- Change vmsix_table_base() to use .guest_addr
In v11:
- Add vmsix_guest_table_addr() and vmsix_guest_table_base() functions
to access guest's view of the VMSIx tables.
- Use MFN (not GFN) to check access permissions
- Move page offset check to this patch
- Call rangeset_remove_range() with correct parameters
In v10:
- Moved GFN variable definition outside the loop in map_range()
- Updated printk error message in map_range()
- Now BAR address is always stored in bar->guest_addr, even for
HW dom, this removes bunch of ugly is_hwdom() checks in modify_bars()
- vmsix_table_base() now uses .guest_addr instead of .addr
In v9:
- Extended the commit message
- Use bar->guest_addr in modify_bars
- Extended printk error message in map_range
- Moved map_data initialization so .bar can be initialized during declaration
Since v5:
- remove debug print in map_range callback
- remove "identity" from the debug print
Since v4:
- moved start_{gfn|mfn} calculation into map_range
- pass vpci_bar in the map_data instead of start_{gfn|mfn}
- s/guest_addr/guest_reg
Since v3:
- updated comment (Roger)
- removed gfn_add(map->start_gfn, rc); which is wrong
- use v->domain instead of v->vpci.pdev->domain
- removed odd e.g. in comment
- s/d%d/%pd in altered code
- use gdprintk for map/unmap logs
Since v2:
- improve readability for data.start_gfn and restructure ?: construct
Since v1:
- s/MSI/MSI-X in comments
---
xen/drivers/vpci/header.c | 83 ++-
xen/include/xen/vpci.h| 3 +-
2 files changed, 66 insertions(+), 20 deletions(-)
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index feccd070ddd0..47648c395132 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -34,6 +34,7 @@
struct map_data {
struct domain *d;
+const struct vpci_bar *bar;
bool map;
};
@@ -41,26 +42,37 @@ static int cf_check map_range(
unsigned long s, unsigned long e, void *data, unsigned long *c)
{
const struct map_data *map = data;
+/* Start address of the BAR as seen by the guest. */
+unsigned long start_gfn = PFN_DOWN(map->bar->guest_addr);
+/* Physical start address of the BAR. */
+unsigned long start_mfn = PFN_DOWN(map->bar->addr);
int rc;
for ( ; ; )
{
unsigned long size = e - s + 1;
+/*
+ * Ranges to be mapped don't always start at the BAR start address, as
+ * there can be holes or partially consumed ranges. Account for the
+ * offset of the current address from the BAR start.
+ */
+unsigned long map_mfn = start_mfn + s - start_gfn;
+unsigned long m_end = map_mfn + size - 1;
-if ( !iomem_access_permitted(map->d, s, e) )
+if ( !iomem_access_permitted(map->d, map_mfn, m_end) )
{
printk(XENLOG_G_WARNING
"%pd denied access to MMIO range [%#lx, %#lx]\n",
- map->d, s, e);
+ map->d, map_mfn, m_end);
return -EPERM;
}
-rc = xsm_iomem_mapping(XSM_HOOK, map->d, s, e, map->map);
+rc = xsm_iomem_mapping(XSM_HOOK, map->d, map_mfn, m_end, map->map);
if ( rc )
{
printk(XENLOG_G_WARNING
"%pd XSM denied access to MMIO range [%#lx, %#lx]: %d\n",
- map->d, s, e, rc);
+ map->d, map_mfn, m_end, rc);
return rc;
}
@@ -73,8 +85,8 @@ static int cf_check map_range(
* - {un}map_mmio_regions doesn't support preemption.
*/
-rc = map->map ? map_mmio_regions(map->d, _gfn(s), size, _mfn(s))
- : unmap_mmio_regions(map->d, _gfn(s), size, _mfn(s));
+rc = map->map ? map_mmio_regions(map->d, _gfn(s), size, _mfn(map_mfn))
+ : unmap_mmio_regions(map->d, _gfn(s), size,
_mfn(map_mfn));
if ( rc == 0 )
{
*c += size;
@@ -83,8
[PATCH v13 14/14] arm/vpci: honor access size when returning an error
From: Volodymyr Babchuk
Guest can try to read config space using different access sizes: 8,
16, 32, 64 bits. We need to take this into account when we are
returning an error back to MMIO handler, otherwise it is possible to
provide more data than requested: i.e. guest issues LDRB instruction
to read one byte, but we are writing 0x in the target
register.
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
v9->10:
* New patch in v10.
---
xen/arch/arm/vpci.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/vpci.c b/xen/arch/arm/vpci.c
index 348ba0fbc860..aaf9d9120c3d 100644
--- a/xen/arch/arm/vpci.c
+++ b/xen/arch/arm/vpci.c
@@ -41,6 +41,8 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
{
struct pci_host_bridge *bridge = p;
pci_sbdf_t sbdf;
+const uint8_t access_size = (1 << info->dabt.size) * 8;
+const uint64_t access_mask = GENMASK_ULL(access_size - 1, 0);
/* data is needed to prevent a pointer cast on 32bit */
unsigned long data;
@@ -48,7 +50,7 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
if ( !vpci_sbdf_from_gpa(v->domain, bridge, info->gpa, ) )
{
-*r = ~0UL;
+*r = access_mask;
return 1;
}
@@ -59,7 +61,7 @@ static int vpci_mmio_read(struct vcpu *v, mmio_info_t *info,
return 1;
}
-*r = ~0UL;
+*r = access_mask;
return 0;
}
--
2.43.0
[PATCH v13 08/14] vpci/header: handle p2m range sets per BAR
From: Oleksandr Andrushchenko
Instead of handling a single range set, that contains all the memory
regions of all the BARs and ROM, have them per BAR.
As the range sets are now created when a PCI device is added and destroyed
when it is removed so make them named and accounted.
Note that rangesets were chosen here despite there being only up to
3 separate ranges in each set (typically just 1). But rangeset per BAR
was chosen for the ease of implementation and existing code re-usability.
Also note that error handling of vpci_process_pending() is slightly
modified, and that vPCI handlers are no longer removed if the creation
of the mappings in vpci_process_pending() fails, as that's unlikely to
lead to a functional device in any case.
This is in preparation of making non-identity mappings in p2m for the MMIOs.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Reviewed-by: Roger Pau Monné
Signed-off-by: Stewart Hildebrand
---
In v12:
- s/rangeset_empty/rangeset_purge/
- change i to num_bars for expansion ROM (purely cosmetic change)
In v11:
- Modified commit message to note changes in error handling in
vpci_process_pending()
- Removed redundant ASSERT() in defer_map. There is no reason to
introduce it in this patch and there is no other patch where
introducing that ASSERT() was appropriate.
- Fixed formatting
- vpci_process_pending() clears v->vpci.pdev if it failed
checks at the beginning
- Added Roger's R-B tag
In v10:
- Added additional checks to vpci_process_pending()
- vpci_process_pending() now clears rangeset in case of failure
- Fixed locks in vpci_process_pending()
- Fixed coding style issues
- Fixed error handling in init_bars
In v9:
- removed d->vpci.map_pending in favor of checking v->vpci.pdev !=
NULL
- printk -> gprintk
- renamed bar variable to fix shadowing
- fixed bug with iterating on remote device's BARs
- relaxed lock in vpci_process_pending
- removed stale comment
Since v6:
- update according to the new locking scheme
- remove odd fail label in modify_bars
Since v5:
- fix comments
- move rangeset allocation to init_bars and only allocate
for MAPPABLE BARs
- check for overlap with the already setup BAR ranges
Since v4:
- use named range sets for BARs (Jan)
- changes required by the new locking scheme
- updated commit message (Jan)
Since v3:
- re-work vpci_cancel_pending accordingly to the per-BAR handling
- s/num_mem_ranges/map_pending and s/uint8_t/bool
- ASSERT(bar->mem) in modify_bars
- create and destroy the rangesets on add/remove
---
xen/drivers/vpci/header.c | 257 ++
xen/drivers/vpci/vpci.c | 6 +
xen/include/xen/vpci.h| 2 +-
3 files changed, 185 insertions(+), 80 deletions(-)
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 39e11e141b38..feccd070ddd0 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -162,63 +162,107 @@ static void modify_decoding(const struct pci_dev *pdev,
uint16_t cmd,
bool vpci_process_pending(struct vcpu *v)
{
-if ( v->vpci.mem )
+struct pci_dev *pdev = v->vpci.pdev;
+struct map_data data = {
+.d = v->domain,
+.map = v->vpci.cmd & PCI_COMMAND_MEMORY,
+};
+struct vpci_header *header = NULL;
+unsigned int i;
+
+if ( !pdev )
+return false;
+
+read_lock(>domain->pci_lock);
+
+if ( !pdev->vpci || (v->domain != pdev->domain) )
{
-struct map_data data = {
-.d = v->domain,
-.map = v->vpci.cmd & PCI_COMMAND_MEMORY,
-};
-int rc = rangeset_consume_ranges(v->vpci.mem, map_range, );
+v->vpci.pdev = NULL;
+read_unlock(>domain->pci_lock);
+return false;
+}
+
+header = >vpci->header;
+for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
+{
+struct vpci_bar *bar = >bars[i];
+int rc;
+
+if ( rangeset_is_empty(bar->mem) )
+continue;
+
+rc = rangeset_consume_ranges(bar->mem, map_range, );
if ( rc == -ERESTART )
+{
+read_unlock(>domain->pci_lock);
return true;
+}
-write_lock(>domain->pci_lock);
-spin_lock(>vpci.pdev->vpci->lock);
-/* Disable memory decoding unconditionally on failure. */
-modify_decoding(v->vpci.pdev,
-rc ? v->vpci.cmd & ~PCI_COMMAND_MEMORY : v->vpci.cmd,
-!rc && v->vpci.rom_only);
-spin_unlock(>vpci.pdev->vpci->lock);
-
-rangeset_destroy(v->vpci.mem);
-v->vpci.mem = NULL;
if ( rc )
-/*
- * FIXME: in case of failure remove the device from the domain.
- * Note that there might still be leftover mappings. While this is
- * safe for Dom0, for DomUs the domain will likely need to be
- * killed in order to avoid leaking stale p2m mappings on
- * failure.
- */
-
[PATCH v13 06/14] rangeset: add RANGESETF_no_print flag
From: Oleksandr Andrushchenko
There are range sets which should not be printed, so introduce a flag
which allows marking those as such. Implement relevant logic to skip
such entries while printing.
While at it also simplify the definition of the flags by directly
defining those without helpers.
Suggested-by: Jan Beulich
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Reviewed-by: Jan Beulich
Signed-off-by: Stewart Hildebrand
---
Since v5:
- comment indentation (Jan)
Since v1:
- update BUG_ON with new flag
- simplify the definition of the flags
---
xen/common/rangeset.c | 5 -
xen/include/xen/rangeset.h | 5 +++--
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index 16a4c3b842e6..0ccd53caac52 100644
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -433,7 +433,7 @@ struct rangeset *rangeset_new(
INIT_LIST_HEAD(>range_list);
r->nr_ranges = -1;
-BUG_ON(flags & ~RANGESETF_prettyprint_hex);
+BUG_ON(flags & ~(RANGESETF_prettyprint_hex | RANGESETF_no_print));
r->flags = flags;
safe_strcpy(r->name, name ?: "(no name)");
@@ -575,6 +575,9 @@ void rangeset_domain_printk(
list_for_each_entry ( r, >rangesets, rangeset_list )
{
+if ( r->flags & RANGESETF_no_print )
+continue;
+
printk("");
rangeset_printk(r);
printk("\n");
diff --git a/xen/include/xen/rangeset.h b/xen/include/xen/rangeset.h
index 8be0722787ed..87bd956962b5 100644
--- a/xen/include/xen/rangeset.h
+++ b/xen/include/xen/rangeset.h
@@ -49,8 +49,9 @@ void rangeset_limit(
/* Flags for passing to rangeset_new(). */
/* Pretty-print range limits in hexadecimal. */
-#define _RANGESETF_prettyprint_hex 0
-#define RANGESETF_prettyprint_hex (1U << _RANGESETF_prettyprint_hex)
+#define RANGESETF_prettyprint_hex (1U << 0)
+ /* Do not print entries marked with this flag. */
+#define RANGESETF_no_print (1U << 1)
bool __must_check rangeset_is_empty(
const struct rangeset *r);
--
2.43.0
[PATCH v13 05/14] vpci/header: implement guest BAR register handlers
From: Oleksandr Andrushchenko
Add relevant vpci register handlers when assigning PCI device to a domain
and remove those when de-assigning. This allows having different
handlers for different domains, e.g. hwdom and other guests.
Emulate guest BAR register values: this allows creating a guest view
of the registers and emulates size and properties probe as it is done
during PCI device enumeration by the guest.
All empty, IO and ROM BARs for guests are emulated by returning 0 on
reads and ignoring writes: this BARs are special with this respect as
their lower bits have special meaning, so returning default ~0 on read
may confuse guest OS.
Introduce is_hwdom convenience variable and convert an existing
is_hardware_domain() check.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Reviewed-by: Roger Pau Monné
Signed-off-by: Stewart Hildebrand
---
In v12:
- Add Roger's R-b
- Get rid of empty_bar_read, use vpci_read_val instead
- Convert an existing is_hardware_domain() check to is_hwdom
- Re-indent usage of ternary operator
- Use ternary operator to avoid re-indenting expansion ROM block
In v11:
- Access guest_addr after adjusting for MEM64_HI bar in
guest_bar_write()
- guest bar handlers renamed and now _mem_ part to denote
that they are handling only memory BARs
- refuse to update guest BAR address if BAR is enabled
In v10:
- ull -> ULL to be MISRA-compatbile
- Use PAGE_OFFSET() instead of combining with ~PAGE_MASK
- Set type of empty bars to VPCI_BAR_EMPTY
In v9:
- factored-out "fail" label introduction in init_bars()
- replaced #ifdef CONFIG_X86 with IS_ENABLED()
- do not pass bars[i] to empty_bar_read() handler
- store guest's BAR address instead of guests BAR register view
Since v6:
- unify the writing of the PCI_COMMAND register on the
error path into a label
- do not introduce bar_ignore_access helper and open code
- s/guest_bar_ignore_read/empty_bar_read
- update error message in guest_bar_write
- only setup empty_bar_read for IO if !x86
Since v5:
- make sure that the guest set address has the same page offset
as the physical address on the host
- remove guest_rom_{read|write} as those just implement the default
behaviour of the registers not being handled
- adjusted comment for struct vpci.addr field
- add guest handlers for BARs which are not handled and will otherwise
return ~0 on read and ignore writes. The BARs are special with this
respect as their lower bits have special meaning, so returning ~0
doesn't seem to be right
Since v4:
- updated commit message
- s/guest_addr/guest_reg
Since v3:
- squashed two patches: dynamic add/remove handlers and guest BAR
handler implementation
- fix guest BAR read of the high part of a 64bit BAR (Roger)
- add error handling to vpci_assign_device
- s/dom%pd/%pd
- blank line before return
Since v2:
- remove unneeded ifdefs for CONFIG_HAS_VPCI_GUEST_SUPPORT as more code
has been eliminated from being built on x86
Since v1:
- constify struct pci_dev where possible
- do not open code is_system_domain()
- simplify some code3. simplify
- use gdprintk + error code instead of gprintk
- gate vpci_bar_{add|remove}_handlers with CONFIG_HAS_VPCI_GUEST_SUPPORT,
so these do not get compiled for x86
- removed unneeded is_system_domain check
- re-work guest read/write to be much simpler and do more work on write
than read which is expected to be called more frequently
- removed one too obvious comment
---
xen/drivers/vpci/header.c | 109 +++---
xen/include/xen/vpci.h| 3 ++
2 files changed, 106 insertions(+), 6 deletions(-)
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 803fe4bb99a6..39e11e141b38 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -478,6 +478,69 @@ static void cf_check bar_write(
pci_conf_write32(pdev->sbdf, reg, val);
}
+static void cf_check guest_mem_bar_write(const struct pci_dev *pdev,
+ unsigned int reg, uint32_t val,
+ void *data)
+{
+struct vpci_bar *bar = data;
+bool hi = false;
+uint64_t guest_addr;
+
+if ( bar->type == VPCI_BAR_MEM64_HI )
+{
+ASSERT(reg > PCI_BASE_ADDRESS_0);
+bar--;
+hi = true;
+}
+else
+{
+val &= PCI_BASE_ADDRESS_MEM_MASK;
+}
+
+guest_addr = bar->guest_addr;
+guest_addr &= ~(0xULL << (hi ? 32 : 0));
+guest_addr |= (uint64_t)val << (hi ? 32 : 0);
+
+/* Allow guest to size BAR correctly */
+guest_addr &= ~(bar->size - 1);
+
+/*
+ * Xen only cares whether the BAR is mapped into the p2m, so allow BAR
+ * writes as long as the BAR is not mapped into the p2m.
+ */
+if ( bar->enabled )
+{
+/* If the value written is the current one avoid printing a warning. */
+if ( guest_addr != bar->guest_addr )
+gprintk(XENLOG_WARNING,
+"%pp: ignored guest
[PATCH v13 04/14] vpci/header: rework exit path in init_header()
From: Volodymyr Babchuk
Introduce "fail" label in init_header() function to have the centralized
error return path. This is the pre-requirement for the future changes
in this function.
This patch does not introduce functional changes.
Suggested-by: Roger Pau Monné
Signed-off-by: Volodymyr Babchuk
Acked-by: Roger Pau Monné
Signed-off-by: Stewart Hildebrand
---
In v12:
- s/init_bars/init_header/
- Re-order tags
- Fixup scissors line
In v11:
- Do not remove empty line between "goto fail;" and "continue;"
In v10:
- Added Roger's A-b tag.
In v9:
- New in v9
---
xen/drivers/vpci/header.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 2f2d98ada012..803fe4bb99a6 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -656,10 +656,7 @@ static int cf_check init_header(struct pci_dev *pdev)
rc = vpci_add_register(pdev->vpci, vpci_hw_read32, bar_write, reg,
4, [i]);
if ( rc )
-{
-pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
-return rc;
-}
+goto fail;
continue;
}
@@ -679,10 +676,7 @@ static int cf_check init_header(struct pci_dev *pdev)
rc = pci_size_mem_bar(pdev->sbdf, reg, , ,
(i == num_bars - 1) ? PCI_BAR_LAST : 0);
if ( rc < 0 )
-{
-pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
-return rc;
-}
+goto fail;
if ( size == 0 )
{
@@ -697,10 +691,7 @@ static int cf_check init_header(struct pci_dev *pdev)
rc = vpci_add_register(pdev->vpci, vpci_hw_read32, bar_write, reg, 4,
[i]);
if ( rc )
-{
-pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
-return rc;
-}
+goto fail;
}
/* Check expansion ROM. */
@@ -722,6 +713,10 @@ static int cf_check init_header(struct pci_dev *pdev)
}
return (cmd & PCI_COMMAND_MEMORY) ? modify_bars(pdev, cmd, false) : 0;
+
+ fail:
+pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
+return rc;
}
REGISTER_VPCI_INIT(init_header, VPCI_PRIORITY_MIDDLE);
--
2.43.0
[PATCH v13 03/14] vpci: add hooks for PCI device assign/de-assign
From: Oleksandr Andrushchenko
When a PCI device gets assigned/de-assigned we need to
initialize/de-initialize vPCI state for the device.
Also, rename vpci_add_handlers() to vpci_assign_device() and
vpci_remove_device() to vpci_deassign_device() to better reflect role
of the functions.
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Reviewed-by: Roger Pau Monné
Signed-off-by: Stewart Hildebrand
Acked-by: Jan Beulich
---
In v13:
- Add Jan's A-b
- Rebase on
cb4ecb3cc17b ("pci: fail device assignment if phantom functions cannot be
assigned")
and add if ( rc ) goto done; in assign_device()
In v12:
- Add Roger's R-b
- Clean up comment in xen/include/xen/vpci.h
- Add comment in xen/drivers/passthrough/pci.c:deassign_device() to
clarify vpci_assign_device() call
In v11:
- Call vpci_assign_device() in "deassign_device" if IOMMU call
"reassign_device" was successful.
In v10:
- removed HAS_VPCI_GUEST_SUPPORT checks
- HAS_VPCI_GUEST_SUPPORT config option (in Kconfig) as it is not used
anywhere
In v9:
- removed previous vpci_[de]assign_device function and renamed
existing handlers
- dropped attempts to handle errors in assign_device() function
- do not call vpci_assign_device for dom_io
- use d instead of pdev->domain
- use IS_ENABLED macro
In v8:
- removed vpci_deassign_device
In v6:
- do not pass struct domain to vpci_{assign|deassign}_device as
pdev->domain can be used
- do not leave the device assigned (pdev->domain == new domain) in case
vpci_assign_device fails: try to de-assign and if this also fails, then
crash the domain
In v5:
- do not split code into run_vpci_init
- do not check for is_system_domain in vpci_{de}assign_device
- do not use vpci_remove_device_handlers_locked and re-allocate
pdev->vpci completely
- make vpci_deassign_device void
In v4:
- de-assign vPCI from the previous domain on device assignment
- do not remove handlers in vpci_assign_device as those must not
exist at that point
In v3:
- remove toolstack roll-back description from the commit message
as error are to be handled with proper cleanup in Xen itself
- remove __must_check
- remove redundant rc check while assigning devices
- fix redundant CONFIG_HAS_VPCI check for CONFIG_HAS_VPCI_GUEST_SUPPORT
- use REGISTER_VPCI_INIT machinery to run required steps on device
init/assign: add run_vpci_init helper
In v2:
- define CONFIG_HAS_VPCI_GUEST_SUPPORT so dead code is not compiled
for x86
In v1:
- constify struct pci_dev where possible
- do not open code is_system_domain()
- extended the commit message
---
xen/drivers/passthrough/pci.c | 28
xen/drivers/vpci/header.c | 2 +-
xen/drivers/vpci/vpci.c | 6 +++---
xen/include/xen/vpci.h| 10 +-
4 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index c97dd4504a7a..4c0a836486ec 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -755,7 +755,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
* For devices not discovered by Xen during boot, add vPCI handlers
* when Dom0 first informs Xen about such devices.
*/
-ret = vpci_add_handlers(pdev);
+ret = vpci_assign_device(pdev);
if ( ret )
{
list_del(>domain_list);
@@ -769,7 +769,7 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
if ( ret )
{
write_lock(_domain->pci_lock);
-vpci_remove_device(pdev);
+vpci_deassign_device(pdev);
list_del(>domain_list);
write_unlock(_domain->pci_lock);
pdev->domain = NULL;
@@ -817,7 +817,7 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
list_for_each_entry ( pdev, >alldevs_list, alldevs_list )
if ( pdev->bus == bus && pdev->devfn == devfn )
{
-vpci_remove_device(pdev);
+vpci_deassign_device(pdev);
pci_cleanup_msi(pdev);
ret = iommu_remove_device(pdev);
if ( pdev->domain )
@@ -875,6 +875,10 @@ static int deassign_device(struct domain *d, uint16_t seg,
uint8_t bus,
goto out;
}
+write_lock(>pci_lock);
+vpci_deassign_device(pdev);
+write_unlock(>pci_lock);
+
devfn = pdev->devfn;
ret = iommu_call(hd->platform_ops, reassign_device, d, target, devfn,
pci_to_dev(pdev));
@@ -886,6 +890,11 @@ static int deassign_device(struct domain *d, uint16_t seg,
uint8_t bus,
pdev->fault.count = 0;
+write_lock(>pci_lock);
+/* Re-assign back to hardware_domain */
+ret = vpci_assign_device(pdev);
+write_unlock(>pci_lock);
+
out:
if ( ret )
printk(XENLOG_G_ERR "%pd: deassign (%pp) failed (%d)\n",
@@ -1146,7 +1155,7 @@ static void __hwdom_init setup_one_hwdom_device(const
struct setup_hwdom *ctxt,
PCI_SLOT(devfn) ==
[PATCH v13 02/14] vpci: restrict unhandled read/write operations for guests
From: Oleksandr Andrushchenko
A guest would be able to read and write those registers which are not
emulated and have no respective vPCI handlers, so it will be possible
for it to access the hardware directly.
In order to prevent a guest from reads and writes from/to the unhandled
registers make sure only hardware domain can access the hardware directly
and restrict guests from doing so.
Suggested-by: Roger Pau Monné
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Reviewed-by: Roger Pau Monné
Signed-off-by: Stewart Hildebrand
---
Since v9:
- removed stray formatting change
- added Roger's R-b tag
Since v6:
- do not use is_hwdom parameter for vpci_{read|write}_hw and use
current->domain internally
- update commit message
New in v6
---
xen/drivers/vpci/vpci.c | 8
1 file changed, 8 insertions(+)
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 475272b173f3..d545dc633c40 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -268,6 +268,10 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int
reg,
{
uint32_t data;
+/* Guest domains are not allowed to read real hardware. */
+if ( !is_hardware_domain(current->domain) )
+return ~(uint32_t)0;
+
switch ( size )
{
case 4:
@@ -311,6 +315,10 @@ static uint32_t vpci_read_hw(pci_sbdf_t sbdf, unsigned int
reg,
static void vpci_write_hw(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
uint32_t data)
{
+/* Guest domains are not allowed to write real hardware. */
+if ( !is_hardware_domain(current->domain) )
+return;
+
switch ( size )
{
case 4:
--
2.43.0
[PATCH v13 01/14] vpci: use per-domain PCI lock to protect vpci structure
From: Oleksandr Andrushchenko
Use the per-domain PCI read/write lock to protect the presence of the
pci device vpci field. This lock can be used (and in a few cases is used
right away) so that vpci removal can be performed while holding the lock
in write mode. Previously such removal could race with vpci_read for
example.
When taking both d->pci_lock and pdev->vpci->lock, they should be
taken in this exact order: d->pci_lock then pdev->vpci->lock to avoid
possible deadlock situations.
1. Per-domain's pci_lock is used to protect pdev->vpci structure
from being removed.
2. Writing the command register and ROM BAR register may trigger
modify_bars to run, which in turn may access multiple pdevs while
checking for the existing BAR's overlap. The overlapping check, if
done under the read lock, requires vpci->lock to be acquired on both
devices being compared, which may produce a deadlock. It is not
possible to upgrade read lock to write lock in such a case. So, in
order to prevent the deadlock, use d->pci_lock in write mode instead.
All other code, which doesn't lead to pdev->vpci destruction and does
not access multiple pdevs at the same time, can still use a
combination of the read lock and pdev->vpci->lock.
3. Drop const qualifier where the new rwlock is used and this is
appropriate.
4. Do not call process_pending_softirqs with any locks held. For that
unlock prior the call and re-acquire the locks after. After
re-acquiring the lock there is no need to check if pdev->vpci exists:
- in apply_map because of the context it is called (no race condition
possible)
- for MSI/MSI-X debug code because it is called at the end of
pdev->vpci access and no further access to pdev->vpci is made
5. Use d->pci_lock around for_each_pdev and pci_get_pdev()
while accessing pdevs in vpci code.
6. Switch vPCI functions to use per-domain pci_lock for ensuring pdevs
do not go away. The vPCI functions call several MSI-related functions
which already have existing non-vPCI callers. Change those MSI-related
functions to allow using either pcidevs_lock() or d->pci_lock for
ensuring pdevs do not go away. Holding d->pci_lock in read mode is
sufficient. Note that this pdev protection mechanism does not protect
other state or critical sections. These MSI-related functions already
have other race condition and state protection mechanims (e.g.
d->event_lock and msixtbl RCU), so we deduce that the use of the global
pcidevs_lock() is to ensure that pdevs do not go away. Existing non-vPCI
callers of these MSI-related functions will remain (ab)using the global
pcidevs_lock() to ensure pdevs do not go away so as to minimize changes
to existing non-vPCI call paths.
7. Introduce wrapper construct, pdev_list_is_read_locked(), for checking
that pdevs do not go away. The purpose of this wrapper is to aid
readability and document the intent of the pdev protection mechanism.
Suggested-by: Roger Pau Monné
Suggested-by: Jan Beulich
Signed-off-by: Oleksandr Andrushchenko
Signed-off-by: Volodymyr Babchuk
Signed-off-by: Stewart Hildebrand
---
Changes in v13:
- hold off adding Roger's R-b tag even though it was provided on v12.2
- use a wrapper construct to ease readability of odd-looking ASSERTs
- new placement of ASSERT in __pci_enable_msix(), __pci_enable_msi(),
and pci_enable_msi(). Rearrange/add pdev NULL check.
- expand commit description with details about using either
pcidevs_lock() or d->pci_lock
Changes in v12.2:
- drop Roger's R-b
- drop both locks on error paths in vpci_msix_arch_print()
- add another ASSERT in vpci_msix_arch_print(), to enforce the
expectation both locks are held before calling vpci_msix_arch_print()
- move pdev_done label in vpci_dump_msi()
- update comments in vpci_dump_msi() to say locks (plural)
Changes in v12.1:
- use read_trylock() in vpci_msix_arch_print()
- fixup in-code comments (revert double space, use DomXEN) in
vpci_{read,write}()
- minor updates in commit message
- add Roger's R-b
Changes in v12:
- s/pci_rwlock/pci_lock/ in commit message
- expand comment about scope of pci_lock in sched.h
- in vpci_{read,write}, if hwdom is trying to access a device assigned
to dom_xen, holding hwdom->pci_lock is sufficient (no need to hold
dom_xen->pci_lock)
- reintroduce ASSERT in vmx_pi_update_irte()
- reintroduce ASSERT in __pci_enable_msi{x}()
- delete note 6. in commit message about removing ASSERTs since we have
reintroduced them
Changes in v11:
- Fixed commit message regarding possible spinlocks
- Removed parameter from allocate_and_map_msi_pirq(), which was added
in the prev version. Now we are taking pcidevs_lock in
physdev_map_pirq()
- Returned ASSERT to pci_enable_msi
- Fixed case when we took read lock instead of write one
- Fixed label indentation
Changes in v10:
- Moved printk pas locked area
- Returned back ASSERTs
- Added new parameter to allocate_and_map_msi_pirq() so it knows if
it should take the global pci lock
- Added comment
[PATCH v13 00/14] PCI devices passthrough on Arm, part 3
This is next version of vPCI rework. Aim of this series is to prepare
ground for introducing PCI support on ARM platform.
in v13:
- drop ("xen/arm: vpci: permit access to guest vpci space") as it was
unnecessary
in v12:
- I (Stewart) coordinated with Volodomyr to send this whole series. So,
add my (Stewart) Signed-off-by to all patches.
- The biggest change is to re-work the PCI_COMMAND register patch.
Additional feedback has also been addressed - see individual patches.
- Drop ("pci: msi: pass pdev to pci_enable_msi() function") and
("pci: introduce per-domain PCI rwlock") as they were committed
- Rename ("rangeset: add rangeset_empty() function")
to ("rangeset: add rangeset_purge() function")
- Rename ("vpci/header: rework exit path in init_bars")
to ("vpci/header: rework exit path in init_header()")
in v11:
- Added my (Volodymyr) Signed-off-by tag to all patches
- Patch "vpci/header: emulate PCI_COMMAND register for guests" is in
intermediate state, because it was agreed to rework it once Stewart's
series on register handling are in.
- Addressed comments, please see patch descriptions for details.
in v10:
- Removed patch ("xen/arm: vpci: check guest range"), proper fix
for the issue is part of ("vpci/header: emulate PCI_COMMAND
register for guests")
- Removed patch ("pci/header: reset the command register when adding
devices")
- Added patch ("rangeset: add rangeset_empty() function") because
this function is needed in ("vpci/header: handle p2m range sets
per BAR")
- Added ("vpci/header: handle p2m range sets per BAR") which addressed
an issue discovered by Andrii Chepurnyi during virtio integration
- Added ("pci: msi: pass pdev to pci_enable_msi() function"), which is
prereq for ("pci: introduce per-domain PCI rwlock")
- Fixed "Since v9/v8/... " comments in changelogs to reduce confusion.
I left "Since" entries for older versions, because they were added
by original author of the patches.
in v9:
v9 includes addressed commentes from a previous one. Also it
introduces a couple patches from Stewart. This patches are related to
vPCI use on ARM. Patch "vpci/header: rework exit path in init_bars"
was factored-out from "vpci/header: handle p2m range sets per BAR".
in v8:
The biggest change from previous, mistakenly named, v7 series is how
locking is implemented. Instead of d->vpci_rwlock we introduce
d->pci_lock which has broader scope, as it protects not only domain's
vpci state, but domain's list of PCI devices as well.
As we discussed in IRC with Roger, it is not feasible to rework all
the existing code to use the new lock right away. It was agreed that
any write access to d->pdev_list will be protected by **both**
d->pci_lock in write mode and pcidevs_lock(). Read access on other
hand should be protected by either d->pci_lock in read mode or
pcidevs_lock(). It is expected that existing code will use
pcidevs_lock() and new users will use new rw lock. Of course, this
does not mean that new users shall not use pcidevs_lock() when it is
appropriate.
Changes from previous versions are described in each separate patch.
Oleksandr Andrushchenko (11):
vpci: use per-domain PCI lock to protect vpci structure
vpci: restrict unhandled read/write operations for guests
vpci: add hooks for PCI device assign/de-assign
vpci/header: implement guest BAR register handlers
rangeset: add RANGESETF_no_print flag
vpci/header: handle p2m range sets per BAR
vpci/header: program p2m with guest BAR view
vpci/header: emulate PCI_COMMAND register for guests
vpci: add initial support for virtual PCI bus topology
xen/arm: translate virtual PCI bus topology for guests
xen/arm: account IO handlers for emulated PCI MSI-X
Volodymyr Babchuk (3):
vpci/header: rework exit path in init_header()
rangeset: add rangeset_purge() function
arm/vpci: honor access size when returning an error
xen/arch/arm/vpci.c | 63 -
xen/arch/x86/hvm/vmsi.c | 31 ++-
xen/arch/x86/hvm/vmx/vmx.c| 2 +-
xen/arch/x86/irq.c| 8 +-
xen/arch/x86/msi.c| 20 +-
xen/arch/x86/physdev.c| 2 +
xen/common/rangeset.c | 21 +-
xen/drivers/Kconfig | 4 +
xen/drivers/passthrough/pci.c | 35 ++-
xen/drivers/vpci/header.c | 495 +++---
xen/drivers/vpci/msi.c| 37 ++-
xen/drivers/vpci/msix.c | 59 +++-
xen/drivers/vpci/vpci.c | 125 -
xen/include/xen/pci_regs.h| 1 +
xen/include/xen/rangeset.h| 8 +-
xen/include/xen/sched.h | 25 +-
xen/include/xen/vpci.h| 45 +++-
17 files changed, 808 insertions(+), 173 deletions(-)
base-commit: 3f819af8a796c0e2f798dd301ec8c3f8cccbc9fc
--
2.43.0
[libvirt test] 184562: tolerable all pass - PUSHED
flight 184562 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/184562/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184537 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184537 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184537 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-libvirt 15 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-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: libvirt 5e95cedbb2c5d5af049110369c627f90bbdb0799 baseline version: libvirt 2757e91c2b28b704d9a0b586fb60012450110b1a Last test of basis 184537 2024-01-31 04:24:45 Z2 days Failing since184548 2024-02-01 06:57:31 Z1 days2 attempts Testing same since 184562 2024-02-02 04:20:30 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Göran Uddeborg Jiri Denemark Peter Krempa Stefano Brivio jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-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 pass 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 pass test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw pass 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
Re: Nullifying Recently Introduced Xen Headers Check
On 2/2/2024 11:25 AM, David Woodhouse wrote:
> On Thu, 2024-02-01 at 18:39 +, John L. Poole wrote:
>> While the Xen Project "make" works, the Gentoo emerge
>> of app-emulation/xen-tools does not unless the three lines are
>> removed to simulate prior 4.17.3 and earlier code.
>>
>> I suspect the Gentoo approach
>> of building tools first contributes to the problem.
> The problem is that -D__XEN_INTERFACE_VERSION=… is being set on the
> compiler command line, in ${CFLAGS}.
>
> Clearly it isn't qemu itself which does that. And I think you said
> above that invoking the qemu build even from the Xen makefiles directly
> is also working fine?
>
> So the question is why the Gentoo build is setting that explicitly in
> CFLAGS?
Correct.
I downloaded
https://downloads.xenproject.org/release/xen/4.18.0/xen-4.18.0.tar.gz,
staged and entered the tree, ".configure", then "make" and everything
worked. That's when
I decided Gentoo's package/build system was affecting matters and I
later determined that Gentoo
had made the decision to bifurcate the build order and build tools in
its own package,
app-emulation/xen-tools, and then xen in its own package
app-emulation/xen, the
former being a prerequisite to the latter.
I've postured this divergence in the bug: https://bugs.gentoo.org/921932#c21
-- John
Re: Nullifying Recently Introduced Xen Headers Check
On Thu, 2024-02-01 at 18:39 +, John L. Poole wrote:
>
> While the Xen Project "make" works, the Gentoo emerge
> of app-emulation/xen-tools does not unless the three lines are
> removed to simulate prior 4.17.3 and earlier code.
>
> I suspect the Gentoo approach
> of building tools first contributes to the problem.
The problem is that -D__XEN_INTERFACE_VERSION=… is being set on the
compiler command line, in ${CFLAGS}.
Clearly it isn't qemu itself which does that. And I think you said
above that invoking the qemu build even from the Xen makefiles directly
is also working fine?
So the question is why the Gentoo build is setting that explicitly in
CFLAGS?
smime.p7s
Description: S/MIME cryptographic signature
[xen-unstable test] 184557: tolerable FAIL - PUSHED
flight 184557 xen-unstable real [real] flight 184567 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/184557/ http://logs.test-lab.xenproject.org/osstest/logs/184567/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-dom0pvh-xl-intel 20 guest-localmigrate/x10 fail pass in 184567-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184547 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184547 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 184547 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184547 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 184547 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 184547 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184547 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184547 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184547 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184547 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 184547 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184547 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-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass version targeted for testing: xen bc45f20c01f1711bc56a4bb0955c49c182a5a03a baseline version: xen cc6ba68edf6dcd18c3865e7d7c0f1ed822796426 Last test of basis 184547 2024-02-01
Re: [XEN PATCH 4/9] x86/smp: move stack_base to cpu_data
Hi,
On 14/11/2023 17:50, Krystian Hebel wrote:
This location is easier to access from assembly. Having it close to
other data required during initialization has also positive (although
rather small) impact on prefetching data from RAM.
I understand your goal but...
Signed-off-by: Krystian Hebel
---
xen/arch/x86/boot/x86_64.S| 5 ++---
xen/arch/x86/include/asm/cpufeature.h | 1 +
xen/arch/x86/include/asm/smp.h| 2 +-
xen/arch/x86/setup.c | 6 +++---
xen/arch/x86/smpboot.c| 25 +
xen/arch/x86/traps.c | 4 ++--
xen/arch/x86/x86_64/asm-offsets.c | 1 +
xen/include/xen/smp.h | 2 --
8 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 195550b5c0ea..8d61f270761f 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -33,9 +33,8 @@ ENTRY(__high_start)
cmp %esp, CPUINFO_X86_apicid(%rcx)
jne 1b
-/* %eax is now Xen CPU index. */
-lea stack_base(%rip), %rcx
-mov (%rcx, %rax, 8), %rsp
+/* %rcx is now cpu_data[cpu], read stack base from it. */
+mov CPUINFO_X86_stack_base(%rcx), %rsp
test%rsp,%rsp
jnz 1f
diff --git a/xen/arch/x86/include/asm/cpufeature.h
b/xen/arch/x86/include/asm/cpufeature.h
index 06e1dd7f3332..ff0e18864cc7 100644
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
... cpufeature.h feels a rather odd place for storing the stack. I am
not entirely sure where else to place. Andrew, Jan, Roger?
@@ -37,6 +37,7 @@ struct cpuinfo_x86 {
unsigned int phys_proc_id; /* package ID of each logical CPU */
unsigned int cpu_core_id; /* core ID of each logical CPU */
unsigned int compute_unit_id; /* AMD compute unit ID of each logical
CPU */
+void *stack_base;
AFAICT, this means there will be a padding before stack_base and ...
unsigned short x86_clflush_size;
... another one here. Is there any particular reason the new field
wasn't added at the end?
} __cacheline_aligned;
diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h
index 94c557491860..98739028a6ed 100644
--- a/xen/arch/x86/include/asm/smp.h
+++ b/xen/arch/x86/include/asm/smp.h
@@ -69,7 +69,7 @@ extern cpumask_t **socket_cpumask;
* by certain scheduling code only.
*/
#define get_cpu_current(cpu) \
-(get_cpu_info_from_stack((unsigned long)stack_base[cpu])->current_vcpu)
+(get_cpu_info_from_stack((unsigned
long)cpu_data[cpu].stack_base)->current_vcpu)
extern unsigned int disabled_cpus;
extern bool unaccounted_cpus;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a19fe219bbf6..b2c0679725ea 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -798,7 +798,7 @@ static void __init noreturn reinit_bsp_stack(void)
/* Update SYSCALL trampolines */
percpu_traps_init();
-stack_base[0] = stack;
+cpu_data[0].stack_base = stack;
rc = setup_cpu_root_pgt(0);
if ( rc )
@@ -1959,8 +1959,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
/* Set up node_to_cpumask based on cpu_to_node[]. */
numa_add_cpu(i);
-if ( stack_base[i] == NULL )
-stack_base[i] = cpu_alloc_stack(i);
+if ( cpu_data[i].stack_base == NULL )
+cpu_data[i].stack_base = cpu_alloc_stack(i);
}
for_each_present_cpu ( i )
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index f061486e56eb..8ae65ab1769f 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -75,13 +75,15 @@ static enum cpu_state {
} cpu_state;
#define set_cpu_state(state) do { smp_mb(); cpu_state = (state); } while (0)
-void *stack_base[NR_CPUS];
-
void initialize_cpu_data(unsigned int cpu)
{
uint32_t apicid = cpu_physical_id(cpu);
+void *stack = cpu_data[cpu].stack_base;
+
cpu_data[cpu] = boot_cpu_data;
+
cpu_physical_id(cpu) = apicid;
+cpu_data[cpu].stack_base = stack;
}
static bool smp_store_cpu_info(unsigned int id)
@@ -579,8 +581,6 @@ static int do_boot_cpu(int apicid, int cpu)
printk("Booting processor %d/%d eip %lx\n",
cpu, apicid, start_eip);
-stack_start = stack_base[cpu] + STACK_SIZE - sizeof(struct cpu_info);
-
You remove this line because I can't quite figure out where stack_start
is now set. This is used...
/* This grunge runs the startup process for the targeted processor. */
set_cpu_state(CPU_STATE_INIT);
@@ -856,7 +856,7 @@ int setup_cpu_root_pgt(unsigned int cpu)
/* Install direct map page table entries for stack, IDT, and TSS. */
for ( off = rc = 0; !rc && off < STACK_SIZE; off += PAGE_SIZE )
-rc =
Re: [XEN PATCH 3/9] x86/smp: drop x86_cpu_to_apicid, use cpu_data[cpu].apicid instead
Hi,
On 14/11/2023 17:50, Krystian Hebel wrote:
Both fields held the same data.
Signed-off-by: Krystian Hebel
---
xen/arch/x86/boot/x86_64.S | 8 +---
xen/arch/x86/include/asm/asm_defns.h | 2 +-
xen/arch/x86/include/asm/processor.h | 2 ++
xen/arch/x86/include/asm/smp.h | 4
xen/arch/x86/numa.c | 15 +++
xen/arch/x86/smpboot.c | 8
xen/arch/x86/x86_64/asm-offsets.c| 4 +++-
7 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index b85b47b5c1a0..195550b5c0ea 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -20,15 +20,17 @@ ENTRY(__high_start)
jz .L_stack_set
/* APs only: get stack base from APIC ID saved in %esp. */
-mov $-1, %rax
-lea x86_cpu_to_apicid(%rip), %rcx
+mov $0, %rax
+lea cpu_data(%rip), %rcx
+/* cpu_data[0] is BSP, skip it. */
1:
add $1, %rax
+add $CPUINFO_X86_sizeof, %rcx
cmp $NR_CPUS, %eax
jb 2f
hlt
2:
-cmp %esp, (%rcx, %rax, 4)
+cmp %esp, CPUINFO_X86_apicid(%rcx)
jne 1b
/* %eax is now Xen CPU index. */
As mentioned in an earlier patch, I think you want to re-order the
patches. This will avoid to modify twice the same code within the same
series (it is best to avoid if you can).
diff --git a/xen/arch/x86/include/asm/asm_defns.h
b/xen/arch/x86/include/asm/asm_defns.h
index baaaccb26e17..6b05d9d140b8 100644
--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -158,7 +158,7 @@ register unsigned long current_stack_pointer asm("rsp");
#endif
#define CPUINFO_FEATURE_OFFSET(feature) \
-(CPUINFO_features + (cpufeat_word(feature) * 4))
+(CPUINFO_X86_features + (cpufeat_word(feature) * 4))
#else
diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index b0d2a62c075f..8345d58094da 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -92,6 +92,8 @@ struct x86_cpu_id {
extern struct cpuinfo_x86 cpu_data[];
#define current_cpu_data cpu_data[smp_processor_id()]
+#define cpu_physical_id(cpu) cpu_data[cpu].apicid
+
extern bool probe_cpuid_faulting(void);
extern void ctxt_switch_levelling(const struct vcpu *next);
extern void (*ctxt_switch_masking)(const struct vcpu *next);
diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h
index c0b5d7cdd8dd..94c557491860 100644
--- a/xen/arch/x86/include/asm/smp.h
+++ b/xen/arch/x86/include/asm/smp.h
@@ -39,10 +39,6 @@ extern void (*mtrr_hook) (void);
extern void zap_low_mappings(void);
-extern u32 x86_cpu_to_apicid[];
-
-#define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
-
#define cpu_is_offline(cpu) unlikely(!cpu_online(cpu))
extern void cpu_exit_clear(unsigned int cpu);
extern void cpu_uninit(unsigned int cpu);
diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index 39e131cb4f35..91527be5b406 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -54,14 +54,13 @@ bool __init arch_numa_unavailable(void)
/*
* Setup early cpu_to_node.
*
- * Populate cpu_to_node[] only if x86_cpu_to_apicid[],
- * and apicid_to_node[] tables have valid entries for a CPU.
- * This means we skip cpu_to_node[] initialisation for NUMA
- * emulation and faking node case (when running a kernel compiled
- * for NUMA on a non NUMA box), which is OK as cpu_to_node[]
- * is already initialized in a round robin manner at numa_init_array,
- * prior to this call, and this initialization is good enough
- * for the fake NUMA cases.
+ * Populate cpu_to_node[] only if cpu_data[], and apicid_to_node[]
+ * tables have valid entries for a CPU. This means we skip
+ * cpu_to_node[] initialisation for NUMA emulation and faking node
+ * case (when running a kernel compiled for NUMA on a non NUMA box),
+ * which is OK as cpu_to_node[] is already initialized in a round
+ * robin manner at numa_init_array, prior to this call, and this
+ * initialization is good enough for the fake NUMA cases.
*/
void __init init_cpu_to_node(void)
{
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index de87c5a41926..f061486e56eb 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -61,10 +61,8 @@ unsigned int __read_mostly nr_sockets;
cpumask_t **__read_mostly socket_cpumask;
static cpumask_t *secondary_socket_cpumask;
-struct cpuinfo_x86 cpu_data[NR_CPUS];
-
-u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly =
- { [0 ... NR_CPUS-1] = BAD_APICID };
+struct cpuinfo_x86 cpu_data[NR_CPUS] =
+{ [0 ... NR_CPUS-1] .apicid = BAD_APICID };
static int cpu_error;
static enum cpu_state {
@@ -81,7 +79,9 @@ void *stack_base[NR_CPUS];
void
[PATCH] xen: move BUG_ON(), WARN_ON(), ASSERT(), ASSERT_UNREACHABLE() to xen/bug.h
Move the macros mentioned in the commit subject to their appropriate
locations.
Additionally, eliminate the dependency of xen/lib.h from xen/bug.h and
include "xen/bug.h" in files where xen/bug.h macros are utilized.
Most of the changes were made because a file requires macros from xen/bug.h,
except for some files for Arm which require definitions of BUG_OPCODE,
BUG_INSTR, BUG_FN_REG.
xen/lib.h was added to list-sort.c ( otherwise compilation errors related
to {d}printk occur during compilation of list-sort.c. ) as xen/lib.h was
removed from xen/list.h. Since nothing in xen/list.h depends on xen/lib.h
functionality and only xen/bug.h is needed.
cpufeature.h requires the inclusion of ;
otherwise, the following error will occur:
ld: common/monitor.o:/build/xen/./arch/x86/include/asm/cpufeature.h:41:
multiple definitions of `__cacheline_aligned';
Signed-off-by: Oleksii Kurochko
---
xen/arch/arm/arm32/insn.c| 3 ++-
xen/arch/arm/arm64/cpufeature.c | 1 +
xen/arch/arm/arm64/insn.c| 1 +
xen/arch/arm/cpufeature.c| 1 +
xen/arch/arm/include/asm/arm32/cmpxchg.h | 1 +
xen/arch/arm/include/asm/arm64/cmpxchg.h | 2 ++
xen/arch/arm/include/asm/regs.h | 2 +-
xen/arch/arm/include/asm/vgic.h | 1 +
xen/arch/ppc/include/asm/time.h | 2 +-
xen/arch/x86/bitops.c| 2 +-
xen/arch/x86/include/asm/cpufeature.h| 1 +
xen/arch/x86/include/asm/system.h| 2 +-
xen/arch/x86/include/asm/x86_64/page.h | 2 ++
xen/arch/x86/x86_emulate/private.h | 1 +
xen/common/efi/common-stub.c | 2 +-
xen/common/version.c | 1 +
xen/include/public/hvm/save.h| 2 +-
xen/include/xen/bug.h| 19 +++
xen/include/xen/cpumask.h| 1 +
xen/include/xen/device_tree.h| 1 +
xen/include/xen/lib.h| 19 ---
xen/include/xen/list.h | 2 +-
xen/include/xen/livepatch.h | 2 ++
xen/include/xen/mm.h | 1 +
xen/include/xen/param.h | 1 +
xen/lib/list-sort.c | 1 +
xen/xsm/flask/ss/ebitmap.h | 1 +
27 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/xen/arch/arm/arm32/insn.c b/xen/arch/arm/arm32/insn.c
index 49953a042a..2a62bb9cce 100644
--- a/xen/arch/arm/arm32/insn.c
+++ b/xen/arch/arm/arm32/insn.c
@@ -13,8 +13,9 @@
* You should have received a copy of the GNU General Public License
* along with this program. If not, see
[PATCH] x86/ucode: Remove accidentally introduced tabs
Fixes: cf7fe8b72dea ("x86/ucode: Fix stability of the raw CPU Policy rescan")
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
I don't know what went wrong here.
---
xen/arch/x86/cpu/microcode/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/core.c
b/xen/arch/x86/cpu/microcode/core.c
index 6f95f7bbe223..1c9f66ea8a0f 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -685,12 +685,12 @@ static long cf_check microcode_update_helper(void *data)
* Disable CPUID masking if in use, to avoid having current's
* cpu_policy affect the rescan.
*/
- if ( ctxt_switch_masking )
+if ( ctxt_switch_masking )
alternative_vcall(ctxt_switch_masking, NULL);
calculate_raw_cpu_policy();
- if ( ctxt_switch_masking )
+if ( ctxt_switch_masking )
alternative_vcall(ctxt_switch_masking, current);
}
else
base-commit: 3f819af8a796c0e2f798dd301ec8c3f8cccbc9fc
--
2.30.2
Re: [PATCH v3 31/34] xen/riscv: add minimal stuff to mm.h to build full Xen
On Tue, 2024-01-23 at 14:03 +0100, Jan Beulich wrote:
> On 22.12.2023 16:13, Oleksii Kurochko wrote:
> > Signed-off-by: Oleksii Kurochko
> > ---
> > Changes in V3:
> > - update the commit message
>
> ??? (yet again)
>
> > --- a/xen/arch/riscv/include/asm/mm.h
> > +++ b/xen/arch/riscv/include/asm/mm.h
> > @@ -3,8 +3,251 @@
> > #ifndef _ASM_RISCV_MM_H
> > #define _ASM_RISCV_MM_H
> >
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > #include
> >
> > +#define paddr_to_pdx(pa) mfn_to_pdx(maddr_to_mfn(pa))
> > +#define gfn_to_gaddr(gfn) pfn_to_paddr(gfn_x(gfn))
> > +#define gaddr_to_gfn(ga) _gfn(paddr_to_pfn(ga))
> > +#define mfn_to_maddr(mfn) pfn_to_paddr(mfn_x(mfn))
> > +#define maddr_to_mfn(ma) _mfn(paddr_to_pfn(ma))
> > +#define vmap_to_mfn(va)
> > maddr_to_mfn(virt_to_maddr((vaddr_t)va))
> > +#define vmap_to_page(va) mfn_to_page(vmap_to_mfn(va))
>
> Everything you have above ...
>
> > +#define paddr_to_pdx(pa) mfn_to_pdx(maddr_to_mfn(pa))
> > +#define gfn_to_gaddr(gfn) pfn_to_paddr(gfn_x(gfn))
> > +#define gaddr_to_gfn(ga) _gfn(paddr_to_pfn(ga))
> > +#define mfn_to_maddr(mfn) pfn_to_paddr(mfn_x(mfn))
> > +#define maddr_to_mfn(ma) _mfn(paddr_to_pfn(ma))
> > +#define vmap_to_mfn(va)
> > maddr_to_mfn(virt_to_maddr((vaddr_t)va))
> > +#define vmap_to_page(va) mfn_to_page(vmap_to_mfn(va))
>
> ... appears a 2nd time right afterwards.
>
> > +#define virt_to_maddr(va) ((paddr_t)((vaddr_t)(va) & PADDR_MASK))
> > +#define maddr_to_virt(pa) ((void *)((paddr_t)(pa) |
> > DIRECTMAP_VIRT_START))
> > +
> > +/* Convert between Xen-heap virtual addresses and machine frame
> > numbers. */
> > +#define __virt_to_mfn(va) (virt_to_maddr(va) >> PAGE_SHIFT)
> > +#define __mfn_to_virt(mfn) maddr_to_virt((paddr_t)(mfn) <<
> > PAGE_SHIFT)
>
> These would imo better use maddr_to_mfn() and mfn_to_maddr(), rather
> than
> kind of open-coding them. The former could also use PFN_DOWN() as an
> alternative.
We can't to as __virt_to_mfn() when is used it is usually wrapped by
_mfn() which expect to have unsigned long as an argument.
>
> > +/* Convert between Xen-heap virtual addresses and page-info
> > structures. */
> > +static inline struct page_info *virt_to_page(const void *v)
> > +{
> > + BUG();
> > + return NULL;
> > +}
> > +
> > +/*
> > + * We define non-underscored wrappers for above conversion
> > functions.
> > + * These are overriden in various source files while underscored
> > version
> > + * remain intact.
> > + */
> > +#define virt_to_mfn(va) __virt_to_mfn(va)
> > +#define mfn_to_virt(mfn) __mfn_to_virt(mfn)
>
> Is this really still needed? Would be pretty nice if in a new port we
> could get to start cleanly right away (i.e. by not needing per-file
> overrides, but using type-safe expansions here right away).
We still need __virt_to_mfn and __mfn_to_virt as common code use them:
* xen/common/xenoprof.c:24:#define virt_to_mfn(va)
mfn(__virt_to_mfn(va))
* xen/include/xen/domain_page.h:59:#define domain_page_map_to_mfn(ptr)
_mfn(__virt_to_mfn((unsigned long)(ptr)))
~ Oleksii
>
> > +struct page_info
> > +{
> > + /* Each frame can be threaded onto a doubly-linked list. */
> > + struct page_list_entry list;
> > +
> > + /* Reference count and various PGC_xxx flags and fields. */
> > + unsigned long count_info;
> > +
> > + /* Context-dependent fields follow... */
> > + union {
> > + /* Page is in use: ((count_info & PGC_count_mask) != 0).
> > */
> > + struct {
> > + /* Type reference count and various PGT_xxx flags and
> > fields. */
> > + unsigned long type_info;
> > + } inuse;
> > + /* Page is on a free list: ((count_info & PGC_count_mask)
> > == 0). */
> > + union {
> > + struct {
> > + /*
> > + * Index of the first *possibly* unscrubbed page
> > in the buddy.
> > + * One more bit than maximum possible order to
> > accommodate
> > + * INVALID_DIRTY_IDX.
> > + */
> > +#define INVALID_DIRTY_IDX ((1UL << (MAX_ORDER + 1)) - 1)
> > + unsigned long first_dirty:MAX_ORDER + 1;
> > +
> > + /* Do TLBs need flushing for safety before next
> > page use? */
> > + bool need_tlbflush:1;
> > +
> > +#define BUDDY_NOT_SCRUBBING 0
> > +#define BUDDY_SCRUBBING 1
> > +#define BUDDY_SCRUB_ABORT 2
> > + unsigned long scrub_state:2;
> > + };
> > +
> > + unsigned long val;
> > + } free;
>
> Indentation is wrong (and thus misleading) for these two lines.
>
> > +
> > + } u;
>
> Nit: I don't see the value of the trailing blank line inside the
> union.
>
> > + union {
> > + /* Page is in use, but not as a shadow. */
>
> I question the appicability of "shadow" here.
>
> > + struct {
> > + /* Owner of this page (zero if page is anonymous). */
> > +
Re: [PATCH v3 21/33] tools: add 9pfs device to xenstore-stubdom
On 15.01.24 16:31, Anthony PERARD wrote: On Thu, Jan 04, 2024 at 10:00:43AM +0100, Juergen Gross wrote: Add a 9pfs device to Xenstore stubdom in order to allow it to do e.g. logging into a dom0 file. Use the following parameters for the new device: - tag = "xen" Is it ok to have here tag "xen" when the default tag is "Xen" ? It is okay, but I agree it should be "Xen". I'll change it. diff --git a/tools/include/libxl.h b/tools/include/libxl.h index 907aa0a330..00693264f7 100644 --- a/tools/include/libxl.h +++ b/tools/include/libxl.h @@ -2060,6 +2067,16 @@ int libxl_console_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend, const libxl_asyncop_how *ao_how) LIBXL_EXTERNAL_CALLERS_ONLY; +/* libxl_p9_add_xenstore writes the Xenstore entries for a domain's + * primary 9pfs device based on domid, backend type and device parameters. + */ +int libxl_p9_add_xenstore(libxl_ctx *ctx, uint32_t domid, uint32_t backend, + libxl_p9_type type, char *tag, char *path, + char *security_model, unsigned int max_space, + unsigned int max_files, unsigned int max_open_files, + bool auto_delete, const libxl_asyncop_how *ao_how) Could we simply pass a "libxl_device_p9*" instead of all these parameters? It would also mean that we can update the list of parameters without having to change the function prototype. Fine with me. These functions tend to be called "libxl_device_*_add()", is it possible to follow the same schema? In particular, I don't see anything xenstore specific in the function. It was meant to be similar to libxl_console_add_xenstore(), which just writes the Xenstore contents of the device. I think you are right that libxl_device_9pfs_add() is a better name. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH v3 17/33] tools/xl: support new 9pfs backend xen-9pfsd
On 02.02.24 16:28, Juergen Gross wrote:
On 31.01.24 16:20, Jürgen Groß wrote:
On 15.01.24 16:14, Anthony PERARD wrote:
On Thu, Jan 04, 2024 at 10:00:39AM +0100, Juergen Gross wrote:
@@ -2242,6 +2256,28 @@ void parse_config_data(const char *config_source,
libxl_string_list_dispose();
+ if (p9->type == LIBXL_P9_TYPE_UNKNOWN) {
+ p9->type = LIBXL_P9_TYPE_QEMU;
The defaulting is normally done in libxl, so that it works for all users
of libxl. Can this be done instead in libxl? Hopefully, it's enough to
do it in libxl__device_p9_setdefault().
Same question for the followup checks and default values.
I'll look into it.
Turns out this would need some hack, as I have no access to the name of
the domain being created in libxl__device_p9_setdefault(). And this is needed
for setting the path default value.
Do you have any idea how to resolve this issue, or are you fine to keep the
patch as is?
Just found a hopefully acceptable solution: if the type is XEN_9PFSD and no
path has been specified I set it in xl_parse.c. The rest of the default
settings and consistency checks are done in libxl.
Is this okay for you?
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
[PATCH] xen/bitmap: Deduplicate __bitmap_weight() implementations
We have two copies of __bitmap_weight() that differ by whether they make
hweight32() or hweight64() calls, yet we already have hweight_long() which is
the form that __bitmap_weight() wants.
Fix hweight_long() to return unsigned int like all the other hweight helpers,
and fix __bitmap_weight() to used unsigned integers.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: George Dunlap
CC: Jan Beulich
CC: Stefano Stabellini
CC: Wei Liu
CC: Julien Grall
---
xen/common/bitmap.c | 21 +++--
xen/include/xen/bitops.h | 2 +-
2 files changed, 4 insertions(+), 19 deletions(-)
diff --git a/xen/common/bitmap.c b/xen/common/bitmap.c
index c57b35f0042c..9d9ff09f3dde 100644
--- a/xen/common/bitmap.c
+++ b/xen/common/bitmap.c
@@ -186,33 +186,18 @@ int __bitmap_subset(const unsigned long *bitmap1,
}
EXPORT_SYMBOL(__bitmap_subset);
-#if BITS_PER_LONG == 32
unsigned int __bitmap_weight(const unsigned long *bitmap, unsigned int bits)
{
- int k, w = 0, lim = bits/BITS_PER_LONG;
+ unsigned int k, w = 0, lim = bits / BITS_PER_LONG;
for (k = 0; k < lim; k++)
- w += hweight32(bitmap[k]);
+ w += hweight_long(bitmap[k]);
if (bits % BITS_PER_LONG)
- w += hweight32(bitmap[k] & BITMAP_LAST_WORD_MASK(bits));
+ w += hweight_long(bitmap[k] & BITMAP_LAST_WORD_MASK(bits));
return w;
}
-#else
-unsigned int __bitmap_weight(const unsigned long *bitmap, unsigned int bits)
-{
- int k, w = 0, lim = bits/BITS_PER_LONG;
-
- for (k = 0; k < lim; k++)
- w += hweight64(bitmap[k]);
-
- if (bits % BITS_PER_LONG)
- w += hweight64(bitmap[k] & BITMAP_LAST_WORD_MASK(bits));
-
- return w;
-}
-#endif
EXPORT_SYMBOL(__bitmap_weight);
void __bitmap_set(unsigned long *map, unsigned int start, int len)
diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h
index a88d45475c40..2cb7892bcca0 100644
--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -199,7 +199,7 @@ static inline unsigned int generic_hweight64(uint64_t w)
return (w + (w >> 32)) & 0xFF;
}
-static inline unsigned long hweight_long(unsigned long w)
+static inline unsigned int hweight_long(unsigned long w)
{
return sizeof(w) == 4 ? generic_hweight32(w) : generic_hweight64(w);
}
--
2.30.2
Re: [PATCH v3 17/33] tools/xl: support new 9pfs backend xen-9pfsd
On 31.01.24 16:20, Jürgen Groß wrote:
On 15.01.24 16:14, Anthony PERARD wrote:
On Thu, Jan 04, 2024 at 10:00:39AM +0100, Juergen Gross wrote:
@@ -2242,6 +2256,28 @@ void parse_config_data(const char *config_source,
libxl_string_list_dispose();
+ if (p9->type == LIBXL_P9_TYPE_UNKNOWN) {
+ p9->type = LIBXL_P9_TYPE_QEMU;
The defaulting is normally done in libxl, so that it works for all users
of libxl. Can this be done instead in libxl? Hopefully, it's enough to
do it in libxl__device_p9_setdefault().
Same question for the followup checks and default values.
I'll look into it.
Turns out this would need some hack, as I have no access to the name of
the domain being created in libxl__device_p9_setdefault(). And this is needed
for setting the path default value.
Do you have any idea how to resolve this issue, or are you fine to keep the
patch as is?
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3 0/4] address violation of MISRA C:2012 Rule 13.1
On 02/02/24 10:37, Simone Ballarin wrote: From: Maria Celeste Cesario The Xen sources contain violations of MISRA C:2012 Rule 13.1 whose headline states: "Initializer lists shall not contain persistent side effects". The file properties.json containing function and macro properties is introduced, as stated in v2 discussion. Some functions and macros are found to have properties that can be exploited by static analyzers. For this reason, the file docs/properties.json contains all the needed properties. A description of the json file is documented in docs/properties.rst. Some persistent effects have been moved outside initializer lists to address violations of Rule 13.1. Link to the discussion: https://lore.kernel.org/all/cover.1700844359.git.simone.balla...@bugseng.com/T/#u Changes in v3: - change prefix from xen to xen/ns16550 - add assignment of rc in xen/ns16550 - use rc as controlling expression in the following if-statement - change commit prefix from xen/arm to xen - specify where saf-3-safe comments are applied in guestcopy.c - reword saf comments text Maria Celeste Cesario (1): eclair: add and manage properties Simone Ballarin (3): xen: add SAF deviation for debugging and logging effects xen/ns16550: address violations of MISRA C:2012 Rule 13.1 xen/x86: address violations of MISRA C:2012 Rule 13.1 .../eclair_analysis/ECLAIR/analysis.ecl | 1 + automation/eclair_analysis/prepare.sh | 2 + docs/misra/safe.json | 16 + docs/properties.json | 841 ++ docs/properties.rst | 58 ++ xen/arch/arm/device.c | 1 + xen/arch/arm/guestcopy.c | 16 +- xen/arch/x86/hvm/hvm.c| 1 + xen/arch/x86/io_apic.c| 9 +- xen/arch/x86/mpparse.c| 3 +- xen/arch/x86/setup.c | 3 +- xen/common/sched/core.c | 3 + xen/drivers/char/ns16550.c| 4 +- 13 files changed, 948 insertions(+), 10 deletions(-) create mode 100644 docs/properties.json create mode 100644 docs/properties.rst Sorry, patch "eclair: add and manage properties" is incomplete, please ignore this series: v4 has already been submitted. -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com)
[linux-linus test] 184554: tolerable FAIL - PUSHED
flight 184554 linux-linus real [real] flight 184565 linux-linus real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/184554/ http://logs.test-lab.xenproject.org/osstest/logs/184565/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-armhf-armhf-libvirt-qcow2 10 host-ping-check-xen fail pass in 184565-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail in 184565 like 184545 test-armhf-armhf-libvirt-qcow2 14 migrate-support-check fail in 184565 never pass test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184545 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184545 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184545 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184545 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184545 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184545 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184545 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-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-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-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass version targeted for testing: linux5c24e4e9e70822cf49955fc8174bc5efaa93d17f baseline version: linux6764c317b6bb91bd806ef79adf6d9c0e428b191e Last test of basis 184545 2024-01-31 22:37:40 Z1 days Testing same since 184554 2024-02-01 18:42:19 Z0 days1 attempts People who touched revisions under test: Adam Goldman Benjamin Tissoires Damian Muszynski Dan Carpenter Gaurav Jain Herbert Xu Jason Gerecke Jiri Kosina Johan Hovold Kai-Heng Feng Kunwu Chan Linus Torvalds Mark Brown Martin Blumenstingl Naresh Solanki Ondrej Mosnacek Patrick Rudolph Paul Moore Romain Naour Su Hui Takashi Sakamoto Wolfram Sang jobs: build-amd64-xsm pass
[XEN PATCH v4 2/4] xen/ns16550: address violations of MISRA C:2012 Rule 13.1
Rule 13.1: Initializer lists shall not contain persistent side effects
The assignment operation in:
.irq = rc = uart->irq,
is a persistent side effect in a struct initializer list.
This patch assigns rc separately outside the structure.
No functional change.
Signed-off-by: Simone Ballarin
Signed-off-by: Maria Celeste Cesario
---
Changes in v3:
- add assignment of rc;
- use rc as controlling expression in the following if-statement;
- change prefix from xen to xen/ns16550.
Changes in v2:
- avoid assignment of rc;
- drop changes in vcpu_yield(void).
---
xen/drivers/char/ns16550.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index afe3d514b9..97bf098534 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -441,10 +441,12 @@ static void __init cf_check ns16550_init_postirq(struct
serial_port *port)
struct msi_info msi = {
.sbdf = PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2]),
-.irq = rc = uart->irq,
+.irq = uart->irq,
.entry_nr = 1
};
+rc = uart->irq;
+
if ( rc > 0 )
{
struct msi_desc *msi_desc = NULL;
--
2.34.1
[XEN PATCH v4 0/4] address violation of MISRA C:2012 Rule 13.1
The Xen sources contain violations of MISRA C:2012 Rule 13.1 whose headline states: "Initializer lists shall not contain persistent side effects". The file properties.json containing function and macro properties is introduced, as stated in v2 discussion. Some functions and macros are found to have properties that can be exploited by static analyzers. For this reason, the file docs/properties.json contains all the needed properties. A description of the json file is documented in docs/properties.rst. Some persistent effects have been moved outside initializer lists to address violations of Rule 13.1. Link to the discussion: https://lore.kernel.org/all/cover.1700844359.git.simone.balla...@bugseng.com/T/#u Changes in v4: - added missing script for converting function_macro_properties.json in ECL configurations and other related improvements. Changes in v3: - change prefix from xen to xen/ns16550 - add assignment of rc in xen/ns16550 - use rc as controlling expression in the following if-statement - change commit prefix from xen/arm to xen - specify where saf-3-safe comments are applied in guestcopy.c - reword saf comments text Maria Celeste Cesario (1): eclair: move function and macro properties outside ECLAIR Simone Ballarin (3): xen: add SAF deviation for debugging and logging effects xen/ns16550: address violations of MISRA C:2012 Rule 13.1 xen/x86: address violations of MISRA C:2012 Rule 13.1 .../eclair_analysis/ECLAIR/analysis.ecl | 1 + .../ECLAIR/call_properties.ecl| 128 --- automation/eclair_analysis/prepare.sh | 2 + automation/eclair_analysis/propertyparser.py | 37 + docs/function_macro_properties.json | 841 ++ docs/function_macro_properties.rst| 58 ++ docs/misra/safe.json | 16 + xen/arch/arm/device.c | 1 + xen/arch/arm/guestcopy.c | 16 +- xen/arch/x86/hvm/hvm.c| 1 + xen/arch/x86/io_apic.c| 9 +- xen/arch/x86/mpparse.c| 3 +- xen/arch/x86/setup.c | 3 +- xen/common/sched/core.c | 3 + xen/drivers/char/ns16550.c| 4 +- 15 files changed, 985 insertions(+), 138 deletions(-) delete mode 100644 automation/eclair_analysis/ECLAIR/call_properties.ecl create mode 100644 automation/eclair_analysis/propertyparser.py create mode 100644 docs/function_macro_properties.json create mode 100644 docs/function_macro_properties.rst -- 2.34.1
[XEN PATCH v4 4/4] eclair: move function and macro properties outside ECLAIR
From: Maria Celeste Cesario
Function and macro properties contained in ECLAIR/call_properties.ecl are of
general interest: this patch moves these annotations in a generaric JSON file
in docs. In this way, they can be exploited for other purposes (i.e.
documentation,
other tools).
Add rst file containing explanation on how to update
function_macro_properties.json.
Add script to convert the JSON file in ECL configurations.
Remove ECLAIR/call_properties.ecl: the file is now automatically generated from
the JSON file.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
---
Changes in v4:
- add missing script for converting the JSON file in ECL configurations;
- improve commit message;
- remove call_properties.ecs.
---
.../eclair_analysis/ECLAIR/analysis.ecl | 1 +
.../ECLAIR/call_properties.ecl| 128 ---
automation/eclair_analysis/prepare.sh | 2 +
automation/eclair_analysis/propertyparser.py | 37 +
docs/function_macro_properties.json | 841 ++
docs/function_macro_properties.rst| 58 ++
6 files changed, 939 insertions(+), 128 deletions(-)
delete mode 100644 automation/eclair_analysis/ECLAIR/call_properties.ecl
create mode 100644 automation/eclair_analysis/propertyparser.py
create mode 100644 docs/function_macro_properties.json
create mode 100644 docs/function_macro_properties.rst
diff --git a/automation/eclair_analysis/ECLAIR/analysis.ecl
b/automation/eclair_analysis/ECLAIR/analysis.ecl
index a604582da3..684c5b0b39 100644
--- a/automation/eclair_analysis/ECLAIR/analysis.ecl
+++ b/automation/eclair_analysis/ECLAIR/analysis.ecl
@@ -30,6 +30,7 @@ if(not(scheduled_analysis),
-eval_file=deviations.ecl
-eval_file=call_properties.ecl
-eval_file=tagging.ecl
+-eval_file=properties.ecl
-eval_file=concat(set,".ecl")
-doc="Hide reports in external code."
diff --git a/automation/eclair_analysis/ECLAIR/call_properties.ecl
b/automation/eclair_analysis/ECLAIR/call_properties.ecl
deleted file mode 100644
index c2b2a6182e..00
--- a/automation/eclair_analysis/ECLAIR/call_properties.ecl
+++ /dev/null
@@ -1,128 +0,0 @@
-
--call_properties+={"name(printk)", {"pointee_write(1..=never)", "taken()"}}
--call_properties+={"name(debugtrace_printk)", {"pointee_write(1..=never)",
"taken()"}}
--call_properties+={"name(panic)", {"pointee_write(1..=never)", "taken()"}}
--call_properties+={"macro(^domain_crash$)", {"pointee_write(2..=never)",
"taken()"}}
--call_properties+={"macro(^(g?d|mm_)?printk$)", {"pointee_write(2..=never)",
"taken()"}}
--call_properties+={"macro(^guest_bug_on_failed$)", {"pointee_write(1=never)",
"taken()"}}
--call_properties+={"macro(^spin_lock_init_prof$)", {"pointee_write(2=never)",
"taken()"}}
--call_properties+={"macro(^sched_test_func$)", {"pointee_write(1..=never)",
"taken()"}}
--call_properties+={"macro(^dev_(info|warn)$)", {"pointee_write(1..=never)",
"taken()"}}
--call_properties+={"macro(^PAGING_DEBUG$)", {"pointee_write(1..=never)",
"taken()"}}
--call_properties+={"macro(^ACPI_(WARNING|ERROR|INFO)$)",
{"pointee_write(1..=never)", "taken()"}}
--call_properties+={"name(fdt_get_property_by_offset_)",
{"pointee_write(3=always)", "pointee_read(3=never)", "taken()"}}
--call_properties+={"name(read_atomic_size)", {"pointee_write(2=always)",
"pointee_read(2=never)", "taken()"}}
--call_properties+={"name(device_tree_get_reg)", {"pointee_write(4..=always)",
"pointee_read(4..=never)", "taken()"}}
--call_properties+={"name(dt_get_range)", {"pointee_write(3..=always)",
"pointee_read(3..=never)", "taken()"}}
--call_properties+={"name(parse_static_mem_prop)",
{"pointee_write(2..=always)", "pointee_read(2..=never)", "taken()"}}
--call_properties+={"name(get_ttbr_and_gran_64bit)",
{"pointee_write(1..2=always)", "pointee_read(1..2=never)", "taken()"}}
--call_properties+={"name(hvm_emulate_init_once)", {"pointee_write(1=always)",
"pointee_read(1=never)", "taken()"}}
--call_properties+={"name(__vmread)", {"pointee_write(2=always)",
"pointee_read(2=never)", "taken()"}}
--call_properties+={"name(hvm_pci_decode_addr)", {"pointee_write(3=always)",
"pointee_read(3=never)", "taken()"}}
--call_properties+={"name(vpci_mmcfg_decode_addr)", {"pointee_write(3=always)",
"pointee_read(3=never)", "taken()"}}
--call_properties+={"name(x86emul_decode)", {"pointee_write(1=always)",
"pointee_read(1=never)", "taken()"}}
--call_properties+={"name(unmap_grant_ref)", {"pointee_write(2=always)",
"pointee_read(2=never)", "taken()"}}
--call_properties+={"name(arm_smmu_cmdq_build_cmd)",
{"pointee_write(1=always)", "pointee_read(1=never)", "taken()"}}
--call_properties+={"name(pci_size_mem_bar)", {"pointee_write(4=always)",
"pointee_read(4=never)", "taken()"}}
--call_properties+={"name(_hvm_read_entry)", {"pointee_write(2=always)",
"pointee_read(2=never)", "taken()"}}
--call_properties+={"name(hvm_map_guest_frame_rw)", {"pointee_write(3=always)",
"pointee_read(3=never)", "taken()"}}
[XEN PATCH v4 3/4] xen/x86: address violations of MISRA C:2012 Rule 13.1
Rule 13.1: Initializer lists shall not contain persistent side effects
This patch moves expressions with side-effects into new variables before
the initializer lists.
No functional changes.
Signed-off-by: Simone Ballarin
---
xen/arch/x86/io_apic.c | 9 ++---
xen/arch/x86/mpparse.c | 3 ++-
xen/arch/x86/setup.c | 3 ++-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index b48a642465..4a6ab85689 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2559,9 +2559,12 @@ integer_param("max_gsi_irqs", max_gsi_irqs);
static __init bool bad_ioapic_register(unsigned int idx)
{
-union IO_APIC_reg_00 reg_00 = { .raw = io_apic_read(idx, 0) };
-union IO_APIC_reg_01 reg_01 = { .raw = io_apic_read(idx, 1) };
-union IO_APIC_reg_02 reg_02 = { .raw = io_apic_read(idx, 2) };
+uint32_t reg_00_raw = io_apic_read(idx, 0);
+uint32_t reg_01_raw = io_apic_read(idx, 1);
+uint32_t reg_02_raw = io_apic_read(idx, 2);
+union IO_APIC_reg_00 reg_00 = { .raw = reg_00_raw };
+union IO_APIC_reg_01 reg_01 = { .raw = reg_01_raw };
+union IO_APIC_reg_02 reg_02 = { .raw = reg_02_raw };
if ( reg_00.raw == -1 && reg_01.raw == -1 && reg_02.raw == -1 )
{
diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
index d8ccab2449..81a819403b 100644
--- a/xen/arch/x86/mpparse.c
+++ b/xen/arch/x86/mpparse.c
@@ -798,11 +798,12 @@ void __init mp_register_lapic_address (
int mp_register_lapic(u32 id, bool enabled, bool hotplug)
{
+ u32 apic = apic_read(APIC_LVR);
struct mpc_config_processor processor = {
.mpc_type = MP_PROCESSOR,
/* Note: We don't fill in fields not consumed anywhere. */
.mpc_apicid = id,
- .mpc_apicver = GET_APIC_VERSION(apic_read(APIC_LVR)),
+ .mpc_apicver = GET_APIC_VERSION(apic),
.mpc_cpuflag = (enabled ? CPU_ENABLED : 0) |
(id == boot_cpu_physical_apicid ?
CPU_BOOTPROCESSOR : 0),
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ee682dd136..886031d86a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -885,13 +885,14 @@ static struct domain *__init create_dom0(const module_t
*image,
{
static char __initdata cmdline[MAX_GUEST_CMDLINE];
+unsigned int max_vcpus = dom0_max_vcpus();
struct xen_domctl_createdomain dom0_cfg = {
.flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : 0,
.max_evtchn_port = -1,
.max_grant_frames = -1,
.max_maptrack_frames = -1,
.grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
-.max_vcpus = dom0_max_vcpus(),
+.max_vcpus = max_vcpus,
.arch = {
.misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
},
--
2.34.1
[XEN PATCH v4 1/4] xen: add SAF deviation for debugging and logging effects
Rule 13.1: Initializer lists shall not contain persistent side effects
Effects caused by debug/logging macros and functions (like ASSERT,
__bad_atomic_size,
LOG, etc ...) that crash execution or produce logs are not dangerous in
initializer
lists. The evaluation order in abnormal conditions is not relevant. Evaluation
order
of logging effects is always safe.
Function hvm_get_guest_tsc_fixed (indirectly) performs different side effects.
For example it calls hvm_get_guest_time_fixed that contains an ASSERT and calls
to spin_lock and spin_unlock.
These side effects are not dangerous: they can be executed regardless of the
initializer list evaluation order
This patch deviates violations using SAF commits caused by debug/logging macros
and
functions.
Asm volatile statements in initializer lists that do not perform any persistent
side
effect are safe: this patch deviates violations caused by uses of the current
macro
(that contains an asm volatile) in initializer lists.
No functional changes.
Signed-off-by: Simone Ballarin
Signed-off-by: Maria Celeste Cesario
---
Changes in v3:
- change commit prefix from xen/arm to xen
- specify where saf-3-safe comments are applied in guestcopy.c
- reword SAF text
Changes in v2:
New patch based on the discussion for "xen/arm: address violations of MISRA
C:2012 Rule 13.1".
---
docs/misra/safe.json | 16
xen/arch/arm/device.c| 1 +
xen/arch/arm/guestcopy.c | 16
xen/arch/x86/hvm/hvm.c | 1 +
xen/common/sched/core.c | 3 +++
5 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 952324f85c..5539e8dfda 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -28,6 +28,22 @@
},
{
"id": "SAF-3-safe",
+"analyser": {
+"eclair": "MC3R1.R13.1"
+},
+"name": "MC3R1.R13.1: effects for debugging and logging",
+"text": "Effects for debugging and loggings reasons that crash
execution or produce logs are allowed in initializer lists. The evaluation
order in abnormal conditions is not relevant."
+},
+{
+"id": "SAF-4-safe",
+"analyser": {
+"eclair": "MC3R1.R13.1"
+},
+"name": "MC3R1.R13.1: volatile asm statements that do not perform
any persistent side effect",
+"text": "Volatile asm statement in an initializer list that does
not perform persistent side effects is safe."
+},
+{
+"id": "SAF-5-safe",
"analyser": {},
"name": "Sentinel",
"text": "Next ID to be used"
diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 1f631d3274..fa331f164d 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -331,6 +331,7 @@ int handle_device(struct domain *d, struct dt_device_node
*dev, p2m_type_t p2mt,
.p2mt = p2mt,
.skip_mapping = !own_device ||
(is_pci_passthrough_enabled() &&
+/* SAF-3-safe effects for debugging/logging reasons
are safe */
(device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),
.iomem_ranges = iomem_ranges,
.irq_ranges = irq_ranges
diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 6716b03561..b75538252a 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -110,26 +110,34 @@ static unsigned long copy_guest(void *buf, uint64_t addr,
unsigned int len,
unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
{
return copy_guest((void *)from, (vaddr_t)to, len,
- GVA_INFO(current), COPY_to_guest | COPY_linear);
+ /* SAF-4-safe No persistent side effects */
+ GVA_INFO(current),
+ COPY_to_guest | COPY_linear);
}
unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
unsigned int len)
{
-return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
+return copy_guest((void *)from, (vaddr_t)to, len,
+ /* SAF-4-safe No persistent side effects */
+ GVA_INFO(current),
COPY_to_guest | COPY_flush_dcache | COPY_linear);
}
unsigned long raw_clear_guest(void *to, unsigned int len)
{
-return copy_guest(NULL, (vaddr_t)to, len, GVA_INFO(current),
+return copy_guest(NULL, (vaddr_t)to, len,
+ /* SAF-4-safe No persistent side effects */
+ GVA_INFO(current),
COPY_to_guest | COPY_linear);
}
unsigned long raw_copy_from_guest(void *to, const void __user *from,
unsigned int len)
{
-return copy_guest(to, (vaddr_t)from, len, GVA_INFO(current),
+return copy_guest(to, (vaddr_t)from, len,
+
Community call recording: 1st February 2024
Hi all, Please find the community call recording below. https://www.youtube.com/watch?v=MJSTvwvNtF8_channel=TheXenProject These are unlisted and published on our new YouTube channel. Only users with the specific link below will be able to access the recording. This has also been saved in the agenda notes. Many thanks, Kelly Choi Community Manager Xen Project
Re: [PATCH] libxl: Add "grant_usage" parameter for virtio disk devices
On 02.02.24 13:03, Viresh Kumar wrote: Hello Viresh > On 02-02-24, 12:49, Oleksandr Tyshchenko wrote: >> diff --git a/docs/man/xl-disk-configuration.5.pod.in >> b/docs/man/xl-disk-configuration.5.pod.in >> index bc945cc517..3c035456d5 100644 >> --- a/docs/man/xl-disk-configuration.5.pod.in >> +++ b/docs/man/xl-disk-configuration.5.pod.in >> @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please >> note, the virtual >> device (vdev) is not passed to the guest in that case, but it still must be >> specified for the internal purposes. >> >> +=item B >> + >> +=over 4 >> + >> +=item Description >> + >> +Specifies the usage of Xen grants for accessing guest memory. Only >> applicable >> +to specification "virtio". >> + >> +=item Supported values >> + >> +If this option is B, the Xen grants are always enabled. >> +If this option is B, the Xen grants are always disabled. >> + >> +=item Mandatory >> + >> +No >> + >> +=item Default value >> + >> +If this option is missing, then the default grant setting will be used, >> +i.e. enable grants if backend-domid != 0. >> + >> +=back >> + >> =back >> >> =head1 COLO Parameters > > I wonder if there is a way to avoid the duplication here and use the > definition > from: docs/man/xl.cfg.5.pod.in somehow ? That's good point. I am not 100% sure, but if we could use something like that it would be really nice. Let's see what other reviewers will say. =item B =over 4 Specifies the usage of Xen grants for accessing guest memory. Only applicable to specification "virtio". Please see B in L for more information on this option. =back
Re: [PATCH v2] x86/CPU: convert vendor hook invocations to altcall
On 02.02.2024 13:13, Andrew Cooper wrote: > On 23/01/2024 11:00 am, Jan Beulich wrote: >> While not performance critical, these hook invocations still want >> converting: This way all pre-filled struct cpu_dev instances can become >> __initconst_cf_clobber, thus allowing to eliminate further 8 ENDBR >> during the 2nd phase of alternatives patching (besides moving previously >> resident data to .init.*). >> >> Since all use sites need touching anyway, take the opportunity and also >> address a Misra C:2012 Rule 5.5 violation: Rename the this_cpu static >> variable. >> >> Signed-off-by: Jan Beulich > > Acked-by: Andrew Cooper Thanks. >> --- >> With LTO it might end up necessary to tag as __used more than just >> "default_cpu". > > Why is it even needed here? > > LTO can't rid early_cpu_init() of the default clause, so can't make > default_cpu unreferenced, I don't think. Even without LTO I've actually seen gcc eliminate default_cpu. The use in early_cpu_init() simply was expanded to assignments, rather then something memcpy()-like. >> Perhaps __used would better be integrated into __initconst_cf_clobber, >> to be independent of the compiler potentially eliding structure >> instances. > > Maybe. I guess the issue here is that the tools really can't see the > connection between being in the clobber section, and alternatives going > and making a modification. The const-ness is the issue, I expect. From that the compiler can infer that it may transform use of the variable into something which in the end doesn't require the variable anymore. And it can't know that we use all contributions to that section for a second purpose. Jan
Re: [PATCH v2] x86/CPU: convert vendor hook invocations to altcall
On 23/01/2024 11:00 am, Jan Beulich wrote: > While not performance critical, these hook invocations still want > converting: This way all pre-filled struct cpu_dev instances can become > __initconst_cf_clobber, thus allowing to eliminate further 8 ENDBR > during the 2nd phase of alternatives patching (besides moving previously > resident data to .init.*). > > Since all use sites need touching anyway, take the opportunity and also > address a Misra C:2012 Rule 5.5 violation: Rename the this_cpu static > variable. > > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper > --- > With LTO it might end up necessary to tag as __used more than just > "default_cpu". Why is it even needed here? LTO can't rid early_cpu_init() of the default clause, so can't make default_cpu unreferenced, I don't think. > Perhaps __used would better be integrated into __initconst_cf_clobber, > to be independent of the compiler potentially eliding structure > instances. Maybe. I guess the issue here is that the tools really can't see the connection between being in the clobber section, and alternatives going and making a modification. ~Andrew
Re: [PATCH v3 4/4] eclair: add and manage properties
On 02/02/24 10:37, Simone Ballarin wrote:
From: Maria Celeste Cesario
Add JSON file containing properties.
Add rst file containing explanation on how to update properties.json.
Add instruction to eclair_analysis/prepare.sh to parse the JSON file.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
---
.../eclair_analysis/ECLAIR/analysis.ecl | 1 +
automation/eclair_analysis/prepare.sh | 2 +
docs/properties.json | 841 ++
docs/properties.rst | 58 ++
4 files changed, 902 insertions(+)
create mode 100644 docs/properties.json
create mode 100644 docs/properties.rst
diff --git a/automation/eclair_analysis/ECLAIR/analysis.ecl
b/automation/eclair_analysis/ECLAIR/analysis.ecl
index a604582da3..684c5b0b39 100644
--- a/automation/eclair_analysis/ECLAIR/analysis.ecl
+++ b/automation/eclair_analysis/ECLAIR/analysis.ecl
@@ -30,6 +30,7 @@ if(not(scheduled_analysis),
-eval_file=deviations.ecl
-eval_file=call_properties.ecl
-eval_file=tagging.ecl
+-eval_file=properties.ecl
-eval_file=concat(set,".ecl")
-doc="Hide reports in external code."
diff --git a/automation/eclair_analysis/prepare.sh
b/automation/eclair_analysis/prepare.sh
index fe9d16e48e..47b2a2f32a 100755
--- a/automation/eclair_analysis/prepare.sh
+++ b/automation/eclair_analysis/prepare.sh
@@ -43,4 +43,6 @@ fi
make -f "${script_dir}/Makefile.prepare" prepare
# Translate the /* SAF-n-safe */ comments into ECLAIR CBTs
scripts/xen-analysis.py --run-eclair --no-build --no-clean
+# Translate function-properties.json into ECLAIR properties
+python3 ${script_dir}/propertyparser.py
)
diff --git a/docs/properties.json b/docs/properties.json
new file mode 100644
index 00..74058297b5
--- /dev/null
+++ b/docs/properties.json
@@ -0,0 +1,841 @@
+{
+ "version": "1.0",
+ "content": [
+ {
+ "type": "function",
+ "value": "^printk.*$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^debugtrace_printk.*$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^panic.*$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^domain_crash$",
+ "properties":{
+"pointee_write": "2..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^(g?d|mm_)?printk$",
+ "properties":{
+"pointee_write": "2..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^guest_bug_on_failed$",
+ "properties":{
+"pointee_write": "1=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^spin_lock_init_prof$",
+ "properties":{
+"pointee_write": "2=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^sched_test_func$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^dev_(info|warn)$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^PAGING_DEBUG$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^ACPI_(WARNING|ERROR|INFO)$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^fdt_get_property_by_offset_.*$",
+ "properties":{
+"pointee_write": "3=always",
+"pointee_read": "3=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^read_atomic_size.*$",
+ "properties":{
+"pointee_write": "2=always",
+"pointee_read": "2=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^device_tree_get_reg.*$",
+ "properties":{
+"pointee_write": "4..=always",
+"pointee_read": "4..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^dt_get_range.*$",
+ "properties":{
+"pointee_write": "3..=always",
+"pointee_read": "3..=never",
+"taken": ""
+ }
+ },
+
Re: [PATCH] libxl: Add "grant_usage" parameter for virtio disk devices
On 02-02-24, 12:49, Oleksandr Tyshchenko wrote: > diff --git a/docs/man/xl-disk-configuration.5.pod.in > b/docs/man/xl-disk-configuration.5.pod.in > index bc945cc517..3c035456d5 100644 > --- a/docs/man/xl-disk-configuration.5.pod.in > +++ b/docs/man/xl-disk-configuration.5.pod.in > @@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please > note, the virtual > device (vdev) is not passed to the guest in that case, but it still must be > specified for the internal purposes. > > +=item B > + > +=over 4 > + > +=item Description > + > +Specifies the usage of Xen grants for accessing guest memory. Only applicable > +to specification "virtio". > + > +=item Supported values > + > +If this option is B, the Xen grants are always enabled. > +If this option is B, the Xen grants are always disabled. > + > +=item Mandatory > + > +No > + > +=item Default value > + > +If this option is missing, then the default grant setting will be used, > +i.e. enable grants if backend-domid != 0. > + > +=back > + > =back > > =head1 COLO Parameters I wonder if there is a way to avoid the duplication here and use the definition from: docs/man/xl.cfg.5.pod.in somehow ? -- viresh
[PATCH] libxl: Add "grant_usage" parameter for virtio disk devices
From: Oleksandr Tyshchenko
Allow administrators to control whether Xen grant mappings for
the virtio disk devices should be used. By default (when new
parameter is not specified), the existing behavior is retained
(we enable grants if backend-domid != 0).
Signed-off-by: Oleksandr Tyshchenko
---
In addition to "libxl: arm: Add grant_usage parameter for virtio devices"
https://github.com/xen-project/xen/commit/c14254065ff4826e34f714e1790eab5217368c38
---
docs/man/xl-disk-configuration.5.pod.in | 25 +
tools/golang/xenlight/helpers.gen.go| 6 ++
tools/golang/xenlight/types.gen.go | 1 +
tools/include/libxl.h | 7 +++
tools/libs/light/libxl_arm.c| 4 ++--
tools/libs/light/libxl_disk.c | 13 +
tools/libs/light/libxl_types.idl| 1 +
tools/libs/util/libxlu_disk_l.l | 3 +++
8 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/docs/man/xl-disk-configuration.5.pod.in
b/docs/man/xl-disk-configuration.5.pod.in
index bc945cc517..3c035456d5 100644
--- a/docs/man/xl-disk-configuration.5.pod.in
+++ b/docs/man/xl-disk-configuration.5.pod.in
@@ -404,6 +404,31 @@ Virtio frontend driver (virtio-blk) to be used. Please
note, the virtual
device (vdev) is not passed to the guest in that case, but it still must be
specified for the internal purposes.
+=item B
+
+=over 4
+
+=item Description
+
+Specifies the usage of Xen grants for accessing guest memory. Only applicable
+to specification "virtio".
+
+=item Supported values
+
+If this option is B, the Xen grants are always enabled.
+If this option is B, the Xen grants are always disabled.
+
+=item Mandatory
+
+No
+
+=item Default value
+
+If this option is missing, then the default grant setting will be used,
+i.e. enable grants if backend-domid != 0.
+
+=back
+
=back
=head1 COLO Parameters
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 35e209ff1b..768ab0f566 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1879,6 +1879,9 @@ x.ActiveDisk = C.GoString(xc.active_disk)
x.HiddenDisk = C.GoString(xc.hidden_disk)
if err := x.Trusted.fromC();err != nil {
return fmt.Errorf("converting field Trusted: %v", err)
+}
+if err := x.GrantUsage.fromC(_usage);err != nil {
+return fmt.Errorf("converting field GrantUsage: %v", err)
}
return nil}
@@ -1927,6 +1930,9 @@ if x.HiddenDisk != "" {
xc.hidden_disk = C.CString(x.HiddenDisk)}
if err := x.Trusted.toC(); err != nil {
return fmt.Errorf("converting field Trusted: %v", err)
+}
+if err := x.GrantUsage.toC(_usage); err != nil {
+return fmt.Errorf("converting field GrantUsage: %v", err)
}
return nil
diff --git a/tools/golang/xenlight/types.gen.go
b/tools/golang/xenlight/types.gen.go
index 7907aa8999..0b712d2aa4 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -740,6 +740,7 @@ ColoExport string
ActiveDisk string
HiddenDisk string
Trusted Defbool
+GrantUsage Defbool
}
type DeviceNic struct {
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index f1652b1664..2b69e08466 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -578,6 +578,13 @@
*/
#define LIBXL_HAVE_DEVICE_DISK_SPECIFICATION 1
+/*
+ * LIBXL_HAVE_DISK_GRANT_USAGE indicates that the libxl_device_disk
+ * has 'grant_usage' field to specify the usage of Xen grants for
+ * the specification 'virtio'.
+ */
+#define LIBXL_HAVE_DISK_GRANT_USAGE 1
+
/*
* LIBXL_HAVE_CONSOLE_ADD_XENSTORE indicates presence of the function
* libxl_console_add_xenstore() in libxl.
diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index 1539191774..1cb89fa584 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -1372,12 +1372,12 @@ next_resize:
libxl_device_disk *disk = _config->disks[i];
if (disk->specification == LIBXL_DISK_SPECIFICATION_VIRTIO) {
-if (disk->backend_domid != LIBXL_TOOLSTACK_DOMID)
+if (libxl_defbool_val(disk->grant_usage))
iommu_needed = true;
FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq,
disk->backend_domid,
- disk->backend_domid !=
LIBXL_TOOLSTACK_DOMID) );
+
libxl_defbool_val(disk->grant_usage)) );
}
}
diff --git a/tools/libs/light/libxl_disk.c b/tools/libs/light/libxl_disk.c
index ea3623dd6f..f39f427091 100644
--- a/tools/libs/light/libxl_disk.c
+++ b/tools/libs/light/libxl_disk.c
@@ -181,6 +181,9 @@ static int libxl__device_disk_setdefault(libxl__gc *gc,
uint32_t domid,
return ERROR_INVAL;
}
disk->transport = LIBXL_DISK_TRANSPORT_MMIO;
+
+libxl_defbool_setdefault(>grant_usage,
+
[PATCH v3 2/4] xen/ns16550: address violations of MISRA C:2012 Rule 13.1
Rule 13.1: Initializer lists shall not contain persistent side effects
The assignment operation in:
.irq = rc = uart->irq,
is a persistent side effect in a struct initializer list.
This patch assigns rc separately outside the structure.
No functional change.
Signed-off-by: Simone Ballarin
Signed-off-by: Maria Celeste Cesario
---
Changes in v3:
- add assignment of rc;
- use rc as controlling expression in the following if-statement;
- change prefix from xen to xen/ns16550.
Changes in v2:
- avoid assignment of rc;
- drop changes in vcpu_yield(void).
---
xen/drivers/char/ns16550.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index afe3d514b9..97bf098534 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -441,10 +441,12 @@ static void __init cf_check ns16550_init_postirq(struct
serial_port *port)
struct msi_info msi = {
.sbdf = PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
uart->ps_bdf[2]),
-.irq = rc = uart->irq,
+.irq = uart->irq,
.entry_nr = 1
};
+rc = uart->irq;
+
if ( rc > 0 )
{
struct msi_desc *msi_desc = NULL;
--
2.40.0
[PATCH v3 4/4] eclair: add and manage properties
From: Maria Celeste Cesario
Add JSON file containing properties.
Add rst file containing explanation on how to update properties.json.
Add instruction to eclair_analysis/prepare.sh to parse the JSON file.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
---
.../eclair_analysis/ECLAIR/analysis.ecl | 1 +
automation/eclair_analysis/prepare.sh | 2 +
docs/properties.json | 841 ++
docs/properties.rst | 58 ++
4 files changed, 902 insertions(+)
create mode 100644 docs/properties.json
create mode 100644 docs/properties.rst
diff --git a/automation/eclair_analysis/ECLAIR/analysis.ecl
b/automation/eclair_analysis/ECLAIR/analysis.ecl
index a604582da3..684c5b0b39 100644
--- a/automation/eclair_analysis/ECLAIR/analysis.ecl
+++ b/automation/eclair_analysis/ECLAIR/analysis.ecl
@@ -30,6 +30,7 @@ if(not(scheduled_analysis),
-eval_file=deviations.ecl
-eval_file=call_properties.ecl
-eval_file=tagging.ecl
+-eval_file=properties.ecl
-eval_file=concat(set,".ecl")
-doc="Hide reports in external code."
diff --git a/automation/eclair_analysis/prepare.sh
b/automation/eclair_analysis/prepare.sh
index fe9d16e48e..47b2a2f32a 100755
--- a/automation/eclair_analysis/prepare.sh
+++ b/automation/eclair_analysis/prepare.sh
@@ -43,4 +43,6 @@ fi
make -f "${script_dir}/Makefile.prepare" prepare
# Translate the /* SAF-n-safe */ comments into ECLAIR CBTs
scripts/xen-analysis.py --run-eclair --no-build --no-clean
+# Translate function-properties.json into ECLAIR properties
+python3 ${script_dir}/propertyparser.py
)
diff --git a/docs/properties.json b/docs/properties.json
new file mode 100644
index 00..74058297b5
--- /dev/null
+++ b/docs/properties.json
@@ -0,0 +1,841 @@
+{
+ "version": "1.0",
+ "content": [
+ {
+ "type": "function",
+ "value": "^printk.*$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^debugtrace_printk.*$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^panic.*$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^domain_crash$",
+ "properties":{
+"pointee_write": "2..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^(g?d|mm_)?printk$",
+ "properties":{
+"pointee_write": "2..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^guest_bug_on_failed$",
+ "properties":{
+"pointee_write": "1=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^spin_lock_init_prof$",
+ "properties":{
+"pointee_write": "2=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^sched_test_func$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^dev_(info|warn)$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^PAGING_DEBUG$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "macro",
+ "value": "^ACPI_(WARNING|ERROR|INFO)$",
+ "properties":{
+"pointee_write": "1..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^fdt_get_property_by_offset_.*$",
+ "properties":{
+"pointee_write": "3=always",
+"pointee_read": "3=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^read_atomic_size.*$",
+ "properties":{
+"pointee_write": "2=always",
+"pointee_read": "2=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^device_tree_get_reg.*$",
+ "properties":{
+"pointee_write": "4..=always",
+"pointee_read": "4..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value": "^dt_get_range.*$",
+ "properties":{
+"pointee_write": "3..=always",
+"pointee_read": "3..=never",
+"taken": ""
+ }
+ },
+ {
+ "type": "function",
+ "value":
[PATCH v3 0/4] address violation of MISRA C:2012 Rule 13.1
From: Maria Celeste Cesario The Xen sources contain violations of MISRA C:2012 Rule 13.1 whose headline states: "Initializer lists shall not contain persistent side effects". The file properties.json containing function and macro properties is introduced, as stated in v2 discussion. Some functions and macros are found to have properties that can be exploited by static analyzers. For this reason, the file docs/properties.json contains all the needed properties. A description of the json file is documented in docs/properties.rst. Some persistent effects have been moved outside initializer lists to address violations of Rule 13.1. Link to the discussion: https://lore.kernel.org/all/cover.1700844359.git.simone.balla...@bugseng.com/T/#u Changes in v3: - change prefix from xen to xen/ns16550 - add assignment of rc in xen/ns16550 - use rc as controlling expression in the following if-statement - change commit prefix from xen/arm to xen - specify where saf-3-safe comments are applied in guestcopy.c - reword saf comments text Maria Celeste Cesario (1): eclair: add and manage properties Simone Ballarin (3): xen: add SAF deviation for debugging and logging effects xen/ns16550: address violations of MISRA C:2012 Rule 13.1 xen/x86: address violations of MISRA C:2012 Rule 13.1 .../eclair_analysis/ECLAIR/analysis.ecl | 1 + automation/eclair_analysis/prepare.sh | 2 + docs/misra/safe.json | 16 + docs/properties.json | 841 ++ docs/properties.rst | 58 ++ xen/arch/arm/device.c | 1 + xen/arch/arm/guestcopy.c | 16 +- xen/arch/x86/hvm/hvm.c| 1 + xen/arch/x86/io_apic.c| 9 +- xen/arch/x86/mpparse.c| 3 +- xen/arch/x86/setup.c | 3 +- xen/common/sched/core.c | 3 + xen/drivers/char/ns16550.c| 4 +- 13 files changed, 948 insertions(+), 10 deletions(-) create mode 100644 docs/properties.json create mode 100644 docs/properties.rst -- 2.40.0
[PATCH v3 1/4] xen: add SAF deviation for debugging and logging effects
Rule 13.1: Initializer lists shall not contain persistent side effects
Effects caused by debug/logging macros and functions (like ASSERT,
__bad_atomic_size,
LOG, etc ...) that crash execution or produce logs are not dangerous in
initializer
lists. The evaluation order in abnormal conditions is not relevant. Evaluation
order
of logging effects is always safe.
Function hvm_get_guest_tsc_fixed (indirectly) performs different side effects.
For example it calls hvm_get_guest_time_fixed that contains an ASSERT and calls
to spin_lock and spin_unlock.
These side effects are not dangerous: they can be executed regardless of the
initializer list evaluation order
This patch deviates violations using SAF commits caused by debug/logging macros
and
functions.
Asm volatile statements in initializer lists that do not perform any persistent
side
effect are safe: this patch deviates violations caused by uses of the current
macro
(that contains an asm volatile) in initializer lists.
No functional changes.
Signed-off-by: Simone Ballarin
Signed-off-by: Maria Celeste Cesario
---
Changes in v3:
- change commit prefix from xen/arm to xen
- specify where saf-3-safe comments are applied in guestcopy.c
- reword SAF text
Changes in v2:
New patch based on the discussion for "xen/arm: address violations of MISRA
C:2012 Rule 13.1".
---
docs/misra/safe.json | 16
xen/arch/arm/device.c| 1 +
xen/arch/arm/guestcopy.c | 16
xen/arch/x86/hvm/hvm.c | 1 +
xen/common/sched/core.c | 3 +++
5 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 952324f85c..5539e8dfda 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -28,6 +28,22 @@
},
{
"id": "SAF-3-safe",
+"analyser": {
+"eclair": "MC3R1.R13.1"
+},
+"name": "MC3R1.R13.1: effects for debugging and logging",
+"text": "Effects for debugging and loggings reasons that crash
execution or produce logs are allowed in initializer lists. The evaluation
order in abnormal conditions is not relevant."
+},
+{
+"id": "SAF-4-safe",
+"analyser": {
+"eclair": "MC3R1.R13.1"
+},
+"name": "MC3R1.R13.1: volatile asm statements that do not perform
any persistent side effect",
+"text": "Volatile asm statement in an initializer list that does
not perform persistent side effects is safe."
+},
+{
+"id": "SAF-5-safe",
"analyser": {},
"name": "Sentinel",
"text": "Next ID to be used"
diff --git a/xen/arch/arm/device.c b/xen/arch/arm/device.c
index 1f631d3274..fa331f164d 100644
--- a/xen/arch/arm/device.c
+++ b/xen/arch/arm/device.c
@@ -331,6 +331,7 @@ int handle_device(struct domain *d, struct dt_device_node
*dev, p2m_type_t p2mt,
.p2mt = p2mt,
.skip_mapping = !own_device ||
(is_pci_passthrough_enabled() &&
+/* SAF-3-safe effects for debugging/logging reasons
are safe */
(device_get_class(dev) == DEVICE_PCI_HOSTBRIDGE)),
.iomem_ranges = iomem_ranges,
.irq_ranges = irq_ranges
diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
index 6716b03561..b75538252a 100644
--- a/xen/arch/arm/guestcopy.c
+++ b/xen/arch/arm/guestcopy.c
@@ -110,26 +110,34 @@ static unsigned long copy_guest(void *buf, uint64_t addr,
unsigned int len,
unsigned long raw_copy_to_guest(void *to, const void *from, unsigned int len)
{
return copy_guest((void *)from, (vaddr_t)to, len,
- GVA_INFO(current), COPY_to_guest | COPY_linear);
+ /* SAF-4-safe No persistent side effects */
+ GVA_INFO(current),
+ COPY_to_guest | COPY_linear);
}
unsigned long raw_copy_to_guest_flush_dcache(void *to, const void *from,
unsigned int len)
{
-return copy_guest((void *)from, (vaddr_t)to, len, GVA_INFO(current),
+return copy_guest((void *)from, (vaddr_t)to, len,
+ /* SAF-4-safe No persistent side effects */
+ GVA_INFO(current),
COPY_to_guest | COPY_flush_dcache | COPY_linear);
}
unsigned long raw_clear_guest(void *to, unsigned int len)
{
-return copy_guest(NULL, (vaddr_t)to, len, GVA_INFO(current),
+return copy_guest(NULL, (vaddr_t)to, len,
+ /* SAF-4-safe No persistent side effects */
+ GVA_INFO(current),
COPY_to_guest | COPY_linear);
}
unsigned long raw_copy_from_guest(void *to, const void __user *from,
unsigned int len)
{
-return copy_guest(to, (vaddr_t)from, len, GVA_INFO(current),
+return copy_guest(to, (vaddr_t)from, len,
+
[PATCH v3 3/4] xen/x86: address violations of MISRA C:2012 Rule 13.1
Rule 13.1: Initializer lists shall not contain persistent side effects
This patch moves expressions with side-effects into new variables before
the initializer lists.
No functional changes.
Signed-off-by: Simone Ballarin
---
xen/arch/x86/io_apic.c | 9 ++---
xen/arch/x86/mpparse.c | 3 ++-
xen/arch/x86/setup.c | 3 ++-
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index b48a642465..4a6ab85689 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2559,9 +2559,12 @@ integer_param("max_gsi_irqs", max_gsi_irqs);
static __init bool bad_ioapic_register(unsigned int idx)
{
-union IO_APIC_reg_00 reg_00 = { .raw = io_apic_read(idx, 0) };
-union IO_APIC_reg_01 reg_01 = { .raw = io_apic_read(idx, 1) };
-union IO_APIC_reg_02 reg_02 = { .raw = io_apic_read(idx, 2) };
+uint32_t reg_00_raw = io_apic_read(idx, 0);
+uint32_t reg_01_raw = io_apic_read(idx, 1);
+uint32_t reg_02_raw = io_apic_read(idx, 2);
+union IO_APIC_reg_00 reg_00 = { .raw = reg_00_raw };
+union IO_APIC_reg_01 reg_01 = { .raw = reg_01_raw };
+union IO_APIC_reg_02 reg_02 = { .raw = reg_02_raw };
if ( reg_00.raw == -1 && reg_01.raw == -1 && reg_02.raw == -1 )
{
diff --git a/xen/arch/x86/mpparse.c b/xen/arch/x86/mpparse.c
index d8ccab2449..81a819403b 100644
--- a/xen/arch/x86/mpparse.c
+++ b/xen/arch/x86/mpparse.c
@@ -798,11 +798,12 @@ void __init mp_register_lapic_address (
int mp_register_lapic(u32 id, bool enabled, bool hotplug)
{
+ u32 apic = apic_read(APIC_LVR);
struct mpc_config_processor processor = {
.mpc_type = MP_PROCESSOR,
/* Note: We don't fill in fields not consumed anywhere. */
.mpc_apicid = id,
- .mpc_apicver = GET_APIC_VERSION(apic_read(APIC_LVR)),
+ .mpc_apicver = GET_APIC_VERSION(apic),
.mpc_cpuflag = (enabled ? CPU_ENABLED : 0) |
(id == boot_cpu_physical_apicid ?
CPU_BOOTPROCESSOR : 0),
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index ee682dd136..886031d86a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -885,13 +885,14 @@ static struct domain *__init create_dom0(const module_t
*image,
{
static char __initdata cmdline[MAX_GUEST_CMDLINE];
+unsigned int max_vcpus = dom0_max_vcpus();
struct xen_domctl_createdomain dom0_cfg = {
.flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : 0,
.max_evtchn_port = -1,
.max_grant_frames = -1,
.max_maptrack_frames = -1,
.grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
-.max_vcpus = dom0_max_vcpus(),
+.max_vcpus = max_vcpus,
.arch = {
.misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
},
--
2.40.0
Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
On 01/02/2024 16:17, John Ernberg wrote: On 2/1/24 13:20, Julien Grall wrote: On 31/01/2024 15:32, John Ernberg wrote: Hi Julien, Hi John, On 1/31/24 13:22, Julien Grall wrote: Hi, On 31/01/2024 11:50, John Ernberg wrote: When using Linux for dom0 there are a bunch of drivers that need to do SMC SIP calls into the PSCI provider to enable certain hardware bits like the watchdog. Do you know which protocol this is under the hood. Is this SCMI? I think I confused myself here when I wrote the commit log. The EL3 code in our case is ATF, and it does not appear to be SCMI, nor PSCI. The register usage of these SMC SIP calls are as follows: a0 - service a1 - function a2-a7 - args In ATF the handler is declared as a runtime service. Would the appropriate commmit message here be something along the lines of below? """ When using Linux for dom0 there are a bunch of drivers that need to do SMC SIP calls into the firmware to enable certain hardware bits like the watchdog. """ It reads better thanks. [...] But even if we restrict to dom0, have you checked that none of the SMCs use buffers? I haven't found any such instances in the Linux kernel where a buffer is used. Adding a call filtering like suggested below additions of such functions can be discovered and adapted for if they would show up later. Rather than providing a blanket forward, to me it sounds more like you want to provide an allowlist of the SMCs. This is more futureproof and avoid the risk to expose unsafe SMCs to any domain. For an example, you can have a look at the EEMI mediator for Xilinx. Ack. Do you prefer to see only on SMCCC service level or also on function level? (a1 register, per description earlier) I am not sure. It will depend on whether it is correct to expose *all* the functions within a service level and they have the same format. If you can't guarantee that, then you will most likely need to allowlist at the function level. Also, do you have a spec in hand that would help to understand which service/function is implemented via those SMCs? I don't have the spec unfortunately, but I will add a filter on both service and function for V2 and we'll take it from there. @Peng, do you have any specification you could share? How stable is the interface? Cheers, -- Julien Grall
[xen-4.18-testing test] 184553: tolerable FAIL - PUSHED
flight 184553 xen-4.18-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/184553/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184532 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184532 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184532 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184532 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184532 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184532 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184532 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 184532 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 184532 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184532 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 184532 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 184532 test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail 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-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-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass version targeted for testing: xen b1fdd7d0e47e0831ac7a99d0417385fc10d3068c baseline version: xen c7ac596a575a05d6ff1e35c3ff98bc4d143712d2 Last test of basis 184532 2024-01-30 14:07:14 Z2 days Testing same since 184553 2024-02-01 17:08:47 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Michal Orzel
Re: Ping: [PATCH] x86/guest: finish conversion to altcall
On 02/02/2024 07:08, Jan Beulich wrote:
On 17.01.2024 10:31, Jan Beulich wrote:
While .setup() and .e820_fixup() don't need fiddling with for being run
only very early, both .ap_setup() and .resume() want converting too:
This way both pre-filled struct hypervisor_ops instances can become
__initconst_cf_clobber, thus allowing to eliminate up to 5 more ENDBR
(configuration dependent) during the 2nd phase of alternatives patching.
While fiddling with section annotations here, also move "ops" itself to
.data.ro_after_init.
Signed-off-by: Jan Beulich
May I ask for an ack (or otherwise)?
Thanks, Jan
Yes, no objections.
Acked-by: Paul Durrant
--- a/xen/arch/x86/guest/hyperv/hyperv.c
+++ b/xen/arch/x86/guest/hyperv/hyperv.c
@@ -207,7 +207,7 @@ static int cf_check flush_tlb(
return hyperv_flush_tlb(mask, va, flags);
}
-static const struct hypervisor_ops __initconstrel ops = {
+static const struct hypervisor_ops __initconst_cf_clobber ops = {
.name = "Hyper-V",
.setup = setup,
.ap_setup = ap_setup,
--- a/xen/arch/x86/guest/hypervisor.c
+++ b/xen/arch/x86/guest/hypervisor.c
@@ -13,7 +13,7 @@
#include
#include
-static struct hypervisor_ops __read_mostly ops;
+static struct hypervisor_ops __ro_after_init ops;
const char *__init hypervisor_probe(void)
{
@@ -49,7 +49,7 @@ void __init hypervisor_setup(void)
int hypervisor_ap_setup(void)
{
if ( ops.ap_setup )
-return ops.ap_setup();
+return alternative_call(ops.ap_setup);
return 0;
}
@@ -57,7 +57,7 @@ int hypervisor_ap_setup(void)
void hypervisor_resume(void)
{
if ( ops.resume )
-ops.resume();
+alternative_vcall(ops.resume);
}
void __init hypervisor_e820_fixup(void)
--- a/xen/arch/x86/guest/xen/xen.c
+++ b/xen/arch/x86/guest/xen/xen.c
@@ -318,7 +318,7 @@ static int cf_check flush_tlb(
return xen_hypercall_hvm_op(HVMOP_flush_tlbs, NULL);
}
-static const struct hypervisor_ops __initconstrel ops = {
+static const struct hypervisor_ops __initconst_cf_clobber ops = {
.name = "Xen",
.setup = setup,
.ap_setup = ap_setup,
