[PATCH] shim: avoid building of vendor IOMMU code
There's no use for IOMMU code in the shim. Disable at least the vendor- specific code, until eventually IOMMU code can be disabled altogether. Signed-off-by: Jan Beulich --- a/xen/arch/x86/configs/pvshim_defconfig +++ b/xen/arch/x86/configs/pvshim_defconfig @@ -21,5 +21,7 @@ CONFIG_EXPERT=y # CONFIG_LIVEPATCH is not set # CONFIG_SUPPRESS_DUPLICATE_SYMBOL_WARNINGS is not set # CONFIG_TRACEBUFFER is not set +# CONFIG_AMD_IOMMU is not set +# CONFIG_INTEL_IOMMU is not set # CONFIG_DEBUG is not set # CONFIG_GDBSX is not set
[libvirt test] 184537: tolerable all pass - PUSHED
flight 184537 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/184537/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184526 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184526 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184526 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-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-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-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 2757e91c2b28b704d9a0b586fb60012450110b1a baseline version: libvirt 3a3f73ea9f1925ca5e256fa54c5aa451ddeaa19e Last test of basis 184526 2024-01-30 04:18:50 Z2 days Testing same since 184537 2024-01-31 04:24:45 Z1 days1 attempts People who touched revisions under test: Andrea Bolognani Göran Uddeborg Michal Privoznik Pavel Hrdina Purna Pavan Chandra Aekkaladevi 김인수 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 files, etc. are available at
Re: [PATCH v12 10/15] vpci/header: emulate PCI_COMMAND register for guests
On 1/25/24 10:43, Jan Beulich wrote:
> On 09.01.2024 22:51, Stewart Hildebrand wrote:
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -168,6 +168,9 @@ static void modify_decoding(const struct pci_dev *pdev,
>> uint16_t cmd,
>> if ( !rom_only )
>> {
>> pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd);
>> +/* Show DomU that we updated P2M */
>> +header->guest_cmd &= ~PCI_COMMAND_MEMORY;
>> +header->guest_cmd |= cmd & PCI_COMMAND_MEMORY;
>> header->bars_mapped = map;
>> }
>
> I don't follow what the comment means to say. The bit in question has no
> real connection to the P2M, and the guest also may have no notion of the
> underlying hypervisor's internals. Likely connected to ...
Indeed. If the comment survives to v13, I'll update it to:
/* Now that we updated P2M, show DomU change to PCI_COMMAND_MEMORY */
>
>> @@ -524,9 +527,26 @@ static void cf_check cmd_write(
>> {
>> struct vpci_header *header = data;
>>
>> +if ( !is_hardware_domain(pdev->domain) )
>> +{
>> +const struct vpci *vpci = pdev->vpci;
>> +
>> +if ( (vpci->msi && vpci->msi->enabled) ||
>> + (vpci->msix && vpci->msix->enabled) )
>> +cmd |= PCI_COMMAND_INTX_DISABLE;
>> +
>> +/*
>> + * Do not show change to PCI_COMMAND_MEMORY bit until we finish
>> + * modifying P2M mappings.
>> + */
>> +header->guest_cmd = (cmd & ~PCI_COMMAND_MEMORY) |
>> +(header->guest_cmd & PCI_COMMAND_MEMORY);
>> +}
>
> ... the comment here, but then shouldn't it be that the guest can't even
> issue a 2nd cfg space access until the present write has been carried out?
> Otherwise I'd be inclined to claim that such a partial update is unlikely
> to be spec-conformant.
Due to the raise_softirq() call added in
3e568fa9e19c ("vpci: fix deferral of long operations")
my current understanding is: when the guest toggles memory decoding, the guest
vcpu doesn't resume execution until vpci_process_pending() and
modify_decoding() have finished. So I think the guest should see a consistent
state of the register, unless it was trying to read from a different vcpu than
the one doing the writing.
Regardless, if the guest did have an opportunity to successfully read the
partially updated state of the register, I'm not really spotting what part of
the spec that would be a violation of. PCIe 6.1 has this description regarding
the bit: "When this bit is Set" and "When this bit is Clear" the device will
decode (or not) memory accesses. The spec doesn't seem to distinguish whether
the host or the device itself is the one to set/clear the bit. One might even
try to argue the opposite: allowing the bit to be toggled before the device
reflects the change would be a violation of spec. Since the spec is ambiguous
in this regard, I don't think either argument is particularly strong.
Chesterton's fence: the logic for deferring the update of PCI_COMMAND_MEMORY in
guest_cmd was added between v10 and v11 of this series. I went back to look at
the review comments on v10 [1], but the rationale is still not entirely clear
to me. At the end of the day, with the information I have at hand, I suspect it
would be fine either way (whether updating guest_cmd is deferred or not). If no
other info comes to light, I'm leaning toward not deferring because it would be
simpler to update the bit right away in cmd_write().
[1] https://lore.kernel.org/xen-devel/ZVy73iJ3E8nJHvgf@macbook.local/
>
>> @@ -843,6 +885,15 @@ static int cf_check init_header(struct pci_dev *pdev)
>> if ( cmd & PCI_COMMAND_MEMORY )
>> pci_conf_write16(pdev->sbdf, PCI_COMMAND, cmd &
>> ~PCI_COMMAND_MEMORY);
>>
>> +/*
>> + * Clear PCI_COMMAND_MEMORY and PCI_COMMAND_IO for DomUs, so they will
>> + * always start with memory decoding disabled and to ensure that we
>> will not
>> + * call modify_bars() at the end of this function.
>> + */
>> +if ( !is_hwdom )
>> +cmd &= ~(PCI_COMMAND_MEMORY | PCI_COMMAND_IO);
>> +header->guest_cmd = cmd;
>
> With PCI_COMMAND_MEMORY clear, the hw reg won't further be written on the
> success return path. Yet wouldn't we better clear PCI_COMMAND_IO also in
> hardware (until we properly support it)?
Yes, I'll clear PCI_COMMAND_IO in hardware too
>
> I also think the insertion point for the new code isn't well chosen: The
> comment just out of context indicates that the code in context above is
> connected to the subsequent code. Whereas the addition is not.
I'll rearrange it
>
>> --- a/xen/drivers/vpci/msi.c
>> +++ b/xen/drivers/vpci/msi.c
>> @@ -70,6 +70,15 @@ static void cf_check control_write(
>>
>> if ( vpci_msi_arch_enable(msi, pdev, vectors) )
>> return;
>> +
>> +/*
>> + * Make sure domU doesn't enable INTx while enabling MSI.
>> + */
>
> Nit: This ought to be a
RE: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
> Cc: Jonas Blixt ; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
>
> Hi Julien,
>
> 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.
> """
> >
> >>
> >> Provide a basic platform glue that implements the needed SMC forwarding.
> >>
> >> Signed-off-by: John Ernberg
> >> ---
> >> NOTE: This is based on code found in NXP Xen tree located here:
> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
> >> hub.com%2Fnxp-imx%2Fimx-xen%2Fblob%2Flf-
> 5.10.y_4.13%2Fxen%2Farch%2Far
> >>
> m%2Fplatforms%2Fimx8qm.c=05%7C02%7Cpeng.fan%40nxp.com%7C
> 573b599a
> >>
> 4b4143ceca1d08dc2271e5be%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7
> >>
> C638423119777601548%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> wMDAiLCJQI
> >>
> joiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C=ZO
> 0TXjL6
> >> g0W7TIZo8x8lTNBXEZW%2BDNcLPndWlEf5D2A%3D=0
> >
> > Anything after --- will be removed while applied to the three. I think
> > this NOTE should be written down in the commit message.
>
> Ack.
> >
> > You also possibly want a signed-off-by from Peng as this is his code.
>
> @Peng: May I add a sign-off from you?
Yeah. You could add my sign off.
> >
> >>
> >> xen/arch/arm/platforms/Makefile | 1 +
> >> xen/arch/arm/platforms/imx8qm.c | 65
> >> +
> >> 2 files changed, 66 insertions(+)
> >> create mode 100644 xen/arch/arm/platforms/imx8qm.c
> >>
> >> diff --git a/xen/arch/arm/platforms/Makefile
> >> b/xen/arch/arm/platforms/Makefile index 8632f4115f..bec6e55d1f
> 100644
> >> --- a/xen/arch/arm/platforms/Makefile
> >> +++ b/xen/arch/arm/platforms/Makefile
> >> @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o
> >> obj-$(CONFIG_ALL64_PLAT) += thunderx.o
> >> obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o
> >> obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o
> >> +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o
> >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o
> >> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o diff --git
> >> a/xen/arch/arm/platforms/imx8qm.c
> b/xen/arch/arm/platforms/imx8qm.c
> >> new file mode 100644 index 00..a9cd9c3615
> >> --- /dev/null
> >> +++ b/xen/arch/arm/platforms/imx8qm.c
> >> @@ -0,0 +1,65 @@
> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> >> +/*
> >> + * xen/arch/arm/platforms/imx8qm.c
> >> + *
> >> + * i.MX 8QM setup
> >> + *
> >> + * Copyright (c) 2016 Freescale Inc.
> >> + * Copyright 2018-2019 NXP
> >> + *
> >> + *
> >> + * Peng Fan
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +
> >> +static const char * const imx8qm_dt_compat[] __initconst = {
> >> + "fsl,imx8qm",
> >> + "fsl,imx8qxp",
> >> + NULL
> >> +};
> >> +
> >> +static bool imx8qm_smc(struct cpu_user_regs *regs) {
> >
> > Your implementation below will not only forward SMC for dom0 but also
> > for any non-trusted domains. Have you investigated that all the SIP
> > calls are safe to be called by anyone?
>
> We use pure virtualized domUs, so we do not expect any calls to this SMC
> interface from the guest. I'll limit it to dom0.
Would you mind to share what features are supported in your DomU?
Pure virtualized, you using xen pv or virtio?
Thanks,
Peng.
> >
> > 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)
> >
> > Cheers,
> >
>
> Thanks! // John Ernberg
[xen-unstable test] 184535: tolerable FAIL - PUSHED
flight 184535 xen-unstable real [real] flight 184546 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/184535/ http://logs.test-lab.xenproject.org/osstest/logs/184546/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 184546-retest Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 20 guest-localmigrate/x10 fail REGR. vs. 184527 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184527 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184527 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 184527 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184527 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 184527 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 184527 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184527 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184527 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184527 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184527 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 184527 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184527 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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
Re: Thoughts on current Xen EDAC/MCE situation
On Wed, Jan 24, 2024 at 07:20:56AM -0800, Elliott Mitchell wrote: > On Wed, Jan 24, 2024 at 08:23:15AM +0100, Jan Beulich wrote: > > > > Third, as to Dom0's purposes of having the address: If all it is to use > > it for is to pass it back to Xen, paths in the respective drivers will > > necessarily be entirely different for the Xen vs the native cases. > > I'm less than certain of the best place for Xen to intercept MCE events. > For UE memory events, the simplest approach on Linux might be to wrap the > memory_failure() function. Yet for Linux/x86, > mce_register_decode_chain() also looks like a very good candidate. I did hope to get some response. It really does look like, aside from being x86-only, mce_register_decode_chain() is the ideal hook point. Xen could forward NMIs to Domain 0, then intercept them from the decode chain. For UEs Xen would mark the event handled, then create a new event for whichever domain (if any) was effected. Right now my main concern is several of the Linux MCE/EDAC drivers are growing `if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR)) return -ENODEV;` calls. This approach is being poisoned and will become quite difficult if this isn't stopped. The justification found for one instance was that it "removed one message", with no useful information. I cannot help suspecting it involved a hypervisor from Redmond, WA and their engineers are encouraged to poison interfaces used by others. -- (\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/) \BS (| ehem+sig...@m5p.com PGP 87145445 |) / \_CS\ | _ -O #include O- _ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Re: [PATCH net] xen-netback: properly sync TX responses
On Mon, 29 Jan 2024 14:03:08 +0100 Jan Beulich wrote:
> Invoking the make_tx_response() / push_tx_responses() pair with no lock
> held would be acceptable only if all such invocations happened from the
> same context (NAPI instance or dealloc thread). Since this isn't the
> case, and since the interface "spec" also doesn't demand that multicast
> operations may only be performed with no in-flight transmits,
> MCAST_{ADD,DEL} processing also needs to acquire the response lock
> around the invocations.
>
> To prevent similar mistakes going forward, "downgrade" the present
> functions to private helpers of just the two remaining ones using them
> directly, with no forward declarations anymore. This involves renaming
> what so far was make_tx_response(), for the new function of that name
> to serve the new (wrapper) purpose.
>
> While there,
> - constify the txp parameters,
> - correct xenvif_idx_release()'s status parameter's type,
> - rename {,_}make_tx_response()'s status parameters for consistency with
> xenvif_idx_release()'s.
Hi Paul, is this one on your TODO list to review or should
we do our best? :)
--
pw-bot: needs-ack
Re: Nullifying Recently Introduced Xen Headers Check
On 01/02/2024 12:05 am, John L. Poole wrote:
> Greetings,
>
> Gentoo Linux maintains two packages for Xen:
>
> 1) app-emulation/xen-tools ("xen-tools")
> 2) app-emulation/xen ("xen")
>
> xen-tools is a prerequisite to xen. So a failure
> to install xen-tools will preclude any attempt to install xen.
> I acknowledge that building xen-tools before building xen
> is backwards and/or runs contrary to your project's way of
> building, but that is what the Gentoo paradigm has been.
>
> In trying to troubleshoot why a Gentoo 4.18.0 ebuild fails,
> I ended up downloading your archive and successfully building it.
> I then decided to compare the products built to see where
> they varied in processing order. The Xen Project builds xen first,
> then the tools.
>
> To see a visualization of the to build systems and the object
> files they create matched in chronological order, see:
>
> https://salemdata.us/xen/gentoo-4.18.0/comparison_of_build_orders_Xen_4.18.xhtml
> or the LibreOffice workbook I created the comparison in:
> https://salemdata.us/xen/gentoo-4.18.0/comparison_of_build_orders_Xen_4.18.ods
>
> In March 2023, this commit added some sort of headers check:
>
> https://xenbits.xen.org/gitweb/?p=qemu-xen.git;a=commit;f=include/hw/xen/xen_native.h;h=e2abfe5ec67b69fb310fbeaacf7e68d61d16609e
>
> Specifically, the lines 4-6 in [qemu-xen.git] / include / hw / xen /
> xen_native.h:
>
> 4 #ifdef __XEN_INTERFACE_VERSION__
> 5 #error In Xen native files, include xen_native.h before other Xen
> headers
> 6 #endif
>
> cause Gentoo's build to error out. See line 24790:
>
> 5 | #error In Xen native files, include xen_native.h before other
> Xen headers
>
> at https://salemdata.us/xen/xen_tools_20240128_Sun_174740.script.html.
>
> What I have done is create a patch for a draft Gentoo ebuild which nullifies
> lines 4-6 by wrapping them in a comment:
>
> --- a/tools/qemu-xen/include/hw/xen/xen_native.h
> +++ b/tools/qemu-xen/include/hw/xen/xen_native.h
> @@ -1,9 +1,9 @@
> #ifndef QEMU_HW_XEN_NATIVE_H
> #define QEMU_HW_XEN_NATIVE_H
>
> -#ifdef __XEN_INTERFACE_VERSION__
> +/* #ifdef __XEN_INTERFACE_VERSION__
> #error In Xen native files, include xen_native.h before other Xen
> headers
> -#endif
> +#endif */
>
> This patch allows the Gentoo xen-tools ebuild to successfully build.
>
> Question: is there a risk nullifying the above 3 lines in xen_native.h
> given Gentoo's backward build paradigm?
CC'ing David who authored that Qemu patch. He'll be best placed to answer.
However, as a word of warning if you're not aware. The name
__XEN_INTERFACE_VERSION__ is massively dishonest; read it as if it said
"unstable interfaces".
We're working to stabilise the tools->xen interface, but the nice shiny
future is still a while away.
~Andrew
Nullifying Recently Introduced Xen Headers Check
Greetings,
Gentoo Linux maintains two packages for Xen:
1) app-emulation/xen-tools ("xen-tools")
2) app-emulation/xen ("xen")
xen-tools is a prerequisite to xen. So a failure
to install xen-tools will preclude any attempt to install xen.
I acknowledge that building xen-tools before building xen
is backwards and/or runs contrary to your project's way of
building, but that is what the Gentoo paradigm has been.
In trying to troubleshoot why a Gentoo 4.18.0 ebuild fails,
I ended up downloading your archive and successfully building it.
I then decided to compare the products built to see where
they varied in processing order. The Xen Project builds xen first,
then the tools.
To see a visualization of the to build systems and the object
files they create matched in chronological order, see:
https://salemdata.us/xen/gentoo-4.18.0/comparison_of_build_orders_Xen_4.18.xhtml
or the LibreOffice workbook I created the comparison in:
https://salemdata.us/xen/gentoo-4.18.0/comparison_of_build_orders_Xen_4.18.ods
In March 2023, this commit added some sort of headers check:
https://xenbits.xen.org/gitweb/?p=qemu-xen.git;a=commit;f=include/hw/xen/xen_native.h;h=e2abfe5ec67b69fb310fbeaacf7e68d61d16609e
Specifically, the lines 4-6 in [qemu-xen.git] / include / hw / xen /
xen_native.h:
4 #ifdef __XEN_INTERFACE_VERSION__
5 #error In Xen native files, include xen_native.h before other Xen
headers
6 #endif
cause Gentoo's build to error out. See line 24790:
5 | #error In Xen native files, include xen_native.h before other
Xen headers
at https://salemdata.us/xen/xen_tools_20240128_Sun_174740.script.html.
What I have done is create a patch for a draft Gentoo ebuild which nullifies
lines 4-6 by wrapping them in a comment:
--- a/tools/qemu-xen/include/hw/xen/xen_native.h
+++ b/tools/qemu-xen/include/hw/xen/xen_native.h
@@ -1,9 +1,9 @@
#ifndef QEMU_HW_XEN_NATIVE_H
#define QEMU_HW_XEN_NATIVE_H
-#ifdef __XEN_INTERFACE_VERSION__
+/* #ifdef __XEN_INTERFACE_VERSION__
#error In Xen native files, include xen_native.h before other Xen
headers
-#endif
+#endif */
This patch allows the Gentoo xen-tools ebuild to successfully build.
Question: is there a risk nullifying the above 3 lines in xen_native.h
given Gentoo's backward build paradigm?
Thank you.
John Poole
[linux-linus test] 184534: regressions - FAIL
flight 184534 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/184534/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 184525 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184525 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184525 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184525 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184525 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184525 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184525 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184525 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184525 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-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-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass 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 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: linux2a6526c4f389bb741e511be11721b3d1cbf1034a baseline version: linux861c0981648f5b64c86fd028ee622096eb7af05a Last test of basis 184525 2024-01-30 04:10:43 Z1 days Testing same since 184534 2024-01-30 23:45:05 Z0 days1 attempts People who touched revisions under test: Alexander Gordeev Anders Roxell Arthur Grillo Dan Carpenter David Gow Joe Lawrence Kees Cook Linus Torvalds Marco Pagani Marcos Paulo de Souza Mark Brown Mathieu Desnoyers Miroslav Benes Rae Moar Richard Fitzgerald Shuah Khan jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386
Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05
On 31/01/2024 5:34 pm, Edwin Torok wrote: > > >> On 31 Jan 2024, at 17:17, Andrew Cooper >> wrote: >> >> On 31/01/2024 4:36 pm, Anthony PERARD wrote: >>> On Wed, Jan 31, 2024 at 10:42:49AM +, Edwin Török wrote: We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us back. So bump to OCaml 4.05 instead, which should match the version on OSSTest? >>> Yes, it's looks that's the version osstest can currently use. >>> I've started an osstest flight with this patch series and your other >>> ocaml patch series, and so far osstest seems happy with it. The flight >>> isn't finished but all build jobs succeed, and a lot of the tests jobs >>> as well. >>> >>> So: >>> Acked-by: Anthony PERARD >> >> A question, while I think about it. >> >> I understand why we want patch 1. The 4.02 -> 4.03 bump is necessary to >> also compile with 5.0 >> >> But why this 4.03 -> 4.05 bump? There is no other change in this patch. > > > The oldest supported Debian has 4.05, and I can’t find a non-EOL > distro with 4.03 or 4.04 here: https://repology.org/project/ocaml/versions > I also have another series (that I haven’t sent out yet) which would > use Dune 1.x in an attempt to use Dune in a way that works on OSSTest, > and the oldest release I can test this on is Debian 10. > > We could keep the minimum at 4.03, but would anything in the CI > actually be able to test that? Nah - that's a good enough reason to go to 4.05. However, the two patches ought to be folded together, with both parts of the justification given in one commit message. Otherwise to anyone doing git blame, you've entirely hidden the 5.0 build fix with something that just looks like 4.03->4.05 I can sort this out on commit. ~Andrew
[ovmf test] 184542: all pass - PUSHED
flight 184542 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/184542/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 97c3f5b8d27230acfc20f479adea64c348750612 baseline version: ovmf 40a45b5a2be3bf886ff481d4b538d20624d02589 Last test of basis 184540 2024-01-31 11:12:46 Z0 days Testing same since 184542 2024-01-31 13:12:38 Z0 days1 attempts People who touched revisions under test: Abdul Lateef Attar Michael D Kinney Min Xu Ray Ni Tom Lendacky jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 40a45b5a2b..97c3f5b8d2 97c3f5b8d27230acfc20f479adea64c348750612 -> xen-tested-master
Re: [RFC KERNEL PATCH v4 3/3] PCI/sysfs: Add gsi sysfs for pci_dev
On Wed, Jan 31, 2024 at 09:58:19AM +0100, Roger Pau Monné wrote: > On Tue, Jan 30, 2024 at 02:44:03PM -0600, Bjorn Helgaas wrote: > > On Tue, Jan 30, 2024 at 10:07:36AM +0100, Roger Pau Monné wrote: > > > On Mon, Jan 29, 2024 at 04:01:13PM -0600, Bjorn Helgaas wrote: > > > > On Thu, Jan 25, 2024 at 07:17:24AM +, Chen, Jiqian wrote: > > > > > On 2024/1/24 00:02, Bjorn Helgaas wrote: > > > > > > On Tue, Jan 23, 2024 at 10:13:52AM +, Chen, Jiqian wrote: > > > > > >> On 2024/1/23 07:37, Bjorn Helgaas wrote: > > > > > >>> On Fri, Jan 05, 2024 at 02:22:17PM +0800, Jiqian Chen wrote: > > > > > There is a need for some scenarios to use gsi sysfs. > > > > > For example, when xen passthrough a device to dumU, it will > > > > > use gsi to map pirq, but currently userspace can't get gsi > > > > > number. > > > > > So, add gsi sysfs for that and for other potential scenarios. > > > > > >> ... > > > > > > > > > > > >>> I don't know enough about Xen to know why it needs the GSI in > > > > > >>> userspace. Is this passthrough brand new functionality that > > > > > >>> can't be > > > > > >>> done today because we don't expose the GSI yet? > > > > > > > > I assume this must be new functionality, i.e., this kind of > > > > passthrough does not work today, right? > > > > > > > > > >> has ACPI support and is responsible for detecting and controlling > > > > > >> the hardware, also it performs privileged operations such as the > > > > > >> creation of normal (unprivileged) domains DomUs. When we give to a > > > > > >> DomU direct access to a device, we need also to route the physical > > > > > >> interrupts to the DomU. In order to do so Xen needs to setup and > > > > > >> map > > > > > >> the interrupts appropriately. > > > > > > > > > > > > What kernel interfaces are used for this setup and mapping? > > > > > > > > > > For passthrough devices, the setup and mapping of routing physical > > > > > interrupts to DomU are done on Xen hypervisor side, hypervisor only > > > > > need userspace to provide the GSI info, see Xen code: > > > > > xc_physdev_map_pirq require GSI and then will call hypercall to pass > > > > > GSI into hypervisor and then hypervisor will do the mapping and > > > > > routing, kernel doesn't do the setup and mapping. > > > > > > > > So we have to expose the GSI to userspace not because userspace itself > > > > uses it, but so userspace can turn around and pass it back into the > > > > kernel? > > > > > > No, the point is to pass it back to Xen, which doesn't know the > > > mapping between GSIs and PCI devices because it can't execute the ACPI > > > AML resource methods that provide such information. > > > > > > The (Linux) kernel is just a proxy that forwards the hypercalls from > > > user-space tools into Xen. > > > > But I guess Xen knows how to interpret a GSI even though it doesn't > > have access to AML? > > On x86 Xen does know how to map a GSI into an IO-APIC pin, in order > configure the RTE as requested. IIUC, mapping a GSI to an IO-APIC pin requires information from the MADT. So I guess Xen does use the static ACPI tables, but not the AML _PRT methods that would connect a GSI with a PCI device? I guess this means Xen would not be able to deal with _MAT methods, which also contains MADT entries? I don't know the implications of this -- maybe it means Xen might not be able to use with hot-added devices? The tables (including DSDT and SSDTS that contain the AML) are exposed to userspace via /sys/firmware/acpi/tables/, but of course that doesn't mean Xen knows how to interpret the AML, and even if it did, Xen probably wouldn't be able to *evaluate* it since that could conflict with the host kernel's use of AML. Bjorn
[PATCH] tools/xentop: add option to display dom0 first
Add a command line option to xentop to be able to display dom0 first, on top of
the list.
This is unconditional, so sorting domains with the S option will also ignore
dom0.
Signed-off-by: Cyril Rébert (zithro)
---
tools/xentop/xentop.c | 31 +++
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/tools/xentop/xentop.c b/tools/xentop/xentop.c
index 950e8935c4..9068c53fd2 100644
--- a/tools/xentop/xentop.c
+++ b/tools/xentop/xentop.c
@@ -211,6 +211,7 @@ int show_networks = 0;
int show_vbds = 0;
int repeat_header = 0;
int show_full_name = 0;
+int dom0_first = -1;
#define PROMPT_VAL_LEN 80
const char *prompt = NULL;
char prompt_val[PROMPT_VAL_LEN];
@@ -240,6 +241,7 @@ static void usage(const char *program)
"-b, --batch output in batch mode, no user input
accepted\n"
"-i, --iterations number of iterations before exiting\n"
"-f, --full-name output the full domain name (not
truncated)\n"
+ "-z, --dom0-first display dom0 first (ignore sorting)\n"
"\n" XENTOP_BUGSTO,
program);
return;
@@ -1162,7 +1164,8 @@ void do_vbd(xenstat_domain *domain)
static void top(void)
{
xenstat_domain **domains;
- unsigned int i, num_domains = 0;
+ unsigned int i, num_domains, sort_start, sort_count = 0;
+ int dom0_index = -1;
/* Now get the node information */
if (prev_node != NULL)
@@ -1183,11 +1186,27 @@ static void top(void)
if(domains == NULL)
fail("Failed to allocate memory\n");
- for (i=0; i < num_domains; i++)
+ for (i=0; i < num_domains; i++) {
domains[i] = xenstat_node_domain_by_index(cur_node, i);
+ if ( strcmp(xenstat_domain_name(domains[i]), "Domain-0") == 0 )
+ dom0_index = i;
+ }
+
+ /* Handle dom0 position, not for dom0-less */
+ if ( dom0_first == 1 && dom0_index != -1 ){
+ /* if dom0 is not first in domains, swap it there */
+ if ( dom0_index != 0 ){
+ xenstat_domain *tmp;
+ tmp = domains[0];
+ domains[0] = domains[dom0_index];
+ domains[dom0_index] = tmp;
+ }
+ sort_start = 1;
+ sort_count = 1;
+ }
/* Sort */
- qsort(domains, num_domains, sizeof(xenstat_domain *),
+ qsort((domains+sort_start), (num_domains-sort_count),
sizeof(xenstat_domain *),
(int(*)(const void *, const void *))compare_domains);
if(first_domain_index >= num_domains)
@@ -1242,9 +1261,10 @@ int main(int argc, char **argv)
{ "batch", no_argument, NULL, 'b' },
{ "iterations",required_argument, NULL, 'i' },
{ "full-name", no_argument, NULL, 'f' },
+ { "dom0-first",no_argument, NULL, 'z' },
{ 0, 0, 0, 0 },
};
- const char *sopts = "hVnxrvd:bi:f";
+ const char *sopts = "hVnxrvd:bi:fz";
if (atexit(cleanup) != 0)
fail("Failed to install cleanup handler.\n");
@@ -1286,6 +1306,9 @@ int main(int argc, char **argv)
case 'f':
show_full_name = 1;
break;
+ case 'z':
+ dom0_first = 1;
+ break;
}
}
--
2.39.2
[RFC PATCH] xen/arm: improve handling of load/store instruction decoding
While debugging VirtIO on Arm we ran into a warning due to memory
being memcpy'd across MMIO space. While the bug was in the mappings
the warning was a little confusing:
(XEN) d47v2 Rn should not be equal to Rt except for r31
(XEN) d47v2 unhandled Arm instruction 0x3d80
(XEN) d47v2 Unable to decode instruction
The Rn == Rt warning is only applicable to single register load/stores
so add some verification steps before to weed out unexpected accesses.
I updated the Arm ARM reference to the online instruction decoding
table which will hopefully be more stable than the Arm ARM section
numbers.
Signed-off-by: Alex Bennée
Cc: Manos Pitsidianakis
---
xen/arch/arm/decode.c | 20
xen/arch/arm/decode.h | 38 +++---
2 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 2537dbebc1..824025c24c 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -87,6 +87,26 @@ static int decode_arm64(register_t pc, mmio_info_t *info)
return 1;
}
+/*
+ * Check this is a load/store of some sort
+ */
+if ( (opcode.top_level.op1 & 0b0101) != 0b0100 )
+{
+gprintk(XENLOG_ERR, "Not a load/store instruction op1=%d",
+opcode.top_level.op1);
+goto bad_loadstore;
+}
+
+/*
+ * We are only expecting single register load/stores
+ */
+if ( (opcode.ld_st.op0 & 0b0011) != 0b0011 )
+{
+gprintk(XENLOG_ERR, "Not single register load/store op0=%d",
+opcode.ld_st.op0);
+goto bad_loadstore;
+}
+
/*
* Refer Arm v8 ARM DDI 0487G.b, Page - C6-1107
* "Shared decode for all encodings" (under ldr immediate)
diff --git a/xen/arch/arm/decode.h b/xen/arch/arm/decode.h
index 13db8ac968..b1580178eb 100644
--- a/xen/arch/arm/decode.h
+++ b/xen/arch/arm/decode.h
@@ -24,9 +24,27 @@
#include
/*
- * Refer to the ARMv8 ARM (DDI 0487G.b), Section C4.1.4 Loads and Stores
- * Page 318 specifies the following bit pattern for
- * "load/store register (immediate post-indexed)".
+ * From:
+ * https://developer.arm.com/documentation/ddi0602/2023-12/Index-by-Encoding
+ *
+ * Top level encoding:
+ *
+ * 31 30 29 28 25 24 0
+ * ___
+ * |op0 | x x | op1 | |
+ * ||__|__|___|
+ *
+ * op0 = 0 is reserved
+ * op1 = x1x0 for Loads and Stores
+ *
+ * Loads and Stores
+ *
+ * 3128 27 26 25 24 9 80
+ * ___
+ * | op0 | 1 | op1 | 0 | op2 | |
+ * ||___|_|___||__|
+ *
+ * Load/store register (immediate post-indexed)
*
* 31 30 29 27 26 25 23 21 20 11 9 4 0
* ___
@@ -35,6 +53,20 @@
*/
union instr {
uint32_t value;
+struct {
+unsigned int ign2:25;
+unsigned int op1:4; /* instruction class */
+unsigned int ign1:2;
+unsigned int op0:1; /* value = 1b */
+} top_level;
+struct {
+unsigned int ign1:9;
+unsigned int op2:15;
+unsigned int fixed1:1; /* value = 0b */
+unsigned int op1:1;
+unsigned int fixed2:1; /* value = 1b */
+unsigned int op0:4;
+} ld_st;
struct {
unsigned int rt:5; /* Rt register */
unsigned int rn:5; /* Rn register */
--
2.39.2
[xen-4.18-testing test] 184532: tolerable FAIL - PUSHED
flight 184532 xen-4.18-testing real [real] flight 184543 xen-4.18-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/184532/ http://logs.test-lab.xenproject.org/osstest/logs/184543/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 18 guest-localmigrate/x10 fail pass in 184543-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184133 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184133 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184133 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184133 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184133 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184133 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184133 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 184133 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 184133 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184133 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 184133 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 184133 test-amd64-i386-libvirt-xsm 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-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-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-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 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen c7ac596a575a05d6ff1e35c3ff98bc4d143712d2 baseline version: xen 1792d1723b7fb45a20b145d2de4d233913b22c09 Last test of basis 184133
Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05
> On 31 Jan 2024, at 17:17, Andrew Cooper wrote: > > On 31/01/2024 4:36 pm, Anthony PERARD wrote: >> On Wed, Jan 31, 2024 at 10:42:49AM +, Edwin Török wrote: >>> We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us >>> back. >>> So bump to OCaml 4.05 instead, which should match the version on >>> OSSTest? >> Yes, it's looks that's the version osstest can currently use. >> I've started an osstest flight with this patch series and your other >> ocaml patch series, and so far osstest seems happy with it. The flight >> isn't finished but all build jobs succeed, and a lot of the tests jobs >> as well. >> >> So: >> Acked-by: Anthony PERARD > > A question, while I think about it. > > I understand why we want patch 1. The 4.02 -> 4.03 bump is necessary to > also compile with 5.0 > > But why this 4.03 -> 4.05 bump? There is no other change in this patch. The oldest supported Debian has 4.05, and I can’t find a non-EOL distro with 4.03 or 4.04 here: https://repology.org/project/ocaml/versions I also have another series (that I haven’t sent out yet) which would use Dune 1.x in an attempt to use Dune in a way that works on OSSTest, and the oldest release I can test this on is Debian 10. We could keep the minimum at 4.03, but would anything in the CI actually be able to test that? Best regards, —Edwin > > If it's "just because", then why should we take it? All it's doing is > moving a baseline which doesn't need appear to need to move. > > ~Andrew
Re: [PATCH v3 14/34] xen/riscv: introduce io.h
On Tue, 2024-01-16 at 17:09 +0100, Jan Beulich wrote:
> On 16.01.2024 16:20, Oleksii wrote:
> > On Mon, 2024-01-15 at 17:57 +0100, Jan Beulich wrote:
> > > On 22.12.2023 16:12, Oleksii Kurochko wrote:
> > > > +/*
> > > > + * Unordered I/O memory access primitives. These are even
> > > > more
> > > > relaxed than
> > > > + * the relaxed versions, as they don't even order accesses
> > > > between
> > > > successive
> > > > + * operations to the I/O regions.
> > > > + */
> > > > +#define readb_cpu(c) ({ u8 __r = __raw_readb(c);
> > > > __r;
> > > > })
> > > > +#define readw_cpu(c) ({ u16 __r =
> > > > le16_to_cpu((__force
> > > > __le16)__raw_readw(c)); __r; })
> > > > +#define readl_cpu(c) ({ u32 __r =
> > > > le32_to_cpu((__force
> > > > __le32)__raw_readl(c)); __r; })
> > > > +
> > > > +#define
> > > > writeb_cpu(v,c) ((void)__raw_writeb((v),(c)))
> > > > +#define
> > > > writew_cpu(v,c) ((void)__raw_writew((__force
> > > > u16)cpu_to_le16(v),(c)))
> > > > +#define
> > > > writel_cpu(v,c) ((void)__raw_writel((__force
> > > > u32)cpu_to_le32(v),(c)))
> > > > +
> > > > +#ifdef CONFIG_64BIT
> > > > +#define readq_cpu(c) ({ u64 __r =
> > > > le64_to_cpu((__force
> > > > __le64)__raw_readq(c)); __r; })
> > > > +#define
> > > > writeq_cpu(v,c) ((void)__raw_writeq((__force
> > > > u64)cpu_to_le64(v),(c)))
> > > > +#endif
> > >
> > > How come there are endianness assumptions here on the MMIO
> > > accessed?
> > It is a hard story.
> >
> > As you might expect it was copy from Linux Kernel where it was
> > decided
> > to follow only LE way:
> > https://patchwork.kernel.org/project/linux-riscv/patch/2019045623.5749-3-...@lst.de/
> > One of the answers of the author of the commit:
> > And we don't know if Linux will be around if that ever changes.
> > The point is:
> > a) the current RISC-V spec is LE only
> > b) the current linux port is LE only except for this little
> > bit
> > There is no point in leaving just this bitrotting code around.
> > It
> > just confuses developers, (very very slightly) slows down
> > compiles
> > and will bitrot. It also won't be any significant help to a
> > future
> > developer down the road doing a hypothetical BE RISC-V Linux
> > port.
>
> Reads to me like a justification to _omit_ the cpu_to_le().
Looks like we can omit cpu_to_le(). Even docs say that memory system
is little-endian except:
However, certain application areas, such as IP networking, operate
on big-endian data structures, and certain legacy code bases have
been built assuming big-endian
processors, so we expect that future specifications will describe
big-endian or bi-endian variants
of RISC-V.
~ Oleksii
Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05
On 31/01/2024 4:36 pm, Anthony PERARD wrote: > On Wed, Jan 31, 2024 at 10:42:49AM +, Edwin Török wrote: >> We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us >> back. >> So bump to OCaml 4.05 instead, which should match the version on >> OSSTest? > Yes, it's looks that's the version osstest can currently use. > I've started an osstest flight with this patch series and your other > ocaml patch series, and so far osstest seems happy with it. The flight > isn't finished but all build jobs succeed, and a lot of the tests jobs > as well. > > So: > Acked-by: Anthony PERARD A question, while I think about it. I understand why we want patch 1. The 4.02 -> 4.03 bump is necessary to also compile with 5.0 But why this 4.03 -> 4.05 bump? There is no other change in this patch. If it's "just because", then why should we take it? All it's doing is moving a baseline which doesn't need appear to need to move. ~Andrew
[PATCH] livepatch-build-tools: allow patch file name sizes up to 128 characters
XenServer uses quite long Xen version names, and encode such in the livepatch
filename, and it's currently running out of space in the file name.
Bump max filename size to 128, so it also matches the patch name length in the
hypervisor interface.
Signed-off-by: Roger Pau Monné
---
livepatch-build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/livepatch-build b/livepatch-build
index cdb852cc7fea..78dd2d801048 100755
--- a/livepatch-build
+++ b/livepatch-build
@@ -72,8 +72,8 @@ function make_patch_name()
fi
# Only allow alphanumerics and '_' and '-' in the patch name. Everything
-# else is replaced with '-'. Truncate to 48 chars.
-echo ${PATCHNAME//[^a-zA-Z0-9_-]/-} |cut -c 1-48
+# else is replaced with '-'. Truncate to 128 chars.
+echo ${PATCHNAME//[^a-zA-Z0-9_-]/-} |cut -c -128
}
# Do a full normal build
--
2.43.0
Re: [PATCH v3 16/33] tools/libs/light: add backend type for 9pfs PV devices
On Wed, Jan 31, 2024 at 04:18:43PM +0100, Jürgen Groß wrote:
> On 12.01.24 17:55, Anthony PERARD wrote:
> > On Thu, Jan 04, 2024 at 10:00:38AM +0100, Juergen Gross wrote:
> > > +static int xen9pfsd_spawn(libxl__egc *egc, uint32_t domid,
> > > libxl_device_p9 *p9,
> > > + libxl__ao_device *aodev)
> > > +{
> > > +STATE_AO_GC(aodev->ao);
> > > +struct libxl__aop9_state *aop9;
> > > +int rc;
> > > +char *args[] = { "xen-9pfsd", NULL };
> > > +char *path = GCSPRINTF("/local/domain/%u/libxl/xen-9pfs",
> > > + p9->backend_domid);
> > > +
> > > +if (p9->type != LIBXL_P9_TYPE_XEN_9PFSD ||
> > > +libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", path)))
> >
> > I feel like this check and this function might not work as expected.
> > What happen if we try to add more than one 9pfs "device"? libxl I think
> > is going to try to start several xen-9pfs daemon before the first one
> > have had time to write the "*/state" path.
>
> I don't think so. The path is specific for the _backend_ domid.
>
> > What about two different libxl process trying to spawn that daemon? Is
> > xen-9pfs going to behave well and have one giveup? But that would
> > probably mean that libxl is going to have an error due to the process
> > exiting early, maybe.
>
> I think I need to handle this case gracefully in the daemon by exiting with
> a 0 exit code.
As long as the scenario is handle somehow, I'm happy.
> > Could you reorder the file, to make it easier to follow the code of
> > the async style? "xen9pfsd_spawn()" should be first, followed by
> > _confirm() _failed and _detached() and finally xen9pfsd_spawn_outcome().
>
> This would need to add some forward declarations. If you really are fine with
> that, I can do the reordering.
What's wrong with forward declarations?
When you write a bunch of function that are supposed to be called one by
one, but the next one to be called is above the current function in the
source file, isn't that a bit like top-posting?
Anyway, writing function in the source code in a chronological order,
with forward declaration, is part of the libxl CODING_STYLE.
Cheers,
--
Anthony PERARD
Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05
On Wed, Jan 31, 2024 at 10:42:49AM +, Edwin Török wrote: > We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us > back. > So bump to OCaml 4.05 instead, which should match the version on > OSSTest? Yes, it's looks that's the version osstest can currently use. I've started an osstest flight with this patch series and your other ocaml patch series, and so far osstest seems happy with it. The flight isn't finished but all build jobs succeed, and a lot of the tests jobs as well. So: Acked-by: Anthony PERARD Thanks, -- Anthony PERARD
Re: [PATCH v1 2/2] oxenstored: make Quota.t pure
> On 31 Jan 2024, at 11:17, Christian Lindig wrote: > > > >> On 31 Jan 2024, at 10:52, Edwin Török wrote: >> >> Now that we no longer have a hashtable inside we can make Quota.t pure, >> and push the mutable update to its callers. >> Store.t already had a mutable Quota.t field. >> >> No functional change. > > Acked-by: Christian Lindig > > This is shifting copying working to GC work, at least potentially. I would > agree that this is a good trade-off and the code looks correct to me. But I > think we should see more testing and benchmarking before merging this unless > we are fine merging speculative improvements. > > — C > > I’ve written this [1] microbenchmark which creates ~300_000 entries in xenstore, assigns quota to 1000 domains and then measure how long it takes to list xenstore (It doesn’t matter whether these domains exist or not). It shows a measurable improvement with this patch, once I’ve run a more realistic test on the original machine with 1000 real VMs I’ll post those results too: On an Intel Xeon Gold 6354 @ 3.0 Ghz: * without my patch: 12.756 +- 0.152 seconds time elapsed ( +- 1.19% ) * with my patch: 5.6051 +- 0.0467 seconds time elapsed ( +- 0.83% ) [1] # cat >create.py
Re: [PATCH v6 01/15] xen/common: add cache coloring common code
On 29.01.2024 18:17, Carlo Nonato wrote: > Last Level Cache (LLC) coloring allows to partition the cache in smaller > chunks called cache colors. Since not all architectures can actually > implement it, add a HAS_LLC_COLORING Kconfig and put other options under > xen/arch. > > LLC colors are a property of the domain, so the domain struct has to be > extended. > > Based on original work from: Luca Miccio > > Signed-off-by: Carlo Nonato > Signed-off-by: Marco Solieri > --- > v6: > - moved almost all code in common > - moved documentation in this patch > - reintroduced range for CONFIG_NR_LLC_COLORS > - reintroduced some stub functions to reduce the number of checks on > llc_coloring_enabled > - moved domain_llc_coloring_free() in same patch where allocation happens > - turned "d->llc_colors" to pointer-to-const > - llc_coloring_init() now returns void and panics if errors are found > v5: > - used - instead of _ for filenames > - removed domain_create_llc_colored() > - removed stub functions > - coloring domain fields are now #ifdef protected > v4: > - Kconfig options moved to xen/arch > - removed range for CONFIG_NR_LLC_COLORS > - added "llc_coloring_enabled" global to later implement the boot-time > switch > - added domain_create_llc_colored() to be able to pass colors > - added is_domain_llc_colored() macro > --- > docs/misc/cache-coloring.rst | 87 +++ > docs/misc/xen-command-line.pandoc | 27 ++ > xen/arch/Kconfig | 17 ++ > xen/common/Kconfig| 3 ++ > xen/common/Makefile | 1 + > xen/common/keyhandler.c | 3 ++ > xen/common/llc-coloring.c | 87 +++ > xen/include/xen/llc-coloring.h| 38 ++ > xen/include/xen/sched.h | 5 ++ > 9 files changed, 268 insertions(+) > create mode 100644 docs/misc/cache-coloring.rst > create mode 100644 xen/common/llc-coloring.c > create mode 100644 xen/include/xen/llc-coloring.h > > diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst > new file mode 100644 > index 00..9fe01e99e1 > --- /dev/null > +++ b/docs/misc/cache-coloring.rst > @@ -0,0 +1,87 @@ > +Xen cache coloring user guide > += > + > +The cache coloring support in Xen allows to reserve Last Level Cache (LLC) > +partitions for Dom0, DomUs and Xen itself. Currently only ARM64 is supported. > + > +To compile LLC coloring support set ``CONFIG_LLC_COLORING=y``. > + > +If needed, change the maximum number of colors with > +``CONFIG_NR_LLC_COLORS=``. > + > +Compile Xen and the toolstack and then configure it via > +`Command line parameters`_. > + > +Background > +** > + > +Cache hierarchy of a modern multi-core CPU typically has first levels > dedicated > +to each core (hence using multiple cache units), while the last level is > shared > +among all of them. Such configuration implies that memory operations on one > +core (e.g. running a DomU) are able to generate interference on another core > +(e.g .hosting another DomU). Cache coloring allows eliminating this > +mutual interference, and thus guaranteeing higher and more predictable > +performances for memory accesses. > +The key concept underlying cache coloring is a fragmentation of the memory > +space into a set of sub-spaces called colors that are mapped to disjoint > cache > +partitions. Technically, the whole memory space is first divided into a > number > +of subsequent regions. Then each region is in turn divided into a number of > +subsequent sub-colors. The generic i-th color is then obtained by all the > +i-th sub-colors in each region. > + > +:: > + > +Region jRegion j+1 > +. > +. . . > +. . > +_ _ ___ _ _ _ _ > +| | | | | | | > +| c_0 | c_1 | | c_n | c_0 | c_1 | > + _ _ _|_|_|_ _ _|_|_|_|_ _ _ > +: : > +: :... ... . > +:color 0 > +:... ... . > +: > + . . ..: > + > +There are two pragmatic lesson to be learnt. > + > +1. If one wants to avoid cache interference between two domains, different > + colors needs to be used for their memory. > + > +2. Color assignment must privilege contiguity in the partitioning. E.g., > + assigning colors (0,1) to domain I and (2,3) to domain J is better than > + assigning colors (0,2) to I and (1,3) to J. I can't connect this 2nd point with any of what was said above. > +How to compute the number of
Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
Hi Julien,
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.
"""
>
>>
>> Provide a basic platform glue that implements the needed SMC forwarding.
>>
>> Signed-off-by: John Ernberg
>> ---
>> NOTE: This is based on code found in NXP Xen tree located here:
>> https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c
>
> Anything after --- will be removed while applied to the three. I think
> this NOTE should be written down in the commit message.
Ack.
>
> You also possibly want a signed-off-by from Peng as this is his code.
@Peng: May I add a sign-off from you?
>
>>
>> xen/arch/arm/platforms/Makefile | 1 +
>> xen/arch/arm/platforms/imx8qm.c | 65 +
>> 2 files changed, 66 insertions(+)
>> create mode 100644 xen/arch/arm/platforms/imx8qm.c
>>
>> diff --git a/xen/arch/arm/platforms/Makefile
>> b/xen/arch/arm/platforms/Makefile
>> index 8632f4115f..bec6e55d1f 100644
>> --- a/xen/arch/arm/platforms/Makefile
>> +++ b/xen/arch/arm/platforms/Makefile
>> @@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o
>> obj-$(CONFIG_ALL64_PLAT) += thunderx.o
>> obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o
>> obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o
>> +obj-$(CONFIG_ALL64_PLAT) += imx8qm.o
>> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o
>> obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o
>> diff --git a/xen/arch/arm/platforms/imx8qm.c
>> b/xen/arch/arm/platforms/imx8qm.c
>> new file mode 100644
>> index 00..a9cd9c3615
>> --- /dev/null
>> +++ b/xen/arch/arm/platforms/imx8qm.c
>> @@ -0,0 +1,65 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * xen/arch/arm/platforms/imx8qm.c
>> + *
>> + * i.MX 8QM setup
>> + *
>> + * Copyright (c) 2016 Freescale Inc.
>> + * Copyright 2018-2019 NXP
>> + *
>> + *
>> + * Peng Fan
>> + */
>> +
>> +#include
>> +#include
>> +
>> +static const char * const imx8qm_dt_compat[] __initconst =
>> +{
>> + "fsl,imx8qm",
>> + "fsl,imx8qxp",
>> + NULL
>> +};
>> +
>> +static bool imx8qm_smc(struct cpu_user_regs *regs)
>> +{
>
> Your implementation below will not only forward SMC for dom0 but also
> for any non-trusted domains. Have you investigated that all the SIP
> calls are safe to be called by anyone?
We use pure virtualized domUs, so we do not expect any calls to this SMC
interface from the guest. I'll limit it to dom0.
>
> 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)
>
> Cheers,
>
Thanks! // John Ernberg
Re: [PATCH 2/2] xen/drivers: imx-lpuart: Add iMX8QXP compatible
Hi Julien,
On 1/31/24 13:29, Julien Grall wrote:
> Hi John,
>
> On 31/01/2024 11:50, John Ernberg wrote:
>> Allow the uart to probe also with iMX8QXP. The ip-block is the same as
>> in the QM,
>> only the compatible is needed.
>>
>> Signed-off-by: John Ernberg
>
> With one remark below:
>
> Acked-by: Julien Grall
>
>> ---
>> xen/drivers/char/imx-lpuart.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/xen/drivers/char/imx-lpuart.c
>> b/xen/drivers/char/imx-lpuart.c
>> index 77f70c2719..c85e81109a 100644
>> --- a/xen/drivers/char/imx-lpuart.c
>> +++ b/xen/drivers/char/imx-lpuart.c
>> @@ -257,6 +257,7 @@ static int __init imx_lpuart_init(struct
>> dt_device_node *dev,
>> static const struct dt_device_match imx_lpuart_dt_compat[]
>> __initconst =
>> {
>> DT_MATCH_COMPATIBLE("fsl,imx8qm-lpuart"),
>> + DT_MATCH_COMPATIBLE("fsl,imx8qxp-lpuart"),
>
> IIUC the binding, the Device-Tree node compatible should have both
> fsl,imx8qm-lpuart and fsl,imx8qxp-lpuart. In fact, the Linux driver
> doesn't recognize the first compatible.
>
> So maybe we can remove the first one.
It's listed in the bindings but the Linux driver indeed never looks for
qm, it should be safe to drop. I'll drop this in V2.
>
> Cheers,
>
Thanks! // John Ernberg
Re: [PATCH v3 17/33] tools/xl: support new 9pfs backend xen-9pfsd
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.
Juergen
Re: [PATCH v3 16/33] tools/libs/light: add backend type for 9pfs PV devices
On 15.01.24 16:38, Anthony PERARD wrote:
On Thu, Jan 04, 2024 at 10:00:38AM +0100, Juergen Gross wrote:
+static void libxl__device_p9_add(libxl__egc *egc, uint32_t domid,
+ libxl_device_p9 *p9,
+ libxl__ao_device *aodev)
+{
+int rc;
Can you make a copy of `p9` here, or maybe in xen9pfsd_spawn? There's no
guaranty that `p9` will still be around by the time
xen9pfsd_spawn_outcome() is executed.
(with libxl_device_p9_copy())
Okay.
+rc = xen9pfsd_spawn(egc, domid, p9, aodev);
+if (rc == 1)
+return;
+
+if (rc == 0)
+libxl__device_add_async(egc, domid, __p9_devtype, p9, aodev);
+
+aodev->rc = rc;
+if (rc)
+aodev->callback(egc, aodev);
+}
+
Juergen
Re: [PATCH v3 16/33] tools/libs/light: add backend type for 9pfs PV devices
On 12.01.24 17:55, Anthony PERARD wrote:
On Thu, Jan 04, 2024 at 10:00:38AM +0100, Juergen Gross wrote:
diff --git a/tools/libs/light/libxl_9pfs.c b/tools/libs/light/libxl_9pfs.c
index 5ab0d3aa21..486bc4326e 100644
--- a/tools/libs/light/libxl_9pfs.c
+++ b/tools/libs/light/libxl_9pfs.c
@@ -33,20 +33,159 @@ static int libxl__set_xenstore_p9(libxl__gc *gc, uint32_t
domid,
flexarray_append_pair(front, "tag", p9->tag);
+if (p9->type == LIBXL_P9_TYPE_XEN_9PFSD) {
+flexarray_append_pair(back, "max-space",
+ GCSPRINTF("%u", p9->max_space));
+flexarray_append_pair(back, "max-files",
+ GCSPRINTF("%u", p9->max_files));
+flexarray_append_pair(back, "max-open-files",
+ GCSPRINTF("%u", p9->max_open_files));
+flexarray_append_pair(back, "auto-delete",
+ p9->auto_delete ? "1" : "0");
+}
+
+return 0;
+}
+
+static int libxl__device_from_p9(libxl__gc *gc, uint32_t domid,
+ libxl_device_p9 *type, libxl__device *device)
+{
+device->backend_devid = type->devid;
+device->backend_domid = type->backend_domid;
+device->backend_kind= type->type == LIBXL_P9_TYPE_QEMU
+ ? LIBXL__DEVICE_KIND_9PFS
+ : LIBXL__DEVICE_KIND_XEN_9PFS;
+device->devid = type->devid;
+device->domid = domid;
+device->kind= LIBXL__DEVICE_KIND_9PFS;
+
return 0;
}
-#define libxl__add_p9s NULL
+static int libxl_device_p9_dm_needed(void *e, unsigned domid)
Prefix of the function should be "libxl__" as it's only internal to
libxl.
Okay.
+{
+libxl_device_p9 *elem = e;
+
+return elem->type == LIBXL_P9_TYPE_QEMU && elem->backend_domid == domid;
+}
+
+typedef struct libxl__aop9_state libxl__aop9_state;
+
+struct libxl__aop9_state {
+libxl__spawn_state spawn;
+libxl__ao_device *aodev;
+libxl_device_p9 *p9;
+uint32_t domid;
+void (*callback)(libxl__egc *, libxl__aop9_state *, int);
+};
+
+static void xen9pfsd_spawn_outcome(libxl__egc *egc, libxl__aop9_state *aop9,
+ int rc)
+{
+aop9->aodev->rc = rc;
+if (rc)
+aop9->aodev->callback(egc, aop9->aodev);
+else
+libxl__device_add_async(egc, aop9->domid, __p9_devtype,
+aop9->p9, aop9->aodev);
+}
+
+static void xen9pfsd_confirm(libxl__egc *egc, libxl__spawn_state *spawn,
+ const char *xsdata)
+{
+STATE_AO_GC(spawn->ao);
+
+if (!xsdata)
+return;
+
+if (strcmp(xsdata, "running"))
+return;
+
+libxl__spawn_initiate_detach(gc, spawn);
+}
+
+static void xen9pfsd_failed(libxl__egc *egc, libxl__spawn_state *spawn, int rc)
+{
+libxl__aop9_state *aop9 = CONTAINER_OF(spawn, *aop9, spawn);
+
+xen9pfsd_spawn_outcome(egc, aop9, rc);
+}
+
+static void xen9pfsd_detached(libxl__egc *egc, libxl__spawn_state *spawn)
+{
+libxl__aop9_state *aop9 = CONTAINER_OF(spawn, *aop9, spawn);
+
+xen9pfsd_spawn_outcome(egc, aop9, 0);
+}
+
+static int xen9pfsd_spawn(libxl__egc *egc, uint32_t domid, libxl_device_p9 *p9,
+ libxl__ao_device *aodev)
+{
+STATE_AO_GC(aodev->ao);
+struct libxl__aop9_state *aop9;
+int rc;
+char *args[] = { "xen-9pfsd", NULL };
+char *path = GCSPRINTF("/local/domain/%u/libxl/xen-9pfs",
+ p9->backend_domid);
+
+if (p9->type != LIBXL_P9_TYPE_XEN_9PFSD ||
+libxl__xs_read(gc, XBT_NULL, GCSPRINTF("%s/state", path)))
I feel like this check and this function might not work as expected.
What happen if we try to add more than one 9pfs "device"? libxl I think
is going to try to start several xen-9pfs daemon before the first one
have had time to write the "*/state" path.
I don't think so. The path is specific for the _backend_ domid.
What about two different libxl process trying to spawn that daemon? Is
xen-9pfs going to behave well and have one giveup? But that would
probably mean that libxl is going to have an error due to the process
exiting early, maybe.
I think I need to handle this case gracefully in the daemon by exiting with
a 0 exit code.
+return 0;
+
+GCNEW(aop9);
+aop9->aodev = aodev;
+aop9->p9 = p9;
+aop9->domid = domid;
+aop9->callback = xen9pfsd_spawn_outcome;
+
+aop9->spawn.ao = aodev->ao;
+aop9->spawn.what = "xen-9pfs daemon";
+aop9->spawn.xspath = GCSPRINTF("%s/state", path);
+aop9->spawn.timeout_ms = LIBXL_DEVICE_MODEL_START_TIMEOUT * 1000;
+aop9->spawn.pidpath = GCSPRINTF("%s/pid", path);
+aop9->spawn.midproc_cb = libxl__spawn_record_pid;
+aop9->spawn.confirm_cb = xen9pfsd_confirm;
+aop9->spawn.failure_cb = xen9pfsd_failed;
+aop9->spawn.detached_cb = xen9pfsd_detached;
+rc = libxl__spawn_spawn(egc, >spawn);
+if (rc < 0)
+
Re: [PATCH v7 6/7] xen/arm: switch Arm to use asm-generic/device.h
On 26.01.2024 16:42, Oleksii Kurochko wrote:
> Signed-off-by: Oleksii Kurochko
I'm not an Arm maintainer, but if I was I wouldn't let you get away with
an empty description here. Specifically at least ...
> --- a/xen/arch/arm/device.c
> +++ b/xen/arch/arm/device.c
> @@ -16,7 +16,10 @@
> #include
>
> extern const struct device_desc _sdevice[], _edevice[];
> +
> +#ifdef CONFIG_ACPI
> extern const struct acpi_device_desc _asdevice[], _aedevice[];
> +#endif
>
> int __init device_init(struct dt_device_node *dev, enum device_class class,
> const void *data)
> @@ -45,6 +48,7 @@ int __init device_init(struct dt_device_node *dev, enum
> device_class class,
> return -EBADF;
> }
>
> +#ifdef CONFIG_ACPI
> int __init acpi_device_init(enum device_class class, const void *data, int
> class_type)
> {
> const struct acpi_device_desc *desc;
> @@ -61,6 +65,7 @@ int __init acpi_device_init(enum device_class class, const
> void *data, int class
>
> return -EBADF;
> }
> +#endif
... this new #ifdef-ary would want justifying, imo.
Jan
Re: [PATCH v7 5/7] xen/asm-generic: introduce generic device.h
On 26.01.2024 16:42, Oleksii Kurochko wrote:
> Arm, PPC and RISC-V use the same device.h thereby device.h
> was moved to asm-generic.
It's not "move" anymore with the splitting off of the Arm and PPC
parts. For reasons mentioned before, I'm not exactly happy with
it not being a move anymore, but I expect you were asked to split.
> Arm's device.h was taken as a base with
> the following changes:
> - #ifdef PCI related things.
Well, not really, with ...
> - #ifdef ACPI related things.
> - Rename #ifdef guards.
> - Add SPDX tag.
> - #ifdef CONFIG_HAS_DEVICE_TREE related things.
> - #ifdef-ing iommu related things with CONFIG_HAS_PASSTHROUGH.
>
> Signed-off-by: Oleksii Kurochko
> ---
> Changes in V7:
> - keeping DEVICE_PCI_HOSTBRIDGE available for every build based on the reply:
>
> https://lore.kernel.org/xen-devel/926a5c12-7f02-42ec-92a8-1c82d060c...@xen.org/
... this. Specifically ...
> --- /dev/null
> +++ b/xen/include/asm-generic/device.h
> @@ -0,0 +1,162 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __ASM_GENERIC_DEVICE_H__
> +#define __ASM_GENERIC_DEVICE_H__
> +
> +#include
> +
> +/*
> + * DEV_TYPE_MAX is currently not in use, but it was added because the enum
> may
> + * be empty when !HAS_DEVICE_TREE and !HAS_PCI, which could lead to
> + * a compilation error.
> + */
> +enum device_type
> +{
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +DEV_DT,
> +#endif
> +
> +#ifdef CONFIG_HAS_PCI
> +DEV_PCI,
> +#endif
... this is now inconsistent with ...
> +DEV_TYPE_MAX,
> +};
> +
> +enum device_class
> +{
> +DEVICE_SERIAL,
> +DEVICE_IOMMU,
> +DEVICE_INTERRUPT_CONTROLLER,
> +DEVICE_PCI_HOSTBRIDGE,
... this. Either we want PCI-related #ifdef-ary, or we don't. There
shouldn't be a mix (unless there's a good reason).
Also the use of blank lines inside the earlier enum would better be
consistent.
> +/* Use for error */
> +DEVICE_UNKNOWN,
> +};
> +
> +struct dev_archdata {
> +#ifdef CONFIG_HAS_PASSTHROUGH
> +void *iommu;/* IOMMU private data */
> +#endif
> +};
> +
> +/* struct device - The basic device structure */
> +struct device
> +{
> +enum device_type type;
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> +#endif
> +struct dev_archdata archdata;
> +#ifdef CONFIG_HAS_PASSTHROUGH
> +struct iommu_fwspec *iommu_fwspec; /* per-device IOMMU instance data */
> +#endif
> +};
> +
> +typedef struct device device_t;
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +
> +#include
> +
> +#define dev_is_dt(dev) ((dev)->type == DEV_DT)
> +
> +/**
> + * device_init - Initialize a device
> + * @dev: device to initialize
> + * @class: class of the device (serial, network...)
> + * @data: specific data for initializing the device
> + *
> + * Return 0 on success.
> + */
> +int device_init(struct dt_device_node *dev, enum device_class class,
> +const void *data);
> +
> +/**
> + * device_get_type - Get the type of the device
> + * @dev: device to match
> + *
> + * Return the device type on success or DEVICE_ANY on failure
> + */
> +enum device_class device_get_class(const struct dt_device_node *dev);
> +
> +#define DT_DEVICE_START(_name, _namestr, _class)\
Would be really nice if in the course of generalization these leading
underscores would also disappear. Yes, that'll require changing two
of the names more than just to drop the underscores, to account for ...
> +static const struct device_desc __dev_desc_##_name __used \
> +__section(".dev.info") = { \
> +.name = _namestr, \
> +.class = _class,\
... these field names.
Also there's a strack backslash on the last line above.
Both comments similarly apply to the ACPI stuff further down.
> +#define DT_DEVICE_END \
> +};
> +
> +#else /* !CONFIG_HAS_DEVICE_TREE */
> +#define dev_is_dt(dev) ((void)(dev), false)
> +#endif /* CONFIG_HAS_DEVICE_TREE */
> +
> +#ifdef CONFIG_HAS_PCI
> +#define dev_is_pci(dev) ((dev)->type == DEV_PCI)
> +#else
> +#define dev_is_pci(dev) ((void)(dev), false)
> +#endif
> +
> +struct device_desc {
> +/* Device name */
> +const char *name;
> +/* Device class */
> +enum device_class class;
> +
> +#ifdef CONFIG_HAS_DEVICE_TREE
> +
> +/* List of devices supported by this driver */
> +const struct dt_device_match *dt_match;
> +/*
> + * Device initialization.
> + *
> + * -EAGAIN is used to indicate that device probing is deferred.
> + */
> +int (*init)(struct dt_device_node *dev, const void *data);
> +
> +#endif
> +};
> +
> +#ifdef CONFIG_ACPI
> +
> +struct acpi_device_desc {
> +/* Device name */
> +const char *name;
> +/* Device class */
> +enum device_class class;
I understand it's this way on Arm right now, and I'm
Re: [XEN v4 3/3] xen/arm: arm32: Add emulation of Debug Data Transfer Registers
Hi Ayan, On 31/01/2024 13:10, Ayan Kumar Halder wrote: > When user enables HVC_DCC config option in Linux, it invokes access to debug > transfer register (i.e. DBGDTRTXINT). As this register is not emulated, Xen > injects an undefined exception to the guest and Linux crashes. > To prevent this crash, introduce a partial emulation of DBGDTRTXINT as RAZ/WI > and TXfull should be set to 1. So that Linux will see that TXfull is set, and > it will not access DBGDTRTXINT. It will access it at least once to later check if TXfull is set. > > As a pre-requisite, DBGOSLSR should be emulated in the same way as its AArch64 > variant (ie OSLSR_EL1). > This is to ensure that DBGOSLSR.OSLK is 0, thus MDSCR_EL1.TXfull is treated > as UNK/SBZP. For that you could just emulate it as RAZ. Instead you also set OSLM[1]. So at least I would make it clear that you do that for consistency. > Only MDCCSR_EL0 can be emulated (which is DBGDSCRINT on arm32). DBGDSCRINT I would write: This way, we only need to emulate DBGDSCRINT. > can be accessed at EL0 as DBGDSCREXT is emulated as RAZ (as DBGOSLSR.OSLK > == 0). So, we tool the opportunity to fix the minimum EL for DBGDSCRINT. > DBGDSCRINT.TXfull is set to 1. I'm not sure why you are mixing AArch64 names with AArch32 ones. It reads a bit difficult. Furthermore, fixing lowest EL deserves a separate section. Also s/tool/took. > > Refer ARM DDI 0487J.a ID042523, G8.3.19, DBGDTRTXint > "If TXfull is set to 1, set DTRTX to UNKNOWN". > So, DBGDTR[TR]XINT is emulated as RAZ/WI. > > Thus, any OS is expected to read DBGDSCRINT and check for TXfull before using > DBGDTRTXINT. > > Signed-off-by: Ayan Kumar Halder > --- > Changes from > > v1 :- 1. DBGDTR_EL0 does not emulate RXfull. This is to avoid giving the OS > any > indication that the RX buffer is full and is waiting to be read. > > 2. In Arm32, DBGOSLSR is emulated. Also DBGDTRTXINT is emulated at EL0 only. > > 3. Fixed the commit message and inline code comments. > > v2 :- 1. Split the patch into two (separate patches for arm64 and arm32). > 2. Fixed in line comments and style related issues. > 3. Updated commit message to mention DBGDSCRINT handling. > > v3 :- 1. The original emulation of DBGDSCRINT is retained when > 'partial_emulation' is false. > > 2. If 'partial_emulation' is false, then access to DBGDTRTXINT will > lead to undefined exception. > > xen/arch/arm/include/asm/cpregs.h | 2 ++ > xen/arch/arm/vcpreg.c | 35 ++- > 2 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/xen/arch/arm/include/asm/cpregs.h > b/xen/arch/arm/include/asm/cpregs.h > index 6b083de204..aec9e8f329 100644 > --- a/xen/arch/arm/include/asm/cpregs.h > +++ b/xen/arch/arm/include/asm/cpregs.h > @@ -75,6 +75,8 @@ > #define DBGDIDR p14,0,c0,c0,0 /* Debug ID Register */ > #define DBGDSCRINT p14,0,c0,c1,0 /* Debug Status and Control Internal > */ > #define DBGDSCREXT p14,0,c0,c2,2 /* Debug Status and Control External > */ > +#define DBGDTRRXINT p14,0,c0,c5,0 /* Debug Data Transfer Register, > Receive */ > +#define DBGDTRTXINT p14,0,c0,c5,0 /* Debug Data Transfer Register, > Transmit */ > #define DBGVCR p14,0,c0,c7,0 /* Vector Catch */ > #define DBGBVR0 p14,0,c0,c0,4 /* Breakpoint Value 0 */ > #define DBGBCR0 p14,0,c0,c0,5 /* Breakpoint Control 0 */ > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c > index a2d0500704..87df4bd238 100644 > --- a/xen/arch/arm/vcpreg.c > +++ b/xen/arch/arm/vcpreg.c > @@ -493,11 +493,12 @@ void do_cp14_32(struct cpu_user_regs *regs, const union > hsr hsr) > * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58 > * > * Unhandled: > - *DBGOSLSR > *DBGPRCR > */ > case HSR_CPREG32(DBGOSLAR): > return handle_wo_wi(regs, regidx, cp32.read, hsr, 1); > +case HSR_CPREG32(DBGOSLSR): > +return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1, 1U << 3); > case HSR_CPREG32(DBGOSDLR): > return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); > > @@ -509,8 +510,6 @@ void do_cp14_32(struct cpu_user_regs *regs, const union > hsr hsr) > * > * Unhandled: > *DBGDCCINT > - *DBGDTRRXint > - *DBGDTRTXint > *DBGWFAR > *DBGDTRTXext > *DBGDTRRXext, > @@ -550,10 +549,22 @@ void do_cp14_32(struct cpu_user_regs *regs, const union > hsr hsr) > > case HSR_CPREG32(DBGDSCRINT): > /* > - * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis > - * is set to 0, which we emulated below. > + * Xen doesn't expose a real (or emulated) Debug Communications > + * Channel (DCC) to a domain. Yet the Arm ARM implies this is not an > + * optional feature. So some domains may start to probe it. For > + * instance, the HVC_DCC driver in Linux (since f35dc083 and at > + * least up to v6.7),
[ovmf test] 184540: all pass - PUSHED
flight 184540 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/184540/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 40a45b5a2be3bf886ff481d4b538d20624d02589 baseline version: ovmf af6e0e728f652f496a6fb1e659617c9662f774ac Last test of basis 184538 2024-01-31 04:36:38 Z0 days Testing same since 184540 2024-01-31 11:12:46 Z0 days1 attempts People who touched revisions under test: levi.yun Pierre Gondois jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git af6e0e728f..40a45b5a2b 40a45b5a2be3bf886ff481d4b538d20624d02589 -> xen-tested-master
Re: [XEN v4 2/3] xen/arm: arm64: Add emulation of Debug Data Transfer Registers
Hi Ayan, On 31/01/2024 13:10, Ayan Kumar Halder wrote: > From: Michal Orzel > > Currently, if user enables HVC_DCC config option in Linux, it invokes access > to debug data transfer registers (i.e. DBGDTRTX_EL0 on arm64, DBGDTRTXINT on > arm32). As these registers are not emulated, Xen injects an undefined > exception to the guest and Linux crashes. > > To prevent this crash, introduce a partial emulation of DBGDTR[TR]X_EL0 > (these registers share the same encoding) as RAZ/WI and MDCCSR_EL0 as TXfull. > > Refer ARM DDI 0487J.a ID042523, D19.3.8, DBGDTRTX_EL0 > "If TXfull is set to 1, set DTRRX and DTRTX to UNKNOWN". > > Thus, any OS is expected to read MDCCSR_EL0 and check for TXfull before > using DBGDTRTX_EL0. Linux does it via hvc_dcc_init() ---> hvc_dcc_check(), > and returns -ENODEV in case TXfull bit is still set after writing a test > character. This way we prevent the guest from making use of HVC DCC as a > console. > > Signed-off-by: Michal Orzel > Signed-off-by: Ayan Kumar Halder Reviewed-by: Michal Orzel ~Michal
Re: [XEN v4 1/3] xen/arm: Introduce CONFIG_PARTIAL_EMULATION and "partial-emulation" cmd option
Hi Ayan, On 31/01/2024 13:10, Ayan Kumar Halder wrote: > There can be situations when the registers cannot be emulated to their full > functionality. This can be due to the complexity involved. In such cases, one > can emulate those registers as RAZ/WI for example. We call them as partial > emulation. > > Some registers are non-optional and as such there is nothing preventing an OS > from accessing them. > Instead of injecting undefined exception (thus crashing a guest), one may want > to prefer a partial emulation to let the guest running (in some cases > accepting > the fact that it might result in unwanted behavior). > > A suitable example of this (as seen in subsequent patches) is emulation of > DBGDTRTX_EL0 (on Arm64) and DBGDTRTXINT(on Arm32). These non-optional > registers can be emulated as RAZ/WI and they can be enclosed within > CONFIG_PARTIAL_EMULATION. > > Further, "partial-emulation" command line option allows us to > enable/disable partial emulation at run time. While CONFIG_PARTIAL_EMULATION > enables support for partial emulation at compile time (i.e. adds code for > partial emulation), this option may be enabled or disabled by Yocto or other > build systems. However if the build system turns this option on, users > can use scripts like Imagebuilder to generate uboot-script which will append > "partial-emulation=false" to xen command line to turn off the partial NIT: given that the option is false by default, it would make more sense to give example with setting it to true to enable it. > emulation. Thus, it helps to avoid rebuilding xen. > > By default, "CONFIG_PARTIAL_EMULATION=y" and "partial-emulation=false". > This is done so that Xen supports partial emulation. However, customers are > fully aware when they enable partial emulation. It's important to note that > enabling such support might result in unwanted/non-spec compliant behavior. > > Signed-off-by: Ayan Kumar Halder > --- > Changes from v1 :- > 1. New patch introduced in v2. > > v2 :- > 1. Reordered the patches so that the config and command line option is > introduced in the first patch. > > v3 :- > 1. Defined a macro 'partial_emulation' to reduce if-defs. > 2. Fixed style issues. > > docs/misc/xen-command-line.pandoc | 11 +++ > xen/arch/arm/Kconfig | 9 + > xen/arch/arm/include/asm/traps.h | 6 ++ > xen/arch/arm/traps.c | 9 + > 4 files changed, 35 insertions(+) > > diff --git a/docs/misc/xen-command-line.pandoc > b/docs/misc/xen-command-line.pandoc > index 8e65f8bd18..22c0d7c9f6 100644 > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -1949,6 +1949,17 @@ This option is ignored in **pv-shim** mode. > > > Default: `on` > > +### partial-emulation (arm) > +> `= ` > + > +> Default: `false` > + > +Flag to enable or disable partial emulation of system/coprocessor registers. > +Only effective if CONFIG_PARTIAL_EMULATION is enabled. > + > +**WARNING: Enabling this option might result in unwanted/non-spec compliant > +behavior.** > + > ### pci > = List of [ serr=, perr= ] > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index 50e9bfae1a..8d8f668e7f 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -225,6 +225,15 @@ config STATIC_EVTCHN > This option enables establishing static event channel communication > between domains on a dom0less system (domU-domU as well as domU-dom0). > > +config PARTIAL_EMULATION > + bool "Enable partial emulation of system/coprocessor registers" > + default y > + help > + This option enables partial emulation of registers to prevent guests > + crashing when accessing registers which are not optional but have not > been > + emulated to its complete functionality. Enabling this might result in NIT: s/its/their Other than that: Reviewed-by: Michal Orzel ~Michal
Re: [PATCH 2/2] xen/drivers: imx-lpuart: Add iMX8QXP compatible
Hi John,
On 31/01/2024 11:50, John Ernberg wrote:
Allow the uart to probe also with iMX8QXP. The ip-block is the same as in the
QM,
only the compatible is needed.
Signed-off-by: John Ernberg
With one remark below:
Acked-by: Julien Grall
---
xen/drivers/char/imx-lpuart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/drivers/char/imx-lpuart.c b/xen/drivers/char/imx-lpuart.c
index 77f70c2719..c85e81109a 100644
--- a/xen/drivers/char/imx-lpuart.c
+++ b/xen/drivers/char/imx-lpuart.c
@@ -257,6 +257,7 @@ static int __init imx_lpuart_init(struct dt_device_node
*dev,
static const struct dt_device_match imx_lpuart_dt_compat[] __initconst =
{
DT_MATCH_COMPATIBLE("fsl,imx8qm-lpuart"),
+DT_MATCH_COMPATIBLE("fsl,imx8qxp-lpuart"),
IIUC the binding, the Device-Tree node compatible should have both
fsl,imx8qm-lpuart and fsl,imx8qxp-lpuart. In fact, the Linux driver
doesn't recognize the first compatible.
So maybe we can remove the first one.
Cheers,
--
Julien Grall
Re: [PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
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?
Provide a basic platform glue that implements the needed SMC forwarding.
Signed-off-by: John Ernberg
---
NOTE: This is based on code found in NXP Xen tree located here:
https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c
Anything after --- will be removed while applied to the three. I think
this NOTE should be written down in the commit message.
You also possibly want a signed-off-by from Peng as this is his code.
xen/arch/arm/platforms/Makefile | 1 +
xen/arch/arm/platforms/imx8qm.c | 65 +
2 files changed, 66 insertions(+)
create mode 100644 xen/arch/arm/platforms/imx8qm.c
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index 8632f4115f..bec6e55d1f 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o
obj-$(CONFIG_ALL64_PLAT) += thunderx.o
obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o
obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o
+obj-$(CONFIG_ALL64_PLAT) += imx8qm.o
obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o
obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o
diff --git a/xen/arch/arm/platforms/imx8qm.c b/xen/arch/arm/platforms/imx8qm.c
new file mode 100644
index 00..a9cd9c3615
--- /dev/null
+++ b/xen/arch/arm/platforms/imx8qm.c
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * xen/arch/arm/platforms/imx8qm.c
+ *
+ * i.MX 8QM setup
+ *
+ * Copyright (c) 2016 Freescale Inc.
+ * Copyright 2018-2019 NXP
+ *
+ *
+ * Peng Fan
+ */
+
+#include
+#include
+
+static const char * const imx8qm_dt_compat[] __initconst =
+{
+"fsl,imx8qm",
+"fsl,imx8qxp",
+NULL
+};
+
+static bool imx8qm_smc(struct cpu_user_regs *regs)
+{
Your implementation below will not only forward SMC for dom0 but also
for any non-trusted domains. Have you investigated that all the SIP
calls are safe to be called by anyone?
But even if we restrict to dom0, have you checked that none of the SMCs
use buffers?
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.
Cheers,
--
Julien Grall
[XEN v4 2/3] xen/arm: arm64: Add emulation of Debug Data Transfer Registers
From: Michal Orzel
Currently, if user enables HVC_DCC config option in Linux, it invokes access
to debug data transfer registers (i.e. DBGDTRTX_EL0 on arm64, DBGDTRTXINT on
arm32). As these registers are not emulated, Xen injects an undefined
exception to the guest and Linux crashes.
To prevent this crash, introduce a partial emulation of DBGDTR[TR]X_EL0
(these registers share the same encoding) as RAZ/WI and MDCCSR_EL0 as TXfull.
Refer ARM DDI 0487J.a ID042523, D19.3.8, DBGDTRTX_EL0
"If TXfull is set to 1, set DTRRX and DTRTX to UNKNOWN".
Thus, any OS is expected to read MDCCSR_EL0 and check for TXfull before
using DBGDTRTX_EL0. Linux does it via hvc_dcc_init() ---> hvc_dcc_check(),
and returns -ENODEV in case TXfull bit is still set after writing a test
character. This way we prevent the guest from making use of HVC DCC as a
console.
Signed-off-by: Michal Orzel
Signed-off-by: Ayan Kumar Halder
---
Changes from
v1 :- 1. DBGDTR_EL0 does not emulate RXfull. This is to avoid giving the OS any
indication that the RX buffer is full and is waiting to be read.
2. In Arm32, DBGOSLSR is emulated. Also DBGDTRTXINT is emulated at EL0 only.
3. Fixed the commit message and inline code comments.
v2 :- 1. Split the patch into two (separate patches for arm64 and arm32).
2. Removed the "fail" label.
3. Fixed the commit message.
v3 :- 1. "HSR_SYSREG_MDCCSR_EL0" emulation differs based on whether
partial_emulation_enabled is true or not.
2. If partial_emulation_enabled is false, then access to HSR_SYSREG_DBGDTR_EL0,
HSR_SYSREG_DBGDTRTX_EL0 would lead to undefined exception.
xen/arch/arm/arm64/vsysreg.c | 28
xen/arch/arm/include/asm/arm64/hsr.h | 3 +++
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/arm64/vsysreg.c b/xen/arch/arm/arm64/vsysreg.c
index b5d54c569b..94f0a6c384 100644
--- a/xen/arch/arm/arm64/vsysreg.c
+++ b/xen/arch/arm/arm64/vsysreg.c
@@ -159,9 +159,6 @@ void do_sysreg(struct cpu_user_regs *regs,
*
* Unhandled:
*MDCCINT_EL1
- *DBGDTR_EL0
- *DBGDTRRX_EL0
- *DBGDTRTX_EL0
*OSDTRRX_EL1
*OSDTRTX_EL1
*OSECCR_EL1
@@ -173,10 +170,32 @@ void do_sysreg(struct cpu_user_regs *regs,
return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
case HSR_SYSREG_MDCCSR_EL0:
/*
+ * Xen doesn't expose a real (or emulated) Debug Communications Channel
+ * (DCC) to a domain. Yet the Arm ARM implies this is not an optional
+ * feature. So some domains may start to probe it. For instance, the
+ * HVC_DCC driver in Linux (since f35dc083 and at least up to
v6.7),
+ * will try to write some characters and check if the transmit buffer
+ * has emptied.
+ *
+ * By setting TX status bit (only if partial emulation is enabled) to
+ * indicate the transmit buffer is full, we would hint the OS that the
+ * DCC is probably not working.
+ *
+ * Bit 29: TX full
+ *
* Accessible at EL0 only if MDSCR_EL1.TDCC is set to 0. We emulate
that
* register as RAZ/WI above. So RO at both EL0 and EL1.
*/
-return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr, 0);
+return handle_ro_read_val(regs, regidx, hsr.sysreg.read, hsr, 0,
+ partial_emulation ? (1U << 29) : 0);
+
+case HSR_SYSREG_DBGDTR_EL0:
+/* DBGDTR[TR]X_EL0 share the same encoding */
+case HSR_SYSREG_DBGDTRTX_EL0:
+if ( !partial_emulation )
+goto fail;
+return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 0);
+
HSR_SYSREG_DBG_CASES(DBGBVR):
HSR_SYSREG_DBG_CASES(DBGBCR):
HSR_SYSREG_DBG_CASES(DBGWVR):
@@ -394,6 +413,7 @@ void do_sysreg(struct cpu_user_regs *regs,
* And all other unknown registers.
*/
default:
+ fail:
{
const struct hsr_sysreg sysreg = hsr.sysreg;
diff --git a/xen/arch/arm/include/asm/arm64/hsr.h
b/xen/arch/arm/include/asm/arm64/hsr.h
index e691d41c17..1495ccddea 100644
--- a/xen/arch/arm/include/asm/arm64/hsr.h
+++ b/xen/arch/arm/include/asm/arm64/hsr.h
@@ -47,6 +47,9 @@
#define HSR_SYSREG_OSDLR_EL1 HSR_SYSREG(2,0,c1,c3,4)
#define HSR_SYSREG_DBGPRCR_EL1HSR_SYSREG(2,0,c1,c4,4)
#define HSR_SYSREG_MDCCSR_EL0 HSR_SYSREG(2,3,c0,c1,0)
+#define HSR_SYSREG_DBGDTR_EL0 HSR_SYSREG(2,3,c0,c4,0)
+#define HSR_SYSREG_DBGDTRTX_EL0 HSR_SYSREG(2,3,c0,c5,0)
+#define HSR_SYSREG_DBGDTRRX_EL0 HSR_SYSREG(2,3,c0,c5,0)
#define HSR_SYSREG_DBGBVRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,4)
#define HSR_SYSREG_DBGBCRn_EL1(n) HSR_SYSREG(2,0,c0,c##n,5)
--
2.25.1
[XEN v4 3/3] xen/arm: arm32: Add emulation of Debug Data Transfer Registers
When user enables HVC_DCC config option in Linux, it invokes access to debug transfer register (i.e. DBGDTRTXINT). As this register is not emulated, Xen injects an undefined exception to the guest and Linux crashes. To prevent this crash, introduce a partial emulation of DBGDTRTXINT as RAZ/WI and TXfull should be set to 1. So that Linux will see that TXfull is set, and it will not access DBGDTRTXINT. As a pre-requisite, DBGOSLSR should be emulated in the same way as its AArch64 variant (ie OSLSR_EL1). This is to ensure that DBGOSLSR.OSLK is 0, thus MDSCR_EL1.TXfull is treated as UNK/SBZP. Only MDCCSR_EL0 can be emulated (which is DBGDSCRINT on arm32). DBGDSCRINT can be accessed at EL0 as DBGDSCREXT is emulated as RAZ (as DBGOSLSR.OSLK == 0). So, we tool the opportunity to fix the minimum EL for DBGDSCRINT. DBGDSCRINT.TXfull is set to 1. Refer ARM DDI 0487J.a ID042523, G8.3.19, DBGDTRTXint "If TXfull is set to 1, set DTRTX to UNKNOWN". So, DBGDTR[TR]XINT is emulated as RAZ/WI. Thus, any OS is expected to read DBGDSCRINT and check for TXfull before using DBGDTRTXINT. Signed-off-by: Ayan Kumar Halder --- Changes from v1 :- 1. DBGDTR_EL0 does not emulate RXfull. This is to avoid giving the OS any indication that the RX buffer is full and is waiting to be read. 2. In Arm32, DBGOSLSR is emulated. Also DBGDTRTXINT is emulated at EL0 only. 3. Fixed the commit message and inline code comments. v2 :- 1. Split the patch into two (separate patches for arm64 and arm32). 2. Fixed in line comments and style related issues. 3. Updated commit message to mention DBGDSCRINT handling. v3 :- 1. The original emulation of DBGDSCRINT is retained when 'partial_emulation' is false. 2. If 'partial_emulation' is false, then access to DBGDTRTXINT will lead to undefined exception. xen/arch/arm/include/asm/cpregs.h | 2 ++ xen/arch/arm/vcpreg.c | 35 ++- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h index 6b083de204..aec9e8f329 100644 --- a/xen/arch/arm/include/asm/cpregs.h +++ b/xen/arch/arm/include/asm/cpregs.h @@ -75,6 +75,8 @@ #define DBGDIDR p14,0,c0,c0,0 /* Debug ID Register */ #define DBGDSCRINT p14,0,c0,c1,0 /* Debug Status and Control Internal */ #define DBGDSCREXT p14,0,c0,c2,2 /* Debug Status and Control External */ +#define DBGDTRRXINT p14,0,c0,c5,0 /* Debug Data Transfer Register, Receive */ +#define DBGDTRTXINT p14,0,c0,c5,0 /* Debug Data Transfer Register, Transmit */ #define DBGVCR p14,0,c0,c7,0 /* Vector Catch */ #define DBGBVR0 p14,0,c0,c0,4 /* Breakpoint Value 0 */ #define DBGBCR0 p14,0,c0,c0,5 /* Breakpoint Control 0 */ diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c index a2d0500704..87df4bd238 100644 --- a/xen/arch/arm/vcpreg.c +++ b/xen/arch/arm/vcpreg.c @@ -493,11 +493,12 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * ARMv8 (DDI 0487A.d): D1-1509 Table D1-58 * * Unhandled: - *DBGOSLSR *DBGPRCR */ case HSR_CPREG32(DBGOSLAR): return handle_wo_wi(regs, regidx, cp32.read, hsr, 1); +case HSR_CPREG32(DBGOSLSR): +return handle_ro_read_val(regs, regidx, cp32.read, hsr, 1, 1U << 3); case HSR_CPREG32(DBGOSDLR): return handle_raz_wi(regs, regidx, cp32.read, hsr, 1); @@ -509,8 +510,6 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) * * Unhandled: *DBGDCCINT - *DBGDTRRXint - *DBGDTRTXint *DBGWFAR *DBGDTRTXext *DBGDTRRXext, @@ -550,10 +549,22 @@ void do_cp14_32(struct cpu_user_regs *regs, const union hsr hsr) case HSR_CPREG32(DBGDSCRINT): /* - * Read-only register. Accessible by EL0 if DBGDSCRext.UDCCdis - * is set to 0, which we emulated below. + * Xen doesn't expose a real (or emulated) Debug Communications + * Channel (DCC) to a domain. Yet the Arm ARM implies this is not an + * optional feature. So some domains may start to probe it. For + * instance, the HVC_DCC driver in Linux (since f35dc083 and at + * least up to v6.7), will try to write some characters and check if + * the transmit buffer has emptied. By setting TX status bit to + * indicate the transmit buffer is full. This we would hint the OS + * that the DCC is probably not working. + * + * Bit 29: TX full + * + * Accessible by EL0 if DBGDSCRext.UDCCdis is set to 0, which we + * emulate as RAZ/WI in the next case. */ -return handle_ro_raz(regs, regidx, cp32.read, hsr, 1); +return handle_ro_read_val(regs, regidx, cp32.read, hsr, 0, + partial_emulation ? (1U << 29) : 0); case HSR_CPREG32(DBGDSCREXT): /* @@ -562,6 +573,12 @@ void
[XEN v4 1/3] xen/arm: Introduce CONFIG_PARTIAL_EMULATION and "partial-emulation" cmd option
There can be situations when the registers cannot be emulated to their full
functionality. This can be due to the complexity involved. In such cases, one
can emulate those registers as RAZ/WI for example. We call them as partial
emulation.
Some registers are non-optional and as such there is nothing preventing an OS
from accessing them.
Instead of injecting undefined exception (thus crashing a guest), one may want
to prefer a partial emulation to let the guest running (in some cases accepting
the fact that it might result in unwanted behavior).
A suitable example of this (as seen in subsequent patches) is emulation of
DBGDTRTX_EL0 (on Arm64) and DBGDTRTXINT(on Arm32). These non-optional
registers can be emulated as RAZ/WI and they can be enclosed within
CONFIG_PARTIAL_EMULATION.
Further, "partial-emulation" command line option allows us to
enable/disable partial emulation at run time. While CONFIG_PARTIAL_EMULATION
enables support for partial emulation at compile time (i.e. adds code for
partial emulation), this option may be enabled or disabled by Yocto or other
build systems. However if the build system turns this option on, users
can use scripts like Imagebuilder to generate uboot-script which will append
"partial-emulation=false" to xen command line to turn off the partial
emulation. Thus, it helps to avoid rebuilding xen.
By default, "CONFIG_PARTIAL_EMULATION=y" and "partial-emulation=false".
This is done so that Xen supports partial emulation. However, customers are
fully aware when they enable partial emulation. It's important to note that
enabling such support might result in unwanted/non-spec compliant behavior.
Signed-off-by: Ayan Kumar Halder
---
Changes from v1 :-
1. New patch introduced in v2.
v2 :-
1. Reordered the patches so that the config and command line option is
introduced in the first patch.
v3 :-
1. Defined a macro 'partial_emulation' to reduce if-defs.
2. Fixed style issues.
docs/misc/xen-command-line.pandoc | 11 +++
xen/arch/arm/Kconfig | 9 +
xen/arch/arm/include/asm/traps.h | 6 ++
xen/arch/arm/traps.c | 9 +
4 files changed, 35 insertions(+)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index 8e65f8bd18..22c0d7c9f6 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1949,6 +1949,17 @@ This option is ignored in **pv-shim** mode.
> Default: `on`
+### partial-emulation (arm)
+> `= `
+
+> Default: `false`
+
+Flag to enable or disable partial emulation of system/coprocessor registers.
+Only effective if CONFIG_PARTIAL_EMULATION is enabled.
+
+**WARNING: Enabling this option might result in unwanted/non-spec compliant
+behavior.**
+
### pci
= List of [ serr=, perr= ]
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 50e9bfae1a..8d8f668e7f 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -225,6 +225,15 @@ config STATIC_EVTCHN
This option enables establishing static event channel communication
between domains on a dom0less system (domU-domU as well as domU-dom0).
+config PARTIAL_EMULATION
+ bool "Enable partial emulation of system/coprocessor registers"
+ default y
+ help
+ This option enables partial emulation of registers to prevent guests
+ crashing when accessing registers which are not optional but have not
been
+ emulated to its complete functionality. Enabling this might result in
+ unwanted/non-spec compliant behavior.
+
endmenu
menu "ARM errata workaround via the alternative framework"
diff --git a/xen/arch/arm/include/asm/traps.h b/xen/arch/arm/include/asm/traps.h
index 883dae368e..9a60dbf70e 100644
--- a/xen/arch/arm/include/asm/traps.h
+++ b/xen/arch/arm/include/asm/traps.h
@@ -10,6 +10,12 @@
# include
#endif
+#ifdef CONFIG_PARTIAL_EMULATION
+extern bool partial_emulation;
+#else
+#define partial_emulation false
+#endif
+
/*
* GUEST_BUG_ON is intended for checking that the guest state has not been
* corrupted in hardware and/or that the hardware behaves as we
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 9c10e8f78c..d1c7a6c516 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -42,6 +42,15 @@
#include
#include
+/*
+ * partial_emulation: If true, partial emulation for system/coprocessor
+ * registers will be enabled.
+ */
+#ifdef CONFIG_PARTIAL_EMULATION
+bool __ro_after_init partial_emulation = false;
+boolean_param("partial-emulation", partial_emulation);
+#endif
+
/* The base of the stack must always be double-word aligned, which means
* that both the kernel half of struct cpu_user_regs (which is pushed in
* entry.S) and struct cpu_info (which lives at the bottom of a Xen
--
2.25.1
[XEN v4 0/3] xen/arm: Add emulation of Debug Data Transfer Registers
Hi, Refer https://lore.kernel.org/all/alpine.DEB.2.22.394.2312071341540.1265976@ubuntu-linux-20-04-desktop/T/ for the previous discussion on this issue. Also, the linux earlycon hvc driver has been fixed. See https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-next=0ec058ece2f933aed66b76bd5cb9b5e6764853c3 Changes from v1:- 1. Split the change across 3 patches as per the design consensus. v2 :- 1. Reordered the patches. v3 :- 1. Change mentioned in individual patches. Ayan Kumar Halder (2): xen/arm: Introduce CONFIG_PARTIAL_EMULATION and "partial-emulation" cmd option xen/arm: arm32: Add emulation of Debug Data Transfer Registers Michal Orzel (1): xen/arm: arm64: Add emulation of Debug Data Transfer Registers docs/misc/xen-command-line.pandoc| 11 + xen/arch/arm/Kconfig | 9 +++ xen/arch/arm/arm64/vsysreg.c | 28 ++ xen/arch/arm/include/asm/arm64/hsr.h | 3 +++ xen/arch/arm/include/asm/cpregs.h| 2 ++ xen/arch/arm/include/asm/traps.h | 6 + xen/arch/arm/traps.c | 9 +++ xen/arch/arm/vcpreg.c| 35 8 files changed, 89 insertions(+), 14 deletions(-) -- 2.25.1
Re: [PATCH v3 09/33] tools/xenlogd: add 9pfs walk request support
On 09.01.24 20:19, Jason Andryuk wrote:
On Thu, Jan 4, 2024 at 4:10 AM Juergen Gross wrote:
Add the walk request of the 9pfs protocol.
Signed-off-by: Juergen Gross
Reviewed-by: Jason Andryuk
With one minor comment.
+path = calloc(path_len + 1, 1);
+if ( !path )
+{
+p9_error(ring, hdr->tag, ENOMEM);
+goto out;
+}
+strcpy(path, fidp->path);
+
+if ( n_names )
+{
+qids = calloc(n_names, sizeof(*qids));
+if ( !qids )
+{
+p9_error(ring, hdr->tag, ENOMEM);
+goto out;
+}
+for ( i = 0; i < n_names; i++ )
+{
+if (strcmp(path, "/"))
+strcat(path, "/");
strcmp() can only return 0 on the first iteration, so it seems
inefficient to have it inside this loop. But the added complexity to
avoid calling it doesn't seem worthwhile.
With storing the relative path in fidp->path this call can be dropped.
Juergen
Re: [PATCH v3 08/33] tools/xenlogd: add 9pfs attach request support
On 09.01.24 19:48, Jason Andryuk wrote:
On Thu, Jan 4, 2024 at 4:12 AM Juergen Gross wrote:
Add the attach request of the 9pfs protocol. This introduces the "fid"
scheme of the 9pfs protocol.
As this will be needed later, use a dedicated memory allocation
function in alloc_fid() and prepare a fid reference count.
For filling the qid data take the approach from the qemu 9pfs backend
implementation.
Signed-off-by: Juergen Gross
---
V2:
- make fill_qid() parameter stbuf const (Jason Andryuk)
- free fids after disconnecting guest (Jason Andryuk)
V3:
- only store relative path in fid (Jason Andryuk)
The code looks good. I did have a thought though.
+static struct p9_fid *alloc_fid_mem(device *device, unsigned int fid,
+const char *path)
+{
+struct p9_fid *fidp;
+size_t pathlen;
+
+/* Paths always start with "/" as they are starting at the mount point. */
+assert(path[0] == '/');
+
...
+
+static const char *relpath_from_path(const char *path)
+{
+if (!strcmp(path, "/"))
+return ".";
+
+return (path[0] == '/') ? path + 1 : path;
+}
You've carefully written the code to ensure the *at() functions are
not called with paths starting with "/". What do you think about
storing the converted paths when storing into the p9_fid? That way
the code doesn't have to worry about always going through
relpath_from_path() before use. Another option beside performing the
relpath_from_path() conversion, would be to save fidp->path with "./"
at the start to eliminate absolute paths that way. My thinking is
it's more robust to not have any absolute paths that could be passed
to a *at() function.
I've had a thorough look at the code at the end of the series and I agree
this is a good idea.
I'll change the related patches accordingly.
Juergen
Re: [PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
On 31/01/2024 11:09 am, Jan Beulich wrote: > On 31.01.2024 11:40, Andrew Cooper wrote: >> On 31/01/2024 9:20 am, Jan Beulich wrote: >>> Allow the compiler a little more room on DCE by moving the compile-time- >>> constant condition into the predicate (from the one place where it was >>> added in an open-coded fashion for XSA-450). >>> >>> Signed-off-by: Jan Beulich >> Acked-by: Andrew Cooper > Thanks, but ... > >> I'm surprised that's happy compiling if it's actually doing anything at >> all, given that it's an unguarded variable reference. > ... I'm afraid I don't understand: What "unguarded variable reference" > are you seeing me add? Even if you were talking about the hap_pt_share > struct field (which was accessed before as well) - that's part of > struct domain_iommu no matter what .config has. Ah. That will be why it compiles. Fine. ~Andrew
[PATCH 1/2] xen/arm: Add imx8q{m,x} platform glue
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.
Provide a basic platform glue that implements the needed SMC forwarding.
Signed-off-by: John Ernberg
---
NOTE: This is based on code found in NXP Xen tree located here:
https://github.com/nxp-imx/imx-xen/blob/lf-5.10.y_4.13/xen/arch/arm/platforms/imx8qm.c
xen/arch/arm/platforms/Makefile | 1 +
xen/arch/arm/platforms/imx8qm.c | 65 +
2 files changed, 66 insertions(+)
create mode 100644 xen/arch/arm/platforms/imx8qm.c
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index 8632f4115f..bec6e55d1f 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -9,5 +9,6 @@ obj-$(CONFIG_ALL_PLAT) += sunxi.o
obj-$(CONFIG_ALL64_PLAT) += thunderx.o
obj-$(CONFIG_ALL64_PLAT) += xgene-storm.o
obj-$(CONFIG_ALL64_PLAT) += brcm-raspberry-pi.o
+obj-$(CONFIG_ALL64_PLAT) += imx8qm.o
obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp.o
obj-$(CONFIG_MPSOC_PLATFORM) += xilinx-zynqmp-eemi.o
diff --git a/xen/arch/arm/platforms/imx8qm.c b/xen/arch/arm/platforms/imx8qm.c
new file mode 100644
index 00..a9cd9c3615
--- /dev/null
+++ b/xen/arch/arm/platforms/imx8qm.c
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * xen/arch/arm/platforms/imx8qm.c
+ *
+ * i.MX 8QM setup
+ *
+ * Copyright (c) 2016 Freescale Inc.
+ * Copyright 2018-2019 NXP
+ *
+ *
+ * Peng Fan
+ */
+
+#include
+#include
+
+static const char * const imx8qm_dt_compat[] __initconst =
+{
+"fsl,imx8qm",
+"fsl,imx8qxp",
+NULL
+};
+
+static bool imx8qm_smc(struct cpu_user_regs *regs)
+{
+struct arm_smccc_res res;
+
+if ( !cpus_have_const_cap(ARM_SMCCC_1_1) )
+{
+printk_once(XENLOG_WARNING "no SMCCC 1.1 support. Disabling firmware
calls\n");
+
+return false;
+}
+
+arm_smccc_1_1_smc(get_user_reg(regs, 0),
+ get_user_reg(regs, 1),
+ get_user_reg(regs, 2),
+ get_user_reg(regs, 3),
+ get_user_reg(regs, 4),
+ get_user_reg(regs, 5),
+ get_user_reg(regs, 6),
+ get_user_reg(regs, 7),
+ );
+
+set_user_reg(regs, 0, res.a0);
+set_user_reg(regs, 1, res.a1);
+set_user_reg(regs, 2, res.a2);
+set_user_reg(regs, 3, res.a3);
+
+return true;
+}
+
+PLATFORM_START(imx8qm, "i.MX 8")
+.compatible = imx8qm_dt_compat,
+.smc = imx8qm_smc,
+PLATFORM_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.43.0
[PATCH 2/2] xen/drivers: imx-lpuart: Add iMX8QXP compatible
Allow the uart to probe also with iMX8QXP. The ip-block is the same as in the
QM,
only the compatible is needed.
Signed-off-by: John Ernberg
---
xen/drivers/char/imx-lpuart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/drivers/char/imx-lpuart.c b/xen/drivers/char/imx-lpuart.c
index 77f70c2719..c85e81109a 100644
--- a/xen/drivers/char/imx-lpuart.c
+++ b/xen/drivers/char/imx-lpuart.c
@@ -257,6 +257,7 @@ static int __init imx_lpuart_init(struct dt_device_node
*dev,
static const struct dt_device_match imx_lpuart_dt_compat[] __initconst =
{
DT_MATCH_COMPATIBLE("fsl,imx8qm-lpuart"),
+DT_MATCH_COMPATIBLE("fsl,imx8qxp-lpuart"),
{ /* sentinel */ },
};
--
2.43.0
[PATCH 0/2] Xen: ARM: Improved NXP iMX8 platform support
The iMX lpuart driver added at 44e17aa60d47 ("xen/arm: Add i.MX lpuart driver")
is not enough to boot a Linux based dom0 when certain drivers, such as the
watchdog driver, are enabled.
We're also adding iMX8QXP compatibles to allow Xen to use the UART on the
QXP variant as well.
John Ernberg (2):
xen/arm: Add imx8q{m,x} platform glue
xen/drivers: imx-lpuart: Add iMX8QXP compatible
xen/arch/arm/platforms/Makefile | 1 +
xen/arch/arm/platforms/imx8qm.c | 65 +
xen/drivers/char/imx-lpuart.c | 1 +
3 files changed, 67 insertions(+)
create mode 100644 xen/arch/arm/platforms/imx8qm.c
--
2.43.0
Re: [PATCH v1 2/2] oxenstored: make Quota.t pure
> On 31 Jan 2024, at 10:52, Edwin Török wrote: > > Now that we no longer have a hashtable inside we can make Quota.t pure, > and push the mutable update to its callers. > Store.t already had a mutable Quota.t field. > > No functional change. Acked-by: Christian Lindig This is shifting copying working to GC work, at least potentially. I would agree that this is a good trade-off and the code looks correct to me. But I think we should see more testing and benchmarking before merging this unless we are fine merging speculative improvements. — C
Re: [PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
On 31.01.2024 11:40, Andrew Cooper wrote: > On 31/01/2024 9:20 am, Jan Beulich wrote: >> Allow the compiler a little more room on DCE by moving the compile-time- >> constant condition into the predicate (from the one place where it was >> added in an open-coded fashion for XSA-450). >> >> Signed-off-by: Jan Beulich > > Acked-by: Andrew Cooper Thanks, but ... > I'm surprised that's happy compiling if it's actually doing anything at > all, given that it's an unguarded variable reference. ... I'm afraid I don't understand: What "unguarded variable reference" are you seeing me add? Even if you were talking about the hap_pt_share struct field (which was accessed before as well) - that's part of struct domain_iommu no matter what .config has. Jan
[PATCH 1/3] xen-analysis.py: Use named group for tag regex
Use named group for the regex matching a tag identifier, this
is done to ease retrieving the matching group instead of using
an index.
Signed-off-by: Luca Fancellu
---
xen/scripts/xen_analysis/cppcheck_analysis.py | 4 ++--
xen/scripts/xen_analysis/tag_database.py | 24 +--
xen/scripts/xen_analysis/utils.py | 2 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py
b/xen/scripts/xen_analysis/cppcheck_analysis.py
index e54848aa5339..919eb153ce9c 100644
--- a/xen/scripts/xen_analysis/cppcheck_analysis.py
+++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
@@ -41,7 +41,7 @@ def __generate_suppression_list(out_file):
# The following lambda function will return a file if it contains lines
with
# a comment containing "cppcheck-suppress[*]" on a single line.
grep_action = lambda x: utils.grep(x,
-r'^[ \t]*/\* cppcheck-suppress\[(.*)\] \*/$')
+r'^[ \t]*/\* cppcheck-suppress\[(?P.*)\] \*/$')
# Look for a list of .h files that matches the condition above
headers = utils.recursive_find_file(settings.xen_dir, r'.*\.h$',
grep_action)
@@ -97,7 +97,7 @@ def __generate_suppression_list(out_file):
if (not comment_section) and comment_line_starts:
comment_section = True
if (len(line.strip()) != 0) and (not comment_section):
-cppcheck_violation_id =
entry["matches"][line_num][0]
+cppcheck_violation_id =
entry["matches"][line_num]['id']
break
if comment_section and comment_line_stops:
comment_section = False
diff --git a/xen/scripts/xen_analysis/tag_database.py
b/xen/scripts/xen_analysis/tag_database.py
index ca374bbb62dd..abba163aec71 100644
--- a/xen/scripts/xen_analysis/tag_database.py
+++ b/xen/scripts/xen_analysis/tag_database.py
@@ -18,11 +18,12 @@ tool_syntax = {
def get_xen_tag_index_type_regex(tool):
-return r'^SAF-(\d+)-(safe|false-positive-' + tool + ')$'
+return rf'^SAF-(?P\d+)-(?Psafe|false-positive-{tool})$'
def get_xen_tag_comment_regex(tool):
-return r'^[ \t]*/\* +(SAF-\d+-(?:safe|false-positive-' + tool + ')).*\*/$'
+tag=rf'(?PSAF-\d+-(?:safe|false-positive-{tool}))'
+return rf'^[ \t]*/\* +{tag}.*\*/$'
# Returns a data structure containing dictionaries for safe and
false-positive-*
@@ -60,12 +61,11 @@ def load_tag_database(tool, input_files, data_struct =
None, schema = "safe"):
if proprietary_id != "":
comment=tool_syntax[tool].replace("VID",proprietary_id)
# Regex to capture the index of the Xen tag and the schema
-xen_tag = re.compile(get_xen_tag_index_type_regex(tool))\
-.match(entry["id"])
-if xen_tag and xen_tag.group(1) and xen_tag.group(2):
+xen_tag = re.match(get_xen_tag_index_type_regex(tool), entry["id"])
+if xen_tag and xen_tag.group('id') and xen_tag.group('type'):
# Save in safe or false-positive-* the key {#id: "comment"}
-id_number = int(xen_tag.group(1))
-key = xen_tag.group(2)
+id_number = int(xen_tag.group('id'))
+key = xen_tag.group('type')
ret[key][id_number] = "/* {} */\n".format(comment)
else:
raise TagDatabaseError(
@@ -95,11 +95,11 @@ def substitute_tags(tool, input_file, grep_struct,
subs_rules):
# information access the subs_rules dictionary to see if there is
# a match
for line_number in grep_struct["matches"]:
-xen_tag = grep_struct["matches"][line_number][0]
-xen_tag_regex_obj = re.compile(
-get_xen_tag_index_type_regex(tool)).match(xen_tag)
-id_number = int(xen_tag_regex_obj.group(1))
-key = xen_tag_regex_obj.group(2)
+xen_tag = grep_struct["matches"][line_number]['tag']
+xen_tag_regex_obj =
re.match(get_xen_tag_index_type_regex(tool),
+ xen_tag)
+id_number = int(xen_tag_regex_obj.group('id'))
+key = xen_tag_regex_obj.group('type')
if id_number in subs_rules[key]:
parsed_content[line_number-1] = subs_rules[key][id_number]
diff --git a/xen/scripts/xen_analysis/utils.py
b/xen/scripts/xen_analysis/utils.py
index 1193e3f4631e..eef48eeb7e87 100644
--- a/xen/scripts/xen_analysis/utils.py
+++ b/xen/scripts/xen_analysis/utils.py
@@ -12,7 +12,7 @@ def grep(filepath, regex):
for line in f:
match = regObj.match(line)
if match:
-res["matches"][line_number] =
[PATCH 0/3] xen-analysis.py: Enable Xen deviation tag at the end of the line
This serie is enabling the xen-analysis tool to parse and substitute correctly
a deviation tag put at the end of the line containing a deviation to be
deviated.
Before this serie the only way to deviate a violation was to put the tag in the
line above:
/* SAF--safe deviate the bla bla bla */
But there are places in the code base where using the tag in the line above is
not convinient, for example:
if ( (expression) &&
((expression with violation) ||
(expression) )
{
[...]
}
In the above example is better to have the suppression comment at the end of the
line:
if ( (expression) &&
((expression with violation) || /* SAF--safe deviate the bla bla bla */
(expression) )
{
[...]
}
This clearly brings up the question about the code style line length, which in
this case needs to be amended for Xen deviation tags that goes above the limit.
Luca Fancellu (3):
xen-analysis.py: Use named group for tag regex
xen-analysis.py: Substitute only the comment instead of the line
xen-analysis.py: Accept deviation comment at the end of the line
docs/misra/documenting-violations.rst | 10
xen/scripts/xen_analysis/cppcheck_analysis.py | 4 +-
xen/scripts/xen_analysis/generic_analysis.py | 14 +++--
xen/scripts/xen_analysis/tag_database.py | 59 ++-
xen/scripts/xen_analysis/utils.py | 2 +-
5 files changed, 67 insertions(+), 22 deletions(-)
--
2.34.1
[PATCH 2/3] xen-analysis.py: Substitute only the comment instead of the line
Change the code in tag_database.py to substitute only the SAF-*
comment instead of replacing the line.
Signed-off-by: Luca Fancellu
---
xen/scripts/xen_analysis/tag_database.py | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/xen/scripts/xen_analysis/tag_database.py
b/xen/scripts/xen_analysis/tag_database.py
index abba163aec71..dc0558a46ed6 100644
--- a/xen/scripts/xen_analysis/tag_database.py
+++ b/xen/scripts/xen_analysis/tag_database.py
@@ -16,14 +16,18 @@ tool_syntax = {
"eclair":"-E> hide VID 1 \"\""
}
+def get_xen_tag_regex(tool):
+return rf'(?PSAF-(?P\d+)-(?Psafe|false-positive-{tool}))'
+
def get_xen_tag_index_type_regex(tool):
-return rf'^SAF-(?P\d+)-(?Psafe|false-positive-{tool})$'
+return rf'^{get_xen_tag_regex(tool)}$'
def get_xen_tag_comment_regex(tool):
-tag=rf'(?PSAF-\d+-(?:safe|false-positive-{tool}))'
-return rf'^[ \t]*/\* +{tag}.*\*/$'
+before_comment = r'(?P[ \t]*)'
+comment = rf'(?P/\* +{get_xen_tag_regex(tool)}.*\*/)'
+return rf'^(?P{before_comment}{comment})$'
# Returns a data structure containing dictionaries for safe and
false-positive-*
@@ -66,7 +70,7 @@ def load_tag_database(tool, input_files, data_struct = None,
schema = "safe"):
# Save in safe or false-positive-* the key {#id: "comment"}
id_number = int(xen_tag.group('id'))
key = xen_tag.group('type')
-ret[key][id_number] = "/* {} */\n".format(comment)
+ret[key][id_number] = "/* {} */".format(comment)
else:
raise TagDatabaseError(
"Error in database file, entry {} has unexpected "
@@ -101,7 +105,11 @@ def substitute_tags(tool, input_file, grep_struct,
subs_rules):
id_number = int(xen_tag_regex_obj.group('id'))
key = xen_tag_regex_obj.group('type')
if id_number in subs_rules[key]:
-parsed_content[line_number-1] = subs_rules[key][id_number]
+comment_in = grep_struct["matches"][line_number]['comment']
+comment_out = subs_rules[key][id_number]
+parsed_content[line_number-1] = re.sub(
+re.escape(comment_in), comment_out,
+parsed_content[line_number-1])
outfile.writelines(parsed_content)
except Exception as e:
--
2.34.1
[PATCH 3/3] xen-analysis.py: Accept deviation comment at the end of the line
Implement the in-code suppression comment at the end of the line.
Now it is possible to add a Xen deviaiton comment with the syntax
described in the docs at the end of the line affected by the
violation, to deviate it.
Eclair natively supports it, so the translation for the tool will
be straighforward, but the other tool needs to translate an
occurrence of the tag into a suppressino comment at the end of the
previous line, this will have a corner case where the line number
1 of the file can't be deviated in this way for such tools.
Updated documentation.
Signed-off-by: Luca Fancellu
---
docs/misra/documenting-violations.rst | 10
xen/scripts/xen_analysis/cppcheck_analysis.py | 2 +-
xen/scripts/xen_analysis/generic_analysis.py | 14 +++
xen/scripts/xen_analysis/tag_database.py | 25 ++-
4 files changed, 44 insertions(+), 7 deletions(-)
diff --git a/docs/misra/documenting-violations.rst
b/docs/misra/documenting-violations.rst
index 0d02a8e6f905..8f1cbd83b859 100644
--- a/docs/misra/documenting-violations.rst
+++ b/docs/misra/documenting-violations.rst
@@ -210,3 +210,13 @@ will be an entry like the following and the violation id
will be in the column
Given the violation id "misra-c2012-20.7", the procedure above can be followed
to justify this finding.
+
+Another way to justify the above violation is to put the in-code comment tag
+at the end of the affected line::
+
+| extern char _start[], _end[], start[]; /* SAF-1-safe [...] */
+
+This way of deviating violations needs however to be used only when placing the
+tag above the line can't be done. This option suffers from some limitation on
+cppcheck and coverity tool that don't support natively the suppression comment
+at the end of the line.
diff --git a/xen/scripts/xen_analysis/cppcheck_analysis.py
b/xen/scripts/xen_analysis/cppcheck_analysis.py
index 919eb153ce9c..850335c998a3 100644
--- a/xen/scripts/xen_analysis/cppcheck_analysis.py
+++ b/xen/scripts/xen_analysis/cppcheck_analysis.py
@@ -41,7 +41,7 @@ def __generate_suppression_list(out_file):
# The following lambda function will return a file if it contains lines
with
# a comment containing "cppcheck-suppress[*]" on a single line.
grep_action = lambda x: utils.grep(x,
-r'^[ \t]*/\* cppcheck-suppress\[(?P.*)\] \*/$')
+r'^.*/\* cppcheck-suppress\[(?P.*)\] \*/$')
# Look for a list of .h files that matches the condition above
headers = utils.recursive_find_file(settings.xen_dir, r'.*\.h$',
grep_action)
diff --git a/xen/scripts/xen_analysis/generic_analysis.py
b/xen/scripts/xen_analysis/generic_analysis.py
index 94122aebace0..9e0cfc1bbc08 100644
--- a/xen/scripts/xen_analysis/generic_analysis.py
+++ b/xen/scripts/xen_analysis/generic_analysis.py
@@ -52,11 +52,15 @@ def parse_xen_tags():
os.rename(file, bkp_file)
time_bkp_file = os.stat(bkp_file)
# Create from .safparse but with the Xen tag parsed
-tag_database.substitute_tags(settings.analysis_tool, bkp_file, entry,
- subs_list)
-# Set timestamp for file equal to bkp_file, so that if the file is
-# modified during the process by the user, we can catch it
-os.utime(file, (time_bkp_file.st_atime, time_bkp_file.st_mtime))
+try:
+tag_database.substitute_tags(settings.analysis_tool, bkp_file,
entry,
+ subs_list)
+except Exception as e:
+raise ParseTagPhaseError("{}".format(e))
+finally:
+# Set timestamp for file equal to bkp_file, so that if the file is
+# modified during the process by the user, we can catch it
+os.utime(file, (time_bkp_file.st_atime, time_bkp_file.st_mtime))
def build_xen():
diff --git a/xen/scripts/xen_analysis/tag_database.py
b/xen/scripts/xen_analysis/tag_database.py
index dc0558a46ed6..57746ca0ddb6 100644
--- a/xen/scripts/xen_analysis/tag_database.py
+++ b/xen/scripts/xen_analysis/tag_database.py
@@ -25,7 +25,7 @@ def get_xen_tag_index_type_regex(tool):
def get_xen_tag_comment_regex(tool):
-before_comment = r'(?P[ \t]*)'
+before_comment = r'(?P.*)'
comment = rf'(?P/\* +{get_xen_tag_regex(tool)}.*\*/)'
return rf'^(?P{before_comment}{comment})$'
@@ -106,7 +106,30 @@ def substitute_tags(tool, input_file, grep_struct,
subs_rules):
key = xen_tag_regex_obj.group('type')
if id_number in subs_rules[key]:
comment_in = grep_struct["matches"][line_number]['comment']
+before = grep_struct["matches"][line_number]['before']
comment_out = subs_rules[key][id_number]
+if before != '' and not re.match(r'^[ \t]+$', before):
+# The comment is at the end of some line with some code
+
Re: [PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05
On 31/01/2024 10:44 am, Christian Lindig wrote: >> On 31 Jan 2024, at 10:42, Edwin Török wrote: >> >> Fix building oxenstored with OCaml 5.x. >> OCaml 5.x has removed some functions that have been deprecated for many >> years, >> in order to support OCaml 5.x we need to drop support for OCaml 4.02. >> >> Tested in gitlab CI (together with my other series): >> https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827 >> >> Edwin Török (2): >> oxenstored: fix build on OCaml 5.x >> tools/ocaml: bump minimum version to OCaml 4.05 >> >> tools/configure | 2 +- >> tools/configure.ac| 2 +- >> tools/ocaml/xenstored/disk.ml | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> -- >> 2.43.0 >> > Acked-by: Christian Lindig It occurs to me that this is the kind of thing which should get a CHANGELOG.md entry these days. Something like: diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f55c9c72d10..fd7c8f5c6b82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Changed - Changed flexible array definitions in public I/O interface headers to not use "1" as the number of array elements. + - The minimum supported Ocaml toolchain version is now 4.05 - On x86: - HVM PIRQs are disabled by default. - Reduce IOMMU setup time for hardware domain. ought to do. Have we checked to see whether this drops Ocaml from any of the build containers ? ~Andrew
[PATCH v1 1/2] oxenstored: use Map instead of Hashtbl for quotas
On a stress test running 1000 VMs flamegraphs have shown that
`oxenstored` spends a large amount of time in `Hashtbl.copy` and the GC.
Hashtable complexity:
* read/write: O(1) average
* copy: O(domains) -- copying the entire table
Map complexity:
* read/write: O(log n) worst case
* copy: O(1) -- a word copy
We always perform at least one 'copy' when processing each xenstore
packet (regardless whether it is a readonly operation or inside a
transaction or not), so the actual complexity per packet is:
* Hashtbl: O(domains)
* Map: O(log domains)
Maps are the clear winner, and a better fit for the immutable xenstore
tree.
Signed-off-by: Edwin Török
---
tools/ocaml/xenstored/quota.ml | 65 ++
1 file changed, 34 insertions(+), 31 deletions(-)
diff --git a/tools/ocaml/xenstored/quota.ml b/tools/ocaml/xenstored/quota.ml
index 300d78a50b..f6e28ecc6a 100644
--- a/tools/ocaml/xenstored/quota.ml
+++ b/tools/ocaml/xenstored/quota.ml
@@ -23,66 +23,69 @@ let activate = ref true
let maxent = ref (1000)
let maxsize = ref (2048)
+module Domid = struct
+ type t = Xenctrl.domid
+ let compare (a:t) (b:t) = compare a b
+end
+
+module DomidMap = Map.Make(Domid)
+
type t = {
maxent: int; (* max entities per domU *)
maxsize: int; (* max size of data store in one node *)
- cur: (Xenctrl.domid, int) Hashtbl.t; (* current domains quota *)
+ mutable cur: int DomidMap.t; (* current domains quota *)
}
let to_string quota domid =
- if Hashtbl.mem quota.cur domid
- then Printf.sprintf "dom%i quota: %i/%i" domid (Hashtbl.find quota.cur
domid) quota.maxent
- else Printf.sprintf "dom%i quota: not set" domid
+ try
+Printf.sprintf "dom%i quota: %i/%i" domid (DomidMap.find domid quota.cur)
quota.maxent
+ with Not_found ->
+Printf.sprintf "dom%i quota: not set" domid
let create () =
- { maxent = !maxent; maxsize = !maxsize; cur = Hashtbl.create 100; }
+ { maxent = !maxent; maxsize = !maxsize; cur = DomidMap.empty; }
-let copy quota = { quota with cur = (Hashtbl.copy quota.cur) }
+let copy quota = { quota with cur = quota.cur }
-let del quota id = Hashtbl.remove quota.cur id
+let del quota id = { quota with cur = DomidMap.remove id quota.cur }
let _check quota id size =
if size > quota.maxsize then (
warn "domain %u err create entry: data too big %d" id size;
raise Data_too_big
);
- if id > 0 && Hashtbl.mem quota.cur id then
-let entry = Hashtbl.find quota.cur id in
+ if id > 0 then
+ try
+let entry = DomidMap.find id quota.cur in
if entry >= quota.maxent then (
warn "domain %u cannot create entry: quota reached" id;
raise Limit_reached
)
+ with Not_found -> ()
let check quota id size =
if !activate then
_check quota id size
-let get_entry quota id = Hashtbl.find quota.cur id
+let find_or_zero quota_cur id =
+ try DomidMap.find id quota_cur with Not_found -> 0
-let set_entry quota id nb =
- if nb = 0
- then Hashtbl.remove quota.cur id
- else begin
-if Hashtbl.mem quota.cur id then
- Hashtbl.replace quota.cur id nb
-else
- Hashtbl.add quota.cur id nb
- end
+let update_entry quota_cur id diff =
+ let nb = diff + find_or_zero quota_cur id in
+ if nb = 0 then DomidMap.remove id quota_cur
+ else DomidMap.add id nb quota_cur
let del_entry quota id =
- try
-let nb = get_entry quota id in
-set_entry quota id (nb - 1)
- with Not_found -> ()
+ quota.cur <- update_entry quota.cur id (-1)
let add_entry quota id =
- let nb = try get_entry quota id with Not_found -> 0 in
- set_entry quota id (nb + 1)
-
-let add quota diff =
- Hashtbl.iter (fun id nb -> set_entry quota id (get_entry quota id + nb))
diff.cur
+ quota.cur <- update_entry quota.cur id (+1)
let merge orig_quota mod_quota dest_quota =
- Hashtbl.iter (fun id nb -> let diff = nb - (try get_entry orig_quota id with
Not_found -> 0) in
- if diff <> 0 then
-set_entry dest_quota id ((try get_entry dest_quota id with Not_found
-> 0) + diff)) mod_quota.cur
+ let fold_merge id nb dest =
+match nb - find_or_zero orig_quota.cur id with
+| 0 -> dest (* not modified *)
+| diff -> update_entry dest id diff (* update with [x=x+diff] *)
+ in
+ dest_quota.cur <- DomidMap.fold fold_merge mod_quota.cur dest_quota.cur
+ (* dest_quota = dest_quota + (mod_quota - orig_quota) *)
--
2.43.0
[PATCH v1 0/2] reduce oxenstored quota processing overhead under load
A recent stress test with 1000 VMs has shown that oxenstored spends ~40% of time in Quota.copy, even when processing read-only xenstore commands. Use an immutable data structure instead. I have tested this in the gitlab CI here: https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827 For convenience the changes in this (and the other series I sent out) are also available as a git repository: https://gitlab.com/xen-project/people/edwintorok/xen/-/compare/private%2Fedvint%2Fdune1x...private%2Fedvint%2Fno-hashtbl-dev?from_project_id=47263871=false I haven't yet measured the speedup, but thought to send out the patch for review early. Edwin Török (2): oxenstored: use Map instead of Hashtbl for quotas oxenstored: make Quota.t pure tools/ocaml/xenstored/quota.ml | 65 ++ tools/ocaml/xenstored/store.ml | 17 + 2 files changed, 44 insertions(+), 38 deletions(-) -- 2.43.0
[PATCH v1 2/2] oxenstored: make Quota.t pure
Now that we no longer have a hashtable inside we can make Quota.t pure,
and push the mutable update to its callers.
Store.t already had a mutable Quota.t field.
No functional change.
Signed-off-by: Edwin Török
---
tools/ocaml/xenstored/quota.ml | 8
tools/ocaml/xenstored/store.ml | 17 ++---
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/tools/ocaml/xenstored/quota.ml b/tools/ocaml/xenstored/quota.ml
index f6e28ecc6a..1f652040d8 100644
--- a/tools/ocaml/xenstored/quota.ml
+++ b/tools/ocaml/xenstored/quota.ml
@@ -33,7 +33,7 @@ module DomidMap = Map.Make(Domid)
type t = {
maxent: int; (* max entities per domU *)
maxsize: int; (* max size of data store in one node *)
- mutable cur: int DomidMap.t; (* current domains quota *)
+ cur: int DomidMap.t; (* current domains quota *)
}
let to_string quota domid =
@@ -76,10 +76,10 @@ let update_entry quota_cur id diff =
else DomidMap.add id nb quota_cur
let del_entry quota id =
- quota.cur <- update_entry quota.cur id (-1)
+ {quota with cur = update_entry quota.cur id (-1)}
let add_entry quota id =
- quota.cur <- update_entry quota.cur id (+1)
+ {quota with cur = update_entry quota.cur id (+1)}
let merge orig_quota mod_quota dest_quota =
let fold_merge id nb dest =
@@ -87,5 +87,5 @@ let merge orig_quota mod_quota dest_quota =
| 0 -> dest (* not modified *)
| diff -> update_entry dest id diff (* update with [x=x+diff] *)
in
- dest_quota.cur <- DomidMap.fold fold_merge mod_quota.cur dest_quota.cur
+ {dest_quota with cur = DomidMap.fold fold_merge mod_quota.cur dest_quota.cur}
(* dest_quota = dest_quota + (mod_quota - orig_quota) *)
diff --git a/tools/ocaml/xenstored/store.ml b/tools/ocaml/xenstored/store.ml
index 38a4945372..9b8dd2812d 100644
--- a/tools/ocaml/xenstored/store.ml
+++ b/tools/ocaml/xenstored/store.ml
@@ -85,7 +85,9 @@ module Node = struct
raise Define.Permission_denied;
end
- let rec recurse fct node = fct node; SymbolMap.iter (fun _ -> recurse fct)
node.children
+ let rec recurse fct node acc =
+let acc = fct node acc in
+SymbolMap.fold (fun _ -> recurse fct) node.children acc
(** [recurse_filter_map f tree] applies [f] on each node in the tree
recursively,
possibly removing some nodes.
@@ -408,7 +410,7 @@ let dump_buffer store = dump_store_buf store.root
let set_node store path node orig_quota mod_quota =
let root = Path.set_node store.root path node in
store.root <- root;
- Quota.merge orig_quota mod_quota store.quota
+ store.quota <- Quota.merge orig_quota mod_quota store.quota
let write store perm path value =
let node, existing = get_deepest_existing_node store path in
@@ -422,7 +424,7 @@ let write store perm path value =
let root, node_created = path_write store perm path value in
store.root <- root;
if node_created
- then Quota.add_entry store.quota owner
+ then store.quota <- Quota.add_entry store.quota owner
let mkdir store perm path =
let node, existing = get_deepest_existing_node store path in
@@ -431,7 +433,7 @@ let mkdir store perm path =
if not (existing || (Perms.Connection.is_dom0 perm)) then Quota.check
store.quota owner 0;
store.root <- path_mkdir store perm path;
if not existing then
-Quota.add_entry store.quota owner
+store.quota <- Quota.add_entry store.quota owner
let rm store perm path =
let rmed_node = Path.get_node store.root path in
@@ -439,7 +441,7 @@ let rm store perm path =
| None -> raise Define.Doesnt_exist
| Some rmed_node ->
store.root <- path_rm store perm path;
-Node.recurse (fun node -> Quota.del_entry store.quota (Node.get_owner
node)) rmed_node
+store.quota <- Node.recurse (fun node quota -> Quota.del_entry quota
(Node.get_owner node)) rmed_node store.quota
let setperms store perm path nperms =
match Path.get_node store.root path with
@@ -450,8 +452,9 @@ let setperms store perm path nperms =
if not ((old_owner = new_owner) || (Perms.Connection.is_dom0 perm)) then
raise Define.Permission_denied;
store.root <- path_setperms store perm path nperms;
-Quota.del_entry store.quota old_owner;
-Quota.add_entry store.quota new_owner
+store.quota <-
+ let quota = Quota.del_entry store.quota old_owner in
+ Quota.add_entry quota new_owner
let reset_permissions store domid =
Logging.info "store|node" "Cleaning up xenstore ACLs for domid %d" domid;
--
2.43.0
Re: [PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05
> On 31 Jan 2024, at 10:42, Edwin Török wrote: > > Fix building oxenstored with OCaml 5.x. > OCaml 5.x has removed some functions that have been deprecated for many years, > in order to support OCaml 5.x we need to drop support for OCaml 4.02. > > Tested in gitlab CI (together with my other series): > https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827 > > Edwin Török (2): > oxenstored: fix build on OCaml 5.x > tools/ocaml: bump minimum version to OCaml 4.05 > > tools/configure | 2 +- > tools/configure.ac| 2 +- > tools/ocaml/xenstored/disk.ml | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > -- > 2.43.0 > Acked-by: Christian Lindig
[PATCH v1 1/2] oxenstored: fix build on OCaml 5.x
Char.lowercase got removed in OCaml 5.0 (it has been deprecated since 2014). Char.lowercase_ascii has existed since OCaml 4.03, so that is the new minimum version for oxenstored. (Given the choice between supporting a new release and dropping support for an 8y+ old release, we drop support for OCaml <4.03) Signed-off-by: Edwin Török --- tools/configure | 2 +- tools/configure.ac| 2 +- tools/ocaml/xenstored/disk.ml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/configure b/tools/configure index 0135a0059a..5723efaa56 100755 --- a/tools/configure +++ b/tools/configure @@ -6836,7 +6836,7 @@ else -e 's/[^0-9]//g'` - ax_compare_version_B=`echo "4.02.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \ + ax_compare_version_B=`echo "4.03.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \ -e 's/Z\([0-9]\)Z/Z0\1Z/g' \ -e 's/Z\([0-9][0-9]\)Z/Z0\1Z/g' \ -e 's/Z\([0-9][0-9][0-9]\)Z/Z0\1Z/g' \ diff --git a/tools/configure.ac b/tools/configure.ac index 618ef8c63f..c979c3de7c 100644 --- a/tools/configure.ac +++ b/tools/configure.ac @@ -336,7 +336,7 @@ AS_IF([test "x$ocamltools" = "xy"], [ AC_MSG_ERROR([Ocaml tools enabled, but missing ocamlopt or ocamlfind])]) ocamltools="n" ], [ -AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.02.0], [ +AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.03.0], [ AS_IF([test "x$enable_ocamltools" = "xyes"], [ AC_MSG_ERROR([Your version of OCaml: $OCAMLVERSION is not supported])]) ocamltools="n" diff --git a/tools/ocaml/xenstored/disk.ml b/tools/ocaml/xenstored/disk.ml index 91f945f2bd..ccaa048faf 100644 --- a/tools/ocaml/xenstored/disk.ml +++ b/tools/ocaml/xenstored/disk.ml @@ -30,7 +30,7 @@ let undec c = | _ -> raise (Failure "undecify") let unhex c = - let c = Char.lowercase c in + let c = Char.lowercase_ascii c in match c with | '0' .. '9' -> (Char.code c) - (Char.code '0') | 'a' .. 'f' -> (Char.code c) - (Char.code 'a') + 10 -- 2.43.0
[PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05
Fix building oxenstored with OCaml 5.x. OCaml 5.x has removed some functions that have been deprecated for many years, in order to support OCaml 5.x we need to drop support for OCaml 4.02. Tested in gitlab CI (together with my other series): https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827 Edwin Török (2): oxenstored: fix build on OCaml 5.x tools/ocaml: bump minimum version to OCaml 4.05 tools/configure | 2 +- tools/configure.ac| 2 +- tools/ocaml/xenstored/disk.ml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) -- 2.43.0
[PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05
We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us back. So bump to OCaml 4.05 instead, which should match the version on OSSTest? [1]: https://patchwork.kernel.org/project/xen-devel/patch/ac885ce2b63159d26d857dc3e53cf8aa63ae3646.1659118200.git.edvin.to...@citrix.com/ Signed-off-by: Edwin Török --- tools/configure| 2 +- tools/configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/configure b/tools/configure index 5723efaa56..3d557234b3 100755 --- a/tools/configure +++ b/tools/configure @@ -6836,7 +6836,7 @@ else -e 's/[^0-9]//g'` - ax_compare_version_B=`echo "4.03.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \ + ax_compare_version_B=`echo "4.05.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \ -e 's/Z\([0-9]\)Z/Z0\1Z/g' \ -e 's/Z\([0-9][0-9]\)Z/Z0\1Z/g' \ -e 's/Z\([0-9][0-9][0-9]\)Z/Z0\1Z/g' \ diff --git a/tools/configure.ac b/tools/configure.ac index c979c3de7c..851887080c 100644 --- a/tools/configure.ac +++ b/tools/configure.ac @@ -336,7 +336,7 @@ AS_IF([test "x$ocamltools" = "xy"], [ AC_MSG_ERROR([Ocaml tools enabled, but missing ocamlopt or ocamlfind])]) ocamltools="n" ], [ -AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.03.0], [ +AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.05.0], [ AS_IF([test "x$enable_ocamltools" = "xyes"], [ AC_MSG_ERROR([Your version of OCaml: $OCAMLVERSION is not supported])]) ocamltools="n" -- 2.43.0
Re: [PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
On 31/01/2024 9:20 am, Jan Beulich wrote: > Allow the compiler a little more room on DCE by moving the compile-time- > constant condition into the predicate (from the one place where it was > added in an open-coded fashion for XSA-450). > > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper I'm surprised that's happy compiling if it's actually doing anything at all, given that it's an unguarded variable reference.
[xen-4.17-testing test] 184531: tolerable FAIL - PUSHED
flight 184531 xen-4.17-testing real [real] flight 184539 xen-4.17-testing real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/184531/ http://logs.test-lab.xenproject.org/osstest/logs/184539/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-shadow 20 guest-localmigrate/x10 fail pass in 184539-retest Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 184165 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 184165 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 184165 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 184165 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 184165 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 184165 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 184165 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 184165 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 184165 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 184165 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 184165 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 184165 test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-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-xsm 15 migrate-support-checkfail never pass test-amd64-i386-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-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-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-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-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen 6b1864afc14d484cdbc9754ce3172ac3dc189846 baseline version: xen 949a4aad417621638231e4cf9f1b35e8e0328463 Last test of basis 184165 2023-12-18
[PATCH] IOMMU: iommu_use_hap_pt() implies CONFIG_HVM
Allow the compiler a little more room on DCE by moving the compile-time-
constant condition into the predicate (from the one place where it was
added in an open-coded fashion for XSA-450).
Signed-off-by: Jan Beulich
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -438,7 +438,7 @@ static paddr_t domain_pgd_maddr(struct d
if ( pgd_maddr )
/* nothing */;
-else if ( IS_ENABLED(CONFIG_HVM) && iommu_use_hap_pt(d) )
+else if ( iommu_use_hap_pt(d) )
{
pagetable_t pgt = p2m_get_pagetable(p2m_get_hostp2m(d));
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -381,7 +381,8 @@ struct domain_iommu {
#define iommu_clear_feature(d, f) clear_bit(f, dom_iommu(d)->features)
/* Are we using the domain P2M table as its IOMMU pagetable? */
-#define iommu_use_hap_pt(d) (dom_iommu(d)->hap_pt_share)
+#define iommu_use_hap_pt(d) (IS_ENABLED(CONFIG_HVM) && \
+ dom_iommu(d)->hap_pt_share)
/* Does the IOMMU pagetable need to be kept synchronized with the P2M */
#ifdef CONFIG_HAS_PASSTHROUGH
Re: [PATCH] xen/*/asm-offset: Fix bad copy from x86
Hi Andrew, On 30/01/2024 22:28, Andrew Cooper wrote: All architectures have copy bad logic from x86. OFFSET() having a trailing semi-colon within the macro expansion can be a problematic pattern. It's benign in this case, but fix it anyway. Perform style fixes for the other macros, and tame the mess of BLANK() position to be consistent (one BLANK() after each block) so the intermediate form is legible too. No functional change. Signed-off-by: Andrew Cooper With Jan's comments addressed on the Arm side as well: Acked-by: Julien Grall Cheers, -- Julien Grall
Re: [PATCH] xen/sched: Fix UB shift in compat_set_timer_op()
On 30.01.2024 22:27, Andrew Cooper wrote:
> Tamas reported this UBSAN failure from fuzzing:
>
> (XEN)
>
> (XEN) UBSAN: Undefined behaviour in common/sched/compat.c:48:37
> (XEN) left shift of negative value -2147425536
> (XEN) [ Xen-4.19-unstable x86_64 debug=y ubsan=y Not tainted ]
> ...
> (XEN) Xen call trace:
> (XEN)[] R ubsan.c#ubsan_epilogue+0xa/0xd9
> (XEN)[] F
> __ubsan_handle_shift_out_of_bounds+0x11a/0x1c5
> (XEN)[] F compat_set_timer_op+0x41/0x43
> (XEN)[] F hvm_do_multicall_call+0x77f/0xa75
> (XEN)[] F arch_do_multicall_call+0xec/0xf1
> (XEN)[] F do_multicall+0x1dc/0xde3
> (XEN)[] F hvm_hypercall+0xa00/0x149a
> (XEN)[] F vmx_vmexit_handler+0x1596/0x279c
> (XEN)[] F vmx_asm_vmexit_handler+0xdb/0x200
>
> Left-shifting any negative value is strictly undefined behaviour in C, and
> the two parameters here come straight from the guest.
>
> The fuzzer happened to choose lo 0xf, hi 0x8000e300.
>
> Switch everything to be unsigned values, making the shift well defined.
>
> As GCC documents:
>
> As an extension to the C language, GCC does not use the latitude given in
> C99 and C11 only to treat certain aspects of signed '<<' as undefined.
> However, -fsanitize=shift (and -fsanitize=undefined) will diagnose such
> cases.
>
> this was deemed not to need an XSA.
>
> Fixes: 2942f45e09fb ("Enable compatibility mode operation for
> HYPERVISOR_sched_op and HYPERVISOR_set_timer_op.")
> Reported-by: Tamas K Lengyel
> Signed-off-by: Andrew Cooper
Reviewed-by: Jan Beulich
with the additional observation that the subsequent unsigned->signed conversion
then exercises implementation defined behavior, i.e. is also fine given what
gcc doc states in this regard. Not sure whether that's worth mentioning, seeing
that we have such conversions all over the place (albeit I think it would be
nice if we could reduce their amount some).
Jan
[ovmf test] 184538: all pass - PUSHED
flight 184538 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/184538/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf af6e0e728f652f496a6fb1e659617c9662f774ac baseline version: ovmf 909a9a5ae4b8236c1ca7cad7f214c752a579bd67 Last test of basis 184533 2024-01-30 15:12:47 Z0 days Testing same since 184538 2024-01-31 04:36:38 Z0 days1 attempts People who touched revisions under test: Michael D Kinney jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 909a9a5ae4..af6e0e728f af6e0e728f652f496a6fb1e659617c9662f774ac -> xen-tested-master
Re: [PATCH] x86/hvm: Fix UBSAN failure in do_hvm_op() printk
On 30.01.2024 19:37, Andrew Cooper wrote: > Tamas reported this UBSAN failure from fuzzing: > > (XEN) > > (XEN) UBSAN: Undefined behaviour in common/vsprintf.c:64:19 > (XEN) negation of -9223372036854775808 cannot be represented in type 'long > long int': > (XEN) [ Xen-4.19-unstable x86_64 debug=y ubsan=y Not tainted ] > ... > (XEN) Xen call trace: > (XEN)[] R ubsan.c#ubsan_epilogue+0xa/0xd9 > (XEN)[] F __ubsan_handle_negate_overflow+0x99/0xce > (XEN)[] F vsprintf.c#number+0x10a/0x93e > (XEN)[] F vsnprintf+0x19e2/0x1c56 > (XEN)[] F console.c#vprintk_common+0x76/0x34d > (XEN)[] F printk+0x4d/0x4f > (XEN)[] F do_hvm_op+0x288e/0x28f5 > (XEN)[] F hvm_hypercall+0xad2/0x149a > (XEN)[] F vmx_vmexit_handler+0x1596/0x279c > (XEN)[] F vmx_asm_vmexit_handler+0xdb/0x200 > > The problem is an unsigned -> signed converstion because of a bad > formatter (%ld trying to format an unsigned long). > > We could fix it by swapping to %lu, but this is a useless printk() even in > debug builds, so just drop it completely. > > Reported-by: Tamas K Lengyel > Signed-off-by: Andrew Cooper Acked-by: Jan Beulich
Re: [PATCH] xen/*/asm-offset: Fix bad copy from x86
On 30.01.2024 23:28, Andrew Cooper wrote:
> --- a/xen/arch/x86/x86_64/asm-offsets.c
> +++ b/xen/arch/x86/x86_64/asm-offsets.c
> @@ -18,11 +18,11 @@
>
> #define DEFINE(_sym, _val) \
> asm volatile ("\n.ascii\"==>#define " #_sym " %0 /* " #_val " */<==\"" \
> - : : "i" (_val) )
> + :: "i" (_val))
The removal of the last blank is against our style; instead a blank wants
insertion after the opening parenthesis.
> #define BLANK()\
> -asm volatile ( "\n.ascii\"==><==\"" : : )
> +asm volatile ("\n.ascii\"==><==\"")
Similarly here while dropping the colons is fine, the blanks next to the
parentheses want keeping.
With that adjusted throughout
Reviewed-by: Jan Beulich
Jan
Re: [PATCH] xen: Drop superfluous semi-colons
On 30.01.2024 23:18, Andrew Cooper wrote: > All these cases happen to be benign, but drop them anyway. This is one step > towards making -Wextra-semi work. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich
Re: [XEN PATCH v2 0/3] Introduce and use STATIC_ASSERT_UNREACHABLE()
On 31.01.2024 09:19, Federico Serafini wrote:
> On 26/01/24 11:05, Federico Serafini wrote:
>> Introduce macro STATIC_ASSERT_UNREACHABLE(),
>> use it to replace __{get,put}_user_bad(),
>> update ECLAIR configuration to allow the use of such macro at the end of
>> switch-caluses.
>>
>> Federico Serafini (3):
>>xen: introduce STATIC_ASSERT_UNREACHABLE()
>>x86/uaccess: replace __{get,put}_user_bad() with
>> STATIC_ASSERT_UNREACHABLE()
>>automation/eclair: add deviation for MISRA C:2012 Rule 16.3
>>
>> automation/eclair_analysis/ECLAIR/deviations.ecl | 4
>> docs/misra/deviations.rst| 5 +
>> xen/arch/x86/include/asm/uaccess.h | 7 ++-
>> xen/include/xen/compiler.h | 7 +++
>> 4 files changed, 18 insertions(+), 5 deletions(-)
>>
>
> Hello everyone,
>
> do you have any comments on this series?
Yes, but in due course.
Jan
Re: [RFC KERNEL PATCH v4 3/3] PCI/sysfs: Add gsi sysfs for pci_dev
On Tue, Jan 30, 2024 at 02:44:03PM -0600, Bjorn Helgaas wrote: > On Tue, Jan 30, 2024 at 10:07:36AM +0100, Roger Pau Monné wrote: > > On Mon, Jan 29, 2024 at 04:01:13PM -0600, Bjorn Helgaas wrote: > > > On Thu, Jan 25, 2024 at 07:17:24AM +, Chen, Jiqian wrote: > > > > On 2024/1/24 00:02, Bjorn Helgaas wrote: > > > > > On Tue, Jan 23, 2024 at 10:13:52AM +, Chen, Jiqian wrote: > > > > >> On 2024/1/23 07:37, Bjorn Helgaas wrote: > > > > >>> On Fri, Jan 05, 2024 at 02:22:17PM +0800, Jiqian Chen wrote: > > > > There is a need for some scenarios to use gsi sysfs. > > > > For example, when xen passthrough a device to dumU, it will > > > > use gsi to map pirq, but currently userspace can't get gsi > > > > number. > > > > So, add gsi sysfs for that and for other potential scenarios. > > > > >> ... > > > > > > > > > >>> I don't know enough about Xen to know why it needs the GSI in > > > > >>> userspace. Is this passthrough brand new functionality that can't > > > > >>> be > > > > >>> done today because we don't expose the GSI yet? > > > > > > I assume this must be new functionality, i.e., this kind of > > > passthrough does not work today, right? > > > > > > > >> has ACPI support and is responsible for detecting and controlling > > > > >> the hardware, also it performs privileged operations such as the > > > > >> creation of normal (unprivileged) domains DomUs. When we give to a > > > > >> DomU direct access to a device, we need also to route the physical > > > > >> interrupts to the DomU. In order to do so Xen needs to setup and map > > > > >> the interrupts appropriately. > > > > > > > > > > What kernel interfaces are used for this setup and mapping? > > > > > > > > For passthrough devices, the setup and mapping of routing physical > > > > interrupts to DomU are done on Xen hypervisor side, hypervisor only > > > > need userspace to provide the GSI info, see Xen code: > > > > xc_physdev_map_pirq require GSI and then will call hypercall to pass > > > > GSI into hypervisor and then hypervisor will do the mapping and > > > > routing, kernel doesn't do the setup and mapping. > > > > > > So we have to expose the GSI to userspace not because userspace itself > > > uses it, but so userspace can turn around and pass it back into the > > > kernel? > > > > No, the point is to pass it back to Xen, which doesn't know the > > mapping between GSIs and PCI devices because it can't execute the ACPI > > AML resource methods that provide such information. > > > > The (Linux) kernel is just a proxy that forwards the hypercalls from > > user-space tools into Xen. > > But I guess Xen knows how to interpret a GSI even though it doesn't > have access to AML? On x86 Xen does know how to map a GSI into an IO-APIC pin, in order configure the RTE as requested. > > > It seems like it would be better for userspace to pass an identifier > > > of the PCI device itself back into the hypervisor. Then the interface > > > could be generic and potentially work even on non-ACPI systems where > > > the GSI concept doesn't apply. > > > > We would still need a way to pass the GSI to PCI device relation to > > the hypervisor, and then cache such data in the hypervisor. > > > > I don't think we have any preference of where such information should > > be exposed, but given GSIs are an ACPI concept not specific to Xen > > they should be exposed by a non-Xen specific interface. > > AFAIK Linux doesn't expose GSIs directly to userspace yet. The GSI > concept relies on ACPI MADT, _MAT, _PRT, etc. A GSI is associated > with some device (PCI in this case) and some interrupt controller > entry. I don't understand how a GSI value is useful without knowing > something about that framework in which GSIs exist. I wouldn't say it's strictly associated with PCI. A GSI is a way for ACPI to have a single space that unifies all possible IO-APICs pins in the system in a flat way. A GSI is useful in itself because there's a single GSI space for the whole host. > Obviously I know less than nothing about Xen, so I apologize for > asking all these stupid questions, but it just doesn't all make sense > to me yet. That's all fine, maybe there's a better path or way to expose this ACPI information. Maybe introduce a per-device acpi directory and expose it there? Or rename the entry to acpi_gsi? Thanks, Roger.
Re: [XEN PATCH v2 0/3] Introduce and use STATIC_ASSERT_UNREACHABLE()
On 26/01/24 11:05, Federico Serafini wrote:
Introduce macro STATIC_ASSERT_UNREACHABLE(),
use it to replace __{get,put}_user_bad(),
update ECLAIR configuration to allow the use of such macro at the end of
switch-caluses.
Federico Serafini (3):
xen: introduce STATIC_ASSERT_UNREACHABLE()
x86/uaccess: replace __{get,put}_user_bad() with
STATIC_ASSERT_UNREACHABLE()
automation/eclair: add deviation for MISRA C:2012 Rule 16.3
automation/eclair_analysis/ECLAIR/deviations.ecl | 4
docs/misra/deviations.rst| 5 +
xen/arch/x86/include/asm/uaccess.h | 7 ++-
xen/include/xen/compiler.h | 7 +++
4 files changed, 18 insertions(+), 5 deletions(-)
Hello everyone,
do you have any comments on this series?
--
Federico Serafini, M.Sc.
Software Engineer, BUGSENG (http://bugseng.com)
