Re: [PATCH v16 1/4] xen/domain: unify domain ID allocation
Hi Denis, On 12/08/2025 23:30, dm...@proton.me wrote: From: Denis Mukhin Currently, there are two different domain ID allocation implementations: 1) Sequential IDs allocation in dom0less Arm code based on max_init_domid; 2) Sequential IDs allocation in XEN_DOMCTL_createdomain; does not use max_init_domid (both Arm and x86). The domain ID allocation covers dom0 or late hwdom, predefined domains, post-boot domains, excluding Xen system domains (domid >= DOMID_FIRST_RESERVED). It makes sense to have a common helper code for such task across architectures (Arm and x86) and between dom0less / toolstack domU allocation. Note, fixing dependency on max_init_domid is out of scope of this patch. Wrap the domain ID allocation as an arch-independent function domid_alloc() in new common/domid.c based on the bitmap. Allocation algorithm: - If an explicit domain ID is provided, verify its availability and use it if ID is not used; - If DOMID_INVALID is provided, search the range [1..DOMID_FIRST_RESERVED-1], starting from the last used ID. Implementation guarantees that two consecutive calls will never return the same ID. ID#0 is reserved for the first boot domain (currently, dom0) and excluded from the allocation range. Remove is_free_domid() helper as it is not needed now. No functional change intended. Signed-off-by: Denis Mukhin Reviewed-by: Julien Grall > Reviewed-by: Alejandro Vallejo > ---> Changes since v15: - fixup for check after the first pass in the bitarray in domid_alloc() This was a good catch from Jan. Has a unit-test been added for this issue? Anyway, my reviewed-by holds. Cheers, -- Julien Grall
[PATCH v2 0/3] dom0less pci passthrough support on Arm part 1
This series adds basic PCI device enumeration in Xen. This will allow us to not rely on Dom0 enumeration for supported controllers, which will enable PCI passthrough for dom0less setups. Enumeration is disabled by default and can be enabled with "pci-scan" cmdline option. Discovered devices are added to Xen and assigned to dom_io by default. Edward Pickup (1): arm/pci: Add pci-scan boot argument Luca Fancellu (2): xen/pci: modify pci_add_device to handle device add by Xen xen/pci: add discovered PCI device at boot docs/misc/xen-command-line.pandoc | 7 ++ xen/arch/arm/include/asm/pci.h | 3 + xen/arch/arm/pci/pci-host-common.c | 1 + xen/arch/arm/pci/pci.c | 25 +++- xen/arch/x86/physdev.c | 9 ++- xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +- xen/drivers/passthrough/pci.c | 71 ++--- xen/drivers/passthrough/vtd/iommu.c | 2 +- xen/drivers/pci/physdev.c | 3 +- xen/include/xen/pci.h | 10 +-- 10 files changed, 94 insertions(+), 39 deletions(-) -- 2.34.1
[PATCH] misra: add deviation for MISRA C Rule 18.3
MISRA C Rule 18.3 states:"The relational operators >, >=, < and <= shall
not be applied to objects of pointer type except where they point into
the same object."
Comparisons in the 'find_text_region()' function are safe because linker
symbols '_stext' and '_etext' represent fixed virtual addresses within
the same '.text' region, and the function 'addr' argument is explicitly
compared to known valid memory bounds ('text_start' and 'text_end')
derived from these linker symbols:
if ( (void *)addr >= iter->text_start &&
(void *)addr < iter->text_end )
Configure Eclair to suppress violation reports occured in the function
'find_text_region()'. Update 'deviations.rst' file accordingly.
No functional changes.
Signed-off-by: Dmytro Prokopchuk
---
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1993673043
---
automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++
docs/misra/deviations.rst| 8
2 files changed, 14 insertions(+)
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 7f3fd35a33..32b596c9da 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -590,6 +590,12 @@ C99 Undefined Behaviour 45: Pointers that do not point
into, or just beyond, the
-config=MC3A2.R18.2,reports+={safe,
"any_area(any_loc(any_exp(macro(^page_to_mfn$"}
-doc_end
+-doc_begin="Comparisons in the 'find_text_region()' function are safe because
linker symbols '_stext' and '_etext' represent fixed
+virtual addresses within the same '.text' region, and the function 'addr'
argument is explicitly compared to known valid memory
+bounds ('text_start' and 'text_end') derived from these linker symbols."
+-config=MC3A2.R18.3,reports+={safe,
"any_area(any_loc(file(^xen/common/virtual_region\\.c$))&&context(name(find_text_region)))"}
+-doc_end
+
-doc_begin="Flexible array members are deliberately used and XEN developers
are aware of the dangers related to them:
unexpected result when the structure is given as argument to a sizeof()
operator and the truncation in assignment between structures."
-config=MC3A2.R18.7,reports+={deliberate, "any()"}
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 2119066531..a726fb22a8 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -586,6 +586,14 @@ Deviations related to MISRA C:2012 Rules:
are safe.
- Tagged as `safe` for ECLAIR.
+ * - R18.3
+ - Comparisons in the 'find_text_region()' function are safe because
+ linker symbols '_stext' and '_etext' represent fixed virtual
+ addresses within the same '.text' region, and the function 'addr'
+ argument is explicitly compared to known valid memory bounds
+ ('text_start' and 'text_end') derived from these linker symbols.
+ - Tagged as `safe` for ECLAIR.
+
* - R20.4
- The override of the keyword \"inline\" in xen/compiler.h is present so
that section contents checks pass when the compiler chooses not to
--
2.43.0
Re: [PATCH v4 1/3] tools: add sizeof_field and offsetof_end macros
On 30/07/2025 10:56 am, Petr Beneš wrote: > From: Petr Beneš > > * `sizeof_field` returns the size of a specific struct member > * `offsetof_end` returns the offset to the end of the member within the struct > > It will be useful in upcoming layout checks of SMBIOS structs. > > Signed-off-by: Petr Beneš > --- > tools/include/xen-tools/common-macros.h | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/tools/include/xen-tools/common-macros.h > b/tools/include/xen-tools/common-macros.h > index 0088208c2e..3899e06753 100644 > --- a/tools/include/xen-tools/common-macros.h > +++ b/tools/include/xen-tools/common-macros.h > @@ -83,6 +83,11 @@ > #define __packed __attribute__((__packed__)) > #endif > > +#define sizeof_field(type, member) sizeof(((type *)NULL)->member) > + > +#define offsetof_end(type, member) \ > +(offsetof(type, member) + sizeof_field(type, member)) In Xen, we have this called endof_field() (although I realise doing this post-dated your patch). Please could you adjust, and send out a full series. I see there were some minor changes needed in patch 2. Also, please add a final patch which adds to CHANGELOG.md in the root directly. You want a bullet point under Added/x86 about this. ~Andrew
Re: [PATCH] misra: add deviation for MISRA C Rule 18.3
On 2025-08-20 14:44, Dmytro Prokopchuk1 wrote:
MISRA C Rule 18.3 states:"The relational operators >, >=, < and <=
shall
not be applied to objects of pointer type except where they point into
the same object."
Comparisons in the 'find_text_region()' function are safe because
linker
symbols '_stext' and '_etext' represent fixed virtual addresses within
the same '.text' region, and the function 'addr' argument is explicitly
compared to known valid memory bounds ('text_start' and 'text_end')
derived from these linker symbols:
if ( (void *)addr >= iter->text_start &&
(void *)addr < iter->text_end )
Configure Eclair to suppress violation reports occured in the function
'find_text_region()'. Update 'deviations.rst' file accordingly.
No functional changes.
Signed-off-by: Dmytro Prokopchuk
Reviewed-by: Nicola Vetrini
with two NITs below:
---
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1993673043
---
automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++
docs/misra/deviations.rst| 8
2 files changed, 14 insertions(+)
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 7f3fd35a33..32b596c9da 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -590,6 +590,12 @@ C99 Undefined Behaviour 45: Pointers that do not
point into, or just beyond, the
-config=MC3A2.R18.2,reports+={safe,
"any_area(any_loc(any_exp(macro(^page_to_mfn$"}
-doc_end
+-doc_begin="Comparisons in the 'find_text_region()' function are safe
because linker symbols '_stext' and '_etext' represent fixed
+virtual addresses within the same '.text' region, and the function
'addr' argument is explicitly compared to known valid memory
+bounds ('text_start' and 'text_end') derived from these linker
symbols."
Maybe ['text_start', 'text_end') here and below to emphasize that this
is a range?
+-config=MC3A2.R18.3,reports+={safe,
"any_area(any_loc(file(^xen/common/virtual_region\\.c$))&&context(name(find_text_region)))"}
+-doc_end
+
-doc_begin="Flexible array members are deliberately used and XEN
developers are aware of the dangers related to them:
unexpected result when the structure is given as argument to a
sizeof() operator and the truncation in assignment between structures."
-config=MC3A2.R18.7,reports+={deliberate, "any()"}
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 2119066531..a726fb22a8 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -586,6 +586,14 @@ Deviations related to MISRA C:2012 Rules:
are safe.
- Tagged as `safe` for ECLAIR.
+ * - R18.3
+ - Comparisons in the 'find_text_region()' function are safe
because
+ linker symbols '_stext' and '_etext' represent fixed virtual
+ addresses within the same '.text' region, and the function
'addr'
s/function 'addr' argument/function parameter 'addr'/
+ argument is explicitly compared to known valid memory bounds
+ ('text_start' and 'text_end') derived from these linker
symbols.
+ - Tagged as `safe` for ECLAIR.
+
* - R20.4
- The override of the keyword \"inline\" in xen/compiler.h is
present so
that section contents checks pass when the compiler chooses not
to
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253
Re: [PATCH] x86/iommu: setup MMCFG ahead of IOMMU
On Wed, Aug 20, 2025 at 12:37:51PM +0100, Andrew Cooper wrote:
> On 20/08/2025 12:33 pm, Roger Pau Monné wrote:
> > On Tue, Aug 19, 2025 at 07:23:57PM +0100, Andrew Cooper wrote:
> >> On 19/08/2025 6:18 pm, Roger Pau Monne wrote:
> >>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> >>> index 6fb42c5a5f95..bd648323bfed 100644
> >>> --- a/xen/arch/x86/setup.c
> >>> +++ b/xen/arch/x86/setup.c
> >>> @@ -1938,11 +1938,10 @@ void asmlinkage __init noreturn __start_xen(void)
> >>> setup_system_domains();
> >>>
> >>> /*
> >>> - * Ahead of any ACPI table parsing make sure we have control
> >>> structures
> >>> - * for PCI segment 0.
> >>> + * Initialize PCI (create segment 0, setup MMCFG access) ahead of
> >>> IOMMU
> >>> + * setup, as it requires access to the PCI config space.
> >>> */
> >> Again, this isn't terribly clear IMO.
> >>
> >> "ahead of IOMMU setup, as the IOMMUs might not all live on segment 0." ?
> > It's not just IOMMUs, but for example on VT-d we also need to poke at
> > the config space of bridges, and when such bridges live in segment > 0
> > that results in garbage being returned.
> >
> > I'm not sure acpi_iommu_init() accesses the IOMMU PCI device config
> > space, but it does at least access the config space of bridges in
> > order to detect hierarchy. See how acpi_parse_dev_scope() performs
> > PCI reads.
> >
> > What about using:
> >
> > /*
> > * Initialize PCI (create segment 0, setup MMCFG access) ahead of IOMMU
> > * setup, as devices in segment > 0 must also be discoverable.
> > */
>
> Yeah, that works.
>
> With those comment adjustments, Reviewed-by: Andrew Cooper
>
Forgot to add it, this patch should have:
Fixes: 3950f2485bbc ('x86/x2APIC: defer probe until after IOMMU ACPI table
parsing')
In the commit message.
Thanks, Roger.
[PATCH] tools/xenstored: fix possible NULL deref
Avoid dereferencing NULL in introduce_domain() when called during
live update.
Coverity ID: 1665111
Fixes: f78895ad78c9 ("tools/xenstored: Read event channel from xenstored page")
Signed-off-by: Juergen Gross
---
tools/xenstored/domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index 694d68670a..5ce539d640 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -1032,7 +1032,7 @@ static struct domain *introduce_domain(const void *ctx,
if (!interface && !restore)
return NULL;
- if (interface->evtchn_port)
+ if (!restore && interface->evtchn_port)
port = interface->evtchn_port;
if (new_domain(domain, port, restore)) {
--
2.43.0
Re: [PATCH] x86/iommu: setup MMCFG ahead of IOMMU
On Tue, Aug 19, 2025 at 07:23:57PM +0100, Andrew Cooper wrote:
> On 19/08/2025 6:18 pm, Roger Pau Monne wrote:
> > Otherwise the PCI accesses to segments different than the first one done by
> > the IOMMU initialization code would silently fail by returning all ones.
> >
> > Introduce a new helper, called pci_setup(), and move both the creation of
> > PCI segment 0 internal data structures, plus the parsing of ACPI MMCFG
> > table to it.
> >
> > Signed-off-by: Roger Pau Monné
>
> And moving acpi_mmcfg_init() slightly earlier from acpi_boot_init() into
> pci_setup().
>
> > diff --git a/xen/arch/x86/pci.c b/xen/arch/x86/pci.c
> > index 26bb7f6a3c3a..e75a29e851a7 100644
> > --- a/xen/arch/x86/pci.c
> > +++ b/xen/arch/x86/pci.c
> > @@ -139,6 +142,19 @@ int pci_sanitize_bar_memory(struct rangeset *r)
> > return 0;
> > }
> >
> > +void __init pci_setup(void)
> > +{
> > +/*
> > + * Ahead of any ACPI table parsing make sure we have control structures
> > + * for PCI segment 0.
> > + */
> > +if ( pci_add_segment(0) )
> > +panic("Could not initialize PCI segment 0\n");
> > +
> > +/* Parse ACPI MMCFG ahead of IOMMU, so accesses to segments > 0 is
> > setup. */
>
> "ahead of IOMMU" isn't helpful here because the relevant context is in
> the caller. Instead, I'd just say:
>
> /* Parse ACPI MMCFG to see if other segments are available. */
Sure.
> > +acpi_mmcfg_init();
> > +}
> > +
> > /*
> > * Local variables:
> > * mode: C
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 6fb42c5a5f95..bd648323bfed 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -1938,11 +1938,10 @@ void asmlinkage __init noreturn __start_xen(void)
> > setup_system_domains();
> >
> > /*
> > - * Ahead of any ACPI table parsing make sure we have control structures
> > - * for PCI segment 0.
> > + * Initialize PCI (create segment 0, setup MMCFG access) ahead of IOMMU
> > + * setup, as it requires access to the PCI config space.
> > */
>
> Again, this isn't terribly clear IMO.
>
> "ahead of IOMMU setup, as the IOMMUs might not all live on segment 0." ?
It's not just IOMMUs, but for example on VT-d we also need to poke at
the config space of bridges, and when such bridges live in segment > 0
that results in garbage being returned.
I'm not sure acpi_iommu_init() accesses the IOMMU PCI device config
space, but it does at least access the config space of bridges in
order to detect hierarchy. See how acpi_parse_dev_scope() performs
PCI reads.
What about using:
/*
* Initialize PCI (create segment 0, setup MMCFG access) ahead of IOMMU
* setup, as devices in segment > 0 must also be discoverable.
*/
Thanks, Roger.
Re: [PATCH] releases: use newer compression methods for tarballs
On 15/07/2025 7:33 am, Jan Beulich wrote: > Other projects have long switched to xz and/or lzip. > > In the mktarball script, don't rely on the tar used supporting the -J > (xz) or --lzip (lzip) options. > > Signed-off-by: Jan Beulich > --- > Question (to the release technicians) is whether this would be adding > undue overhead, primarily as a result of anything done outside of the > build system. I'm happy with this. > > It's unclear to me why we have git create a tarball, extract that, just > to then make a tarball again (without any special options, like to > override owner or timestamps; That's because in c5be91eb8140 you deleted the intermediate step of merging qemus > in this context I notice that tarballs > created by Andrew have file ownership of andrew/andrew, while ones made > by Julien use root/root). Ownership where exactly? the tarball itself (which will be down to accounts on downloads.xenproject.org) or the tarball contents itself? > > Without passing -9, I observe lzip to compress worse than xz; the win > of passing -9 to xz isn't overly big anyway (about 100k, compared to > about 250k with lzip). As these are created once and downloaded many times, we should always do max compression. Even if it takes minutes extra to create, that's still a win overall. > > lzip, unlike the other two tools, doesn't really show a progress > indicator with -v. Merely having final statistics may make the use of > the option here questionable. I can't say I find any of the stats relevant. > --- a/docs/process/xen-release-management.pandoc > +++ b/docs/process/xen-release-management.pandoc > @@ -274,10 +274,10 @@ Xen X.Y rcZ is tagged. You can check tha > https://xenbits.xen.org/git-http/xen.git X.Y.0-rcZ > > For your convenience there is also a tarball at: > -https://downloads.xenproject.org/release/xen/X.Y.0-rcZ/xen-X.Y.0-rcZ.tar.gz > +https://downloads.xenproject.org/release/xen/X.Y.0-rcZ/xen-X.Y.0-rcZ.tar.[glx]z > > And the signature is at: > -https://downloads.xenproject.org/release/xen/X.Y.0-rcZ/xen-X.Y.0-rcZ.tar.gz.sig > +https://downloads.xenproject.org/release/xen/X.Y.0-rcZ/xen-X.Y.0-rcZ.tar.[glx]z.sig > > Please send bug reports and test reports to xen-devel@lists.xenproject.org. > When sending bug reports, please CC relevant maintainers and me > --- a/tools/misc/mktarball > +++ b/tools/misc/mktarball > @@ -31,4 +31,14 @@ git_archive_into $xen_root $tdir/xen-$de > > GZIP=-9v tar cz -f $xen_root/dist/xen-$desc.tar.gz -C $tdir xen-$desc > > -echo "Source tarball in $xen_root/dist/xen-$desc.tar.gz" > +if [ -n "$(command -v xz)" ] > +then > + tar c -C $tdir xen-$desc | $(command -v xz) -v -9 > >$xen_root/dist/xen-$desc.tar.xz > +fi > + > +if [ -n "$(command -v lzip)" ] > +then > + tar c -C $tdir xen-$desc | $(command -v lzip) -v -9 > >$xen_root/dist/xen-$desc.tar.lz > +fi > + > +echo "Source tarball in $xen_root/dist/xen-$desc".tar.[glx]z If we're deciding to use multiple compressions, they want to not be optional here. I'd far rather have a reminder to install the package, than for it to simply be omitted. We don't want to be re-tar-ing now that the qemu's are gone, so I think the structure wants to end up as: git archive --format=tar HEAD > tmp.tar gzip -9 < tmp.tar > $xen_root/dist/xen-$desc.tar.gz & zx -9 < tmp.tar > $xen_root/dist/xen-$desc.tar.xz & lzip -9 < tmp.tar > $xen_root/dist/xen-$desc.tar.lz & wait Might as well use the multiple cores better... ~Andrew
[PATCH v2 2/3] xen/pci: modify pci_add_device to handle device add by Xen
From: Luca Fancellu
Currently pci_add_device is called from hypercalls requested by Dom0
to add pci devices and when the device has no domain associated with
it, it is assumed that hardware_domain is the owner.
On the dom0less scenario, the enumeration is performed by the
firmware and Xen at boot time might want to assign some pci devices
to guests, so it has to firstly add the device and then assign it to
the final guest.
Modify pci_add_device to have the owner domain passed as a parameter
to the function, so that when it is called from the hypercall the
owner would be the caller domain, otherwise when Xen is calling it,
it would be another domain since hw domain could not be there
(dom0less guests without Dom0 use case).
Signed-off-by: Luca Fancellu
Signed-off-by: Mykyta Poturai
---
(cherry picked from commit f0c85d9043f7c9402e85b73361c8a13c683428ca from
the downstream branch poc/pci-passthrough from
https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git)
v1->v2:
* remove dom_io check
* fixup pci_add_device parameters
* use current->domain instead of hardware_domain
changes since cherry-pick:
* s/hardware_domain/d/ in some write_unlock calls in
xen/drivers/passthrough/pci.c
---
xen/arch/x86/physdev.c| 9 -
xen/drivers/passthrough/pci.c | 29 +++--
xen/drivers/pci/physdev.c | 3 ++-
xen/include/xen/pci.h | 5 +++--
4 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 4dfa1c0191..04d179e81b 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -446,8 +446,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
arg)
if ( copy_from_guest(&manage_pci, arg, 1) != 0 )
break;
-ret = pci_add_device(0, manage_pci.bus, manage_pci.devfn,
- NULL, NUMA_NO_NODE);
+ret = pci_add_device(0, manage_pci.bus, manage_pci.devfn, NULL,
+ NUMA_NO_NODE, current->domain);
break;
}
@@ -477,9 +477,8 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
arg)
pdev_info.is_virtfn = manage_pci_ext.is_virtfn;
pdev_info.physfn.bus = manage_pci_ext.physfn.bus;
pdev_info.physfn.devfn = manage_pci_ext.physfn.devfn;
-ret = pci_add_device(0, manage_pci_ext.bus,
- manage_pci_ext.devfn,
- &pdev_info, NUMA_NO_NODE);
+ret = pci_add_device(0, manage_pci_ext.bus, manage_pci_ext.devfn,
+ &pdev_info, NUMA_NO_NODE, current->domain);
break;
}
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 3edcfa8a04..09b424d1b3 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -654,8 +654,9 @@ unsigned int pci_size_mem_bar(pci_sbdf_t sbdf, unsigned int
pos,
return is64bits ? 2 : 1;
}
-int pci_add_device(u16 seg, u8 bus, u8 devfn,
- const struct pci_dev_info *info, nodeid_t node)
+int pci_add_device(uint16_t seg, uint8_t bus, uint8_t devfn,
+ const struct pci_dev_info *info, nodeid_t node,
+ struct domain *d)
{
struct pci_seg *pseg;
struct pci_dev *pdev;
@@ -663,6 +664,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
const char *type;
int ret;
+if ( !d )
+return -EINVAL;
+
if ( !info )
type = "device";
else if ( info->is_virtfn )
@@ -767,9 +771,9 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
ret = 0;
if ( !pdev->domain )
{
-pdev->domain = hardware_domain;
-write_lock(&hardware_domain->pci_lock);
-list_add(&pdev->domain_list, &hardware_domain->pdev_list);
+pdev->domain = d;
+write_lock(&d->pci_lock);
+list_add(&pdev->domain_list, &pdev->domain->pdev_list);
/*
* For devices not discovered by Xen during boot, add vPCI handlers
@@ -779,25 +783,30 @@ int pci_add_device(u16 seg, u8 bus, u8 devfn,
if ( ret )
{
list_del(&pdev->domain_list);
-write_unlock(&hardware_domain->pci_lock);
+write_unlock(&d->pci_lock);
pdev->domain = NULL;
printk(XENLOG_ERR "Setup of vPCI failed: %d\n", ret);
goto out;
}
-write_unlock(&hardware_domain->pci_lock);
+write_unlock(&d->pci_lock);
ret = iommu_add_device(pdev);
if ( ret )
{
-write_lock(&hardware_domain->pci_lock);
+write_lock(&d->pci_lock);
vpci_deassign_device(pdev);
list_del(&pdev->domain_list);
-write_unlock(&hardware_domain->pci_lock);
+write_unlock(&d->pci_lock);
pdev->domain = NULL;
goto out;
}
}
-else
+else if ( pdev->domain == d )
iommu_enable_device(pdev);
+else
+{
+
[PATCH v2 3/3] xen/pci: add discovered PCI device at boot
From: Luca Fancellu
In dom0less mode, there is no dom0 that can call PCI physdev ops to
register PCI devices to iommu, so it needs to be done by Xen.
pci_add_device requires some default domain, we don't have hwdom, and
the guests are not yet created during the PCI init phase, so use dom_io
as a temporary sentinel before devices are assigned to their target
domains.
Rename setup_hwdom_pci_devices to setup_pci_devices and add dom0less
handling to it.
In pci_add_device there is a call to xsm that doesn't consider the
requester of the function to be Xen itself, so add a check to skip
the call if the owner domain is dom_io, since it means the call is
coming directly from Xen.
Signed-off-by: Luca Fancellu
Signed-off-by: Mykyta Poturai
---
(cherry picked from commit eff51e50021b75f5a50533f7de681b2ce044f5bd from
the downstream branch poc/pci-passthrough from
https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git)
v1->v2:
* integrate add_discovered_pci_devices into existing routines
* better explain the need for dom_io
---
xen/arch/arm/pci/pci.c | 1 +
xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
xen/drivers/passthrough/pci.c | 42 ++---
xen/drivers/passthrough/vtd/iommu.c | 2 +-
xen/include/xen/pci.h | 5 +--
5 files changed, 33 insertions(+), 19 deletions(-)
diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
index 1b34e17517..909fbdd694 100644
--- a/xen/arch/arm/pci/pci.c
+++ b/xen/arch/arm/pci/pci.c
@@ -124,6 +124,7 @@ static int __init pci_init(void)
if ( ret < 0 )
return ret;
+setup_pci_devices(dom_io, NULL);
}
return 0;
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 3a14770855..f3a83a0ab7 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -401,7 +401,7 @@ static void __hwdom_init cf_check
amd_iommu_hwdom_init(struct domain *d)
/* Make sure workarounds are applied (if needed) before adding devices. */
arch_iommu_hwdom_init(d);
-setup_hwdom_pci_devices(d, amd_iommu_add_device);
+setup_pci_devices(d, amd_iommu_add_device);
}
static void amd_iommu_disable_domain_device(const struct domain *domain,
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 09b424d1b3..6ddc6811df 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -35,6 +35,7 @@
#include
#include
#include "ats.h"
+#include "xen/dom0less-build.h"
struct pci_seg {
struct list_head alldevs_list;
@@ -676,9 +677,12 @@ int pci_add_device(uint16_t seg, uint8_t bus, uint8_t
devfn,
else
type = "device";
-ret = xsm_resource_plug_pci(XSM_PRIV, (seg << 16) | (bus << 8) | devfn);
-if ( ret )
-return ret;
+if ( d != dom_io )
+{
+ret = xsm_resource_plug_pci(XSM_PRIV, (seg << 16) | (bus << 8) |
devfn);
+if ( ret )
+return ret;
+}
ret = -ENOMEM;
@@ -1181,19 +1185,21 @@ int __init scan_pci_devices(void)
return ret;
}
-struct setup_hwdom {
+struct setup_ctxt {
struct domain *d;
int (*handler)(uint8_t devfn, struct pci_dev *pdev);
};
-static void __hwdom_init setup_one_hwdom_device(const struct setup_hwdom *ctxt,
+static void __hwdom_init setup_one_pci_device(const struct setup_ctxt *ctxt,
struct pci_dev *pdev)
{
u8 devfn = pdev->devfn;
-int err;
+int err = 0;
do {
-err = ctxt->handler(devfn, pdev);
+if ( ctxt->handler )
+err = ctxt->handler(devfn, pdev);
+
if ( err )
{
printk(XENLOG_ERR "setup %pp for d%d failed (%d)\n",
@@ -1213,10 +1219,10 @@ static void __hwdom_init setup_one_hwdom_device(const
struct setup_hwdom *ctxt,
ctxt->d->domain_id, err);
}
-static int __hwdom_init cf_check _setup_hwdom_pci_devices(
+static int __hwdom_init cf_check _setup_pci_devices(
struct pci_seg *pseg, void *arg)
{
-struct setup_hwdom *ctxt = arg;
+struct setup_ctxt *ctxt = arg;
int bus, devfn;
for ( bus = 0; bus < 256; bus++ )
@@ -1229,18 +1235,26 @@ static int __hwdom_init cf_check
_setup_hwdom_pci_devices(
if ( !pdev )
continue;
+if ( is_dom0less_mode() ) {
+int ret = pci_add_device(pdev->seg, pdev->bus, pdev->devfn,
NULL,
+ NUMA_NO_NODE, ctxt->d);
+if (ret)
+printk(XENLOG_ERR "Failed to add PCI device %pp: %d\n",
&pdev->sbdf, ret);
+continue;
+}
+
if ( !pdev->domain )
{
pdev->domain = ctxt->d;
write_lock(&ctxt->d->pci_lock);
list_add(&pdev->domain_list, &ctxt->d->pdev_list);
writ
[PATCH v2 1/3] arm/pci: Add pci-scan boot argument
From: Edward Pickup
This patch adds a Xen boot arguments that, if enabled, causes a call to
existing code to scan pci devices enumerated by the firmware.
This will be needed ahead of dom0less support for pci passthrough on
arm.
Signed-off-by: Luca Fancellu
Signed-off-by: Mykyta Poturai
---
(cherry picked from commit bce463e1588a45e1bfdf59fc0d5f88b16604e439 from
the downstream branch poc/pci-passthrough from
https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git)
v1->v2:
* remove is_pci_scan_enabled wrapper
* make pci_scan_enabled ro_after_init
* drop debug prints
* drop Edward's SOB
changes since cherry-pick:
* s/always_inline/inline/
* replace additional kconfig option with config DEBUG
Signed-off-by: Stewart Hildebrand
---
docs/misc/xen-command-line.pandoc | 7 +++
xen/arch/arm/include/asm/pci.h | 3 +++
xen/arch/arm/pci/pci-host-common.c | 1 +
xen/arch/arm/pci/pci.c | 24 ++--
4 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index a75b6c9301..762a1a4f5f 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2059,6 +2059,13 @@ This option can be specified more than once (up to 8
times at present).
Flag to enable or disable support for PCI passthrough
+### pci-scan (arm)
+> `= `
+
+> Default: `false`
+
+Flag to enable or disable Xen PCI scan at boot.
+
### pcid (x86)
> `= | xpti=`
diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
index 08ffcd4438..7289f7688b 100644
--- a/xen/arch/arm/include/asm/pci.h
+++ b/xen/arch/arm/include/asm/pci.h
@@ -23,6 +23,7 @@
#define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
extern bool pci_passthrough_enabled;
+extern bool pci_scan_enabled;
struct rangeset;
@@ -155,6 +156,8 @@ bool arch_pci_device_physdevop(void);
#else /*!CONFIG_HAS_PCI*/
+#define pci_scan_enabled false
+
struct pci_dev;
static inline void arch_pci_init_pdev(struct pci_dev *pdev) {}
diff --git a/xen/arch/arm/pci/pci-host-common.c
b/xen/arch/arm/pci/pci-host-common.c
index 487c545f3a..d3481b05eb 100644
--- a/xen/arch/arm/pci/pci-host-common.c
+++ b/xen/arch/arm/pci/pci-host-common.c
@@ -284,6 +284,7 @@ pci_host_common_probe(struct dt_device_node *dev,
}
pci_add_host_bridge(bridge);
+pci_add_segment(bridge->segment);
return bridge;
diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
index beb1f971fa..1b34e17517 100644
--- a/xen/arch/arm/pci/pci.c
+++ b/xen/arch/arm/pci/pci.c
@@ -91,8 +91,14 @@ bool arch_pci_device_physdevop(void)
bool __read_mostly pci_passthrough_enabled;
boolean_param("pci-passthrough", pci_passthrough_enabled);
+/* By default pci scan is disabled. */
+bool __ro_after_init pci_scan_enabled;
+boolean_param("pci-scan", pci_scan_enabled);
+
static int __init pci_init(void)
{
+int ret;
+
/*
* Enable PCI passthrough when has been enabled explicitly
* (pci-passthrough=on).
@@ -104,9 +110,23 @@ static int __init pci_init(void)
panic("Could not initialize PCI segment 0\n");
if ( acpi_disabled )
-return dt_pci_init();
+ret = dt_pci_init();
else
-return acpi_pci_init();
+ret = acpi_pci_init();
+
+if ( ret < 0 )
+return ret;
+
+if ( pci_scan_enabled )
+{
+ret = scan_pci_devices();
+
+if ( ret < 0 )
+return ret;
+
+}
+
+return 0;
}
__initcall(pci_init);
--
2.34.1
Re: [PATCH] tools/xenstored: fix possible NULL deref
On 2025-08-20 06:46, Juergen Gross wrote:
Avoid dereferencing NULL in introduce_domain() when called during
live update.
Coverity ID: 1665111
Fixes: f78895ad78c9 ("tools/xenstored: Read event channel from xenstored page")
Signed-off-by: Juergen Gross
Reviewed-by: Jason Andryuk
Thanks,
Jason
Re: [PATCH] xen/dom0less: Clear xenstore page
On 20.08.25 23:12, Jason Andryuk wrote:
C Xenstored was seen failing to map a domU's event channel. Enhancing
Xen's error messages shows:
common/event_channel.c:427:d0v0 EVTCHNOP failure: d1, port 3267543746 error -22
Port 3267543746 is 0xc2c2c2c2 - SCRUB_PATTERN.
Xenstore expects a non-zero evtchn_port to be valid. Explicitly zero
the page to avoid this.
Fixes: f78895ad78c9 ("tools/xenstored: Read event channel from xenstored page")
Signed-off-by: Jason Andryuk
---
Tweaked stubdom & dom0less C xenstored pipeline success:
Test CI pipeline:
https://gitlab.com/xen-project/people/jandryuk-amd/xen/-/pipelines/1994879466
Example failure:
https://gitlab.com/xen-project/people/jandryuk-amd/xen/-/jobs/11084356847
Alternatively, a fixes could be:
Fixes: df2c03e2ebfe ("xen/arm: Alloc XenStore page for Dom0less DomUs from
hypervisor")
If the page should have been zero from the start.
I think this one is better. Either with or without the Fixes: tag changed:
Reviewed-by: Juergen Gross
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH] misra: address MISRA C Rule 18.3 compliance
On 20.08.25 11:18, Dmytro Prokopchuk1 wrote:
On 7/30/25 01:09, Stefano Stabellini wrote:
On Fri, 25 Jul 2025, Dmytro Prokopchuk1 wrote:
On 7/23/25 16:58, Jan Beulich wrote:
On 23.07.2025 12:12, Dmytro Prokopchuk1 wrote:
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -568,6 +568,14 @@ C99 Undefined Behaviour 45: Pointers that do not point
into, or just beyond, the
-config=MC3A2.R18.2,reports+={safe,
"any_area(any_loc(any_exp(macro(^page_to_mfn$"}
-doc_end
+-doc_begin="Consider relational pointer comparisons in kernel-related sections as safe and compliant."
+-config=MC3R1.R18.3,reports+={safe,
"any_area(any_loc(any_exp(macro(name(is_kernel||is_kernel_text||is_kernel_rodata||is_kernel_inittext)"}
+-doc_end
+
+-doc_begin="Allow deviations for pointer comparisons in BUG_ON and ASSERT macros,
treating them as safe for debugging and validation."
+-config=MC3R1.R18.3,reports+={safe,
"any_area(any_loc(any_exp(macro(name(BUG_ON||ASSERT)"}
+-doc_end
Nit: Drop "deviations for" from the verbal description?
Ok.
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -461,7 +461,8 @@ static void __init efi_arch_edd(void)
params->device_path_info_length =
sizeof(struct edd_device_params) -
offsetof(struct edd_device_params, key);
-for ( p = (const u8 *)¶ms->key; p < ¶ms->checksum; ++p
)
+for ( p = (const u8 *)¶ms->key;
+ (uintptr_t)p < (uintptr_t)¶ms->checksum; ++p )
There mere addition of such casts makes code more fragile imo. What about the
alternative of using != instead of < here? I certainly prefer < in such
situations,
but functionally != ought to be equivalent (and less constrained by C and
Misra).
As mentioned several times when discussing these rules, it's also not easy to
see
how "pointers of different objects" could be involved here: It's all within
*params, isn't it?
Hard to say something. Let's hold this so far.
Finally, when touching such code it would be nice if u was converted to
uint_t.
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -360,7 +360,7 @@ static always_inline void sched_spin_lock_double(
{
*flags = _spin_lock_irqsave(lock1);
}
-else if ( lock1 < lock2 )
+else if ( (uintptr_t)lock1 < (uintptr_t)lock2 )
Could we assume that these 'lock1' and 'lock2' pointers belong to the
same allocation region - 'sched_resource' ?
No, they can be either in sched_resource, in a per-scheduler private memory
area, or even in the .data section of the hypervisor.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [PATCH] misra: address MISRA C Rule 18.3 compliance
On 7/30/25 01:09, Stefano Stabellini wrote:
> On Fri, 25 Jul 2025, Dmytro Prokopchuk1 wrote:
>> On 7/23/25 16:58, Jan Beulich wrote:
>>> On 23.07.2025 12:12, Dmytro Prokopchuk1 wrote:
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -568,6 +568,14 @@ C99 Undefined Behaviour 45: Pointers that do not
point into, or just beyond, the
-config=MC3A2.R18.2,reports+={safe,
"any_area(any_loc(any_exp(macro(^page_to_mfn$"}
-doc_end
+-doc_begin="Consider relational pointer comparisons in kernel-related
sections as safe and compliant."
+-config=MC3R1.R18.3,reports+={safe,
"any_area(any_loc(any_exp(macro(name(is_kernel||is_kernel_text||is_kernel_rodata||is_kernel_inittext)"}
+-doc_end
+
+-doc_begin="Allow deviations for pointer comparisons in BUG_ON and ASSERT
macros, treating them as safe for debugging and validation."
+-config=MC3R1.R18.3,reports+={safe,
"any_area(any_loc(any_exp(macro(name(BUG_ON||ASSERT)"}
+-doc_end
>>>
>>> Nit: Drop "deviations for" from the verbal description?
>> Ok.
>>
>>>
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -461,7 +461,8 @@ static void __init efi_arch_edd(void)
params->device_path_info_length =
sizeof(struct edd_device_params) -
offsetof(struct edd_device_params, key);
-for ( p = (const u8 *)¶ms->key; p <
¶ms->checksum; ++p )
+for ( p = (const u8 *)¶ms->key;
+ (uintptr_t)p < (uintptr_t)¶ms->checksum; ++p )
>>>
>>> There mere addition of such casts makes code more fragile imo. What about
>>> the
>>> alternative of using != instead of < here? I certainly prefer < in such
>>> situations,
>>> but functionally != ought to be equivalent (and less constrained by C and
>>> Misra).
>>>
>>> As mentioned several times when discussing these rules, it's also not easy
>>> to see
>>> how "pointers of different objects" could be involved here: It's all within
>>> *params, isn't it?
>> Hard to say something. Let's hold this so far.
>>
>>>
>>> Finally, when touching such code it would be nice if u was converted to
>>> uint_t.
>>>
--- a/xen/common/sched/core.c
+++ b/xen/common/sched/core.c
@@ -360,7 +360,7 @@ static always_inline void sched_spin_lock_double(
{
*flags = _spin_lock_irqsave(lock1);
}
-else if ( lock1 < lock2 )
+else if ( (uintptr_t)lock1 < (uintptr_t)lock2 )
Could we assume that these 'lock1' and 'lock2' pointers belong to the
same allocation region - 'sched_resource' ?
Dmytro.
>>>
>>> Similarly, no matter what C or Misra may have to say here, imo such casts
>>> are
>>> simply dangerous. Especially when open-coded.
>> Well, this function 'sched_spin_lock_double' has the following description:
>> "If locks are different, take the one with the lower address first."
>>
>> I think it's save to compare the integer representations of 'lock1' and
>> 'lock2' addresses explicitly (casting the pointers values to an integer
>> type). We need to find the 'lower address'.
>> Any risks here?
>>
>> Dmytro
>>>
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -83,8 +83,8 @@ const struct virtual_region *find_text_region(unsigned
long addr)
rcu_read_lock(&rcu_virtual_region_lock);
list_for_each_entry_rcu ( iter, &virtual_region_list, list )
{
-if ( (void *)addr >= iter->text_start &&
- (void *)addr < iter->text_end )
+if ( addr >= (unsigned long)iter->text_start &&
+ addr < (unsigned long)iter->text_end )
>>>
>>> Considering further casts to unsigned long of the same struct fields, was it
>>> considered to alter the type of the struct fields instead?
>> There are present casts of struct fields 'text_start' and 'text_end' in
>> the file 'xen/common/virtual_region.c'.
>>
>> modify_xen_mappings_lite((unsigned long)region->text_start,
>>(unsigned long)region->text_end,
>>PAGE_HYPERVISOR_RWX);
>>
>> Changing fields type to 'unsigned long' will give the opportunity to
>> remove casts from source code (mentioned before),
>> and remove '(void*)' casts from here:
>>
>> if ( (void *)addr >= iter->text_start &&
>>(void *)addr < iter->text_end )
>>
>> Unfortunately there are places where these fields are initialized, so
>> cast to the 'unsigned long' should be there.
>> Example:
>> .text_start = _sinittext,
>> .text_end = _einittext,
>> and
>> .text_start = _sinittext,
>> .text_end = _einittext,
>>
>> where
>> extern char _sinittext[], _einittext[];
>> extern char _stext[], _etext[];
Ping: [PATCH v4 0/3] hvmloader: add new SMBIOS tables (7,8,9,26,27,28)
On Wed, Jul 30, 2025 at 11:56 AM Petr Beneš wrote: > > From: Petr Beneš > > Changes since v3: > - Renamed offsetofend macro to offsetof_end. > > Changes since v2: > - Added sizeof_field and offsetofend macros to common-macros.h. > - Used offsetofend macro everywhere to make semantically sense. > - Added BUILD_BUG_ON to ensure offsets are correct based on the SMBIOS > specification (with explanation in the comments). > - Fixed commit message of the second patch to mention the new smbios_pt_copy > function. > - Fixed nits (empty lines, misplaced *). > - Retained the original comment "Only present when passed in" + added it to > the new tables where applicable. > - Replaced hardcoded offset for chassis_handle in smbios_type_2_init. > > Changes since v1: > - Swapped the order of S-o-b in the last commit message. > > Resubmitting patch from Anton Belousov and addressing review comments > from Jan: > https://old-list-archives.xen.org/archives/html/xen-devel/2022-01/msg00725.html > > Original message: > > SMBIOS tables like 7,8,9,26,27,28 are neccessary to prevent sandbox > > detection > > by malware using WMI-queries. New tables can be mapped to memory from binary > > file specified in "smbios_firmware" parameter of domain configuration. > > If particular table is absent in binary file, then it will not be mapped to > > memory. This method works for Windows domains as tables 7,8,9,26,27,28 are > > not > > critical for OS boot and runtime. Also if "smbios_firmware" parameter is not > > provided, these tables will be skipped in write_smbios_tables function. > > Further explanation: > Some malware samples are known to check presence of various hardware > components > (like CPU fan, CPU temperature sensor, etc.) by WMI queries. If these > components > are not present, then malware can assume that it is running in a sandbox and > will not execute its payload. > > This patch will allow security researchers to create a custom SMBIOS > firmware binary file that contains these tables. > > Petr Beneš (3): > tools: add sizeof_field and offsetof_end macros > hvmloader: fix SMBIOS table length checks > hvmloader: add new SMBIOS tables (7, 8, 9, 26, 27, 28) > > tools/firmware/hvmloader/smbios.c | 290 ++-- > tools/firmware/hvmloader/smbios_types.h | 109 +++-- > tools/include/xen-tools/common-macros.h | 5 + > 3 files changed, 317 insertions(+), 87 deletions(-) > > -- > 2.34.1 >
Ping: [PATCH v12 0/6] x86: Make MAX_ALTP2M configurable
On Thu, Jul 31, 2025 at 11:28 AM Petr Beneš wrote:
>
> From: Petr Beneš
>
> This series introduces the ability to configure the maximum number of altp2m
> tables during domain creation. Previously, the limits were hardcoded to a
> maximum of 10. This change allows for greater flexibility in environments that
> require more or fewer altp2m views.
>
> This enhancement is particularly relevant for users leveraging Xen's features
> for virtual machine introspection.
>
> Changes since v11:
> - altp2m: Drop p2m_altp2m_check() stubs on non-x86, move prototype, and guard
> uses
> - s/following/subsequent/ in the commit message + Reviewed-by: Jan Beulich
> - Despite agreeing to let Jan make this change, this commit wasn't still
> in
> the staging branch at the time of sending this patch.
>
> - x86/altp2m: Wrap altp2m-specific code in #ifdef CONFIG_ALTP2M
> - Change typo #else /* !CONFIG_HVM */ -> #else /* !CONFIG_ALTP2M */ in
> p2m.h
> - Move p2m_altp2m_get_or_propagate & p2m_altp2m_propagate_change outside
> of
> #ifdef CONFIG_ALTP2M / #else / #endif and added comment mentioning that
> these altp2m declarations are outside of the #ifdef scope intentionally.
>
> Changes since v10:
> - x86/altp2m: Remove p2m_altp2m_check stubs from unsupported architectures
> - (now "altp2m: drop p2m_altp2m_check() stubs on non-x86, move prototype,
>and guard uses").
> - Changed commit message to reflect new changes.
> - Removed p2m_altp2m_check from x86/include/asm/p2m.h.
> - Introduced p2m_altp2m_check in xen/include/xen/p2m-common.h.
> - Replaced #ifdef CONFIG_ALTP2M with IS_ENABLED(CONFIG_ALTP2M) in
> vm_event.c.
>
> - x86/altp2m: Wrap altp2m-specific code in #ifdef CONFIG_ALTP2M
> - Added `else if ( IS_ENABLED(CONFIG_ALTP2M) )` before calling
> hvm_fast_singlestep in vm_event_toggle_singlestep, to make sure that
> hvm_fast_singlestep is unreachable when ALTP2M=n.
>
> Also, the #ifdef CONFIG_ALTP2M inside hvm_fast_singlestep was moved to
> be
> _outside_ the function (wrapping it entirely).
>
> - Removed formatting of the hvmemul_vmfunc function header + moved
> the #ifdef to be inside the function body.
>
> - Removed altp2m_vcpu_update_{p2m,vmfunc_ve}() declarations
> from hvm.h, since they are not used when CONFIG_ALTP2M is not set.
>
> - xen: Make the maximum number of altp2m views configurable for x86
> - In do_altp2m_op, -EOPNOTSUPP is now returned when nr_altp2m == 0,
> instead
> of -EINVAL.
>
> - Removed stray empty lines that were introduced in
> p2m_mem_access_emulate_check & p2m_mem_access_check.
>
> Changes since v9:
> - Introduced 3 more patches:
> - x86/p2m: Short-circuit p2m_is_altp2m() when CONFIG_ALTP2M=n
> - Needed to optimize out `if ( p2m_is_altp2m(p2m) )` when CONFIG_ALTP2M=n.
> - x86/altp2m: Remove p2m_altp2m_check stubs from unsupported architectures
> - Removes p2m_altp2m_check stubs from archs that don't support altp2m.
> - x86/altp2m: Wrap altp2m-specific code in #ifdef CONFIG_ALTP2M
> - Self-explanatory - based on Jans review suggestion
> - Small changes based on review:
> - if ( v->domain->nr_altp2m == 0 ) -> if ( !v->domain->nr_altp2m )
> - removed unnecessary condition in p2m_teardown_altp2m
> - Added ASSERT(!config->altp2m.nr) when CONFIG_ALTP2M=n in domain_create
>
> Changes since v8:
> - Removed 1st patch, which was already applied.
> - Removed Reviewed-by from "tools/xl: Add altp2m_count parameter" patch.
> - (No code changes)
>
> Changes since v7:
> - Fix condition in libxl_create.c that caused assertions in CI.
> - Removed incorrect mention of introduction of hvm_altp2m_supported() in
> commit
> message of patch 0004.
> - Adjust comments in altp2m_is_eptp_valid().
> - Guard ALTP2M features with CONFIG_ALTP2M where appropriate. I made my best
> to
> ensure that the following configurations compile correctly:
> * CONFIG_HVM=n (CONFIG_ALTP2M=n implied)
> * CONFIG_HVM=y CONFIG_ALTP2M=n
> * CONFIG_HVM=y CONFIG_ALTP2M=y
> * Note: Besides taking inspiration from CONFIG_MEM_SHARING &
> CONFIG_MEM_PAGING, the reason for guards instead of wrapper methods is
> that
> all ALTP2M features and fields became guarded by CONFIG_ALTP2M -
> therefore,
> lots of code inside ALTP2M blocks would be touching inexisting fields.
>
> This could be solved by introducing wrapped functions for each ALTP2M
> field, but I believe that would be overkill.
>
> Changes since v6:
> - Rebased on top of staging
> - Added missing Acks/Reviewed-bys where appropriate.
> - No changes in patches since v6, with the exception of 0004: xen: Make the
> maximum number of altp2m views configurable for x86... which was the only
> patch that was left unacked/not reviewed
> - In that patch, I made changes suggested by Jan - that is:
> - Create altp2m_is_eptp_valid function and use it in places where
> we don't control the index
Re: [PATCH] x86/iommu: setup MMCFG ahead of IOMMU
On 20/08/2025 12:33 pm, Roger Pau Monné wrote: > On Tue, Aug 19, 2025 at 07:23:57PM +0100, Andrew Cooper wrote: >> On 19/08/2025 6:18 pm, Roger Pau Monne wrote: >>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >>> index 6fb42c5a5f95..bd648323bfed 100644 >>> --- a/xen/arch/x86/setup.c >>> +++ b/xen/arch/x86/setup.c >>> @@ -1938,11 +1938,10 @@ void asmlinkage __init noreturn __start_xen(void) >>> setup_system_domains(); >>> >>> /* >>> - * Ahead of any ACPI table parsing make sure we have control structures >>> - * for PCI segment 0. >>> + * Initialize PCI (create segment 0, setup MMCFG access) ahead of IOMMU >>> + * setup, as it requires access to the PCI config space. >>> */ >> Again, this isn't terribly clear IMO. >> >> "ahead of IOMMU setup, as the IOMMUs might not all live on segment 0." ? > It's not just IOMMUs, but for example on VT-d we also need to poke at > the config space of bridges, and when such bridges live in segment > 0 > that results in garbage being returned. > > I'm not sure acpi_iommu_init() accesses the IOMMU PCI device config > space, but it does at least access the config space of bridges in > order to detect hierarchy. See how acpi_parse_dev_scope() performs > PCI reads. > > What about using: > > /* > * Initialize PCI (create segment 0, setup MMCFG access) ahead of IOMMU > * setup, as devices in segment > 0 must also be discoverable. > */ Yeah, that works. With those comment adjustments, Reviewed-by: Andrew Cooper
Re: [PATCH 2/5] xen/vpci: make BAR mapping more resilient for the hardware domain
On 8/18/25 03:42, Roger Pau Monné wrote:
> On Fri, Aug 15, 2025 at 02:53:35PM -0400, Stewart Hildebrand wrote:
>> On 8/14/25 12:03, Roger Pau Monne wrote:
>>> The logic in map_range() will bubble up failures to the upper layer, which
>>> will result in any remaining regions being skip, and for the non-hardware
>>> domain case the owner domain of the device would be destroyed. However for
>>> the hardware domain the intent is to continue execution, hopping the
>>> failure to modify the p2m could be worked around by the hardware domain.
>>>
>>> To accomplish that in a better way, ignore failures and skip the range in
>>> that case, possibly continuing to map further ranges.
>>>
>>> Since the error path in vpci_process_pending() should only be used by domUs
>>> now, and it will unconditionally end up calling domain_crash(), simplify
>>> it: there's no need to cleanup if the domain will be destroyed.
>>>
>>> No functional change for domUs intended.
>>>
>>> Signed-off-by: Roger Pau Monné
>>> ---
>>> xen/drivers/vpci/header.c | 51 +--
>>> 1 file changed, 28 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
>>> index b9756b364300..1035dcca242d 100644
>>> --- a/xen/drivers/vpci/header.c
>>> +++ b/xen/drivers/vpci/header.c
>>> @@ -213,28 +224,22 @@ bool vpci_process_pending(struct vcpu *v)
>>> return true;
>>> }
>>>
>>> -if ( rc )
>>> +if ( rc && !is_hardware_domain(v->domain) )
>>> {
>>> -spin_lock(&pdev->vpci->lock);
>>> -/* Disable memory decoding unconditionally on failure. */
>>> -modify_decoding(pdev, v->vpci.cmd & ~PCI_COMMAND_MEMORY,
>>> -false);
>>
>> This path could be taken for either map or unmap. Do we still want to disable
>> memory decoding in case of unmap?
>
> Does it make an effective difference? For the hardware domain we
> should never get here, and for domUs the domain will be destroyed, so
> disabling memory decoding is not helpful?
Since the domU will be destroyed, the PCI device will get assigned back to hwdom
or put into quarantine. In case of quarantine, it shouldn't matter. In case of
assignment back to hwdom, I think by keeping memory decoding enabled it could
potentially trigger additional p2m operations.
In any case, I don't have a strong opinion on disabling memory decoding vs not,
but I think the commit message ought to mention something about it.
[PATCH] common/efi: deviate Rule 2.1 violation in read_file()
MISRA C Rule 2.1 states: "A project shall not contain unreachable code."
The return statements in the 'read_file()' function is unreachable due
to function 'PrintErrMesg()' which is noreturn:
PrintErrMesg(name, ret);
/* not reached */
return false;
This is deviated using a SAF-xx-safe comment (specified in the file
'docs/misra/safe.json').
No functional change.
Signed-off-by: Dmytro Prokopchuk
---
Previous thread:
https://patchew.org/Xen/5944d87aae330246b7dab6eebd04d5d71a7d7e8f.1755608417.git.dmytro._5fprokopch...@epam.com/
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1994619131
---
docs/misra/safe.json | 8
xen/common/efi/boot.c | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 3584cb90c6..2b3178de2d 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -124,6 +124,14 @@
},
{
"id": "SAF-15-safe",
+"analyser": {
+"eclair": "MC3A2.R2.1"
+},
+"name": "Rule 2.1: unreachable code",
+"text": "This is a deliberate use of unreachable code. The return
statement is retained to improve code clarity and readability by explicitly
specifying the intended behavior for a case if PrintErrMesg() was to return."
+},
+{
+"id": "SAF-16-safe",
"analyser": {},
"name": "Sentinel",
"text": "Next ID to be used"
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 50ff1d1bd2..860c41c8e7 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -852,7 +852,7 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle,
CHAR16 *name,
PrintErr(L" failed for ");
PrintErrMesg(name, ret);
-/* not reached */
+/* SAF-15-safe deliberately unreachable code */
return false;
}
--
2.43.0
[PATCH] xen/dom0less: Clear xenstore page
C Xenstored was seen failing to map a domU's event channel. Enhancing
Xen's error messages shows:
common/event_channel.c:427:d0v0 EVTCHNOP failure: d1, port 3267543746 error -22
Port 3267543746 is 0xc2c2c2c2 - SCRUB_PATTERN.
Xenstore expects a non-zero evtchn_port to be valid. Explicitly zero
the page to avoid this.
Fixes: f78895ad78c9 ("tools/xenstored: Read event channel from xenstored page")
Signed-off-by: Jason Andryuk
---
Tweaked stubdom & dom0less C xenstored pipeline success:
Test CI pipeline:
https://gitlab.com/xen-project/people/jandryuk-amd/xen/-/pipelines/1994879466
Example failure:
https://gitlab.com/xen-project/people/jandryuk-amd/xen/-/jobs/11084356847
Alternatively, a fixes could be:
Fixes: df2c03e2ebfe ("xen/arm: Alloc XenStore page for Dom0less DomUs from
hypervisor")
If the page should have been zero from the start.
gitlab-ci uses oxenstored for most testings, so this isn't seen by
default. I don't see this in local testing, so I guess I don't have
SCRUB_PATTERN writing enabled?
---
xen/common/device-tree/dom0less-build.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/common/device-tree/dom0less-build.c
b/xen/common/device-tree/dom0less-build.c
index 6bb038111d..badc227031 100644
--- a/xen/common/device-tree/dom0less-build.c
+++ b/xen/common/device-tree/dom0less-build.c
@@ -600,6 +600,7 @@ static int __init alloc_xenstore_page(struct domain *d)
d->arch.hvm.params[HVM_PARAM_STORE_PFN] = gfn_x(gfn);
#endif
interface = map_domain_page(mfn);
+clear_page(interface);
interface->connection = XENSTORE_RECONNECT;
unmap_domain_page(interface);
--
2.50.1
Re: [PATCH v2 3/3] iommu/ipmmu-vmsa: Implement basic PCIE-IPMMU OSID support
On Mon, 14 Jul 2025, Mykyta Poturai wrote: > From: Oleksandr Tyshchenko > > Program PCIE BDF-OSID assignment according to the S4_PCIe_IPMMU-OSID > when adding PCI device to the IOMMU in ipmmu_add_device callback. > This is needed for being able to assign PCI devices to different > domains at the same time. Programmed OSID is emmited as sideband data on > the AXI bus during PCI DMA transactions and then used by IPMMU to match > the transaction to the corresponding uTLB. > > Signed-off-by: Oleksandr Tyshchenko > Signed-off-by: Mykyta Poturai Acked-by: Stefano Stabellini
Re: [PATCH v2 1/3] arm/pci: Add pci-scan boot argument
On Wed, 20 Aug 2025, Mykyta Poturai wrote:
> From: Edward Pickup
>
> This patch adds a Xen boot arguments that, if enabled, causes a call to
> existing code to scan pci devices enumerated by the firmware.
>
> This will be needed ahead of dom0less support for pci passthrough on
> arm.
>
> Signed-off-by: Luca Fancellu
> Signed-off-by: Mykyta Poturai
Acked-by: Stefano Stabellini
> ---
> (cherry picked from commit bce463e1588a45e1bfdf59fc0d5f88b16604e439 from
> the downstream branch poc/pci-passthrough from
> https://gitlab.com/xen-project/people/bmarquis/xen-arm-poc.git)
>
> v1->v2:
> * remove is_pci_scan_enabled wrapper
> * make pci_scan_enabled ro_after_init
> * drop debug prints
> * drop Edward's SOB
>
> changes since cherry-pick:
> * s/always_inline/inline/
> * replace additional kconfig option with config DEBUG
>
> Signed-off-by: Stewart Hildebrand
> ---
> docs/misc/xen-command-line.pandoc | 7 +++
> xen/arch/arm/include/asm/pci.h | 3 +++
> xen/arch/arm/pci/pci-host-common.c | 1 +
> xen/arch/arm/pci/pci.c | 24 ++--
> 4 files changed, 33 insertions(+), 2 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.pandoc
> b/docs/misc/xen-command-line.pandoc
> index a75b6c9301..762a1a4f5f 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -2059,6 +2059,13 @@ This option can be specified more than once (up to 8
> times at present).
>
> Flag to enable or disable support for PCI passthrough
>
> +### pci-scan (arm)
> +> `= `
> +
> +> Default: `false`
> +
> +Flag to enable or disable Xen PCI scan at boot.
> +
> ### pcid (x86)
> > `= | xpti=`
>
> diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h
> index 08ffcd4438..7289f7688b 100644
> --- a/xen/arch/arm/include/asm/pci.h
> +++ b/xen/arch/arm/include/asm/pci.h
> @@ -23,6 +23,7 @@
> #define pci_to_dev(pcidev) (&(pcidev)->arch.dev)
>
> extern bool pci_passthrough_enabled;
> +extern bool pci_scan_enabled;
>
> struct rangeset;
>
> @@ -155,6 +156,8 @@ bool arch_pci_device_physdevop(void);
>
> #else /*!CONFIG_HAS_PCI*/
>
> +#define pci_scan_enabled false
> +
> struct pci_dev;
>
> static inline void arch_pci_init_pdev(struct pci_dev *pdev) {}
> diff --git a/xen/arch/arm/pci/pci-host-common.c
> b/xen/arch/arm/pci/pci-host-common.c
> index 487c545f3a..d3481b05eb 100644
> --- a/xen/arch/arm/pci/pci-host-common.c
> +++ b/xen/arch/arm/pci/pci-host-common.c
> @@ -284,6 +284,7 @@ pci_host_common_probe(struct dt_device_node *dev,
> }
>
> pci_add_host_bridge(bridge);
> +pci_add_segment(bridge->segment);
>
> return bridge;
>
> diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
> index beb1f971fa..1b34e17517 100644
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -91,8 +91,14 @@ bool arch_pci_device_physdevop(void)
> bool __read_mostly pci_passthrough_enabled;
> boolean_param("pci-passthrough", pci_passthrough_enabled);
>
> +/* By default pci scan is disabled. */
> +bool __ro_after_init pci_scan_enabled;
> +boolean_param("pci-scan", pci_scan_enabled);
> +
> static int __init pci_init(void)
> {
> +int ret;
> +
> /*
> * Enable PCI passthrough when has been enabled explicitly
> * (pci-passthrough=on).
> @@ -104,9 +110,23 @@ static int __init pci_init(void)
> panic("Could not initialize PCI segment 0\n");
>
> if ( acpi_disabled )
> -return dt_pci_init();
> +ret = dt_pci_init();
> else
> -return acpi_pci_init();
> +ret = acpi_pci_init();
> +
> +if ( ret < 0 )
> +return ret;
> +
> +if ( pci_scan_enabled )
> +{
> +ret = scan_pci_devices();
> +
> +if ( ret < 0 )
> +return ret;
> +
> +}
> +
> +return 0;
> }
> __initcall(pci_init);
>
> --
> 2.34.1
>
Re: [PATCH v2 2/3] xen/pci: modify pci_add_device to handle device add by Xen
On Wed, 20 Aug 2025, Mykyta Poturai wrote: > From: Luca Fancellu > > Currently pci_add_device is called from hypercalls requested by Dom0 > to add pci devices and when the device has no domain associated with > it, it is assumed that hardware_domain is the owner. > > On the dom0less scenario, the enumeration is performed by the > firmware and Xen at boot time might want to assign some pci devices > to guests, so it has to firstly add the device and then assign it to > the final guest. > > Modify pci_add_device to have the owner domain passed as a parameter > to the function, so that when it is called from the hypercall the > owner would be the caller domain, otherwise when Xen is calling it, > it would be another domain since hw domain could not be there > (dom0less guests without Dom0 use case). > > Signed-off-by: Luca Fancellu > Signed-off-by: Mykyta Poturai Reviewed-by: Stefano Stabellini
[PATCH] misra: fix Rule 11.3 violation in 'vcpu_mark_events_pending'
MISRA C:2012 Rule 11.3 states: "A cast shall not be performed between
a pointer to object type and a pointer to a different object type."
The function 'vcpu_mark_events_pending' contains a non-compliant cast
to (unsigned long*). Remove the explicit cast and pass the compatible
pointer type to the 'guest_test_and_set_bit' macro.
Fixes: c626aa1a5a (arm: implement event injection, 2012-06-01)
Signed-off-by: Dmytro Prokopchuk
---
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1993054203
---
xen/arch/arm/domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 310c578909..6371e68cc7 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -1166,7 +1166,7 @@ void arch_dump_vcpu_info(struct vcpu *v)
void vcpu_mark_events_pending(struct vcpu *v)
{
bool already_pending = guest_test_and_set_bit(v->domain,
-0, (unsigned long *)&vcpu_info(v, evtchn_upcall_pending));
+0, &vcpu_info(v, evtchn_upcall_pending));
if ( already_pending )
return;
--
2.43.0
[PATCH] misra: add deviation for MISRA C Rule 11.3
MISRA C Rule 11.3 states: "A cast shall not be performed between a pointer
to object type and a pointer to a different object type."
Violations of this rule arise due to the 'container_of' macro, which casts
a member of a structure to its containing structure:
container_of(ptr, type, member) ({ \
typeof_field(type, member) *__mptr = (ptr); \
(type *)( (char *)__mptr - offsetof(type,member) );})
The 'container_of' macro is safe because it relies on the standardized and
well-defined 'offsetof' macro to calculate the memory address of the
containing structure, while assuming proper alignment and ensuring no
undefined behavior, provided that the input pointer is valid and points to
the specified member.
Configure Eclair to suppress violation reports related to 'container_of
macro. Update 'deviations.rst' file accordingly. Add Rule 11.3 to the
monitored list.
No functional changes.
Signed-off-by: Dmytro Prokopchuk
---
Test CI pipeline:
https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/1992968166
---
automation/eclair_analysis/ECLAIR/deviations.ecl | 8
automation/eclair_analysis/ECLAIR/monitored.ecl | 1 +
docs/misra/deviations.rst| 8
3 files changed, 17 insertions(+)
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index 7f3fd35a33..3e595e3a8c 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -403,6 +403,14 @@ because the semantics of the 'noreturn' attribute do not
alter the calling conve
}
-doc_end
+-doc_begin="Convesions in the 'container_of' macro are safe because it relies
on
+the standardized and well-defined 'offsetof' macro to calculate the memory
address
+of the containing structure, while assuming proper alignment and ensuring no
+undefined behavior, provided that the input pointer is valid and points to the
+specified member."
+-config=MC3A2.R11.3,reports+={safe,"any_area(any_loc(any_exp(macro(^container_of$"}
+-doc_end
+
-doc_begin="Conversions from and to integral types are safe, in the assumption
that the target type has enough bits to store the value.
See also Section \"4.7 Arrays and Pointers\" of \"GCC_MANUAL\""
-config=MC3A2.R11.6,casts+={safe,
diff --git a/automation/eclair_analysis/ECLAIR/monitored.ecl
b/automation/eclair_analysis/ECLAIR/monitored.ecl
index 72698b2eb1..abfc102d60 100644
--- a/automation/eclair_analysis/ECLAIR/monitored.ecl
+++ b/automation/eclair_analysis/ECLAIR/monitored.ecl
@@ -45,6 +45,7 @@
-enable=MC3A2.R10.2
-enable=MC3A2.R11.1
-enable=MC3A2.R11.2
+-enable=MC3A2.R11.3
-enable=MC3A2.R11.6
-enable=MC3A2.R11.7
-enable=MC3A2.R11.8
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 2119066531..33b045a5a9 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -393,6 +393,14 @@ Deviations related to MISRA C:2012 Rules:
(i.e., less strict) alignment requirement are safe.
- Tagged as `safe` for ECLAIR.
+ * - R11.3
+ - Convesions in the 'container_of' macro are safe because it relies on
+ the standardized and well-defined 'offsetof' macro to calculate the
memory
+ address of the containing structure, while assuming proper alignment and
+ ensuring no undefined behavior, provided that the input pointer is valid
+ and points to the specified member.
+ - Tagged as `safe` for ECLAIR.
+
* - R11.6
- Conversions from and to integral types are safe, in the assumption that
the target type has enough bits to store the value.
--
2.43.0
