Re: [PATCH] tools: Fix removal of COPYING and .gitignore
On 06.12.22 08:25, Viresh Kumar wrote:
The Makefile continues to remove the entire tools/include/xen/ directory
on "make clean", which isn't the right thing to do anymore since this
file contains files like COPYING and .gitignore now.
I don't see a .gitignore file in tools/include/xen, and the COYPING file
is created by tools/include/Makefile when creating tools/include/xen.
Since there are only two files at the moment, use "xen/[a-z]*" regex to
remove everything else.
Fixes: 4ea75e9a9058 ("Rework COPYING installed in /usr/include/xen/, due to several
licences")
Signed-off-by: Viresh Kumar
NAK.
---
I got into trouble as my build script does a "make clean" before building
everything again and so build fails without the COPYING file.
This is working just fine for me.
Based of:
https://xenbits.xen.org/gitweb/?p=people/julieng/xen-unstable.git;a=shortlog;h=refs/heads/for-next/4.18
Maybe in the tree you are using commit 25b55688e1f20ebb is missing?
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH] tools: Fix removal of COPYING and .gitignore
The Makefile continues to remove the entire tools/include/xen/ directory
on "make clean", which isn't the right thing to do anymore since this
file contains files like COPYING and .gitignore now.
Since there are only two files at the moment, use "xen/[a-z]*" regex to
remove everything else.
Fixes: 4ea75e9a9058 ("Rework COPYING installed in /usr/include/xen/, due to
several licences")
Signed-off-by: Viresh Kumar
---
I got into trouble as my build script does a "make clean" before building
everything again and so build fails without the COPYING file.
Based of:
https://xenbits.xen.org/gitweb/?p=people/julieng/xen-unstable.git;a=shortlog;h=refs/heads/for-next/4.18
tools/include/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/include/Makefile b/tools/include/Makefile
index f838171e8cd5..2ca344cba521 100644
--- a/tools/include/Makefile
+++ b/tools/include/Makefile
@@ -84,7 +84,7 @@ install: all
.PHONY: clean
clean:
- rm -rf xen xen-xsm acpi
+ rm -rf xen/[a-z]* xen-xsm acpi
$(MAKE) -C xen-foreign clean
rm -f _*.h
--
2.31.1.272.g89b43f80a514
[xen-unstable test] 175055: tolerable FAIL - PUSHED
flight 175055 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/175055/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 175050 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175050 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175050 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175050 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175050 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 175050 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 175050 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175050 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 175050 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175050 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175050 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175050 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-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-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen 9d67161388d8f611467660300bdf9715b7f110b0 baseline version: xen 15241c92677a0276
RE: information and training on Xen Hypervisor
Hi All, We are planning to use xen hypervisor for one of our in-house development of project. Since this is in-house project we have limited budget. We are facing some difficulties to bring-up drivers since we are new to the hypervisor. Can you please suggest me some sought of training or support from the expert who can support us within our budget and who can train the team on Xen to overcome some difficulties. Thanks and Regards, Sandeep
[linux-linus test] 175052: regressions - FAIL
flight 175052 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/175052/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-examine 8 reboot fail REGR. vs. 173462 test-arm64-arm64-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-seattle 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-examine 8 reboot fail REGR. vs. 173462 test-armhf-armhf-xl-multivcpu 8 xen-bootfail REGR. vs. 173462 test-armhf-armhf-libvirt-qcow2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-arndale 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 173462 Tests which are failing intermittently (not blocking): test-amd64-amd64-libvirt-qcow2 19 guest-start/debian.repeat fail pass in 175046 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 8 xen-boot fail REGR. vs. 173462 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 173462 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 173462 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 173462 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 173462 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 173462 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: linux76dcd734eca23168cb008912c0f69ff408905235 baseline version: linux9d84bb40bcb30a7fa16f33baa967aeb9953dda78 Last test of basis 173462 2022-10-07 18:41:45 Z 59 days Failing since173470 2022-10-08 06:21:34 Z 58 days 116 attempts Testing same since 175046 2022-12-05 05:24:16 Z0 days2 attempts 1951 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-coresched-amd64-xlpass test-arm64-arm64-xl fail test-armhf-armhf-x
[PATCH 6/8] x86: Derive XEN_MSR_PAT from its individual entries
This avoids it being a magic constant that is difficult for humans to decode. Use a _Static_assert to check that the old and new values are identical. Signed-off-by: Demi Marie Obenour --- xen/arch/x86/include/asm/processor.h | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h index 8e2816fae9b97bd4e153a30cc3802971fe0355af..64b75e444947c64e2e5eba457deec92a873d7a63 100644 --- a/xen/arch/x86/include/asm/processor.h +++ b/xen/arch/x86/include/asm/processor.h @@ -92,13 +92,33 @@ X86_EFLAGS_NT|X86_EFLAGS_DF|X86_EFLAGS_IF|\ X86_EFLAGS_TF) +/* Individual entries in IA32_CR_PAT */ +#define MSR_PAT_UC _AC(0x00, ULL) +#define MSR_PAT_WC _AC(0x01, ULL) +#define MSR_PAT_RESERVED_1 _AC(0x02, ULL) +#define MSR_PAT_RESERVED_2 _AC(0x03, ULL) +#define MSR_PAT_WT _AC(0x04, ULL) +#define MSR_PAT_WP _AC(0x05, ULL) +#define MSR_PAT_WB _AC(0x06, ULL) +#define MSR_PAT_UCM _AC(0x07, ULL) + /* * Host IA32_CR_PAT value to cover all memory types. This is not the default * MSR_PAT value, and is an ABI with PV guests. */ -#define XEN_MSR_PAT _AC(0x050100070406, ULL) +#define XEN_MSR_PAT (MSR_PAT_WB << 0x00 | \ + MSR_PAT_WT << 0x08 | \ + MSR_PAT_UCM << 0x10 | \ + MSR_PAT_UC << 0x18 | \ + MSR_PAT_WC << 0x20 | \ + MSR_PAT_WP << 0x28 | \ + MSR_PAT_UC << 0x30 | \ + MSR_PAT_UC << 0x38 | \ + 0) #ifndef __ASSEMBLY__ +_Static_assert(XEN_MSR_PAT == 0x050100070406ULL, + "wrong XEN_MSR_PAT breaks PV guests"); struct domain; struct vcpu; -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab
[PATCH 7/8] x86/mm: make code robust to future PAT changes
It may be desirable to change Xen's PAT for various reasons. This
requires changes to several _PAGE_* macros as well. Add static
assertions to check that XEN_MSR_PAT is consistent with the _PAGE_*
macros.
Additionally, Xen has two unused entries in the PAT. Currently these
are UC, but this will change if the hardware ever supports additional
memory types. To avoid future problems, this adds a check in debug
builds that injects #GP into a guest that tries to use one of these
entries, along with returning -EINVAL from the hypercall. Future
versions of Xen will refuse to use these entries even in release builds.
Signed-off-by: Demi Marie Obenour
---
xen/arch/x86/mm.c | 58 +++
1 file changed, 54 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index
5d05399c3a841bf03991a3bed63df9a815c1e891..517fccee699b2a673ba537e47933aefc80017aa5
100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -849,6 +849,45 @@ static int cf_check print_mmio_emul_range(
}
#endif
+static void __init __maybe_unused build_assertions(void)
+{
+/* A bunch of static assertions to check that the XEN_MSR_PAT is valid
+ * and consistent with the _PAGE_* macros */
+#define PAT_VALUE(v) (0xFF & (XEN_MSR_PAT >> (8 * (v
+#define BAD_VALUE(v) ((v) < 0 || (v) > 7 ||
\
+ (v) == MSR_PAT_RESERVED_1 || (v) == MSR_PAT_RESERVED_2)
+#define BAD_PAT_VALUE(v) BUILD_BUG_ON(BAD_VALUE(PAT_VALUE(v)))
+BAD_PAT_VALUE(0);
+BAD_PAT_VALUE(1);
+BAD_PAT_VALUE(2);
+BAD_PAT_VALUE(3);
+BAD_PAT_VALUE(4);
+BAD_PAT_VALUE(5);
+BAD_PAT_VALUE(6);
+BAD_PAT_VALUE(7);
+#undef BAD_PAT_VALUE
+#undef BAD_VALUE
+#define PAT_SHIFT(page_value) (((page_value) & _PAGE_PAT) >> 5 |
\
+ ((page_value) & (_PAGE_PCD | _PAGE_PWT)) >> 3)
+#define CHECK_PAGE_VALUE(page_value) do {
\
+/* Check that the _PAGE_* macros only use bits from PAGE_CACHE_ATTRS */
\
+BUILD_BUG_ON(((_PAGE_##page_value) & PAGE_CACHE_ATTRS) !=
\
+ (_PAGE_##page_value));
\
+/* Check that the _PAGE_* are consistent with XEN_MSR_PAT */
\
+BUILD_BUG_ON(PAT_VALUE(PAT_SHIFT(_PAGE_##page_value)) !=
\
+ (MSR_PAT_##page_value));
\
+} while (0)
+CHECK_PAGE_VALUE(WT);
+CHECK_PAGE_VALUE(WB);
+CHECK_PAGE_VALUE(WC);
+CHECK_PAGE_VALUE(UC);
+CHECK_PAGE_VALUE(UCM);
+CHECK_PAGE_VALUE(WP);
+#undef CHECK_PAGE_VALUE
+#undef PAT_SHIFT
+#undef PAT_VALUE
+}
+
/*
* get_page_from_l1e returns:
* 0 => success (page not present also counts as such)
@@ -961,13 +1000,24 @@ get_page_from_l1e(
switch ( l1f & PAGE_CACHE_ATTRS )
{
-case _PAGE_WB:
+default:
+#ifndef NDEBUG
+printk(XENLOG_G_WARNING
+ "d%d: Guest tried to use bad cachability attribute %u for
MFN %lx\n",
+ l1e_owner->domain_id, l1f & PAGE_CACHE_ATTRS, mfn);
+pv_inject_hw_exception(TRAP_gp_fault, 0);
+return -EINVAL;
+#endif
case _PAGE_WT:
case _PAGE_WP:
-flip |= (l1f & PAGE_CACHE_ATTRS) ^ _PAGE_UC;
+case _PAGE_WB:
+/* Force this to be uncachable */
+return flip | ( (l1f & PAGE_CACHE_ATTRS) ^ _PAGE_UC );
+case _PAGE_WC:
+case _PAGE_UC:
+case _PAGE_UCM:
+return flip;
}
-
-return flip;
}
if ( unlikely((real_pg_owner != pg_owner) &&
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[RFC PATCH 8/8] Use Linux's PAT
This is purely for testing, to see if it works around a bug in i915. It is not intended to be merged. NOT-signed-off-by: DO NOT MERGE --- xen/arch/x86/include/asm/page.h | 4 ++-- xen/arch/x86/include/asm/processor.h | 12 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/xen/arch/x86/include/asm/page.h b/xen/arch/x86/include/asm/page.h index b585235d064a567082582c8e92a4e8283fd949ca..ab9b46f1d0901e50a83fd035ff28d1bda0b781a2 100644 --- a/xen/arch/x86/include/asm/page.h +++ b/xen/arch/x86/include/asm/page.h @@ -333,11 +333,11 @@ void efi_update_l4_pgtable(unsigned int l4idx, l4_pgentry_t); /* Memory types, encoded under Xen's choice of MSR_PAT. */ #define _PAGE_WB (0) -#define _PAGE_WT (_PAGE_PWT) +#define _PAGE_WC (_PAGE_PWT) #define _PAGE_UCM(_PAGE_PCD) #define _PAGE_UC (_PAGE_PCD | _PAGE_PWT) -#define _PAGE_WC (_PAGE_PAT) #define _PAGE_WP (_PAGE_PAT | _PAGE_PWT) +#define _PAGE_WT (_PAGE_PAT | _PAGE_PCD | _PAGE_PWT) /* * Debug option: Ensure that granted mappings are not implicitly unmapped. diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h index 64b75e444947c64e2e5eba457deec92a873d7a63..7a27d19d1dc8aa9c102683985c30fb85864303fa 100644 --- a/xen/arch/x86/include/asm/processor.h +++ b/xen/arch/x86/include/asm/processor.h @@ -107,18 +107,18 @@ * MSR_PAT value, and is an ABI with PV guests. */ #define XEN_MSR_PAT (MSR_PAT_WB << 0x00 | \ - MSR_PAT_WT << 0x08 | \ + MSR_PAT_WC << 0x08 | \ MSR_PAT_UCM << 0x10 | \ MSR_PAT_UC << 0x18 | \ - MSR_PAT_WC << 0x20 | \ + MSR_PAT_WB << 0x20 | \ MSR_PAT_WP << 0x28 | \ - MSR_PAT_UC << 0x30 | \ - MSR_PAT_UC << 0x38 | \ + MSR_PAT_UCM << 0x30 | \ + MSR_PAT_WT << 0x38 | \ 0) #ifndef __ASSEMBLY__ -_Static_assert(XEN_MSR_PAT == 0x050100070406ULL, - "wrong XEN_MSR_PAT breaks PV guests"); +_Static_assert(XEN_MSR_PAT == 0x0407050600070106ULL, + "wrong XEN_MSR_PAT breaks i915 driver on PV Linux"); struct domain; struct vcpu; -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab
[PATCH 0/8] Make PAT handling less brittle
While working on Qubes OS Marek found out that there were some PAT hacks in the Linux i915 driver. I decided to make Xen use Linux’s PAT to see if it solved the graphics glitches that were observed; it did. This required a substantial amount of preliminary work that is useful even without using Linux’s PAT. Patches 1 through 7 are the preliminary work and I would like them to be accepted into upstream Xen. Patch 7 does technically break ABI by rejecting the unused PAT entries in debug builds, but as release builds are not impacted I suspect it is not a serious concern. Patch 8 actually switches to Linux’s PAT and is NOT intended to be merged (at least for now) as it would at a minimum break migration of PV guests from hosts that do not have the patch. Demi Marie Obenour (8): x86/mm: Avoid hard-coding PAT in get_page_from_l1e() p2m-pt: Avoid hard-coding Xen's PAT x86/mm/shadow: avoid assuming a specific Xen PAT efi: Avoid hard-coding the various PAT constants x86/mm/shadow: do not open-code PAGE_CACHE_ATTRS x86: Derive XEN_MSR_PAT from its individual entries x86/mm: make code robust to future PAT changes Use Linux's PAT xen/arch/x86/include/asm/page.h | 4 +- xen/arch/x86/include/asm/processor.h | 22 +- xen/arch/x86/mm.c| 65 xen/arch/x86/mm/p2m-pt.c | 6 +-- xen/arch/x86/mm/shadow/multi.c | 8 ++-- xen/common/efi/boot.c| 10 ++--- 6 files changed, 91 insertions(+), 24 deletions(-) -- Sincerely, Demi Marie Obenour (she/her/hers) Invisible Things Lab
[PATCH 1/8] x86/mm: Avoid hard-coding PAT in get_page_from_l1e()
This still hard-codes the assumption that the two spare values are
mapped to UC. Removing this assumption would require a more complex
patch.
Signed-off-by: Demi Marie Obenour
---
xen/arch/x86/mm.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index
78b1972e4170ca9c37c6e64e76e66a7da87f..5d05399c3a841bf03991a3bed63df9a815c1e891
100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -961,13 +961,10 @@ get_page_from_l1e(
switch ( l1f & PAGE_CACHE_ATTRS )
{
-case 0: /* WB */
-flip |= _PAGE_PWT | _PAGE_PCD;
-break;
-case _PAGE_PWT: /* WT */
-case _PAGE_PWT | _PAGE_PAT: /* WP */
-flip |= _PAGE_PCD | (l1f & _PAGE_PAT);
-break;
+case _PAGE_WB:
+case _PAGE_WT:
+case _PAGE_WP:
+flip |= (l1f & PAGE_CACHE_ATTRS) ^ _PAGE_UC;
}
return flip;
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[PATCH 4/8] efi: Avoid hard-coding the various PAT constants
This makes the code much easier to understand, and avoids problems if
Xen's PAT ever changes in the future.
Signed-off-by: Demi Marie Obenour
---
xen/common/efi/boot.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index
8e880fe30c7541a202dec3e665300d6549953aa3..260997b251b09dae4b48c1b1db665778e02d760a
100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1746,21 +1746,21 @@ void __init efi_init_memory(void)
if ( desc->Attribute & EFI_MEMORY_WB )
/* nothing */;
else if ( desc->Attribute & EFI_MEMORY_WT )
-prot |= _PAGE_PWT | MAP_SMALL_PAGES;
+prot |= _PAGE_WT | MAP_SMALL_PAGES;
else if ( desc->Attribute & EFI_MEMORY_WC )
-prot |= _PAGE_PAT | MAP_SMALL_PAGES;
+prot |= _PAGE_WC | MAP_SMALL_PAGES;
else if ( desc->Attribute & (EFI_MEMORY_UC | EFI_MEMORY_UCE) )
-prot |= _PAGE_PWT | _PAGE_PCD | MAP_SMALL_PAGES;
+prot |= _PAGE_UC | MAP_SMALL_PAGES;
else if ( efi_bs_revision >= EFI_REVISION(2, 5) &&
(desc->Attribute & EFI_MEMORY_WP) )
-prot |= _PAGE_PAT | _PAGE_PWT | MAP_SMALL_PAGES;
+prot |= _PAGE_WP | MAP_SMALL_PAGES;
else
{
printk(XENLOG_ERR "Unknown cachability for MFNs %#lx-%#lx%s\n",
smfn, emfn - 1, efi_map_uc ? ", assuming UC" : "");
if ( !efi_map_uc )
continue;
-prot |= _PAGE_PWT | _PAGE_PCD | MAP_SMALL_PAGES;
+prot |= _PAGE_UC | MAP_SMALL_PAGES;
}
if ( desc->Attribute & (efi_bs_revision < EFI_REVISION(2, 5)
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[PATCH 2/8] p2m-pt: Avoid hard-coding Xen's PAT
This makes the code much easier to understand.
Signed-off-by: Demi Marie Obenour
---
xen/arch/x86/mm/p2m-pt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c
index
eaba2b0fb4e6830f52b7d112fba8175dfe6d2770..cd1af33b6772ab1016e8d4c3284a6bc5d282869d
100644
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -99,13 +99,13 @@ static unsigned long p2m_type_to_flags(const struct
p2m_domain *p2m,
return flags | P2M_BASE_FLAGS | _PAGE_RW | _PAGE_NX_BIT;
case p2m_mmio_direct:
if ( !rangeset_contains_singleton(mmio_ro_ranges, mfn_x(mfn)) )
-flags |= _PAGE_RW;
+flags |= _PAGE_RW | _PAGE_UCM;
else
{
-flags |= _PAGE_PWT;
+flags |= _PAGE_UC;
ASSERT(!level);
}
-return flags | P2M_BASE_FLAGS | _PAGE_PCD;
+return flags | P2M_BASE_FLAGS;
}
}
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[PATCH 5/8] x86/mm/shadow: do not open-code PAGE_CACHE_ATTRS
This makes the code easier to understand.
Signed-off-by: Demi Marie Obenour
---
xen/arch/x86/mm/shadow/multi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index
4e94fec3d50cde0e5a26ecb62ff4d00dd00f759d..6bb564b0145285afc93b72a60b7797fcfe8696dc
100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -535,7 +535,7 @@ _sh_propagate(struct vcpu *v,
if ( guest_nx_enabled(v) )
pass_thru_flags |= _PAGE_NX_BIT;
if ( level == 1 && !shadow_mode_refcounts(d) && mmio_mfn )
-pass_thru_flags |= _PAGE_PAT | _PAGE_PCD | _PAGE_PWT;
+pass_thru_flags |= PAGE_CACHE_ATTRS;
sflags = gflags & pass_thru_flags;
/*
@@ -548,7 +548,7 @@ _sh_propagate(struct vcpu *v,
{
int type;
-ASSERT(!(sflags & (_PAGE_PAT | _PAGE_PCD | _PAGE_PWT)));
+ASSERT(!(sflags & PAGE_CACHE_ATTRS));
/* compute the PAT index for shadow page entry when VT-d is enabled
* and device assigned.
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
[PATCH 3/8] x86/mm/shadow: avoid assuming a specific Xen PAT
This makes the code easier to understand and more robust if Xen's PAT
ever changes.
Signed-off-by: Demi Marie Obenour
---
xen/arch/x86/mm/shadow/multi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index
2370b3060285fee895f335f2a82d3d22ca5d31ed..4e94fec3d50cde0e5a26ecb62ff4d00dd00f759d
100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -629,8 +629,8 @@ _sh_propagate(struct vcpu *v,
else if ( p2mt == p2m_mmio_direct &&
rangeset_contains_singleton(mmio_ro_ranges, mfn_x(target_mfn)) )
{
-sflags &= ~(_PAGE_RW | _PAGE_PAT);
-sflags |= _PAGE_PCD | _PAGE_PWT;
+sflags &= ~(_PAGE_RW | PAGE_CACHE_ATTRS);
+sflags |= _PAGE_UC;
}
// protect guest page tables
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
Re: [PATCH v2 2/5] xen/scripts: add cppcheck tool to the xen-analysis.py script
On Mon, 5 Dec 2022, Luca Fancellu wrote: > Add Cppcheck analysis to the xen-analysis.py script using the > arguments --run-cppcheck. > > Now cppcheck analysis will build Xen while the analysis is performed > on the source files, it will produce a text report and an additional > html output when the script is called with --cppcheck-html. > > With this patch cppcheck will benefit of platform configuration files > that will help it to understand the target of the compilation and > improve the analysis. These are XML files placed in > xen/tools/cppcheck-plat/, describing to cppcheck the length of basic > types to help it in the analysis. > > To do so: > - Update xen-analysis.py with the code to integrate cppcheck. > - add platform configuration files for cppcheck.. > - add cppcheck-cc.sh script that is a wrapper for cppcheck and it's >used as Xen compiler, it will intercept all flags given from the >make build system and it will execute cppcheck on the compiled >file together with the file compilation. > - guarded hypercall-defs.c with CPPCHECK define because cppcheck >gets confused as the file does not contain c code. > - add false-positive-cppcheck.json file > - update documentation. > - update .gitignore > > Signed-off-by: Luca Fancellu > --- > Changes in v2: > - changes to commit message (Jan) > - changes to the cppcheck-cc.sh script and on >platform XML files, fixed version handling >in cppcheck_analysis.py (Stefano) > - Move revert of Makefile and xen/tools/merge_cppcheck_reports.py >to the following patch, modified .gitignore to handle the >presence of both Makefile invoked cppcheck and this new method >(Stefano) > --- > --- > .gitignore| 6 + > docs/misra/cppcheck.txt | 27 +- > docs/misra/documenting-violations.rst | 7 +- > docs/misra/false-positive-cppcheck.json | 12 + > docs/misra/xen-static-analysis.rst| 42 ++- > xen/include/hypercall-defs.c | 9 + > xen/scripts/xen-analysis.py | 18 +- > xen/scripts/xen_analysis/cppcheck_analysis.py | 273 ++ > .../xen_analysis/cppcheck_report_utils.py | 130 + > xen/scripts/xen_analysis/generic_analysis.py | 21 +- > xen/scripts/xen_analysis/settings.py | 77 - > xen/scripts/xen_analysis/utils.py | 21 +- > xen/tools/cppcheck-cc.sh | 223 ++ > xen/tools/cppcheck-plat/arm32-wchar_t4.xml| 17 ++ > xen/tools/cppcheck-plat/arm64-wchar_t2.xml| 17 ++ > xen/tools/cppcheck-plat/x86_64-wchar_t2.xml | 17 ++ > 16 files changed, 860 insertions(+), 57 deletions(-) > create mode 100644 docs/misra/false-positive-cppcheck.json > create mode 100644 xen/scripts/xen_analysis/cppcheck_analysis.py > create mode 100644 xen/scripts/xen_analysis/cppcheck_report_utils.py > create mode 100755 xen/tools/cppcheck-cc.sh > create mode 100644 xen/tools/cppcheck-plat/arm32-wchar_t4.xml > create mode 100644 xen/tools/cppcheck-plat/arm64-wchar_t2.xml > create mode 100644 xen/tools/cppcheck-plat/x86_64-wchar_t2.xml > > diff --git a/.gitignore b/.gitignore > index f5a66f6194dd..39efe2933a33 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -8,8 +8,11 @@ > *.d > *.d2 > *.c.cppcheck > +*.cppcheck.txt > +*.cppcheck.xml > *.opic > *.a > +*.c.json > *.safparse > *.so > *.so.[0-9]* > @@ -282,9 +285,11 @@ xen/arch/*/efi/efi.h > xen/arch/*/efi/pe.c > xen/arch/*/efi/runtime.c > xen/arch/*/include/asm/asm-offsets.h > +xen/build-dir-cppcheck/ > xen/common/config_data.S > xen/common/config.gz > xen/cppcheck-htmlreport/ > +xen/cppcheck-report/ > xen/cppcheck-misra.* > xen/include/headers*.chk > xen/include/compat/* > @@ -315,6 +320,7 @@ xen/xsm/flask/xenpolicy-* > tools/flask/policy/policy.conf > tools/flask/policy/xenpolicy-* > xen/xen > +xen/suppression-list.txt > xen/xen-cppcheck.xml > xen/xen-syms > xen/xen-syms.map > diff --git a/docs/misra/cppcheck.txt b/docs/misra/cppcheck.txt > index 25d8c3050b72..f7b9f678b4d5 100644 > --- a/docs/misra/cppcheck.txt > +++ b/docs/misra/cppcheck.txt > @@ -3,8 +3,7 @@ Cppcheck for Xen static and MISRA analysis > > Xen can be analysed for both static analysis problems and MISRA violation > using > cppcheck, the open source tool allows the creation of a report with all the > -findings. Xen has introduced the support in the Makefile so it's very easy to > -use and in this document we can see how. > +findings. > > The minimum version required for cppcheck is 2.7. Note that at the time of > writing (June 2022), the version 2.8 is known to be broken [1]. > @@ -38,27 +37,7 @@ Dependencies are listed in the readme.md of the project > repository. > Use cppcheck to analyse Xen > === > > -Using cppcheck integration is very simple, it requires few steps: > - > - 1) Compile Xen > - 2) call the cppcheck make target to generate a report
Re: [PATCH v2 1/5] xen/scripts: add xen-analysis.py for coverity and eclair analysis
On Mon, 5 Dec 2022, Luca Fancellu wrote:
> Add new script for coverity/eclair analysis tool that will enable
> the procedure to suppress findings when these tool are used.
> The procedure is documented in docs/misra/documenting-violations.rst
> and the script is documented in docs/misra/xen-static-analysis.rst.
>
> Add in docs/misra/ the files safe.json and
> false-positive-{coverity,eclair}.json that are JSON files containing
> the data structures for the justifications, they are used by the
> analysis script to link the Xen tags to the proprietary tool comment.
>
> Add docs/misra/documenting-violations.rst to explain how to add
> justifications.
>
> Add docs/misra/xen-static-analysis.rst to explain how to use the
> script to analyse Xen.
>
> Add analysis artifacts files to .gitignore.
>
> Signed-off-by: Luca Fancellu
Hi Luca,
You can add my Tested-by to the entire series. I tested this series
successfully as is. Then I tried adding one cppcheck suppression
(shadowVariable) and everything worked as expected. Very nice!
Also for this patch:
Acked-by: Stefano Stabellini
> ---
> v2 changes:
> - no change
> ---
> ---
> .gitignore | 1 +
> docs/misra/documenting-violations.rst| 191 +++
> docs/misra/false-positive-coverity.json | 12 ++
> docs/misra/false-positive-eclair.json| 12 ++
> docs/misra/safe.json | 11 ++
> docs/misra/xen-static-analysis.rst | 54 ++
> xen/scripts/xen-analysis.py | 31 +++
> xen/scripts/xen_analysis/__init__.py | 0
> xen/scripts/xen_analysis/generic_analysis.py | 93 +
> xen/scripts/xen_analysis/settings.py | 97 ++
> xen/scripts/xen_analysis/tag_database.py | 109 +++
> xen/scripts/xen_analysis/utils.py| 37
> 12 files changed, 648 insertions(+)
> create mode 100644 docs/misra/documenting-violations.rst
> create mode 100644 docs/misra/false-positive-coverity.json
> create mode 100644 docs/misra/false-positive-eclair.json
> create mode 100644 docs/misra/safe.json
> create mode 100644 docs/misra/xen-static-analysis.rst
> create mode 100755 xen/scripts/xen-analysis.py
> create mode 100644 xen/scripts/xen_analysis/__init__.py
> create mode 100644 xen/scripts/xen_analysis/generic_analysis.py
> create mode 100644 xen/scripts/xen_analysis/settings.py
> create mode 100644 xen/scripts/xen_analysis/tag_database.py
> create mode 100644 xen/scripts/xen_analysis/utils.py
>
> diff --git a/.gitignore b/.gitignore
> index ea3243af9dde..f5a66f6194dd 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -10,6 +10,7 @@
> *.c.cppcheck
> *.opic
> *.a
> +*.safparse
> *.so
> *.so.[0-9]*
> *.bin
> diff --git a/docs/misra/documenting-violations.rst
> b/docs/misra/documenting-violations.rst
> new file mode 100644
> index ..1d23447556d2
> --- /dev/null
> +++ b/docs/misra/documenting-violations.rst
> @@ -0,0 +1,191 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Documenting violations
> +==
> +
> +Static analysers are used on the Xen codebase for both static analysis and
> MISRA
> +compliance.
> +There might be the need to suppress some findings instead of fixing them and
> +many tools permit the usage of in-code comments that suppress findings so
> that
> +they are not shown in the final report.
> +
> +Xen includes a tool capable of translating a specific comment used in its
> +codebase to the right proprietary in-code comment understandable by the
> selected
> +analyser that suppress its finding.
> +
> +In the Xen codebase, these tags will be used to document and suppress
> findings:
> +
> + - SAF-X-safe: This tag means that the next line of code contains a finding,
> but
> + the non compliance to the checker is analysed and demonstrated to be safe.
> + - SAF-X-false-positive-: This tag means that the next line of code
> + contains a finding, but the finding is a bug of the tool.
> +
> +SAF stands for Static Analyser Finding, the X is a placeholder for a positive
> +number that starts from zero, the number after SAF- shall be incremental and
> +unique, base ten notation and without leading zeros.
> +
> +Entries in the database shall never be removed, even if they are not used
> +anymore in the code (if a patch is removing or modifying the faulty line).
> +This is to make sure that numbers are not reused which could lead to
> conflicts
> +with old branches or misleading justifications.
> +
> +An entry can be reused in multiple places in the code to suppress a finding
> if
> +and only if the justification holds for the same non-compliance to the coding
> +standard.
> +
> +An orphan entry, that is an entry who was justifying a finding in the code,
> but
> +later that code was removed and there is no other use of that entry in the
> code,
> +can be reused as long as the justification for the finding holds. This is
> done
> +to avoid the
[qemu-mainline test] 175051: tolerable FAIL - PUSHED
flight 175051 qemu-mainline real [real] flight 175054 qemu-mainline real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/175051/ http://logs.test-lab.xenproject.org/osstest/logs/175054/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qcow2 21 guest-start/debian.repeat fail pass in 175054-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175043 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175043 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175043 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175043 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175043 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175043 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175043 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175043 test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: qemuuedc93f455fa823a9a5fcb3a8da430104c7375a0a baseline version: qemuuaaf47c7e8b19e2f7093d565115713a9496d316d1 Last test of basis 175043 2022-12-05 00:08:50
[xen-unstable test] 175050: tolerable FAIL - PUSHED
flight 175050 xen-unstable real [real] flight 175053 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/175050/ http://logs.test-lab.xenproject.org/osstest/logs/175053/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-freebsd10-amd64 7 xen-install fail pass in 175053-retest test-amd64-i386-freebsd10-i386 7 xen-install fail pass in 175053-retest Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 175044 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175044 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175044 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175044 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175044 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 175044 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 175044 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175044 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 175044 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175044 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175044 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175044 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-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-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail ne
Re: [RFC PATCH 00/21] Add SMMUv3 Stage 1 Support for XEN guests
On Sat, 3 Dec 2022, Julien Grall wrote: > On 01/12/2022 16:02, Rahul Singh wrote: > > This patch series is sent as RFC to get the initial feedback from the > > community. This patch series consists of 21 patches which is a big number > > for > > the reviewer to review the patches but to understand the feature end-to-end > > we > > thought of sending this as a big series. Once we will get initial feedback, > > we > > will divide the series into a small number of patches for review. > > From the cover letter, it is not clear to me what sort of input you are > expecting for the RFC. Is this about the design itself? > > If so, I think it would be more helpful to write an high level document on how > you plan to emulate the vIOMMU in Xen. So there is one place to > read/agree/verify rather than trying to collate all the information from the > 20+ patches. > > Briefly skimming through I think the main things that need to be addressed in > order of priority: > - How to secure the vIOMMU > - 1 vs multiple vIOMMU > > The questions are very similar to the vITS because the SMMUv3 is based on a > queue. And given you are selling this feature as a security one, I don't think > we can go forward with the review without any understanding/agreement on what > needs to be implemented in order to have a safe/secure vIOMMU. I think we are all aligned here, but let me try to clarify further. As the vIOMMU is exposed to the guest, and exposing a queue-based interface to the guest is not simple, it would be good to clarify in a document the following points: - how is the queue exposed to the guest - how are guest-inputs sanitized - how do the virtual queue resources map to the physical queue resources - lifecycle of the resource mappings - any memory allocations triggered by guest actions and their lifecycle It is difficult to extrapole these details from 21 patches. Having these key detailed written down in the 0/21 email would greatly help with the review. It would make the review go a lot faster.
[xen-unstable-smoke test] 175049: tolerable all pass - PUSHED
flight 175049 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/175049/ 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 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 9d67161388d8f611467660300bdf9715b7f110b0 baseline version: xen 15241c92677a027694e609b55a77ae5ba3616104 Last test of basis 175048 2022-12-05 12:03:13 Z0 days Testing same since 175049 2022-12-05 15:00:25 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Ayan Kumar Halder Jan Beulich Julien Grall Roger Pau Monné Stefano Stabellini jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass 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 15241c9267..9d67161388 9d67161388d8f611467660300bdf9715b7f110b0 -> smoke
Re: [PATCH 5/5] multiboot2: parse vga= option when setting GOP mode
On 23.11.2022 16:45, Roger Pau Monne wrote: > Currently the vga command line gfx- option is ignored when booted > using multboot2 and EFI, as the setting of the GOP mode is done way > before the command line is processed. > > Add support for parsing the vga gfx- selection if present in order to > set the selected GOP mode. > > Signed-off-by: Roger Pau Monné Reviewed-by: Jan Beulich albeit personally I think this should be folded into the previous patch. Jan
[linux-linus test] 175046: regressions - FAIL
flight 175046 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/175046/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-examine 8 reboot fail REGR. vs. 173462 test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-seattle 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-examine 8 reboot fail REGR. vs. 173462 test-armhf-armhf-xl-multivcpu 8 xen-bootfail REGR. vs. 173462 test-armhf-armhf-libvirt-qcow2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-arndale 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-credit1 8 xen-boot fail REGR. vs. 173462 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 8 xen-boot fail REGR. vs. 173462 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 173462 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 173462 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 173462 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 173462 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 173462 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: linux76dcd734eca23168cb008912c0f69ff408905235 baseline version: linux9d84bb40bcb30a7fa16f33baa967aeb9953dda78 Last test of basis 173462 2022-10-07 18:41:45 Z 58 days Failing since173470 2022-10-08 06:21:34 Z 58 days 115 attempts Testing same since 175046 2022-12-05 05:24:16 Z0 days1 attempts 1951 people touched revisions under test, not listing them all jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-xl pass test-amd64-coresched-amd64-xlpass test-arm64-arm64-xl fail test-armhf-armhf-xl fail test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-am
Re: [PATCH 2/6] macio: Make remove callback of macio driver void returned
On Mon, Dec 05, 2022 at 11:36:40PM +0800, Dawei Li wrote:
> Commit fc7a6209d571 ("bus: Make remove callback return
> void") forces bus_type::remove be void-returned, it doesn't
> make much sense for any bus based driver implementing remove
> callbalk to return non-void to its caller.
>
> This change is for macio bus based drivers.
>
> Signed-off-by: Dawei Li
> ---
> arch/powerpc/include/asm/macio.h| 12 ++--
> drivers/ata/pata_macio.c| 4 +---
> drivers/macintosh/rack-meter.c | 4 +---
> drivers/net/ethernet/apple/bmac.c | 4 +---
> drivers/net/ethernet/apple/mace.c | 4 +---
> drivers/net/wireless/intersil/orinoco/airport.c | 4 +---
> drivers/scsi/mac53c94.c | 5 +
> drivers/scsi/mesh.c | 5 +
> drivers/tty/serial/pmac_zilog.c | 7 ++-
> sound/aoa/soundbus/i2sbus/core.c| 4 +---
> 10 files changed, 16 insertions(+), 37 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/macio.h
> b/arch/powerpc/include/asm/macio.h
> index ff5fd82d9ff0..f641c730c3b7 100644
> --- a/arch/powerpc/include/asm/macio.h
> +++ b/arch/powerpc/include/asm/macio.h
> @@ -124,15 +124,15 @@ static inline struct pci_dev *macio_get_pci_dev(struct
> macio_dev *mdev)
> */
> struct macio_driver
> {
> - int (*probe)(struct macio_dev* dev, const struct of_device_id
> *match);
> - int (*remove)(struct macio_dev* dev);
> + int (*probe)(struct macio_dev *dev, const struct of_device_id
> *match);
> + void(*remove)(struct macio_dev *dev);
Again, you are changing lines you do not need to here.
thanks,
greg k-h
Re: [PATCH 6/6] soundbus: make remove callback of soundbus driver void returned
On Mon, Dec 05, 2022 at 11:36:44PM +0800, Dawei Li wrote:
> Since commit fc7a6209d571 ("bus: Make remove callback return
> void") forces bus_type::remove be void-returned, it doesn't
> make much sense for any bus based driver implementing remove
> callbalk to return non-void to its caller.
>
> This change is for soundbus based drivers.
>
> Signed-off-by: Dawei Li
> ---
> sound/aoa/fabrics/layout.c| 3 +--
> sound/aoa/soundbus/soundbus.h | 6 +++---
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
> index ec4ef18555bc..850dc8c53e9b 100644
> --- a/sound/aoa/fabrics/layout.c
> +++ b/sound/aoa/fabrics/layout.c
> @@ -1094,7 +1094,7 @@ static int aoa_fabric_layout_probe(struct soundbus_dev
> *sdev)
> return -ENODEV;
> }
>
> -static int aoa_fabric_layout_remove(struct soundbus_dev *sdev)
> +static void aoa_fabric_layout_remove(struct soundbus_dev *sdev)
> {
> struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
> int i;
> @@ -1123,7 +1123,6 @@ static int aoa_fabric_layout_remove(struct soundbus_dev
> *sdev)
> kfree(ldev);
> sdev->pcmid = -1;
> sdev->pcmname = NULL;
> - return 0;
> }
>
> #ifdef CONFIG_PM_SLEEP
> diff --git a/sound/aoa/soundbus/soundbus.h b/sound/aoa/soundbus/soundbus.h
> index 3a99c1f1a3ca..230dfa1ba270 100644
> --- a/sound/aoa/soundbus/soundbus.h
> +++ b/sound/aoa/soundbus/soundbus.h
> @@ -184,10 +184,10 @@ struct soundbus_driver {
>
> /* we don't implement any matching at all */
>
> - int (*probe)(struct soundbus_dev* dev);
> - int (*remove)(struct soundbus_dev* dev);
> + int (*probe)(struct soundbus_dev *dev);
Why change this line too?
thanks,
greg k-h
Re: [PATCH 5/6] ac97: make remove callback of ac97 driver void returned
On Mon, Dec 05, 2022 at 11:36:43PM +0800, Dawei Li wrote:
> Since commit fc7a6209d571 ("bus: Make remove callback return
> void") forces bus_type::remove be void-returned, it doesn't
> make much sense for any bus based driver implementing remove
> callbalk to return non-void to its caller.
>
> This change is for ac97 bus based drivers.
>
> Signed-off-by: Dawei Li
> ---
> drivers/mfd/wm97xx-core.c | 4 +---
> include/sound/ac97/codec.h | 6 +++---
> sound/ac97/bus.c | 5 ++---
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mfd/wm97xx-core.c b/drivers/mfd/wm97xx-core.c
> index 9a2331eb1bfa..663acbb1854c 100644
> --- a/drivers/mfd/wm97xx-core.c
> +++ b/drivers/mfd/wm97xx-core.c
> @@ -319,13 +319,11 @@ static int wm97xx_ac97_probe(struct ac97_codec_device
> *adev)
> return ret;
> }
>
> -static int wm97xx_ac97_remove(struct ac97_codec_device *adev)
> +static void wm97xx_ac97_remove(struct ac97_codec_device *adev)
> {
> struct wm97xx_priv *wm97xx = ac97_get_drvdata(adev);
>
> snd_ac97_compat_release(wm97xx->ac97);
> -
> - return 0;
> }
>
> static const struct ac97_id wm97xx_ac97_ids[] = {
> diff --git a/include/sound/ac97/codec.h b/include/sound/ac97/codec.h
> index 9792d25fa369..a26e9e0082f6 100644
> --- a/include/sound/ac97/codec.h
> +++ b/include/sound/ac97/codec.h
> @@ -62,9 +62,9 @@ struct ac97_codec_device {
> */
> struct ac97_codec_driver {
> struct device_driverdriver;
> - int (*probe)(struct ac97_codec_device *);
> - int (*remove)(struct ac97_codec_device *);
> - void(*shutdown)(struct ac97_codec_device *);
> + int (*probe)(struct ac97_codec_device *dev);
Why did you change this line?
> + void(*remove)(struct ac97_codec_device *dev);
> + void(*shutdown)(struct ac97_codec_device *dev);
And this line?
Don't change things that you don't describe in your changelog and that
are not needed for your change.
thanks,
greg k-h
Re: [PATCH 4/6] xen: make remove callback of xen driver void returned
On Mon, Dec 05, 2022 at 11:36:42PM +0800, Dawei Li wrote:
> Since commit fc7a6209d571 ("bus: Make remove callback return
> void") forces bus_type::remove be void-returned, it doesn't
> make much sense for any bus based driver implementing remove
> callbalk to return non-void to its caller.
Please wrap changelogs at 72 columns.
And this should go through the maintainers of the Xen bus code, not me,
right?
And why wasn't this attached to the 0/6 email properly? Did you use
different tools? If so, our tools can't find the link to keep them in
sync either :(
thanks,
greg k-h
Re: [PATCH 4/5] multiboot2: parse console= option when setting GOP mode
On 05.12.2022 16:10, Jan Beulich wrote:
> On 23.11.2022 16:45, Roger Pau Monne wrote:
>> @@ -807,7 +822,21 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle,
>> EFI_SYSTEM_TABLE *SystemTable
>>
>> if ( gop )
>> {
>> -gop_mode = efi_find_gop_mode(gop, 0, 0, 0);
>> +const char *opt = get_option(cmdline, "console=");
>> +bool vga = false;
>> +
>> +if ( opt )
>> +{
>> +const char *s = strstr(opt, "vga");
>> +
>> +if ( s && s < strpbrk(opt, " \0"))
>> +vga = true;
>> +}
>
> Don't you also want to find a "vga=gfx-..." option, to avoid ...
Apologies - I should have looked at the title of the next patch
before writing this.
Jan
Re: [PATCH 0/6] Make remove() of any bus based driver void returned
On Mon, Dec 05, 2022 at 11:36:38PM +0800, Dawei Li wrote:
> For bus-based driver, device removal is implemented as:
> device_remove() => bus->remove() => driver->remove()
>
> Driver core needs no feedback from bus driver about the result of
> remove callback. In which case, commit fc7a6209d571 ("bus: Make
> remove callback return void") forces bus_type::remove be void-returned.
>
> Now we have the situation that both 1st & 2nd part of calling chain
> are void returned, so it does not make much sense for the last one
> (driver->remove) to return non-void to its caller.
>
> So the basic idea behind this patchset is making remove() callback of
> any bus-based driver to be void returned.
>
> This patchset includes changes for drivers below:
> 1. hyperv
> 2. macio
> 3. apr
> 4. xen
> 5. ac87
> 6. soundbus
Then that should be 6 different patchsets going to 6 different
subsystems. No need to make this seems like a unified set of patches at
all.
> Q: Why not platform drivers?
> A: Too many of them.(maybe 4K+)
That will have to be done eventually, right?
thanks,
greg k-h
Re: [PATCH v2 3/5] xen/Makefile: remove Cppcheck invocation from the Makefile
On 05.12.2022 16:40, Luca Fancellu wrote: > The script xen-analysis.py is going to be used for the analysis with > cppcheck, so remove the rules from the Makefile > > The python script xen/tools/merge_cppcheck_reports.py was used by the > makefile rules, but its functionality is integrated in the > xen-analysis.py script now, so it can be removed. > > Remove some entry from the .gitignore related to Cppcheck invocation > from Makefile > > Signed-off-by: Luca Fancellu Is this a proper revert of one or more earlier patches? If so, you want to say so, for being potentially relevant for (at least) review purposes. If not, it would also help saying so to clarify what is being kept. Jan
Re: [PATCH 4/6] xen: make remove callback of xen driver void returned
On 05.12.22 16:36, Dawei Li wrote:
Since commit fc7a6209d571 ("bus: Make remove callback return
void") forces bus_type::remove be void-returned, it doesn't
make much sense for any bus based driver implementing remove
callbalk to return non-void to its caller.
This change is for xen bus based drivers.
Signed-off-by: Dawei Li
Acked-by: Juergen Gross
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
[PATCH v2 2/5] xen/scripts: add cppcheck tool to the xen-analysis.py script
Add Cppcheck analysis to the xen-analysis.py script using the arguments --run-cppcheck. Now cppcheck analysis will build Xen while the analysis is performed on the source files, it will produce a text report and an additional html output when the script is called with --cppcheck-html. With this patch cppcheck will benefit of platform configuration files that will help it to understand the target of the compilation and improve the analysis. These are XML files placed in xen/tools/cppcheck-plat/, describing to cppcheck the length of basic types to help it in the analysis. To do so: - Update xen-analysis.py with the code to integrate cppcheck. - add platform configuration files for cppcheck.. - add cppcheck-cc.sh script that is a wrapper for cppcheck and it's used as Xen compiler, it will intercept all flags given from the make build system and it will execute cppcheck on the compiled file together with the file compilation. - guarded hypercall-defs.c with CPPCHECK define because cppcheck gets confused as the file does not contain c code. - add false-positive-cppcheck.json file - update documentation. - update .gitignore Signed-off-by: Luca Fancellu --- Changes in v2: - changes to commit message (Jan) - changes to the cppcheck-cc.sh script and on platform XML files, fixed version handling in cppcheck_analysis.py (Stefano) - Move revert of Makefile and xen/tools/merge_cppcheck_reports.py to the following patch, modified .gitignore to handle the presence of both Makefile invoked cppcheck and this new method (Stefano) --- --- .gitignore| 6 + docs/misra/cppcheck.txt | 27 +- docs/misra/documenting-violations.rst | 7 +- docs/misra/false-positive-cppcheck.json | 12 + docs/misra/xen-static-analysis.rst| 42 ++- xen/include/hypercall-defs.c | 9 + xen/scripts/xen-analysis.py | 18 +- xen/scripts/xen_analysis/cppcheck_analysis.py | 273 ++ .../xen_analysis/cppcheck_report_utils.py | 130 + xen/scripts/xen_analysis/generic_analysis.py | 21 +- xen/scripts/xen_analysis/settings.py | 77 - xen/scripts/xen_analysis/utils.py | 21 +- xen/tools/cppcheck-cc.sh | 223 ++ xen/tools/cppcheck-plat/arm32-wchar_t4.xml| 17 ++ xen/tools/cppcheck-plat/arm64-wchar_t2.xml| 17 ++ xen/tools/cppcheck-plat/x86_64-wchar_t2.xml | 17 ++ 16 files changed, 860 insertions(+), 57 deletions(-) create mode 100644 docs/misra/false-positive-cppcheck.json create mode 100644 xen/scripts/xen_analysis/cppcheck_analysis.py create mode 100644 xen/scripts/xen_analysis/cppcheck_report_utils.py create mode 100755 xen/tools/cppcheck-cc.sh create mode 100644 xen/tools/cppcheck-plat/arm32-wchar_t4.xml create mode 100644 xen/tools/cppcheck-plat/arm64-wchar_t2.xml create mode 100644 xen/tools/cppcheck-plat/x86_64-wchar_t2.xml diff --git a/.gitignore b/.gitignore index f5a66f6194dd..39efe2933a33 100644 --- a/.gitignore +++ b/.gitignore @@ -8,8 +8,11 @@ *.d *.d2 *.c.cppcheck +*.cppcheck.txt +*.cppcheck.xml *.opic *.a +*.c.json *.safparse *.so *.so.[0-9]* @@ -282,9 +285,11 @@ xen/arch/*/efi/efi.h xen/arch/*/efi/pe.c xen/arch/*/efi/runtime.c xen/arch/*/include/asm/asm-offsets.h +xen/build-dir-cppcheck/ xen/common/config_data.S xen/common/config.gz xen/cppcheck-htmlreport/ +xen/cppcheck-report/ xen/cppcheck-misra.* xen/include/headers*.chk xen/include/compat/* @@ -315,6 +320,7 @@ xen/xsm/flask/xenpolicy-* tools/flask/policy/policy.conf tools/flask/policy/xenpolicy-* xen/xen +xen/suppression-list.txt xen/xen-cppcheck.xml xen/xen-syms xen/xen-syms.map diff --git a/docs/misra/cppcheck.txt b/docs/misra/cppcheck.txt index 25d8c3050b72..f7b9f678b4d5 100644 --- a/docs/misra/cppcheck.txt +++ b/docs/misra/cppcheck.txt @@ -3,8 +3,7 @@ Cppcheck for Xen static and MISRA analysis Xen can be analysed for both static analysis problems and MISRA violation using cppcheck, the open source tool allows the creation of a report with all the -findings. Xen has introduced the support in the Makefile so it's very easy to -use and in this document we can see how. +findings. The minimum version required for cppcheck is 2.7. Note that at the time of writing (June 2022), the version 2.8 is known to be broken [1]. @@ -38,27 +37,7 @@ Dependencies are listed in the readme.md of the project repository. Use cppcheck to analyse Xen === -Using cppcheck integration is very simple, it requires few steps: - - 1) Compile Xen - 2) call the cppcheck make target to generate a report in xml format: -make CPPCHECK_MISRA=y cppcheck - 3) call the cppcheck-html make target to generate a report in xml and html -format: -make CPPCHECK_MISRA=y cppcheck-html - -In case the cppcheck binaries are not in the PATH, CPPCHECK and -CPPCHECK_HTMLREPORT variables ca
[PATCH v2 4/5] tools/misra: fix skipped rule numbers
MISRA rules are in the format Rule X.Y, currently the script
convert_misra_doc.py is using two nested loop through range(1,22) to
enumerate rules that needs to be skipped, using combination of X.Y in
that range, however there are two issues in the code:
- rule 22 is never included because the range(1,22) produces a range
in [1..21]
- the second issue is that the code is producing invalid MISRA C 2012
rules, for example 1.21 and so on
Fix the issue using a dictionary that list the rules in misra c2012.
Fixes: 57caa5375321 ("xen: Add MISRA support to cppcheck make rule")
Signed-off-by: Luca Fancellu
Reviewed-by: Stefano Stabellini
---
Changes in v2:
- modified commit message, add R-by (Stefano)
---
---
xen/tools/convert_misra_doc.py | 32 ++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/xen/tools/convert_misra_doc.py b/xen/tools/convert_misra_doc.py
index caa4487f645f..13074d8a2e91 100755
--- a/xen/tools/convert_misra_doc.py
+++ b/xen/tools/convert_misra_doc.py
@@ -14,6 +14,34 @@ Usage:
import sys, getopt, re
+# MISRA rule are identified by two numbers, e.g. Rule 1.2, the main rule number
+# and a sub-number. This dictionary contains the number of the MISRA rule as
key
+# and the maximum sub-number for that rule as value.
+misra_c2012_rules = {
+1:4,
+2:7,
+3:2,
+4:2,
+5:9,
+6:2,
+7:4,
+8:14,
+9:5,
+10:8,
+11:9,
+12:5,
+13:6,
+14:4,
+15:7,
+16:7,
+17:8,
+18:8,
+19:2,
+20:14,
+21:21,
+22:10
+}
+
def main(argv):
infile = ''
outfile = ''
@@ -142,8 +170,8 @@ def main(argv):
skip_list = []
# Search for missing rules and add a dummy text with the rule number
-for i in list(range(1,22)):
-for j in list(range(1,22)):
+for i in misra_c2012_rules:
+for j in list(range(1,misra_c2012_rules[i]+1)):
if str(i) + '.' + str(j) not in rule_list:
outstr.write('Rule ' + str(i) + '.' + str(j) + '\n')
outstr.write('No description for rule ' + str(i) + '.' + str(j)
--
2.17.1
[PATCH v2 1/5] xen/scripts: add xen-analysis.py for coverity and eclair analysis
Add new script for coverity/eclair analysis tool that will enable
the procedure to suppress findings when these tool are used.
The procedure is documented in docs/misra/documenting-violations.rst
and the script is documented in docs/misra/xen-static-analysis.rst.
Add in docs/misra/ the files safe.json and
false-positive-{coverity,eclair}.json that are JSON files containing
the data structures for the justifications, they are used by the
analysis script to link the Xen tags to the proprietary tool comment.
Add docs/misra/documenting-violations.rst to explain how to add
justifications.
Add docs/misra/xen-static-analysis.rst to explain how to use the
script to analyse Xen.
Add analysis artifacts files to .gitignore.
Signed-off-by: Luca Fancellu
---
v2 changes:
- no change
---
---
.gitignore | 1 +
docs/misra/documenting-violations.rst| 191 +++
docs/misra/false-positive-coverity.json | 12 ++
docs/misra/false-positive-eclair.json| 12 ++
docs/misra/safe.json | 11 ++
docs/misra/xen-static-analysis.rst | 54 ++
xen/scripts/xen-analysis.py | 31 +++
xen/scripts/xen_analysis/__init__.py | 0
xen/scripts/xen_analysis/generic_analysis.py | 93 +
xen/scripts/xen_analysis/settings.py | 97 ++
xen/scripts/xen_analysis/tag_database.py | 109 +++
xen/scripts/xen_analysis/utils.py| 37
12 files changed, 648 insertions(+)
create mode 100644 docs/misra/documenting-violations.rst
create mode 100644 docs/misra/false-positive-coverity.json
create mode 100644 docs/misra/false-positive-eclair.json
create mode 100644 docs/misra/safe.json
create mode 100644 docs/misra/xen-static-analysis.rst
create mode 100755 xen/scripts/xen-analysis.py
create mode 100644 xen/scripts/xen_analysis/__init__.py
create mode 100644 xen/scripts/xen_analysis/generic_analysis.py
create mode 100644 xen/scripts/xen_analysis/settings.py
create mode 100644 xen/scripts/xen_analysis/tag_database.py
create mode 100644 xen/scripts/xen_analysis/utils.py
diff --git a/.gitignore b/.gitignore
index ea3243af9dde..f5a66f6194dd 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,6 +10,7 @@
*.c.cppcheck
*.opic
*.a
+*.safparse
*.so
*.so.[0-9]*
*.bin
diff --git a/docs/misra/documenting-violations.rst
b/docs/misra/documenting-violations.rst
new file mode 100644
index ..1d23447556d2
--- /dev/null
+++ b/docs/misra/documenting-violations.rst
@@ -0,0 +1,191 @@
+.. SPDX-License-Identifier: CC-BY-4.0
+
+Documenting violations
+==
+
+Static analysers are used on the Xen codebase for both static analysis and
MISRA
+compliance.
+There might be the need to suppress some findings instead of fixing them and
+many tools permit the usage of in-code comments that suppress findings so that
+they are not shown in the final report.
+
+Xen includes a tool capable of translating a specific comment used in its
+codebase to the right proprietary in-code comment understandable by the
selected
+analyser that suppress its finding.
+
+In the Xen codebase, these tags will be used to document and suppress findings:
+
+ - SAF-X-safe: This tag means that the next line of code contains a finding,
but
+ the non compliance to the checker is analysed and demonstrated to be safe.
+ - SAF-X-false-positive-: This tag means that the next line of code
+ contains a finding, but the finding is a bug of the tool.
+
+SAF stands for Static Analyser Finding, the X is a placeholder for a positive
+number that starts from zero, the number after SAF- shall be incremental and
+unique, base ten notation and without leading zeros.
+
+Entries in the database shall never be removed, even if they are not used
+anymore in the code (if a patch is removing or modifying the faulty line).
+This is to make sure that numbers are not reused which could lead to conflicts
+with old branches or misleading justifications.
+
+An entry can be reused in multiple places in the code to suppress a finding if
+and only if the justification holds for the same non-compliance to the coding
+standard.
+
+An orphan entry, that is an entry who was justifying a finding in the code, but
+later that code was removed and there is no other use of that entry in the
code,
+can be reused as long as the justification for the finding holds. This is done
+to avoid the allocation of a new entry with exactly the same justification,
that
+would lead to waste of space and maintenance issues of the database.
+
+The files where to store all the justifications are in xen/docs/misra/ and are
+named as safe.json and false-positive-.json, they have JSON format, each
+one has a different justification schema which shares some fields.
+
+Here is an example to add a new justification in safe.json::
+
+|{
+|"version": "1.0",
+|"content": [
+|{
+|"id": "SAF-0-safe",
+|"analyse
[PATCH v2 3/5] xen/Makefile: remove Cppcheck invocation from the Makefile
The script xen-analysis.py is going to be used for the analysis with
cppcheck, so remove the rules from the Makefile
The python script xen/tools/merge_cppcheck_reports.py was used by the
makefile rules, but its functionality is integrated in the
xen-analysis.py script now, so it can be removed.
Remove some entry from the .gitignore related to Cppcheck invocation
from Makefile
Signed-off-by: Luca Fancellu
---
v2 changes:
- new patch
---
---
.gitignore | 2 -
xen/Makefile| 116 ++--
xen/tools/merge_cppcheck_reports.py | 86 -
3 files changed, 6 insertions(+), 198 deletions(-)
delete mode 100755 xen/tools/merge_cppcheck_reports.py
diff --git a/.gitignore b/.gitignore
index 39efe2933a33..68566d0c2587 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,7 +7,6 @@
*.o
*.d
*.d2
-*.c.cppcheck
*.cppcheck.txt
*.cppcheck.xml
*.opic
@@ -321,7 +320,6 @@ tools/flask/policy/policy.conf
tools/flask/policy/xenpolicy-*
xen/xen
xen/suppression-list.txt
-xen/xen-cppcheck.xml
xen/xen-syms
xen/xen-syms.map
xen/xen.*
diff --git a/xen/Makefile b/xen/Makefile
index e55fb98e11ee..2d55bb9401f4 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -457,7 +457,7 @@ endif # need-config
__all: build
-main-targets := build install uninstall clean distclean MAP cppcheck
cppcheck-html
+main-targets := build install uninstall clean distclean MAP
.PHONY: $(main-targets)
ifneq ($(XEN_TARGET_ARCH),x86_32)
$(main-targets): %: _% ;
@@ -566,18 +566,16 @@ _clean:
$(Q)$(MAKE) $(clean)=tools/kconfig
find . \( -name "*.o" -o -name ".*.d" -o -name ".*.d2" \
-o -name ".*.o.tmp" -o -name "*~" -o -name "core" \
- -o -name '*.lex.c' -o -name '*.tab.[ch]' -o -name
'*.c.cppcheck' \
- -o -name "*.gcno" -o -name ".*.cmd" -o -name "lib.a" \) -exec
rm -f {} \;
+ -o -name '*.lex.c' -o -name '*.tab.[ch]' -o -name "*.gcno" \
+ -o -name ".*.cmd" -o -name "lib.a" \) -exec rm -f {} \;
rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET)-syms
$(TARGET)-syms.map
rm -f $(TARGET).efi $(TARGET).efi.map $(TARGET).efi.stripped
rm -f asm-offsets.s arch/*/include/asm/asm-offsets.h
rm -f .banner .allconfig.tmp include/xen/compile.h
- rm -f cppcheck-misra.* xen-cppcheck.xml
.PHONY: _distclean
_distclean: clean
rm -f tags TAGS cscope.files cscope.in.out cscope.out cscope.po.out
GTAGS GPATH GRTAGS GSYMS .config source
- rm -rf $(CPPCHECK_HTMLREPORT_OUTDIR)
$(TARGET).gz: $(TARGET)
gzip -n -f -9 < $< > $@.new
@@ -651,111 +649,9 @@ cloc:
done; \
done | cloc --list-file=-
-# What cppcheck command to use.
-# To get proper results, it is recommended to build cppcheck manually from the
-# latest source and use CPPCHECK to give the full path to the built version.
-CPPCHECK ?= cppcheck
-
-# What cppcheck-htmlreport to use.
-# If you give the full path to a self compiled cppcheck, this should be set
-# to the full path to cppcheck-html in the htmlreport directory of cppcheck.
-# On recent distribution, this is available in the standard path.
-CPPCHECK_HTMLREPORT ?= cppcheck-htmlreport
-
-# By default we generate the report in cppcheck-htmlreport directory in the
-# build directory. This can be changed by giving a directory in this variable.
-CPPCHECK_HTMLREPORT_OUTDIR ?= cppcheck-htmlreport
-
-# By default we do not check misra rules, to enable pass "CPPCHECK_MISRA=y" to
-# make command line.
-CPPCHECK_MISRA ?= n
-
-# Compile flags to pass to cppcheck:
-# - include directories and defines Xen Makefile is passing (from CFLAGS)
-# - include config.h as this is passed directly to the compiler.
-# - define CPPCHECK as we use to disable or enable some specific part of the
-# code to solve some cppcheck issues.
-# - explicitely enable some cppcheck checks as we do not want to use "all"
-# which includes unusedFunction which gives wrong positives as we check file
-# per file.
-#
-# Compiler defines are in compiler-def.h which is included in config.h
-#
-CPPCHECKFLAGS := -DCPPCHECK --max-ctu-depth=10 \
- --enable=style,information,missingInclude \
- --include=$(srctree)/include/xen/config.h \
- -I $(srctree)/xsm/flask/include \
- -I $(srctree)/include/xen/libfdt \
- $(filter -D% -I%,$(CFLAGS))
-
-# We need to find all C files (as we are not checking assembly files) so
-# we find all generated .o files which have a .c corresponding file.
-CPPCHECKFILES := $(wildcard $(patsubst $(objtree)/%.o,$(srctree)/%.c, \
- $(filter-out $(objtree)/tools/%, \
- $(shell find $(objtree) -name "*.o"
-
-# Headers and files required to run cppcheck on a file
-CPPCHECKDEPS := $(objtree)/include/generated/autoconf.h \
-$(objtree)/include/generated/compiler-def.h
-
-ifeq ($(CPPCHECK_MISRA),y)
-
[PATCH v2 0/5] Static analyser finding deviation
This serie introduces a way to suppress a static analyser finding providing a proper justification for it. The process is explained in the docs/misra/documenting-violations.rst document that this serie will provide. The tools currently supported are eclair, coverity and cppcheck, but the design is open to support many other static analysis tool. The changes are split between the first two patches to reduce the review effort, the first patch is introducing the deviation process for the eclair and coverity tools, this is because their analysis system is similar. The second patch is introducing the same deviation process for cppcheck, modifying the current way it is called from the makefile and improving its analysis. The third is reverting cppcheck in the makefile. The fourth patch is a fix for a tool used for cppcheck and the fifth patch is an example of how a deviation can be applied for some MISRA findings. --- This serie was pushed as RFC and collected many feedbacks, thank you for the review. In this serie to analyse the codebase, a script is used instead of integrating the process into the makefile. --- Luca Fancellu (5): xen/scripts: add xen-analysis.py for coverity and eclair analysis xen/scripts: add cppcheck tool to the xen-analysis.py script xen/Makefile: remove Cppcheck invocation from the Makefile tools/misra: fix skipped rule numbers xen: Justify linker script defined symbols in include/xen/kernel.h .gitignore| 9 +- docs/misra/cppcheck.txt | 27 +- docs/misra/documenting-violations.rst | 192 docs/misra/false-positive-coverity.json | 12 + docs/misra/false-positive-cppcheck.json | 12 + docs/misra/false-positive-eclair.json | 12 + docs/misra/safe.json | 20 ++ docs/misra/xen-static-analysis.rst| 90 ++ xen/Makefile | 116 +--- xen/include/hypercall-defs.c | 9 + xen/include/xen/kernel.h | 4 + xen/scripts/xen-analysis.py | 45 +++ xen/scripts/xen_analysis/__init__.py | 0 xen/scripts/xen_analysis/cppcheck_analysis.py | 273 ++ .../xen_analysis/cppcheck_report_utils.py | 130 + xen/scripts/xen_analysis/generic_analysis.py | 88 ++ xen/scripts/xen_analysis/settings.py | 152 ++ xen/scripts/xen_analysis/tag_database.py | 109 +++ xen/scripts/xen_analysis/utils.py | 56 xen/tools/convert_misra_doc.py| 32 +- xen/tools/cppcheck-cc.sh | 223 ++ xen/tools/cppcheck-plat/arm32-wchar_t4.xml| 17 ++ xen/tools/cppcheck-plat/arm64-wchar_t2.xml| 17 ++ xen/tools/cppcheck-plat/x86_64-wchar_t2.xml | 17 ++ xen/tools/merge_cppcheck_reports.py | 86 -- 25 files changed, 1524 insertions(+), 224 deletions(-) create mode 100644 docs/misra/documenting-violations.rst create mode 100644 docs/misra/false-positive-coverity.json create mode 100644 docs/misra/false-positive-cppcheck.json create mode 100644 docs/misra/false-positive-eclair.json create mode 100644 docs/misra/safe.json create mode 100644 docs/misra/xen-static-analysis.rst create mode 100755 xen/scripts/xen-analysis.py create mode 100644 xen/scripts/xen_analysis/__init__.py create mode 100644 xen/scripts/xen_analysis/cppcheck_analysis.py create mode 100644 xen/scripts/xen_analysis/cppcheck_report_utils.py create mode 100644 xen/scripts/xen_analysis/generic_analysis.py create mode 100644 xen/scripts/xen_analysis/settings.py create mode 100644 xen/scripts/xen_analysis/tag_database.py create mode 100644 xen/scripts/xen_analysis/utils.py create mode 100755 xen/tools/cppcheck-cc.sh create mode 100644 xen/tools/cppcheck-plat/arm32-wchar_t4.xml create mode 100644 xen/tools/cppcheck-plat/arm64-wchar_t2.xml create mode 100644 xen/tools/cppcheck-plat/x86_64-wchar_t2.xml delete mode 100755 xen/tools/merge_cppcheck_reports.py -- 2.17.1
[PATCH v2 5/5] xen: Justify linker script defined symbols in include/xen/kernel.h
Eclair and Coverity found violation of the MISRA rule 8.6 for the
symbols _start, _end, start, _stext, _etext, _srodata, _erodata,
_sinittext, _einittext which are declared in
xen/include/xen/kernel.h.
All those symbols are defined by the liker script so we can deviate
from the rule 8.6 for these cases.
Signed-off-by: Luca Fancellu
Reviewed-by: Stefano Stabellini
Acked-by: Jan Beulich
---
v2 changes:
- add R-by and Ack-by (Stefano, Jan)
---
---
docs/misra/safe.json | 9 +
xen/include/xen/kernel.h | 4
2 files changed, 13 insertions(+)
diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index e079d3038120..e3c8a1d8eb36 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -3,6 +3,15 @@
"content": [
{
"id": "SAF-0-safe",
+"analyser": {
+"eclair": "MC3R1.R8.6",
+"coverity": "misra_c_2012_rule_8_6_violation"
+},
+"name": "Rule 8.6: linker script defined symbols",
+"text": "It is safe to declare this symbol because it is defined
in the linker script."
+},
+{
+"id": "SAF-1-safe",
"analyser": {},
"name": "Sentinel",
"text": "Next ID to be used"
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 8cd142032d3b..f1a7713784fc 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -65,24 +65,28 @@
1; \
})
+/* SAF-0-safe */
extern char _start[], _end[], start[];
#define is_kernel(p) ({ \
char *__p = (char *)(unsigned long)(p); \
(__p >= _start) && (__p < _end);\
})
+/* SAF-0-safe */
extern char _stext[], _etext[];
#define is_kernel_text(p) ({\
char *__p = (char *)(unsigned long)(p); \
(__p >= _stext) && (__p < _etext); \
})
+/* SAF-0-safe */
extern const char _srodata[], _erodata[];
#define is_kernel_rodata(p) ({ \
const char *__p = (const char *)(unsigned long)(p); \
(__p >= _srodata) && (__p < _erodata); \
})
+/* SAF-0-safe */
extern char _sinittext[], _einittext[];
#define is_kernel_inittext(p) ({\
char *__p = (char *)(unsigned long)(p); \
--
2.17.1
[PATCH 2/6] macio: Make remove callback of macio driver void returned
Commit fc7a6209d571 ("bus: Make remove callback return
void") forces bus_type::remove be void-returned, it doesn't
make much sense for any bus based driver implementing remove
callbalk to return non-void to its caller.
This change is for macio bus based drivers.
Signed-off-by: Dawei Li
---
arch/powerpc/include/asm/macio.h| 12 ++--
drivers/ata/pata_macio.c| 4 +---
drivers/macintosh/rack-meter.c | 4 +---
drivers/net/ethernet/apple/bmac.c | 4 +---
drivers/net/ethernet/apple/mace.c | 4 +---
drivers/net/wireless/intersil/orinoco/airport.c | 4 +---
drivers/scsi/mac53c94.c | 5 +
drivers/scsi/mesh.c | 5 +
drivers/tty/serial/pmac_zilog.c | 7 ++-
sound/aoa/soundbus/i2sbus/core.c| 4 +---
10 files changed, 16 insertions(+), 37 deletions(-)
diff --git a/arch/powerpc/include/asm/macio.h b/arch/powerpc/include/asm/macio.h
index ff5fd82d9ff0..f641c730c3b7 100644
--- a/arch/powerpc/include/asm/macio.h
+++ b/arch/powerpc/include/asm/macio.h
@@ -124,15 +124,15 @@ static inline struct pci_dev *macio_get_pci_dev(struct
macio_dev *mdev)
*/
struct macio_driver
{
- int (*probe)(struct macio_dev* dev, const struct of_device_id
*match);
- int (*remove)(struct macio_dev* dev);
+ int (*probe)(struct macio_dev *dev, const struct of_device_id
*match);
+ void(*remove)(struct macio_dev *dev);
- int (*suspend)(struct macio_dev* dev, pm_message_t state);
- int (*resume)(struct macio_dev* dev);
- int (*shutdown)(struct macio_dev* dev);
+ int (*suspend)(struct macio_dev *dev, pm_message_t state);
+ int (*resume)(struct macio_dev *dev);
+ int (*shutdown)(struct macio_dev *dev);
#ifdef CONFIG_PMAC_MEDIABAY
- void(*mediabay_event)(struct macio_dev* dev, int mb_state);
+ void(*mediabay_event)(struct macio_dev *dev, int mb_state);
#endif
struct device_driverdriver;
};
diff --git a/drivers/ata/pata_macio.c b/drivers/ata/pata_macio.c
index 9ccaac9e2bc3..653106716a4b 100644
--- a/drivers/ata/pata_macio.c
+++ b/drivers/ata/pata_macio.c
@@ -1187,7 +1187,7 @@ static int pata_macio_attach(struct macio_dev *mdev,
return rc;
}
-static int pata_macio_detach(struct macio_dev *mdev)
+static void pata_macio_detach(struct macio_dev *mdev)
{
struct ata_host *host = macio_get_drvdata(mdev);
struct pata_macio_priv *priv = host->private_data;
@@ -1202,8 +1202,6 @@ static int pata_macio_detach(struct macio_dev *mdev)
ata_host_detach(host);
unlock_media_bay(priv->mdev->media_bay);
-
- return 0;
}
#ifdef CONFIG_PM_SLEEP
diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index c28893e41a8b..f2f83c4f3af5 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -523,7 +523,7 @@ static int rackmeter_probe(struct macio_dev* mdev,
return rc;
}
-static int rackmeter_remove(struct macio_dev* mdev)
+static void rackmeter_remove(struct macio_dev *mdev)
{
struct rackmeter *rm = dev_get_drvdata(&mdev->ofdev.dev);
@@ -558,8 +558,6 @@ static int rackmeter_remove(struct macio_dev* mdev)
/* Get rid of me */
kfree(rm);
-
- return 0;
}
static int rackmeter_shutdown(struct macio_dev* mdev)
diff --git a/drivers/net/ethernet/apple/bmac.c
b/drivers/net/ethernet/apple/bmac.c
index 334de0d93c89..8b37ac1cc3d1 100644
--- a/drivers/net/ethernet/apple/bmac.c
+++ b/drivers/net/ethernet/apple/bmac.c
@@ -1591,7 +1591,7 @@ bmac_proc_info(char *buffer, char **start, off_t offset,
int length)
}
#endif
-static int bmac_remove(struct macio_dev *mdev)
+static void bmac_remove(struct macio_dev *mdev)
{
struct net_device *dev = macio_get_drvdata(mdev);
struct bmac_data *bp = netdev_priv(dev);
@@ -1609,8 +1609,6 @@ static int bmac_remove(struct macio_dev *mdev)
macio_release_resources(mdev);
free_netdev(dev);
-
- return 0;
}
static const struct of_device_id bmac_match[] =
diff --git a/drivers/net/ethernet/apple/mace.c
b/drivers/net/ethernet/apple/mace.c
index d0a771b65e88..b214524a5964 100644
--- a/drivers/net/ethernet/apple/mace.c
+++ b/drivers/net/ethernet/apple/mace.c
@@ -272,7 +272,7 @@ static int mace_probe(struct macio_dev *mdev, const struct
of_device_id *match)
return rc;
}
-static int mace_remove(struct macio_dev *mdev)
+static void mace_remove(struct macio_dev *mdev)
{
struct net_device *dev = macio_get_drvdata(mdev);
struct mace_data *mp;
@@ -296,8 +296,6 @@ static int mace_remove(struct macio_dev *mdev)
free_netdev(dev);
macio_release_resources(mdev);
-
- return 0;
}
static void dbdma_reset(volatile struct dbdma_regs __iomem *dma)
diff --git a/drivers/net/wireless/intersil/orinoc
[PATCH 0/6] Make remove() of any bus based driver void returned
For bus-based driver, device removal is implemented as:
device_remove() => bus->remove() => driver->remove()
Driver core needs no feedback from bus driver about the result of
remove callback. In which case, commit fc7a6209d571 ("bus: Make
remove callback return void") forces bus_type::remove be void-returned.
Now we have the situation that both 1st & 2nd part of calling chain
are void returned, so it does not make much sense for the last one
(driver->remove) to return non-void to its caller.
So the basic idea behind this patchset is making remove() callback of
any bus-based driver to be void returned.
This patchset includes changes for drivers below:
1. hyperv
2. macio
3. apr
4. xen
5. ac87
6. soundbus
Q: Why not platform drivers?
A: Too many of them.(maybe 4K+)
Dawei Li (6):
hyperv: Make remove callback of hyperv driver void returned
macio: Make remove callback of macio driver void returned
apr: make remove callback of apr driver void returned
xen: make remove callback of xen driver void returned
ac97: make remove callback of ac97 driver void returned
soundbus: make remove callback of soundbus driver void returned
arch/powerpc/include/asm/macio.h| 12 ++--
drivers/ata/pata_macio.c| 4 +---
drivers/block/xen-blkback/xenbus.c | 4 +---
drivers/block/xen-blkfront.c| 3 +--
drivers/char/tpm/xen-tpmfront.c | 3 +--
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 +---
drivers/gpu/drm/xen/xen_drm_front.c | 3 +--
drivers/hid/hid-hyperv.c| 4 +---
drivers/hv/hv_balloon.c | 5 +
drivers/hv/hv_util.c| 4 +---
drivers/input/misc/xen-kbdfront.c | 5 ++---
drivers/input/serio/hyperv-keyboard.c | 4 +---
drivers/macintosh/rack-meter.c | 4 +---
drivers/mfd/wm97xx-core.c | 4 +---
drivers/net/ethernet/apple/bmac.c | 4 +---
drivers/net/ethernet/apple/mace.c | 4 +---
drivers/net/hyperv/netvsc_drv.c | 4 +---
drivers/net/wireless/intersil/orinoco/airport.c | 4 +---
drivers/net/xen-netback/xenbus.c| 3 +--
drivers/net/xen-netfront.c | 4 +---
drivers/pci/controller/pci-hyperv.c | 3 +--
drivers/pci/xen-pcifront.c | 4 +---
drivers/scsi/mac53c94.c | 5 +
drivers/scsi/mesh.c | 5 +
drivers/scsi/storvsc_drv.c | 4 +---
drivers/scsi/xen-scsifront.c| 4 +---
drivers/tty/hvc/hvc_xen.c | 4 ++--
drivers/tty/serial/pmac_zilog.c | 7 ++-
drivers/uio/uio_hv_generic.c| 5 ++---
drivers/usb/host/xen-hcd.c | 4 +---
drivers/video/fbdev/hyperv_fb.c | 5 +
drivers/video/fbdev/xen-fbfront.c | 6 ++
drivers/xen/pvcalls-back.c | 3 +--
drivers/xen/pvcalls-front.c | 3 +--
drivers/xen/xen-pciback/xenbus.c| 4 +---
drivers/xen/xen-scsiback.c | 4 +---
include/linux/hyperv.h | 2 +-
include/linux/soc/qcom/apr.h| 2 +-
include/sound/ac97/codec.h | 6 +++---
include/xen/xenbus.h| 2 +-
net/9p/trans_xen.c | 3 +--
net/vmw_vsock/hyperv_transport.c| 4 +---
sound/ac97/bus.c| 5 ++---
sound/aoa/fabrics/layout.c | 3 +--
sound/aoa/soundbus/i2sbus/core.c| 4 +---
sound/aoa/soundbus/soundbus.h | 6 +++---
sound/soc/qcom/qdsp6/q6core.c | 4 +---
sound/xen/xen_snd_front.c | 3 +--
48 files changed, 63 insertions(+), 137 deletions(-)
--
2.25.1
[PATCH 3/6] apr: make remove callback of apr driver void returned
Since commit fc7a6209d571 ("bus: Make remove callback return
void") forces bus_type::remove be void-returned, it doesn't
make much sense for any bus based driver implementing remove
callbalk to return non-void to its caller.
This change is for apr bus based drivers.
Signed-off-by: Dawei Li
---
include/linux/soc/qcom/apr.h | 2 +-
sound/soc/qcom/qdsp6/q6core.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/linux/soc/qcom/apr.h b/include/linux/soc/qcom/apr.h
index 23c5b30f3511..be98aebcb3e1 100644
--- a/include/linux/soc/qcom/apr.h
+++ b/include/linux/soc/qcom/apr.h
@@ -153,7 +153,7 @@ typedef struct apr_device gpr_device_t;
struct apr_driver {
int (*probe)(struct apr_device *sl);
- int (*remove)(struct apr_device *sl);
+ void(*remove)(struct apr_device *sl);
int (*callback)(struct apr_device *a,
struct apr_resp_pkt *d);
int (*gpr_callback)(struct gpr_resp_pkt *d, void *data, int op);
diff --git a/sound/soc/qcom/qdsp6/q6core.c b/sound/soc/qcom/qdsp6/q6core.c
index 5358fefd4210..49cfb32cd209 100644
--- a/sound/soc/qcom/qdsp6/q6core.c
+++ b/sound/soc/qcom/qdsp6/q6core.c
@@ -339,7 +339,7 @@ static int q6core_probe(struct apr_device *adev)
return 0;
}
-static int q6core_exit(struct apr_device *adev)
+static void q6core_exit(struct apr_device *adev)
{
struct q6core *core = dev_get_drvdata(&adev->dev);
@@ -350,8 +350,6 @@ static int q6core_exit(struct apr_device *adev)
g_core = NULL;
kfree(core);
-
- return 0;
}
#ifdef CONFIG_OF
--
2.25.1
[PATCH 6/6] soundbus: make remove callback of soundbus driver void returned
Since commit fc7a6209d571 ("bus: Make remove callback return
void") forces bus_type::remove be void-returned, it doesn't
make much sense for any bus based driver implementing remove
callbalk to return non-void to its caller.
This change is for soundbus based drivers.
Signed-off-by: Dawei Li
---
sound/aoa/fabrics/layout.c| 3 +--
sound/aoa/soundbus/soundbus.h | 6 +++---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
index ec4ef18555bc..850dc8c53e9b 100644
--- a/sound/aoa/fabrics/layout.c
+++ b/sound/aoa/fabrics/layout.c
@@ -1094,7 +1094,7 @@ static int aoa_fabric_layout_probe(struct soundbus_dev
*sdev)
return -ENODEV;
}
-static int aoa_fabric_layout_remove(struct soundbus_dev *sdev)
+static void aoa_fabric_layout_remove(struct soundbus_dev *sdev)
{
struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
int i;
@@ -1123,7 +1123,6 @@ static int aoa_fabric_layout_remove(struct soundbus_dev
*sdev)
kfree(ldev);
sdev->pcmid = -1;
sdev->pcmname = NULL;
- return 0;
}
#ifdef CONFIG_PM_SLEEP
diff --git a/sound/aoa/soundbus/soundbus.h b/sound/aoa/soundbus/soundbus.h
index 3a99c1f1a3ca..230dfa1ba270 100644
--- a/sound/aoa/soundbus/soundbus.h
+++ b/sound/aoa/soundbus/soundbus.h
@@ -184,10 +184,10 @@ struct soundbus_driver {
/* we don't implement any matching at all */
- int (*probe)(struct soundbus_dev* dev);
- int (*remove)(struct soundbus_dev* dev);
+ int (*probe)(struct soundbus_dev *dev);
+ void(*remove)(struct soundbus_dev *dev);
- int (*shutdown)(struct soundbus_dev* dev);
+ int (*shutdown)(struct soundbus_dev *dev);
struct device_driver driver;
};
--
2.25.1
[PATCH 1/6] hyperv: Make remove callback of hyperv driver void returned
Since commit fc7a6209d571 ("bus: Make remove callback return
void") forces bus_type::remove be void-returned, it doesn't
make much sense for any bus based driver implementing remove
callbalk to return non-void to its caller.
This change is for hyperv bus based drivers.
Signed-off-by: Dawei Li
---
drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 4 +---
drivers/hid/hid-hyperv.c| 4 +---
drivers/hv/hv_balloon.c | 5 +
drivers/hv/hv_util.c| 4 +---
drivers/input/serio/hyperv-keyboard.c | 4 +---
drivers/net/hyperv/netvsc_drv.c | 4 +---
drivers/pci/controller/pci-hyperv.c | 3 +--
drivers/scsi/storvsc_drv.c | 4 +---
drivers/uio/uio_hv_generic.c| 5 ++---
drivers/video/fbdev/hyperv_fb.c | 5 +
include/linux/hyperv.h | 2 +-
net/vmw_vsock/hyperv_transport.c| 4 +---
12 files changed, 13 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index ca127ff797f7..d117fff26d99 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -165,7 +165,7 @@ static int hyperv_vmbus_probe(struct hv_device *hdev,
return ret;
}
-static int hyperv_vmbus_remove(struct hv_device *hdev)
+static void hyperv_vmbus_remove(struct hv_device *hdev)
{
struct drm_device *dev = hv_get_drvdata(hdev);
struct hyperv_drm_device *hv = to_hv(dev);
@@ -176,8 +176,6 @@ static int hyperv_vmbus_remove(struct hv_device *hdev)
hv_set_drvdata(hdev, NULL);
vmbus_free_mmio(hv->mem->start, hv->fb_size);
-
- return 0;
}
static int hyperv_vmbus_suspend(struct hv_device *hdev)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index ab57b49a44ed..ef16c2a54362 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -535,7 +535,7 @@ static int mousevsc_probe(struct hv_device *device,
}
-static int mousevsc_remove(struct hv_device *dev)
+static void mousevsc_remove(struct hv_device *dev)
{
struct mousevsc_dev *input_dev = hv_get_drvdata(dev);
@@ -544,8 +544,6 @@ static int mousevsc_remove(struct hv_device *dev)
hid_hw_stop(input_dev->hid_device);
hid_destroy_device(input_dev->hid_device);
mousevsc_free_device(input_dev);
-
- return 0;
}
static int mousevsc_suspend(struct hv_device *dev)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 6c127f061f06..6bbd2e6fa3d4 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1990,7 +1990,7 @@ static int balloon_probe(struct hv_device *dev,
return ret;
}
-static int balloon_remove(struct hv_device *dev)
+static void balloon_remove(struct hv_device *dev)
{
struct hv_dynmem_device *dm = hv_get_drvdata(dev);
struct hv_hotadd_state *has, *tmp;
@@ -2031,8 +2031,6 @@ static int balloon_remove(struct hv_device *dev)
kfree(has);
}
spin_unlock_irqrestore(&dm_device.ha_lock, flags);
-
- return 0;
}
static int balloon_suspend(struct hv_device *hv_dev)
@@ -2112,7 +2110,6 @@ static struct hv_driver balloon_drv = {
static int __init init_balloon_drv(void)
{
-
return vmbus_driver_register(&balloon_drv);
}
diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
index 835e6039c186..24995ac41c86 100644
--- a/drivers/hv/hv_util.c
+++ b/drivers/hv/hv_util.c
@@ -602,7 +602,7 @@ static int util_probe(struct hv_device *dev,
return ret;
}
-static int util_remove(struct hv_device *dev)
+static void util_remove(struct hv_device *dev)
{
struct hv_util_service *srv = hv_get_drvdata(dev);
@@ -610,8 +610,6 @@ static int util_remove(struct hv_device *dev)
srv->util_deinit();
vmbus_close(dev->channel);
kfree(srv->recv_buffer);
-
- return 0;
}
/*
diff --git a/drivers/input/serio/hyperv-keyboard.c
b/drivers/input/serio/hyperv-keyboard.c
index d62aefb2e245..31def6ce5157 100644
--- a/drivers/input/serio/hyperv-keyboard.c
+++ b/drivers/input/serio/hyperv-keyboard.c
@@ -369,7 +369,7 @@ static int hv_kbd_probe(struct hv_device *hv_dev,
return error;
}
-static int hv_kbd_remove(struct hv_device *hv_dev)
+static void hv_kbd_remove(struct hv_device *hv_dev)
{
struct hv_kbd_dev *kbd_dev = hv_get_drvdata(hv_dev);
@@ -378,8 +378,6 @@ static int hv_kbd_remove(struct hv_device *hv_dev)
kfree(kbd_dev);
hv_set_drvdata(hv_dev, NULL);
-
- return 0;
}
static int hv_kbd_suspend(struct hv_device *hv_dev)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 89eb4f179a3c..50c20e4d4147 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2594,7 +2594,7 @@ static int netvsc_probe(struct hv_device *dev,
return ret;
}
-static int netvsc_remove(struct hv_device *dev)
+static void netvsc_remove(st
[PATCH 5/6] ac97: make remove callback of ac97 driver void returned
Since commit fc7a6209d571 ("bus: Make remove callback return
void") forces bus_type::remove be void-returned, it doesn't
make much sense for any bus based driver implementing remove
callbalk to return non-void to its caller.
This change is for ac97 bus based drivers.
Signed-off-by: Dawei Li
---
drivers/mfd/wm97xx-core.c | 4 +---
include/sound/ac97/codec.h | 6 +++---
sound/ac97/bus.c | 5 ++---
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/wm97xx-core.c b/drivers/mfd/wm97xx-core.c
index 9a2331eb1bfa..663acbb1854c 100644
--- a/drivers/mfd/wm97xx-core.c
+++ b/drivers/mfd/wm97xx-core.c
@@ -319,13 +319,11 @@ static int wm97xx_ac97_probe(struct ac97_codec_device
*adev)
return ret;
}
-static int wm97xx_ac97_remove(struct ac97_codec_device *adev)
+static void wm97xx_ac97_remove(struct ac97_codec_device *adev)
{
struct wm97xx_priv *wm97xx = ac97_get_drvdata(adev);
snd_ac97_compat_release(wm97xx->ac97);
-
- return 0;
}
static const struct ac97_id wm97xx_ac97_ids[] = {
diff --git a/include/sound/ac97/codec.h b/include/sound/ac97/codec.h
index 9792d25fa369..a26e9e0082f6 100644
--- a/include/sound/ac97/codec.h
+++ b/include/sound/ac97/codec.h
@@ -62,9 +62,9 @@ struct ac97_codec_device {
*/
struct ac97_codec_driver {
struct device_driverdriver;
- int (*probe)(struct ac97_codec_device *);
- int (*remove)(struct ac97_codec_device *);
- void(*shutdown)(struct ac97_codec_device *);
+ int (*probe)(struct ac97_codec_device *dev);
+ void(*remove)(struct ac97_codec_device *dev);
+ void(*shutdown)(struct ac97_codec_device *dev);
const struct ac97_id*id_table;
};
diff --git a/sound/ac97/bus.c b/sound/ac97/bus.c
index 045330883a96..6067c04ce4c0 100644
--- a/sound/ac97/bus.c
+++ b/sound/ac97/bus.c
@@ -524,10 +524,9 @@ static void ac97_bus_remove(struct device *dev)
if (ret < 0)
return;
- ret = adrv->remove(adev);
+ adrv->remove(adev);
pm_runtime_put_noidle(dev);
- if (ret == 0)
- ac97_put_disable_clk(adev);
+ ac97_put_disable_clk(adev);
pm_runtime_disable(dev);
}
--
2.25.1
[PATCH 4/6] xen: make remove callback of xen driver void returned
Since commit fc7a6209d571 ("bus: Make remove callback return
void") forces bus_type::remove be void-returned, it doesn't
make much sense for any bus based driver implementing remove
callbalk to return non-void to its caller.
This change is for xen bus based drivers.
Signed-off-by: Dawei Li
---
drivers/block/xen-blkback/xenbus.c | 4 +---
drivers/block/xen-blkfront.c| 3 +--
drivers/char/tpm/xen-tpmfront.c | 3 +--
drivers/gpu/drm/xen/xen_drm_front.c | 3 +--
drivers/input/misc/xen-kbdfront.c | 5 ++---
drivers/net/xen-netback/xenbus.c| 3 +--
drivers/net/xen-netfront.c | 4 +---
drivers/pci/xen-pcifront.c | 4 +---
drivers/scsi/xen-scsifront.c| 4 +---
drivers/tty/hvc/hvc_xen.c | 4 ++--
drivers/usb/host/xen-hcd.c | 4 +---
drivers/video/fbdev/xen-fbfront.c | 6 ++
drivers/xen/pvcalls-back.c | 3 +--
drivers/xen/pvcalls-front.c | 3 +--
drivers/xen/xen-pciback/xenbus.c| 4 +---
drivers/xen/xen-scsiback.c | 4 +---
include/xen/xenbus.h| 2 +-
net/9p/trans_xen.c | 3 +--
sound/xen/xen_snd_front.c | 3 +--
19 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/drivers/block/xen-blkback/xenbus.c
b/drivers/block/xen-blkback/xenbus.c
index c0227dfa4688..4807af1d5805 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -524,7 +524,7 @@ static int xen_vbd_create(struct xen_blkif *blkif,
blkif_vdev_t handle,
return 0;
}
-static int xen_blkbk_remove(struct xenbus_device *dev)
+static void xen_blkbk_remove(struct xenbus_device *dev)
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
@@ -547,8 +547,6 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
/* Put the reference we set in xen_blkif_alloc(). */
xen_blkif_put(be->blkif);
}
-
- return 0;
}
int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt,
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 35b9bcad9db9..e68576ded7cb 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2468,7 +2468,7 @@ static void blkback_changed(struct xenbus_device *dev,
}
}
-static int blkfront_remove(struct xenbus_device *xbdev)
+static void blkfront_remove(struct xenbus_device *xbdev)
{
struct blkfront_info *info = dev_get_drvdata(&xbdev->dev);
@@ -2489,7 +2489,6 @@ static int blkfront_remove(struct xenbus_device *xbdev)
}
kfree(info);
- return 0;
}
static int blkfront_is_ready(struct xenbus_device *dev)
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index 379291826261..80cca3b83b22 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -360,14 +360,13 @@ static int tpmfront_probe(struct xenbus_device *dev,
return tpm_chip_register(priv->chip);
}
-static int tpmfront_remove(struct xenbus_device *dev)
+static void tpmfront_remove(struct xenbus_device *dev)
{
struct tpm_chip *chip = dev_get_drvdata(&dev->dev);
struct tpm_private *priv = dev_get_drvdata(&chip->dev);
tpm_chip_unregister(chip);
ring_free(priv);
dev_set_drvdata(&chip->dev, NULL);
- return 0;
}
static int tpmfront_resume(struct xenbus_device *dev)
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
b/drivers/gpu/drm/xen/xen_drm_front.c
index 0d8e6bd1ccbf..90996c108146 100644
--- a/drivers/gpu/drm/xen/xen_drm_front.c
+++ b/drivers/gpu/drm/xen/xen_drm_front.c
@@ -717,7 +717,7 @@ static int xen_drv_probe(struct xenbus_device *xb_dev,
return xenbus_switch_state(xb_dev, XenbusStateInitialising);
}
-static int xen_drv_remove(struct xenbus_device *dev)
+static void xen_drv_remove(struct xenbus_device *dev)
{
struct xen_drm_front_info *front_info = dev_get_drvdata(&dev->dev);
int to = 100;
@@ -751,7 +751,6 @@ static int xen_drv_remove(struct xenbus_device *dev)
xen_drm_drv_fini(front_info);
xenbus_frontend_closed(dev);
- return 0;
}
static const struct xenbus_device_id xen_driver_ids[] = {
diff --git a/drivers/input/misc/xen-kbdfront.c
b/drivers/input/misc/xen-kbdfront.c
index 8d8ebdc2039b..67f1c7364c95 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -51,7 +51,7 @@ module_param_array(ptr_size, int, NULL, 0444);
MODULE_PARM_DESC(ptr_size,
"Pointing device width, height in pixels (default 800,600)");
-static int xenkbd_remove(struct xenbus_device *);
+static void xenkbd_remove(struct xenbus_device *);
static int xenkbd_connect_backend(struct xenbus_device *, struct xenkbd_info
*);
static void xenkbd_disconnect_backend(struct xenkbd_info *);
@@ -404,7 +404,7 @@ static int xenkbd_resume(struct xenbus_device *dev)
return xenkbd_connect_backend(dev, info);
}
-static int xenkbd_remove(struct xenbus_device *dev
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
On 05/12/2022 14:25, Michal Orzel wrote:
diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index 1528ced509..33d32835e7 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -297,10 +297,14 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
#define XEN_DOMCTL_CONFIG_TEE_NONE 0
#define XEN_DOMCTL_CONFIG_TEE_OPTEE 1
+#define XEN_DOMCTL_CONFIG_VIOMMU_NONE 0
+
struct xen_arch_domainconfig {
/* IN/OUT */
uint8_t gic_version;
/* IN */
+uint8_t viommu_type;
this should be uint16_t and not uint8_t
I will modify the in viommu_type to uint8_t in "arch/arm/include/asm/viommu.h"
and will
also fix everywhere the viommu_type to uint8_t.
Also I think that you need to bump XEN_DOMCTL_INTERFACE_VERSION due to the
change
in struct xen_arch_domainconfig.
We only need to bump the domctl version once per release. So if this is
the first modification of domctl.h in 4.18 then yes.
That said, I am not sure whether this is necessary here as you are using
a padding.
@Rahul, BTW, I think you may need to regenerate the bindings for OCaml
and Go.
Cheers,
--
Julien Grall
Re: [PATCH 4/5] multiboot2: parse console= option when setting GOP mode
On 23.11.2022 16:45, Roger Pau Monne wrote:
> Only set the GOP mode if vga is selected in the console option,
> otherwise just fetch the information from the current mode in order to
> make it available to dom0.
>
> Introduce support for passing the command line to the efi_multiboot2()
> helper, and parse the console= option if present.
>
> Signed-off-by: Roger Pau Monné
> ---
> I'm unsure why the parsing of the multiboot2 tags is done in assembly,
> it could very well be done in efi_multiboot2() in C, but I don't want
> to switch that code now.
I guess that's mainly mirroring the non-EFI boot path, where the amount
of work needed to eventually enter C land is quite a bit larger?
Anything beyond that Daniel may want to point out.
> @@ -265,6 +266,15 @@ __efi64_mb2_start:
> cmpl$MULTIBOOT2_TAG_TYPE_END,MB2_tag_type(%rcx)
> je .Lrun_bs
>
> +/*
> + * Get command line from Multiboot2 information.
> + * Must be last parsed tag.
Why? And how do you guarantee this?
> + */
> +cmpl$MULTIBOOT2_TAG_TYPE_CMDLINE,MB2_tag_type(%rcx)
> +jne .Lefi_mb2_next_tag
> +mov %rcx,%rdx
> +add $(MB2_tag_string),%rdx
Simply "lea MB2_tag_string(%rcx),%rdx"?
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -786,7 +786,22 @@ static bool __init
> efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
>
> static void __init efi_arch_flush_dcache_area(const void *vaddr, UINTN size)
> { }
>
> -void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable)
> +/* Return the last occurrence of opt in cmd. */
Is this sufficient in the general case (it may be for "console=", but
perhaps not for "vga=", which may also need finding as per below)?
> +static const char __init *get_option(const char *cmd, const char *opt)
Nit: The first * wants to move earlier.
> +{
> +const char *s = cmd, *o = NULL;
> +
> +while ( (s = strstr(s, opt)) != NULL )
I'm afraid this is too easy to break without considering separators as
well. If I'm not mistaken you'd also match e.g. "sync_console=1" for
the sole present caller.
> +{
> +s += strlen(opt);
> +o = s;
> +}
> +
> +return o;
> +}
> +
> +void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable,
> + const char *cmdline)
> {
> EFI_GRAPHICS_OUTPUT_PROTOCOL *gop;
> EFI_HANDLE gop_handle;
> @@ -807,7 +822,21 @@ void __init efi_multiboot2(EFI_HANDLE ImageHandle,
> EFI_SYSTEM_TABLE *SystemTable
>
> if ( gop )
> {
> -gop_mode = efi_find_gop_mode(gop, 0, 0, 0);
> +const char *opt = get_option(cmdline, "console=");
> +bool vga = false;
> +
> +if ( opt )
> +{
> +const char *s = strstr(opt, "vga");
> +
> +if ( s && s < strpbrk(opt, " \0"))
> +vga = true;
> +}
Don't you also want to find a "vga=gfx-..." option, to avoid ...
> +if ( vga )
> +{
> +gop_mode = efi_find_gop_mode(gop, 0, 0, 0);
... requesting a "random" mode here?
> +}
Nit: No need for the braces in cases like this one.
Jan
Re: [RFC PATCH 08/21] xen/arm: vsmmuv3: Add support for registers emulation
Hi Julien,
> On 3 Dec 2022, at 9:16 pm, Julien Grall wrote:
>
> Hi Rahul,
>
> I have only skimmed through the patch so far.
>
> On 01/12/2022 16:02, Rahul Singh wrote:
>> static int vsmmuv3_mmio_write(struct vcpu *v, mmio_info_t *info,
>>register_t r, void *priv)
>> {
>> +struct virt_smmu *smmu = priv;
>> +uint64_t reg;
>> +uint32_t reg32;
>> +
>> +switch ( info->gpa & 0x )
>> +{
>> +case VREG32(ARM_SMMU_CR0):
>
>
> Shouldn't this code (and all the other register emulations) be protected for
> concurrent access in some way?
Yes, I agree I will add the lock for register emulations in next v2.
>
>
>> +reg32 = smmu->cr[0];
>> +vreg_reg32_update(®32, r, info);
>> +smmu->cr[0] = reg32;
>> +smmu->cr0ack = reg32 & ~CR0_RESERVED;
>
> Looking at the use. I think it doesn't look necessary to have a copy of cr0
> with just the reserved bit(s) unset. Instead, it would be better to clear the
> bit(s) when reading it.
Ack.
Regards,
Rahul
[XEN v5 07/11] xen/Arm: GICv3: Define ICH_LR_EL2 on AArch32
Refer "Arm IHI 0069H ID020922", 12.4.6, Interrupt Controller List Registers
AArch64 System register ICH_LR_EL2 bits [31:0] are architecturally
mapped to AArch32 System register ICH_LR[31:0].
AArch64 System register ICH_LR_EL2 bits [63:32] are architecturally
mapped to AArch32 System register ICH_LRC[31:0].
Defined ICH_LR<0...15>_EL2 and ICH_LRC<0...15>_EL2 for AArch32.
For AArch32, the link register is stored as :-
(((uint64_t) ICH_LRC<0...15>_EL2) << 32) | ICH_LR<0...15>_EL2
Also, ICR_LR macros need to be modified as ULL is 64 bits for AArch32 and
AArch64.
Signed-off-by: Ayan Kumar Halder
---
Changes from :-
v1 - 1. Moved the coproc register definitions to asm/cpregs.h.
2. Use GENMASK(31, 0) to represent 0x
3. Use READ_CP32()/WRITE_CP32() instead of READ_SYSREG()/WRITE_SYSREG().
4. Multi-line macro definitions should be enclosed within ({ }).
v2 - 1. Use WRITE_SYSREG_LR(V, R) to make it consistent with before.
2. Defined the register alias.
3. Style issues.
v3 - 1. Addressed style issues.
v4 - 1. Replaces ___CP32(foo) with foo.
2. Removed the definition of ___CP32().
xen/arch/arm/gic-v3.c| 132 +++
xen/arch/arm/include/asm/arm32/sysregs.h | 19
xen/arch/arm/include/asm/arm64/sysregs.h | 5 +
xen/arch/arm/include/asm/cpregs.h| 74 +
xen/arch/arm/include/asm/gic_v3_defs.h | 8 +-
5 files changed, 168 insertions(+), 70 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 64a76307dd..6457e7033c 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -73,37 +73,37 @@ static inline void gicv3_save_lrs(struct vcpu *v)
switch ( gicv3_info.nr_lrs )
{
case 16:
-v->arch.gic.v3.lr[15] = READ_SYSREG(ICH_LR15_EL2);
+v->arch.gic.v3.lr[15] = READ_SYSREG_LR(15);
case 15:
-v->arch.gic.v3.lr[14] = READ_SYSREG(ICH_LR14_EL2);
+v->arch.gic.v3.lr[14] = READ_SYSREG_LR(14);
case 14:
-v->arch.gic.v3.lr[13] = READ_SYSREG(ICH_LR13_EL2);
+v->arch.gic.v3.lr[13] = READ_SYSREG_LR(13);
case 13:
-v->arch.gic.v3.lr[12] = READ_SYSREG(ICH_LR12_EL2);
+v->arch.gic.v3.lr[12] = READ_SYSREG_LR(12);
case 12:
-v->arch.gic.v3.lr[11] = READ_SYSREG(ICH_LR11_EL2);
+v->arch.gic.v3.lr[11] = READ_SYSREG_LR(11);
case 11:
-v->arch.gic.v3.lr[10] = READ_SYSREG(ICH_LR10_EL2);
+v->arch.gic.v3.lr[10] = READ_SYSREG_LR(10);
case 10:
-v->arch.gic.v3.lr[9] = READ_SYSREG(ICH_LR9_EL2);
+v->arch.gic.v3.lr[9] = READ_SYSREG_LR(9);
case 9:
-v->arch.gic.v3.lr[8] = READ_SYSREG(ICH_LR8_EL2);
+v->arch.gic.v3.lr[8] = READ_SYSREG_LR(8);
case 8:
-v->arch.gic.v3.lr[7] = READ_SYSREG(ICH_LR7_EL2);
+v->arch.gic.v3.lr[7] = READ_SYSREG_LR(7);
case 7:
-v->arch.gic.v3.lr[6] = READ_SYSREG(ICH_LR6_EL2);
+v->arch.gic.v3.lr[6] = READ_SYSREG_LR(6);
case 6:
-v->arch.gic.v3.lr[5] = READ_SYSREG(ICH_LR5_EL2);
+v->arch.gic.v3.lr[5] = READ_SYSREG_LR(5);
case 5:
-v->arch.gic.v3.lr[4] = READ_SYSREG(ICH_LR4_EL2);
+v->arch.gic.v3.lr[4] = READ_SYSREG_LR(4);
case 4:
-v->arch.gic.v3.lr[3] = READ_SYSREG(ICH_LR3_EL2);
+v->arch.gic.v3.lr[3] = READ_SYSREG_LR(3);
case 3:
-v->arch.gic.v3.lr[2] = READ_SYSREG(ICH_LR2_EL2);
+v->arch.gic.v3.lr[2] = READ_SYSREG_LR(2);
case 2:
-v->arch.gic.v3.lr[1] = READ_SYSREG(ICH_LR1_EL2);
+v->arch.gic.v3.lr[1] = READ_SYSREG_LR(1);
case 1:
- v->arch.gic.v3.lr[0] = READ_SYSREG(ICH_LR0_EL2);
+ v->arch.gic.v3.lr[0] = READ_SYSREG_LR(0);
break;
default:
BUG();
@@ -120,37 +120,37 @@ static inline void gicv3_restore_lrs(const struct vcpu *v)
switch ( gicv3_info.nr_lrs )
{
case 16:
-WRITE_SYSREG(v->arch.gic.v3.lr[15], ICH_LR15_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[15], 15);
case 15:
-WRITE_SYSREG(v->arch.gic.v3.lr[14], ICH_LR14_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[14], 14);
case 14:
-WRITE_SYSREG(v->arch.gic.v3.lr[13], ICH_LR13_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[13], 13);
case 13:
-WRITE_SYSREG(v->arch.gic.v3.lr[12], ICH_LR12_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[12], 12);
case 12:
-WRITE_SYSREG(v->arch.gic.v3.lr[11], ICH_LR11_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[11], 11);
case 11:
-WRITE_SYSREG(v->arch.gic.v3.lr[10], ICH_LR10_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[10], 10);
case 10:
-WRITE_SYSREG(v->arch.gic.v3.lr[9], ICH_LR9_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[9], 9);
case 9:
-WRITE_SYSREG(v->arch.gic.v3.lr[8], ICH_LR8_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[8], 8);
case 8:
-WRITE_SYSREG(v->arch.gic.v3.lr[7], ICH_LR7_EL2);
+WRITE_SYSREG_LR(v->arch.gic.v3.lr[7], 7);
case
[XEN v5 11/11] xen/Arm: GICv3: Enable GICv3 for AArch32
One can now use GICv3 on AArch32 systems. However, ITS is not supported. The reason being currently we are trying to validate GICv3 on an AArch32_v8R system. Refer ARM DDI 0568A.c ID110520, B1.3.1, "A Generic Interrupt Controller (GIC) implemented with an Armv8-R PE must not implement LPI support." By default GICv3 is disabled on AArch32 and enabled on AArch64. Updated SUPPORT.md to state that GICv3 on Arm32 is not security supported. Signed-off-by: Ayan Kumar Halder Reviewed-by: Michal Orzel --- Changed from :- v1 - 1. Remove "ARM_64 || ARM_32" as it is always true. 2. Updated SUPPORT.md. v2 - 1. GICv3 is enabled by default only on ARM_64. 2. Updated SUPPORT.md. v3 - 1. GICv3 is not selected by ARM_64. Rather, it is optionally enabled. 2. GICv3 is disabled by default on ARM_32. v4 - 1. Updated the help message for GICV3. 2. I have kept the Rb given on v4 as the change looks trivial. SUPPORT.md| 7 +++ xen/arch/arm/Kconfig | 9 + xen/arch/arm/include/asm/cpufeature.h | 1 + 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/SUPPORT.md b/SUPPORT.md index ab71464cf6..295369998e 100644 --- a/SUPPORT.md +++ b/SUPPORT.md @@ -76,6 +76,13 @@ For the Cortex A57 r0p0 - r1p1, see Errata 832075. Status, ARM SMMUv3: Tech Preview Status, Renesas IPMMU-VMSA: Supported, not security supported +### ARM/GICv3 + +GICv3 is an interrupt controller specification designed by Arm. + +Status, Arm64: Security supported +Status, Arm32: Supported, not security supported + ### ARM/GICv3 ITS Extension to the GICv3 interrupt controller to support MSI. diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 52a05f704d..239d3aed3c 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -41,16 +41,17 @@ config ARM_EFI config GICV3 bool "GICv3 driver" - depends on ARM_64 && !NEW_VGIC - default y + depends on !NEW_VGIC + default n if ARM_32 + default y if ARM_64 ---help--- Driver for the ARM Generic Interrupt Controller v3. - If unsure, say Y + If unsure, use the default setting. config HAS_ITS bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if UNSUPPORTED -depends on GICV3 && !NEW_VGIC +depends on GICV3 && !NEW_VGIC && !ARM_32 config HVM def_bool y diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h index c86a2e7f29..c62cf6293f 100644 --- a/xen/arch/arm/include/asm/cpufeature.h +++ b/xen/arch/arm/include/asm/cpufeature.h @@ -33,6 +33,7 @@ #define cpu_has_aarch32 (cpu_has_arm || cpu_has_thumb) #ifdef CONFIG_ARM_32 +#define cpu_has_gicv3 (boot_cpu_feature32(gic) >= 1) #define cpu_has_gentimer (boot_cpu_feature32(gentimer) == 1) /* * On Armv7, the value 0 is used to indicate that PMUv2 is not -- 2.17.1
[XEN v5 10/11] xen/Arm: GICv3: Define macros to read/write 64 bit
On AArch32, ldrd/strd instructions are not atomic when used to access MMIO.
Furthermore, ldrd/strd instructions are not decoded by Arm when running as
a guest to access emulated MMIO region.
Thus, we have defined readq_relaxed_non_atomic()/writeq_relaxed_non_atomic()
which in turn calls readl_relaxed()/writel_relaxed() for the lower and upper
32 bits.
For AArch64, readq_relaxed_non_atomic()/writeq_relaxed_non_atomic() invokes
readq_relaxed()/writeq_relaxed() respectively.
As GICv3 registers (GICD_IROUTER, GICR_TYPER) can be accessed in a non atomic
manner, so we have used readq_relaxed_non_atomic()/readq_relaxed_non_atomic().
However, the following points are noted for the non atomic access :-
1. In gicv3_dist_init(), using non atomic write on GICD_IROUTER is fine as this
gets invoked when interrupts are disabled.
2. In gicv3_populate_rdist(), using non atomic read on GICR_TYPER is fine as
the register is read and the interrupts are disabled as well.
3. In gicv3_irq_set_affinity(), using non atomic write on GICD_IROUTER. This
may be called with interrupts enabled. So, a non-atomic access (on AArch32)
means the GIC will see a transient value when only one of two 32-bit will be
updated. However, only AFF3 is defined in the upper 32 bits and they are 0, so
this will never change.
On AArch64, writeq_relaxed_non_atomic() invokes writeq_relaxed() (which is
atomic), so this problem does not arise.
Signed-off-by: Ayan Kumar Halder
---
Changes from :-
v1 - 1. Use ldrd/strd for readq_relaxed()/writeq_relaxed().
2. No need to use le64_to_cpu() as the returned byte order is already in cpu
endianess.
v2 - 1. Replace {read/write}q_relaxed with {read/write}q_relaxed_non_atomic().
v3 - 1. Use inline function definitions for {read/write}q_relaxed_non_atomic().
2. For AArch64, {read/write}q_relaxed_non_atomic() should invoke
{read/write}q_relaxed().
Thus, we can avoid any ifdef in gic-v3.c.
v4 - 1. Updated the commit message.
xen/arch/arm/gic-v3.c | 6 +++---
xen/arch/arm/include/asm/arm32/io.h | 20
xen/arch/arm/include/asm/arm64/io.h | 2 ++
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 6457e7033c..3c5b88148c 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -651,7 +651,7 @@ static void __init gicv3_dist_init(void)
affinity &= ~GICD_IROUTER_SPI_MODE_ANY;
for ( i = NR_GIC_LOCAL_IRQS; i < nr_lines; i++ )
-writeq_relaxed(affinity, GICD + GICD_IROUTER + i * 8);
+writeq_relaxed_non_atomic(affinity, GICD + GICD_IROUTER + i * 8);
}
static int gicv3_enable_redist(void)
@@ -745,7 +745,7 @@ static int __init gicv3_populate_rdist(void)
}
do {
-typer = readq_relaxed(ptr + GICR_TYPER);
+typer = readq_relaxed_non_atomic(ptr + GICR_TYPER);
if ( (typer >> 32) == aff )
{
@@ -1265,7 +1265,7 @@ static void gicv3_irq_set_affinity(struct irq_desc *desc,
const cpumask_t *mask)
affinity &= ~GICD_IROUTER_SPI_MODE_ANY;
if ( desc->irq >= NR_GIC_LOCAL_IRQS )
-writeq_relaxed(affinity, (GICD + GICD_IROUTER + desc->irq * 8));
+writeq_relaxed_non_atomic(affinity, (GICD + GICD_IROUTER + desc->irq *
8));
spin_unlock(&gicv3.lock);
}
diff --git a/xen/arch/arm/include/asm/arm32/io.h
b/xen/arch/arm/include/asm/arm32/io.h
index 73a879e9fb..782b564809 100644
--- a/xen/arch/arm/include/asm/arm32/io.h
+++ b/xen/arch/arm/include/asm/arm32/io.h
@@ -80,10 +80,30 @@ static inline u32 __raw_readl(const volatile void __iomem
*addr)
__raw_readw(c)); __r; })
#define readl_relaxed(c) ({ u32 __r = le32_to_cpu((__force __le32) \
__raw_readl(c)); __r; })
+/*
+ * ldrd instructions are not decoded by Arm when running as a guest to access
+ * emulated MMIO region. Thus, readq_relaxed_non_atomic() invokes
readl_relaxed()
+ * twice to read the lower and upper 32 bits.
+ */
+static inline u64 readq_relaxed_non_atomic(const volatile void __iomem *addr)
+{
+u64 val = (((u64)readl_relaxed(addr + 4)) << 32) | readl_relaxed(addr);
+return val;
+}
#define writeb_relaxed(v,c) __raw_writeb(v,c)
#define writew_relaxed(v,c) __raw_writew((__force u16) cpu_to_le16(v),c)
#define writel_relaxed(v,c) __raw_writel((__force u32) cpu_to_le32(v),c)
+/*
+ * strd instructions are not decoded by Arm when running as a guest to access
+ * emulated MMIO region. Thus, writeq_relaxed_non_atomic() invokes
writel_relaxed()
+ * twice to write the lower and upper 32 bits.
+ */
+static inline void writeq_relaxed_non_atomic(u64 val, volatile void __iomem
*addr)
+{
+writel_relaxed((u32)val, addr);
+writel_relaxed((u32)(val >> 32), addr + 4);
+}
#define readb(c)({ u8 __v = readb_relaxed(c); __iormb(); __v;
})
#define readw(c)({ u16 __v = readw_relaxed(c); __iormb(); __v;
Re: [RFC PATCH 06/21] xen/domctl: Add XEN_DOMCTL_CONFIG_VIOMMU_* and viommu config param
Hi Rahul, On 01/12/2022 17:02, Rahul Singh wrote: > > > Add new viommu_type field and field values XEN_DOMCTL_CONFIG_VIOMMU_NONE > XEN_DOMCTL_CONFIG_VIOMMU_SMMUV3 in xen_arch_domainconfig to > enable/disable vIOMMU support for domains. > > Also add viommu="N" parameter to xl domain configuration to enable the > vIOMMU for the domains. Currently, only the "smmuv3" type is supported > for ARM. > > Signed-off-by: Rahul Singh > --- > docs/man/xl.cfg.5.pod.in | 11 +++ > tools/golang/xenlight/helpers.gen.go | 2 ++ > tools/golang/xenlight/types.gen.go | 1 + > tools/include/libxl.h| 5 + > tools/libs/light/libxl_arm.c | 13 + > tools/libs/light/libxl_types.idl | 6 ++ > tools/xl/xl_parse.c | 9 + > 7 files changed, 47 insertions(+) > > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in > index ec444fb2ba..5854d777ed 100644 > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -2870,6 +2870,17 @@ Currently, only the "sbsa_uart" model is supported for > ARM. > > =back > > +=item B > + > +To enable viommu, user must specify the following option in the VM > +config file: > + > +viommu = "smmuv3" > + > +Currently, only the "smmuv3" type is supported for ARM. > + > +=back You need to remove this "=back" or the one before your new section, because currently the build of docs fails with error: man/xl.cfg.5.pod around line 2873: '=item' outside of any '=over' ~Michal
[XEN v5 06/11] xen/Arm: vGICv3: Fix emulation of ICC_SGI1R on AArch32
Refer Arm IHI 0069H ID020922, 12.5.23, ICC_SGI1R is a 64 bit register on
AArch32 systems. Thus, the function needs to change to reflect this.
The reason being 'register_t' is defined as 'u32' on AArch32.
Signed-off-by: Ayan Kumar Halder
Reviewed-by: Michal Orzel
Acked-by: Julien Grall
---
Changes from :-
v1 - 1. Updated the commit message.
v2 - 1. No changes.
v3 - 1. No changes.
v4 - 1. No changes.
xen/arch/arm/vgic-v3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index e0b636b95f..47575d4944 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1483,7 +1483,7 @@ write_reserved:
return 1;
}
-static bool vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
+static bool vgic_v3_to_sgi(struct vcpu *v, uint64_t sgir)
{
int virq;
int irqmode;
--
2.17.1
[XEN v5 05/11] xen/Arm: GICv3: Fix GICR_{PENDBASER, PROPBASER} emulation on 32-bit host
'unsigned long long' is defined as 64 bit across both AArch32 and AArch64. So, use 'ULL' for 64 bit word instead of UL which is 32 bits for AArch32. GICR_PENDBASER and GICR_PROPBASER both are 64 bit registers. Signed-off-by: Ayan Kumar Halder Reviewed-by: Michal Orzel Acked-by: Julien Grall --- Changes from - v1 - 1. Extract the bug fix for incorrect bit clearing (GICR_PENDBASER_PTZ) into a separate patch fix. https://patchwork.kernel.org/project/xen-devel/patch/2022102718.46125-1-ayank...@amd.com/ v2 - No changes. v3 - No changes. v4 - No changes. xen/arch/arm/include/asm/gic_v3_defs.h | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h index 728e28d5e5..48a1bc401e 100644 --- a/xen/arch/arm/include/asm/gic_v3_defs.h +++ b/xen/arch/arm/include/asm/gic_v3_defs.h @@ -134,15 +134,15 @@ #define GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT 56 #define GICR_PROPBASER_OUTER_CACHEABILITY_MASK \ -(7UL << GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT) +(7ULL << GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT) #define GICR_PROPBASER_SHAREABILITY_SHIFT 10 #define GICR_PROPBASER_SHAREABILITY_MASK \ -(3UL << GICR_PROPBASER_SHAREABILITY_SHIFT) +(3ULL << GICR_PROPBASER_SHAREABILITY_SHIFT) #define GICR_PROPBASER_INNER_CACHEABILITY_SHIFT 7 #define GICR_PROPBASER_INNER_CACHEABILITY_MASK \ -(7UL << GICR_PROPBASER_INNER_CACHEABILITY_SHIFT) +(7ULL << GICR_PROPBASER_INNER_CACHEABILITY_SHIFT) #define GICR_PROPBASER_RES0_MASK \ -(GENMASK(63, 59) | GENMASK(55, 52) | GENMASK(6, 5)) +(GENMASK_ULL(63, 59) | GENMASK_ULL(55, 52) | GENMASK_ULL(6, 5)) #define GICR_PENDBASER_SHAREABILITY_SHIFT 10 #define GICR_PENDBASER_INNER_CACHEABILITY_SHIFT 7 @@ -152,11 +152,11 @@ #define GICR_PENDBASER_INNER_CACHEABILITY_MASK \ (7UL << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT) #define GICR_PENDBASER_OUTER_CACHEABILITY_MASK \ -(7UL << GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT) -#define GICR_PENDBASER_PTZ BIT(62, UL) +(7ULL << GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT) +#define GICR_PENDBASER_PTZ BIT(62, ULL) #define GICR_PENDBASER_RES0_MASK \ -(BIT(63, UL) | GENMASK(61, 59) | GENMASK(55, 52) | \ - GENMASK(15, 12) | GENMASK(6, 0)) +(BIT(63, ULL) | GENMASK_ULL(61, 59) | GENMASK_ULL(55, 52) | \ + GENMASK_ULL(15, 12) | GENMASK_ULL(6, 0)) #define DEFAULT_PMR_VALUE0xff -- 2.17.1
[XEN v5 03/11] xen/Arm: vreg: Support vreg_reg64_* helpers on AArch32
In some situations (e.g. GICR_TYPER), the hypervior may need to emulate
64bit registers in AArch32 mode. In such situations, the hypervisor may
need to read/modify the lower or upper 32 bits of the 64 bit register.
In AArch32, 'unsigned long' is 32 bits. Thus, we cannot use it for 64 bit
registers.
While we could replace 'unsigned long' by 'uint64_t', it is not entirely clear
whether a 32-bit compiler would not allocate register for the upper 32-bit.
Therefore fold vreg_reg_* helper in the size specific one and use the
appropriate type based on the size requested.
Signed-off-by: Ayan Kumar Halder
Reviewed-by: Michal Orzel
Acked-by: Julien Grall
---
Changes from -
v1 - 1. Remove vreg_reg_extract(), vreg_reg_update(), vreg_reg_setbits() and
vreg_reg_clearbits(). Moved the implementation to vreg_reg##sz##_*.
'mask' and 'val' is now using uint##sz##_t.
v2 - 1. Use 'unsigned int' for 'shift' variable.
2. Updated the commit message.
v3 - 1. No changes. Added Rb and Ack.
v4 - 1. No changes.
xen/arch/arm/include/asm/vreg.h | 86 -
1 file changed, 19 insertions(+), 67 deletions(-)
diff --git a/xen/arch/arm/include/asm/vreg.h b/xen/arch/arm/include/asm/vreg.h
index f26a70d024..d92450017b 100644
--- a/xen/arch/arm/include/asm/vreg.h
+++ b/xen/arch/arm/include/asm/vreg.h
@@ -89,106 +89,58 @@ static inline bool vreg_emulate_sysreg(struct
cpu_user_regs *regs, union hsr hsr
* The check on the size supported by the register has to be done by
* the caller of vreg_regN_*.
*
- * vreg_reg_* should never be called directly. Instead use the vreg_regN_*
- * according to size of the emulated register
- *
* Note that the alignment fault will always be taken in the guest
* (see B3.12.7 DDI0406.b).
*/
-static inline register_t vreg_reg_extract(unsigned long reg,
- unsigned int offset,
- enum dabt_size size)
-{
-reg >>= 8 * offset;
-reg &= VREG_REG_MASK(size);
-
-return reg;
-}
-
-static inline void vreg_reg_update(unsigned long *reg, register_t val,
- unsigned int offset,
- enum dabt_size size)
-{
-unsigned long mask = VREG_REG_MASK(size);
-int shift = offset * 8;
-
-*reg &= ~(mask << shift);
-*reg |= ((unsigned long)val & mask) << shift;
-}
-
-static inline void vreg_reg_setbits(unsigned long *reg, register_t bits,
-unsigned int offset,
-enum dabt_size size)
-{
-unsigned long mask = VREG_REG_MASK(size);
-int shift = offset * 8;
-
-*reg |= ((unsigned long)bits & mask) << shift;
-}
-
-static inline void vreg_reg_clearbits(unsigned long *reg, register_t bits,
- unsigned int offset,
- enum dabt_size size)
-{
-unsigned long mask = VREG_REG_MASK(size);
-int shift = offset * 8;
-
-*reg &= ~(((unsigned long)bits & mask) << shift);
-}
/* N-bit register helpers */
#define VREG_REG_HELPERS(sz, offmask) \
static inline register_t vreg_reg##sz##_extract(uint##sz##_t reg, \
const mmio_info_t *info)\
{ \
-return vreg_reg_extract(reg, info->gpa & (offmask), \
-info->dabt.size); \
+unsigned int offset = info->gpa & (offmask);\
+\
+reg >>= 8 * offset; \
+reg &= VREG_REG_MASK(info->dabt.size); \
+\
+return reg; \
} \
\
static inline void vreg_reg##sz##_update(uint##sz##_t *reg, \
register_t val,\
const mmio_info_t *info) \
{ \
-unsigned long tmp = *reg; \
+unsigned int offset = info->gpa & (offmask);\
+uint##sz##_t mask = VREG_REG_MASK(info->dabt.size); \
+unsigned int shift = offset * 8;\
\
-vreg_reg_update(&tmp, val, info->gpa & (offmask), \
-info->dabt.size); \
-
[XEN v5 04/11] xen/Arm: vGICv3: Adapt emulation of GICR_TYPER for AArch32
Refer ARM DDI 0487I.a ID081822, G8-9650, G8.2.113
Aff3 does not exist on AArch32.
Also, refer ARM DDI 0406C.d ID040418, B4-1644, B4.1.106
Aff3 does not exist on Armv7 (ie arm32).
Thus, access to aff3 has been protected with "#ifdef CONFIG_ARM_64".
Also, v->arch.vmpidr is a 32 bit register on AArch32. So, we have assigned it to
'uint64_t vmpidr' to perform the shifts.
Signed-off-by: Ayan Kumar Halder
Reviewed-by: Michal Orzel
Acked-by: Julien Grall
---
Changes from :-
v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use
MPIDR_AFFINITY_LEVEL macros to extract the affinity value.
v2 - 1. "MPIDR_AFFINITY_LEVEL(vmpidr, 3)" is contained within
"#ifdef CONFIG_ARM_64".
2. Updated commit message.
v3 - 1. Added an inline comment to explain type widening for v->arch.vmpidr.
2. Updated the commit message. Added Rb.
v4 - 1. Added Ack.
xen/arch/arm/vgic-v3.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 3f4509dcd3..e0b636b95f 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -191,12 +191,20 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v,
mmio_info_t *info,
case VREG64(GICR_TYPER):
{
uint64_t typer, aff;
+/*
+ * This is to enable shifts greater than 32 bits which would have
+ * otherwise caused overflow (as v->arch.vmpidr is 32 bit on AArch32).
+ */
+uint64_t vmpidr = v->arch.vmpidr;
if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
-aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
- MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
- MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
- MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
+aff = (
+#ifdef CONFIG_ARM_64
+ MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
+#endif
+ MPIDR_AFFINITY_LEVEL(vmpidr, 2) << 48 |
+ MPIDR_AFFINITY_LEVEL(vmpidr, 1) << 40 |
+ MPIDR_AFFINITY_LEVEL(vmpidr, 0) << 32);
typer = aff;
/* We use the VCPU ID as the redistributor ID in bits[23:8] */
typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;
--
2.17.1
Re: [RFC PATCH 06/21] xen/domctl: Add XEN_DOMCTL_CONFIG_VIOMMU_* and viommu config param
Hi Jan,
> On 2 Dec 2022, at 8:45 am, Jan Beulich wrote:
>
> On 01.12.2022 17:02, Rahul Singh wrote:
>> Add new viommu_type field and field values XEN_DOMCTL_CONFIG_VIOMMU_NONE
>> XEN_DOMCTL_CONFIG_VIOMMU_SMMUV3 in xen_arch_domainconfig to
>> enable/disable vIOMMU support for domains.
>>
>> Also add viommu="N" parameter to xl domain configuration to enable the
>> vIOMMU for the domains. Currently, only the "smmuv3" type is supported
>> for ARM.
>>
>> Signed-off-by: Rahul Singh
>> ---
>> docs/man/xl.cfg.5.pod.in | 11 +++
>> tools/golang/xenlight/helpers.gen.go | 2 ++
>> tools/golang/xenlight/types.gen.go | 1 +
>> tools/include/libxl.h| 5 +
>> tools/libs/light/libxl_arm.c | 13 +
>> tools/libs/light/libxl_types.idl | 6 ++
>> tools/xl/xl_parse.c | 9 +
>> 7 files changed, 47 insertions(+)
>
> This diffstat taken together with the title makes me assume that e.g. ...
>
>> --- a/tools/libs/light/libxl_arm.c
>> +++ b/tools/libs/light/libxl_arm.c
>> @@ -179,6 +179,19 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>> return ERROR_FAIL;
>> }
>>
>> +switch (d_config->b_info.arch_arm.viommu_type) {
>> +case LIBXL_VIOMMU_TYPE_NONE:
>> +config->arch.viommu_type = XEN_DOMCTL_CONFIG_VIOMMU_NONE;
>
> ... this constant doesn't exist yet, and hence this would fail to build
> at this point in the series. I notice, however, that the constants are
> introduced in earlier patches. Perhaps the title here wants re-wording?
Yes, I will fix the commit msg.
Regards,
Rahul
[xen-unstable-smoke test] 175048: tolerable all pass - PUSHED
flight 175048 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/175048/ 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 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass version targeted for testing: xen 15241c92677a027694e609b55a77ae5ba3616104 baseline version: xen 68f551ec5fa9ad96397a62cd463a7ff7e4db72d0 Last test of basis 175035 2022-12-03 19:03:17 Z1 days Testing same since 175048 2022-12-05 12:03:13 Z0 days1 attempts People who touched revisions under test: Jan Beulich Julien Grall jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass 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 68f551ec5f..15241c9267 15241c92677a027694e609b55a77ae5ba3616104 -> smoke
Re: [PATCH net] xen/netback: don't call kfree_skb() under spin_lock_irqsave()
On 05/12/2022 14:13, Yang Yingliang wrote:
It is not allowed to call kfree_skb() from hardware interrupt
context or with interrupts being disabled. So replace kfree_skb()
with dev_kfree_skb_irq() under spin_lock_irqsave().
Fixes: be81992f9086 ("xen/netback: don't queue unlimited number of packages")
Signed-off-by: Yang Yingliang
Reviewed-by: Paul Durrant
Re: [PATCH 3/5] efi: try to use the currently set GOP mode
On 23.11.2022 16:45, Roger Pau Monne wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -864,6 +864,26 @@ static UINTN __init
> efi_find_gop_mode(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop,
> UINTN gop_mode = ~0, info_size, size;
> unsigned int i;
>
> +if ( (!cols || !rows) && gop->Mode->Mode < gop->Mode->MaxMode )
> +{
> +/* If no (valid) resolution suggested, try to use the current mode.
> */
> +status = gop->QueryMode(gop, gop->Mode->Mode, &info_size,
> &mode_info);
> +if ( EFI_ERROR(status) )
> +PrintErr(L"Invalid current graphics mode\r\n");
> +else if ( mode_info->PixelFormat < PixelBltOnly )
> +return gop->Mode->Mode;
> +else
> +{
> +/*
> + * Try to find a mode with the same resolution and a valid pixel
> + * format.
> + */
> +cols = mode_info->HorizontalResolution;
> +rows = mode_info->VerticalResolution;
For these I think you want to replace cols and rows individually, i.e.
if ( !cols )
cols = mode_info->HorizontalResolution;
if ( !rows )
rows = mode_info->VerticalResolution;
whereas ...
> +depth = 0;
... this case looks more complicated, as 0 is also already legitimate
as a value to come in. By zapping a non-zero incoming value you may end
up switching modes _just_ to alter depth, and then you may not fulfill
what was requested. For now I think depth simply wants leaving alone
here (and perhaps using the current mode's value if the incoming value
was zero, thus eliminating the need for a mode change in certain cases).
In a separate change we might then enhance this, e.g. by finding a
supported mode matching geometry but only coming close for "depth".
Jan
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
Hi Rahul,
On 05/12/2022 14:53, Rahul Singh wrote:
>
>
> Hi Michal,
>
>> On 5 Dec 2022, at 8:26 am, Michal Orzel wrote:
>>
>> Hi Rahul,
>>
>> On 01/12/2022 17:02, Rahul Singh wrote:
>>>
>>>
>>> This patch adds basic framework for vIOMMU.
>>>
>>> Signed-off-by: Rahul Singh
>>> ---
>>> xen/arch/arm/domain.c| 17 +++
>>> xen/arch/arm/domain_build.c | 3 ++
>>> xen/arch/arm/include/asm/viommu.h| 70
>>> xen/drivers/passthrough/Kconfig | 6 +++
>>> xen/drivers/passthrough/arm/Makefile | 1 +
>>> xen/drivers/passthrough/arm/viommu.c | 48 +++
>>> xen/include/public/arch-arm.h| 4 ++
>>> 7 files changed, 149 insertions(+)
>>> create mode 100644 xen/arch/arm/include/asm/viommu.h
>>> create mode 100644 xen/drivers/passthrough/arm/viommu.c
>>>
>>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>>> index 38e22f12af..2a85209736 100644
>>> --- a/xen/arch/arm/domain.c
>>> +++ b/xen/arch/arm/domain.c
>>> @@ -37,6 +37,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>>
>>> #include "vpci.h"
>>> @@ -691,6 +692,13 @@ int arch_sanitise_domain_config(struct
>>> xen_domctl_createdomain *config)
>>> return -EINVAL;
>>> }
>>>
>>> +if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
>>> +{
>>> +dprintk(XENLOG_INFO,
>>> +"vIOMMU type requested not supported by the platform or
>>> Xen\n");
>> Maybe a simpler message like for TEE would be better: "Unsupported vIOMMU
>> type"
>
> Ack.
>>
>>> +return -EINVAL;
>>> +}
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -783,6 +791,9 @@ int arch_domain_create(struct domain *d,
>>> if ( (rc = domain_vpci_init(d)) != 0 )
>>> goto fail;
>>>
>>> +if ( (rc = domain_viommu_init(d, config->arch.viommu_type)) != 0 )
>>> +goto fail;
>>> +
>>> return 0;
>>>
>>> fail:
>>> @@ -998,6 +1009,7 @@ static int relinquish_memory(struct domain *d, struct
>>> page_list_head *list)
>>> enum {
>>> PROG_pci = 1,
>>> PROG_tee,
>>> +PROG_viommu,
>>> PROG_xen,
>>> PROG_page,
>>> PROG_mapping,
>>> @@ -1048,6 +1060,11 @@ int domain_relinquish_resources(struct domain *d)
>>> if (ret )
>>> return ret;
>>>
>>> +PROGRESS(viommu):
>>> +ret = viommu_relinquish_resources(d);
>>> +if (ret )
>>> +return ret;
>>> +
>>> PROGRESS(xen):
>>> ret = relinquish_memory(d, &d->xenpage_list);
>>> if ( ret )
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index bd30d3798c..abbaf37a2e 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -27,6 +27,7 @@
>>> #include
>>> #include
>>> #include
>>> +#include
>>> #include
>>>
>>> #include
>>> @@ -3858,6 +3859,7 @@ void __init create_domUs(void)
>>> struct domain *d;
>>> struct xen_domctl_createdomain d_cfg = {
>>> .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
>>> +.arch.viommu_type = viommu_get_type(),
>>> .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
>>> /*
>>> * The default of 1023 should be sufficient for guests because
>>> @@ -4052,6 +4054,7 @@ void __init create_dom0(void)
>>> printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
>>> dom0_cfg.arch.tee_type = tee_get_type();
>>> dom0_cfg.max_vcpus = dom0_max_vcpus();
>>> +dom0_cfg.arch.viommu_type = viommu_get_type();
>>>
>>> if ( iommu_enabled )
>>> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>>> diff --git a/xen/arch/arm/include/asm/viommu.h
>>> b/xen/arch/arm/include/asm/viommu.h
>>> new file mode 100644
>>> index 00..7cd3818a12
>>> --- /dev/null
>>> +++ b/xen/arch/arm/include/asm/viommu.h
>>> @@ -0,0 +1,70 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
>>> +#ifndef __ARCH_ARM_VIOMMU_H__
>>> +#define __ARCH_ARM_VIOMMU_H__
>>> +
>>> +#ifdef CONFIG_VIRTUAL_IOMMU
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +struct viommu_ops {
>>> +/*
>>> + * Called during domain construction if toolstack requests to enable
>>> + * vIOMMU support.
>>> + */
>>> +int (*domain_init)(struct domain *d);
>>> +
>>> +/*
>>> + * Called during domain destruction to free resources used by vIOMMU.
>>> + */
>>> +int (*relinquish_resources)(struct domain *d);
>>> +};
>>> +
>>> +struct viommu_desc {
>>> +/* vIOMMU domains init/free operations described above. */
>>> +const struct viommu_ops *ops;
>>> +
>>> +/*
>>> + * ID of vIOMMU. Corresponds to xen_arch_domainconfig.viommu_type.
>>> + * Should be one of XEN_DOMCTL_CONFIG_VIOMMU_xxx
>>> + */
>>> +uint16_t viommu_type;
>> Here the viommu_type is uint16_t ...
>>
>>> +};
>>> +
>>> +int domain_viommu_init(struct domain *d, uint16_t viommu_type);
>>> +int viommu_relinquish_resources(struct domain *d);
Re: [PATCH 2/5] efi: only set a console mode if the current one is invalid
On 23.11.2022 16:45, Roger Pau Monne wrote: > Do not unconditionally set a mode in efi_console_set_mode(), do so > only if the currently set mode is not valid. You don't say why you want to do so. Furthermore ... > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -799,6 +799,11 @@ static void __init efi_console_set_mode(void) > UINTN cols, rows, size; > unsigned int best, i; > > +/* Only set a mode if the current one is not valid. */ > +if ( StdOut->QueryMode(StdOut, StdOut->Mode->Mode, &cols, &rows) == > + EFI_SUCCESS ) > +return; ... it might be okay if you put such a check in efi_multiboot2(), but the call here from efi_start() is specifically guarded by a check of whether "-basevideo" was passed to xen.efi. This _may_ not be as relevant anymore today, but it certainly was 20 years ago (recall that we've inherited this code from a much older project of ours) - at that time EFI usually started in 80x25 text mode. And I think that even today when you end up launching xen.efi from the EFI shell, you'd be stuck with 80x25 text mode on at least some implementations. Overall, looking at (for now) just the titles of subsequent patches, I'm not convinced the change here is needed at all. Or if anything it may want to go at the end, taking action only when "vga=current" was specified. Jan
[PATCH net] xen/netback: don't call kfree_skb() under spin_lock_irqsave()
It is not allowed to call kfree_skb() from hardware interrupt
context or with interrupts being disabled. So replace kfree_skb()
with dev_kfree_skb_irq() under spin_lock_irqsave().
Fixes: be81992f9086 ("xen/netback: don't queue unlimited number of packages")
Signed-off-by: Yang Yingliang
---
drivers/net/xen-netback/rx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/xen-netback/rx.c b/drivers/net/xen-netback/rx.c
index 932762177110..d022206a0d17 100644
--- a/drivers/net/xen-netback/rx.c
+++ b/drivers/net/xen-netback/rx.c
@@ -92,7 +92,7 @@ void xenvif_rx_queue_tail(struct xenvif_queue *queue, struct
sk_buff *skb)
struct net_device *dev = queue->vif->dev;
netif_tx_stop_queue(netdev_get_tx_queue(dev, queue->id));
- kfree_skb(skb);
+ dev_kfree_skb_irq(skb);
queue->vif->dev->stats.rx_dropped++;
} else {
if (skb_queue_empty(&queue->rx_queue))
--
2.25.1
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
On 05/12/2022 13:48, Rahul Singh wrote:
Hi Julien,
On 3 Dec 2022, at 9:54 pm, Julien Grall wrote:
Hi Rahul,
On 01/12/2022 16:02, Rahul Singh wrote:
This patch adds basic framework for vIOMMU.
Signed-off-by: Rahul Singh
---
xen/arch/arm/domain.c| 17 +++
xen/arch/arm/domain_build.c | 3 ++
xen/arch/arm/include/asm/viommu.h| 70
xen/drivers/passthrough/Kconfig | 6 +++
xen/drivers/passthrough/arm/Makefile | 1 +
xen/drivers/passthrough/arm/viommu.c | 48 +++
xen/include/public/arch-arm.h| 4 ++
7 files changed, 149 insertions(+)
create mode 100644 xen/arch/arm/include/asm/viommu.h
create mode 100644 xen/drivers/passthrough/arm/viommu.c
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 38e22f12af..2a85209736 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#include "vpci.h"
@@ -691,6 +692,13 @@ int arch_sanitise_domain_config(struct
xen_domctl_createdomain *config)
return -EINVAL;
}
+if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
+{
+dprintk(XENLOG_INFO,
+"vIOMMU type requested not supported by the platform or
Xen\n");
+return -EINVAL;
+}
+
return 0;
}
@@ -783,6 +791,9 @@ int arch_domain_create(struct domain *d,
if ( (rc = domain_vpci_init(d)) != 0 )
goto fail;
+if ( (rc = domain_viommu_init(d, config->arch.viommu_type)) != 0 )
+goto fail;
+
return 0;
fail:
@@ -998,6 +1009,7 @@ static int relinquish_memory(struct domain *d, struct
page_list_head *list)
enum {
PROG_pci = 1,
PROG_tee,
+PROG_viommu,
PROG_xen,
PROG_page,
PROG_mapping,
@@ -1048,6 +1060,11 @@ int domain_relinquish_resources(struct domain *d)
if (ret )
return ret;
+PROGRESS(viommu):
+ret = viommu_relinquish_resources(d);
+if (ret )
+return ret;
I would have expected us to relinquish the vIOMMU resources *before* we detach
the devices. So can you explain the ordering?
I think first we need to detach the device that will set the S1 and S2 stage
translation to bypass/abort then we
need to remove the vIOMMU. Do you have anything that you feel if we relinquish
the vIOMMU after detach is not right.
iommu_release_dt_devices() will effectively remove the device from the
domain. One could decide to store the S1 information per device and
therefore it feels wrong that we are removing the S1 information later on.
In addition to that, a device can be removed while the domain is running.
Therefore I think it would make more sense to remove the vIOMMU every
time we deassign the device.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index bd30d3798c..abbaf37a2e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
@@ -3858,6 +3859,7 @@ void __init create_domUs(void)
struct domain *d;
struct xen_domctl_createdomain d_cfg = {
.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
+.arch.viommu_type = viommu_get_type(),
I don't think the vIOMMU should be enabled to dom0less domU by default.
I am not enabling the vIOMMU by default. If vIOMMU is disabled via the command
line or config option
then viommu_get_type() will return XEN_DOMCTL_CONFIG_VIOMMU_NONE and in that
case
domain_viommu_init() will return 0 without doing anything.
What I mean by default is if the command line is set then you will
enable the vIOMMU for both dom0 and dom0less domUs.
vIOMMU should be selected per-domain.
.flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
/*
* The default of 1023 should be sufficient for guests because
@@ -4052,6 +4054,7 @@ void __init create_dom0(void)
printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
dom0_cfg.arch.tee_type = tee_get_type();
dom0_cfg.max_vcpus = dom0_max_vcpus();
+dom0_cfg.arch.viommu_type = viommu_get_type();
Same here.
if ( iommu_enabled )
dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
diff --git a/xen/arch/arm/include/asm/viommu.h
b/xen/arch/arm/include/asm/viommu.h
new file mode 100644
index 00..7cd3818a12
--- /dev/null
+++ b/xen/arch/arm/include/asm/viommu.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
+#ifndef __ARCH_ARM_VIOMMU_H__
+#define __ARCH_ARM_VIOMMU_H__
+
+#ifdef CONFIG_VIRTUAL_IOMMU
+
+#include
+#include
+#include
+
+struct viommu_ops {
+/*
+ * Called during domain construction if toolstack requests to enable
+ * vIOMMU support.
+ */
+int (*domain_init)(struct domain *d);
+
+/*
+ * Called during domain destruc
Re: [RFC PATCH 05/21] xen/arm: vsmmuv3: Add dummy support for virtual SMMUv3 for guests
Hi Michal,
> On 5 Dec 2022, at 8:33 am, Michal Orzel wrote:
>
> Hi Rahul,
>
> On 01/12/2022 17:02, Rahul Singh wrote:
>>
>>
>> domain_viommu_init() will be called during domain creation and will add
>> the dummy trap handler for virtual IOMMUs for guests.
>>
>> A host IOMMU list will be created when host IOMMU devices are probed
>> and this list will be used to create the IOMMU device tree node for
>> dom0. For dom0, 1-1 mapping will be established between vIOMMU in dom0
>> and physical IOMMU.
>>
>> For domUs, the 1-N mapping will be established between domU and physical
>> IOMMUs. A new area has been reserved in the arm guest physical map at
>> which the emulated vIOMMU node is created in the device tree.
>>
>> Also set the vIOMMU type to vSMMUv3 to enable vIOMMU framework to call
>> vSMMUv3 domain creation/destroy functions.
>>
>> Signed-off-by: Rahul Singh
>> ---
>> xen/arch/arm/domain.c | 3 +-
>> xen/arch/arm/include/asm/domain.h | 4 +
>> xen/arch/arm/include/asm/viommu.h | 20
>> xen/drivers/passthrough/Kconfig| 8 ++
>> xen/drivers/passthrough/arm/Makefile | 1 +
>> xen/drivers/passthrough/arm/smmu-v3.c | 7 ++
>> xen/drivers/passthrough/arm/viommu.c | 30 ++
>> xen/drivers/passthrough/arm/vsmmu-v3.c | 124 +
>> xen/drivers/passthrough/arm/vsmmu-v3.h | 20
>> xen/include/public/arch-arm.h | 7 +-
>> 10 files changed, 222 insertions(+), 2 deletions(-)
>> create mode 100644 xen/drivers/passthrough/arm/vsmmu-v3.c
>> create mode 100644 xen/drivers/passthrough/arm/vsmmu-v3.h
>>
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 2a85209736..9a2b613500 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -692,7 +692,8 @@ int arch_sanitise_domain_config(struct
>> xen_domctl_createdomain *config)
>> return -EINVAL;
>> }
>>
>> -if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
>> +if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE &&
>> + config->arch.viommu_type != viommu_get_type() )
>> {
>> dprintk(XENLOG_INFO,
>> "vIOMMU type requested not supported by the platform or
>> Xen\n");
>> diff --git a/xen/arch/arm/include/asm/domain.h
>> b/xen/arch/arm/include/asm/domain.h
>> index 2ce6764322..8eb4eb5fd6 100644
>> --- a/xen/arch/arm/include/asm/domain.h
>> +++ b/xen/arch/arm/include/asm/domain.h
>> @@ -114,6 +114,10 @@ struct arch_domain
>> void *tee;
>> #endif
>>
>> +#ifdef CONFIG_VIRTUAL_IOMMU
>> +struct list_head viommu_list; /* List of virtual IOMMUs */
>> +#endif
>> +
>> } __cacheline_aligned;
>>
>> struct arch_vcpu
>> diff --git a/xen/arch/arm/include/asm/viommu.h
>> b/xen/arch/arm/include/asm/viommu.h
>> index 7cd3818a12..4785877e2a 100644
>> --- a/xen/arch/arm/include/asm/viommu.h
>> +++ b/xen/arch/arm/include/asm/viommu.h
>> @@ -5,9 +5,21 @@
>> #ifdef CONFIG_VIRTUAL_IOMMU
>>
>> #include
>> +#include
>> #include
>> #include
>>
>> +extern struct list_head host_iommu_list;
>> +
>> +/* data structure for each hardware IOMMU */
>> +struct host_iommu {
>> +struct list_head entry;
>> +const struct dt_device_node *dt_node;
>> +paddr_t addr;
>> +paddr_t size;
>> +uint32_t irq;
> You want this to be int and not unsigned. The reason is ...
>
>> +};
>> +
>> struct viommu_ops {
>> /*
>> * Called during domain construction if toolstack requests to enable
>> @@ -35,6 +47,8 @@ struct viommu_desc {
>> int domain_viommu_init(struct domain *d, uint16_t viommu_type);
>> int viommu_relinquish_resources(struct domain *d);
>> uint16_t viommu_get_type(void);
>> +void add_to_host_iommu_list(paddr_t addr, paddr_t size,
>> +const struct dt_device_node *node);
>>
>> #else
>>
>> @@ -56,6 +70,12 @@ static inline int viommu_relinquish_resources(struct
>> domain *d)
>> return 0;
>> }
>>
>> +static inline void add_to_host_iommu_list(paddr_t addr, paddr_t size,
>> + const struct dt_device_node *node)
>> +{
>> +return;
>> +}
>> +
>> #endif /* CONFIG_VIRTUAL_IOMMU */
>>
>> #endif /* __ARCH_ARM_VIOMMU_H__ */
>> diff --git a/xen/drivers/passthrough/Kconfig
>> b/xen/drivers/passthrough/Kconfig
>> index 19924fa2de..4c725f5f67 100644
>> --- a/xen/drivers/passthrough/Kconfig
>> +++ b/xen/drivers/passthrough/Kconfig
>> @@ -41,6 +41,14 @@ config VIRTUAL_IOMMU
>>help
>> Support virtual IOMMU infrastructure to implement vIOMMU.
>>
>> +config VIRTUAL_ARM_SMMU_V3
>> + bool "ARM Ltd. Virtual SMMUv3 Support (UNSUPPORTED)" if UNSUPPORTED
>> + depends on ARM_SMMU_V3 && VIRTUAL_IOMMU
>> + help
>> +Support for implementations of the virtual ARM System MMU
>> architecture
>> +version 3. Virtual SMMUv3 is unsupported feature and should not be
>> used
>> +in production.
>> +
>> endif
>>
>> config IOMMU_FORCE_PT_SHARE
>> diff --git a/xe
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
Hi Michal,
> On 5 Dec 2022, at 8:26 am, Michal Orzel wrote:
>
> Hi Rahul,
>
> On 01/12/2022 17:02, Rahul Singh wrote:
>>
>>
>> This patch adds basic framework for vIOMMU.
>>
>> Signed-off-by: Rahul Singh
>> ---
>> xen/arch/arm/domain.c| 17 +++
>> xen/arch/arm/domain_build.c | 3 ++
>> xen/arch/arm/include/asm/viommu.h| 70
>> xen/drivers/passthrough/Kconfig | 6 +++
>> xen/drivers/passthrough/arm/Makefile | 1 +
>> xen/drivers/passthrough/arm/viommu.c | 48 +++
>> xen/include/public/arch-arm.h| 4 ++
>> 7 files changed, 149 insertions(+)
>> create mode 100644 xen/arch/arm/include/asm/viommu.h
>> create mode 100644 xen/drivers/passthrough/arm/viommu.c
>>
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 38e22f12af..2a85209736 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -37,6 +37,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> #include "vpci.h"
>> @@ -691,6 +692,13 @@ int arch_sanitise_domain_config(struct
>> xen_domctl_createdomain *config)
>> return -EINVAL;
>> }
>>
>> +if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
>> +{
>> +dprintk(XENLOG_INFO,
>> +"vIOMMU type requested not supported by the platform or
>> Xen\n");
> Maybe a simpler message like for TEE would be better: "Unsupported vIOMMU
> type"
Ack.
>
>> +return -EINVAL;
>> +}
>> +
>> return 0;
>> }
>>
>> @@ -783,6 +791,9 @@ int arch_domain_create(struct domain *d,
>> if ( (rc = domain_vpci_init(d)) != 0 )
>> goto fail;
>>
>> +if ( (rc = domain_viommu_init(d, config->arch.viommu_type)) != 0 )
>> +goto fail;
>> +
>> return 0;
>>
>> fail:
>> @@ -998,6 +1009,7 @@ static int relinquish_memory(struct domain *d, struct
>> page_list_head *list)
>> enum {
>> PROG_pci = 1,
>> PROG_tee,
>> +PROG_viommu,
>> PROG_xen,
>> PROG_page,
>> PROG_mapping,
>> @@ -1048,6 +1060,11 @@ int domain_relinquish_resources(struct domain *d)
>> if (ret )
>> return ret;
>>
>> +PROGRESS(viommu):
>> +ret = viommu_relinquish_resources(d);
>> +if (ret )
>> +return ret;
>> +
>> PROGRESS(xen):
>> ret = relinquish_memory(d, &d->xenpage_list);
>> if ( ret )
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index bd30d3798c..abbaf37a2e 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -27,6 +27,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>> #include
>> @@ -3858,6 +3859,7 @@ void __init create_domUs(void)
>> struct domain *d;
>> struct xen_domctl_createdomain d_cfg = {
>> .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
>> +.arch.viommu_type = viommu_get_type(),
>> .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
>> /*
>> * The default of 1023 should be sufficient for guests because
>> @@ -4052,6 +4054,7 @@ void __init create_dom0(void)
>> printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
>> dom0_cfg.arch.tee_type = tee_get_type();
>> dom0_cfg.max_vcpus = dom0_max_vcpus();
>> +dom0_cfg.arch.viommu_type = viommu_get_type();
>>
>> if ( iommu_enabled )
>> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>> diff --git a/xen/arch/arm/include/asm/viommu.h
>> b/xen/arch/arm/include/asm/viommu.h
>> new file mode 100644
>> index 00..7cd3818a12
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/viommu.h
>> @@ -0,0 +1,70 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
>> +#ifndef __ARCH_ARM_VIOMMU_H__
>> +#define __ARCH_ARM_VIOMMU_H__
>> +
>> +#ifdef CONFIG_VIRTUAL_IOMMU
>> +
>> +#include
>> +#include
>> +#include
>> +
>> +struct viommu_ops {
>> +/*
>> + * Called during domain construction if toolstack requests to enable
>> + * vIOMMU support.
>> + */
>> +int (*domain_init)(struct domain *d);
>> +
>> +/*
>> + * Called during domain destruction to free resources used by vIOMMU.
>> + */
>> +int (*relinquish_resources)(struct domain *d);
>> +};
>> +
>> +struct viommu_desc {
>> +/* vIOMMU domains init/free operations described above. */
>> +const struct viommu_ops *ops;
>> +
>> +/*
>> + * ID of vIOMMU. Corresponds to xen_arch_domainconfig.viommu_type.
>> + * Should be one of XEN_DOMCTL_CONFIG_VIOMMU_xxx
>> + */
>> +uint16_t viommu_type;
> Here the viommu_type is uint16_t ...
>
>> +};
>> +
>> +int domain_viommu_init(struct domain *d, uint16_t viommu_type);
>> +int viommu_relinquish_resources(struct domain *d);
>> +uint16_t viommu_get_type(void);
>> +
>> +#else
>> +
>> +static inline uint8_t viommu_get_type(void)
> Here you are returning uint8_t ...
>
>> +{
>> +return XEN_DOMCTL_CONFIG_VIOMMU_NONE;
>> +}
>>
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
Hi Julien,
> On 3 Dec 2022, at 9:54 pm, Julien Grall wrote:
>
> Hi Rahul,
>
> On 01/12/2022 16:02, Rahul Singh wrote:
>> This patch adds basic framework for vIOMMU.
>> Signed-off-by: Rahul Singh
>> ---
>> xen/arch/arm/domain.c| 17 +++
>> xen/arch/arm/domain_build.c | 3 ++
>> xen/arch/arm/include/asm/viommu.h| 70
>> xen/drivers/passthrough/Kconfig | 6 +++
>> xen/drivers/passthrough/arm/Makefile | 1 +
>> xen/drivers/passthrough/arm/viommu.c | 48 +++
>> xen/include/public/arch-arm.h| 4 ++
>> 7 files changed, 149 insertions(+)
>> create mode 100644 xen/arch/arm/include/asm/viommu.h
>> create mode 100644 xen/drivers/passthrough/arm/viommu.c
>> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 38e22f12af..2a85209736 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -37,6 +37,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>#include "vpci.h"
>> @@ -691,6 +692,13 @@ int arch_sanitise_domain_config(struct
>> xen_domctl_createdomain *config)
>> return -EINVAL;
>> }
>> +if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
>> +{
>> +dprintk(XENLOG_INFO,
>> +"vIOMMU type requested not supported by the platform or
>> Xen\n");
>> +return -EINVAL;
>> +}
>> +
>> return 0;
>> }
>> @@ -783,6 +791,9 @@ int arch_domain_create(struct domain *d,
>> if ( (rc = domain_vpci_init(d)) != 0 )
>> goto fail;
>> +if ( (rc = domain_viommu_init(d, config->arch.viommu_type)) != 0 )
>> +goto fail;
>> +
>> return 0;
>>fail:
>> @@ -998,6 +1009,7 @@ static int relinquish_memory(struct domain *d, struct
>> page_list_head *list)
>> enum {
>> PROG_pci = 1,
>> PROG_tee,
>> +PROG_viommu,
>> PROG_xen,
>> PROG_page,
>> PROG_mapping,
>> @@ -1048,6 +1060,11 @@ int domain_relinquish_resources(struct domain *d)
>> if (ret )
>> return ret;
>> +PROGRESS(viommu):
>> +ret = viommu_relinquish_resources(d);
>> +if (ret )
>> +return ret;
>
> I would have expected us to relinquish the vIOMMU resources *before* we
> detach the devices. So can you explain the ordering?
I think first we need to detach the device that will set the S1 and S2 stage
translation to bypass/abort then we
need to remove the vIOMMU. Do you have anything that you feel if we relinquish
the vIOMMU after detach is not right.
>
>> +
>> PROGRESS(xen):
>> ret = relinquish_memory(d, &d->xenpage_list);
>> if ( ret )
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index bd30d3798c..abbaf37a2e 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -27,6 +27,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>#include
>> @@ -3858,6 +3859,7 @@ void __init create_domUs(void)
>> struct domain *d;
>> struct xen_domctl_createdomain d_cfg = {
>> .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
>> +.arch.viommu_type = viommu_get_type(),
>
> I don't think the vIOMMU should be enabled to dom0less domU by default.
I am not enabling the vIOMMU by default. If vIOMMU is disabled via the command
line or config option
then viommu_get_type() will return XEN_DOMCTL_CONFIG_VIOMMU_NONE and in that
case
domain_viommu_init() will return 0 without doing anything.
>
>> .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
>> /*
>> * The default of 1023 should be sufficient for guests because
>> @@ -4052,6 +4054,7 @@ void __init create_dom0(void)
>> printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
>> dom0_cfg.arch.tee_type = tee_get_type();
>> dom0_cfg.max_vcpus = dom0_max_vcpus();
>> +dom0_cfg.arch.viommu_type = viommu_get_type();
>
> Same here.
>
>>if ( iommu_enabled )
>> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>> diff --git a/xen/arch/arm/include/asm/viommu.h
>> b/xen/arch/arm/include/asm/viommu.h
>> new file mode 100644
>> index 00..7cd3818a12
>> --- /dev/null
>> +++ b/xen/arch/arm/include/asm/viommu.h
>> @@ -0,0 +1,70 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
>> +#ifndef __ARCH_ARM_VIOMMU_H__
>> +#define __ARCH_ARM_VIOMMU_H__
>> +
>> +#ifdef CONFIG_VIRTUAL_IOMMU
>> +
>> +#include
>> +#include
>> +#include
>> +
>> +struct viommu_ops {
>> +/*
>> + * Called during domain construction if toolstack requests to enable
>> + * vIOMMU support.
>> + */
>> +int (*domain_init)(struct domain *d);
>> +
>> +/*
>> + * Called during domain destruction to free resources used by vIOMMU.
>> + */
>> +int (*relinquish_resources)(struct domain *d);
>> +};
>> +
>> +struct viommu_desc {
>> +/* vIOMMU domains init/free opera
Re: [PATCH] x86/HVM: don't mark evtchn upcall vector as pending when vLAPIC is disabled
On 21.11.2022 13:23, Andrew Cooper wrote:
> On 21/11/2022 08:56, Jan Beulich wrote:
>> On 18.11.2022 15:27, Andrew Cooper wrote:
>>> On 18/11/2022 12:54, Jan Beulich wrote:
On 18.11.2022 13:33, Andrew Cooper wrote:
> On 18/11/2022 10:31, Jan Beulich wrote:
>> Linux'es relatively new use of HVMOP_set_evtchn_upcall_vector has
>> exposed a problem with the marking of the respective vector as
>> pending: For quite some time Linux has been checking whether any stale
>> ISR or IRR bits would still be set while preparing the LAPIC for use.
>> This check is now triggering on the upcall vector, as the registration,
>> at least for APs, happens before the LAPIC is actually enabled.
>>
>> In software-disabled state an LAPIC would not accept any interrupt
>> requests and hence no IRR bit would newly become set while in this
>> state. As a result it is also wrong for us to mark the upcall vector as
>> having a pending request when the vLAPIC is in this state.
> I agree with this.
>
>> To compensate for the "enabled" check added to the assertion logic, add
>> logic to (conditionally) mark the upcall vector as having a request
>> pending at the time the LAPIC is being software-enabled by the guest.
> But this, I don't think is appropriate.
>
> The point of raising on enable is allegedly to work around setup race
> conditions. I'm unconvinced by this reasoning, but it is what it is,
> and the stated behaviour is to raise there and then.
>
> If a guest enables evtchn while the LAPIC is disabled, then the
> interrupt is lost. Like every other interrupt in an x86 system.
Edge triggered ones you mean, I suppose, but yes.
>>> For IO-APIC systems, you mostly lose line interrupts too, don't you?
>>>
>>> The line will remain pending at the IO-APIC, but nothing in the system
>>> will unwedge until someone polls the IO-APIC.
>>>
>>> Either way...
>>>
> I don't think there is any credible way a guest kernel author can expect
> the weird evtchn edgecase to wait for an arbitrary point in the future,
> and it's a corner case that I think is worth not keeping.
Well - did you look at 7b5b8ca7dffd ("x86/upcall: inject a spurious event
after setting upcall vector"), referenced by the Fixes: tag? The issue is
that with evtchn_upcall_pending once set, there would never again be a
notification.
>>> Ok, so we do need to do something.
>>>
So if what you say is to be the model we follow, then that
earlier change was perhaps wrong as well. Instead it should then have
been a guest change (as also implicit from your reply) to clear
evtchn_upcall_pending after vCPU info registration (there) or LAPIC
enabling (here), perhaps by way of "manually" invoking the handling of
that pending event, or by issuing a self-IPI with that vector.
Especially the LAPIC enabling case would then be yet another Xen-specific
on a guest code path which better wouldn't have to be aware of Xen.
>>> Without trying to prescribe how to fix this specific issue, wherever
>>> possible we should be trying to limit the Xen-isms from non-Xen areas.
>>> There's a whole lot of poorly described and surprising behaviours which
>>> have not stood the test of time.
>>>
>>> In this case, it seems that we have yet another x86 PV-ism which hasn't
>>> translated well x86 HVM. Specifically, we're trying to overlay an
>>> entirely shared-memory (and delayed return-to-guest) interrupt
>>> controller onto one which is properly constructed to handle events in
>>> realtime.
>>>
>>>
>>> I even got as far as writing that maybe leaving it as-is was the best
>>> option (principle of least surprise for Xen developers), but our
>>> "friend" apic acceleration strikes again.
>>>
>>> Xen doesn't always get a VMExit when the guest clears SW_DISABLE,
>>> because microcode may have accelerated it.
>> But as per "APIC-Write Emulation" in the SDM we'd still get an APIC-write
>> VM exit.
>
> Intel isn't the only accelerated implementation, and there future
> details not in the public docs.
>
> There will be an implementation we will want to support where Xen
> doesn't get a vmexit for a write to SPIV.
To cover for that, as just discussed, I've added
"Note however that, like for the pt_may_unmask_irq() we already have
there, long term we may need to find a different solution. This will be
especially relevant in case yet better LAPIC acceleration would
eliminate notifications of guest writes to this and other registers."
to the last paragraph of the commit message.
Jan
[XEN v5 08/11] xen/Arm: GICv3: Define ICH_AP0R and ICH_AP1R for AArch32
Adapt save_aprn_regs()/restore_aprn_regs() for AArch32. For which we have defined the following registers:- 1. Interrupt Controller Hyp Active Priorities Group0 Registers 0-3 2. Interrupt Controller Hyp Active Priorities Group1 Registers 0-3 Signed-off-by: Ayan Kumar Halder --- Changes from :- v1 - 1. Moved coproc register definition to asm/cpregs.h. v2 - 1. Defined register alias. 2. Style issues. 3. Dropped R-b and Ack. v3 - 1. Style issues. v4 - 1. Replaced ___CP32(foo) with foo. xen/arch/arm/include/asm/cpregs.h | 28 1 file changed, 28 insertions(+) diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h index 7550fb25f5..4476c9f11b 100644 --- a/xen/arch/arm/include/asm/cpregs.h +++ b/xen/arch/arm/include/asm/cpregs.h @@ -259,6 +259,26 @@ #define VBARp15,0,c12,c0,0 /* Vector Base Address Register */ #define HVBAR p15,4,c12,c0,0 /* Hyp. Vector Base Address Register */ +/* + * CP15 CR12: Interrupt Controller Hyp Active Priorities Group 0 Registers, + * n = 0 - 3 + */ +#define __AP0Rx(x) p15, 4, c12, c8, x +#define ICH_AP0R0 __AP0Rx(0) +#define ICH_AP0R1 __AP0Rx(1) +#define ICH_AP0R2 __AP0Rx(2) +#define ICH_AP0R3 __AP0Rx(3) + +/* + * CP15 CR12: Interrupt Controller Hyp Active Priorities Group 1 Registers, + * n = 0 - 3 + */ +#define __AP1Rx(x) p15, 4, c12, c9, x +#define ICH_AP1R0 __AP1Rx(0) +#define ICH_AP1R1 __AP1Rx(1) +#define ICH_AP1R2 __AP1Rx(2) +#define ICH_AP1R3 __AP1Rx(3) + /* CP15 CR12: Interrupt Controller List Registers, n = 0 - 15 */ #define __LR0(x)p15, 4, c12, c12, x #define __LR8(x)p15, 4, c12, c13, x @@ -359,6 +379,14 @@ #define HCR_EL2 HCR #define HPFAR_EL2 HPFAR #define HSTR_EL2HSTR +#define ICH_AP0R0_EL2 ICH_AP0R0 +#define ICH_AP0R1_EL2 ICH_AP0R1 +#define ICH_AP0R2_EL2 ICH_AP0R2 +#define ICH_AP0R3_EL2 ICH_AP0R3 +#define ICH_AP1R0_EL2 ICH_AP1R0 +#define ICH_AP1R1_EL2 ICH_AP1R1 +#define ICH_AP1R2_EL2 ICH_AP1R2 +#define ICH_AP1R3_EL2 ICH_AP1R3 #define ICH_LR0_EL2 ICH_LR0 #define ICH_LR1_EL2 ICH_LR1 #define ICH_LR2_EL2 ICH_LR2 -- 2.17.1
[XEN v5 09/11] xen/Arm: GICv3: Define remaining GIC registers for AArch32
Define missing assembly aliases for GIC registers on arm32, taking the ones defined already for arm64 as a base. Aliases are defined according to the GIC Architecture Specification ARM IHI 0069H. Defined the following registers:- 1. Interrupt Controller Interrupt Priority Mask Register 2. Interrupt Controller System Register Enable register 3. Interrupt Controller Deactivate Interrupt Register 4. Interrupt Controller End Of Interrupt Register 1 5. Interrupt Controller Interrupt Acknowledge Register 1 6. Interrupt Controller Binary Point Register 1 7. Interrupt Controller Control Register 8. Interrupt Controller Interrupt Group 1 Enable register 9. Interrupt Controller Maintenance Interrupt State Register 10. Interrupt Controller End of Interrupt Status Register 11. Interrupt Controller Empty List Register Status Register 12. Interrupt Controller Virtual Machine Control Register Signed-off-by: Ayan Kumar Halder Reviewed-by: Michal Orzel --- Changes from :- v1 - 1. Moved coproc regs definition to asm/cpregs.h v2 - 1. Defined register alias. 2. Style issues. 3. Defined ELSR, MISR, EISR to make it consistent with AArch64. v3 - 1. Rectified some of the register names. v4 - 1. Placed ICC_DIR after VBAR. 2. Added Rb. xen/arch/arm/include/asm/cpregs.h | 32 +++ 1 file changed, 32 insertions(+) diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h index 4476c9f11b..6b083de204 100644 --- a/xen/arch/arm/include/asm/cpregs.h +++ b/xen/arch/arm/include/asm/cpregs.h @@ -161,6 +161,7 @@ #define DACRp15,0,c3,c0,0 /* Domain Access Control Register */ /* CP15 CR4: */ +#define ICC_PMR p15,0,c4,c6,0 /* Interrupt Priority Mask Register */ /* CP15 CR5: Fault Status Registers */ #define DFSRp15,0,c5,c0,0 /* Data Fault Status Register */ @@ -257,6 +258,7 @@ #define ICC_ASGI1R p15,1,c12 /* Interrupt Controller Alias SGI Group 1 Register */ #define ICC_SGI0R p15,2,c12 /* Interrupt Controller SGI Group 0 */ #define VBARp15,0,c12,c0,0 /* Vector Base Address Register */ +#define ICC_DIR p15,0,c12,c11,1 /* Interrupt Controller Deactivate Interrupt Register */ #define HVBAR p15,4,c12,c0,0 /* Hyp. Vector Base Address Register */ /* @@ -279,6 +281,20 @@ #define ICH_AP1R2 __AP1Rx(2) #define ICH_AP1R3 __AP1Rx(3) +#define ICC_IAR1p15,0,c12,c12,0 /* Interrupt Controller Interrupt Acknowledge Register 1 */ +#define ICC_EOIR1 p15,0,c12,c12,1 /* Interrupt Controller End Of Interrupt Register 1 */ +#define ICC_BPR1p15,0,c12,c12,3 /* Interrupt Controller Binary Point Register 1 */ +#define ICC_CTLRp15,0,c12,c12,4 /* Interrupt Controller Control Register */ +#define ICC_SRE p15,0,c12,c12,5 /* Interrupt Controller System Register Enable register */ +#define ICC_IGRPEN1 p15,0,c12,c12,7 /* Interrupt Controller Interrupt Group 1 Enable register */ +#define ICC_HSREp15,4,c12,c9,5 /* Interrupt Controller Hyp System Register Enable register */ +#define ICH_HCR p15,4,c12,c11,0 /* Interrupt Controller Hyp Control Register */ +#define ICH_VTR p15,4,c12,c11,1 /* Interrupt Controller VGIC Type Register */ +#define ICH_MISRp15,4,c12,c11,2 /* Interrupt Controller Maintenance Interrupt State Register */ +#define ICH_EISRp15,4,c12,c11,3 /* Interrupt Controller End of Interrupt Status Register */ +#define ICH_ELRSR p15,4,c12,c11,5 /* Interrupt Controller Empty List Register Status Register */ +#define ICH_VMCRp15,4,c12,c11,7 /* Interrupt Controller Virtual Machine Control Register */ + /* CP15 CR12: Interrupt Controller List Registers, n = 0 - 15 */ #define __LR0(x)p15, 4, c12, c12, x #define __LR8(x)p15, 4, c12, c13, x @@ -379,6 +395,15 @@ #define HCR_EL2 HCR #define HPFAR_EL2 HPFAR #define HSTR_EL2HSTR +#define ICC_BPR1_EL1ICC_BPR1 +#define ICC_CTLR_EL1ICC_CTLR +#define ICC_DIR_EL1 ICC_DIR +#define ICC_EOIR1_EL1 ICC_EOIR1 +#define ICC_IGRPEN1_EL1 ICC_IGRPEN1 +#define ICC_PMR_EL1 ICC_PMR +#define ICC_SGI1R_EL1 ICC_SGI1R +#define ICC_SRE_EL1 ICC_SRE +#define ICC_SRE_EL2 ICC_HSRE #define ICH_AP0R0_EL2 ICH_AP0R0 #define ICH_AP0R1_EL2 ICH_AP0R1 #define ICH_AP0R2_EL2 ICH_AP0R2 @@ -387,6 +412,10 @@ #define ICH_AP1R1_EL2 ICH_AP1R1 #define ICH_AP1R2_EL2 ICH_AP1R2 #define ICH_AP1R3_EL2 ICH_AP1R3 +#define ICH_EISR_EL2ICH_EISR +#define ICH_ELRSR_EL2 ICH_ELRSR +#define ICH_HCR_EL2 ICH_HCR +#define ICC_IAR1_EL1ICC_IAR1 #define ICH_LR0_EL2 ICH_LR0 #define ICH_LR1_EL2 ICH_LR1 #define ICH_LR2_EL2 ICH_LR2 @@ -419,6 +448,9 @@ #define ICH_LRC13_EL2
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
Hi Jan, > On 2 Dec 2022, at 8:39 am, Jan Beulich wrote: > > On 01.12.2022 17:02, Rahul Singh wrote: >> --- a/xen/drivers/passthrough/Kconfig >> +++ b/xen/drivers/passthrough/Kconfig >> @@ -35,6 +35,12 @@ config IPMMU_VMSA >> (H3 ES3.0, M3-W+, etc) or Gen4 SoCs which IPMMU hardware supports stage 2 >> translation table format and is able to use CPU's P2M table as is. >> >> +config VIRTUAL_IOMMU >> + bool "Virtual IOMMU Support (UNSUPPORTED)" if UNSUPPORTED >> + default n >> + help >> + Support virtual IOMMU infrastructure to implement vIOMMU. > > I simply "virtual" specific enough in the name? Seeing that there are > multiple IOMMU flavors for Arm, and judging from the titles of subsequent > patches, you're implementing a virtualized form of only one variant. I agree with you I will remove the virtual in next version. > > Also, nit: Please omit "default n" here - it leads to a needless > line in the resulting .config, which in addition prevents the prompt > from appearing for user selection when someone later enables > UNSUPPORTED in their config and then runs e.g. "make oldconfig". But > perhaps you anyway really mean > > config VIRTUAL_IOMMU > bool "Virtual IOMMU Support (UNSUPPORTED)" > depends on UNSUPPORTED > help > Support virtual IOMMU infrastructure to implement vIOMMU. > > ? > > Note (nit again) the slightly altered indentation I'm also using in > the alternative suggestion. > I will modify as below: config VIRTUAL_IOMMU bool "Virtual IOMMU Support (UNSUPPORTED)” depends on UNSUPPORTED help Support IOMMU infrastructure to implement different variants of virtual IOMMUs. Regards, Rahul
[XEN v5 02/11] xen/Arm: GICv3: Do not calculate affinity level 3 for AArch32
Refer ARM DDI 0487I.a ID081822, G8-9817, G8.2.169
Affinity level 3 is not present in AArch32.
Also, refer ARM DDI 0406C.d ID040418, B4-1644, B4.1.106,
Affinity level 3 is not present in Armv7 (ie arm32).
Thus, any access to affinity level 3 needs to be guarded within
"ifdef CONFIG_ARM_64".
Signed-off-by: Ayan Kumar Halder
Reviewed-by: Michal Orzel
Acked-by: Julien Grall
---
Changes from -
v1 - NA (as it is a new patch)
v2 - NA (as it is a new patch)
v3 - Modified the title. Added Rb.
v4 - Added Ack.
xen/arch/arm/gic-v3.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index 018fa0dfa0..64a76307dd 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -527,7 +527,10 @@ static void gicv3_set_pending_state(struct irq_desc *irqd,
bool pending)
static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
{
uint64_t mpidr = cpu_logical_map(cpu);
- return (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 |
+ return (
+#ifdef CONFIG_ARM_64
+ MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 |
+#endif
MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 |
MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 |
MPIDR_AFFINITY_LEVEL(mpidr, 0));
@@ -720,7 +723,10 @@ static int __init gicv3_populate_rdist(void)
* Convert affinity to a 32bit value that can be matched to GICR_TYPER
* bits [63:32]
*/
-aff = (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 |
+aff = (
+#ifdef CONFIG_ARM_64
+ MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 |
+#endif
MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 |
MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 |
MPIDR_AFFINITY_LEVEL(mpidr, 0));
@@ -972,7 +978,10 @@ static void gicv3_send_sgi_list(enum gic_sgi sgi, const
cpumask_t *cpumask)
* Prepare affinity path of the cluster for which SGI is generated
* along with SGI number
*/
-val = (MPIDR_AFFINITY_LEVEL(cluster_id, 3) << 48 |
+val = (
+#ifdef CONFIG_ARM_64
+ MPIDR_AFFINITY_LEVEL(cluster_id, 3) << 48 |
+#endif
MPIDR_AFFINITY_LEVEL(cluster_id, 2) << 32 |
sgi << 24 |
MPIDR_AFFINITY_LEVEL(cluster_id, 1) << 16 |
--
2.17.1
[XEN v5 01/11] xen/Arm: vGICv3: Sysreg emulation is applicable for AArch64 only
Sysreg emulation is 64-bit specific, so guard the calls to
vgic_v3_emulate_sysreg() as well as the function itself with
"#ifdef CONFIG_ARM_64".
Signed-off-by: Ayan Kumar Halder
Reviewed-by: Michal Orzel
Acked-by: Julien Grall
---
Changes from -
v1 - 1. Updated the commit message.
v2 - 1. Updated the commit message (removed the reference to Arm ARM as it is
not required).
v3 - No changes. Added Rb and Ack.
v4 - No changes.
xen/arch/arm/vgic-v3.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 015446be17..3f4509dcd3 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1519,6 +1519,7 @@ static bool vgic_v3_emulate_sgi1r(struct cpu_user_regs
*regs, uint64_t *r,
}
}
+#ifdef CONFIG_ARM_64
static bool vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
{
struct hsr_sysreg sysreg = hsr.sysreg;
@@ -1539,6 +1540,7 @@ static bool vgic_v3_emulate_sysreg(struct cpu_user_regs
*regs, union hsr hsr)
return false;
}
}
+#endif
static bool vgic_v3_emulate_cp64(struct cpu_user_regs *regs, union hsr hsr)
{
@@ -1562,8 +1564,10 @@ static bool vgic_v3_emulate_reg(struct cpu_user_regs
*regs, union hsr hsr)
{
switch (hsr.ec)
{
+#ifdef CONFIG_ARM_64
case HSR_EC_SYSREG:
return vgic_v3_emulate_sysreg(regs, hsr);
+#endif
case HSR_EC_CP15_64:
return vgic_v3_emulate_cp64(regs, hsr);
default:
--
2.17.1
[XEN v5 00/11] Arm: Enable GICv3 for AArch32
Hi All,
Please find the following patches to enable GICv3 for AArch32.
This is a pre-requisite to support Xen on Cortex-R52 (AArch32-v8R system)
Let me know your thoughts.
Changes from -
v1 :-
1. Updated in the changelog for each of the patches.
v2 :-
1. Dropped "xen/Arm: GICv3: Move the macros to compute the affnity level to
arm64/arm32". The reason being aff3 does not exist on arm32. And aff0..2 is
the same between arm32, AArch32 and AArch64.
2. Introduce a new patch "xen/Arm: GICv3: Adapt access to VMPIDR register for
AArch32".
3. For the new registers introduced, we have defined the arm32 name and then
an alias.
4. Use 'AArch32' across all the patches.
5. Dropped the 'R-b' and 'Ack' in "[XEN v3 08/12] xen/Arm: GICv3: Define
ICH_AP0R and ICH_AP1R for AArch32".
v3 :-
1. "xen/Arm: GICv3: Use ULL instead of UL for 64bits" has been dropped.
The change has been merged with "xen/Arm: GICv3: Define ICH_LR_EL2 on
AArch32".
2. I have marked the patches which have been Rb + Ack vs Rb only.
3. Dropped Rb from "xen/Arm: GICv3: Enable GICv3 for AArch32"
v4 :-
1. The first six patches have been reviewed and acked.
2. Dropped "Rb michal.or...@amd.com" from patches 7/11, 8/11.
5. Kept "Rb michal.or...@amd.com" on patch 11/11 as the change seemed trivial.
Ayan Kumar Halder (11):
These patches have been reviewed and acked, so the reviewer may choose to
ignore :-
xen/Arm: vGICv3: Sysreg emulation is applicable for AArch64 only
xen/Arm: GICv3: Do not calculate affinity level 3 for AArch32
xen/Arm: vreg: Support vreg_reg64_* helpers on AArch32
xen/Arm: vGICv3: Adapt emulation of GICR_TYPER for AArch32
xen/Arm: GICv3: Fix GICR_{PENDBASER, PROPBASER} emulation on 32-bit
host
xen/Arm: vGICv3: Fix emulation of ICC_SGI1R on AArch32
These patches are yet to be reviewed and acked :-
xen/Arm: GICv3: Define ICH_LR_EL2 on AArch32
xen/Arm: GICv3: Define ICH_AP0R and ICH_AP1R for AArch32
xen/Arm: GICv3: Define macros to read/write 64 bit
These patches are yet to be acked :-
xen/Arm: GICv3: Define remaining GIC registers for AArch32
xen/Arm: GICv3: Enable GICv3 for AArch32
SUPPORT.md | 7 ++
xen/arch/arm/Kconfig | 9 +-
xen/arch/arm/gic-v3.c| 153 ---
xen/arch/arm/include/asm/arm32/io.h | 20 +++
xen/arch/arm/include/asm/arm32/sysregs.h | 19 +++
xen/arch/arm/include/asm/arm64/io.h | 2 +
xen/arch/arm/include/asm/arm64/sysregs.h | 5 +
xen/arch/arm/include/asm/cpregs.h| 134
xen/arch/arm/include/asm/cpufeature.h| 1 +
xen/arch/arm/include/asm/gic_v3_defs.h | 24 ++--
xen/arch/arm/include/asm/vreg.h | 86 +++--
xen/arch/arm/vgic-v3.c | 22 +++-
12 files changed, 322 insertions(+), 160 deletions(-)
--
2.17.1
Re: [PATCH 1/5] x86/platform: introduce hypercall to get initial video console settings
On 23.11.2022 16:45, Roger Pau Monne wrote: > This is required so PVH dom0 can get the initial video console state > as handled by Xen. PV dom0 will get this as part of the start_info, > but it doesn't seem necessary to place such information in the > HVM start info. > > Signed-off-by: Roger Pau Monné I'm okay with this as is, so Reviewed-by: Jan Beulich but ... > xen/arch/x86/platform_hypercall.c | 11 +++ > xen/drivers/video/vga.c | 2 +- > xen/include/public/platform.h | 6 ++ > 3 files changed, 18 insertions(+), 1 deletion(-) ... wasn't the goal for Arm to have the same interface? Jan
Re: [PATCH v5 1/2] xen/arm: vpl011: emulate non-SBSA registers as WI/RAZ
Hi Jiamei, On 05/12/2022 08:26, Jiamei Xie wrote: > > > When the guest kernel enables DMA engine with "CONFIG_DMA_ENGINE=y", > Linux SBSA PL011 driver will access PL011 DMACR register in some > functions. As chapter "B Generic UART" in "ARM Server Base System > Architecture"[1] documentation describes, SBSA UART doesn't support > DMA. In current code, when the kernel tries to access DMACR register, > Xen will inject a data abort: > Unhandled fault at 0xffc00944d048 > Mem abort info: > ESR = 0x9600 > EC = 0x25: DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > FSC = 0x00: ttbr address size fault > Data abort info: > ISV = 0, ISS = 0x > CM = 0, WnR = 0 > swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000 > [ffc00944d048] pgd=10003803, p4d=10003803, > pud=10003803, pmd=10003fffa803, pte=00689c090f13 > Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP > ... > Call trace: > pl011_stop_rx+0x70/0x80 > tty_port_shutdown+0x7c/0xb4 > tty_port_close+0x60/0xcc > uart_close+0x34/0x8c > tty_release+0x144/0x4c0 > __fput+0x78/0x220 > fput+0x1c/0x30 > task_work_run+0x88/0xc0 > do_notify_resume+0x8d0/0x123c > el0_svc+0xa8/0xc0 > el0t_64_sync_handler+0xa4/0x130 > el0t_64_sync+0x1a0/0x1a4 > Code: b983 b901f001 794038a0 8b42 (b941) > ---[ end trace 83dd93df15c3216f ]--- > note: bootlogd[132] exited with preempt_count 1 > /etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon > > As discussed in [2], this commit makes the access to non-SBSA registers > RAZ/WI as an improvement. > > [1] https://developer.arm.com/documentation/den0094/c/?lang=en > [2] > https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-desktop/ > > Signed-off-by: Jiamei Xie As I wrote in v4: Reviewed-by: Michal Orzel ~Michal
Re: [PATCH v8 2/3] freezer: refactor pm_freezing into a function.
Hi Rafael
On Fri, 2 Dec 2022 at 18:48, Rafael J. Wysocki wrote:
>
> On Thu, Dec 1, 2022 at 12:08 PM Ricardo Ribalda wrote:
> >
> > Add a way to let the drivers know if the processes are frozen.
> >
> > This is needed by drivers that are waiting for processes to end on their
> > shutdown path.
> >
> > Convert pm_freezing into a function and export it, so it can be used by
> > drivers that are either built-in or modules.
> >
> > Cc: sta...@vger.kernel.org
> > Fixes: 83bfc7e793b5 ("ASoC: SOF: core: unregister clients and machine
> > drivers in .shutdown")
> > Signed-off-by: Ricardo Ribalda
>
> Why can't you export the original pm_freezing variable and why is this
> fixing anything?
Because then any module will be able to modify the content of the variable.
The Fixes: is because the last patch on the set is doing a real fix.
If you only cherry-pick the last patch on a stable branch, the build
will fail. (Also, the zero-day builder complains)
Anyway, I think we can hold this patch for a bit. The snd people are
discussing if this the way to handle it, or if we should handle
.shutdown in a different way.
Thanks!
>
> > ---
> > include/linux/freezer.h | 3 ++-
> > kernel/freezer.c| 3 +--
> > kernel/power/process.c | 24
> > 3 files changed, 23 insertions(+), 7 deletions(-)
> >
> > diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> > index b303472255be..3413c869d68b 100644
> > --- a/include/linux/freezer.h
> > +++ b/include/linux/freezer.h
> > @@ -13,7 +13,7 @@
> > #ifdef CONFIG_FREEZER
> > DECLARE_STATIC_KEY_FALSE(freezer_active);
> >
> > -extern bool pm_freezing; /* PM freezing in effect */
> > +bool pm_freezing(void);
> > extern bool pm_nosig_freezing; /* PM nosig freezing in effect */
> >
> > /*
> > @@ -80,6 +80,7 @@ static inline int freeze_processes(void) { return
> > -ENOSYS; }
> > static inline int freeze_kernel_threads(void) { return -ENOSYS; }
> > static inline void thaw_processes(void) {}
> > static inline void thaw_kernel_threads(void) {}
> > +static inline bool pm_freezing(void) { return false; }
> >
> > static inline bool try_to_freeze(void) { return false; }
> >
> > diff --git a/kernel/freezer.c b/kernel/freezer.c
> > index 4fad0e6fca64..2d3530ebdb7e 100644
> > --- a/kernel/freezer.c
> > +++ b/kernel/freezer.c
> > @@ -20,7 +20,6 @@ EXPORT_SYMBOL(freezer_active);
> > * indicate whether PM freezing is in effect, protected by
> > * system_transition_mutex
> > */
> > -bool pm_freezing;
> > bool pm_nosig_freezing;
> >
> > /* protects freezing and frozen transitions */
> > @@ -46,7 +45,7 @@ bool freezing_slow_path(struct task_struct *p)
> > if (pm_nosig_freezing || cgroup_freezing(p))
> > return true;
> >
> > - if (pm_freezing && !(p->flags & PF_KTHREAD))
> > + if (pm_freezing() && !(p->flags & PF_KTHREAD))
> > return true;
> >
> > return false;
> > diff --git a/kernel/power/process.c b/kernel/power/process.c
> > index ddd9988327fe..8a4d0e2c8c20 100644
> > --- a/kernel/power/process.c
> > +++ b/kernel/power/process.c
> > @@ -108,6 +108,22 @@ static int try_to_freeze_tasks(bool user_only)
> > return todo ? -EBUSY : 0;
> > }
> >
> > +/*
> > + * Indicate whether PM freezing is in effect, protected by
> > + * system_transition_mutex.
> > + */
> > +static bool pm_freezing_internal;
> > +
> > +/**
> > + * pm_freezing - indicate whether PM freezing is in effect.
> > + *
> > + */
> > +bool pm_freezing(void)
> > +{
> > + return pm_freezing_internal;
> > +}
> > +EXPORT_SYMBOL(pm_freezing);
>
> Use EXPORT_SYMBOL_GPL() instead, please.
>
> > +
> > /**
> > * freeze_processes - Signal user space processes to enter the
> > refrigerator.
> > * The current thread will not be frozen. The same process that calls
> > @@ -126,12 +142,12 @@ int freeze_processes(void)
> > /* Make sure this task doesn't get frozen */
> > current->flags |= PF_SUSPEND_TASK;
> >
> > - if (!pm_freezing)
> > + if (!pm_freezing())
> > static_branch_inc(&freezer_active);
> >
> > pm_wakeup_clear(0);
> > pr_info("Freezing user space processes ... ");
> > - pm_freezing = true;
> > + pm_freezing_internal = true;
> > error = try_to_freeze_tasks(true);
> > if (!error) {
> > __usermodehelper_set_disable_depth(UMH_DISABLED);
> > @@ -187,9 +203,9 @@ void thaw_processes(void)
> > struct task_struct *curr = current;
> >
> > trace_suspend_resume(TPS("thaw_processes"), 0, true);
> > - if (pm_freezing)
> > + if (pm_freezing())
> > static_branch_dec(&freezer_active);
> > - pm_freezing = false;
> > + pm_freezing_internal = false;
> > pm_nosig_freezing = false;
> >
> > oom_killer_enable();
> >
> > --
>
> --
> You received this message because you are subscribed to the Google Groups
> "Chromeos Kdump" group.
> To unsu
RE: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated
Hi Julien, > > Speaking of the checklist and maybe it is slightly off topic: Will it bother > > you or other maintainers if I send a patch to remove the "qemu-iwj" > > section in the branch list? According to the discussion with Ian [1], I > > think > > this can be safely dropped, and I am asking because I think this indeed > > causes some confusion in the 4.17 branching process. > There are a few bits in the branch checklist that needs to be updated. > This include removing the section "qemu-iwj.git". I will aim to send the > series this week. Cool. Thanks for your time :) Kind regards, Henry > > Cheers, > > -- > Julien Grall
Re: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated
On 05/12/2022 11:28, Henry Wang wrote: Hi Julien, Hi Henry, -Original Message- From: Julien Grall Subject: Re: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated Acked-by: Jan Beulich Release-acked-by: Henry Wang Thanks for the release-acked-by. However, I have no plan to port this patch to 4.17 (IMHO, a release technician should always read the unstable checklist). No problem. Sorry for the noise :) Speaking of the checklist and maybe it is slightly off topic: Will it bother you or other maintainers if I send a patch to remove the "qemu-iwj" section in the branch list? According to the discussion with Ian [1], I think this can be safely dropped, and I am asking because I think this indeed causes some confusion in the 4.17 branching process. There are a few bits in the branch checklist that needs to be updated. This include removing the section "qemu-iwj.git". I will aim to send the series this week. Cheers, -- Julien Grall
Re: [XEN v4] xen/page_alloc: Relax the BUILD_BUG_ON() in xenheap_max_mfn()
On 05.12.2022 10:48, Ayan Kumar Halder wrote: > In the near future, we are considering to use a common xen/domain heap for > Arm 32-bit V8-R. In this setup, both PADDR_BITS and BITS_PER_LONG will be > 32. Therefore, the condition PADDR_BITS >= BITS_PER_LONG will become true. > > Looking at the commit that introduce the BUILD_BUG_ON (88e3ed61642b > "x86/NUMA: make init_node_heap() respect Xen heap limit") and the current > use, it seems this check was mainly to ensure that the shift of xenheap_bits > is not going to be undefined (>> N for a N-bit type is undefined). > > So far, all the shifts are using "xenheap_bits - PAGE_SHIFT". Therefore, the > existing code should work for 32-bit architecture as long as the result of > the substraction is below 32. > > Therefore relax the BUILD_BUG_ON() to check that (PADDR_BITS - PAGE_SHIFT) > is not equal of above BITS_PER_LONG. > > Note that we would need further precaution if we ended up to use > 'xenheap_bits' alone in shifts. > > Signed-off-by: Ayan Kumar Halder > Signed-off-by: Julien Grall Acked-by: Jan Beulich
Re: [PATCH net] xen-netfront: Fix NULL sring after live migration
Hello: This patch was applied to netdev/net.git (master) by David S. Miller : On Fri, 2 Dec 2022 08:52:48 + you wrote: > A NAPI is setup for each network sring to poll data to kernel > The sring with source host is destroyed before live migration and > new sring with target host is setup after live migration. > The NAPI for the old sring is not deleted until setup new sring > with target host after migration. With busy_poll/busy_read enabled, > the NAPI can be polled before got deleted when resume VM. > > [...] Here is the summary with links: - [net] xen-netfront: Fix NULL sring after live migration https://git.kernel.org/netdev/net/c/d50b7914fae0 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH V6 1/3] libxl: Add support for generic virtio device
On 05-12-22, 11:45, Viresh Kumar wrote: > > > +rc = libxl__backendpath_parse_domid(gc, be_path, > > > &virtio->backend_domid); > > > +if (rc) goto out; > > > + > > > +rc = libxl__parse_backend_path(gc, be_path, &dev); > > > +if (rc) goto out; > > > > The same question for dev variable. > > Hmm, this we aren't using at all, which KBD does use it. Maybe we > should even call libxl__parse_backend_path() ? Removing it works just fine for me. -- viresh
RE: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated
Hi Julien, > -Original Message- > From: Julien Grall > Subject: Re: [PATCH v2] process/release-technician-checklist: Explain how the > banner in README is generated > >> Acked-by: Jan Beulich > > > > Release-acked-by: Henry Wang > > Thanks for the release-acked-by. However, I have no plan to port this > patch to 4.17 (IMHO, a release technician should always read the > unstable checklist). No problem. Sorry for the noise :) Speaking of the checklist and maybe it is slightly off topic: Will it bother you or other maintainers if I send a patch to remove the "qemu-iwj" section in the branch list? According to the discussion with Ian [1], I think this can be safely dropped, and I am asking because I think this indeed causes some confusion in the 4.17 branching process. [1] https://lore.kernel.org/xen-devel/25482.10006.140155.984...@chiark.greenend.org.uk/ Kind regards, Henry > > Cheers, > > -- > Julien Grall
Re: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated
On 01/12/2022 10:50, Henry Wang wrote: Hi, Hi Henry, -Original Message- Subject: Re: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated On 24.11.2022 20:08, Julien Grall wrote: From: Julien Grall Explain how the banner in README is generated and take the opportunity to mention what it should look like for RC. Signed-off-by: Julien Grall Acked-by: Jan Beulich Release-acked-by: Henry Wang Thanks for the release-acked-by. However, I have no plan to port this patch to 4.17 (IMHO, a release technician should always read the unstable checklist). Cheers, -- Julien Grall
Re: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated
On 05/12/2022 11:16, Julien Grall wrote: Hi Jan, On 25/11/2022 09:17, Jan Beulich wrote: On 24.11.2022 20:08, Julien Grall wrote: From: Julien Grall Explain how the banner in README is generated and take the opportunity to mention what it should look like for RC. Signed-off-by: Julien Grall Acked-by: Jan Beulich albeit with a couple of nits: --- a/docs/process/release-technician-checklist.txt +++ b/docs/process/release-technician-checklist.txt @@ -48,7 +48,12 @@ t=RELEASE-$r * consider bumping sonames of shlibs -* change xen-unstable README (should say "Xen 4.5" in releases and on stable branches, "Xen 4.5-unstable" on unstable) +* change xen-unstable README. Should say: + - "Xen 4.5" in releases and on stable branches + - "Xen 4.5-unstable" on unstable + - "Xen 4.5-rc" for release candidate + +* The banner is generated using figlet * change xen-unstable Config.mk Perhaps insert another blank line between these two bullet points? Or, if they're deemed to belong together, remove the one you insert? I will add another blank line. The new bullet point you add also would likely want to match the others in style, both for the number of spaces after * and for not using a capital first letter (maybe "generate banner using figlet"). Sure. I will address them before committing. I had another look and went with the following: +* change xen-unstable README. The banner (generated using figlet) should say: +- "Xen 4.5" in releases and on stable branches +- "Xen 4.5-unstable" on unstable +- "Xen 4.5-rc" for release candidate + So only one bullet point and using () to explain how the banner is generated. Cheers, -- Julien Grall
Re: [PATCH v2] process/release-technician-checklist: Explain how the banner in README is generated
Hi Jan, On 25/11/2022 09:17, Jan Beulich wrote: On 24.11.2022 20:08, Julien Grall wrote: From: Julien Grall Explain how the banner in README is generated and take the opportunity to mention what it should look like for RC. Signed-off-by: Julien Grall Acked-by: Jan Beulich albeit with a couple of nits: --- a/docs/process/release-technician-checklist.txt +++ b/docs/process/release-technician-checklist.txt @@ -48,7 +48,12 @@ t=RELEASE-$r * consider bumping sonames of shlibs -* change xen-unstable README (should say "Xen 4.5" in releases and on stable branches, "Xen 4.5-unstable" on unstable) +* change xen-unstable README. Should say: +- "Xen 4.5" in releases and on stable branches +- "Xen 4.5-unstable" on unstable +- "Xen 4.5-rc" for release candidate + +* The banner is generated using figlet * change xen-unstable Config.mk Perhaps insert another blank line between these two bullet points? Or, if they're deemed to belong together, remove the one you insert? I will add another blank line. The new bullet point you add also would likely want to match the others in style, both for the number of spaces after * and for not using a capital first letter (maybe "generate banner using figlet"). Sure. I will address them before committing. Cheers, -- Julien Grall
Re: [PATCH 1/2] Reduce hard-coding of PAT value
On Sun, Dec 4, 2022 at 5:14 PM Demi Marie Obenour < d...@invisiblethingslab.com> wrote: > This makes the code easier to read and more robust against any future > changes to this value. No change in behavior (modulo bugs). > Minor tweak: We generally say something like "No functional change intended." No need to respin just for this, but handy to keep in your back pocket for future patches. -George
Re: [PATCH v6 05/11] xen/arm: define Xen start address for FVP BaseR platform
Hi, On 05/12/2022 10:17, Wei Chen wrote: On 2022/11/10 2:24, Julien Grall wrote: diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig index c93a6b2756..0904793a0b 100644 --- a/xen/arch/arm/platforms/Kconfig +++ b/xen/arch/arm/platforms/Kconfig @@ -1,6 +1,7 @@ choice prompt "Platform Support" default ALL_PLAT + default FVP_BASER if ARM_V8R Is there any reason to create a new Kconfig rather than using MPU? Did you mean FVP_BASER? If yes, we want to give each board a MACRO to indicate its specific configurations. In current series, this MACRO only be used for board specific start address. See above for this. If we move board specific information to tailored config file, I think we don't need FVP_BASER. If you meant Armv8R, that's because Armv8R does not equal to MPU. I am not entirely sure to understand. Are you saying that an existing Xen can boot on Armv8R? No, I didn't mean that. I just think we can't use only one MPU or one ARM_V8R to cover all our changes in this series. For example, some changes like new system registers are brought by Armv8R not the MPU. I understand the theory. But in practice this needs to be a balance between finer grain and too much Kconfig. From this series alone, it doesn't seem to be make sense to introduce the two. Furthermore, I am not entirely sure you will be able to make the MPU work without enable the ARMv8-R Kconfig. Cheers, -- Julien Grall
Re: [PATCH 1/2] Reduce hard-coding of PAT value
On 04.12.2022 18:13, Demi Marie Obenour wrote:
> This makes the code easier to read and more robust against any future
> changes to this value. No change in behavior (modulo bugs).
>
> To: Xen developer discussion
Looks like you mean to send this to the list, but you actually sent it
to yourself according to my mail UI. Cc-ing the list now.
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -961,13 +961,10 @@ get_page_from_l1e(
>
> switch ( l1f & PAGE_CACHE_ATTRS )
> {
> -case 0: /* WB */
> -flip |= _PAGE_PWT | _PAGE_PCD;
> -break;
> -case _PAGE_PWT: /* WT */
> -case _PAGE_PWT | _PAGE_PAT: /* WP */
> -flip |= _PAGE_PCD | (l1f & _PAGE_PAT);
> -break;
> +case _PAGE_WB: /* WB */
> +case _PAGE_WT: /* WT */
> +case _PAGE_WP: /* WP */
The comments are now redundant and should hence be dropped, to improve
readability.
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -535,7 +535,7 @@ _sh_propagate(struct vcpu *v,
> if ( guest_nx_enabled(v) )
> pass_thru_flags |= _PAGE_NX_BIT;
> if ( level == 1 && !shadow_mode_refcounts(d) && mmio_mfn )
> -pass_thru_flags |= _PAGE_PAT | _PAGE_PCD | _PAGE_PWT;
> +pass_thru_flags |= PAGE_CACHE_ATTRS;
Personally I think the switch to using PAGE_CACHE_ATTRS (here and
elsewhere) would benefit from being a separate change.
Jan
Re: [XEN v4 09/11] xen/Arm: GICv3: Define remaining GIC registers for AArch32
On 29/11/2022 14:57, Michal Orzel wrote: Hi Ayan, On 28/11/2022 16:56, Ayan Kumar Halder wrote: Define missing assembly aliases for GIC registers on arm32, taking the ones defined already for arm64 as a base. Aliases are defined according to the GIC Architecture Specification ARM IHI 0069H. Defined the following registers:- 1. Interrupt Controller Interrupt Priority Mask Register 2. Interrupt Controller System Register Enable register 3. Interrupt Controller Deactivate Interrupt Register 4. Interrupt Controller End Of Interrupt Register 1 5. Interrupt Controller Interrupt Acknowledge Register 1 6. Interrupt Controller Binary Point Register 1 7. Interrupt Controller Control Register 8. Interrupt Controller Interrupt Group 1 Enable register 9. Interrupt Controller Maintenance Interrupt State Register 10. Interrupt Controller End of Interrupt Status Register 11. Interrupt Controller Empty List Register Status Register 12. Interrupt Controller Virtual Machine Control Register Signed-off-by: Ayan Kumar Halder Reviewed-by: Michal Orzel with one remark... --- Changes from :- v1 - 1. Moved coproc regs definition to asm/cpregs.h v2 - 1. Defined register alias. 2. Style issues. 3. Defined ELSR, MISR, EISR to make it consistent with AArch64. v3 - 1. Rectified some of the register names. xen/arch/arm/include/asm/cpregs.h | 32 +++ 1 file changed, 32 insertions(+) diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h index 53142fc2b2..8f4d097a15 100644 --- a/xen/arch/arm/include/asm/cpregs.h +++ b/xen/arch/arm/include/asm/cpregs.h @@ -163,6 +163,7 @@ #define DACRp15,0,c3,c0,0 /* Domain Access Control Register */ /* CP15 CR4: */ +#define ICC_PMR p15,0,c4,c6,0 /* Interrupt Priority Mask Register */ /* CP15 CR5: Fault Status Registers */ #define DFSRp15,0,c5,c0,0 /* Data Fault Status Register */ @@ -256,6 +257,7 @@ /* CP15 CR12: */ #define ICC_SGI1R p15,0,c12 /* Interrupt Controller SGI Group 1 */ +#define ICC_DIR p15,0,c12,c11,1 /* Interrupt Controller Deactivate Interrupt Register */ Shouldn't ICC_DIR be placed after VBAR? I have moved ICC_DIR after VBAR in my v5 patch. Actually, this ordering is not very clear to me. It defines the following ordering "Coprocessor-> CRn-> Opcode 1-> CRm-> Opcode 2" So "p15, 0, c12 " should come before "p15,1,c12 ...". However, if I see in the file for examples where "crm, opc2" is not present, the order does not seem to make sense. For eg #define TTBCR p15,0,c2,c0,2 /* Translation Table Base Control Register */ #define TTBCR2 p15,0,c2,c0,3 /* Translation Table Base Control Register 2 */ #define TTBR0 p15,0,c2 /* Translation Table Base Reg. 0 */ #define TTBR1 p15,1,c2 /* Translation Table Base Reg. 1 */ #define HTTBR p15,4,c2 /* Hyp. Translation Table Base Register */ #define TTBR0_32 p15,0,c2,c0,0 /* 32-bit access to TTBR0 */ <<<-- does not seem in correct position , it should have been at the top #define TTBR1_32 p15,0,c2,c0,1 /* 32-bit access to TTBR1 */ <<<--- does not seem in correct position , it should have been between TTBR0_32 and TTBCR - Ayan #define ICC_ASGI1R p15,1,c12 /* Interrupt Controller Alias SGI Group 1 Register */ #define ICC_SGI0R p15,2,c12 /* Interrupt Controller SGI Group 0 */ #define VBARp15,0,c12,c0,0 /* Vector Base Address Register */ @@ -281,6 +283,20 @@ #define ICH_AP1R2 __AP1Rx(2) #define ICH_AP1R3 __AP1Rx(3) +#define ICC_IAR1p15,0,c12,c12,0 /* Interrupt Controller Interrupt Acknowledge Register 1 */ +#define ICC_EOIR1 p15,0,c12,c12,1 /* Interrupt Controller End Of Interrupt Register 1 */ +#define ICC_BPR1p15,0,c12,c12,3 /* Interrupt Controller Binary Point Register 1 */ +#define ICC_CTLRp15,0,c12,c12,4 /* Interrupt Controller Control Register */ +#define ICC_SRE p15,0,c12,c12,5 /* Interrupt Controller System Register Enable register */ +#define ICC_IGRPEN1 p15,0,c12,c12,7 /* Interrupt Controller Interrupt Group 1 Enable register */ +#define ICC_HSREp15,4,c12,c9,5 /* Interrupt Controller Hyp System Register Enable register */ +#define ICH_HCR p15,4,c12,c11,0 /* Interrupt Controller Hyp Control Register */ +#define ICH_VTR p15,4,c12,c11,1 /* Interrupt Controller VGIC Type Register */ +#define ICH_MISRp15,4,c12,c11,2 /* Interrupt Controller Maintenance Interrupt State Register */ +#define ICH_EISRp15,4,c12,c11,3 /* Interrupt Controller End of Interrupt Status Register */ +#define ICH_ELRSR p15,4,c12,c11,5 /* Interrupt Controller Empty List Register Status Register */ +#define ICH_VMCRp15,4,c12,c11,7 /* Interrupt Controller Virtual Machine Control Register */ + /* CP15 CR12: Interrupt Co
Re: [PATCH v6 05/11] xen/arm: define Xen start address for FVP BaseR platform
Hi Julien, I thought I had replied to your email, but when I am checking to see if I have addressed all your comments, I didn't find my reply on the mailing list. Maybe there is something wrong with my mail system, I didn't find it sooner. Sorry about it. Hope my reply is not too late. On 2022/11/10 2:24, Julien Grall wrote: On 09/11/2022 04:55, Wei Chen wrote: Hi Julien, Hi Wei, We had considered to use Kconfig to define the start addresses of v8R64 platforms (prompt users to input the address). But we also want to provide a default start address for each platform (Discussed in [1], header for default value, Kconfig option for customized address). Why do you want to provide a default value? And how it is guaranteed that it will work for most of the users? We also had thought to use Kconfig to define a default start address for each platform like what we had done for early printk in RFC[2]. But this method has been deprecated. Most of the current Xen is board agnostic except the UART. We push back the addition of new one because the address can be found in the firmware table and I wanted to avoid increase the number of option (there are dozens of platform out...). So if we don’t use header files, just use the Kconfig, we can't provide a default start address for platforms, and have to force users to enter the start address. I am not sure I see the problem to force the user to enter the start address. My worry with per-platform default value is we end up to force each vendor to provide an header in order to boot Xen. I think it would be better to provide a config tailored for that platform (whether it is part of Xen can be debatable). This would allow a user to try a release Xen on their platform with zero changes in the code. Above comments have been answered in my reply to you and Stefano. diff --git a/xen/arch/arm/platforms/Kconfig b/xen/arch/arm/platforms/Kconfig index c93a6b2756..0904793a0b 100644 --- a/xen/arch/arm/platforms/Kconfig +++ b/xen/arch/arm/platforms/Kconfig @@ -1,6 +1,7 @@ choice prompt "Platform Support" default ALL_PLAT + default FVP_BASER if ARM_V8R Is there any reason to create a new Kconfig rather than using MPU? Did you mean FVP_BASER? If yes, we want to give each board a MACRO to indicate its specific configurations. In current series, this MACRO only be used for board specific start address. See above for this. If we move board specific information to tailored config file, I think we don't need FVP_BASER. If you meant Armv8R, that's because Armv8R does not equal to MPU. I am not entirely sure to understand. Are you saying that an existing Xen can boot on Armv8R? No, I didn't mean that. I just think we can't use only one MPU or one ARM_V8R to cover all our changes in this series. For example, some changes like new system registers are brought by Armv8R not the MPU. Cheers, Wei Chen Cheers,
[XEN v4] xen/page_alloc: Relax the BUILD_BUG_ON() in xenheap_max_mfn()
In the near future, we are considering to use a common xen/domain heap for
Arm 32-bit V8-R. In this setup, both PADDR_BITS and BITS_PER_LONG will be
32. Therefore, the condition PADDR_BITS >= BITS_PER_LONG will become true.
Looking at the commit that introduce the BUILD_BUG_ON (88e3ed61642b
"x86/NUMA: make init_node_heap() respect Xen heap limit") and the current
use, it seems this check was mainly to ensure that the shift of xenheap_bits
is not going to be undefined (>> N for a N-bit type is undefined).
So far, all the shifts are using "xenheap_bits - PAGE_SHIFT". Therefore, the
existing code should work for 32-bit architecture as long as the result of
the substraction is below 32.
Therefore relax the BUILD_BUG_ON() to check that (PADDR_BITS - PAGE_SHIFT)
is not equal of above BITS_PER_LONG.
Note that we would need further precaution if we ended up to use
'xenheap_bits' alone in shifts.
Signed-off-by: Ayan Kumar Halder
Signed-off-by: Julien Grall
---
Currently this change will not have any impact on the existing architectures.
The following table illustrates PADDR_BITS vs BITS_PER_LONG of different archs
| Arch | PADDR_BITS| BITS_PER_LONG |
| Arm_64| 48| 64|
| Arm_32| 40| 32|
| RISCV_64 | 56| 64|
| x86 | 52| 64|
-
However, this will change when we introduce a platform (For eg Cortex-R52) which
supports 32 bit physical address and BITS_PER_LONG. This platform does not
follow
the same code path as Arm_32.
Thus, I have introduced this change as I don't see it causing a regression on
any of the supported platforms.
Changes from v1:-
1. Changed the check from "(PADDR_BITS > BITS_PER_LONG)" to "((PADDR_BITS -
PAGE_SHIFT) >= BITS_PER_LONG)"
2. Updated the commit message to explain the reason for this.
v2 :-
1. Updated the commit message.
v3 :-
1. Updated the commit message.
2. Added Julien's SOB.
xen/common/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 62afb07bc6..c5b8c7444f 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2245,7 +2245,7 @@ void __init xenheap_max_mfn(unsigned long mfn)
{
ASSERT(!first_node_initialised);
ASSERT(!xenheap_bits);
-BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
+BUILD_BUG_ON((PADDR_BITS - PAGE_SHIFT) >= BITS_PER_LONG);
xenheap_bits = min(flsl(mfn + 1) - 1 + PAGE_SHIFT, PADDR_BITS);
printk(XENLOG_INFO "Xen heap: %u bits\n", xenheap_bits);
}
--
2.17.1
[xen-unstable test] 175044: tolerable FAIL
flight 175044 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/175044/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-i386-pair 11 xen-install/dst_host fail in 175040 pass in 175044 test-amd64-amd64-xl-credit1 20 guest-localmigrate/x10 fail in 175040 pass in 175044 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail pass in 175040 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 175040 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175040 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175040 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175040 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175040 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 175040 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 175040 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175040 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 175040 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175040 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175040 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175040 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-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-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-ar
Re: [PATCH V6 3/3] docs: Add documentation for generic virtio devices
On 04-12-22, 20:52, Oleksandr Tyshchenko wrote:
> So as I understand current series adds support for two virtio devices
> (i2c/gpio) that require specific device-tree sub node with specific
> compatible in it [1]. Those backends are standalone userspace applications
> (daemons) that do not require any additional configuration parameters from
> the toolstack other than just virtio-mmio irq and base (please correct me if
> I am wrong).
For now, yes. But we may want to link these devices with other devices
in DT, like GPIO line consumers. I am not pushing a half informed
solution for that right now and that can be taken up later.
> Well, below just some thoughts (which might be wrong) regarding the possible
> extensions for future use. Please note, I do not suggest the following to be
> implemented right now (I mean within the context of current series):
>
> 1. For supporting usual virtio devices that don't require specific
> device-tree sub node with specific compatible in it [2] we would probably
> need to either make "compatible" (or type?) string optional or to reserve
> some value for it ("common" for the instance).
I agree. Maybe we can use "virtio,device" without a number for the
device in this case.
> 2. For supporting Qemu based virtio devices we would probably need to add
> "backendtype" string (with "standalone" value for daemons like yours and
> "qemu" value for Qemu backends).
Hmm, I realize now that my patch did define a new type for this,
libxl_virtio_backend, which defines STANDALONE already, but it isn't
used currently. Maybe I should remove it too.
And I am not sure sure how to use these values, STANDALONE or QEMU.
Should the DT nodes be created only for STANDALONE and never for QEMU
?
Maybe we can add these fields and a config param, once someone wants
to reuse this stuff for QEMU ?
> 3. For supporting additional configuration parameters for Qemu based virtio
> devices we could probably reuse "device_model_args" (although it is not
> clear to me what alternative to use for daemons).
I would leave it for the person who will make use of this eventually,
as then we will have more information on the same.
> > +=item B
>
> Shouldn't it be "type" instead (the parsing code is looking for type and the
> example below suggests the type)?
Yes.
> > +Specifies the compatible string for the specific Virtio device. The same
> > will be
> > +written in the Device Tree compatible property of the Virtio device. For
> > +example, "type=virtio,device22" for the I2C device > +
> > +=item B
> > +
> > +Specifies the transport mechanism for the Virtio device, like "mmio" or
> > "pci".
> > +
> > +=back
> > +
> > =item B
> > B Set TEE type for the guest. TEE is a Trusted Execution
>
> Also the commit description for #1/3 mentions that Virtio backend could run
> in any domain. So looks like the "backend" string is missing here. I would
> add the following:
>
> =item B
>
> Specify the backend domain name or id, defaults to dom0.
I haven't used the backend in any other domain for now, just Dom0, but
the idea is definitely there to run backends in separate user domains.
> P.S. I am wondering do i2c/gpio virtio backends support Xen grant mappings
> for the virtio?
Not yet, we haven't made much progress in that area until now, but it
is very much part of what we intend to do.
> Have you tried to run the backends in non-hardware domain
> with CONFIG_XEN_VIRTIO=y in Linux?
Not yet.
--
viresh
[ovmf test] 175047: all pass - PUSHED
flight 175047 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/175047/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 7bee2498910a9034faaf90802c49188afb7582dc baseline version: ovmf 735a7496cb35e48ccad51aad0934844a475e3fef Last test of basis 175045 2022-12-05 02:10:48 Z0 days Testing same since 175047 2022-12-05 05:40:43 Z0 days1 attempts People who touched revisions under test: Ni, Ray Ray Ni 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 735a7496cb..7bee249891 7bee2498910a9034faaf90802c49188afb7582dc -> xen-tested-master
Re: [RFC PATCH 05/21] xen/arm: vsmmuv3: Add dummy support for virtual SMMUv3 for guests
Hi Rahul,
On 01/12/2022 17:02, Rahul Singh wrote:
>
>
> domain_viommu_init() will be called during domain creation and will add
> the dummy trap handler for virtual IOMMUs for guests.
>
> A host IOMMU list will be created when host IOMMU devices are probed
> and this list will be used to create the IOMMU device tree node for
> dom0. For dom0, 1-1 mapping will be established between vIOMMU in dom0
> and physical IOMMU.
>
> For domUs, the 1-N mapping will be established between domU and physical
> IOMMUs. A new area has been reserved in the arm guest physical map at
> which the emulated vIOMMU node is created in the device tree.
>
> Also set the vIOMMU type to vSMMUv3 to enable vIOMMU framework to call
> vSMMUv3 domain creation/destroy functions.
>
> Signed-off-by: Rahul Singh
> ---
> xen/arch/arm/domain.c | 3 +-
> xen/arch/arm/include/asm/domain.h | 4 +
> xen/arch/arm/include/asm/viommu.h | 20
> xen/drivers/passthrough/Kconfig| 8 ++
> xen/drivers/passthrough/arm/Makefile | 1 +
> xen/drivers/passthrough/arm/smmu-v3.c | 7 ++
> xen/drivers/passthrough/arm/viommu.c | 30 ++
> xen/drivers/passthrough/arm/vsmmu-v3.c | 124 +
> xen/drivers/passthrough/arm/vsmmu-v3.h | 20
> xen/include/public/arch-arm.h | 7 +-
> 10 files changed, 222 insertions(+), 2 deletions(-)
> create mode 100644 xen/drivers/passthrough/arm/vsmmu-v3.c
> create mode 100644 xen/drivers/passthrough/arm/vsmmu-v3.h
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 2a85209736..9a2b613500 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -692,7 +692,8 @@ int arch_sanitise_domain_config(struct
> xen_domctl_createdomain *config)
> return -EINVAL;
> }
>
> -if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
> +if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE &&
> + config->arch.viommu_type != viommu_get_type() )
> {
> dprintk(XENLOG_INFO,
> "vIOMMU type requested not supported by the platform or
> Xen\n");
> diff --git a/xen/arch/arm/include/asm/domain.h
> b/xen/arch/arm/include/asm/domain.h
> index 2ce6764322..8eb4eb5fd6 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -114,6 +114,10 @@ struct arch_domain
> void *tee;
> #endif
>
> +#ifdef CONFIG_VIRTUAL_IOMMU
> +struct list_head viommu_list; /* List of virtual IOMMUs */
> +#endif
> +
> } __cacheline_aligned;
>
> struct arch_vcpu
> diff --git a/xen/arch/arm/include/asm/viommu.h
> b/xen/arch/arm/include/asm/viommu.h
> index 7cd3818a12..4785877e2a 100644
> --- a/xen/arch/arm/include/asm/viommu.h
> +++ b/xen/arch/arm/include/asm/viommu.h
> @@ -5,9 +5,21 @@
> #ifdef CONFIG_VIRTUAL_IOMMU
>
> #include
> +#include
> #include
> #include
>
> +extern struct list_head host_iommu_list;
> +
> +/* data structure for each hardware IOMMU */
> +struct host_iommu {
> +struct list_head entry;
> +const struct dt_device_node *dt_node;
> +paddr_t addr;
> +paddr_t size;
> +uint32_t irq;
You want this to be int and not unsigned. The reason is ...
> +};
> +
> struct viommu_ops {
> /*
> * Called during domain construction if toolstack requests to enable
> @@ -35,6 +47,8 @@ struct viommu_desc {
> int domain_viommu_init(struct domain *d, uint16_t viommu_type);
> int viommu_relinquish_resources(struct domain *d);
> uint16_t viommu_get_type(void);
> +void add_to_host_iommu_list(paddr_t addr, paddr_t size,
> +const struct dt_device_node *node);
>
> #else
>
> @@ -56,6 +70,12 @@ static inline int viommu_relinquish_resources(struct
> domain *d)
> return 0;
> }
>
> +static inline void add_to_host_iommu_list(paddr_t addr, paddr_t size,
> + const struct dt_device_node *node)
> +{
> +return;
> +}
> +
> #endif /* CONFIG_VIRTUAL_IOMMU */
>
> #endif /* __ARCH_ARM_VIOMMU_H__ */
> diff --git a/xen/drivers/passthrough/Kconfig b/xen/drivers/passthrough/Kconfig
> index 19924fa2de..4c725f5f67 100644
> --- a/xen/drivers/passthrough/Kconfig
> +++ b/xen/drivers/passthrough/Kconfig
> @@ -41,6 +41,14 @@ config VIRTUAL_IOMMU
> help
> Support virtual IOMMU infrastructure to implement vIOMMU.
>
> +config VIRTUAL_ARM_SMMU_V3
> + bool "ARM Ltd. Virtual SMMUv3 Support (UNSUPPORTED)" if UNSUPPORTED
> + depends on ARM_SMMU_V3 && VIRTUAL_IOMMU
> + help
> +Support for implementations of the virtual ARM System MMU
> architecture
> +version 3. Virtual SMMUv3 is unsupported feature and should not be
> used
> +in production.
> +
> endif
>
> config IOMMU_FORCE_PT_SHARE
> diff --git a/xen/drivers/passthrough/arm/Makefile
> b/xen/drivers/passthrough/arm/Makefile
> index 4cc54f3f4d..e758a9d6aa 100644
> --- a/xen/drivers/passthrough/arm/Makefile
> +++ b/xen/dri
Re: [RFC PATCH 04/21] xen/arm: vIOMMU: add generic vIOMMU framework
Hi Rahul,
On 01/12/2022 17:02, Rahul Singh wrote:
>
>
> This patch adds basic framework for vIOMMU.
>
> Signed-off-by: Rahul Singh
> ---
> xen/arch/arm/domain.c| 17 +++
> xen/arch/arm/domain_build.c | 3 ++
> xen/arch/arm/include/asm/viommu.h| 70
> xen/drivers/passthrough/Kconfig | 6 +++
> xen/drivers/passthrough/arm/Makefile | 1 +
> xen/drivers/passthrough/arm/viommu.c | 48 +++
> xen/include/public/arch-arm.h| 4 ++
> 7 files changed, 149 insertions(+)
> create mode 100644 xen/arch/arm/include/asm/viommu.h
> create mode 100644 xen/drivers/passthrough/arm/viommu.c
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 38e22f12af..2a85209736 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include "vpci.h"
> @@ -691,6 +692,13 @@ int arch_sanitise_domain_config(struct
> xen_domctl_createdomain *config)
> return -EINVAL;
> }
>
> +if ( config->arch.viommu_type != XEN_DOMCTL_CONFIG_VIOMMU_NONE )
> +{
> +dprintk(XENLOG_INFO,
> +"vIOMMU type requested not supported by the platform or
> Xen\n");
Maybe a simpler message like for TEE would be better: "Unsupported vIOMMU type"
> +return -EINVAL;
> +}
> +
> return 0;
> }
>
> @@ -783,6 +791,9 @@ int arch_domain_create(struct domain *d,
> if ( (rc = domain_vpci_init(d)) != 0 )
> goto fail;
>
> +if ( (rc = domain_viommu_init(d, config->arch.viommu_type)) != 0 )
> +goto fail;
> +
> return 0;
>
> fail:
> @@ -998,6 +1009,7 @@ static int relinquish_memory(struct domain *d, struct
> page_list_head *list)
> enum {
> PROG_pci = 1,
> PROG_tee,
> +PROG_viommu,
> PROG_xen,
> PROG_page,
> PROG_mapping,
> @@ -1048,6 +1060,11 @@ int domain_relinquish_resources(struct domain *d)
> if (ret )
> return ret;
>
> +PROGRESS(viommu):
> +ret = viommu_relinquish_resources(d);
> +if (ret )
> +return ret;
> +
> PROGRESS(xen):
> ret = relinquish_memory(d, &d->xenpage_list);
> if ( ret )
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index bd30d3798c..abbaf37a2e 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -27,6 +27,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -3858,6 +3859,7 @@ void __init create_domUs(void)
> struct domain *d;
> struct xen_domctl_createdomain d_cfg = {
> .arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE,
> +.arch.viommu_type = viommu_get_type(),
> .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
> /*
> * The default of 1023 should be sufficient for guests because
> @@ -4052,6 +4054,7 @@ void __init create_dom0(void)
> printk(XENLOG_WARNING "Maximum number of vGIC IRQs exceeded.\n");
> dom0_cfg.arch.tee_type = tee_get_type();
> dom0_cfg.max_vcpus = dom0_max_vcpus();
> +dom0_cfg.arch.viommu_type = viommu_get_type();
>
> if ( iommu_enabled )
> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
> diff --git a/xen/arch/arm/include/asm/viommu.h
> b/xen/arch/arm/include/asm/viommu.h
> new file mode 100644
> index 00..7cd3818a12
> --- /dev/null
> +++ b/xen/arch/arm/include/asm/viommu.h
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */
> +#ifndef __ARCH_ARM_VIOMMU_H__
> +#define __ARCH_ARM_VIOMMU_H__
> +
> +#ifdef CONFIG_VIRTUAL_IOMMU
> +
> +#include
> +#include
> +#include
> +
> +struct viommu_ops {
> +/*
> + * Called during domain construction if toolstack requests to enable
> + * vIOMMU support.
> + */
> +int (*domain_init)(struct domain *d);
> +
> +/*
> + * Called during domain destruction to free resources used by vIOMMU.
> + */
> +int (*relinquish_resources)(struct domain *d);
> +};
> +
> +struct viommu_desc {
> +/* vIOMMU domains init/free operations described above. */
> +const struct viommu_ops *ops;
> +
> +/*
> + * ID of vIOMMU. Corresponds to xen_arch_domainconfig.viommu_type.
> + * Should be one of XEN_DOMCTL_CONFIG_VIOMMU_xxx
> + */
> +uint16_t viommu_type;
Here the viommu_type is uint16_t ...
> +};
> +
> +int domain_viommu_init(struct domain *d, uint16_t viommu_type);
> +int viommu_relinquish_resources(struct domain *d);
> +uint16_t viommu_get_type(void);
> +
> +#else
> +
> +static inline uint8_t viommu_get_type(void)
Here you are returning uint8_t ...
> +{
> +return XEN_DOMCTL_CONFIG_VIOMMU_NONE;
> +}
> +
> +static inline int domain_viommu_init(struct domain *d, uint16_t viommu_type)
Here you are taking uint16_t. So it looks like that ...
> +{
> +if ( likely(viommu_type == XEN_DOMCTL_CONFIG_VI
