Re: [adhoc test] 179924: trouble: blocked/broken/fail/pass
On 24.03.23 20:13, aper...@xenbits.xen.org wrote: [adhoc play] harness ed1d8de4: PDU/IPMI: Be less aggressive with IPMI commands 179924: trouble: blocked/broken/fail/pass flight 179924 linux-linus play [play] http://logs.test-lab.xenproject.org/osstest/logs/179924/ Seems the main problem is gone now. The patch has another issue, but this can be fixed easily (the per skb copy_count needs not to be incremented when the copy is being split, as otherwise the pending_idx array might be overrun). Anthony, thanks for setting up the test runs! Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
[qemu-mainline test] 179915: regressions - trouble: fail/pass/starved
flight 179915 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/179915/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qcow212 debian-di-installfail REGR. vs. 179518 test-amd64-amd64-libvirt-vhd 12 debian-di-installfail REGR. vs. 179518 test-amd64-i386-xl-vhd 12 debian-di-installfail REGR. vs. 179518 test-amd64-i386-libvirt-raw 12 debian-di-installfail REGR. vs. 179518 test-amd64-i386-libvirt 14 guest-start fail REGR. vs. 179518 test-amd64-i386-libvirt-pair 25 guest-start/debian fail REGR. vs. 179518 test-amd64-i386-libvirt-xsm 14 guest-start fail REGR. vs. 179518 test-arm64-arm64-xl-vhd 12 debian-di-installfail REGR. vs. 179518 test-arm64-arm64-libvirt-xsm 14 guest-start fail REGR. vs. 179518 test-arm64-arm64-libvirt-raw 12 debian-di-installfail REGR. vs. 179518 test-amd64-amd64-libvirt 14 guest-start fail REGR. vs. 179518 test-amd64-amd64-libvirt-xsm 14 guest-start fail REGR. vs. 179518 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install fail REGR. vs. 179518 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 debian-hvm-install fail REGR. vs. 179518 test-amd64-amd64-libvirt-pair 25 guest-start/debian fail REGR. vs. 179518 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 179518 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179518 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179518 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179518 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179518 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-arm64-arm64-xl-credit2 3 hosts-allocate starved n/a test-arm64-arm64-xl 3 hosts-allocate starved n/a build-armhf 2 hosts-allocate starved n/a test-arm64-arm64-xl-thunderx 3 hosts-allocate starved n/a test-arm64-arm64-xl-xsm 3 hosts-allocate starved n/a test-arm64-arm64-xl-credit1 3 hosts-allocate starved n/a version targeted for testing: qemuu60ca584b8af0de525656f959991a440f8c191f12 baseline version: qemuu7b0f0aa55fd292fa3489755a3a896e496c51ea86 Last test of basis 179518 2023-03-09 10:37:19 Z 15 days Failing since179526 2023-03-10 01:53:40 Z 15 days 23 attempts Testing same since 179915 2023-03-24 03:04:18 Z0 days1 attempts People who touched revisions under test: Akihiko Odaki Albert Esteve Alex Bennée Alex Williamson Alistair Francis Andreas Schwab Anton Johansson Avihai Horon BALATON Zoltan Bernhard Beschow Bin Meng Carlos López Chen Baozi Cédric Le Goater Cédric Le Goater Damien Hedde Daniel P. Berrangé David Hildenbrand David Woodhouse David Woodhouse Dr. David Alan Gilbert Erico Nunes Eugenio Pérez Fabiano Rosas Fan Ni fanwenjie fa...@mail.ustc.edu.cn Fiona Ebner Gal Hammer Gerd Hoffmann Guenter Roeck Guo Ren Hanna Czenczek Helge Deller Idan Horowitz Igor Mammedov Ilya Leoshkevich Ivan Klokov Jared Rossi Jason A. Donenfeld Jason Wang Jiaxun Yang Joao Martins John Snow Jonathan Cameron Juan Quintela Karthikeyan Pasupathi Kevin Wolf Kfir Manor Konstantin Kostiuk Laurent Vivier Lei Yang Li Zhijian Mads Ynddal Marc-André Lureau Marcel Apfelbaum Marcin Juszkiewicz Marcin Nowakowski Mark Cave-Ayland Markus Armbruster Matheus Tavares Be
[PATCH v2 1/3] x86/msi: passthrough all MSI-X vector ctrl writes to device model
QEMU needs to know whether clearing maskbit of a vector is really
clearing, or was already cleared before. Currently Xen sends only
clearing that bit to the device model, but not setting it, so QEMU
cannot detect it. Because of that, QEMU is working this around by
checking via /dev/mem, but that isn't the proper approach.
Give all necessary information to QEMU by passing all ctrl writes,
including masking a vector. This does include forwarding also writes
that did not change the value, but as tested on both Linux (6.1.12) and
Windows (10 pro), they don't do excessive writes of unchanged values
(Windows seems to clear maskbit in some cases twice, but not more).
Signed-off-by: Marek Marczykowski-Górecki
---
v2:
- passthrough quad writes to emulator too (Jan)
- (ab)use len==0 for write len=4 completion (Jan), but add descriptive
#define for this magic value
This behavior change needs to be surfaced to the device model somehow,
so it knows whether it can rely on it. I'm open for suggestions.
---
xen/arch/x86/hvm/vmsi.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 3cd4923060c8..9c82bf9b4ec2 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -272,6 +272,15 @@ out:
return r;
}
+/*
+ * This function returns X86EMUL_UNHANDLEABLE even if write is properly
+ * handled, to propagate it to the device model (so it can keep its internal
+ * state in sync).
+ * len==0 means really len==4, but as a write completion that will return
+ * X86EMUL_OKAY on successful processing. Use WRITE_LEN4_COMPLETION to make it
+ * less confusing.
+ */
+#define WRITE_LEN4_COMPLETION 0
static int msixtbl_write(struct vcpu *v, unsigned long address,
unsigned int len, unsigned long val)
{
@@ -283,9 +292,6 @@ static int msixtbl_write(struct vcpu *v, unsigned long
address,
unsigned long flags;
struct irq_desc *desc;
-if ( (len != 4 && len != 8) || (address & (len - 1)) )
-return r;
-
rcu_read_lock(&msixtbl_rcu_lock);
entry = msixtbl_find_entry(v, address);
@@ -345,7 +351,7 @@ static int msixtbl_write(struct vcpu *v, unsigned long
address,
unlock:
spin_unlock_irqrestore(&desc->lock, flags);
-if ( len == 4 )
+if ( len == WRITE_LEN4_COMPLETION )
r = X86EMUL_OKAY;
out:
@@ -357,6 +363,9 @@ static int cf_check _msixtbl_write(
const struct hvm_io_handler *handler, uint64_t address, uint32_t len,
uint64_t val)
{
+if ( (len != 4 && len != 8) || (address & (len - 1)) )
+return X86EMUL_UNHANDLEABLE;
+
return msixtbl_write(current, address, len, val);
}
@@ -635,7 +644,7 @@ void msix_write_completion(struct vcpu *v)
return;
v->arch.hvm.hvm_io.msix_unmask_address = 0;
-if ( msixtbl_write(v, ctrl_address, 4, 0) != X86EMUL_OKAY )
+if ( msixtbl_write(v, ctrl_address, WRITE_LEN4_COMPLETION, 0) !=
X86EMUL_OKAY )
gdprintk(XENLOG_WARNING, "MSI-X write completion failure\n");
}
--
2.39.2
[PATCH v2 2/3] x86/hvm: Allow writes to registers on the same page as MSI-X table
Some devices (notably Intel Wifi 6 AX210 card) keep auxiliary registers
on the same page as MSI-X table. Device model (especially one in
stubdomain) cannot really handle those, as direct writes to that page is
refused (page is on the mmio_ro_ranges list). Instead, add internal ioreq
server that handle those writes.
Doing this, requires correlating write location with guest view
of MSI-X table address. Since QEMU doesn't map MSI-X table to the guest,
it requires msixtbl_entry->gtable, which is HVM-only. Similar feature
for PV would need to be done separately.
This can be also used to read Pending Bit Array, if it lives on the same
page, making QEMU not needing /dev/mem access at all (especially helpful
with lockdown enabled in dom0). If PBA lives on another page, QEMU will
map it to the guest directly.
If PBA lives on the same page, forbid writes and log a message.
Technically, writes outside of PBA could be allowed, but at this moment
the precise location of PBA isn't saved.
Signed-off-by: Marek Marczykowski-Górecki
---
v2:
- adjust commit message
- pass struct domain to msixtbl_page_handler_get_hwaddr()
- reduce local variables used only once
- log a warning if write is forbidden if MSI-X and PBA lives on the same
page
- do not passthrough unaligned accesses
- handle accesses both before and after MSI-X table
---
xen/arch/x86/hvm/vmsi.c| 154 +
xen/arch/x86/include/asm/msi.h | 5 ++
xen/arch/x86/msi.c | 38
3 files changed, 197 insertions(+)
diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 9c82bf9b4ec2..9293009a4075 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -438,6 +438,152 @@ static const struct hvm_io_ops msixtbl_mmio_ops = {
.write = _msixtbl_write,
};
+static void __iomem *msixtbl_page_handler_get_hwaddr(
+const struct domain *d,
+uint64_t address,
+bool write)
+{
+const struct pci_dev *pdev = NULL;
+const struct msixtbl_entry *entry;
+int adj_type;
+
+rcu_read_lock(&msixtbl_rcu_lock);
+/*
+ * Check if it's on the same page as the end of the MSI-X table, but
+ * outside of the table itself.
+ */
+list_for_each_entry( entry, &d->arch.hvm.msixtbl_list, list )
+{
+if ( PFN_DOWN(address) == PFN_DOWN(entry->gtable) &&
+ address < entry->gtable )
+{
+adj_type = ADJ_IDX_FIRST;
+pdev = entry->pdev;
+break;
+}
+if ( PFN_DOWN(address) == PFN_DOWN(entry->gtable + entry->table_len) &&
+ address >= entry->gtable + entry->table_len )
+{
+adj_type = ADJ_IDX_LAST;
+pdev = entry->pdev;
+break;
+}
+}
+rcu_read_unlock(&msixtbl_rcu_lock);
+
+if ( !pdev )
+return NULL;
+
+ASSERT(pdev->msix);
+
+if ( !pdev->msix->adj_access_table_idx[adj_type] )
+{
+gdprintk(XENLOG_WARNING,
+ "Page for adjacent MSI-X table access not initialized for
%pp\n",
+ pdev);
+
+return NULL;
+}
+
+/* If PBA lives on the same page too, forbid writes. */
+if ( write && (
+(adj_type == ADJ_IDX_LAST &&
+ pdev->msix->table.last == pdev->msix->pba.first) ||
+(adj_type == ADJ_IDX_FIRST &&
+ pdev->msix->table.first == pdev->msix->pba.last)) )
+{
+gdprintk(XENLOG_WARNING,
+ "MSI-X table and PBA of %pp live on the same page, "
+ "writing to other registers there is not implemented\n",
+ pdev);
+return NULL;
+}
+
+return fix_to_virt(pdev->msix->adj_access_table_idx[adj_type]) +
+(address & (PAGE_SIZE - 1));
+
+}
+
+static bool cf_check msixtbl_page_accept(
+const struct hvm_io_handler *handler, const ioreq_t *r)
+{
+ASSERT(r->type == IOREQ_TYPE_COPY);
+
+return msixtbl_page_handler_get_hwaddr(
+current->domain, r->addr, r->dir == IOREQ_WRITE);
+}
+
+static int cf_check msixtbl_page_read(
+const struct hvm_io_handler *handler,
+uint64_t address, uint32_t len, uint64_t *pval)
+{
+void __iomem *hwaddr;
+
+if ( address & (len - 1) )
+return X86EMUL_UNHANDLEABLE;
+
+hwaddr = msixtbl_page_handler_get_hwaddr(
+current->domain, address, false);
+
+if ( !hwaddr )
+return X86EMUL_UNHANDLEABLE;
+
+switch ( len )
+{
+case 1:
+*pval = readb(hwaddr);
+break;
+case 2:
+*pval = readw(hwaddr);
+break;
+case 4:
+*pval = readl(hwaddr);
+break;
+case 8:
+*pval = readq(hwaddr);
+break;
+default:
+return X86EMUL_UNHANDLEABLE;
+}
+return X86EMUL_OKAY;
+}
+
+static int cf_check msixtbl_page_write(
+const struct hvm_io_handler *handler,
+uint64_t address, uint32_t len, uint64_t val)
+{
+void __iomem *hwaddr = msixtbl_page_handler_get_hwaddr(
+
[PATCH v2 3/3] x86/msi: clear initial MSI-X state on boot
Some firmware/devices are found to not reset MSI-X properly, leaving MASKALL set. Xen relies on initial state being both disabled. Especially, pci_reset_msix_state() assumes if MASKALL is set, it was Xen setting it due to msix->host_maskall or msix->guest_maskall. Clearing just MASKALL might be unsafe if ENABLE is set, so clear them both. Reported-by: Jason Andryuk Signed-off-by: Marek Marczykowski-Górecki --- v2: - new patch --- xen/drivers/passthrough/msi.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/xen/drivers/passthrough/msi.c b/xen/drivers/passthrough/msi.c index ce1a450f6f4a..60adad47e379 100644 --- a/xen/drivers/passthrough/msi.c +++ b/xen/drivers/passthrough/msi.c @@ -48,6 +48,13 @@ int pdev_msi_init(struct pci_dev *pdev) ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos)); msix->nr_entries = msix_table_size(ctrl); +/* + * Clear both ENABLE and MASKALL, pci_reset_msix_state() relies on this + * initial state. + */ +ctrl &= ~(PCI_MSIX_FLAGS_ENABLE|PCI_MSIX_FLAGS_MASKALL); +pci_conf_write16(pdev->sbdf, msix_control_reg(pos), ctrl); + pdev->msix = msix; } -- 2.39.2
[ovmf test] 179950: all pass - PUSHED
flight 179950 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/179950/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf f6bd3286edfbe5eb6b50946cc8bb89e5c60b9388 baseline version: ovmf 1f26a9e62e0d7d930b8c260c58b16cb9fb1cc940 Last test of basis 179927 2023-03-24 15:10:43 Z0 days Testing same since 179950 2023-03-25 00:10:39 Z0 days1 attempts People who touched revisions under test: Chasel Chiu 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 1f26a9e62e..f6bd3286ed f6bd3286edfbe5eb6b50946cc8bb89e5c60b9388 -> xen-tested-master
[xen-unstable-smoke test] 179947: tolerable trouble: pass/starved - PUSHED
flight 179947 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/179947/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen b1f11273d5a774cc88a3685c96c2e7cf6385e3b6 baseline version: xen b5cc3c25a242ddb9c5b108884061b17f35c3084b Last test of basis 179940 2023-03-24 21:00:25 Z0 days Testing same since 179947 2023-03-24 23:00:25 Z0 days1 attempts People who touched revisions under test: Andrew Cooper jobs: build-arm64-xsm pass build-amd64 pass build-armhf starved build-amd64-libvirt pass test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git b5cc3c25a2..b1f11273d5 b1f11273d5a774cc88a3685c96c2e7cf6385e3b6 -> smoke
[xen-unstable test] 179904: regressions - trouble: fail/pass/starved
flight 179904 xen-unstable real [real] flight 179949 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/179904/ http://logs.test-lab.xenproject.org/osstest/logs/179949/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl7 xen-install fail REGR. vs. 179853 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 179853 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 179949-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 179853 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 179853 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 179853 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 179853 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 179853 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 179853 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 179853 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 179853 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 179853 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 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-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-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-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-examine 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-xl 1 build-check(1) starved n/a test-armhf-armhf-xl-credit1 1 build-check(1) starved n/a test-armhf-armhf-xl-credit2 1 build-check(1) starved n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) starved n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) starved n/a test-armhf-armhf-xl-rtds 1 build-check(1) starved n/a test-armhf-armhf-xl-vhd 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 95b757598f699bcb37f7d1fa60faa0ccd0d55c77 baseline version: xen 245d030f4aa79f766e575684127f86748c63bb32 Last test of basis 179853 2023-03-21 22:12:49 Z3 days Failing since179882 2023-03-23 03:10:34 Z1 days2 attempts Testing same since 179904 2023-03-23 17:39:36 Z1 days1 attempts People who touched revisions under test: Andrew Cooper Anthony PERARD Christian Lindig Jan Beulich Juergen Gross Julien Grall Marek Marcz
[xen-unstable-smoke test] 179940: tolerable trouble: pass/starved - PUSHED
flight 179940 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/179940/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen b5cc3c25a242ddb9c5b108884061b17f35c3084b baseline version: xen 715b92ba30f792e326bdd37b5a4969da9c5d4a6c Last test of basis 179926 2023-03-24 14:01:58 Z0 days Failing since179929 2023-03-24 17:00:25 Z0 days3 attempts Testing same since 179940 2023-03-24 21:00:25 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Demi Marie Obenour jobs: build-arm64-xsm pass build-amd64 pass build-armhf starved build-amd64-libvirt pass test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 715b92ba30..b5cc3c25a2 b5cc3c25a242ddb9c5b108884061b17f35c3084b -> smoke
[xen-unstable-smoke bisection] complete build-amd64
branch xen-unstable-smoke xenbranch xen-unstable-smoke job build-amd64 testid xen-build Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: xen git://xenbits.xen.org/xen.git Bug introduced: e1d75084443f676be681fdaf47585cc9a5f5b820 Bug not present: eb23eef476bf44f933fcff42e55119473a1d6e19 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/179945/ commit e1d75084443f676be681fdaf47585cc9a5f5b820 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:36 2023 -0400 build: Change remaining xenbits.xen.org link to HTTPS Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considered deprecated. This patch enforces the use of secure transports for all xenbits.xen.org URLs. All altered links have been tested and are known to work. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable-smoke/build-amd64.xen-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/xen-unstable-smoke/build-amd64.xen-build --summary-out=tmp/179945.bisection-summary --basis-template=179926 --blessings=real,real-bisect,real-retry xen-unstable-smoke build-amd64 xen-build Searching for failure / basis pass: 179931 fail [host=himrod2] / 179926 ok. Failure / basis pass flights: 179931 / 179926 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 Basis pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 715b92ba30f792e326bdd37b5a4969da9c5d4a6c Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/qemu-xen-traditional.git#3d273dd05e51e5a1ffba3d98c7437ee84e8f8764-3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 git://xenbits.xen.org/qemu-xen.git#625eb5e96dc96aa7fddef59a08edae215527f19c-625eb5e96dc96aa7fddef59a08edae215527f19c git://xenbits.xen.org/xen.git#715b92ba30f792e326bdd37b5a4969da9c5d4a6c-054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 Loaded 5001 nodes in revision graph Searching for test results: 179926 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 715b92ba30f792e326bdd37b5a4969da9c5d4a6c 179929 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 179932 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 715b92ba30f792e326bdd37b5a4969da9c5d4a6c 179934 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 179935 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 79493f2b33eeeccc78db25435181a03f5c46b3e6 179936 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c e1d75084443f676be681fdaf47585cc9a5f5b820 179931 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 179937 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c eb23eef476bf44f933fcff42e55119473a1d6e19 179939 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c e1d75084443f676be681fdaf47585cc9a5f5b820 179941 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c eb23eef476bf44f933fcff42e55119473a1d6e19 179942 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c e1d75084443f676be681fdaf47585cc9a5f5b820 179944 pass 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c eb23eef476bf44f933fcff42e55119473a1d6e19 179945 fail 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c e1d75084443f676be681fdaf47585cc9a5f5b820 Searching for interesting versions Result found: flight 179926 (pass), for basis pass For basis failure, parent search stopping at 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7fddef59a08edae215527f19c eb23eef476bf44f933fcff42e55119473a1d6e19, results HASH(0x55d01057b648) HASH(0x55d00f2f2d70) HASH(0x55d0105888c8) For basis failure, parent search stopping at 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764 625eb5e96dc96aa7f
[PATCH] ARM+RISC-V: BSS handling improvements
* Correct comments in arm{32,64}/head.S
* Provide Linker assertions to check the safety of the zeroing loops
Signed-off-by: Andrew Cooper
---
CC: Stefano Stabellini
CC: Julien Grall
CC: Volodymyr Babchuk
CC: Bertrand Marquis
CC: Bob Eshleman
CC: Alistair Francis
CC: Connor Davis
CC: Oleksii Kurochko
Pulled out of the very start of my work to try and unify the handling of
xen_phys_addr across architectures.
---
xen/arch/arm/arm32/head.S | 2 +-
xen/arch/arm/arm64/head.S | 2 +-
xen/arch/arm/xen.lds.S| 2 ++
xen/arch/riscv/xen.lds.S | 4
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index df51550baa8a..f9f7be9588b1 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -301,7 +301,7 @@ ENDPROC(check_cpu_mode)
zero_bss:
PRINT("- Zero BSS -\r\n")
mov_w r0, __bss_start/* r0 := vaddr(__bss_start) */
-mov_w r1, __bss_end /* r1 := vaddr(__bss_start) */
+mov_w r1, __bss_end /* r1 := vaddr(__bss_end) */
mov r2, #0
1: str r2, [r0], #4
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 4a3f87117c83..8a4dd64c99ad 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -437,7 +437,7 @@ zero_bss:
PRINT("- Zero BSS -\r\n")
ldr x0, =__bss_start /* x0 := vaddr(__bss_start) */
-ldr x1, =__bss_end /* x1 := vaddr(__bss_start) */
+ldr x1, =__bss_end /* x1 := vaddr(__bss_end) */
1: str xzr, [x0], #8
cmp x0, x1
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 1b392345bc3b..6ca3caefe607 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -240,3 +240,5 @@ ASSERT(_idmap_end - _idmap_start <= PAGE_SIZE, "Identity
mapped code is larger t
*/
ASSERT(IS_ALIGNED(__init_begin, 4), "__init_begin is misaligned")
ASSERT(IS_ALIGNED(__init_end, 4), "__init_end is misaligned")
+ASSERT(IS_ALIGNED(__bss_start, POINTER_ALIGN), "__bss_start is
misaligned")
+ASSERT(IS_ALIGNED(__bss_end,POINTER_ALIGN), "__bss_end is misaligned")
diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S
index ca57cce75cba..2ed70eccc62a 100644
--- a/xen/arch/riscv/xen.lds.S
+++ b/xen/arch/riscv/xen.lds.S
@@ -1,3 +1,4 @@
+#include
#include
#undef ENTRY
@@ -156,3 +157,6 @@ SECTIONS
ELF_DETAILS_SECTIONS
}
+
+ASSERT(IS_ALIGNED(__bss_start, POINTER_ALIGN), "__bss_start is
misaligned")
+ASSERT(IS_ALIGNED(__bss_end,POINTER_ALIGN), "__bss_end is misaligned")
--
2.30.2
[PATCH] x86/boot: Restrict directmap permissions for .text/.rodata
While we've been diligent to ensure that the main text/data/rodata mappings
have suitable restrictions, their aliases via the directmap were left fully
read/write. Worse, we even had pieces of code making use of this as a
feature.
Restrict the permissions for .text/rodata, as we have no legitimate need for
writeability of these areas via the directmap alias. Note that the
compile-time allocated pagetables do get written through their directmap
alias, so need to remain writeable.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
v2:
* Update comments and commit message for clarity, and over changes.
Notes:
* The stubs are still have RX via one alias, RW via another, and these need
to stay. We should harden this using PKS (available on SPR and later) to
block incidental writes.
* Backing memory for livepatch text/rodata needs similar treatment.
* For backporting, this patch depends on c/s e7f147bf4ac7 ("x86/crash: Drop
manual hooking of exception_table[]") and c/s e7db635f4428 ("x86/pv-shim:
Don't modify the hypercall table"). No compile error will occur from
getting these dependencies wrong.
---
xen/arch/x86/setup.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2b44a3ae26dd..b29229933d8c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1667,6 +1667,16 @@ void __init noreturn __start_xen(unsigned long mbi_p)
destroy_xen_mappings((unsigned long)&__2M_rwdata_end,
ROUNDUP((unsigned long)&__2M_rwdata_end, MB(2)));
+/*
+ * Mark all of .text and .rodata as RO in the directmap - we don't want
+ * these sections writeable via any alias. The compile-time allocated
+ * pagetables are written via their directmap alias, so data/bss needs to
+ * remain writeable.
+ */
+modify_xen_mappings((unsigned long)__va(__pa(_start)),
+(unsigned long)__va(__pa(__2M_rodata_end)),
+PAGE_HYPERVISOR_RO);
+
nr_pages = 0;
for ( i = 0; i < e820.nr_map; i++ )
if ( e820.map[i].type == E820_RAM )
--
2.30.2
[PATCH] x86/ucode: Fix error paths control_thread_fn()
These two early exits skipped re-enabling the watchdog, and restoring the NMI
callback. Always execute the tail of the function on the way out.
Fixes: 8dd4dfa92d62 ("x86/microcode: Synchronize late microcode loading")
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Roger Pau Monné
CC: Wei Liu
CC: Sergey Dyasli
Also, added in the same patch is:
* Note that RDTSC (in wait_for_condition()) is safe for threads to
* execute while waiting for completion of loading an update.
which is absolutely not true in the slightest. RDTSC is all microcode, and
Intel's guidance on the matter right now is that LFENCE is about the only safe
instruction to execute in a wait loop. Even PAUSE is explcitily prohibited...
---
xen/arch/x86/cpu/microcode/core.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/core.c
b/xen/arch/x86/cpu/microcode/core.c
index cfa2d5053a52..61cd36d601d6 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -492,10 +492,7 @@ static int control_thread_fn(const struct microcode_patch
*patch)
ret = wait_for_condition(wait_cpu_callin, num_online_cpus(),
MICROCODE_CALLIN_TIMEOUT_US);
if ( ret )
-{
-set_state(LOADING_EXIT);
-return ret;
-}
+goto out;
/* Control thread loads ucode first while others are in NMI handler. */
ret = alternative_call(ucode_ops.apply_microcode, patch);
@@ -507,8 +504,7 @@ static int control_thread_fn(const struct microcode_patch
*patch)
{
printk(XENLOG_ERR
"Late loading aborted: CPU%u failed to update ucode\n", cpu);
-set_state(LOADING_EXIT);
-return ret;
+goto out;
}
/* Let primary threads load the given ucode update */
@@ -539,6 +535,7 @@ static int control_thread_fn(const struct microcode_patch
*patch)
}
}
+ out:
/* Mark loading is done to unblock other threads */
set_state(LOADING_EXIT);
--
2.30.2
Re: [PATCH] CI: Remove llvm-8 from the Debian Stretch container
On Fri, 24 Mar 2023, Andrew Cooper wrote: > For similar reasons to c/s a6b1e2b80fe20. While this container is still > build-able, all the other problems with explicitly-versioned compilers remain. > > Signed-off-by: Andrew Cooper Reviewed-by: Stefano Stabellini > --- > CC: Anthony PERARD > CC: Stefano Stabellini > CC: Michal Orzel > CC: Doug Goldstein > > This will require backporting to older trees, but there's already a list it > can be added too. > --- > automation/build/debian/stretch-llvm-8.list | 3 --- > automation/build/debian/stretch.dockerfile | 12 - > automation/gitlab-ci/build.yaml | 27 - > 3 files changed, 42 deletions(-) > delete mode 100644 automation/build/debian/stretch-llvm-8.list > > diff --git a/automation/build/debian/stretch-llvm-8.list > b/automation/build/debian/stretch-llvm-8.list > deleted file mode 100644 > index 590001ca81e8.. > --- a/automation/build/debian/stretch-llvm-8.list > +++ /dev/null > @@ -1,3 +0,0 @@ > -# Strech LLVM 8 repos > -deb https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 main > -deb-src https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 main > diff --git a/automation/build/debian/stretch.dockerfile > b/automation/build/debian/stretch.dockerfile > index 2c086b197cba..1af6c691f8f4 100644 > --- a/automation/build/debian/stretch.dockerfile > +++ b/automation/build/debian/stretch.dockerfile > @@ -54,15 +54,3 @@ RUN apt-get update && \ > apt-get autoremove -y && \ > apt-get clean && \ > rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* > - > -RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add - > -COPY stretch-llvm-8.list /etc/apt/sources.list.d/ > - > -RUN apt-get update && \ > -apt-get --quiet --yes install \ > -clang-8 \ > -lld-8 \ > -&& \ > -apt-get autoremove -y && \ > -apt-get clean && \ > -rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* > diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml > index 820cc0af83bd..3547aa419097 100644 > --- a/automation/gitlab-ci/build.yaml > +++ b/automation/gitlab-ci/build.yaml > @@ -28,13 +28,6 @@ > CXX: clang++ > clang: y > > -.clang-8-tmpl: > - variables: &clang-8 > -CC: clang-8 > -CXX: clang++-8 > -LD: ld.lld-8 > -clang: y > - > .x86-64-build-tmpl: ><<: *build >variables: > @@ -99,16 +92,6 @@ >variables: > <<: *clang > > -.clang-8-x86-64-build: > - extends: .x86-64-build > - variables: > -<<: *clang-8 > - > -.clang-8-x86-64-build-debug: > - extends: .x86-64-build-debug > - variables: > -<<: *clang-8 > - > .clang-x86-32-build: >extends: .x86-32-build >variables: > @@ -285,16 +268,6 @@ debian-stretch-clang-debug: >variables: > CONTAINER: debian:stretch > > -debian-stretch-clang-8: > - extends: .clang-8-x86-64-build > - variables: > -CONTAINER: debian:stretch > - > -debian-stretch-clang-8-debug: > - extends: .clang-8-x86-64-build-debug > - variables: > -CONTAINER: debian:stretch > - > debian-stretch-gcc: >extends: .gcc-x86-64-build >variables: > -- > 2.30.2 >
Re: [PATCH] CI: Minor updates to buster-gcc-ibt
On Fri, 24 Mar 2023, Andrew Cooper wrote: > * Update from GCC 11.2 to 11.3 > * Use python3-minimal instead of python > * Use --no-install-recommends, requiring ca-certificates, g++-multilib and >build-essential to be listed explicitly > > The resulting container is ~50M smaller > > Signed-off-by: Andrew Cooper I assume you have tested this successfully. Acked-by: Stefano Stabellini > --- > CC: Anthony PERARD > CC: Stefano Stabellini > CC: Michal Orzel > CC: Doug Goldstein > --- > automation/build/debian/buster-gcc-ibt.dockerfile | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/automation/build/debian/buster-gcc-ibt.dockerfile > b/automation/build/debian/buster-gcc-ibt.dockerfile > index 441d9a9ab37a..96ab4fe8a2f1 100644 > --- a/automation/build/debian/buster-gcc-ibt.dockerfile > +++ b/automation/build/debian/buster-gcc-ibt.dockerfile > @@ -4,10 +4,12 @@ ENV DEBIAN_FRONTEND=noninteractive > ENV USER root > > RUN apt-get update && \ > -apt-get --quiet --yes install \ > +apt-get --quiet --yes --no-install-recommends install \ > bison \ > build-essential \ > +ca-certificates \ > flex \ > +g++-multilib \ > libc6-dev-i386 \ > libgmp-dev \ > libisl-dev \ > @@ -19,7 +21,7 @@ RUN apt-get update && \ > RUN mkdir /build > WORKDIR /build > > -RUN wget -q https://ftp.gnu.org/gnu/gcc/gcc-11.2.0/gcc-11.2.0.tar.xz -O - | > tar xJ --strip=1 > +RUN wget -q https://ftp.gnu.org/gnu/gcc/gcc-11.3.0/gcc-11.3.0.tar.xz -O - | > tar xJ --strip=1 > RUN wget -q > https://xenbits.xen.org/people/andrewcoop/gcc-11.2-Add-fcf-check-attribute-yes-no.patch > -O - | patch -p1 > RUN ./configure \ > --prefix=/opt/gcc-11-ibt \ > @@ -53,13 +55,14 @@ RUN mkdir /build > WORKDIR /build > > RUN apt-get update && \ > -apt-get --quiet --yes install \ > +apt-get --quiet --yes --no-install-recommends install \ > bison \ > +build-essential \ > checkpolicy \ > flex \ > gawk \ > make \ > -python3 \ > +python3-minimal \ > && \ > apt-get autoremove -y && \ > apt-get clean && \ > -- > 2.30.2 >
[xen-unstable-smoke test] 179931: regressions - trouble: blocked/fail/pass/starved
flight 179931 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/179931/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 179926 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a 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 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 baseline version: xen 715b92ba30f792e326bdd37b5a4969da9c5d4a6c Last test of basis 179926 2023-03-24 14:01:58 Z0 days Testing same since 179929 2023-03-24 17:00:25 Z0 days2 attempts People who touched revisions under test: Andrew Cooper Demi Marie Obenour jobs: build-arm64-xsm pass build-amd64 fail build-armhf starved build-amd64-libvirt blocked test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked test-amd64-amd64-libvirt blocked 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 Not pushing. commit 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:42 2023 -0400 misc: Replace git:// and http:// with https:// Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considered deprecated. This patch enforces the use of secure transports in misc places. All URLs are known to work. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit 04988f1c595330fd39cdac2c6034ebb30616557e Author: Demi Marie Obenour Date: Tue Mar 21 13:33:40 2023 -0400 configure: Replace git:// and http:// with https:// Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considered deprecated. This patch enforces the use of secure transports in the build system. Some URLs returned 301 or 302 redirects, so I replaced them with the URLs that were redirected to. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit 47ee23f05ac945e5fedf118b8e85af95c5da3276 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:38 2023 -0400 configure: Do not try to use broken links The upstream URLs for zlib, PolarSSL, and the TPM emulator do not work anymore, so do not attempt to use them. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit e1d75084443f676be681fdaf47585cc9a5f5b820 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:36 2023 -0400 build: Change remaining xenbits.xen.org link to HTTPS Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considered deprecated. This patch enforces the use of secure transports for all xenbits.xen.org URLs. All altered links have been tested and are known to work. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit eb23eef476bf44f933fcff42e55119473a1d6e19 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:34 2023 -0400 build: Use HTTPS for all xenbits.xen.org Git repos Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considere
Re: [xen-unstable-smoke test] 179929: regressions - trouble: blocked/fail/pass/starved
On 24/03/2023 8:28 pm, Andrew Cooper wrote: > On 24/03/2023 6:58 pm, osstest service owner wrote: >> flight 179929 xen-unstable-smoke real [real] >> http://logs.test-lab.xenproject.org/osstest/logs/179929/ >> >> Regressions :-( >> >> Tests which did not succeed and are blocking, >> including tests which could not be run: >> build-amd64 6 xen-buildfail REGR. vs. >> 179926 > > Bah. > > make[6]: Entering directory > '/home/osstest/build.179929.build-amd64/xen/tools/firmware/etherboot' > set -e; if ! /usr/bin/wget -c -O _ipxe.tar.gz > https://xenbits.xen.org/xen-extfiles/ipxe-git-3c040ad387099483102708bb1839110bc788cefb.tar.gz; > then \ > git clone file:osstest/IPXE-GIT-FORBIDDEN ipxe.git; \ > (cd ipxe.git && git archive --format=tar --prefix=ipxe/ \ > 3c040ad387099483102708bb1839110bc788cefb | gzip -n >../_ipxe.tar.gz); \ > rm -rf ipxe.git; \ > fi > --2023-03-24 17:06:51-- > https://xenbits.xen.org/xen-extfiles/ipxe-git-3c040ad387099483102708bb1839110bc788cefb.tar.gz > Resolving cache (cache)... 172.16.148.6 > Connecting to cache (cache)|172.16.148.6|:3128... connected. > ERROR: The certificate of 'xenbits.xen.org' is not trusted. > ERROR: The certificate of 'xenbits.xen.org' has expired. > Cloning into 'ipxe.git'... > fatal: '//osstest/IPXE-GIT-FORBIDDEN' does not appear to be a git repository > fatal: Could not read from remote repository. > > That's OSSTest choking, apparently with the same LE root cert problem? Given that there's plenty of content wanting testing right now, and no chance of this being looked at until next week, I've reverted e1d750844 (which was just a single hunk anyway) in the hopes that we can still get a useful weekend of testing ~Andrew
Re: [xen-unstable-smoke test] 179929: regressions - trouble: blocked/fail/pass/starved
On 24/03/2023 6:58 pm, osstest service owner wrote: > flight 179929 xen-unstable-smoke real [real] > http://logs.test-lab.xenproject.org/osstest/logs/179929/ > > Regressions :-( > > Tests which did not succeed and are blocking, > including tests which could not be run: > build-amd64 6 xen-buildfail REGR. vs. > 179926 Bah. make[6]: Entering directory '/home/osstest/build.179929.build-amd64/xen/tools/firmware/etherboot' set -e; if ! /usr/bin/wget -c -O _ipxe.tar.gz https://xenbits.xen.org/xen-extfiles/ipxe-git-3c040ad387099483102708bb1839110bc788cefb.tar.gz; then \ git clone file:osstest/IPXE-GIT-FORBIDDEN ipxe.git; \ (cd ipxe.git && git archive --format=tar --prefix=ipxe/ \ 3c040ad387099483102708bb1839110bc788cefb | gzip -n >../_ipxe.tar.gz); \ rm -rf ipxe.git; \ fi --2023-03-24 17:06:51-- https://xenbits.xen.org/xen-extfiles/ipxe-git-3c040ad387099483102708bb1839110bc788cefb.tar.gz Resolving cache (cache)... 172.16.148.6 Connecting to cache (cache)|172.16.148.6|:3128... connected. ERROR: The certificate of 'xenbits.xen.org' is not trusted. ERROR: The certificate of 'xenbits.xen.org' has expired. Cloning into 'ipxe.git'... fatal: '//osstest/IPXE-GIT-FORBIDDEN' does not appear to be a git repository fatal: Could not read from remote repository. That's OSSTest choking, apparently with the same LE root cert problem? ~Andrew
[PATCH] tools/ocaml/mmap: Drop the len parameter from Xenmmap.write
Strings in Ocaml carry their own length. Absolutely nothing good can come
from having caml_string_length(data) be different to len.
Use the appropriate accessor, String_val(), but retain the workaround for the
Ocaml -safe-string constness bug in the same way as we've done elsewhere in
Xen.
Signed-off-by: Andrew Cooper
---
CC: Christian Lindig
CC: David Scott
CC: Edwin Török
CC: Rob Hoes
---
tools/ocaml/libs/mmap/xenmmap.ml | 4 ++--
tools/ocaml/libs/mmap/xenmmap.mli | 2 +-
tools/ocaml/libs/mmap/xenmmap_stubs.c | 11 +--
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/tools/ocaml/libs/mmap/xenmmap.ml b/tools/ocaml/libs/mmap/xenmmap.ml
index fd6735649f4c..746ca6e21c52 100644
--- a/tools/ocaml/libs/mmap/xenmmap.ml
+++ b/tools/ocaml/libs/mmap/xenmmap.ml
@@ -25,7 +25,7 @@ external mmap: Unix.file_descr -> mmap_prot_flag ->
mmap_map_flag
external unmap: mmap_interface -> unit = "stub_mmap_final"
(* read: interface -> start -> length -> data *)
external read: mmap_interface -> int -> int -> string = "stub_mmap_read"
-(* write: interface -> data -> start -> length -> unit *)
-external write: mmap_interface -> string -> int -> int -> unit =
"stub_mmap_write"
+(* write: interface -> data -> start -> unit *)
+external write: mmap_interface -> string -> int -> unit = "stub_mmap_write"
(* getpagesize: unit -> size of page *)
external getpagesize: unit -> int = "stub_mmap_getpagesize"
diff --git a/tools/ocaml/libs/mmap/xenmmap.mli
b/tools/ocaml/libs/mmap/xenmmap.mli
index d097b68a8fdf..5d6aa19ca6cb 100644
--- a/tools/ocaml/libs/mmap/xenmmap.mli
+++ b/tools/ocaml/libs/mmap/xenmmap.mli
@@ -22,7 +22,7 @@ external mmap : Unix.file_descr -> mmap_prot_flag ->
mmap_map_flag -> int -> int
-> mmap_interface = "stub_mmap_init"
external unmap : mmap_interface -> unit = "stub_mmap_final"
external read : mmap_interface -> int -> int -> string = "stub_mmap_read"
-external write : mmap_interface -> string -> int -> int -> unit
+external write : mmap_interface -> string -> int -> unit
= "stub_mmap_write"
external getpagesize : unit -> int = "stub_mmap_getpagesize"
diff --git a/tools/ocaml/libs/mmap/xenmmap_stubs.c
b/tools/ocaml/libs/mmap/xenmmap_stubs.c
index c85b1fcce7d5..c15a565aaa52 100644
--- a/tools/ocaml/libs/mmap/xenmmap_stubs.c
+++ b/tools/ocaml/libs/mmap/xenmmap_stubs.c
@@ -99,27 +99,26 @@ CAMLprim value stub_mmap_read(value intf, value start,
value len)
caml_invalid_argument("len invalid");
data = caml_alloc_string(c_len);
- memcpy((char *) data, Intf_val(intf)->addr + c_start, c_len);
+ memcpy((char *)String_val(data), Intf_val(intf)->addr + c_start, c_len);
CAMLreturn(data);
}
-CAMLprim value stub_mmap_write(value intf, value data,
- value start, value len)
+CAMLprim value stub_mmap_write(value intf, value data, value start)
{
- CAMLparam4(intf, data, start, len);
+ CAMLparam3(intf, data, start);
int c_start;
int c_len;
c_start = Int_val(start);
- c_len = Int_val(len);
+ c_len = caml_string_length(data);
if (c_start > Intf_val(intf)->len)
caml_invalid_argument("start invalid");
if (c_start + c_len > Intf_val(intf)->len)
caml_invalid_argument("len invalid");
- memcpy(Intf_val(intf)->addr + c_start, (char *) data, c_len);
+ memcpy(Intf_val(intf)->addr + c_start, String_val(data), c_len);
CAMLreturn(Val_unit);
}
--
2.30.2
Re: [PATCH] configure: Drop --enable-githttp
On Fri, Mar 24, 2023 at 08:14:04PM +, Andrew Cooper wrote:
> Following Demi's work to use HTTPS everywhere, all users of GIT_HTTP have
> been removed from the build system. Drop the configure knob.
>
> Signed-off-by: Andrew Cooper
Reviewed-by: Demi Marie Obenour
> ---
> CC: Anthony PERARD
> CC: Demi Marie Obenour
> CC: George Dunlap
> CC: Jan Beulich
> CC: Stefano Stabellini
> CC: Wei Liu
> CC: Julien Grall
> ---
> INSTALL | 5 -
> config/Tools.mk.in| 6 --
> config/Toplevel.mk.in | 1 -
> configure | 27 ---
> configure.ac | 1 -
> tools/configure | 27 ---
> tools/configure.ac| 1 -
> 7 files changed, 68 deletions(-)
>
> diff --git a/INSTALL b/INSTALL
> index 0d3eb89f0298..3816c17dcde8 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -89,11 +89,6 @@ from a wrong location. Compiling the tools with rpath will
> force the
> linker to look in the correct location.
>--enable-rpath
>
> -During build in a git checkout the buildsystem needs to download
> -additional tools such as qemu. This is done with either the native git
> -protocol, or via http if this option is enabled.
> - --enable-githttp
> -
> Disable xenstat and xentop monitoring tools.
>--disable-monitors
>
> diff --git a/config/Tools.mk.in b/config/Tools.mk.in
> index d0d460f922d8..6abb377564db 100644
> --- a/config/Tools.mk.in
> +++ b/config/Tools.mk.in
> @@ -37,12 +37,6 @@ LIBNL3_LIBS := @LIBNL3_LIBS@
> LIBNL3_CFLAGS := @LIBNL3_CFLAGS@
> XEN_TOOLS_RPATH := @rpath@
>
> -# Download GIT repositories via HTTP or GIT's own protocol?
> -# GIT's protocol is faster and more robust, when it works at all (firewalls
> -# may block it). We make it the default, but if your GIT repository downloads
> -# fail or hang, please pass --enable-githttp to configure.
> -GIT_HTTP?= @githttp@
> -
> # Optional components
> XENSTAT_XENTOP := @monitors@
> OCAML_TOOLS := @ocamltools@
> diff --git a/config/Toplevel.mk.in b/config/Toplevel.mk.in
> index 4ecacbb37d68..4db7eafcab5d 100644
> --- a/config/Toplevel.mk.in
> +++ b/config/Toplevel.mk.in
> @@ -1,2 +1 @@
> SUBSYSTEMS := @SUBSYSTEMS@
> -GIT_HTTP ?= @githttp@
> diff --git a/configure b/configure
> index f5cd3c286b55..99f8434cbf64 100755
> --- a/configure
> +++ b/configure
> @@ -594,7 +594,6 @@ stubdom
> tools
> xen
> subdirs
> -githttp
> DEBUG_DIR
> XEN_DUMP_DIR
> XEN_PAGING_DIR
> @@ -673,7 +672,6 @@ with_xen_scriptdir
> with_xen_dumpdir
> with_rundir
> with_debugdir
> -enable_githttp
> enable_xen
> enable_tools
> enable_stubdom
> @@ -1309,8 +1307,6 @@ Optional Features:
>--disable-option-checking ignore unrecognized --enable/--with options
>--disable-FEATURE do not include FEATURE (same as
> --enable-FEATURE=no)
>--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
> - --enable-githttpDownload GIT repositories via HTTP (default is
> - DISABLED)
>--disable-xen Disable build and install of xen
>--disable-tools Disable build and install of tools
>--enable-stubdomEnable build and install of stubdom
> @@ -2124,29 +2120,6 @@ DEBUG_DIR=$debugdir_path
>
>
>
> -# Check whether --enable-githttp was given.
> -if test "${enable_githttp+set}" = set; then :
> - enableval=$enable_githttp;
> -fi
> -
> -
> -if test "x$enable_githttp" = "xno"; then :
> -
> -ax_cv_githttp="n"
> -
> -elif test "x$enable_githttp" = "xyes"; then :
> -
> -ax_cv_githttp="y"
> -
> -elif test -z $ax_cv_githttp; then :
> -
> -ax_cv_githttp="n"
> -
> -fi
> -githttp=$ax_cv_githttp
> -
> -
> -
> case "$host_cpu" in
> i[3456]86|x86_64)
> arch_enable_stubdom=y
> diff --git a/configure.ac b/configure.ac
> index 3aea40715307..19d9311c2ae4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -17,7 +17,6 @@ m4_include([m4/subsystem.m4])
> m4_include([m4/paths.m4])
>
> AX_XEN_EXPAND_CONFIG()
> -AX_ARG_DEFAULT_DISABLE([githttp], [Download GIT repositories via HTTP])
>
> dnl mini-os is only ported to certain platforms
> case "$host_cpu" in
> diff --git a/tools/configure b/tools/configure
> index dae377c98252..bb5b1ae45067 100755
> --- a/tools/configure
> +++ b/tools/configure
> @@ -714,7 +714,6 @@ ovmf
> xsmpolicy
> ocamltools
> monitors
> -githttp
> rpath
> werror
> DEBUG_DIR
> @@ -807,7 +806,6 @@ with_rundir
> with_debugdir
> enable_werror
> enable_rpath
> -enable_githttp
> enable_monitors
> enable_ocamltools
> enable_xsmpolicy
> @@ -1494,8 +1492,6 @@ Optional Features:
>--disable-werrorBuild tools without -Werror (default is ENABLED)
>--enable-rpath Build tools with -Wl,-rpath,LIBDIR (default is
>DISABLED)
> - --enable-githttpDownload GIT repositories via HTTP (default is
> - DISABLED)
>--disable-monitors Disa
[PATCH] configure: Drop --enable-githttp
Following Demi's work to use HTTPS everywhere, all users of GIT_HTTP have
been removed from the build system. Drop the configure knob.
Signed-off-by: Andrew Cooper
---
CC: Anthony PERARD
CC: Demi Marie Obenour
CC: George Dunlap
CC: Jan Beulich
CC: Stefano Stabellini
CC: Wei Liu
CC: Julien Grall
---
INSTALL | 5 -
config/Tools.mk.in| 6 --
config/Toplevel.mk.in | 1 -
configure | 27 ---
configure.ac | 1 -
tools/configure | 27 ---
tools/configure.ac| 1 -
7 files changed, 68 deletions(-)
diff --git a/INSTALL b/INSTALL
index 0d3eb89f0298..3816c17dcde8 100644
--- a/INSTALL
+++ b/INSTALL
@@ -89,11 +89,6 @@ from a wrong location. Compiling the tools with rpath will
force the
linker to look in the correct location.
--enable-rpath
-During build in a git checkout the buildsystem needs to download
-additional tools such as qemu. This is done with either the native git
-protocol, or via http if this option is enabled.
- --enable-githttp
-
Disable xenstat and xentop monitoring tools.
--disable-monitors
diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index d0d460f922d8..6abb377564db 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -37,12 +37,6 @@ LIBNL3_LIBS := @LIBNL3_LIBS@
LIBNL3_CFLAGS := @LIBNL3_CFLAGS@
XEN_TOOLS_RPATH := @rpath@
-# Download GIT repositories via HTTP or GIT's own protocol?
-# GIT's protocol is faster and more robust, when it works at all (firewalls
-# may block it). We make it the default, but if your GIT repository downloads
-# fail or hang, please pass --enable-githttp to configure.
-GIT_HTTP?= @githttp@
-
# Optional components
XENSTAT_XENTOP := @monitors@
OCAML_TOOLS := @ocamltools@
diff --git a/config/Toplevel.mk.in b/config/Toplevel.mk.in
index 4ecacbb37d68..4db7eafcab5d 100644
--- a/config/Toplevel.mk.in
+++ b/config/Toplevel.mk.in
@@ -1,2 +1 @@
SUBSYSTEMS := @SUBSYSTEMS@
-GIT_HTTP ?= @githttp@
diff --git a/configure b/configure
index f5cd3c286b55..99f8434cbf64 100755
--- a/configure
+++ b/configure
@@ -594,7 +594,6 @@ stubdom
tools
xen
subdirs
-githttp
DEBUG_DIR
XEN_DUMP_DIR
XEN_PAGING_DIR
@@ -673,7 +672,6 @@ with_xen_scriptdir
with_xen_dumpdir
with_rundir
with_debugdir
-enable_githttp
enable_xen
enable_tools
enable_stubdom
@@ -1309,8 +1307,6 @@ Optional Features:
--disable-option-checking ignore unrecognized --enable/--with options
--disable-FEATURE do not include FEATURE (same as --enable-FEATURE=no)
--enable-FEATURE[=ARG] include FEATURE [ARG=yes]
- --enable-githttpDownload GIT repositories via HTTP (default is
- DISABLED)
--disable-xen Disable build and install of xen
--disable-tools Disable build and install of tools
--enable-stubdomEnable build and install of stubdom
@@ -2124,29 +2120,6 @@ DEBUG_DIR=$debugdir_path
-# Check whether --enable-githttp was given.
-if test "${enable_githttp+set}" = set; then :
- enableval=$enable_githttp;
-fi
-
-
-if test "x$enable_githttp" = "xno"; then :
-
-ax_cv_githttp="n"
-
-elif test "x$enable_githttp" = "xyes"; then :
-
-ax_cv_githttp="y"
-
-elif test -z $ax_cv_githttp; then :
-
-ax_cv_githttp="n"
-
-fi
-githttp=$ax_cv_githttp
-
-
-
case "$host_cpu" in
i[3456]86|x86_64)
arch_enable_stubdom=y
diff --git a/configure.ac b/configure.ac
index 3aea40715307..19d9311c2ae4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,7 +17,6 @@ m4_include([m4/subsystem.m4])
m4_include([m4/paths.m4])
AX_XEN_EXPAND_CONFIG()
-AX_ARG_DEFAULT_DISABLE([githttp], [Download GIT repositories via HTTP])
dnl mini-os is only ported to certain platforms
case "$host_cpu" in
diff --git a/tools/configure b/tools/configure
index dae377c98252..bb5b1ae45067 100755
--- a/tools/configure
+++ b/tools/configure
@@ -714,7 +714,6 @@ ovmf
xsmpolicy
ocamltools
monitors
-githttp
rpath
werror
DEBUG_DIR
@@ -807,7 +806,6 @@ with_rundir
with_debugdir
enable_werror
enable_rpath
-enable_githttp
enable_monitors
enable_ocamltools
enable_xsmpolicy
@@ -1494,8 +1492,6 @@ Optional Features:
--disable-werrorBuild tools without -Werror (default is ENABLED)
--enable-rpath Build tools with -Wl,-rpath,LIBDIR (default is
DISABLED)
- --enable-githttpDownload GIT repositories via HTTP (default is
- DISABLED)
--disable-monitors Disable xenstat and xentop monitoring tools (default
is ENABLED)
--disable-ocamltoolsDisable Ocaml tools (default is ENABLED)
@@ -4156,29 +4152,6 @@ rpath=$ax_cv_rpath
-# Check whether --enable-githttp was given.
-if test "${enable_githttp+set}" = set; then :
- enableval=$enable_githttp;
-fi
-
-
-if test "x$enable_githttp" = "xno"; then :
-
-ax_cv_githttp="n"
-
-elif test "x$enab
[PATCH] CI: Remove llvm-8 from the Debian Stretch container
For similar reasons to c/s a6b1e2b80fe20. While this container is still build-able, all the other problems with explicitly-versioned compilers remain. Signed-off-by: Andrew Cooper --- CC: Anthony PERARD CC: Stefano Stabellini CC: Michal Orzel CC: Doug Goldstein This will require backporting to older trees, but there's already a list it can be added too. --- automation/build/debian/stretch-llvm-8.list | 3 --- automation/build/debian/stretch.dockerfile | 12 - automation/gitlab-ci/build.yaml | 27 - 3 files changed, 42 deletions(-) delete mode 100644 automation/build/debian/stretch-llvm-8.list diff --git a/automation/build/debian/stretch-llvm-8.list b/automation/build/debian/stretch-llvm-8.list deleted file mode 100644 index 590001ca81e8.. --- a/automation/build/debian/stretch-llvm-8.list +++ /dev/null @@ -1,3 +0,0 @@ -# Strech LLVM 8 repos -deb https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 main -deb-src https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 main diff --git a/automation/build/debian/stretch.dockerfile b/automation/build/debian/stretch.dockerfile index 2c086b197cba..1af6c691f8f4 100644 --- a/automation/build/debian/stretch.dockerfile +++ b/automation/build/debian/stretch.dockerfile @@ -54,15 +54,3 @@ RUN apt-get update && \ apt-get autoremove -y && \ apt-get clean && \ rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* - -RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add - -COPY stretch-llvm-8.list /etc/apt/sources.list.d/ - -RUN apt-get update && \ -apt-get --quiet --yes install \ -clang-8 \ -lld-8 \ -&& \ -apt-get autoremove -y && \ -apt-get clean && \ -rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/* diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index 820cc0af83bd..3547aa419097 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -28,13 +28,6 @@ CXX: clang++ clang: y -.clang-8-tmpl: - variables: &clang-8 -CC: clang-8 -CXX: clang++-8 -LD: ld.lld-8 -clang: y - .x86-64-build-tmpl: <<: *build variables: @@ -99,16 +92,6 @@ variables: <<: *clang -.clang-8-x86-64-build: - extends: .x86-64-build - variables: -<<: *clang-8 - -.clang-8-x86-64-build-debug: - extends: .x86-64-build-debug - variables: -<<: *clang-8 - .clang-x86-32-build: extends: .x86-32-build variables: @@ -285,16 +268,6 @@ debian-stretch-clang-debug: variables: CONTAINER: debian:stretch -debian-stretch-clang-8: - extends: .clang-8-x86-64-build - variables: -CONTAINER: debian:stretch - -debian-stretch-clang-8-debug: - extends: .clang-8-x86-64-build-debug - variables: -CONTAINER: debian:stretch - debian-stretch-gcc: extends: .gcc-x86-64-build variables: -- 2.30.2
[PATCH] CI: Minor updates to buster-gcc-ibt
* Update from GCC 11.2 to 11.3 * Use python3-minimal instead of python * Use --no-install-recommends, requiring ca-certificates, g++-multilib and build-essential to be listed explicitly The resulting container is ~50M smaller Signed-off-by: Andrew Cooper --- CC: Anthony PERARD CC: Stefano Stabellini CC: Michal Orzel CC: Doug Goldstein --- automation/build/debian/buster-gcc-ibt.dockerfile | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/automation/build/debian/buster-gcc-ibt.dockerfile b/automation/build/debian/buster-gcc-ibt.dockerfile index 441d9a9ab37a..96ab4fe8a2f1 100644 --- a/automation/build/debian/buster-gcc-ibt.dockerfile +++ b/automation/build/debian/buster-gcc-ibt.dockerfile @@ -4,10 +4,12 @@ ENV DEBIAN_FRONTEND=noninteractive ENV USER root RUN apt-get update && \ -apt-get --quiet --yes install \ +apt-get --quiet --yes --no-install-recommends install \ bison \ build-essential \ +ca-certificates \ flex \ +g++-multilib \ libc6-dev-i386 \ libgmp-dev \ libisl-dev \ @@ -19,7 +21,7 @@ RUN apt-get update && \ RUN mkdir /build WORKDIR /build -RUN wget -q https://ftp.gnu.org/gnu/gcc/gcc-11.2.0/gcc-11.2.0.tar.xz -O - | tar xJ --strip=1 +RUN wget -q https://ftp.gnu.org/gnu/gcc/gcc-11.3.0/gcc-11.3.0.tar.xz -O - | tar xJ --strip=1 RUN wget -q https://xenbits.xen.org/people/andrewcoop/gcc-11.2-Add-fcf-check-attribute-yes-no.patch -O - | patch -p1 RUN ./configure \ --prefix=/opt/gcc-11-ibt \ @@ -53,13 +55,14 @@ RUN mkdir /build WORKDIR /build RUN apt-get update && \ -apt-get --quiet --yes install \ +apt-get --quiet --yes --no-install-recommends install \ bison \ +build-essential \ checkpolicy \ flex \ gawk \ make \ -python3 \ +python3-minimal \ && \ apt-get autoremove -y && \ apt-get clean && \ -- 2.30.2
[adhoc test] 179924: trouble: blocked/broken/fail/pass
[adhoc play] harness ed1d8de4: PDU/IPMI: Be less aggressive with IPMI commands 179924: trouble: blocked/broken/fail/pass flight 179924 linux-linus play [play] http://logs.test-lab.xenproject.org/osstest/logs/179924/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf broken build-armhf 5 capture-logs broken REGR. vs. 178042 build-armhf 3 syslog-serverrunning Tests which did not succeed, but are not blocking: test-armhf-armhf-examine 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a build-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a build-armhf 4 host-install(4) broken blocked in 178042 test-amd64-amd64-libvirt-qcow2 14 migrate-support-check fail like 178042 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail like 178042 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail like 178042 test-arm64-arm64-xl-vhd 14 migrate-support-checkfail like 178042 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail like 178042 test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail like 178042 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 178042 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail like 178042 test-amd64-amd64-libvirt 15 migrate-support-checkfail like 178042 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail like 178042 test-arm64-arm64-xl 15 migrate-support-checkfail like 178042 test-arm64-arm64-xl 16 saverestore-support-checkfail like 178042 test-arm64-arm64-xl-xsm 15 migrate-support-checkfail like 178042 test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail like 178042 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail like 178042 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail like 178042 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail like 178042 test-arm64-arm64-xl-credit2 15 migrate-support-checkfail like 178042 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail like 178042 test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail like 178042 test-arm64-arm64-xl-credit1 15 migrate-support-checkfail like 178042 test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail like 178042 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 178042 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 178042 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 178042 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 178042 baseline version: flight 178042 jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf broken build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt blocked build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-coresched-amd64-xl
[xen-unstable-smoke test] 179929: regressions - trouble: blocked/fail/pass/starved
flight 179929 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/179929/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 179926 Tests which did not succeed, but are not blocking: build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a 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 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 baseline version: xen 715b92ba30f792e326bdd37b5a4969da9c5d4a6c Last test of basis 179926 2023-03-24 14:01:58 Z0 days Testing same since 179929 2023-03-24 17:00:25 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Demi Marie Obenour jobs: build-arm64-xsm pass build-amd64 fail build-armhf starved build-amd64-libvirt blocked test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64blocked test-amd64-amd64-libvirt blocked 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 Not pushing. commit 054acfc4443cda51bc000c2e3ba08d9fd1bd77f1 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:42 2023 -0400 misc: Replace git:// and http:// with https:// Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considered deprecated. This patch enforces the use of secure transports in misc places. All URLs are known to work. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit 04988f1c595330fd39cdac2c6034ebb30616557e Author: Demi Marie Obenour Date: Tue Mar 21 13:33:40 2023 -0400 configure: Replace git:// and http:// with https:// Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considered deprecated. This patch enforces the use of secure transports in the build system. Some URLs returned 301 or 302 redirects, so I replaced them with the URLs that were redirected to. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit 47ee23f05ac945e5fedf118b8e85af95c5da3276 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:38 2023 -0400 configure: Do not try to use broken links The upstream URLs for zlib, PolarSSL, and the TPM emulator do not work anymore, so do not attempt to use them. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit e1d75084443f676be681fdaf47585cc9a5f5b820 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:36 2023 -0400 build: Change remaining xenbits.xen.org link to HTTPS Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considered deprecated. This patch enforces the use of secure transports for all xenbits.xen.org URLs. All altered links have been tested and are known to work. Signed-off-by: Demi Marie Obenour Acked-by: Andrew Cooper commit eb23eef476bf44f933fcff42e55119473a1d6e19 Author: Demi Marie Obenour Date: Tue Mar 21 13:33:34 2023 -0400 build: Use HTTPS for all xenbits.xen.org Git repos Obtaining code over an insecure transport is a terrible idea for blatently obvious reasons. Even for non-executable data, insecure transports are considere
Re: [GIT PULL] xen: branch for v6.3-rc4
The pull request you sent on Fri, 24 Mar 2023 12:35:50 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git > for-linus-6.3-rc4-tag has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2495697422d374b097151205d399ff0dcbaa08e0 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
[ovmf test] 179927: all pass - PUSHED
flight 179927 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/179927/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 1f26a9e62e0d7d930b8c260c58b16cb9fb1cc940 baseline version: ovmf cf6a0a52b07195ba278e48b89cfb7ddbad332ab1 Last test of basis 179892 2023-03-23 09:43:42 Z1 days Testing same since 179927 2023-03-24 15:10:43 Z0 days1 attempts People who touched revisions under test: Rebecca Cran 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 cf6a0a52b0..1f26a9e62e 1f26a9e62e0d7d930b8c260c58b16cb9fb1cc940 -> xen-tested-master
Re: [PATCH v6 0/5] Stop using insecure transports
On 24/03/2023 4:37 pm, Anthony PERARD wrote: > On Wed, Mar 22, 2023 at 08:37:43AM +, Andrew Cooper wrote: >> On 21/03/2023 5:33 pm, Demi Marie Obenour wrote: >>> Demi Marie Obenour (5): >>> Use HTTPS for all xenbits.xen.org Git repos >>> Change remaining xenbits.xen.org link to HTTPS >>> Build system: Do not try to use broken links >>> Build system: Replace git:// and http:// with https:// >>> Automation and CI: Replace git:// and http:// with https:// >> https://gitlab.com/xen-project/patchew/xen/-/pipelines/813510934 from >> patchew, so I think we're good now on the containers. >> >>> Config.mk | 20 - >>> README | 4 +-- >>> automation/build/debian/stretch-llvm-8.list | 4 +-- >> Except for this, where I thought we'd already dropped it... > We dropped llvm-8 on the unstable container, I don't think there's been > patch for the stretch container. Yeah, I was just figuring that out. I'm going to commit Demi's series as is, and fix the container afterwards. ~Andrew
Re: [PATCH v6 0/5] Stop using insecure transports
On Wed, Mar 22, 2023 at 08:37:43AM +, Andrew Cooper wrote: > On 21/03/2023 5:33 pm, Demi Marie Obenour wrote: > > Demi Marie Obenour (5): > > Use HTTPS for all xenbits.xen.org Git repos > > Change remaining xenbits.xen.org link to HTTPS > > Build system: Do not try to use broken links > > Build system: Replace git:// and http:// with https:// > > Automation and CI: Replace git:// and http:// with https:// > > https://gitlab.com/xen-project/patchew/xen/-/pipelines/813510934 from > patchew, so I think we're good now on the containers. > > > > > Config.mk | 20 - > > README | 4 +-- > > automation/build/debian/stretch-llvm-8.list | 4 +-- > > Except for this, where I thought we'd already dropped it... We dropped llvm-8 on the unstable container, I don't think there's been patch for the stretch container. -- Anthony PERARD
[xen-unstable-smoke test] 179926: tolerable trouble: pass/starved - PUSHED
flight 179926 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/179926/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 715b92ba30f792e326bdd37b5a4969da9c5d4a6c baseline version: xen 9fa425875362cfdb4717a68455fa7ba5dd969780 Last test of basis 179922 2023-03-24 11:02:23 Z0 days Testing same since 179926 2023-03-24 14:01:58 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Edwin Török Jan Beulich Olaf Hering jobs: build-arm64-xsm pass build-amd64 pass build-armhf starved build-amd64-libvirt pass test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 9fa4258753..715b92ba30 715b92ba30f792e326bdd37b5a4969da9c5d4a6c -> smoke
Re: [PATCH] hw/xenpv: Initialize Xen backend operations
On Thu, Mar 23, 2023 at 10:57:34AM +, David Woodhouse wrote:
> From: David Woodhouse
>
> As the Xen backend operations were abstracted out into a function table to
> allow for internally emulated Xen support, we missed the xen_init_pv()
> code path which also needs to install the operations for the true Xen
> libraries. Add the missing call to setup_xen_backend_ops().
>
> Fixes: b6cacfea0b38 ("hw/xen: Add evtchn operations to allow redirection to
> internal emulation")
> Reported-by: Anthony PERARD
> Signed-off-by: David Woodhouse
Tested-by: Anthony PERARD
Thanks,
--
Anthony PERARD
Re: [PATCH] x86/boot: Restrict directmap permissions for .text/.rodata
On 06/12/2021 3:21 pm, Jan Beulich wrote:
> On 06.12.2021 16:11, Andrew Cooper wrote:
>> On 06/12/2021 13:58, Jan Beulich wrote:
>>> On 06.12.2021 14:08, Andrew Cooper wrote:
While we've been diligent to ensure that the main text/data/rodata
mappings
have suitable restrictions, their aliases via the directmap were
left fully
RW. Worse, we even had pieces of code making use of this as a
feature.
Restrict the permissions, as we have no legitimate need for
writeability of
these areas via the directmap alias.
>>> Where do we end up reading .text and/or .rodata through the
>>> directmap? Can't
>>> we zap the mappings altogether?
>>
>> I felt it was safer to keep readability via the directmap.
>>
>> I'm not aware of any logic we have which reads the directmap in order,
>> but it ought to be possible.
>
> Could you add a sentence to this effect to this description, please?
Ok. The commit message a rewrite anyway, given changes to the boot cpu
stack.
>
>>> As to superpage shattering - I understand this is not deemed to be
>>> an issue
>>> in the common case since, with Xen moved as high up below 4G as
>>> possible,
>>> it wouldn't normally live inside a 1G mapping anyway? This may want
>>> calling
>>> out here. Plus, in non-EFI, non-XEN_ALIGN_2M builds isn't this going to
>>> shatter a 2M page at the tail of .rodata?
>>
>> cpu0_stack has already shattered down to 4k, which is likely in the same
>> superpage as rodata in a non-2M build.
>>
>> But at the end of the day, it is a security/performance tradeoff.
>>
>> memcpy(__va(__pa(divide_error)), "\x0f\x0b", 2);
>> asm ("div %ecx" :: "c" (0));
>>
>> is an especially low barrier for an attacker who has a partial write
>> gadget.
>>
>> The security benefits are substantial, and the perf downsides are a
>> handful of extra pagetables, and a handful of pagewalks taking extra
>> steps, in non-fast paths (i.e. distinctly marginal).
>
> How do you easily know what paths there are accessing data on the same
> (potential) superpage? However, thinking about it, with the directmap
> mapping presumably not getting used at all, how the mapping is arranged
> doesn't really matter (except for the extra memory needed, but as you
> say that's probably marginal).
Indeed. Any path which requires Xen to reach into guest state via the
directmap isn't a fastpath in the first place.
~Andrew
Re: [PATCH v3] x86: Fix XEN_DOMCTL_gdbsx_guestmemio crash
On 25/04/2022 2:36 pm, Jan Beulich wrote:
> On 25.04.2022 14:37, Andrew Cooper wrote:
>> When CONFIG_GDBSX is compiled out, iommu_do_domctl() falls over a NULL
>> pointer. One of several bugs here is known-but-compiled-out subops
>> falling
>> into the default chain and hitting unrelated logic.
>>
>> Remove the CONFIG_GDBSX ifdefary in arch_do_domctl() by implementing
>> gdbsx_domctl() and moving the logic across.
>>
>> As minor cleanup,
>> * gdbsx_guest_mem_io() can become static
>> * Remove opencoding of domain_vcpu() and %pd
>>
>> Signed-off-by: Andrew Cooper
>
> Technically
> Reviewed-by: Jan Beulich
Thanks.
I'll tweak the commit message now that the IOMMU fix has gone in.
> Yet as mentioned before, ...
>
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -816,71 +816,12 @@ long arch_do_domctl(
>> }
>> #endif
>>
>> -#ifdef CONFIG_GDBSX
>> case XEN_DOMCTL_gdbsx_guestmemio:
>> - ret = gdbsx_guest_mem_io(d, &domctl->u.gdbsx_guest_memio);
>> - if ( !ret )
>> - copyback = true;
>> - break;
>> -
>> case XEN_DOMCTL_gdbsx_pausevcpu:
>> - {
>> - struct vcpu *v;
>> -
>> - ret = -EBUSY;
>> - if ( !d->controller_pause_count )
>> - break;
>> - ret = -EINVAL;
>> - if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
>> - (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) ==
>> NULL )
>> - break;
>> - ret = vcpu_pause_by_systemcontroller(v);
>> - break;
>> - }
>> -
>> case XEN_DOMCTL_gdbsx_unpausevcpu:
>> - {
>> - struct vcpu *v;
>> -
>> - ret = -EBUSY;
>> - if ( !d->controller_pause_count )
>> - break;
>> - ret = -EINVAL;
>> - if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= d->max_vcpus ||
>> - (v = d->vcpu[domctl->u.gdbsx_pauseunp_vcpu.vcpu]) ==
>> NULL )
>> - break;
>> - ret = vcpu_unpause_by_systemcontroller(v);
>> - if ( ret == -EINVAL )
>> - printk(XENLOG_G_WARNING
>> - "WARN: d%d attempting to unpause %pv which is not
>> paused\n",
>> - currd->domain_id, v);
>> - break;
>> - }
>> -
>> case XEN_DOMCTL_gdbsx_domstatus:
>> - {
>> - struct vcpu *v;
>> -
>> - domctl->u.gdbsx_domstatus.vcpu_id = -1;
>> - domctl->u.gdbsx_domstatus.paused = d->controller_pause_count
>> > 0;
>> - if ( domctl->u.gdbsx_domstatus.paused )
>> - {
>> - for_each_vcpu ( d, v )
>> - {
>> - if ( v->arch.gdbsx_vcpu_event )
>> - {
>> - domctl->u.gdbsx_domstatus.vcpu_id = v->vcpu_id;
>> - domctl->u.gdbsx_domstatus.vcpu_ev =
>> - v->arch.gdbsx_vcpu_event;
>> - v->arch.gdbsx_vcpu_event = 0;
>> - break;
>> - }
>> - }
>> - }
>> - copyback = true;
>> + ret = gdbsx_domctl(d, domctl, ©back);
>> break;
>> - }
>> -#endif
>
> ... I'm not overly happy with the retaining of the case labels here
> (and the knock on effect it'll have for other subsystem domctl-s),
The crash in do_iommu_op() happened because things which weren't iommu
subops ended up in a function only expecting iommu subops, *because*
unrelated case labels got compiled out.
We've also had multiple XSAs elsewhere created by related "just chain
everything through default:" patterns. The legacy MSR paths are still
especially guilty of doing this.
The case labels need to never ever get compiled out, even if their
subsystem has been.
So you have a choice between this patch, or a pattern of the form:
case XEN_DOMCTL_gdbsx_domstatus:
if ( !IS_ENABLED(CONFIG_GDBSX) )
{
ret = -Exxx;
break;
}
...
but the top level case labels need to stay one way or another.
For this patch, it moves a chunk of logic out of a fairly generic file
into its proper subsystem file, and a few extra case labels is a very
cheap price to pay.
~Andrew
[xen-unstable-smoke test] 179922: tolerable trouble: pass/starved - PUSHED
flight 179922 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/179922/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: xen 9fa425875362cfdb4717a68455fa7ba5dd969780 baseline version: xen 95b757598f699bcb37f7d1fa60faa0ccd0d55c77 Last test of basis 179889 2023-03-23 09:00:25 Z1 days Testing same since 179922 2023-03-24 11:02:23 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Anthony PERARD Jan Beulich Juergen Gross jobs: build-arm64-xsm pass build-amd64 pass build-armhf starved build-amd64-libvirt pass test-armhf-armhf-xl starved test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 95b757598f..9fa4258753 9fa425875362cfdb4717a68455fa7ba5dd969780 -> smoke
Re: [RFC QEMU PATCH 08/18] virtio-gpu: Initialize Venus
On Thu, Mar 16, 2023 at 07:14:47AM +0800, Dmitry Osipenko wrote:
> On 3/13/23 18:55, Huang Rui wrote:
> > On Mon, Mar 13, 2023 at 01:51:03AM +0800, Dmitry Osipenko wrote:
> >> On 3/12/23 12:22, Huang Rui wrote:
> >>> From: Antonio Caggiano
> >>>
> >>> Request Venus when initializing VirGL.
> >>>
> >>> Signed-off-by: Antonio Caggiano
> >>> ---
> >>> hw/display/virtio-gpu-virgl.c | 4
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> >>> index fe03dc916f..f5ce206b93 100644
> >>> --- a/hw/display/virtio-gpu-virgl.c
> >>> +++ b/hw/display/virtio-gpu-virgl.c
> >>> @@ -803,7 +803,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
> >>> {
> >>> int ret;
> >>>
> >>> +#ifdef VIRGL_RENDERER_VENUS
> >>> +ret = virgl_renderer_init(g, VIRGL_RENDERER_VENUS,
> >>> &virtio_gpu_3d_cbs);
> >>> +#else
> >>> ret = virgl_renderer_init(g, 0, &virtio_gpu_3d_cbs);
> >>> +#endif
> >>
> >> Note that Venus now requires VIRGL_RENDERER_RENDER_SERVER flag to be
> >> set. Please test the patches with the latest virglrenderer and etc.
> >>
> >> The #ifdef also doesn't allow adding new flags, it should look like:
> >>
> >> #ifdef VIRGL_RENDERER_VENUS
> >> flags |= VIRGL_RENDERER_RENDER_SERVER;
> >> #endif
> >>
> >> ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
> >
> > In fact, we have rebased to the latest virglrenderer:
> >
> > We check both VIRGL_RENDERER_RENDER_SERVER or VIRGL_RENDERER_VENUS in
> > virglrenderer, alternative of them works.
> >
> > https://gitlab.freedesktop.org/rui/virglrenderer/-/commit/c1322a8a84379b1ef7939f56c6761b0114716f45
>
> All the extra changes you made to virglrenderer that Qemu depends on
> need to go upstream. Please open all the relevant merge requests. Thanks!
>
Dmitry, sorry to late response, I have created relevant merge requests
below:
Virglrenderer:
https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/1068
Mesa:
https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22108
I'd appreciate any comments. :-)
Thanks,
Ray
[PATCH v4] vpci/msix: handle accesses adjacent to the MSI-X table
The handling of the MSI-X table accesses by Xen requires that any
pages part of the MSI-X related tables are not mapped into the domain
physmap. As a result, any device registers in the same pages as the
start or the end of the MSIX or PBA tables is not currently
accessible, as the accesses are just dropped.
Note the spec forbids such placing of registers, as the MSIX and PBA
tables must be 4K isolated from any other registers:
"If a Base Address register that maps address space for the MSI-X
Table or MSI-X PBA also maps other usable address space that is not
associated with MSI-X structures, locations (e.g., for CSRs) used in
the other address space must not share any naturally aligned 4-KB
address range with one where either MSI-X structure resides."
Yet the 'Intel Wi-Fi 6 AX201' device on one of my boxes has registers
in the same page as the MSIX tables, and thus won't work on a PVH dom0
without this fix.
In order to cope with the behavior passthrough any accesses that fall
on the same page as the MSIX tables (but don't fall in between) to the
underlying hardware. Such forwarding also takes care of the PBA
accesses, so it allows to remove the code doing this handling in
msix_{read,write}. Note that as a result accesses to the PBA array
are no longer limited to 4 and 8 byte sizes, there's no access size
restriction for PBA accesses documented in the specification.
Signed-off-by: Roger Pau Monné
---
Changes since v3:
- Keep the vpci lock taken for the duration of the access to the
mapped region.
- Move back the handling of unaligned accesses before getting the
table map.
Changes since v2:
- Slightly adjust VMSIX_ADDR_SAME_PAGE().
- Use IS_ALIGNED and unlikely for the non-aligned access checking.
- Move the check for the page mapped before the aligned one.
- Remove cast of data to uint8_t and instead use a mask in order to
avoid undefined behaviour when shifting.
- Remove Xen maps of the MSIX related regions when memory decoding
for the device is enabled by dom0, in order to purge stale maps.
Changes since v1:
- Properly handle the PBA also.
- Merge the handlers for adjacent writes into the existing MSIX table
ones.
---
xen/drivers/vpci/msix.c | 349 +---
xen/drivers/vpci/vpci.c | 7 +-
xen/include/xen/vpci.h | 8 +-
3 files changed, 269 insertions(+), 95 deletions(-)
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index bea0cc7aed..99dd249c15 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -27,6 +27,11 @@
((addr) >= vmsix_table_addr(vpci, nr) && \
(addr) < vmsix_table_addr(vpci, nr) + vmsix_table_size(vpci, nr))
+#define VMSIX_ADDR_SAME_PAGE(addr, vpci, nr) \
+(PFN_DOWN(addr) >= PFN_DOWN(vmsix_table_addr(vpci, nr)) &&\
+ PFN_DOWN(addr) <= PFN_DOWN(vmsix_table_addr(vpci, nr) + \
+vmsix_table_size(vpci, nr) - 1))
+
static uint32_t cf_check control_read(
const struct pci_dev *pdev, unsigned int reg, void *data)
{
@@ -149,7 +154,7 @@ static struct vpci_msix *msix_find(const struct domain *d,
unsigned long addr)
for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
if ( bars[msix->tables[i] & PCI_MSIX_BIRMASK].enabled &&
- VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, i) )
+ VMSIX_ADDR_SAME_PAGE(addr, msix->pdev->vpci, i) )
return msix;
}
@@ -182,36 +187,167 @@ static struct vpci_msix_entry *get_entry(struct
vpci_msix *msix,
return &msix->entries[(addr - start) / PCI_MSIX_ENTRY_SIZE];
}
-static void __iomem *get_pba(struct vpci *vpci)
+static void __iomem *get_table(const struct vpci *vpci, unsigned int slot)
{
struct vpci_msix *msix = vpci->msix;
+paddr_t addr = 0;
+
+ASSERT(spin_is_locked(&vpci->lock));
+
+if ( likely(msix->table[slot]) )
+return msix->table[slot];
+
+switch ( slot )
+{
+case VPCI_MSIX_TBL_TAIL:
+addr = vmsix_table_size(vpci, VPCI_MSIX_TABLE);
+fallthrough;
+case VPCI_MSIX_TBL_HEAD:
+addr += vmsix_table_addr(vpci, VPCI_MSIX_TABLE);
+break;
+
+case VPCI_MSIX_PBA_TAIL:
+addr = vmsix_table_size(vpci, VPCI_MSIX_PBA);
+fallthrough;
+case VPCI_MSIX_PBA_HEAD:
+addr += vmsix_table_addr(vpci, VPCI_MSIX_PBA);
+break;
+
+default:
+ASSERT_UNREACHABLE();
+return NULL;
+}
+
+msix->table[slot] = ioremap(round_pgdown(addr), PAGE_SIZE);
+
+return msix->table[slot];
+}
+
+unsigned int get_slot(const struct vpci *vpci, unsigned long addr)
+{
+unsigned long pfn = PFN_DOWN(addr);
+
/*
- * PBA will only be unmapped when the device is deassigned, so access it
- * without holding the vpci lock.
+ * The logic below relies on having the tables identity mapped to the guest
+ * address space, or for the `addr` parame
Re: [PATCH] x86/hvm: Improve hvm_set_guest_pat() code generation again
On 24/03/2023 9:32 am, Jan Beulich wrote: > On 24.03.2023 01:59, Andrew Cooper wrote: >> On 19/12/2022 7:28 am, Jan Beulich wrote: >>> On 16.12.2022 21:53, Andrew Cooper wrote: >>> Again - one way to look at things. Plus, with Demi's series now also in >>> mind, what's done here is moving us in exactly the opposite direction. >>> Is this hot enough a function to warrant that? >> Yes - from the first cset, 9ce0a5e207f3 - it's used on virtual >> vmentry/exit so is (or will be) reasonably hot in due course. >> >> What is more important in the short term is avoiding the catastrophic >> code generation that Clang still does with default Xen build settings, >> also linked from the commit message. >> >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -302,24 +302,43 @@ void hvm_get_guest_pat(struct vcpu *v, u64 >> *guest_pat) >> *guest_pat = v->arch.hvm.pat_cr; >> } >> >> -int hvm_set_guest_pat(struct vcpu *v, uint64_t guest_pat) >> +/* >> + * MSR_PAT takes 8 uniform fields, each of which must be a valid >> architectural >> + * memory type (0, 1, 4-7). This is a fully vectorised form of the >> + * 8-iteration loop over bytes looking for PAT_TYPE_* constants. > While grep-ing for PAT_TYPE_ will hit this line, I think we want > every individual type to also be found here when grep-ing for one. > The actual values aren't going to change, but perhaps the beast > way to do so would still be by way of BUILD_BUG_ON()s. Why? What does that solve or improve? "pat" is the thing people are going to be looking for if they're actually trying to find this logic. (And I bring this patch up specifically after reviewing Demi's series, where PAT_TYPE_* changes to X86_MT_* but "pat" is still the useful search term IMO.) >>> I don't think "PAT" is a good thing to grep for when trying to find uses >>> of particular memory types. >> This is not a logical use of a particular memory type. Being an >> architectural auditing function, the only legitimate use of these >> constants here is all of them at once. This is the one place you firmly >> don't care about finding when asking the question "How does Xen go about >> handling WP mappings". >> >> I have swapped PAT_TYPE_* for X86_MT_* now that Demi's series has been >> committed, but that is the extent to which I think there are relevant >> changes to be made. > In the interest of getting the code gen issue addressed, but without > being fully convinced this is a good move: > Acked-by: Jan Beulich Thankyou. ~Andrew
Re: [adhoc test] 179901: regressions - trouble: blocked/broken/fail/pass
On Fri, Mar 24, 2023 at 12:16:10PM +0100, Juergen Gross wrote: > Next try. > > Anthony, could you please use this patch instead of the previous one? I've created flight 179924, with: https://xenbits.xen.org/gitweb/?p=people/aperard/linux.git;a=shortlog;h=refs/heads/0593df9b-f8ad-0042-e024-354623f16...@suse.com Cheers, -- Anthony PERARD
Re: [PATCH 1/4] x86/hvmloader: SMP improvements
On 24/08/2022 3:21 pm, Jan Beulich wrote:
> On 24.08.2022 12:59, Andrew Cooper wrote:
>
>> - ap_callin = 1;
>> +
>> + /*
>> + * Call in to the BSP. For APs, take ourselves offline.
>> + *
>> + * We must not use the stack after calling in to the BSP.
>> + */
>> + asm volatile (
>> + " movb $1, ap_callin \n"
>> +
>> + " test %[icr2], %[icr2] \n"
>> + " jz .Lbsp \n"
>
> Are we intending to guarantee going forward that the BSP always has
> APIC ID zero?
It's currently true, and I doubt that will change, but I prefer the
suggestion to not call this at all on the BSP.
>
>> + " movl %[icr2], %[ICR2] \n"
>> + " movl %[init], %[ICR1] \n"
>> + "1: hlt \n"
>> + " jmp 1b \n"
>
> The use of the function for the BSP is questionable anyway. What is
> really needed is the call to cacheattr_init(). I'm inclined to
> suggest to move to something like
>
> void smp_initialise(void)
> {
> unsigned int i, nr_cpus = hvm_info->nr_vcpus;
>
> cacheattr_init();
>
> if ( nr_cpus <= 1 )
> return;
>
> memcpy((void *)AP_BOOT_EIP, ap_boot_start, ap_boot_end -
> ap_boot_start);
>
> printf("Multiprocessor initialisation:\n");
> for ( i = 1; i < nr_cpus; i++ )
> boot_cpu(i);
> }
>
> thus eliminating bogus output when there's just one vCPU.
> Then the function here can become noreturn (which I was about to suggest
> until spotting that for the BSP the function actually does return).
Dropping the printk() isn't nice, because you'll then get unqualified
information from cacheattr_init().
I'll see if I can rearrange this a bit more nicely.
>
>> + ".Lbsp: \n"
>> + :
>> + : [icr2] "r" (SET_APIC_DEST_FIELD(LAPIC_ID(cpu))),
>> + [init] "i" (APIC_DM_INIT),
>> + [ICR1] "m" (*(uint32_t *)(LAPIC_BASE_ADDRESS + APIC_ICR)),
>> + [ICR2] "m" (*(uint32_t *)(LAPIC_BASE_ADDRESS + APIC_ICR2))
>> + : "memory" );
>
> Can't you use APIC_DEST_SELF now, avoiding the need to fiddle
> with ICR2?
No. Fixed is the only message type which can use self or all-inc-self.
All others are only permitted to use the all-excluding-self.
This makes sense as a consequence of likely shortcuts taking when
integrating the LAPIC into the core. Either way, it's documented
behaviour now, however inconvenient this is for this case.
~Andrew
[libvirt test] 179890: tolerable trouble: pass/starved - PUSHED
flight 179890 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/179890/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: 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-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-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-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass build-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-raw 1 build-check(1) starved n/a test-armhf-armhf-libvirt 1 build-check(1) starved n/a test-armhf-armhf-libvirt-qcow2 1 build-check(1) starved n/a build-armhf 2 hosts-allocate starved n/a version targeted for testing: libvirt d1690ae485393b15c765bb2149687fd5f2552e41 baseline version: libvirt 743fdb97c81f38adc6e9b55f402244f7982352f4 Last test of basis 179861 2023-03-22 04:18:51 Z2 days Testing same since 179890 2023-03-23 09:29:51 Z1 days1 attempts People who touched revisions under test: Andrea Bolognani grimst Jan Kuparinen Jiri Denemark Ján Tomko Michal Privoznik jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf starved build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt starved build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt starved test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-qcow2 starved test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw starved test-amd64-i386-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.o
[GIT PULL] xen: branch for v6.3-rc4
Linus, Please git pull the following tag: git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git for-linus-6.3-rc4-tag xen: branch for v6.3-rc4 It contains two fixes: - a build warning fix for a patch which went into rc3 - a fix for avoiding concurrent accesses to the Xen PV console ring page Thanks. Juergen arch/x86/xen/enlighten_pvh.c | 2 +- drivers/tty/hvc/hvc_xen.c| 19 +-- 2 files changed, 18 insertions(+), 3 deletions(-) Jan Beulich (1): x86/PVH: avoid 32-bit build warning when obtaining VGA console info Roger Pau Monne (1): hvc/xen: prevent concurrent accesses to the shared ring
Re: [PATCH 2/4] x86/hvmloader: Don't build as PIC/PIE
On 25/08/2022 8:20 am, Jan Beulich wrote: > On 24.08.2022 12:59, Andrew Cooper wrote: >> HVMLoader is not relocatable in memory, and 32bit PIC code has a large >> overhead. Build it as non-relocatable. >> >> Bloat-o-meter reports a net: >> add/remove: 0/0 grow/shrink: 3/107 up/down: 14/-3370 (-3356) >> >> No functional change. >> >> Signed-off-by: Andrew Cooper >> --- >> CC: Jan Beulich >> CC: Roger Pau Monné >> CC: Wei Liu >> --- >> tools/firmware/hvmloader/Makefile | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/tools/firmware/hvmloader/Makefile >> b/tools/firmware/hvmloader/Makefile >> index 4f31c881613c..eb757819274b 100644 >> --- a/tools/firmware/hvmloader/Makefile >> +++ b/tools/firmware/hvmloader/Makefile >> @@ -23,7 +23,8 @@ include $(XEN_ROOT)/tools/firmware/Rules.mk >> # SMBIOS spec requires format mm/dd/ >> SMBIOS_REL_DATE ?= $(shell date +%m/%d/%Y) >> >> -CFLAGS += $(CFLAGS_xeninclude) >> +CFLAGS += $(CFLAGS_xeninclude) -fno-pic >> +$(call cc-option-add,CFLAGS,-no-pie) > > This is supposed to be coming from EMBEDDED_EXTRA_CFLAGS, if only > it was spelled correctly there. See the patch just sent. This line > (see that other patch) is meaningless anyway, as we don't use > $(CFLAGS) for linking here. So with it dropped > Reviewed-by: Jan Beulich > > I do think though that the description could do with some expanding, > as I don't think -fpic or -fPIC is the default normally. I suppose > it's only specific distros which make this the default. Yeah, for ASLR reasons, but that covers ~all of our downstream users. I'll tweak the commit message and drop the PIE part. ~Andrew
Re: [adhoc test] 179901: regressions - trouble: blocked/broken/fail/pass
On 24.03.23 08:46, Jan Beulich wrote:
On 24.03.2023 08:07, Juergen Gross wrote:
On 24.03.23 01:18, aper...@xenbits.xen.org wrote:
[adhoc play]
harness ed1d8de4: PDU/IPMI: Be less aggressive with IPMI commands
179901: regressions - trouble: blocked/broken/fail/pass
flight 179901 linux-linus play [play]
http://logs.test-lab.xenproject.org/osstest/logs/179901/
Regressions :-(
It seems the grant copy failures are gone, but the tests are still failing.
I have spotted:
[ 18.258030] net eth0: Response for inactive request
[ 18.258080] net eth0: Disabled for further use
in
http://logs.test-lab.xenproject.org/osstest/logs/179901/test-amd64-amd64-xl/elbling1---var-log-xen-console-guest-debian.guest.osstest.log
This is clearly an explanation for the failed tests. I'm looking into it.
Right - xenvif_tx_check_gop() now sends two responses for the split copy.
Next try.
Anthony, could you please use this patch instead of the previous one?
Juergen
From 060a3bb73df2a0fa5718a7a05c740afc8e4e39ad Mon Sep 17 00:00:00 2001
From: Juergen Gross
Date: Thu, 23 Mar 2023 07:29:56 +0100
Subject: [PATCH] xen/netback: don't do grant copy across page boundary
Fix xenvif_get_requests() not to do grant copy operations across local
page boundaries. This requires to double the maximum number of copy
operations per queue, as each copy could now be split into 2.
Make sure that struct xenvif_tx_cb doesn't grow too large.
Cc: sta...@vger.kernel.org
Fixes: ad7f402ae4f4 ("xen/netback: Ensure protocol headers don't fall in the non-linear area")
Signed-off-by: Juergen Gross
---
drivers/net/xen-netback/common.h | 2 +-
drivers/net/xen-netback/netback.c | 28
2 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 3dbfc8a6924e..1fcbd83f7ff2 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -166,7 +166,7 @@ struct xenvif_queue { /* Per-queue data for xenvif */
struct pending_tx_info pending_tx_info[MAX_PENDING_REQS];
grant_handle_t grant_tx_handle[MAX_PENDING_REQS];
- struct gnttab_copy tx_copy_ops[MAX_PENDING_REQS];
+ struct gnttab_copy tx_copy_ops[2 * MAX_PENDING_REQS];
struct gnttab_map_grant_ref tx_map_ops[MAX_PENDING_REQS];
struct gnttab_unmap_grant_ref tx_unmap_ops[MAX_PENDING_REQS];
/* passed to gnttab_[un]map_refs with pages under (un)mapping */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 1b42676ca141..b832e6f24d60 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -334,6 +334,7 @@ static int xenvif_count_requests(struct xenvif_queue *queue,
struct xenvif_tx_cb {
u16 copy_pending_idx[XEN_NETBK_LEGACY_SLOTS_MAX + 1];
u8 copy_count;
+ u32 split_mask;
};
#define XENVIF_TX_CB(skb) ((struct xenvif_tx_cb *)(skb)->cb)
@@ -361,6 +362,8 @@ static inline struct sk_buff *xenvif_alloc_skb(unsigned int size)
struct sk_buff *skb =
alloc_skb(size + NET_SKB_PAD + NET_IP_ALIGN,
GFP_ATOMIC | __GFP_NOWARN);
+
+ BUILD_BUG_ON(sizeof(*XENVIF_TX_CB(skb)) > sizeof(skb->cb));
if (unlikely(skb == NULL))
return NULL;
@@ -396,6 +399,7 @@ static void xenvif_get_requests(struct xenvif_queue *queue,
nr_slots = shinfo->nr_frags + 1;
copy_count(skb) = 0;
+ XENVIF_TX_CB(skb)->split_mask = 0;
/* Create copy ops for exactly data_len bytes into the skb head. */
__skb_put(skb, data_len);
@@ -413,6 +417,12 @@ static void xenvif_get_requests(struct xenvif_queue *queue,
cop->dest.u.gmfn = virt_to_gfn(skb->data + skb_headlen(skb)
- data_len);
+ /* Don't cross local page boundary! */
+ if (cop->dest.offset + amount > XEN_PAGE_SIZE) {
+ amount = XEN_PAGE_SIZE - cop->dest.offset;
+ XENVIF_TX_CB(skb)->split_mask |= 1U << copy_count(skb);
+ }
+
cop->len = amount;
cop->flags = GNTCOPY_source_gref;
@@ -441,7 +451,8 @@ static void xenvif_get_requests(struct xenvif_queue *queue,
nr_slots--;
} else {
/* The copy op partially covered the tx_request.
- * The remainder will be mapped.
+ * The remainder will be mapped or copied in the next
+ * iteration.
*/
txp->offset += amount;
txp->size -= amount;
@@ -518,6 +529,7 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue,
{
struct gnttab_map_grant_ref *gop_map = *gopp_map;
u16 pending_idx;
+ u16 last_copy_idx = copy_pending_idx(skb, copy_count(skb) - 1);
/* This always points to the shinfo of the skb being checked, which
* could be either the first or the one on the frag_list
*/
@@ -529,7 +541,7 @@ static int xenvif_tx_check_gop(struct xenvif_queue *queue,
int nr_frags = shinfo->nr_frags;
const bool sharedslot = nr_frags &&
frag_get_pending_idx(&shinfo->frags[0]) ==
-copy_pending_idx(skb, copy_count(skb) - 1);
+last_copy_idx;
int i, err = 0;
for (i = 0; i < copy_count(skb); i++) {
@@ -539,9 +551,17 @@ static int xenvif_tx_
Re: [PATCH v2 2/2] xen/arm: vpl011: Fix domain_vpl011_init error path
> On 23 Mar 2023, at 13:56, Michal Orzel wrote:
>
> When vgic_reserve_virq() fails and backend is in domain, we should also
> free the allocated event channel.
>
> When backend is in Xen and call to xzalloc() returns NULL, there is no
> need to call xfree(). This should be done instead on an error path
> from vgic_reserve_virq(). Moreover, we should be calling XFREE() to
> prevent an extra free in domain_vpl011_deinit().
>
> In order not to repeat the same steps twice, call domain_vpl011_deinit()
> on an error path whenever there is more work to do than returning rc.
> Since this function can now be called from different places and more
> than once, add proper guards, use XFREE() instead of xfree() and move
> vgic_free_virq() to it.
>
> Take the opportunity to return -ENOMEM instead of -EINVAL when memory
> allocation fails.
>
> Fixes: 1ee1e4b0d1ff ("xen/arm: Allow vpl011 to be used by DomU")
> Signed-off-by: Michal Orzel
Reviewed-by: Luca Fancellu
Re: [PATCH] x86/hvm: Improve hvm_set_guest_pat() code generation again
On 24.03.2023 01:59, Andrew Cooper wrote: > On 19/12/2022 7:28 am, Jan Beulich wrote: >> On 16.12.2022 21:53, Andrew Cooper wrote: >> Again - one way to look at things. Plus, with Demi's series now also in >> mind, what's done here is moving us in exactly the opposite direction. >> Is this hot enough a function to warrant that? > > Yes - from the first cset, 9ce0a5e207f3 - it's used on virtual > vmentry/exit so is (or will be) reasonably hot in due course. > > What is more important in the short term is avoiding the catastrophic > code generation that Clang still does with default Xen build settings, > also linked from the commit message. > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -302,24 +302,43 @@ void hvm_get_guest_pat(struct vcpu *v, u64 > *guest_pat) > *guest_pat = v->arch.hvm.pat_cr; > } > > -int hvm_set_guest_pat(struct vcpu *v, uint64_t guest_pat) > +/* > + * MSR_PAT takes 8 uniform fields, each of which must be a valid > architectural > + * memory type (0, 1, 4-7). This is a fully vectorised form of the > + * 8-iteration loop over bytes looking for PAT_TYPE_* constants. While grep-ing for PAT_TYPE_ will hit this line, I think we want every individual type to also be found here when grep-ing for one. The actual values aren't going to change, but perhaps the beast way to do so would still be by way of BUILD_BUG_ON()s. >>> Why? What does that solve or improve? >>> >>> "pat" is the thing people are going to be looking for if they're >>> actually trying to find this logic. >>> >>> (And I bring this patch up specifically after reviewing Demi's series, >>> where PAT_TYPE_* changes to X86_MT_* but "pat" is still the useful >>> search term IMO.) >> I don't think "PAT" is a good thing to grep for when trying to find uses >> of particular memory types. > > This is not a logical use of a particular memory type. Being an > architectural auditing function, the only legitimate use of these > constants here is all of them at once. This is the one place you firmly > don't care about finding when asking the question "How does Xen go about > handling WP mappings". > > I have swapped PAT_TYPE_* for X86_MT_* now that Demi's series has been > committed, but that is the extent to which I think there are relevant > changes to be made. In the interest of getting the code gen issue addressed, but without being fully convinced this is a good move: Acked-by: Jan Beulich Jan
Re: [PATCH v7 6/6] PCI: Make use of pci_resource_n()
On Fri, Mar 24, 2023 at 10:08:39AM +0100, Philippe Mathieu-Daudé wrote: > On 23/3/23 18:36, Andy Shevchenko wrote: > > Replace open-coded implementations of pci_resource_n() in pci.h. ... > > #define pci_resource_n(dev, bar) (&(dev)->resource[(bar)]) > > -#define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start) > > -#define pci_resource_end(dev, bar) ((dev)->resource[(bar)].end) > > -#define pci_resource_flags(dev, bar) ((dev)->resource[(bar)].flags) > > -#define pci_resource_len(dev,bar) \ > > - ((pci_resource_end((dev), (bar)) == 0) ? 0 :\ > > - \ > > -(pci_resource_end((dev), (bar)) - \ > > - pci_resource_start((dev), (bar)) + 1)) > > +#define pci_resource_start(dev, bar) (pci_resource_n(dev, > > bar)->start) > > +#define pci_resource_end(dev, bar) (pci_resource_n(dev, bar)->end) > > +#define pci_resource_flags(dev, bar) (pci_resource_n(dev, > > bar)->flags) > > +#define pci_resource_len(dev,bar) \ > > + (pci_resource_end((dev), (bar)) ? \ > > +resource_size(pci_resource_n((dev), (bar))) : 0) > > Seems (to me) more logical to have this patch as "PCI: Introduce > pci_resource_n()" ordered before your patch #2 "PCI: Introduce > pci_dev_for_each_resource()". Either way works for me. Bjorn, what do you like? > Here as #6 or as #2: > Reviewed-by: Philippe Mathieu-Daudé Thank you! -- With Best Regards, Andy Shevchenko
Re: [PATCH v7 4/6] EISA: Convert to use less arguments in pci_bus_for_each_resource()
On Fri, Mar 24, 2023 at 10:02:15AM +0100, Philippe Mathieu-Daudé wrote: > On 23/3/23 18:36, Andy Shevchenko wrote: > > The pci_bus_for_each_resource() can hide the iterator loop since > > it may be not used otherwise. With this, we may drop that iterator > > variable definition. > > > > Signed-off-by: Andy Shevchenko > > Reviewed-by: Krzysztof Wilczyński > > --- > > drivers/eisa/pci_eisa.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c > > Since this is *PCI* EISA, could be squashed into previous patch. I believe it would be better to have them separated. But if maintainers want to squash, I can do that. > Reviewed-by: Philippe Mathieu-Daudé Thank you! -- With Best Regards, Andy Shevchenko
Re: [PATCH v7 6/6] PCI: Make use of pci_resource_n()
On 23/3/23 18:36, Andy Shevchenko wrote: Replace open-coded implementations of pci_resource_n() in pci.h. Signed-off-by: Andy Shevchenko --- include/linux/pci.h | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/include/linux/pci.h b/include/linux/pci.h index 70a4684d5f26..9539cf63fe5e 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2006,14 +2006,12 @@ int pci_iobar_pfn(struct pci_dev *pdev, int bar, struct vm_area_struct *vma); * for accessing popular PCI BAR info */ #define pci_resource_n(dev, bar) (&(dev)->resource[(bar)]) -#define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start) -#define pci_resource_end(dev, bar) ((dev)->resource[(bar)].end) -#define pci_resource_flags(dev, bar) ((dev)->resource[(bar)].flags) -#define pci_resource_len(dev,bar) \ - ((pci_resource_end((dev), (bar)) == 0) ? 0 :\ - \ -(pci_resource_end((dev), (bar)) - \ - pci_resource_start((dev), (bar)) + 1)) +#define pci_resource_start(dev, bar) (pci_resource_n(dev, bar)->start) +#define pci_resource_end(dev, bar) (pci_resource_n(dev, bar)->end) +#define pci_resource_flags(dev, bar) (pci_resource_n(dev, bar)->flags) +#define pci_resource_len(dev,bar) \ + (pci_resource_end((dev), (bar)) ? \ +resource_size(pci_resource_n((dev), (bar))) : 0) Seems (to me) more logical to have this patch as "PCI: Introduce pci_resource_n()" ordered before your patch #2 "PCI: Introduce pci_dev_for_each_resource()". Here as #6 or as #2: Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v7 4/6] EISA: Convert to use less arguments in pci_bus_for_each_resource()
On 23/3/23 18:36, Andy Shevchenko wrote: The pci_bus_for_each_resource() can hide the iterator loop since it may be not used otherwise. With this, we may drop that iterator variable definition. Signed-off-by: Andy Shevchenko Reviewed-by: Krzysztof Wilczyński --- drivers/eisa/pci_eisa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c Since this is *PCI* EISA, could be squashed into previous patch. Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v7 3/6] PCI: Allow pci_bus_for_each_resource() to take less arguments
On 23/3/23 18:36, Andy Shevchenko wrote: Refactor pci_bus_for_each_resource() in the same way as it's done in pci_dev_for_each_resource() case. This will allow to hide iterator inside the loop, where it's not used otherwise. No functional changes intended. Signed-off-by: Andy Shevchenko Reviewed-by: Krzysztof Wilczyński --- drivers/pci/bus.c | 7 +++ drivers/pci/hotplug/shpchp_sysfs.c | 8 drivers/pci/pci.c | 3 +-- drivers/pci/probe.c| 2 +- drivers/pci/setup-bus.c| 10 -- include/linux/pci.h| 17 + 6 files changed, 26 insertions(+), 21 deletions(-) Nice. Reviewed-by: Philippe Mathieu-Daudé
Re: [adhoc test] 179901: regressions - trouble: blocked/broken/fail/pass
On 24.03.23 08:46, Jan Beulich wrote: On 24.03.2023 08:07, Juergen Gross wrote: On 24.03.23 01:18, aper...@xenbits.xen.org wrote: [adhoc play] harness ed1d8de4: PDU/IPMI: Be less aggressive with IPMI commands 179901: regressions - trouble: blocked/broken/fail/pass flight 179901 linux-linus play [play] http://logs.test-lab.xenproject.org/osstest/logs/179901/ Regressions :-( It seems the grant copy failures are gone, but the tests are still failing. I have spotted: [ 18.258030] net eth0: Response for inactive request [ 18.258080] net eth0: Disabled for further use in http://logs.test-lab.xenproject.org/osstest/logs/179901/test-amd64-amd64-xl/elbling1---var-log-xen-console-guest-debian.guest.osstest.log This is clearly an explanation for the failed tests. I'm looking into it. Right - xenvif_tx_check_gop() now sends two responses for the split copy. Yes. I'm just figuring out how to keep track of the relation copy_op and pending request. I can't just double the copy_pending_idx[] array size, as this would grow the struct above the allowed size of 48 bytes. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH v3] x86/boot: Factor move_xen() out of __start_xen()
On 23.03.2023 23:01, Andrew Cooper wrote: > Partly for clarity because there is a lot of subtle magic at work here. > Expand the commentary of what's going on. > > Also because there is no need to double copy the stack (32kB). Spilled > content does need accounting for, but this be sorted by only copying only a > handful of words. > > No practical change. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich
Re: [PATCH] x86/shadow: Fix build with no PG_log_dirty
On 24.03.2023 00:46, Andrew Cooper wrote:
> Gitlab Randconfig found:
>
> arch/x86/mm/shadow/common.c: In function 'shadow_prealloc':
> arch/x86/mm/shadow/common.c:1023:18: error: implicit declaration of function
> 'paging_logdirty_levels'; did you mean 'paging_log_dirty_init'?
> [-Werror=implicit-function-declaration]
>1023 | count += paging_logdirty_levels();
> | ^~
> | paging_log_dirty_init
> arch/x86/mm/shadow/common.c:1023:18: error: nested extern declaration of
> 'paging_logdirty_levels' [-Werror=nested-externs]
Okay, that's SHADOW_PAGING && !HVM && PV_SHIM_EXCLUSIVE - I can see
how I missed to test that case.
> Move the declaration outside of #if PG_log_dirty.
>
> Fixes: 33fb3a661223 ("x86/shadow: account for log-dirty mode when
> pre-allocating")
> Signed-off-by: Andrew Cooper
Reviewed-by: Jan Beulich
> An alternative would be to double paging_logdirty_levels() up in the else
> case, returning 0. The underlying logc in shadow_prealloc() succumbs to DCE
> either way.
I think I prefer the way you have it.
Jan
Re: [PATCH] xen: Modify domain_crash() to take a print string
On 24.03.2023 00:15, Andrew Cooper wrote:
> On 04/02/2022 12:54 pm, Jan Beulich wrote:
>> On 03.02.2022 14:38, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -1693,11 +1693,8 @@ static void load_segments(struct vcpu *n)
>>> put_guest(uregs->fs, esp - 5) |
>>> put_guest(uregs->es, esp - 6) |
>>> put_guest(uregs->ds, esp - 7) )
>>> - {
>>> - gprintk(XENLOG_ERR,
>>> - "error while creating compat failsafe
>>> callback frame\n");
>>> - domain_crash(n->domain);
>>> - }
>>> + domain_crash(n->domain,
>>> + "Error creating compat failsafe
>>> callback frame\n");
>>>
>>> if ( n->arch.pv.vgc_flags & VGCF_failsafe_disables_events )
>>> vcpu_info(n, evtchn_upcall_mask) = 1;
>>> @@ -1732,11 +1729,8 @@ static void load_segments(struct vcpu *n)
>>> put_guest(uregs->ds, rsp - 9) |
>>> put_guest(regs->r11, rsp - 10) |
>>> put_guest(regs->rcx, rsp - 11) )
>>> - {
>>> - gprintk(XENLOG_ERR,
>>> - "error while creating failsafe callback frame\n");
>>> - domain_crash(n->domain);
>>> - }
>>> + domain_crash(n->domain,
>>> + "Error creating failsafe callback frame\n");
>>
>> I assume it wasn't really intended to hide potentially relevant
>> information
>> (the subject vCPU) by this change, which - by way of gprintk() - did get
>> logged before (since we already have n == current at this point)?
>
> The information is not lost. __domain_crash() prints current too,
> albeit in a long-winded way.
Oh, right - n == current guarantees the middle path to be taken there.
Considering the other sub-thread also ended up okay-ish:
Reviewed-by: Jan Beulich
Jan
Re: [PATCH 12/16] x86/shadow: make monitor table create/destroy more consistent
On 23.03.2023 19:28, Andrew Cooper wrote: > On 22/03/2023 9:35 am, Jan Beulich wrote: >> While benign at present, it is still a little fragile to operate on a >> wrong "old_mode" value in sh_update_paging_modes(). This can happen when >> no monitor table was present initially - we'd create one for the new >> mode without updating old_mode. Correct this two ways, each of which > > I think you mean "Correct this in two ways" ? > >> would be sufficient on its own: Once by adding "else" to the second of >> the involved if()s in the function, and then by setting the correct >> initial mode for HVM domains in shadow_vcpu_init(). >> >> Further use the same predicate (paging_mode_external()) consistently >> when dealing with shadow mode init/update/cleanup, rather than a mix of >> is_hvm_vcpu() (init), is_hvm_domain() (update), and >> paging_mode_external() (cleanup). >> >> Finally drop a redundant is_hvm_domain() from inside the bigger if() >> (which is being converted to paging_mode_external()) in >> sh_update_paging_modes(). >> >> Signed-off-by: Jan Beulich >> >> --- a/xen/arch/x86/mm/shadow/common.c >> +++ b/xen/arch/x86/mm/shadow/common.c >> @@ -129,8 +129,8 @@ void shadow_vcpu_init(struct vcpu *v) >> } >> #endif >> >> -v->arch.paging.mode = is_hvm_vcpu(v) ? >> - &SHADOW_INTERNAL_NAME(sh_paging_mode, 3) : >> +v->arch.paging.mode = paging_mode_external(v->domain) ? >> + &SHADOW_INTERNAL_NAME(sh_paging_mode, 2) : >>&SHADOW_INTERNAL_NAME(sh_paging_mode, 4); > > As you're changing this, reposition the ? and : to the start of the > following lines? Sure. > But, is 2-level mode actually right? It's better than 3 certainly, and > is what sh_update_paging_modes() selects, but isn't that only right for > the IDENT_PT case? Which is how HVM vCPU-s start, isn't it? For PVH there clearly needs to be a separate (later) paging mode update anyway (or else what - as you say - sh_update_paging_modes() selects would also be wrong), which is going to be whenever we see CR0.PG becoming set by the toolstack. Jan
Re: [adhoc test] 179901: regressions - trouble: blocked/broken/fail/pass
On 24.03.2023 08:07, Juergen Gross wrote: > On 24.03.23 01:18, aper...@xenbits.xen.org wrote: >> [adhoc play] >> harness ed1d8de4: PDU/IPMI: Be less aggressive with IPMI commands >> 179901: regressions - trouble: blocked/broken/fail/pass >> >> flight 179901 linux-linus play [play] >> http://logs.test-lab.xenproject.org/osstest/logs/179901/ >> >> Regressions :-( > > It seems the grant copy failures are gone, but the tests are still failing. > > I have spotted: > > [ 18.258030] net eth0: Response for inactive request > [ 18.258080] net eth0: Disabled for further use > > in > http://logs.test-lab.xenproject.org/osstest/logs/179901/test-amd64-amd64-xl/elbling1---var-log-xen-console-guest-debian.guest.osstest.log > > This is clearly an explanation for the failed tests. I'm looking into it. Right - xenvif_tx_check_gop() now sends two responses for the split copy. Jan
Re: [PATCH 11/16] x86/shadow: drop is_hvm_...() where easily possible
On 23.03.2023 19:18, Andrew Cooper wrote:
> On 22/03/2023 9:35 am, Jan Beulich wrote:
>> Emulation related functions are involved in HVM handling only, and in
>> some cases they even invoke such checks after having already done things
>> which are valid for HVM domains only. OOS active also implies HVM. In
>> sh_remove_all_mappings() one of the two checks is redundant with an
>> earlier paging_mode_external() one (the other, however, needs to stay).
>>
>> Signed-off-by: Jan Beulich
>>
>> --- a/xen/arch/x86/mm/shadow/common.c
>> +++ b/xen/arch/x86/mm/shadow/common.c
>> @@ -1522,7 +1522,7 @@ int sh_remove_all_mappings(struct domain
>> && (page->count_info & PGC_count_mask) <= 3
>> && ((page->u.inuse.type_info & PGT_count_mask)
>> == (is_special_page(page) ||
>> - (is_hvm_domain(d) && is_ioreq_server_page(d,
>> page) )
>> + is_ioreq_server_page(d, page )
>> printk(XENLOG_G_ERR "can't find all mappings of mfn %"PRI_mfn
>> " (gfn %"PRI_gfn"): c=%lx t=%lx s=%d i=%d\n",
>> mfn_x(gmfn), gfn_x(gfn),
>
> Out of context here needs an equivalent adjustment.
I'm afraid I don't seen any further is_hvm_*() in this function.
> But in this case, I'm not sure the commit message covers the relevant
> details. ioreq servers have been made fully common since this code was
> written, and *that* is a better reason for dropping the predicates IMO
> than the redundancy with paging_mode_external().
How does "fully common" matter? It's still a HVM-only thing, hence the
paging_mode_external() check just out of context. Also note that the
ioreq-server-page check is only one side of || (and I realize that by
correcting indentation here at the same time this might be better
visible).
> That said... I'm not sure the logic here is correct any more. It used
> to be the case that ioreq pages were in the p2m, but they're outside of
> the p2m these days, so don't see how there can be any interaction with
> unexpected refcounts any more.
>
> I suspect that one way or another, this change wants to be in a separate
> patch.
I think that if there are further adjustments to make (like dropping
is_ioreq_server_page() altogether, as you appear to suggest), that would
want to be in a separate patch, but the change as done fully fits the
given justification. (Of course in such a patch both _could_ also be
dropped at the same time.)
>> --- a/xen/arch/x86/mm/shadow/multi.c
>> +++ b/xen/arch/x86/mm/shadow/multi.c
>> @@ -3441,7 +3441,7 @@ int sh_rm_write_access_from_sl1p(struct
>>
>> #ifdef CONFIG_HVM
>> /* Remember if we've been told that this process is being torn down */
>> -if ( curr->domain == d && is_hvm_domain(d) )
>> +if ( curr->domain == d )
>> curr->arch.paging.shadow.pagetable_dying
>> = mfn_to_page(gmfn)->pagetable_dying;
>> #endif
>
> This one is dangerous.
>
> After tracing, I can see that sh_rm_write_access_from_sl1p() is only
> called from OOS functions, but this function itself does its very best
> to look like it has mixed PV + HVM usage, and dropping this conditional
> means that pagetable_dying can, in principle at least, become non-NULL
> for a PV guest.
>
> I think this function needs to be made far more obviously HVM-only first.
Oh, sure - the #ifdef inside the functions can be replaced collectively
by one around it, now that OOS code is built separately and for HVM only.
>> --- a/xen/arch/x86/mm/shadow/oos.c
>> +++ b/xen/arch/x86/mm/shadow/oos.c
>> @@ -577,7 +577,6 @@ int sh_unsync(struct vcpu *v, mfn_t gmfn
>> if ( pg->shadow_flags &
>> ((SHF_page_type_mask & ~SHF_L1_ANY) | SHF_out_of_sync)
>> || sh_page_has_multiple_shadows(pg)
>> - || !is_hvm_vcpu(v)
>> || !v->domain->arch.paging.shadow.oos_active )
>
> This is reachable for PV guests as far as I can see. What am I missing ?
Well, the footnote in patch 1 ("x86/shadow: fix and improve
sh_page_has_multiple_shadows()") kind of explains this wrt the safety
of the sh_page_has_multiple_shadows() use here: Since PV guests can't
have OOS pages, there's no way SHF_out_of_sync could be set.
> The changes in hvm.c are all fine, and for those alone, consider it R-by
> if you end up splitting the patch.
Thanks, but for now I'm not meaning to split the patch, as per above.
There will be a new prereq patch as per your request.
Jan
Re: [adhoc test] 179901: regressions - trouble: blocked/broken/fail/pass
On 24.03.23 01:18, aper...@xenbits.xen.org wrote: [adhoc play] harness ed1d8de4: PDU/IPMI: Be less aggressive with IPMI commands 179901: regressions - trouble: blocked/broken/fail/pass flight 179901 linux-linus play [play] http://logs.test-lab.xenproject.org/osstest/logs/179901/ Regressions :-( It seems the grant copy failures are gone, but the tests are still failing. I have spotted: [ 18.258030] net eth0: Response for inactive request [ 18.258080] net eth0: Disabled for further use in http://logs.test-lab.xenproject.org/osstest/logs/179901/test-amd64-amd64-xl/elbling1---var-log-xen-console-guest-debian.guest.osstest.log This is clearly an explanation for the failed tests. I'm looking into it. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
