Re: [PATCH] Config.mk: drop -Wdeclaration-after-statement
On 28.11.2023 18:47, Alexander Kanavin wrote: > Such constructs are fully allowed by C99: > https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations There's more to this: It may also be a policy of ours (or of any sub-component) to demand that declarations and statements are properly separated. This would therefore need discussing first. > If the flag is present, then building against python 3.12 will fail thusly: > > | In file included from > /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:44, > | from xen/lowlevel/xc/xc.c:8: > | > /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h: > In function 'Py_SIZE': > | > /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h:233:5: > error: ISO C90 forbids mixed declarations and code > [-Werror=declaration-after-statement] > | 233 | PyVarObject *var_ob = _PyVarObject_CAST(ob); > | | ^~~ > | In file included from > /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:53: > | > /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h: > In function '_PyLong_CompactValue': > | > /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h:121:5: > error: ISO C90 forbids mixed declarations and code > [-Werror=declaration-after-statement] > | 121 | Py_ssize_t sign = 1 - (op->long_value.lv_tag & > _PyLong_SIGN_MASK); > | | ^~ > | cc1: all warnings being treated as errors At least by the specific wording of the diagnostic I'm inclined to call this a compiler bug: There's no point in mentioning C90 when -std=gnu99 was passed. > --- a/Config.mk > +++ b/Config.mk > @@ -177,8 +177,6 @@ CFLAGS += -std=gnu99 Just up from here there is CFLAGS += -std=gnu99 Yet there's no HOSTCFLAGS += -std=gnu99 anywhere. Hence ... > CFLAGS += -Wall -Wstrict-prototypes > > -$(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) ... imo this removal is inappropriate. Jan > -$(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) > $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable) > $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs) >
[xen-unstable-smoke test] 183925: regressions - all pass
flight 183925 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/183925/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 7 xen-build/dist-test fail REGR. vs. 183851 build-arm64-xsm 7 xen-build/dist-test fail REGR. vs. 183851 build-armhf 7 xen-build/dist-test fail REGR. vs. 183851 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 72ec070930fa4a34d7665ba2a7a42a91d0c85bed baseline version: xen 80c153c48b255bae61948827241c26671207cf4e Last test of basis 183851 2023-11-24 09:03:53 Z4 days Failing since183871 2023-11-27 14:00:26 Z1 days9 attempts Testing same since 183925 2023-11-29 04:00:29 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Federico Serafini Frediano Ziglio Jan Beulich Julien Grall Luca Fancellu Maria Celeste Cesario Maria Celeste Cesario Nicola Vetrini Roger Pau Monné Simone Ballarin Simone Ballarin Stefano Stabellini Tamas K Lengyel 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 Not pushing. (No revision log; it would be 370 lines long.)
[ovmf test] 183923: all pass - PUSHED
flight 183923 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/183923/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 88580a79d49f923ed9347d6161a5cff28a46f627 baseline version: ovmf cdf36b1e36805884a8dc115b1316ac09ce77feee Last test of basis 183914 2023-11-28 21:42:52 Z0 days Testing same since 183923 2023-11-29 01:56:04 Z0 days1 attempts People who touched revisions under test: Gao Cheng 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 cdf36b1e36..88580a79d4 88580a79d49f923ed9347d6161a5cff28a46f627 -> xen-tested-master
[linux-5.4 test] 183907: regressions - FAIL
flight 183907 linux-5.4 real [real] flight 183924 linux-5.4 real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/183907/ http://logs.test-lab.xenproject.org/osstest/logs/183924/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-arm64-arm64-libvirt-raw 18 guest-start.2fail REGR. vs. 183803 Tests which are failing intermittently (not blocking): test-armhf-armhf-libvirt-qcow2 17 guest-start/debian.repeat fail pass in 183924-retest Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds18 guest-start/debian.repeat fail REGR. vs. 183803 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-raw 17 guest-start/debian.repeat fail in 183924 like 183797 test-armhf-armhf-xl-credit1 14 guest-start fail like 183797 test-armhf-armhf-xl-multivcpu 18 guest-start/debian.repeatfail like 183797 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 183803 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183803 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 183803 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 183803 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183803 test-armhf-armhf-xl-credit2 18 guest-start/debian.repeatfail like 183803 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183803 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183803 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 183803 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 183803 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 183803 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 183803 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183803 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-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 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass
xen | Successful pipeline for staging | 72ec0709
Pipeline #1088683622 has passed! Project: xen ( https://gitlab.com/xen-project/xen ) Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging ) Commit: 72ec0709 ( https://gitlab.com/xen-project/xen/-/commit/72ec070930fa4a34d7665ba2a7a42a91d0c85bed ) Commit Message: xen: replace some occurrences of SAF-1-safe wit... Commit Author: Nicola Vetrini Committed by: Stefano Stabellini Pipeline #1088683622 ( https://gitlab.com/xen-project/xen/-/pipelines/1088683622 ) triggered by Ganis ( https://gitlab.com/ganis ) successfully completed 129 jobs in 3 stages. -- You're receiving this email because of your account on gitlab.com.
[xen-unstable-smoke test] 183920: regressions - all pass
flight 183920 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/183920/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 7 xen-build/dist-test fail REGR. vs. 183851 build-arm64-xsm 7 xen-build/dist-test fail REGR. vs. 183851 build-armhf 7 xen-build/dist-test fail REGR. vs. 183851 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 18540a313cc66a04eb15a67d74c7992a8576fbec baseline version: xen 80c153c48b255bae61948827241c26671207cf4e Last test of basis 183851 2023-11-24 09:03:53 Z4 days Failing since183871 2023-11-27 14:00:26 Z1 days8 attempts Testing same since 183920 2023-11-28 23:02:06 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Federico Serafini Frediano Ziglio Jan Beulich Julien Grall Luca Fancellu Maria Celeste Cesario Maria Celeste Cesario Roger Pau Monné Simone Ballarin Simone Ballarin Tamas K Lengyel 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 Not pushing. (No revision log; it would be 330 lines long.)
Re: MISRA: Compatible declarations for sort and bsearch
On Mon, 27 Nov 2023, Nicola Vetrini wrote: > > > /* > > > * TODO: make first_valid_mfn static when NUMA is supported on Arm, this > > > * is required because the dummy helpers are using it. > > > */ > > > extern mfn_t first_valid_mfn; > > > > > > it should probably be deviated. > > > > NUMA work is still in progress for Arm, I think, so I'd rather wait with > > deviating. > > > > +Stefano > > I can leave it as is, if that's indeed going to become static at some point. I see the point in waiting given the TODO comment, but I wouldn't want this issue to be the only thing standing between us and zero violation of Rule 8.4 on ARM. So I think we should add SAF to the comment and remove it when not necessary any longer.
Re: [XEN PATCH v5 1/2] automation/eclair: make the docs for MISRA C:2012 Dir 4.1 visible to ECLAIR
On Tue, 28 Nov 2023, Stefano Stabellini wrote: > On Fri, 17 Nov 2023, Nicola Vetrini wrote: > > To be able to check for the existence of the necessary subsections in > > the documentation for MISRA C:2012 Dir 4.1, ECLAIR needs to have a source > > file that is built. > > > > This file is generated from 'C-runtime-failures.rst' in docs/misra > > and the configuration is updated accordingly. > > > > Signed-off-by: Nicola Vetrini > > It looks like you also addressed all Julien's comments appropriately I meant to add my reviewed-by Reviewed-by: Stefano Stabellini
Re: [XEN PATCH v5 1/3] arm/bitops: encapsulate violation of MISRA C:2012 Rule 10.1
On Fri, 24 Nov 2023, Nicola Vetrini wrote:
> On 2023-11-24 09:06, Jan Beulich wrote:
> > On 23.11.2023 08:37, Nicola Vetrini wrote:
> > > The definitions of ffs{l}? violate Rule 10.1, by using the well-known
> > > pattern (x & -x); its usage is wrapped by the ISOLATE_LSB macro.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Nicola Vetrini
> > > Reviewed-by: Stefano Stabellini
> >
> > This patch failed my pre-push build test; apparently xen/macros.h needs
> > including explicitly.
> >
> > Jan
>
> Yes, indeed. I must have dropped that change when experimenting with single
> evaluation and then never reinstated it.
Please re-send
Re: [XEN PATCH v5 1/2] automation/eclair: make the docs for MISRA C:2012 Dir 4.1 visible to ECLAIR
On Fri, 17 Nov 2023, Nicola Vetrini wrote: > To be able to check for the existence of the necessary subsections in > the documentation for MISRA C:2012 Dir 4.1, ECLAIR needs to have a source > file that is built. > > This file is generated from 'C-runtime-failures.rst' in docs/misra > and the configuration is updated accordingly. > > Signed-off-by: Nicola Vetrini It looks like you also addressed all Julien's comments appropriately
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
On Tue, 28 Nov 2023, Julien Grall wrote:
> On 28/11/2023 18:15, Michal Orzel wrote:
> > On 28/11/2023 18:09, Julien Grall wrote:
> > > On 28/11/2023 18:00, Michal Orzel wrote:
> > > > On 28/11/2023 17:14, Julien Grall wrote:
> > > > > On 27/11/2023 15:41, Michal Orzel wrote:
> > > > > > Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where
> > > > > > prompt
> > > > > > attention to undefined behavior issues, notably during CI test runs,
> > > > > > is
> > > > > > essential. When enabled, this option causes Xen to panic upon
> > > > > > detecting
> > > > > > UBSAN failure (as the last step in ubsan_epilogue()).
> > > > >
> > > > > I have mixed opinions on this patch. This would be a good one if we
> > > > > had
> > > > > a Xen mostly free from UBSAN behavior. But this is not the case at
> > > > > least
> > > > > on arm32. So we would end up to stop at the first error. IOW, we would
> > > > > need to fix the first error before we can see the next one.
> > > > Well, this patch introduces a config option disabled by default.
> > >
> > > I understood this is disabled by default... I am pointing out that I am
> > > not convinced about the usefulness until we are at the stage where Xen
> > > is normally not reporting any USBAN error.
> > >
> > > > If we end up enabling it for CI for reasons* stated by Andrew, then the
> > > > natural way
> > > > of handling such issues is to do the investigation locally.
> > >
> > > This will not always be possible. One example is when you are only able
> > > to reproduce some of the USBAN errors on a specific HW.
> > >
> > > > Then, you are not forced
> > > > to select this option and you can see all the UBSAN issues if you want.
> > >
> > > See above, I got that point. I am mostly concerned about the implication
> > > in the CI right now.
> > >
> > > >
> > > > >
> > > > > So I feel it would be best if the gitlab CI jobs actually check for
> > > > > USBAN in the logs and fail if there are any. With that, we can get the
> > > > > full list of UBSAN issues on each job.
> > > > Well, I prefer Andrew suggestion (both [1] and on IRC), hence this
> > > > patch.
> > > >
> > > > *my plan was to first fix the UBSAN issues and then enable this option
> > > > for CI.
> > >
> > > That would have been useful to describe your goal after "---". With that
> > > in mind, then I suggest to revisit this patch once all the UBSAN issues
> > > in a normal build are fixed.
> > But this patch does not enable this option for CI automatically, right?
>
> Correct.
>
> > Why are you so keen to push it back?
>
> I have been pushing back because your commit message refers to the CI
> specifically and today this would not really work (read as I would not be
> happy if this option is enabled in the CI right now at least on arm32).
>
> If you want to fail a CI job for UBSAN today, then we need to find a different
> approach so we can get the full list of UBSAN error rather than fixing one,
> retry and then next one.
>
> > Is it because you see no point in this option other than for the upstream CI
> > loop?
>
> Even in the upstream CI loop, I am a little unsure about this approach. At
> least, I feel I would want to see all the reports at once in the CI.
>
> But this is not really a strong feeling.
>
> > I find it useful on a day-to-day
> > Xen runs, and I would for sure enable it by default in my config not to miss
> > UBSAN failures.
>
> Fair enough. I view USBAN issues the same a WARN_ON. They all need to be
> investigated. So now you have an inconsistent policy.
>
> Are you are also intending to switch WARN_ON() to fatal? If not, then why
> would UBSAN warnings more important that the other warnings?
I think it would be useful to be able to turn WARN_ONs into BUG_ONs
selectively by component. We could turn all WARN_ONs into BUG_ONs but
that's a bit extreme.
It is true that this patch is a bit... "ad-hoc". But given its
simplicity it doesn't hurt and I think it is nice-to-have for UBSAN. So
I would go with that.
But if we want something more sophisticated, here is an idea. The
concept is that one could specify warn=arch/arm/domain_build.c in the
Xen command line to change a WARN_ON in arch/arm/domain_build.c into a
BUG. I tested it with a WARN_ON I added on purpose and it works.
diff --git a/xen/common/symbols.c b/xen/common/symbols.c
index 133b580768..c78d2963c3 100644
--- a/xen/common/symbols.c
+++ b/xen/common/symbols.c
@@ -20,6 +20,10 @@
#include
#include
#include
+#include
+
+char opt_warn[30] = "";
+string_param("warn", opt_warn);
#ifdef SYMBOLS_ORIGIN
extern const unsigned int symbols_offsets[];
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 1793be5b6b..60e14cdb85 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -12,12 +12,19 @@
#include
#include
+extern char opt_warn[30];
+
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
#define WARN_ON(p) ({ \
bool ret_warn_on_ = (p);
[ovmf test] 183914: all pass - PUSHED
flight 183914 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/183914/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf cdf36b1e36805884a8dc115b1316ac09ce77feee baseline version: ovmf e1627f77201ac55d24e9d0e3380f9dbdc600843c Last test of basis 183909 2023-11-28 18:11:03 Z0 days Testing same since 183914 2023-11-28 21:42:52 Z0 days1 attempts People who touched revisions under test: Michael Kubacki Nhi Pham 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 e1627f7720..cdf36b1e36 cdf36b1e36805884a8dc115b1316ac09ce77feee -> xen-tested-master
[seabios test] 183912: tolerable FAIL - PUSHED
flight 183912 seabios real [real] http://logs.test-lab.xenproject.org/osstest/logs/183912/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 182944 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 182944 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 182944 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 182944 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 182944 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 version targeted for testing: seabios a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 baseline version: seabios 1e1da7a963007d03a4e0e9a9e0ff17990bb1608d Last test of basis 182944 2023-09-11 15:14:00 Z 78 days Testing same since 183912 2023-11-28 20:11:08 Z0 days1 attempts People who touched revisions under test: Gerd Hoffmann 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-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm pass test-amd64-i386-xl-qemuu-debianhvm-i386-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-qemuu-freebsd11-amd64 pass test-amd64-amd64-qemuu-freebsd12-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-qemuu-ws16-amd64 fail test-amd64-i386-xl-qemuu-ws16-amd64 fail test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrictpass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict pass test-amd64-amd64-qemuu-nested-intel pass test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow pass test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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/seabios.git 1e1da7a..a6ed6b7 a6ed6b701f0a57db0569ab98b0661c12a6ec3ff8 -> xen-tested-master
Re: [PATCH v10 13/17] vpci: add initial support for virtual PCI bus topology
Hi Roger,
Roger Pau Monné writes:
> On Wed, Nov 22, 2023 at 01:18:32PM -0800, Stefano Stabellini wrote:
>> On Wed, 22 Nov 2023, Roger Pau Monné wrote:
>> > On Tue, Nov 21, 2023 at 05:12:15PM -0800, Stefano Stabellini wrote:
>> > > Let me expand on this. Like I wrote above, I think it is important that
>> > > Xen vPCI is the only in-use PCI Root Complex emulator. If it makes the
>> > > QEMU implementation easier, it is OK if QEMU emulates an unneeded and
>> > > unused PCI Root Complex. From Xen point of view, it doesn't exist.
>> > >
>> > > In terms if ioreq registration, QEMU calls
>> > > xendevicemodel_map_pcidev_to_ioreq_server for each PCI BDF it wants to
>> > > emulate. That way, Xen vPCI knows exactly what PCI config space
>> > > reads/writes to forward to QEMU.
>> > >
>> > > Let's say that:
>> > > - 00:02.0 is PCI passthrough device
>> > > - 00:03.0 is a PCI emulated device
>> > >
>> > > QEMU would register 00:03.0 and vPCI would know to forward anything
>> > > related to 00:03.0 to QEMU, but not 00:02.0.
>> >
>> > I think there's some work here so that we have a proper hierarchy
>> > inside of Xen. Right now both ioreq and vpci expect to decode the
>> > accesses to the PCI config space, and setup (MM)IO handlers to trap
>> > ECAM, see vpci_ecam_{read,write}().
>> >
>> > I think we want to move to a model where vPCI doesn't setup MMIO traps
>> > itself, and instead relies on ioreq to do the decoding and forwarding
>> > of accesses. We need some work in order to represent an internal
>> > ioreq handler, but that shouldn't be too complicated. IOW: vpci
>> > should register devices it's handling with ioreq, much like QEMU does.
>>
>> I think this could be a good idea.
>>
>> This would be the very first IOREQ handler implemented in Xen itself,
>> rather than outside of Xen. Some code refactoring might be required,
>> which worries me given that vPCI is at v10 and has been pending for
>> years. I think it could make sense as a follow-up series, not v11.
>
> That's perfectly fine for me, most of the series here just deal with
> the logic to intercept guest access to the config space and is
> completely agnostic as to how the accesses are intercepted.
>
>> I think this idea would be beneficial if, in the example above, vPCI
>> doesn't really need to know about device 00:03.0. vPCI registers via
>> IOREQ the PCI Root Complex and device 00:02.0 only, QEMU registers
>> 00:03.0, and everything works. vPCI is not involved at all in PCI config
>> space reads and writes for 00:03.0. If this is the case, then moving
>> vPCI to IOREQ could be good.
>
> Given your description above, with the root complex implemented in
> vPCI, we would need to mandate vPCI together with ioreqs even if no
> passthrough devices are using vPCI itself (just for the emulation of
> the root complex). Which is fine, just wanted to mention the
> dependency.
>
>> On the other hand if vPCI actually needs to know that 00:03.0 exists,
>> perhaps because it changes something in the PCI Root Complex emulation
>> or vPCI needs to take some action when PCI config space registers of
>> 00:03.0 are written to, then I think this model doesn't work well. If
>> this is the case, then I think it would be best to keep vPCI as MMIO
>> handler and let vPCI forward to IOREQ when appropriate.
>
> At first approximation I don't think we would have such interactions,
> otherwise the whole premise of ioreq being able to register individual
> PCI devices would be broken.
>
> XenSever already has scenarios with two different user-space emulators
> (ie: two different ioreq servers) handling accesses to different
> devices in the same PCI bus, and there's no interaction with the root
> complex required.
>
Out of curiosity: how legacy PCI interrupts are handled in this case? In
my understanding, it is Root Complex's responsibility to propagate
correct IRQ levels to an interrupt controller?
[...]
--
WBR, Volodymyr
Re: We are not able to virtualize FreeBSD using xen 4.17 on Arm 32 bit
On Tue, Nov 28, 2023 at 04:10:50PM +0100, Roger Pau Monné wrote: > On Tue, Nov 28, 2023 at 03:09:14PM +0100, Mario Marietto wrote: > > For booting a FreeBSD kernel as a guest OS on XEN,should we install xen > > 4.18 from source ? > I don't think so, I'm not aware of the FreeBSD port requiring a > specific version of Xen. I do think the work is limited to aarch64 > however, so there's no support in sight for arm32 FreeBSD guests as > far as I'm aware. I've only ever tried arm64, but since arm32 didn't appear to need much to have operational I tried to make it possible. In theory it /should/ work on arm32, but I've never tried it. What was missing was I had never added it to the configuration and one link was needed. Updated "submit" branch has the tiny adjustment. (the only difference is the hypercall wrappers, register naming and where the op code goes, very simple compatibility) On Tue, Nov 28, 2023 at 02:45:40PM +0100, Roger Pau Monné wrote: > On Mon, Nov 27, 2023 at 03:04:30PM -0800, Elliott Mitchell wrote: > > BTW Roger Pau Monné, now that Xen 4.18 is out, take a look at the > > "royger" branch? > > I've pushed a bunch of those, there are still some, I've made comments > on the branch. > > I think there isn't much left after the swept I've done. > > If you can rebase and reply to the comments I will take a look at > what's remaining. Done. I'm unsure you'll like the xs_attach_children() approach. Thing is that really is appropriate given the situation. #2 is the urgent one as that is the handy approach to the hypercall declarations. -- (\___(\___(\__ --=> 8-) EHM <=-- __/)___/)___/) \BS (| ehem+sig...@m5p.com PGP 87145445 |) / \_CS\ | _ -O #include O- _ | / _/ 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Re: [PATCH] cirrus-ci: update FreeBSD versions
On Tue, 28 Nov 2023, Andrew Cooper wrote: > On 28/11/2023 5:11 pm, Roger Pau Monne wrote: > > FreeBSD 14.0 has already been released, so switch to the release version > > image, > > and introduce a FreeBSD 15.0 version to track current FreeBSD unstable > > (development) branch. > > > > Sample output at: > > > > https://github.com/royger/xen/runs/19105278189 > > > > Signed-off-by: Roger Pau Monné > > Acked-by: Andrew Cooper Acked-by: Stefano Stabellini
Re: [PATCH v10 03/17] vpci: use per-domain PCI lock to protect vpci structure
Hi Roger
Thank you for the review.
Roger Pau Monné writes:
> On Thu, Oct 12, 2023 at 10:09:15PM +, Volodymyr Babchuk wrote:
>> From: Oleksandr Andrushchenko
>>
>> Use a previously introduced per-domain read/write lock to check
>> whether vpci is present, so we are sure there are no accesses to the
>> contents of the vpci struct if not. This lock can be used (and in a
>> few cases is used right away) so that vpci removal can be performed
>> while holding the lock in write mode. Previously such removal could
>> race with vpci_read for example.
>>
>> When taking both d->pci_lock and pdev->vpci->lock, they should be
>> taken in this exact order: d->pci_lock then pdev->vpci->lock to avoid
>> possible deadlock situations.
>>
>> 1. Per-domain's pci_rwlock is used to protect pdev->vpci structure
>> from being removed.
>>
>> 2. Writing the command register and ROM BAR register may trigger
>> modify_bars to run, which in turn may access multiple pdevs while
>> checking for the existing BAR's overlap. The overlapping check, if
>> done under the read lock, requires vpci->lock to be acquired on both
>> devices being compared, which may produce a deadlock. It is not
>> possible to upgrade read lock to write lock in such a case. So, in
>> order to prevent the deadlock, use d->pci_lock instead. To prevent
>> deadlock while locking both hwdom->pci_lock and dom_xen->pci_lock,
>> always lock hwdom first.
>>
>> All other code, which doesn't lead to pdev->vpci destruction and does
>> not access multiple pdevs at the same time, can still use a
>> combination of the read lock and pdev->vpci->lock.
>>
>> 3. Drop const qualifier where the new rwlock is used and this is
>> appropriate.
>>
>> 4. Do not call process_pending_softirqs with any locks held. For that
>> unlock prior the call and re-acquire the locks after. After
>> re-acquiring the lock there is no need to check if pdev->vpci exists:
>> - in apply_map because of the context it is called (no race condition
>>possible)
>> - for MSI/MSI-X debug code because it is called at the end of
>>pdev->vpci access and no further access to pdev->vpci is made
>>
>> 5. Use d->pci_lock around for_each_pdev and pci_get_pdev_by_domain
>> while accessing pdevs in vpci code.
>>
>> 6. We are removing multiple ASSERT(pcidevs_locked()) instances because
>> they are too strict now: they should be corrected to
>> ASSERT(pcidevs_locked() || rw_is_locked(>pci_lock)), but problem is
>> that mentioned instances does not have access to the domain
>> pointer and it is not feasible to pass a domain pointer to a function
>> just for debugging purposes.
>>
>> There is a possible lock inversion in MSI code, as some parts of it
>> acquire pcidevs_lock() while already holding d->pci_lock.
>
> Is this going to be addressed in a further patch?
>
It is actually addressed in this patch, in the v10. I just forgot to
remove this sentence from the patch description. My bad.
This was fixed by additional parameter to allocate_and_map_msi_pirq(),
as it is being called from two places: from vpci_msi_enable(), while we
already are holding d->pci_lock, or from physdev_map_pirq(), when there
are no locks are taken.
[...]
>> @@ -2908,7 +2908,8 @@ int allocate_and_map_msi_pirq(struct domain *d, int
>> index, int *pirq_p,
>>
>> msi->irq = irq;
>>
>> -pcidevs_lock();
>> +if ( use_pci_lock )
>> +pcidevs_lock();
>
> Instead of passing the flag it might be better if the caller can take
> the lock, as to avoid having to pass an extra parameter.
>
> Then we should also assert that either the pcidev_lock or the
> per-domain pci lock is taken?
>
This is a good idea. I'll add lock in physdev_map_pirq(). This is only
one place where we call physdev_map_pirq() without holding any lock.
>> /* Verify or get pirq. */
>> write_lock(>event_lock);
>> pirq = allocate_pirq(d, index, *pirq_p, irq, type, >entry_nr);
>> @@ -2924,7 +2925,8 @@ int allocate_and_map_msi_pirq(struct domain *d, int
>> index, int *pirq_p,
>>
>> done:
>> write_unlock(>event_lock);
>> -pcidevs_unlock();
>> +if ( use_pci_lock )
>> +pcidevs_unlock();
>> if ( ret )
>> {
>> switch ( type )
>> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
>> index 20275260b3..466725d8ca 100644
>> --- a/xen/arch/x86/msi.c
>> +++ b/xen/arch/x86/msi.c
>> @@ -602,7 +602,7 @@ static int msi_capability_init(struct pci_dev *dev,
>> unsigned int i, mpos;
>> uint16_t control;
>>
>> -ASSERT(pcidevs_locked());
>> +ASSERT(pcidevs_locked() || rw_is_locked(>domain->pci_lock));
>> pos = pci_find_cap_offset(dev->sbdf, PCI_CAP_ID_MSI);
>> if ( !pos )
>> return -ENODEV;
>> @@ -771,7 +771,7 @@ static int msix_capability_init(struct pci_dev *dev,
>> if ( !pos )
>> return -ENODEV;
>>
>> -ASSERT(pcidevs_locked());
>> +ASSERT(pcidevs_locked() || rw_is_locked(>domain->pci_lock));
>>
>> control = pci_conf_read16(dev->sbdf,
Re: [PATCH v4 10/14] xen/asm-generic: introduce stub header monitor.h
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> The header is shared between several archs so it is
> moved to asm-generic.
>
> Signed-off-by: Oleksii Kurochko
> Acked-by: Jan Beulich .
> ---
> Changes in V4:
> - Removed the double blank line.
> - Added Acked-by: Jan Beulich .
> - Update the commit message
> ---
> Changes in V3:
> - Use forward-declaration of struct domain instead of " #include
> ".
> - Add ' include '
> - Drop PPC's monitor.h.
> ---
> Changes in V2:
> - remove inclusion of "+#include "
> - add "struct xen_domctl_monitor_op;"
> - remove one of SPDX tags.
> ---
> xen/arch/ppc/include/asm/Makefile | 1 +
> xen/arch/ppc/include/asm/monitor.h | 43 -
> xen/include/asm-generic/monitor.h | 62 ++
> 3 files changed, 63 insertions(+), 43 deletions(-)
> delete mode 100644 xen/arch/ppc/include/asm/monitor.h
> create mode 100644 xen/include/asm-generic/monitor.h
>
> diff --git a/xen/arch/ppc/include/asm/Makefile
> b/xen/arch/ppc/include/asm/Makefile
> index 319e90955b..bcddcc181a 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -5,6 +5,7 @@ generic-y += div64.h
> generic-y += hardirq.h
> generic-y += hypercall.h
> generic-y += iocap.h
> +generic-y += monitor.h
> generic-y += paging.h
> generic-y += percpu.h
> generic-y += random.h
> diff --git a/xen/arch/ppc/include/asm/monitor.h
> b/xen/arch/ppc/include/asm/monitor.h
> deleted file mode 100644
> index e5b0282bf1..00
> --- a/xen/arch/ppc/include/asm/monitor.h
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/* Derived from xen/arch/arm/include/asm/monitor.h */
> -#ifndef __ASM_PPC_MONITOR_H__
> -#define __ASM_PPC_MONITOR_H__
> -
> -#include
> -#include
> -
> -static inline
> -void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
> -{
> -}
> -
> -static inline
> -int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op
> *mop)
> -{
> -/* No arch-specific monitor ops on PPC. */
> -return -EOPNOTSUPP;
> -}
> -
> -int arch_monitor_domctl_event(struct domain *d,
> - struct xen_domctl_monitor_op *mop);
> -
> -static inline
> -int arch_monitor_init_domain(struct domain *d)
> -{
> -/* No arch-specific domain initialization on PPC. */
> -return 0;
> -}
> -
> -static inline
> -void arch_monitor_cleanup_domain(struct domain *d)
> -{
> -/* No arch-specific domain cleanup on PPC. */
> -}
> -
> -static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> -{
> -BUG_ON("unimplemented");
I'm not sure how I feel about this assertion being dropped in the
generic header. In general my philosophy when creating these stub
headers was to insert as many of these assertions as possible so we
don't end up in a scenario where during early bringup I miss a function
that was originally stubbed but ought to be implemented eventually.
Looking at ARM's monitor.h too, it seems that this function is the only
one that differs from the generic/stub version. I'm wondering if it
would make sense to leave this function out of the generic header, to be
defined by the arch similar to what you've done with some other
functions in this series. That would also allow ARM to be brought in to
using the generic variant.
> -return 0;
> -}
> -
> -#endif /* __ASM_PPC_MONITOR_H__ */
> diff --git a/xen/include/asm-generic/monitor.h
> b/xen/include/asm-generic/monitor.h
> new file mode 100644
> index 00..6be8614431
> --- /dev/null
> +++ b/xen/include/asm-generic/monitor.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * include/asm-GENERIC/monitor.h
> + *
> + * Arch-specific monitor_op domctl handler.
> + *
> + * Copyright (c) 2015 Tamas K Lengyel (ta...@tklengyel.com)
> + * Copyright (c) 2016, Bitdefender S.R.L.
> + *
> + */
> +
> +#ifndef __ASM_GENERIC_MONITOR_H__
> +#define __ASM_GENERIC_MONITOR_H__
> +
> +#include
> +
> +struct domain;
> +struct xen_domctl_monitor_op;
> +
> +static inline
> +void arch_monitor_allow_userspace(struct domain *d, bool allow_userspace)
> +{
> +}
> +
> +static inline
> +int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op
> *mop)
> +{
> +/* No arch-specific monitor ops on GENERIC. */
> +return -EOPNOTSUPP;
> +}
> +
> +int arch_monitor_domctl_event(struct domain *d,
> + struct xen_domctl_monitor_op *mop);
> +
> +static inline
> +int arch_monitor_init_domain(struct domain *d)
> +{
> +/* No arch-specific domain initialization on GENERIC. */
> +return 0;
> +}
> +
> +static inline
> +void arch_monitor_cleanup_domain(struct domain *d)
> +{
> +/* No arch-specific domain cleanup on GENERIC. */
> +}
> +
> +static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
> +{
See previous comment.
> +return 0;
> +}
> +
> +#endif /* __ASM_GENERIC_MONITOR_H__ */
> +
Re: [PATCH v4 09/14] xen/asm-generic: introduce generic header altp2m.h
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > is common between several archs so it is > moved to asm-generic. > > Arm and PPC were switched to asm-generic version of altp2m.h. > Acked-by: Shawn Anastasio Thanks, Shawn
Re: [PATCH v4 08/14] xen/asm-generic: introduce generic div64.h header
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > All archs have the do_div implementation for BITS_PER_LONG == 64 > so do_div64.h is moved to asm-generic. > > x86 and PPC were switched to asm-generic version of div64.h. > Acked-by: Shawn Anastasio Thanks, Shawn
[xen-unstable-smoke test] 183908: regressions - FAIL
flight 183908 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183908/
Regressions :-(
Tests which did not succeed and are blocking,
including tests which could not be run:
build-amd64 7 xen-build/dist-test fail REGR. vs. 183851
build-arm64-xsm 6 xen-buildfail REGR. vs. 183851
build-armhf 7 xen-build/dist-test fail REGR. vs. 183851
Tests which did not succeed, but are not blocking:
test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a
test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass
test-armhf-armhf-xl 15 migrate-support-checkfail never pass
test-armhf-armhf-xl 16 saverestore-support-checkfail never pass
version targeted for testing:
xen 72d51813d631fe27d37736b7a55eeec08f246983
baseline version:
xen 80c153c48b255bae61948827241c26671207cf4e
Last test of basis 183851 2023-11-24 09:03:53 Z4 days
Failing since183871 2023-11-27 14:00:26 Z1 days7 attempts
Testing same since 183874 2023-11-27 19:00:27 Z1 days6 attempts
People who touched revisions under test:
Andrew Cooper
Federico Serafini
Frediano Ziglio
Jan Beulich
Maria Celeste Cesario
Maria Celeste Cesario
Roger Pau Monné
Simone Ballarin
Simone Ballarin
Tamas K Lengyel
jobs:
build-arm64-xsm fail
build-amd64 pass
build-armhf pass
build-amd64-libvirt pass
test-armhf-armhf-xl pass
test-arm64-arm64-xl-xsm blocked
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
Not pushing.
commit 72d51813d631fe27d37736b7a55eeec08f246983
Author: Jan Beulich
Date: Mon Nov 27 15:18:48 2023 +0100
x86: amend cpu_has_xen_{ibt,shstk}
... to evaluate to false at compile-time when the respective Kconfig
control is off, thus allowing the compiler to eliminate then-dead code.
Signed-off-by: Jan Beulich
Acked-by: Andrew Cooper
commit 17754972fa98bff2dfdec09b8094f54530bafcb8
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:56 2023 +0100
x86/atomic: address violations of MISRA C:2012 Rule 11.8
Edit casts that unnecessarily remove const qualifiers
to comply with Rule 11.8.
The type of the provided pointer may be const qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit fc63c0ebefe7e9d166b07971273c1de62428eb18
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:32 2023 +0100
AMD/IOMMU: address violations of MISRA C:2012 Rule 11.8
Drop an unnecessary cast discarding a const qualifier, to comply with
Rule 11.8. The type of the formal parameter ivhd_block is const
qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit fe26cb2dd20fa864deb05e4b278bc9993ba120e6
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:07 2023 +0100
x86/boot/reloc: address violations of MISRA C:2012 Rule 11.8
Add missing const qualifier in casting to comply with Rule 11.8.
Argument tag is typically const qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit 09c2fe438da1dfc83f70d921b52240bea576615f
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:16:43 2023 +0100
x86/platform_hypercall: address violations of MISRA C:2012 Rule 11.8
Add const qualifier in cast that unnecessarily removes it
to comply with Rule 11.8.
The variable info is declared with a const qualified type.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit
Re: [PATCH v4 07/14] xen/asm-generic: introduce generalized hardirq.h
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > is common through archs thereby it is moved > to asm-generic. > > Arm and PPC were switched to asm generic verstion of hardirq.h. > Acked-by: Shawn Anastasio Thanks, Shawn
[linux-linus test] 183884: tolerable FAIL - PUSHED
flight 183884 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/183884/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 183859 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 183859 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 183859 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 183859 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 183859 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 183859 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 183859 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 183859 test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 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-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: linuxdf60cee26a2e3d937a319229e335cb3f9c1f16d2 baseline version: linuxb46ae77f67874918c540feb1e37a63308b2c9290 Last test of basis 183859 2023-11-25 22:13:34 Z2 days Failing since183862 2023-11-26 04:55:31 Z2 days7 attempts Testing same since 183884 2023-11-28 08:21:29 Z0 days1 attempts People who touched revisions under test: Alexander Stein Andrea della Porta Andrew Halaney Arnd Bergmann Asuna Yang Badhri Jagan Sridharan Borislav Petkov (AMD) Christophe JAILLET Chunfeng Yun Conor Dooley Dan Carpenter Dapeng Mi Francesco Dolcini Geert Uytterhoeven Gil Fine Greg Kroah-Hartman Hans de Goede Hans Verkuil Heikki Krogerus Heikki Krogeus Helge Deller Ingo Molnar Ivan Ivanov Johan Hovold Johan Hovold Kent Overstreet Krzysztof Kozlowski Laurent Pinchart Lech Perczak Linus Torvalds Martin Tůma Masami Hiramatsu (Google) Mathias Nyman Mathieu Desnoyers
Re: [PATCH v4 06/14] xen/asm-generic: introduce generic header percpu.h
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > The patch introduces generic percpu.h which was based on Arm's version > with the following changes: > * makes __per_cpu_data_end[] constant > * introduce get_per_cpu_offset() for macros this_cpu() and this_cpu_ptr() > * add inclustion of as get_per_cpu_offset() is located there. > > Also it was changed a place where is included in > because asm-generic version of percpu.h started to include > which > requires definition of DECLARE_PER_CPU. > > As well the patch switches Arm, PPC and x86 architectures to use asm-generic > version of percpu.h. > > Signed-off-by: Oleksii Kurochko > Acked-by: Julien Grall > --- > Changes in V4: > - Added FIXME comment in ppc/current.h for get_per_cpu_offset() Sorry I missed the discussion on this in v3, but yes, the PPC implementation of this will need to be fixed (this was also the case before your patch in PPC's current percpu.h but it was conspicuously missing a TODO comment). In any case, Acked-by: Shawn Anastasio Thanks, Shawn
Re: [PATCH v4 05/14] xen/asm-generic: introduce stub header
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > is common for Arm, PPC and RISC-V thereby it > is moved to asm-generic. > Acked-by: Shawn Anastasio Thanks, Shawn
Re: [PATCH v4 04/14] xen/asm-generic: introduce generic header iocap.h
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > iocap.h is common for Arm, PPC and RISC-V architectures thereby > it was moved to asm-generic. > > Also Arm and PPC were switched to asm-generic version of iocap.h. > Acked-by: Shawn Anastasio Thanks, Shawn
Re: [PATCH v4 03/14] xen/asm-generic: introduce generic hypercall.h
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > Introduce an empty generic hypercall.h for archs which don't > implement it. > > Drop PPC's hypercall.h and switch to generic one instead. Acked-by: Shawn Anastasio Thanks, Shawn
Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
Hi Oleksii, On 11/27/23 1:46 PM, Oleksii wrote: > On Mon, 2023-11-27 at 15:31 +0100, Jan Beulich wrote: >> On 27.11.2023 15:13, Oleksii Kurochko wrote: >>> --- a/xen/arch/ppc/include/asm/irq.h >>> +++ b/xen/arch/ppc/include/asm/irq.h >>> @@ -3,7 +3,9 @@ >>> #define __ASM_PPC_IRQ_H__ >>> >>> #include >>> +#ifdef CONFIG_HAS_DEVICE_TREE >>> #include >>> +#endif >>> #include >> >> Why would this #ifdef not cover the public header as well? (Otherwise >> I'd >> be inclined to ask that the conditional be moved inside that header.) > In that header is defined only consts without additional header > inclusion. At that moment it looked to me pretty save to ifdef only > xen/device_tree.h but you are right we can move incluion of the public > header inside #ifdef OR just remove as xen/device_tree.h already > includes it. > Removing the include of from ppc's asm/irq.h breaks the build, because of platform_get_irq's usage of the `struct dt_device_node` type: CC common/cpu.o In file included from ./include/xen/irq.h:80, from ./include/xen/pci.h:13, from ./include/xen/iommu.h:25, from ./include/xen/sched.h:12, from ./include/xen/event.h:12, from common/cpu.c:3: ./arch/ppc/include/asm/irq.h:28:49: error: ‘struct dt_device_node’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror] 28 | static inline int platform_get_irq(const struct dt_device_node *device, int index) I wouldn't be opposed to a forward declaration of that type, but as I indicated in my other reply to this patch, I'd like to avoid the addition of these essentially always-dead CONFIG_HAS_DEVICE_TREE checks to PPC code anyways, so dropping this hunk entirely from the patch might be the way forward. > ~ Oleksii Thanks, Shawn
Re: [PATCH v4 02/14] xen/asm-generic: introduce generic device.h
Hi Oleksii,
On 11/27/23 8:13 AM, Oleksii Kurochko wrote:
> diff --git a/xen/arch/ppc/include/asm/Makefile
> b/xen/arch/ppc/include/asm/Makefile
> index ece7fa66dd..df4c1ebb08 100644
> --- a/xen/arch/ppc/include/asm/Makefile
> +++ b/xen/arch/ppc/include/asm/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +generic-y += device.h
> generic-y += paging.h
> generic-y += vm_event.h
> diff --git a/xen/arch/ppc/include/asm/device.h
> b/xen/arch/ppc/include/asm/device.h
> deleted file mode 100644
> index 8253e61d51..00
> --- a/xen/arch/ppc/include/asm/device.h
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -#ifndef __ASM_PPC_DEVICE_H__
> -#define __ASM_PPC_DEVICE_H__
> -
> -enum device_type
> -{
> -DEV_DT,
> -DEV_PCI,
> -};
> -
> -struct device {
> -enum device_type type;
> -#ifdef CONFIG_HAS_DEVICE_TREE
> -struct dt_device_node *of_node; /* Used by drivers imported from Linux */
> -#endif
> -};
> -
> -enum device_class
> -{
> -DEVICE_SERIAL,
> -DEVICE_IOMMU,
> -DEVICE_PCI_HOSTBRIDGE,
> -/* Use for error */
> -DEVICE_UNKNOWN,
> -};
> -
> -struct device_desc {
> -/* Device name */
> -const char *name;
> -/* Device class */
> -enum device_class class;
> -/* List of devices supported by this driver */
> -const struct dt_device_match *dt_match;
> -/*
> - * Device initialization.
> - *
> - * -EAGAIN is used to indicate that device probing is deferred.
> - */
> -int (*init)(struct dt_device_node *dev, const void *data);
> -};
> -
> -typedef struct device device_t;
> -
> -#define DT_DEVICE_START(name_, namestr_, class_)\
> -static const struct device_desc __dev_desc_##name_ __used \
> -__section(".dev.info") = { \
> -.name = namestr_, \
> -.class = class_,\
> -
> -#define DT_DEVICE_END \
> -};
> -
> -#endif /* __ASM_PPC_DEVICE_H__ */
> diff --git a/xen/arch/ppc/include/asm/irq.h b/xen/arch/ppc/include/asm/irq.h
> index 5c37d0cf25..49193fddff 100644
> --- a/xen/arch/ppc/include/asm/irq.h
> +++ b/xen/arch/ppc/include/asm/irq.h
> @@ -3,7 +3,9 @@
> #define __ASM_PPC_IRQ_H__
>
> #include
> +#ifdef CONFIG_HAS_DEVICE_TREE
I realize that you were likely following PPC's device.h which also
checks CONFIG_HAS_DEVICE_TREE, but I'm not sure that it makes sense to
check this conditional in PPC code at all -- we will always have
HAS_DEVICE_TREE (selected by PPC) and I can't imagine a scenario where
this will ever not be the case.
Unless Jan (or someone else) disagrees, I'd rather this conditional be
dropped inside of PPC code.
> #include
> +#endif
> #include
>
> /* TODO */
> @@ -25,9 +27,11 @@ static inline void arch_move_irqs(struct vcpu *v)
> BUG_ON("unimplemented");
> }
>
> +#ifdef CONFIG_HAS_DEVICE_TREE
Ditto.
> static inline int platform_get_irq(const struct dt_device_node *device, int
> index)
> {
> BUG_ON("unimplemented");
> }
> +#endif
>
> #endif /* __ASM_PPC_IRQ_H__ */
Thanks,
Shawn
xen | Successful pipeline for staging | 18540a31
Pipeline #1088270744 has passed! Project: xen ( https://gitlab.com/xen-project/xen ) Branch: staging ( https://gitlab.com/xen-project/xen/-/commits/staging ) Commit: 18540a31 ( https://gitlab.com/xen-project/xen/-/commit/18540a313cc66a04eb15a67d74c7992a8576fbec ) Commit Message: arm/dom0less: introduce Kconfig for dom0less fe... Commit Author: Luca Fancellu ( https://gitlab.com/luca.fancellu ) Committed by: Julien Grall Pipeline #1088270744 ( https://gitlab.com/xen-project/xen/-/pipelines/1088270744 ) triggered by Ganis ( https://gitlab.com/ganis ) successfully completed 129 jobs in 3 stages. -- You're receiving this email because of your account on gitlab.com.
[ovmf test] 183909: all pass - PUSHED
flight 183909 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/183909/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf e1627f77201ac55d24e9d0e3380f9dbdc600843c baseline version: ovmf 9eec96bd4fc53d7836b5606f2a8bbb10713cc8f5 Last test of basis 183895 2023-11-28 13:41:02 Z0 days Testing same since 183909 2023-11-28 18:11:03 Z0 days1 attempts People who touched revisions under test: Michael Kubacki 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 9eec96bd4f..e1627f7720 e1627f77201ac55d24e9d0e3380f9dbdc600843c -> xen-tested-master
Re: [PATCH v4 01/14] xen/asm-generic: introduce stub header paging.h
Hi Oleksii, On 11/27/23 8:13 AM, Oleksii Kurochko wrote: > The patch introduces generic paging.h header for Arm, PPC and > RISC-V. > > All mentioned above architectures use hardware virt extensions > and hardware pagetable extensions thereby it makes sense to set > paging_mode_translate and paging_mode_external by default. > > Also in this patch Arm and PPC architectures are switched to > generic paging.h header. Acked-by: Shawn Anastasio Thanks, Shawn
Re: [PATCH v2] xen: remove
My apologies for the delay on this and thank you for reaching to me for
the ping, Oleksii.
Acked-by: Shawn Anastasio
On 11/27/23 4:26 AM, Oleksii wrote:
> Hello Shawn,
>
> Could you kindly review the patch when you have a moment?
> It can help with merging other patch series.
>
> Thanks in advance.
>
> ~ Oleksii
>
> On Tue, 2023-10-31 at 16:28 +0200, Oleksii Kurochko wrote:
>> only declares udelay() function so udelay()
>> declaration was moved to xen/delay.h.
>>
>> For x86, __udelay() was renamed to udelay() and removed
>> inclusion of in x86 code.
>>
>> For ppc, udelay() stub definition was moved to ppc/stubs.c.
>>
>> Suggested-by: Jan Beulich
>> Signed-off-by: Oleksii Kurochko
>> Reviewed-by: Jan Beulich
>> ---
>> Changes in v2:
>> - rebase on top of the latest staging.
>> - add Suggested-by:/Reviewed-by: Jan Beulich .
>> - remove for PPC.
>> - remove changes related to RISC-V's as they've not
>> introduced in staging branch yet.
>> ---
>> xen/arch/arm/include/asm/delay.h | 14 --
>> xen/arch/ppc/include/asm/delay.h | 12
>> xen/arch/ppc/stubs.c | 7 +++
>> xen/arch/x86/cpu/microcode/core.c | 2 +-
>> xen/arch/x86/delay.c | 2 +-
>> xen/arch/x86/include/asm/delay.h | 13 -
>> xen/include/xen/delay.h | 2 +-
>> 7 files changed, 10 insertions(+), 42 deletions(-)
>> delete mode 100644 xen/arch/arm/include/asm/delay.h
>> delete mode 100644 xen/arch/ppc/include/asm/delay.h
>> delete mode 100644 xen/arch/x86/include/asm/delay.h
>>
>> diff --git a/xen/arch/arm/include/asm/delay.h
>> b/xen/arch/arm/include/asm/delay.h
>> deleted file mode 100644
>> index 042907d9d5..00
>> --- a/xen/arch/arm/include/asm/delay.h
>> +++ /dev/null
>> @@ -1,14 +0,0 @@
>> -#ifndef _ARM_DELAY_H
>> -#define _ARM_DELAY_H
>> -
>> -extern void udelay(unsigned long usecs);
>> -
>> -#endif /* defined(_ARM_DELAY_H) */
>> -/*
>> - * Local variables:
>> - * mode: C
>> - * c-file-style: "BSD"
>> - * c-basic-offset: 4
>> - * indent-tabs-mode: nil
>> - * End:
>> - */
>> diff --git a/xen/arch/ppc/include/asm/delay.h
>> b/xen/arch/ppc/include/asm/delay.h
>> deleted file mode 100644
>> index da6635888b..00
>> --- a/xen/arch/ppc/include/asm/delay.h
>> +++ /dev/null
>> @@ -1,12 +0,0 @@
>> -/* SPDX-License-Identifier: GPL-2.0-only */
>> -#ifndef __ASM_PPC_DELAY_H__
>> -#define __ASM_PPC_DELAY_H__
>> -
>> -#include
>> -
>> -static inline void udelay(unsigned long usecs)
>> -{
>> - BUG_ON("unimplemented");
>> -}
>> -
>> -#endif /* __ASM_PPC_DELAY_H__ */
>> diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
>> index 4c276b0e39..a96e45626d 100644
>> --- a/xen/arch/ppc/stubs.c
>> +++ b/xen/arch/ppc/stubs.c
>> @@ -337,3 +337,10 @@ int __init parse_arch_dom0_param(const char *s,
>> const char *e)
>> {
>> BUG_ON("unimplemented");
>> }
>> +
>> +/* delay.c */
>> +
>> +void udelay(unsigned long usecs)
>> +{
>> + BUG_ON("unimplemented");
>> +}
>> diff --git a/xen/arch/x86/cpu/microcode/core.c
>> b/xen/arch/x86/cpu/microcode/core.c
>> index 65ebeb50de..22d5e04552 100644
>> --- a/xen/arch/x86/cpu/microcode/core.c
>> +++ b/xen/arch/x86/cpu/microcode/core.c
>> @@ -23,6 +23,7 @@
>>
>> #include
>> #include
>> +#include
>> #include
>> #include
>> #include
>> @@ -35,7 +36,6 @@
>>
>> #include
>> #include
>> -#include
>> #include
>> #include
>> #include
>> diff --git a/xen/arch/x86/delay.c b/xen/arch/x86/delay.c
>> index 2662c26272..b3a41881a1 100644
>> --- a/xen/arch/x86/delay.c
>> +++ b/xen/arch/x86/delay.c
>> @@ -15,7 +15,7 @@
>> #include
>> #include
>>
>> -void __udelay(unsigned long usecs)
>> +void udelay(unsigned long usecs)
>> {
>> unsigned long ticks = usecs * (cpu_khz / 1000);
>> unsigned long s, e;
>> diff --git a/xen/arch/x86/include/asm/delay.h
>> b/xen/arch/x86/include/asm/delay.h
>> deleted file mode 100644
>> index 9be2f46590..00
>> --- a/xen/arch/x86/include/asm/delay.h
>> +++ /dev/null
>> @@ -1,13 +0,0 @@
>> -#ifndef _X86_DELAY_H
>> -#define _X86_DELAY_H
>> -
>> -/*
>> - * Copyright (C) 1993 Linus Torvalds
>> - *
>> - * Delay routines calling functions in arch/i386/lib/delay.c
>> - */
>> -
>> -extern void __udelay(unsigned long usecs);
>> -#define udelay(n) __udelay(n)
>> -
>> -#endif /* defined(_X86_DELAY_H) */
>> diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h
>> index 9150226271..8fd3b8f99f 100644
>> --- a/xen/include/xen/delay.h
>> +++ b/xen/include/xen/delay.h
>> @@ -3,7 +3,7 @@
>>
>> /* Copyright (C) 1993 Linus Torvalds */
>>
>> -#include
>> +void udelay(unsigned long usecs);
>>
>> static inline void mdelay(unsigned long msec)
>> {
>
Re: [PATCH v6 3/5] arm/dom0less: put dom0less feature code in a separate module
> On 28 Nov 2023, at 18:25, Julien Grall wrote: > > Hi, > > On 24/11/2023 12:01, Michal Orzel wrote: >> On 24/11/2023 10:48, Luca Fancellu wrote: >>> >>> >>> Currently the dom0less feature code is mostly inside domain_build.c >>> and setup.c, it is a feature that may not be useful to everyone so >>> put the code in a different compilation module in order to make it >>> easier to disable the feature in the future. >>> >>> Move gic_interrupt_t in domain_build.h to use it with the function >>> declaration, move its comment above the declaration. >>> >>> The following functions are now visible externally from domain_build >>> because they are used also from the dom0less-build module: >>> - get_allocation_size >>> - set_interrupt >>> - domain_fdt_begin_node >>> - make_memory_node >>> - make_resv_memory_node >>> - make_hypervisor_node >>> - make_psci_node >>> - make_cpus_node >>> - make_timer_node >>> - handle_device_interrupts >>> - construct_domain >>> - process_shm >>> - allocate_bank_memory >>> >>> The functions allocate_static_memory and assign_static_memory_11 >>> are now externally visible, so put their declarations into >>> domain_build.h and move the #else and stub definition in the header >>> as well. >>> >>> Move is_dom0less_mode from setup.c to dom0less-build.c and make it >>> externally visible. >>> >>> The function allocate_bank_memory is used only by dom0less code >>> at the moment, but it's been decided to leave it in domain_build.c >>> in case that in the future the dom0 code can use it. >>> >>> Where spotted, fix code style issues. >>> >>> No functional change is intended. >>> >>> Signed-off-by: Luca Fancellu >>> Reviewed-by: Michal Orzel >>> --- >>> Changes from v5: >>> - remove unneeded include (Michal) >> Including asm/kernel.h was indeed not required. However, I'm thinking that >> if we want the header >> to be self-contained and given that ... >> [...] >>> diff --git a/xen/arch/arm/include/asm/dom0less-build.h >>> b/xen/arch/arm/include/asm/dom0less-build.h >>> new file mode 100644 >>> index ..c5625925d940 >>> --- /dev/null >>> +++ b/xen/arch/arm/include/asm/dom0less-build.h >>> @@ -0,0 +1,18 @@ >>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>> + >>> +#ifndef __ASM_DOM0LESS_BUILD_H_ >>> +#define __ASM_DOM0LESS_BUILD_H_ >>> + >>> +void create_domUs(void); >>> +bool is_dom0less_mode(void); >> here we use bool, I think we should include > > I have done the change on commit. However, this introduced a clash in the > next patch. > > I have done it this time because it is minor. Thank you for that Julien, I didn’t realise there would have been a clash, sorry for the Inconvenience. Cheers, Luca > > Cheers, > > -- > Julien Grall
Re: [PATCH v2 27/29] tools/xenstored: add helpers for filename handling
On Wed, Nov 15, 2023 at 1:14 AM Juergen Gross wrote:
>
> On 14.11.23 21:53, Julien Grall wrote:
> > Hi Juergen,
> >
> > On 14/11/2023 09:26, Juergen Gross wrote:
> >> On 14.11.23 10:10, Julien Grall wrote:
> >>> Hi Juergen,
> >>>
> >>> On 14/11/2023 06:45, Juergen Gross wrote:
> On 13.11.23 23:25, Julien Grall wrote:
> > Hi Juergen,
> >
> > On 10/11/2023 16:08, Juergen Gross wrote:
> >> diff --git a/tools/xenstored/lu_daemon.c b/tools/xenstored/lu_daemon.c
> >> index 71bcabadd3..635ab0 100644
> >> --- a/tools/xenstored/lu_daemon.c
> >> +++ b/tools/xenstored/lu_daemon.c
> >> @@ -24,7 +24,7 @@ void lu_get_dump_state(struct lu_dump_state *state)
> >> state->size = 0;
> >> state->filename = talloc_asprintf(NULL, "%s/state_dump",
> >> - xenstore_daemon_rundir());
> >> + xenstore_rundir());
> >
> > ... call and ...
> >
> >> if (!state->filename)
> >> barf("Allocation failure");
> >> @@ -65,7 +65,7 @@ FILE *lu_dump_open(const void *ctx)
> >> int fd;
> >> filename = talloc_asprintf(ctx, "%s/state_dump",
> >> - xenstore_daemon_rundir());
> >> + xenstore_rundir());
> >
> > ... this one could be replaced with absolute_filename().
>
> No, I don't think this is a good idea.
>
> I don't want the daemon to store trace files specified as relative files
> to be stored in /var/run/xen, while I want all files of the stubdom to be
> stored under /var/lib/xen.
> >>>
> >>> Why? This is a bit odd to have a different behavior between stubdom and
> >>> daemon. It would be much easier for the user if they knew all the files
> >>> would
> >>> be at the same place regardless the version used.
> >>
> >> The main difference is that stubdom has access to only _one_ directory in
> >> dom0.
> >
> > Would you be able to explain why we can only give access to a single
> > directory?
> > Is this because of the 9pfs protocol?
>
> Yes. I can mount a specific dom0 directory in the guest.
I'm fine with a single directory being used for stubdom. Two
directories could be exported, and mini-os would need to use the "tag"
to differentiate the two. That may not be worth the added code. QEMU
can provide multiple 9pfs exports and Linux can mount them by tag
name.
Regards,
Jason
Re: [PATCH v2 18/29] tools/xenstored: rename xenbus_evtchn()
On Fri, Nov 10, 2023 at 11:23 AM Juergen Gross wrote: > > Rename the xenbus_evtchn() function to get_xenbus_evtchn() in order to > avoid two externally visible symbols with the same name when Xenstore- > stubdom is being built with a Mini-OS with CONFIG_XENBUS set. > > Signed-off-by: Juergen Gross > Reviewed-by: Julien Grall Reviewed-by: Jason Andryuk
Re: [PATCH v2 16/29] tools/xl: support new 9pfs backend xen-9pfsd
On Fri, Nov 10, 2023 at 11:19 AM Juergen Gross wrote: > > Add support for the new 9pfs backend "xen-9pfsd". For this backend type > the tag defaults to "Xen" and the host side path to > "/var/log/xen/guests/". > > Signed-off-by: Juergen Gross Reviewed-by: Jason Andryuk
Re: [PATCH v2 11/29] tools/xenlogd: add 9pfs create request support
On Fri, Nov 10, 2023 at 12:14 PM Juergen Gross wrote: > > Add the create request of the 9pfs protocol. > > Signed-off-by: Juergen Gross Reviewed-by: Jason Andryuk
Re: [PATCH v2 10/29] tools/xenlogd: add 9pfs clunk request support
On Fri, Nov 10, 2023 at 11:22 AM Juergen Gross wrote: > > Add the clunk request of the 9pfs protocol. > > Signed-off-by: Juergen Gross Reviewed-by: Jason Andryuk
[PATCH v8 2/2] xen/vpci: header: filter PCI capabilities
Currently, Xen vPCI only supports virtualizing the MSI and MSI-X capabilities.
Hide all other PCI capabilities (including extended capabilities) from domUs for
now, even though there may be certain devices/drivers that depend on being able
to discover certain capabilities.
We parse the physical PCI capabilities linked list and add vPCI register
handlers for the next elements, inserting our own next value, thus presenting a
modified linked list to the domU.
Introduce helper functions vpci_hw_read8 and vpci_read_val. The vpci_read_val
helper function returns a fixed value, which may be used for RAZ registers, or
registers whose value doesn't change.
Introduce pci_find_next_cap_ttl() helper while adapting the logic from
pci_find_next_cap() to suit our needs, and implement the existing
pci_find_next_cap() in terms of the new helper.
Signed-off-by: Stewart Hildebrand
---
v7->v8:
* use to array instead of match function
* include lib.h for ARRAY_SIZE
* don't emulate PCI_CAPABILITY_LIST register if PCI_STATUS_CAP_LIST bit is not
set in hardware
* spell out RAZ/WI acronym
* dropped R-b tag since the patch has changed moderately since the last rev
v6->v7:
* no change
v5->v6:
* add register handlers before status register handler in init_bars()
* s/header->mask_cap_list/mask_cap_list/
v4->v5:
* use more appropriate types, continued
* get rid of unnecessary hook function
* add Jan's R-b
v3->v4:
* move mask_cap_list setting to this patch
* leave pci_find_next_cap signature alone
* use more appropriate types
v2->v3:
* get rid of > 0 in loop condition
* implement pci_find_next_cap in terms of new pci_find_next_cap_ttl function so
that hypothetical future callers wouldn't be required to pass
* change NULL to (void *)0 for RAZ value passed to vpci_read_val
* change type of ttl to unsigned int
* remember to mask off the low 2 bits of next in the initial loop iteration
* change return type of pci_find_next_cap and pci_find_next_cap_ttl
* avoid wrapping the PCI_STATUS_CAP_LIST condition by using ! instead of == 0
v1->v2:
* change type of ttl to int
* use switch statement instead of if/else
* adapt existing pci_find_next_cap helper instead of rolling our own
* pass ttl as in/out
* "pass through" the lower 2 bits of the next pointer
* squash helper functions into this patch to avoid transient dead code situation
* extended capabilities RAZ/WI
---
xen/drivers/pci/pci.c | 31 ---
xen/drivers/vpci/header.c | 63 +++
xen/drivers/vpci/vpci.c | 12
xen/include/xen/pci.h | 3 ++
xen/include/xen/vpci.h| 5
5 files changed, 104 insertions(+), 10 deletions(-)
diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
index 3569ccb24e9e..1645b3118220 100644
--- a/xen/drivers/pci/pci.c
+++ b/xen/drivers/pci/pci.c
@@ -39,31 +39,42 @@ unsigned int pci_find_cap_offset(pci_sbdf_t sbdf, unsigned
int cap)
return 0;
}
-unsigned int pci_find_next_cap(pci_sbdf_t sbdf, unsigned int pos,
- unsigned int cap)
+unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, unsigned int pos,
+ unsigned int *cap, unsigned int n,
+ unsigned int *ttl)
{
-u8 id;
-int ttl = 48;
+unsigned int id, i;
-while ( ttl-- )
+while ( (*ttl)-- )
{
pos = pci_conf_read8(sbdf, pos);
if ( pos < 0x40 )
break;
-pos &= ~3;
-id = pci_conf_read8(sbdf, pos + PCI_CAP_LIST_ID);
+id = pci_conf_read8(sbdf, (pos & ~3) + PCI_CAP_LIST_ID);
if ( id == 0xff )
break;
-if ( id == cap )
-return pos;
+for ( i = 0; i < n; i++ )
+{
+if ( id == cap[i] )
+return pos;
+}
-pos += PCI_CAP_LIST_NEXT;
+pos = (pos & ~3) + PCI_CAP_LIST_NEXT;
}
+
return 0;
}
+unsigned int pci_find_next_cap(pci_sbdf_t sbdf, unsigned int pos,
+ unsigned int cap)
+{
+unsigned int ttl = 48;
+
+return pci_find_next_cap_ttl(sbdf, pos, , 1, ) & ~3;
+}
+
/**
* pci_find_ext_capability - Find an extended capability
* @sbdf: PCI device to query
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 351318121e48..d7dc0c82a6ba 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -18,6 +18,7 @@
*/
#include
+#include
#include
#include
#include
@@ -545,6 +546,68 @@ static int cf_check init_bars(struct pci_dev *pdev)
if ( rc )
return rc;
+if ( !is_hardware_domain(pdev->domain) )
+{
+if ( pci_conf_read16(pdev->sbdf, PCI_STATUS) & PCI_STATUS_CAP_LIST )
+{
+/* Only expose capabilities to the guest that vPCI can handle. */
+unsigned int next, ttl = 48;
+unsigned int supported_caps[] = {
+PCI_CAP_ID_MSI,
+PCI_CAP_ID_MSIX,
+};
+
+
[PATCH v8 1/2] xen/vpci: header: status register handler
Introduce a handler for the PCI status register, with ability to mask
the capabilities bit. The status register contains RsvdZ bits,
read-only bits, and write-1-to-clear bits. Additionally, we use RsvdP to
mask the capabilities bit. Introduce bitmasks to handle these in vPCI.
If a bit in the bitmask is set, then the special meaning applies:
ro_mask: read normal, guest write ignore (preserve on write to hardware)
rw1c_mask: read normal, write 1 to clear
rsvdp_mask: read as zero, guest write ignore (preserve on write to hardware)
rsvdz_mask: read as zero, guest write ignore (write zero to hardware)
The RO/RW1C/RsvdP/RsvdZ naming and definitions were borrowed from the
PCI Express Base 6.1 specification. RsvdP/RsvdZ bits help Xen enforce
our view of the world. Xen preserves the value of read-only bits on
write to hardware, discarding the guests write value. This is done in
case hardware wrongly implements R/O bits as R/W.
The mask_cap_list flag will be set in a follow-on patch.
Signed-off-by: Stewart Hildebrand
---
v7->v8:
* move PCI_STATUS_UDF to rsvdz_mask (per PCI Express Base 6 spec)
* add support for rsvdp bits
* add tests for ro/rw1c/rsvdp/rsvdz bits in tools/tests/vpci/main.c
* dropped R-b tag [1] since the patch has changed moderately since the last rev
[1] https://lists.xenproject.org/archives/html/xen-devel/2023-09/msg00909.html
v6->v7:
* re-work args passed to vpci_add_register_mask() (called in init_bars())
* also check for overlap of (rsvdz_mask & ro_mask) in add_register()
* slightly adjust masking operation in vpci_write_helper()
v5->v6:
* remove duplicate PCI_STATUS_CAP_LIST in constant definition
* style fixup in constant definitions
* s/res_mask/rsvdz_mask/
* combine a new masking operation into single line
* preserve r/o bits on write
* get rid of status_read. Instead, use rsvdz_mask for conditionally masking
PCI_STATUS_CAP_LIST bit
* add comment about PCI_STATUS_CAP_LIST and rsvdp behavior
* add sanity checks in add_register
* move mask_cap_list from struct vpci_header to local variable
v4->v5:
* add support for res_mask
* add support for ro_mask (squash ro_mask patch)
* add constants for reserved, read-only, and rw1c masks
v3->v4:
* move mask_cap_list setting to the capabilities patch
* single pci_conf_read16 in status_read
* align mask_cap_list bitfield in struct vpci_header
* change to rw1c bit mask instead of treating whole register as rw1c
* drop subsystem prefix on renamed add_register function
v2->v3:
* new patch
---
tools/tests/vpci/main.c| 98 ++
xen/drivers/vpci/header.c | 12 +
xen/drivers/vpci/vpci.c| 62 +++-
xen/include/xen/pci_regs.h | 9
xen/include/xen/vpci.h | 9
5 files changed, 178 insertions(+), 12 deletions(-)
diff --git a/tools/tests/vpci/main.c b/tools/tests/vpci/main.c
index b9a0a6006bb9..b0bb993be297 100644
--- a/tools/tests/vpci/main.c
+++ b/tools/tests/vpci/main.c
@@ -70,6 +70,26 @@ static void vpci_write32(const struct pci_dev *pdev,
unsigned int reg,
*(uint32_t *)data = val;
}
+struct mask_data {
+uint32_t val;
+uint32_t rw1c_mask;
+};
+
+static uint32_t vpci_read32_mask(const struct pci_dev *pdev, unsigned int reg,
+ void *data)
+{
+struct mask_data *md = data;
+return md->val;
+}
+
+static void vpci_write32_mask(const struct pci_dev *pdev, unsigned int reg,
+ uint32_t val, void *data)
+{
+struct mask_data *md = data;
+md->val = val | (md->val & md->rw1c_mask);
+md->val &= ~(val & md->rw1c_mask);
+}
+
#define VPCI_READ(reg, size, data) ({ \
data = vpci_read((pci_sbdf_t){ .sbdf = 0 }, reg, size); \
})
@@ -94,9 +114,20 @@ static void vpci_write32(const struct pci_dev *pdev,
unsigned int reg,
assert(!vpci_add_register(test_pdev.vpci, fread, fwrite, off, size, \
))
+#define VPCI_ADD_REG_MASK(fread, fwrite, off, size, store,
\
+ ro_mask, rw1c_mask, rsvdp_mask, rsvdz_mask)
\
+assert(!vpci_add_register_mask(test_pdev.vpci, fread, fwrite, off, size,
\
+ ,
\
+ ro_mask, rw1c_mask, rsvdp_mask, rsvdz_mask))
+
#define VPCI_ADD_INVALID_REG(fread, fwrite, off, size) \
assert(vpci_add_register(test_pdev.vpci, fread, fwrite, off, size, NULL))
+#define VPCI_ADD_INVALID_REG_MASK(fread, fwrite, off, size, \
+ ro_mask, rw1c_mask, rsvdp_mask, rsvdz_mask) \
+assert(vpci_add_register_mask(test_pdev.vpci, fread, fwrite, off, size, \
+ NULL, ro_mask, rw1c_mask, rsvdp_mask,
rsvdz_mask))
+
#define VPCI_REMOVE_REG(off, size) \
assert(!vpci_remove_register(test_pdev.vpci, off,
[PATCH v8 0/2] vPCI capabilities filtering
This small series enables vPCI to filter which PCI capabilities we expose to a
domU. This series adds vPCI register handlers within
xen/drivers/vpci/header.c:init_bars(), along with some supporting functions.
Note there are minor rebase conflicts with the in-progress vPCI series [1].
These conflicts fall into the category of functions and code being added
adjacent to one another, so are easily resolved. I did not identify any
dependency on the vPCI locking work, and the two series deal with different
aspects of emulating the PCI header.
Future work may involve adding handlers for more registers in the vPCI header,
such as VID/DID, etc. Future work may also involve exposing additional
capabilities to the guest for broader device/driver support.
v7->v8:
* address feedback
v6->v7:
* address feedback in ("xen/vpci: header: status register handler")
* drop ("xen/pci: convert pci_find_*cap* to pci_sbdf_t") and
("x86/msi: rearrange read_pci_mem_bar slightly") as they were committed
v5->v6:
* drop ("xen/pci: update PCI_STATUS_* constants") as it has been committed
v4->v5:
* drop ("x86/msi: remove some unused-but-set-variables") as it has been
committed
* add ("xen/pci: update PCI_STATUS_* constants")
* squash ro_mask patch
v3->v4:
* drop "xen/pci: address a violation of MISRA C:2012 Rule 8.3" as it has been
committed
* re-order status register handler and capabilities filtering patches
* split an unrelated change from ("xen/pci: convert pci_find_*cap* to
pci_sbdf_t")
into its own patch
* add new patch ("x86/msi: rearrange read_pci_mem_bar slightly") based on
feedback
* add new RFC patch ("xen/vpci: support ro mask")
v2->v3:
* drop RFC "xen/vpci: header: avoid cast for value passed to vpci_read_val"
* minor misra C violation fixup in preparatory patch
* switch to pci_sbdf_t in preparatory patch
* introduce status handler
v1->v2:
* squash helper functions into the patch where they are used to avoid transient
dead code situation
* add new RFC patch, possibly throwaway, to get an idea of what it would look
like to get rid of the (void *)(uintptr_t) cast by introducing a new memory
allocation
[1] https://lists.xenproject.org/archives/html/xen-devel/2023-08/msg02361.html
Stewart Hildebrand (2):
xen/vpci: header: status register handler
xen/vpci: header: filter PCI capabilities
tools/tests/vpci/main.c| 98 ++
xen/drivers/pci/pci.c | 31
xen/drivers/vpci/header.c | 75 +
xen/drivers/vpci/vpci.c| 74 +++-
xen/include/xen/pci.h | 3 ++
xen/include/xen/pci_regs.h | 9
xen/include/xen/vpci.h | 14 ++
7 files changed, 282 insertions(+), 22 deletions(-)
base-commit: 18540a313cc66a04eb15a67d74c7992a8576fbec
--
2.43.0
Re: [PATCH v12 24/37] x86/idtentry: Incorporate definitions/declarations of the FRED entries
On Tue, Nov 28, 2023 at 10:58:50AM -0800, H. Peter Anvin wrote: > >You have a very good sense I've been reading code of a couple of people for a couple of years. :-) > Remember that Signed-off-by: relates to the *patch flow*. Yes, you should take the time to read Documentation/process/ and especially submitting-patches.rst. We have it all written down and I point new(-er) people at that directory at least once a week. :-\ -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette
RE: [PATCH v12 24/37] x86/idtentry: Incorporate definitions/declarations of the FRED entries
On November 28, 2023 10:39:01 AM PST, "Li, Xin3" wrote: >> > FRED and IDT can share most of the definitions and declarations so >> > that in the majority of cases the actual handler implementation is the >> > same. >> > >> > The differences are the exceptions where FRED stores exception related >> > information on the stack and the sysvec implementations as FRED can >> > handle irqentry/exit() in the dispatcher instead of having it in each >> > handler. >> > >> > Also add stub defines for vectors which are not used due to Kconfig >> > decisions to spare the ifdeffery in the actual FRED dispatch code. >> > >> > Tested-by: Shan Kang >> > Signed-off-by: Thomas Gleixner >> > Signed-off-by: Xin Li >> >> This makes me wonder too who the author is. The commit message text sounds >> like tglx. :) > >You have a very good sense > >This is mostly from his review comments and suggestions on my original >changes to IDTENTRY. So probably I should put a "Suggested-by" instead >of "Signed-off-by" as HPA pointed out! > >Thanks! >Xin > Remember that Signed-off-by: relates to the *patch flow*.
RE: [PATCH v12 24/37] x86/idtentry: Incorporate definitions/declarations of the FRED entries
> > FRED and IDT can share most of the definitions and declarations so > > that in the majority of cases the actual handler implementation is the > > same. > > > > The differences are the exceptions where FRED stores exception related > > information on the stack and the sysvec implementations as FRED can > > handle irqentry/exit() in the dispatcher instead of having it in each > > handler. > > > > Also add stub defines for vectors which are not used due to Kconfig > > decisions to spare the ifdeffery in the actual FRED dispatch code. > > > > Tested-by: Shan Kang > > Signed-off-by: Thomas Gleixner > > Signed-off-by: Xin Li > > This makes me wonder too who the author is. The commit message text sounds > like tglx. :) You have a very good sense This is mostly from his review comments and suggestions on my original changes to IDTENTRY. So probably I should put a "Suggested-by" instead of "Signed-off-by" as HPA pointed out! Thanks! Xin
Re: [PATCH v6 3/5] arm/dom0less: put dom0less feature code in a separate module
Hi, On 24/11/2023 12:01, Michal Orzel wrote: On 24/11/2023 10:48, Luca Fancellu wrote: Currently the dom0less feature code is mostly inside domain_build.c and setup.c, it is a feature that may not be useful to everyone so put the code in a different compilation module in order to make it easier to disable the feature in the future. Move gic_interrupt_t in domain_build.h to use it with the function declaration, move its comment above the declaration. The following functions are now visible externally from domain_build because they are used also from the dom0less-build module: - get_allocation_size - set_interrupt - domain_fdt_begin_node - make_memory_node - make_resv_memory_node - make_hypervisor_node - make_psci_node - make_cpus_node - make_timer_node - handle_device_interrupts - construct_domain - process_shm - allocate_bank_memory The functions allocate_static_memory and assign_static_memory_11 are now externally visible, so put their declarations into domain_build.h and move the #else and stub definition in the header as well. Move is_dom0less_mode from setup.c to dom0less-build.c and make it externally visible. The function allocate_bank_memory is used only by dom0less code at the moment, but it's been decided to leave it in domain_build.c in case that in the future the dom0 code can use it. Where spotted, fix code style issues. No functional change is intended. Signed-off-by: Luca Fancellu Reviewed-by: Michal Orzel --- Changes from v5: - remove unneeded include (Michal) Including asm/kernel.h was indeed not required. However, I'm thinking that if we want the header to be self-contained and given that ... [...] diff --git a/xen/arch/arm/include/asm/dom0less-build.h b/xen/arch/arm/include/asm/dom0less-build.h new file mode 100644 index ..c5625925d940 --- /dev/null +++ b/xen/arch/arm/include/asm/dom0less-build.h @@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __ASM_DOM0LESS_BUILD_H_ +#define __ASM_DOM0LESS_BUILD_H_ + +void create_domUs(void); +bool is_dom0less_mode(void); here we use bool, I think we should include I have done the change on commit. However, this introduced a clash in the next patch. I have done it this time because it is minor. Cheers, -- Julien Grall
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
Hi Michal, On 28/11/2023 18:15, Michal Orzel wrote: On 28/11/2023 18:09, Julien Grall wrote: On 28/11/2023 18:00, Michal Orzel wrote: Hi Julien, On 28/11/2023 17:14, Julien Grall wrote: Hi Michal, On 27/11/2023 15:41, Michal Orzel wrote: Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt attention to undefined behavior issues, notably during CI test runs, is essential. When enabled, this option causes Xen to panic upon detecting UBSAN failure (as the last step in ubsan_epilogue()). I have mixed opinions on this patch. This would be a good one if we had a Xen mostly free from UBSAN behavior. But this is not the case at least on arm32. So we would end up to stop at the first error. IOW, we would need to fix the first error before we can see the next one. Well, this patch introduces a config option disabled by default. I understood this is disabled by default... I am pointing out that I am not convinced about the usefulness until we are at the stage where Xen is normally not reporting any USBAN error. If we end up enabling it for CI for reasons* stated by Andrew, then the natural way of handling such issues is to do the investigation locally. This will not always be possible. One example is when you are only able to reproduce some of the USBAN errors on a specific HW. Then, you are not forced to select this option and you can see all the UBSAN issues if you want. See above, I got that point. I am mostly concerned about the implication in the CI right now. So I feel it would be best if the gitlab CI jobs actually check for USBAN in the logs and fail if there are any. With that, we can get the full list of UBSAN issues on each job. Well, I prefer Andrew suggestion (both [1] and on IRC), hence this patch. *my plan was to first fix the UBSAN issues and then enable this option for CI. That would have been useful to describe your goal after "---". With that in mind, then I suggest to revisit this patch once all the UBSAN issues in a normal build are fixed. But this patch does not enable this option for CI automatically, right? Correct. Why are you so keen to push it back? I have been pushing back because your commit message refers to the CI specifically and today this would not really work (read as I would not be happy if this option is enabled in the CI right now at least on arm32). If you want to fail a CI job for UBSAN today, then we need to find a different approach so we can get the full list of UBSAN error rather than fixing one, retry and then next one. Is it because you see no point in this option other than for the upstream CI loop? Even in the upstream CI loop, I am a little unsure about this approach. At least, I feel I would want to see all the reports at once in the CI. But this is not really a strong feeling. I find it useful on a day-to-day Xen runs, and I would for sure enable it by default in my config not to miss UBSAN failures. Fair enough. I view USBAN issues the same a WARN_ON. They all need to be investigated. So now you have an inconsistent policy. Are you are also intending to switch WARN_ON() to fatal? If not, then why would UBSAN warnings more important that the other warnings? Cheers, -- Julien Grall
[PATCH] Config.mk: drop -Wdeclaration-after-statement
Such constructs are fully allowed by C99: https://gcc.gnu.org/onlinedocs/gcc-12.2.0/gcc/Mixed-Labels-and-Declarations.html#Mixed-Labels-and-Declarations If the flag is present, then building against python 3.12 will fail thusly: | In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:44, | from xen/lowlevel/xc/xc.c:8: | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h: In function 'Py_SIZE': | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h:233:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] | 233 | PyVarObject *var_ob = _PyVarObject_CAST(ob); | | ^~~ | In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:53: | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h: In function '_PyLong_CompactValue': | /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h:121:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] | 121 | Py_ssize_t sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK); | | ^~ | cc1: all warnings being treated as errors Signed-off-by: Alexander Kanavin --- Config.mk | 2 -- 1 file changed, 2 deletions(-) diff --git a/Config.mk b/Config.mk index 2c43702958..7e67b91de2 100644 --- a/Config.mk +++ b/Config.mk @@ -177,8 +177,6 @@ CFLAGS += -std=gnu99 CFLAGS += -Wall -Wstrict-prototypes -$(call cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement) -$(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement) $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable) $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs) -- 2.39.2
Re: [PATCH] xen/livepatch: fix livepatch tests
On 28/11/2023 5:41 pm, Roger Pau Monne wrote:
> The current set of in-tree livepatch tests in xen/test/livepatch started
> failing after the constify of the payload funcs array, and the movement of the
> status data into a separate array.
>
> Fix the tests so they respect the constness of the funcs array and also make
> use of the new location of the per-func state data.
>
> Fixes: 82182ad7b46e ('livepatch: do not use .livepatch.funcs section to store
> internal state')
> Signed-off-by: Roger Pau Monné
Acked-by: Andrew Cooper
[PATCH] xen/livepatch: fix livepatch tests
The current set of in-tree livepatch tests in xen/test/livepatch started
failing after the constify of the payload funcs array, and the movement of the
status data into a separate array.
Fix the tests so they respect the constness of the funcs array and also make
use of the new location of the per-func state data.
Fixes: 82182ad7b46e ('livepatch: do not use .livepatch.funcs section to store
internal state')
Signed-off-by: Roger Pau Monné
---
I will see about getting those tests build in gitlab, in the meantime we should
take this fix in order to unblock osstest.
---
xen/test/livepatch/xen_action_hooks.c | 12 +-
xen/test/livepatch/xen_action_hooks_marker.c | 20 ++---
xen/test/livepatch/xen_action_hooks_noapply.c | 22 +++
xen/test/livepatch/xen_action_hooks_nofunc.c | 6 ++---
.../livepatch/xen_action_hooks_norevert.c | 22 +++
xen/test/livepatch/xen_prepost_hooks.c| 8 +++
xen/test/livepatch/xen_prepost_hooks_fail.c | 2 +-
7 files changed, 53 insertions(+), 39 deletions(-)
diff --git a/xen/test/livepatch/xen_action_hooks.c
b/xen/test/livepatch/xen_action_hooks.c
index 39b531302731..fa0b3ab35f38 100644
--- a/xen/test/livepatch/xen_action_hooks.c
+++ b/xen/test/livepatch/xen_action_hooks.c
@@ -26,9 +26,10 @@ static int apply_hook(livepatch_payload_t *payload)
for (i = 0; i < payload->nfuncs; i++)
{
-struct livepatch_func *func = >funcs[i];
+const struct livepatch_func *func = >funcs[i];
+struct livepatch_fstate *fstate = >fstate[i];
-func->applied = LIVEPATCH_FUNC_APPLIED;
+fstate->applied = LIVEPATCH_FUNC_APPLIED;
apply_cnt++;
printk(KERN_DEBUG "%s: applying: %s\n", __func__, func->name);
@@ -47,9 +48,10 @@ static int revert_hook(livepatch_payload_t *payload)
for (i = 0; i < payload->nfuncs; i++)
{
-struct livepatch_func *func = >funcs[i];
+const struct livepatch_func *func = >funcs[i];
+struct livepatch_fstate *fstate = >fstate[i];
-func->applied = LIVEPATCH_FUNC_NOT_APPLIED;
+fstate->applied = LIVEPATCH_FUNC_NOT_APPLIED;
revert_cnt++;
printk(KERN_DEBUG "%s: reverting: %s\n", __func__, func->name);
@@ -68,7 +70,7 @@ static void post_revert_hook(livepatch_payload_t *payload)
for (i = 0; i < payload->nfuncs; i++)
{
-struct livepatch_func *func = >funcs[i];
+const struct livepatch_func *func = >funcs[i];
printk(KERN_DEBUG "%s: reverted: %s\n", __func__, func->name);
}
diff --git a/xen/test/livepatch/xen_action_hooks_marker.c
b/xen/test/livepatch/xen_action_hooks_marker.c
index 4f807a577f25..d2e22f70d1f4 100644
--- a/xen/test/livepatch/xen_action_hooks_marker.c
+++ b/xen/test/livepatch/xen_action_hooks_marker.c
@@ -23,9 +23,10 @@ static int pre_apply_hook(livepatch_payload_t *payload)
for (i = 0; i < payload->nfuncs; i++)
{
-struct livepatch_func *func = >funcs[i];
+const struct livepatch_func *func = >funcs[i];
+struct livepatch_fstate *fstate = >fstate[i];
-BUG_ON(func->applied == LIVEPATCH_FUNC_APPLIED);
+BUG_ON(fstate->applied == LIVEPATCH_FUNC_APPLIED);
printk(KERN_DEBUG "%s: pre applied: %s\n", __func__, func->name);
}
@@ -42,9 +43,10 @@ static void post_apply_hook(livepatch_payload_t *payload)
for (i = 0; i < payload->nfuncs; i++)
{
-struct livepatch_func *func = >funcs[i];
+const struct livepatch_func *func = >funcs[i];
+struct livepatch_fstate *fstate = >fstate[i];
-BUG_ON(func->applied != LIVEPATCH_FUNC_APPLIED);
+BUG_ON(fstate->applied != LIVEPATCH_FUNC_APPLIED);
printk(KERN_DEBUG "%s: post applied: %s\n", __func__, func->name);
}
@@ -59,9 +61,10 @@ static int pre_revert_hook(livepatch_payload_t *payload)
for (i = 0; i < payload->nfuncs; i++)
{
-struct livepatch_func *func = >funcs[i];
+const struct livepatch_func *func = >funcs[i];
+struct livepatch_fstate *fstate = >fstate[i];
-BUG_ON(func->applied != LIVEPATCH_FUNC_APPLIED);
+BUG_ON(fstate->applied != LIVEPATCH_FUNC_APPLIED);
printk(KERN_DEBUG "%s: pre reverted: %s\n", __func__, func->name);
}
@@ -78,9 +81,10 @@ static void post_revert_hook(livepatch_payload_t *payload)
for (i = 0; i < payload->nfuncs; i++)
{
-struct livepatch_func *func = >funcs[i];
+const struct livepatch_func *func = >funcs[i];
+struct livepatch_fstate *fstate = >fstate[i];
-BUG_ON(func->applied == LIVEPATCH_FUNC_APPLIED);
+BUG_ON(fstate->applied == LIVEPATCH_FUNC_APPLIED);
printk(KERN_DEBUG "%s: post reverted: %s\n", __func__, func->name);
}
diff --git a/xen/test/livepatch/xen_action_hooks_noapply.c
b/xen/test/livepatch/xen_action_hooks_noapply.c
index 4c55c156a621..646a5fd2f002 100644
---
[xen-unstable-smoke test] 183899: regressions - all pass
flight 183899 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183899/
Regressions :-(
Tests which did not succeed and are blocking,
including tests which could not be run:
build-amd64 7 xen-build/dist-test fail REGR. vs. 183851
build-arm64-xsm 7 xen-build/dist-test fail REGR. vs. 183851
build-armhf 7 xen-build/dist-test fail REGR. vs. 183851
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 72d51813d631fe27d37736b7a55eeec08f246983
baseline version:
xen 80c153c48b255bae61948827241c26671207cf4e
Last test of basis 183851 2023-11-24 09:03:53 Z4 days
Failing since183871 2023-11-27 14:00:26 Z1 days6 attempts
Testing same since 183874 2023-11-27 19:00:27 Z0 days5 attempts
People who touched revisions under test:
Andrew Cooper
Federico Serafini
Frediano Ziglio
Jan Beulich
Maria Celeste Cesario
Maria Celeste Cesario
Roger Pau Monné
Simone Ballarin
Simone Ballarin
Tamas K Lengyel
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
Not pushing.
commit 72d51813d631fe27d37736b7a55eeec08f246983
Author: Jan Beulich
Date: Mon Nov 27 15:18:48 2023 +0100
x86: amend cpu_has_xen_{ibt,shstk}
... to evaluate to false at compile-time when the respective Kconfig
control is off, thus allowing the compiler to eliminate then-dead code.
Signed-off-by: Jan Beulich
Acked-by: Andrew Cooper
commit 17754972fa98bff2dfdec09b8094f54530bafcb8
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:56 2023 +0100
x86/atomic: address violations of MISRA C:2012 Rule 11.8
Edit casts that unnecessarily remove const qualifiers
to comply with Rule 11.8.
The type of the provided pointer may be const qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit fc63c0ebefe7e9d166b07971273c1de62428eb18
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:32 2023 +0100
AMD/IOMMU: address violations of MISRA C:2012 Rule 11.8
Drop an unnecessary cast discarding a const qualifier, to comply with
Rule 11.8. The type of the formal parameter ivhd_block is const
qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit fe26cb2dd20fa864deb05e4b278bc9993ba120e6
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:07 2023 +0100
x86/boot/reloc: address violations of MISRA C:2012 Rule 11.8
Add missing const qualifier in casting to comply with Rule 11.8.
Argument tag is typically const qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit 09c2fe438da1dfc83f70d921b52240bea576615f
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:16:43 2023 +0100
x86/platform_hypercall: address violations of MISRA C:2012 Rule 11.8
Add const qualifier in cast that unnecessarily removes it
to comply with Rule 11.8.
The variable info is declared with a const qualified type.
No functional change.
Signed-off-by: Maria Celeste
Re: [XEN PATCH 04/11] xen/dmi: address a violation of MISRA C:2012 Rule 8.2
On 24.11.2023 15:03, Federico Serafini wrote: > Add missing parameter name. No functional change. > > Signed-off-by: Federico Serafini Acked-by: Jan Beulich
Re: [XEN PATCH 03/11] xen/cpumask: address violations of MISRA C:2012 Rule 8.2
On 24.11.2023 15:03, Federico Serafini wrote: > Add missing parameter names. No functional change. > > Signed-off-by: Federico Serafini Acked-by: Jan Beulich irrespective of this being one of the examples where types alone are entirely sufficient and descriptive. Jan > --- a/xen/include/xen/cpumask.h > +++ b/xen/include/xen/cpumask.h > @@ -460,7 +460,9 @@ extern cpumask_t cpu_present_map; > > /* Copy to/from cpumap provided by control tools. */ > struct xenctl_bitmap; > -int cpumask_to_xenctl_bitmap(struct xenctl_bitmap *, const cpumask_t *); > -int xenctl_bitmap_to_cpumask(cpumask_var_t *, const struct xenctl_bitmap *); > +int cpumask_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_cpumap, > + const cpumask_t *cpumask); > +int xenctl_bitmap_to_cpumask(cpumask_var_t *cpumask, > + const struct xenctl_bitmap *xenctl_cpumap); > > #endif /* __XEN_CPUMASK_H */
Re: [XEN PATCH 02/11] xen/aclinux: address violations of MISRA C:2012 Rule 8.2
On 24.11.2023 15:03, Federico Serafini wrote: > Add missing parameter names. No functional change. > > Signed-off-by: Federico Serafini Acked-by: Jan Beulich Albeit as to the subject: There's no component named "aclinux" in Xen. I think this wants to be "acpi". Jan
Re: [XEN PATCH 01/11] xen/console: address violations of MISRA C:2012 Rule 8.2
On 24.11.2023 15:03, Federico Serafini wrote: > Add missing parameter names. No functional change. > > Signed-off-by: Federico Serafini Acked-by: Jan Beulich
Re: [PATCH v12 16/37] x86/ptrace: Add FRED additional information to the pt_regs structure
On November 28, 2023 12:51:22 AM PST, Borislav Petkov wrote:
>On Mon, Oct 02, 2023 at 11:24:37PM -0700, Xin Li wrote:
>> FRED defines additional information in the upper 48 bits of cs/ss
>> fields. Therefore add the information definitions into the pt_regs
>> structure.
>>
>> Specially introduce a new structure fred_ss to denote the FRED flags
>> above SS selector, which avoids FRED_SSX_ macros and makes the code
>> simpler and easier to read.
>>
>> Signed-off-by: H. Peter Anvin (Intel)
>
>You and hpa need to go through all the patches and figure out who's the
>author that's going to land in git.
>
>Because this and others have hpa's SOB first, suggesting he's the
>author. However, the mail doesn't start with
>
>From: H. Peter Anvin (Intel)
>
>and then git will make *you* the author.
>
>> Tested-by: Shan Kang
>> Signed-off-by: Thomas Gleixner
>> Signed-off-by: Xin Li
>
>...
>
>> union {
>> -u64 ssx;// The full 64-bit data slot containing SS
>> -u16 ss; // SS selector
>> +/* SS selector */
>> +u16 ss;
>> +/* The extended 64-bit data slot containing SS */
>> +u64 ssx;
>> +/* The FRED SS extension */
>> +struct fred_ss fred_ss;
>
>Aha, sanity about the right comments has come to your mind in this next
>patch. :-P
>
>Just do them right in the previous one.
>
>> /*
>> - * Top of stack on IDT systems.
>> + * Top of stack on IDT systems, while FRED systems have extra fields
>> + * defined above for storing exception related information, e.g. CR2 or
>> + * DR6.
>
>Btw, I really appreciate the good commenting - thanks for that!
>
For Xin, mainly:
Standard practice is:
1. For a patch with relatively small modifications, or where the changes are
mainly in comments or the patch message:
Keep the authorship, but put a description of what you have changed in brackets
with your username at the bottom of the description, immediately before
Signed-off-by:
[ xin: changed foo, bar, baz ]
2. For a patch with major rewrites:
Take authorship on the From: line, but have an Originally-by: tag (rather than
a Signed-off-by: by the original author):
Originally-by: Someone Else
3. For a patch which is fully or nearly fully your own work (a total rewrite,
or based on a concept idea rather than actual code), credit the original in the
patch comment:
Based on an idea by Someone Else (optional link to
lore.kernel.org).
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
On 28/11/2023 18:15, Andrew Cooper wrote:
>
>
> On 27/11/2023 2:41 pm, Michal Orzel wrote:
>> diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
>> index a3a80fa99eec..dd5ee0013648 100644
>> --- a/xen/common/ubsan/ubsan.c
>> +++ b/xen/common/ubsan/ubsan.c
>> @@ -174,6 +174,10 @@ static void ubsan_epilogue(unsigned long *flags)
>> "\n");
>> spin_unlock_irqrestore(_lock, *flags);
>> current->in_ubsan--;
>> +
>> +#ifdef CONFIG_UBSAN_FATAL
>> + panic("UBSAN failure detected\n");
>> +#endif
>
> if ( IS_ENABLED(CONFIG_UBSAN_FATAL) )
> panic("UBSAN failure detected\n");
>
> please. Happy to fix on commit.
Sounds good to me, thanks.
~Michal
Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 5.6
On 28.11.2023 18:09, Federico Serafini wrote: > --- a/docs/misra/deviations.rst > +++ b/docs/misra/deviations.rst > @@ -90,6 +90,22 @@ Deviations related to MISRA C:2012 Rules: > - __emulate_2op and __emulate_2op_nobyte > - read_debugreg and write_debugreg > > + * - R5.6 > + - The type ret_t is deliberately defined multiple times depending on the > + type of guest to service. > + - Tagged as `deliberate` for ECLAIR. > + > + * - R5.6 > + - Some files are not subject to respect MISRA rules at > + the moment, but, among these out-of-scope files, there are definitions > + of typedef names that are already defined within in-scope files and > + therefore, ECLAIR does report a violation since not all the files > + involved in the violation are excluded from the analysis. > + - Tagged as `deliberate` for ECLAIR. Such excluded files are: > + - xen/include/efi/.* > + - xen/arch/x86/include/asm/x86_64/efibind.h > + - xen/arch/arm/include/asm/arm64/efibind.h Could these two be generalized to xen/arch/*/include/asm/*/efibind.h? Jan
Re: [PATCH] cirrus-ci: update FreeBSD versions
On 28/11/2023 5:11 pm, Roger Pau Monne wrote: > FreeBSD 14.0 has already been released, so switch to the release version > image, > and introduce a FreeBSD 15.0 version to track current FreeBSD unstable > (development) branch. > > Sample output at: > > https://github.com/royger/xen/runs/19105278189 > > Signed-off-by: Roger Pau Monné Acked-by: Andrew Cooper
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
On 27/11/2023 2:41 pm, Michal Orzel wrote:
> diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
> index a3a80fa99eec..dd5ee0013648 100644
> --- a/xen/common/ubsan/ubsan.c
> +++ b/xen/common/ubsan/ubsan.c
> @@ -174,6 +174,10 @@ static void ubsan_epilogue(unsigned long *flags)
> "\n");
> spin_unlock_irqrestore(_lock, *flags);
> current->in_ubsan--;
> +
> +#ifdef CONFIG_UBSAN_FATAL
> + panic("UBSAN failure detected\n");
> +#endif
if ( IS_ENABLED(CONFIG_UBSAN_FATAL) )
panic("UBSAN failure detected\n");
please. Happy to fix on commit.
Reviewed-by: Andrew Cooper
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
On 28/11/2023 18:09, Julien Grall wrote: > > > On 28/11/2023 18:00, Michal Orzel wrote: >> Hi Julien, >> >> On 28/11/2023 17:14, Julien Grall wrote: >>> >>> >>> Hi Michal, >>> >>> On 27/11/2023 15:41, Michal Orzel wrote: Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt attention to undefined behavior issues, notably during CI test runs, is essential. When enabled, this option causes Xen to panic upon detecting UBSAN failure (as the last step in ubsan_epilogue()). >>> >>> I have mixed opinions on this patch. This would be a good one if we had >>> a Xen mostly free from UBSAN behavior. But this is not the case at least >>> on arm32. So we would end up to stop at the first error. IOW, we would >>> need to fix the first error before we can see the next one. >> Well, this patch introduces a config option disabled by default. > > I understood this is disabled by default... I am pointing out that I am > not convinced about the usefulness until we are at the stage where Xen > is normally not reporting any USBAN error. > >> If we end up enabling it for CI for reasons* stated by Andrew, then the >> natural way >> of handling such issues is to do the investigation locally. > > This will not always be possible. One example is when you are only able > to reproduce some of the USBAN errors on a specific HW. > >> Then, you are not forced >> to select this option and you can see all the UBSAN issues if you want. > > See above, I got that point. I am mostly concerned about the implication > in the CI right now. > >> >>> >>> So I feel it would be best if the gitlab CI jobs actually check for >>> USBAN in the logs and fail if there are any. With that, we can get the >>> full list of UBSAN issues on each job. >> Well, I prefer Andrew suggestion (both [1] and on IRC), hence this patch. >> >> *my plan was to first fix the UBSAN issues and then enable this option for >> CI. > > That would have been useful to describe your goal after "---". With that > in mind, then I suggest to revisit this patch once all the UBSAN issues > in a normal build are fixed. But this patch does not enable this option for CI automatically, right? Why are you so keen to push it back? Is it because you see no point in this option other than for the upstream CI loop? I find it useful on a day-to-day Xen runs, and I would for sure enable it by default in my config not to miss UBSAN failures. ~Michal
[PATCH] cirrus-ci: update FreeBSD versions
FreeBSD 14.0 has already been released, so switch to the release version image, and introduce a FreeBSD 15.0 version to track current FreeBSD unstable (development) branch. Sample output at: https://github.com/royger/xen/runs/19105278189 Signed-off-by: Roger Pau Monné --- .cirrus.yml | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 7e0beb200d7b..385618c394ea 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -29,5 +29,11 @@ task: task: name: 'FreeBSD 14' freebsd_instance: -image_family: freebsd-14-0-snap +image_family: freebsd-14-0 + << : *FREEBSD_TEMPLATE + +task: + name: 'FreeBSD 15' + freebsd_instance: +image_family: freebsd-15-0-snap << : *FREEBSD_TEMPLATE -- 2.43.0
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
On 28/11/2023 5:00 pm, Michal Orzel wrote: > Hi Julien, > > On 28/11/2023 17:14, Julien Grall wrote: >> >> Hi Michal, >> >> On 27/11/2023 15:41, Michal Orzel wrote: >>> Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt >>> attention to undefined behavior issues, notably during CI test runs, is >>> essential. When enabled, this option causes Xen to panic upon detecting >>> UBSAN failure (as the last step in ubsan_epilogue()). >> I have mixed opinions on this patch. This would be a good one if we had >> a Xen mostly free from UBSAN behavior. But this is not the case at least >> on arm32. So we would end up to stop at the first error. IOW, we would >> need to fix the first error before we can see the next one. > Well, this patch introduces a config option disabled by default. > If we end up enabling it for CI for reasons* stated by Andrew, then the > natural way > of handling such issues is to do the investigation locally. Then, you are not > forced > to select this option and you can see all the UBSAN issues if you want. > >> So I feel it would be best if the gitlab CI jobs actually check for >> USBAN in the logs and fail if there are any. With that, we can get the >> full list of UBSAN issues on each job. > Well, I prefer Andrew suggestion (both [1] and on IRC), hence this patch. > > *my plan was to first fix the UBSAN issues and then enable this option for CI. > > [1] > https://lore.kernel.org/xen-devel/7421ddfd-8947-4fe1-93a6-dc25a4aa8...@citrix.com/T/#t Searching logs is never reliable. Even within gitlab, we've had error upon error getting the console logs out reliably, and it's even worse on real hardware. This is an optional thing, but for people who want it, it's very highly desirable. ~Andrew
[XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 5.6
Update ECLAIR configuration to take into account the adopted files
and type "ret_t".
Update docs/misra/deviations.rst accordingly.
Signed-off-by: Federico Serafini
---
automation/eclair_analysis/ECLAIR/deviations.ecl | 12
docs/misra/deviations.rst| 16
2 files changed, 28 insertions(+)
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index c9e3a90801..3c6c7ce4a3 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -81,6 +81,18 @@ conform to the directive."
-config=MC3R1.R5.3,reports+={safe,
"any_area(any_loc(any_exp(macro(^read_debugreg$))&_exp(macro(^write_debugreg$"}
-doc_end
+-doc_begin="The type \"ret_t\" is deliberately defined multiple times,
+depending on the guest."
+-config=MC3R1.R5.6,reports+={deliberate,"any_area(any_loc(text(^.*ret_t.*$)))"}
+-doc_end
+
+-doc_begin="The following files are imported from the gnu-efi package."
+-file_tag+={adopted_r5_6,"^xen/include/efi/.*$"}
+-file_tag+={adopted_r5_6,"^xen/arch/x86/include/asm/x86_64/efibind\\.h$"}
+-file_tag+={adopted_r5_6,"^xen/arch/arm/include/asm/arm64/efibind\\.h$"}
+-config=MC3R1.R5.6,reports+={deliberate,"any_area(any_loc(file(adopted_r5_6)))"}
+-doc_end
+
#
# Series 7.
#
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 160513b997..8cec2c556c 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -90,6 +90,22 @@ Deviations related to MISRA C:2012 Rules:
- __emulate_2op and __emulate_2op_nobyte
- read_debugreg and write_debugreg
+ * - R5.6
+ - The type ret_t is deliberately defined multiple times depending on the
+ type of guest to service.
+ - Tagged as `deliberate` for ECLAIR.
+
+ * - R5.6
+ - Some files are not subject to respect MISRA rules at
+ the moment, but, among these out-of-scope files, there are definitions
+ of typedef names that are already defined within in-scope files and
+ therefore, ECLAIR does report a violation since not all the files
+ involved in the violation are excluded from the analysis.
+ - Tagged as `deliberate` for ECLAIR. Such excluded files are:
+ - xen/include/efi/.*
+ - xen/arch/x86/include/asm/x86_64/efibind.h
+ - xen/arch/arm/include/asm/arm64/efibind.h
+
* - R7.1
- It is safe to use certain octal constants the way they are defined
in specifications, manuals, and algorithm descriptions. Such places
--
2.34.1
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
On 28/11/2023 18:00, Michal Orzel wrote: Hi Julien, On 28/11/2023 17:14, Julien Grall wrote: Hi Michal, On 27/11/2023 15:41, Michal Orzel wrote: Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt attention to undefined behavior issues, notably during CI test runs, is essential. When enabled, this option causes Xen to panic upon detecting UBSAN failure (as the last step in ubsan_epilogue()). I have mixed opinions on this patch. This would be a good one if we had a Xen mostly free from UBSAN behavior. But this is not the case at least on arm32. So we would end up to stop at the first error. IOW, we would need to fix the first error before we can see the next one. Well, this patch introduces a config option disabled by default. I understood this is disabled by default... I am pointing out that I am not convinced about the usefulness until we are at the stage where Xen is normally not reporting any USBAN error. If we end up enabling it for CI for reasons* stated by Andrew, then the natural way of handling such issues is to do the investigation locally. This will not always be possible. One example is when you are only able to reproduce some of the USBAN errors on a specific HW. Then, you are not forced to select this option and you can see all the UBSAN issues if you want. See above, I got that point. I am mostly concerned about the implication in the CI right now. So I feel it would be best if the gitlab CI jobs actually check for USBAN in the logs and fail if there are any. With that, we can get the full list of UBSAN issues on each job. Well, I prefer Andrew suggestion (both [1] and on IRC), hence this patch. *my plan was to first fix the UBSAN issues and then enable this option for CI. That would have been useful to describe your goal after "---". With that in mind, then I suggest to revisit this patch once all the UBSAN issues in a normal build are fixed. Cheers, -- Julien Grall
[xen-unstable-smoke bisection] complete build-amd64
branch xen-unstable-smoke
xenbranch xen-unstable-smoke
job build-amd64
testid xen-build/dist-test
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
*** Found and reproduced problem changeset ***
Bug is in tree: xen git://xenbits.xen.org/xen.git
Bug introduced: 82182ad7b46e0f7a3856bb12c7a9bf2e2a4570bc
Bug not present: 46f2e2c3bcd5b17dae0fd1e45ed8619d6c047b55
Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/183905/
commit 82182ad7b46e0f7a3856bb12c7a9bf2e2a4570bc
Author: Roger Pau Monné
Date: Mon Nov 27 15:16:01 2023 +0100
livepatch: do not use .livepatch.funcs section to store internal state
Currently the livepatch logic inside of Xen will use fields of struct
livepatch_func in order to cache internal state of patched functions.
Note
this is a field that is part of the payload, and is loaded as an ELF
section
(.livepatch.funcs), taking into account the SHF_* flags in the section
header.
The flags for the .livepatch.funcs section, as set by
livepatch-build-tools,
are SHF_ALLOC, which leads to its contents (the array of livepatch_func
structures) being placed in read-only memory:
Section Headers:
[Nr] Name Type Address Offset
Size EntSize Flags Link Info Align
[...]
[ 4] .livepatch.funcs PROGBITS 0080
0068 A 0 0 8
This previously went unnoticed, as all writes to the fields of
livepatch_func
happen in the critical region that had WP disabled in CR0. After
8676092a0f16
however WP is no longer toggled in CR0 for patch application, and only the
hypervisor .text mappings are made write-accessible. That leads to the
following page fault when attempting to apply a livepatch:
[ Xen-4.19-unstable x86_64 debug=y Tainted: C]
CPU:4
RIP:e008:[]
common/livepatch.c#apply_payload+0x45/0x1e1
[...]
Xen call trace:
[] R common/livepatch.c#apply_payload+0x45/0x1e1
[] F check_for_livepatch_work+0x385/0xaa5
[] F arch/x86/domain.c#idle_loop+0x92/0xee
Pagetable walk from 82d040625079:
L4[0x105] = 8c6c9063
L3[0x141] = 8c6c6063
L2[0x003] = 00086a1e7063
L1[0x025] = 80086ca5d121
Panic on CPU 4:
FATAL PAGE FAULT
[error_code=0003]
Faulting linear address: 82d040625079
Fix this by moving the internal Xen function patching state out of
livepatch_func into an area not allocated as part of the ELF payload.
While
there also constify the array of livepatch_func structures in order to
prevent
further surprises.
Note there's still one field (old_addr) that gets set during livepatch
load. I
consider this fine since the field is read-only after load, and at the
point
the field gets set the underlying mapping hasn't been made read-only yet.
Fixes: 8676092a0f16 ('x86/livepatch: Fix livepatch application when CET
is active')
Signed-off-by: Roger Pau Monné
Reviewed-by: Ross Lagerwall
For bisection revision-tuple graph see:
http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable-smoke/build-amd64.xen-build--dist-test.html
Revision IDs in each graph node refer, respectively, to the Trees above.
Running cs-bisection-step
--graph-out=/home/logs/results/bisect/xen-unstable-smoke/build-amd64.xen-build--dist-test
--summary-out=tmp/183905.bisection-summary --basis-template=183851
--blessings=real,real-bisect,real-retry xen-unstable-smoke build-amd64
xen-build/dist-test
Searching for failure / basis pass:
183891 fail [host=godello1] / 183851 [host=himrod2] 183846 [host=himrod2]
183844 [host=himrod2] 183840 [host=albana0] 183826 [host=himrod0] 183817
[host=nobling1] 183814 [host=godello0] 183809 [host=nobling1] 183802
[host=godello0] 183798 [host=godello0] 183786 [host=himrod2] 183784
[host=himrod2] 183774 [host=himrod2] 183770 [host=nobling1] 183767
[host=himrod2] 183762 ok.
Failure / basis pass flights: 183891 / 183762
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git
Latest 3d273dd05e51e5a1ffba3d98c7437ee84e8f8764
0df9387c8983e1b1e72d8c574356f572342c03e6
72d51813d631fe27d37736b7a55eeec08f246983
Basis pass
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
Hi Julien, On 28/11/2023 17:14, Julien Grall wrote: > > > Hi Michal, > > On 27/11/2023 15:41, Michal Orzel wrote: >> Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt >> attention to undefined behavior issues, notably during CI test runs, is >> essential. When enabled, this option causes Xen to panic upon detecting >> UBSAN failure (as the last step in ubsan_epilogue()). > > I have mixed opinions on this patch. This would be a good one if we had > a Xen mostly free from UBSAN behavior. But this is not the case at least > on arm32. So we would end up to stop at the first error. IOW, we would > need to fix the first error before we can see the next one. Well, this patch introduces a config option disabled by default. If we end up enabling it for CI for reasons* stated by Andrew, then the natural way of handling such issues is to do the investigation locally. Then, you are not forced to select this option and you can see all the UBSAN issues if you want. > > So I feel it would be best if the gitlab CI jobs actually check for > USBAN in the logs and fail if there are any. With that, we can get the > full list of UBSAN issues on each job. Well, I prefer Andrew suggestion (both [1] and on IRC), hence this patch. *my plan was to first fix the UBSAN issues and then enable this option for CI. [1] https://lore.kernel.org/xen-devel/7421ddfd-8947-4fe1-93a6-dc25a4aa8...@citrix.com/T/#t ~Michal
Re: [RFC XEN PATCH v2 3/3] tools: Add new function to get gsi from irq
On 28.11.23 17:11, Roger Pau Monné wrote: On Tue, Nov 28, 2023 at 03:42:31PM +0100, Juergen Gross wrote: On 28.11.23 15:25, Roger Pau Monné wrote: On Fri, Nov 24, 2023 at 06:41:36PM +0800, Jiqian Chen wrote: In PVH dom0, it uses the linux local interrupt mechanism, when it allocs irq for a gsi, it is dynamic, and follow the principle of applying first, distributing first. And if you debug the kernel codes, you will find the irq number is alloced from small to large, but the applying gsi number is not, may gsi 38 comes before gsi 28, that causes the irq number is not equal with the gsi number. And when we passthrough a device, QEMU will use its gsi number to do mapping actions, see xen_pt_realize-> xc_physdev_map_pirq, but the gsi number is got from file /sys/bus/pci/devices/:xx:xx.x/irq in current code, so it will fail when mapping. And in current codes, there is no method to translate irq to gsi for userspace. I think it would be cleaner to just introduce a new sysfs node that contains the gsi if a device is using one (much like the irq sysfs node)? Such ioctl to translate from IRQ to GSI has nothing to do with Xen, so placing it in privcmd does seem quite strange to me. I understand that for passthrough we need the GSI, but such translation layer from IRQ to GSI is all Linux internal, and it would be much simpler to just expose the GSI in sysfs IMO. You are aware that we have a Xen specific variant of acpi_register_gsi()? It is the Xen event channel driver being responsible for the GSI<->IRQ mapping. I'm kind of lost, this translation function is specifically needed for PVH which doesn't use the Xen specific variant of acpi_register_gsi(), and hence the IRQ <-> GSI relation is whatever the Linux kernel does on native. I do understand that on a PV dom0 the proposed sysfs gsi node would match the irq node, but that doesn't seem like an issue to me. Note also that PVH doesn't use acpi_register_gsi_xen_hvm() because XENFEAT_hvm_pirqs feature is not exposed to PVH, so I expect it uses the x86 acpi_register_gsi_ioapic(). Oh, I wasn't aware of this. Sorry for the noise. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH] ubsan: Introduce CONFIG_UBSAN_FATAL to panic on UBSAN failure
Hi Michal,
On 27/11/2023 15:41, Michal Orzel wrote:
Introduce the CONFIG_UBSAN_FATAL option to cater to scenarios where prompt
attention to undefined behavior issues, notably during CI test runs, is
essential. When enabled, this option causes Xen to panic upon detecting
UBSAN failure (as the last step in ubsan_epilogue()).
I have mixed opinions on this patch. This would be a good one if we had
a Xen mostly free from UBSAN behavior. But this is not the case at least
on arm32. So we would end up to stop at the first error. IOW, we would
need to fix the first error before we can see the next one.
So I feel it would be best if the gitlab CI jobs actually check for
USBAN in the logs and fail if there are any. With that, we can get the
full list of UBSAN issues on each job.
Cheers,
Signed-off-by: Michal Orzel
---
xen/Kconfig.debug| 7 +++
xen/common/ubsan/ubsan.c | 4
2 files changed, 11 insertions(+)
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 94e818ee09b5..e19e9d48817c 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -107,6 +107,13 @@ config UBSAN
If unsure, say N here.
+config UBSAN_FATAL
+ bool "Panic on UBSAN failure"
+ depends on UBSAN
+ help
+ Enabling this option will cause Xen to panic when an undefined
behavior
+ is detected by UBSAN. If unsure, say N here.
+
config DEBUG_TRACE
bool "Debug trace support"
---help---
diff --git a/xen/common/ubsan/ubsan.c b/xen/common/ubsan/ubsan.c
index a3a80fa99eec..dd5ee0013648 100644
--- a/xen/common/ubsan/ubsan.c
+++ b/xen/common/ubsan/ubsan.c
@@ -174,6 +174,10 @@ static void ubsan_epilogue(unsigned long *flags)
"\n");
spin_unlock_irqrestore(_lock, *flags);
current->in_ubsan--;
+
+#ifdef CONFIG_UBSAN_FATAL
+ panic("UBSAN failure detected\n");
+#endif
}
static void handle_overflow(struct overflow_data *data, unsigned long lhs,
--
Julien Grall
Re: [RFC XEN PATCH v2 3/3] tools: Add new function to get gsi from irq
On Tue, Nov 28, 2023 at 03:42:31PM +0100, Juergen Gross wrote: > On 28.11.23 15:25, Roger Pau Monné wrote: > > On Fri, Nov 24, 2023 at 06:41:36PM +0800, Jiqian Chen wrote: > > > In PVH dom0, it uses the linux local interrupt mechanism, > > > when it allocs irq for a gsi, it is dynamic, and follow > > > the principle of applying first, distributing first. And > > > if you debug the kernel codes, you will find the irq > > > number is alloced from small to large, but the applying > > > gsi number is not, may gsi 38 comes before gsi 28, that > > > causes the irq number is not equal with the gsi number. > > > And when we passthrough a device, QEMU will use its gsi > > > number to do mapping actions, see xen_pt_realize-> > > > xc_physdev_map_pirq, but the gsi number is got from file > > > /sys/bus/pci/devices/:xx:xx.x/irq in current code, > > > so it will fail when mapping. > > > And in current codes, there is no method to translate > > > irq to gsi for userspace. > > > > I think it would be cleaner to just introduce a new sysfs node that > > contains the gsi if a device is using one (much like the irq sysfs > > node)? > > > > Such ioctl to translate from IRQ to GSI has nothing to do with Xen, so > > placing it in privcmd does seem quite strange to me. I understand > > that for passthrough we need the GSI, but such translation layer from > > IRQ to GSI is all Linux internal, and it would be much simpler to just > > expose the GSI in sysfs IMO. > > You are aware that we have a Xen specific variant of acpi_register_gsi()? > > It is the Xen event channel driver being responsible for the GSI<->IRQ > mapping. I'm kind of lost, this translation function is specifically needed for PVH which doesn't use the Xen specific variant of acpi_register_gsi(), and hence the IRQ <-> GSI relation is whatever the Linux kernel does on native. I do understand that on a PV dom0 the proposed sysfs gsi node would match the irq node, but that doesn't seem like an issue to me. Note also that PVH doesn't use acpi_register_gsi_xen_hvm() because XENFEAT_hvm_pirqs feature is not exposed to PVH, so I expect it uses the x86 acpi_register_gsi_ioapic(). Thanks, Roger.
Re: [PATCH v7 2/2] xen/vpci: header: filter PCI capabilities
On 11/17/23 06:44, Roger Pau Monné wrote:
> On Wed, Sep 13, 2023 at 10:35:47AM -0400, Stewart Hildebrand wrote:
>> Currently, Xen vPCI only supports virtualizing the MSI and MSI-X
>> capabilities.
>> Hide all other PCI capabilities (including extended capabilities) from domUs
>> for
>> now, even though there may be certain devices/drivers that depend on being
>> able
>> to discover certain capabilities.
>>
>> We parse the physical PCI capabilities linked list and add vPCI register
>> handlers for the next elements, inserting our own next value, thus
>> presenting a
>> modified linked list to the domU.
>>
>> Introduce helper functions vpci_hw_read8 and vpci_read_val. The vpci_read_val
>> helper function returns a fixed value, which may be used for RAZ registers,
>> or
>> registers whose value doesn't change.
>>
>> Introduce pci_find_next_cap_ttl() helper while adapting the logic from
>> pci_find_next_cap() to suit our needs, and implement the existing
>> pci_find_next_cap() in terms of the new helper.
>>
>> Signed-off-by: Stewart Hildebrand
>> Reviewed-by: Jan Beulich
>> ---
>> v6->v7:
>> * no change
>>
>> v5->v6:
>> * add register handlers before status register handler in init_bars()
>> * s/header->mask_cap_list/mask_cap_list/
>>
>> v4->v5:
>> * use more appropriate types, continued
>> * get rid of unnecessary hook function
>> * add Jan's R-b
>>
>> v3->v4:
>> * move mask_cap_list setting to this patch
>> * leave pci_find_next_cap signature alone
>> * use more appropriate types
>>
>> v2->v3:
>> * get rid of > 0 in loop condition
>> * implement pci_find_next_cap in terms of new pci_find_next_cap_ttl function
>> so
>> that hypothetical future callers wouldn't be required to pass
>> * change NULL to (void *)0 for RAZ value passed to vpci_read_val
>> * change type of ttl to unsigned int
>> * remember to mask off the low 2 bits of next in the initial loop iteration
>> * change return type of pci_find_next_cap and pci_find_next_cap_ttl
>> * avoid wrapping the PCI_STATUS_CAP_LIST condition by using ! instead of == 0
>>
>> v1->v2:
>> * change type of ttl to int
>> * use switch statement instead of if/else
>> * adapt existing pci_find_next_cap helper instead of rolling our own
>> * pass ttl as in/out
>> * "pass through" the lower 2 bits of the next pointer
>> * squash helper functions into this patch to avoid transient dead code
>> situation
>> * extended capabilities RAZ/WI
>> ---
>> xen/drivers/pci/pci.c | 26 +-
>> xen/drivers/vpci/header.c | 76 +++
>> xen/drivers/vpci/vpci.c | 12 +++
>> xen/include/xen/pci.h | 3 ++
>> xen/include/xen/vpci.h| 5 +++
>> 5 files changed, 113 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/drivers/pci/pci.c b/xen/drivers/pci/pci.c
>> index 3569ccb24e9e..8799d60c2156 100644
>> --- a/xen/drivers/pci/pci.c
>> +++ b/xen/drivers/pci/pci.c
>> @@ -39,31 +39,39 @@ unsigned int pci_find_cap_offset(pci_sbdf_t sbdf,
>> unsigned int cap)
>> return 0;
>> }
>>
>> -unsigned int pci_find_next_cap(pci_sbdf_t sbdf, unsigned int pos,
>> - unsigned int cap)
>> +unsigned int pci_find_next_cap_ttl(pci_sbdf_t sbdf, unsigned int pos,
>> + bool (*is_match)(unsigned int),
>> + unsigned int cap, unsigned int *ttl)
>
> Maybe this has been discussed in previous patch versions, but why
> pass a match function instead of expanding the cap parameter to
> be an array of capabilities to search for?
Hm. I don't think we did discuss it previously. It's simple enough to change to
an array, so I'll do this for v8.
>
> I find it kind of weird to be able to pass both a specific capability
> to match against and also a match function.
Having two ways to specify it was a way to retain compatibility with the
existing function pci_find_next_cap() without having to introduce another match
function. But I'll change it to an array now.
>
> What the expected behavior if the caller provides both a match
> function and a cap value?
>
>> {
>> -u8 id;
>> -int ttl = 48;
>> +unsigned int id;
>>
>> -while ( ttl-- )
>> +while ( (*ttl)-- )
>> {
>> pos = pci_conf_read8(sbdf, pos);
>> if ( pos < 0x40 )
>> break;
>>
>> -pos &= ~3;
>> -id = pci_conf_read8(sbdf, pos + PCI_CAP_LIST_ID);
>> +id = pci_conf_read8(sbdf, (pos & ~3) + PCI_CAP_LIST_ID);
>>
>> if ( id == 0xff )
>> break;
>> -if ( id == cap )
>> +if ( (is_match && is_match(id)) || (!is_match && id == cap) )
>> return pos;
>>
>> -pos += PCI_CAP_LIST_NEXT;
>> +pos = (pos & ~3) + PCI_CAP_LIST_NEXT;
>> }
>> +
>> return 0;
>> }
>>
>> +unsigned int pci_find_next_cap(pci_sbdf_t sbdf, unsigned int pos,
>> + unsigned int cap)
>> +{
>> +unsigned int ttl = 48;
>> +
>> +return
Re: [PATCH v2 09/29] tools/xenlogd: add 9pfs open request support
On Fri, Nov 10, 2023 at 12:16 PM Juergen Gross wrote: > > Add the open request of the 9pfs protocol. > > Signed-off-by: Juergen Gross Reviewed-by: Jason Andryuk
[ovmf test] 183895: all pass - PUSHED
flight 183895 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/183895/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 9eec96bd4fc53d7836b5606f2a8bbb10713cc8f5 baseline version: ovmf d451bba399687b4102459db5a447fc9abb8fdee1 Last test of basis 183890 2023-11-28 10:41:09 Z0 days Testing same since 183895 2023-11-28 13:41:02 Z0 days1 attempts People who touched revisions under test: Dov Murik Gerd Hoffmann Tom Lendacky jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git d451bba399..9eec96bd4f 9eec96bd4fc53d7836b5606f2a8bbb10713cc8f5 -> xen-tested-master
Re: [RFC XEN PATCH v2 2/3] x86/pvh: Open PHYSDEVOP_map_pirq for PVH dom0
On 24.11.2023 11:41, Jiqian Chen wrote:
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -74,6 +74,8 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
> {
> case PHYSDEVOP_map_pirq:
> case PHYSDEVOP_unmap_pirq:
> +if (is_hardware_domain(currd))
> +break;
> case PHYSDEVOP_eoi:
> case PHYSDEVOP_irq_status_query:
> case PHYSDEVOP_get_free_pirq:
If you wouldn't go the route suggested by Roger, I think you will need
to deny self-mapping requests here.
Also note that both here and in patch 1 you will want to adjust a number
of style violations.
Jan
Re: We are not able to virtualize FreeBSD using xen 4.17 on Arm 32 bit
On Tue, Nov 28, 2023 at 03:09:14PM +0100, Mario Marietto wrote: > For booting a FreeBSD kernel as a guest OS on XEN,should we install xen > 4.18 from source ? Please avoid top-posting. I don't think so, I'm not aware of the FreeBSD port requiring a specific version of Xen. I do think the work is limited to aarch64 however, so there's no support in sight for arm32 FreeBSD guests as far as I'm aware. Regards, Roger.
Re: [PATCH v2 08/29] tools/xenlogd: add 9pfs walk request support
On Fri, Nov 10, 2023 at 1:16 PM Juergen Gross wrote: > > Add the walk request of the 9pfs protocol. > > Signed-off-by: Juergen Gross Reviewed-by: Jason Andryuk 9P2000.u has support for symlinks which you're explicitly not supporting. A symlink would only be present if someone on the backend side created one. So there could be some corner case that is disallowed, but it is probably better to just disallow them. Regards, Jason
Re: [PATCH] .gitignore: generalize *.new
Hi Jan, On Tue, 2023-11-28 at 13:51 +0100, Jan Beulich wrote: > It's not only in xen/include/xen/ that we generate (intermediate) > *.new > files. > > Signed-off-by: Jan Beulich > --- > Really I don't think I can spot what *.new we create in that specific > directory. xen/include/ certainly has some. > > --- a/.gitignore > +++ b/.gitignore > @@ -17,6 +17,7 @@ > *.so.[0-9]* > *.bin > *.bak > +*.new > *.tmp > *.spot > *.spit > @@ -277,7 +278,6 @@ > xen/include/config/ > xen/include/generated/ > xen/include/public/public > -xen/include/xen/*.new > xen/include/xen/acm_policy.h > xen/include/xen/compile.h > xen/include/xen/hypercall-defs.h I am happy with these changes. Reviewed-by: Oleksii Kurochko ~ Oleksii
Re: [RFC XEN PATCH v2 3/3] tools: Add new function to get gsi from irq
On 28.11.23 15:25, Roger Pau Monné wrote: On Fri, Nov 24, 2023 at 06:41:36PM +0800, Jiqian Chen wrote: In PVH dom0, it uses the linux local interrupt mechanism, when it allocs irq for a gsi, it is dynamic, and follow the principle of applying first, distributing first. And if you debug the kernel codes, you will find the irq number is alloced from small to large, but the applying gsi number is not, may gsi 38 comes before gsi 28, that causes the irq number is not equal with the gsi number. And when we passthrough a device, QEMU will use its gsi number to do mapping actions, see xen_pt_realize-> xc_physdev_map_pirq, but the gsi number is got from file /sys/bus/pci/devices/:xx:xx.x/irq in current code, so it will fail when mapping. And in current codes, there is no method to translate irq to gsi for userspace. I think it would be cleaner to just introduce a new sysfs node that contains the gsi if a device is using one (much like the irq sysfs node)? Such ioctl to translate from IRQ to GSI has nothing to do with Xen, so placing it in privcmd does seem quite strange to me. I understand that for passthrough we need the GSI, but such translation layer from IRQ to GSI is all Linux internal, and it would be much simpler to just expose the GSI in sysfs IMO. You are aware that we have a Xen specific variant of acpi_register_gsi()? It is the Xen event channel driver being responsible for the GSI<->IRQ mapping. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [RFC XEN PATCH v2 3/3] tools: Add new function to get gsi from irq
On Fri, Nov 24, 2023 at 06:41:36PM +0800, Jiqian Chen wrote: > In PVH dom0, it uses the linux local interrupt mechanism, > when it allocs irq for a gsi, it is dynamic, and follow > the principle of applying first, distributing first. And > if you debug the kernel codes, you will find the irq > number is alloced from small to large, but the applying > gsi number is not, may gsi 38 comes before gsi 28, that > causes the irq number is not equal with the gsi number. > And when we passthrough a device, QEMU will use its gsi > number to do mapping actions, see xen_pt_realize-> > xc_physdev_map_pirq, but the gsi number is got from file > /sys/bus/pci/devices/:xx:xx.x/irq in current code, > so it will fail when mapping. > And in current codes, there is no method to translate > irq to gsi for userspace. I think it would be cleaner to just introduce a new sysfs node that contains the gsi if a device is using one (much like the irq sysfs node)? Such ioctl to translate from IRQ to GSI has nothing to do with Xen, so placing it in privcmd does seem quite strange to me. I understand that for passthrough we need the GSI, but such translation layer from IRQ to GSI is all Linux internal, and it would be much simpler to just expose the GSI in sysfs IMO. Thanks, Roger.
[xen-unstable-smoke test] 183891: regressions - all pass
flight 183891 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/183891/
Regressions :-(
Tests which did not succeed and are blocking,
including tests which could not be run:
build-amd64 7 xen-build/dist-test fail REGR. vs. 183851
build-arm64-xsm 7 xen-build/dist-test fail REGR. vs. 183851
build-armhf 7 xen-build/dist-test fail REGR. vs. 183851
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 72d51813d631fe27d37736b7a55eeec08f246983
baseline version:
xen 80c153c48b255bae61948827241c26671207cf4e
Last test of basis 183851 2023-11-24 09:03:53 Z4 days
Failing since183871 2023-11-27 14:00:26 Z1 days5 attempts
Testing same since 183874 2023-11-27 19:00:27 Z0 days4 attempts
People who touched revisions under test:
Andrew Cooper
Federico Serafini
Frediano Ziglio
Jan Beulich
Maria Celeste Cesario
Maria Celeste Cesario
Roger Pau Monné
Simone Ballarin
Simone Ballarin
Tamas K Lengyel
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
Not pushing.
commit 72d51813d631fe27d37736b7a55eeec08f246983
Author: Jan Beulich
Date: Mon Nov 27 15:18:48 2023 +0100
x86: amend cpu_has_xen_{ibt,shstk}
... to evaluate to false at compile-time when the respective Kconfig
control is off, thus allowing the compiler to eliminate then-dead code.
Signed-off-by: Jan Beulich
Acked-by: Andrew Cooper
commit 17754972fa98bff2dfdec09b8094f54530bafcb8
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:56 2023 +0100
x86/atomic: address violations of MISRA C:2012 Rule 11.8
Edit casts that unnecessarily remove const qualifiers
to comply with Rule 11.8.
The type of the provided pointer may be const qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit fc63c0ebefe7e9d166b07971273c1de62428eb18
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:32 2023 +0100
AMD/IOMMU: address violations of MISRA C:2012 Rule 11.8
Drop an unnecessary cast discarding a const qualifier, to comply with
Rule 11.8. The type of the formal parameter ivhd_block is const
qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit fe26cb2dd20fa864deb05e4b278bc9993ba120e6
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:17:07 2023 +0100
x86/boot/reloc: address violations of MISRA C:2012 Rule 11.8
Add missing const qualifier in casting to comply with Rule 11.8.
Argument tag is typically const qualified.
No functional change.
Signed-off-by: Maria Celeste Cesario
Signed-off-by: Simone Ballarin
Acked-by: Andrew Cooper
commit 09c2fe438da1dfc83f70d921b52240bea576615f
Author: Maria Celeste Cesario
Date: Mon Nov 27 15:16:43 2023 +0100
x86/platform_hypercall: address violations of MISRA C:2012 Rule 11.8
Add const qualifier in cast that unnecessarily removes it
to comply with Rule 11.8.
The variable info is declared with a const qualified type.
No functional change.
Signed-off-by: Maria Celeste
Re: [RFC XEN PATCH v2 2/3] x86/pvh: Open PHYSDEVOP_map_pirq for PVH dom0
On Fri, Nov 24, 2023 at 06:41:35PM +0800, Jiqian Chen wrote:
> If we run Xen with PVH dom0 and hvm domU, hvm will map a pirq for
> a passthrough device by using gsi, see xen_pt_realize->xc_physdev_map_pirq
> and pci_add_dm_done->xc_physdev_map_pirq. Then xc_physdev_map_pirq will
> call into Xen, but in hvm_physdev_op, PHYSDEVOP_map_pirq is not allowed
> because currd is PVH dom0 and PVH has no X86_EMU_USE_PIRQ flag, it will
> fail at has_pirq check.
>
> So, I think we may need to allow PHYSDEVOP_map_pirq when currd is dom0 (at
And PHYSDEVOP_unmap_pirq also?
> present dom0 is PVH).
IMO it would be better to implement a new set of DMOP hypercalls that
handle the setup of interrupts from physical devices that are assigned
to a guest. That should allow us to get rid of the awkward PHYSDEVOP
+ XEN_DOMCTL_{,un}bind_pt_irq hypercall interface, which currently
prevents QEMU from being hypervisor version agnostic (since the domctl
interface is not stable).
I understand this might be too much to ask for, but something to take
into account.
Thanks, Roger.
[libvirt test] 183880: tolerable all pass - PUSHED
flight 183880 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/183880/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 183861 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 183861 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 183861 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: libvirt 6f11304849f303cbc2d3896261eff9f91acae52d baseline version: libvirt d9a1fe8ac4ca7c503efcb0ed90742fe5d223ad03 Last test of basis 183861 2023-11-26 04:20:30 Z2 days Testing same since 183880 2023-11-28 04:18:52 Z0 days1 attempts People who touched revisions under test: Jiri Denemark Peter Krempa jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair pass test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-qcow2 pass test-arm64-arm64-libvirt-raw pass test-armhf-armhf-libvirt-raw pass test-amd64-i386-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in
Re: [EXTERNAL] [PATCH v3 2/5] xen: backends: don't overwrite XenStore nodes created by toolstack
On Tue, 2023-11-28 at 01:20 +, Volodymyr Babchuk wrote:
> Hi David,
>
> Thank you for the review
>
> David Woodhouse writes:
>
> > [[S/MIME Signed Part:Undecided]]
> > On Fri, 2023-11-24 at 23:24 +, Volodymyr Babchuk wrote:
> > > Xen PV devices in QEMU can be created in two ways: either by QEMU
> > > itself, if they were passed via command line, or by Xen toolstack. In
> > > the latter case, QEMU scans XenStore entries and configures devices
> > > accordingly.
> > >
> > > In the second case we don't want QEMU to write/delete front-end
> > > entries for two reasons: it might have no access to those entries if
> > > it is running in un-privileged domain and it is just incorrect to
> > > overwrite entries already provided by Xen toolstack, because
> > > toolstack
> > > manages those nodes. For example, it might read backend- or frontend-
> > > state to be sure that they are both disconnected and it is safe to
> > > destroy a domain.
> > >
> > > This patch checks presence of xendev->backend to check if Xen PV
> > > device was configured by Xen toolstack to decide if it should touch
> > > frontend entries in XenStore. Also, when we need to remove XenStore
> > > entries during device teardown only if they weren't created by Xen
> > > toolstack. If they were created by toolstack, then it is toolstack's
> > > job to do proper clean-up.
> > >
> > > Suggested-by: Paul Durrant
> > > Suggested-by: David Woodhouse
> > > Co-Authored-by: Oleksandr Tyshchenko
> > > Signed-off-by: Volodymyr Babchuk
> >
> > Reviewed-by: David Woodhouse
> >
> > ... albeit with a couple of suggestions...
> >
> > > diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> > > index bef8a3a621..b52abf 100644
> > > --- a/hw/char/xen_console.c
> > > +++ b/hw/char/xen_console.c
> > > @@ -450,7 +450,7 @@ static void xen_console_realize(XenDevice *xendev,
> > > Error **errp)
> > >
> > > trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
> > >
> > > - if (CHARDEV_IS_PTY(cs)) {
> > > + if (CHARDEV_IS_PTY(cs) && !xendev->backend) {
> > > /* Strip the leading 'pty:' */
> > > xen_device_frontend_printf(xendev, "tty", "%s", cs->filename +
> > > 4);
> > > }
> >
> >
> > It's kind of weird that that one is a frontend node at all; surely it
> > should have been a backend node?
>
> Yeah, AFAIK, console was the first PV driver, so it is a bit strange.
> As I see, this frontend entry is used by "xl console" tool to find PTY
> device to attach to. So yes, it should be in backend part of the
> xenstore. But I don't believe we can fix this right now.
Agreed.
> > But it is known only to QEMU once it
> > actually opens /dev/ptmx and creates a new pty. It can't be populated
> > by the toolstack in advance.
> >
> > So shouldn't the toolstack have made it writable by the driver domain?
>
> Maybe it can lead to a weird situation when user in Dom-0 tries to use
> "xl console" command to attach to a console that is absent in Dom-0,
> because "tty" entry points to PTY in the driver domain.
True, but that possibility exists the moment you have console provided
in a driver domain.
...
> > Perhaps here you should create the "mac" node if it doesn't exist (or
> > is that "if it doesn't match netdev->conf.macaddr"?) and just
> > gracefully accept failure too?
> >
>
> I am not sure that I got this right. conf.maccadr can be sent in two
> ways: via xen_net_device_create(), which will fail if toolstack didn't
> provided a MAC address, or via qemu_macaddr_default_if_unset(), which is
> the case for Xen emulation.
The latter happens with hotplug under Xen too, not just Xen emulation.
But the key point you make is that xen_net_device_create() will refuse
to drive a device which the toolstack created, if the "mac" node isn't
present. So creating it for ourselves based on 'if (!xendev->backend)'
as you did in your patch is reasonable enough. Thanks.
smime.p7s
Description: S/MIME cryptographic signature
Re: We are not able to virtualize FreeBSD using xen 4.17 on Arm 32 bit
For booting a FreeBSD kernel as a guest OS on XEN,should we install xen 4.18 from source ? On Tue, Nov 28, 2023 at 2:45 PM Roger Pau Monné wrote: > On Mon, Nov 27, 2023 at 03:04:30PM -0800, Elliott Mitchell wrote: > > BTW Roger Pau Monné, now that Xen 4.18 is out, take a look at the > > "royger" branch? > > I've pushed a bunch of those, there are still some, I've made comments > on the branch. > > I think there isn't much left after the swept I've done. > > If you can rebase and reply to the comments I will take a look at > what's remaining. > > Regards, Roger. > -- Mario.
Re: [RFC XEN PATCH v2 1/3] xen/vpci: Clear all vpci status of device
On Fri, Nov 24, 2023 at 06:41:34PM +0800, Jiqian Chen wrote:
> When a device has been reset on dom0 side, the vpci on Xen
> side won't get notification, so the cached state in vpci is
> all out of date compare with the real device state.
> To solve that problem, this patch add new hypercall to clear
> all vpci device state. And when reset device happens on dom0
> side, dom0 can call this hypercall to notify vpci.
>
> Signed-off-by: Jiqian Chen
> Signed-off-by: Huang Rui
> ---
> xen/arch/x86/hvm/hypercall.c | 1 +
> xen/drivers/passthrough/pci.c | 21 +
> xen/drivers/pci/physdev.c | 14 ++
> xen/drivers/vpci/vpci.c | 9 +
> xen/include/public/physdev.h | 2 ++
> xen/include/xen/pci.h | 1 +
> xen/include/xen/vpci.h| 6 ++
> 7 files changed, 54 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
> index eeb73e1aa5..6ad5b4d5f1 100644
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -84,6 +84,7 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
> case PHYSDEVOP_pci_mmcfg_reserved:
> case PHYSDEVOP_pci_device_add:
> case PHYSDEVOP_pci_device_remove:
> +case PHYSDEVOP_pci_device_state_reset:
> case PHYSDEVOP_dbgp_op:
> if ( !is_hardware_domain(currd) )
> return -ENOSYS;
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 04d00c7c37..f871715585 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -824,6 +824,27 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
> return ret;
> }
>
> +int pci_reset_device_state(u16 seg, u8 bus, u8 devfn)
You could use pci_sbdf_t here instead of 3 parameters.
I'm however unsure whether we really need this helper just to fetch
the pdev and call vpci_reset_device_state(), I think you could place
this logic directly in pci_physdev_op(). Unless there are plans to
use such logic outside of pci_physdev_op().
> +{
> +struct pci_dev *pdev;
> +int ret = -ENODEV;
Some XSM check should be introduced fro this operation, as none of the
existing ones is suitable. See xsm_resource_unplug_pci() for example.
xsm_resource_reset_state_pci() or some such I would assume.
(adding the XSM maintainer for feedback).
> +
> +pcidevs_lock();
> +
> +pdev = pci_get_pdev(NULL, PCI_SBDF(seg, bus, devfn));
> +if ( !pdev )
> +goto error;
> +
> +ret = vpci_reset_device_state(pdev);
> +if (ret)
> +printk(XENLOG_ERR "PCI reset device %pp state failed\n",
> >sbdf);
> +
> +error:
> +pcidevs_unlock();
> +
> +return ret;
> +}
> +
> /* Caller should hold the pcidevs_lock */
> static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
> uint8_t devfn)
> diff --git a/xen/drivers/pci/physdev.c b/xen/drivers/pci/physdev.c
> index 42db3e6d13..cfdb545654 100644
> --- a/xen/drivers/pci/physdev.c
> +++ b/xen/drivers/pci/physdev.c
> @@ -67,6 +67,20 @@ ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
> break;
> }
>
> +case PHYSDEVOP_pci_device_state_reset: {
> +struct physdev_pci_device dev;
> +
> +if ( !is_pci_passthrough_enabled() )
> +return -EOPNOTSUPP;
> +
> +ret = -EFAULT;
> +if ( copy_from_guest(, arg, 1) != 0 )
> +break;
> +
> +ret = pci_reset_device_state(dev.seg, dev.bus, dev.devfn);
> +break;
> +}
> +
> default:
> ret = -ENOSYS;
> break;
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index 3bec9a4153..e9c3eb1cd6 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -103,6 +103,15 @@ int vpci_add_handlers(struct pci_dev *pdev)
>
> return rc;
> }
> +
> +int vpci_reset_device_state(struct pci_dev *pdev)
> +{
> +ASSERT(pcidevs_locked());
> +
> +vpci_remove_device(pdev);
> +return vpci_add_handlers(pdev);
> +}
> +
> #endif /* __XEN__ */
>
> static int vpci_register_cmp(const struct vpci_register *r1,
> diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h
> index f0c0d4727c..4156948903 100644
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -305,6 +305,8 @@ struct physdev_pci_device {
> typedef struct physdev_pci_device physdev_pci_device_t;
> DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_t);
>
> +#define PHYSDEVOP_pci_device_state_reset 32
This needs some comment in order to explain the expected behaviour,
and might be better placed a bit up after PHYSDEVOP_release_msix.
Thanks, Roger.
Re: [XEN PATCH] x86/guest_walk: address violations of MISRA C:2012 Rule 8.3
On 28.11.2023 14:11, Federico Serafini wrote: > On 28/11/23 14:00, Jan Beulich wrote: >> On 28.11.2023 10:46, Federico Serafini wrote: >>> Uniform declaration and definition of guest_walk_tables() using >>> parameter name "pfec_walk": >>> this name highlights the connection with PFEC_* constants and it is >>> consistent with the use of the parameter within function body. >>> No functional change. >>> >>> Signed-off-by: Federico Serafini >> >> I'm curious what other x86 maintainers think. I for one don't like this, >> but not enough to object if others are happy. That said, there was earlier >> discussion (and perhaps even a patch), yet without a reference I don't >> think I can locate this among all the Misra bits and pieces. >> > > The last thread about guest_walk_tables(): > > https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg02055.html > > Do you have any suggestions on how to handle this? This simply will need solving - see my reply to Andrew sent a minute ago. Personally the best route I would see is to leave things as they are for now, hoping that Andrew's indicated code change will arrive in not too distant a future. Jan
Re: We are not able to virtualize FreeBSD using xen 4.17 on Arm 32 bit
On Mon, Nov 27, 2023 at 03:04:30PM -0800, Elliott Mitchell wrote: > BTW Roger Pau Monné, now that Xen 4.18 is out, take a look at the > "royger" branch? I've pushed a bunch of those, there are still some, I've made comments on the branch. I think there isn't much left after the swept I've done. If you can rebase and reply to the comments I will take a look at what's remaining. Regards, Roger.
Re: [XEN PATCH] x86/guest_walk: address violations of MISRA C:2012 Rule 8.3
On 28.11.2023 14:17, Andrew Cooper wrote: > On 28/11/2023 1:00 pm, Jan Beulich wrote: >> On 28.11.2023 10:46, Federico Serafini wrote: >>> Uniform declaration and definition of guest_walk_tables() using >>> parameter name "pfec_walk": >>> this name highlights the connection with PFEC_* constants and it is >>> consistent with the use of the parameter within function body. >>> No functional change. >>> >>> Signed-off-by: Federico Serafini >> I'm curious what other x86 maintainers think. I for one don't like this, >> but not enough to object if others are happy. That said, there was earlier >> discussion (and perhaps even a patch), yet without a reference I don't >> think I can locate this among all the Misra bits and pieces. > > I looked at this and wanted a bit of time to think. > > Sadly, this code is half way through some cleanup, which started before > speculation and will continue in my copious free time. > > It's wrong to be passing PFEC_* constants, and that's why I renamed pfec > -> walk the last time I was fixing security bugs here (indeed, passing > the wrong constant here *was* the security issue). I missed the > prototype while fixing the implementation. > > At some point, PFEC_* will no longer be passed in. > > Therefore I'd far rather this was a one-line change for the declaration > changing pfec -> walk. So that was what Federico originally had. Did you see my response at https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00122.html ? While I certainly agree with your plans (as far as I understand them), doing as you suggest would make it harder to spot what values are correct to pass to the function today. With a suitable new set of constants and perhaps even a proper typedef, such confusion would likely not arise. Jan
Re: [PATCH v3 04/13] xen/spinlock: introduce new type for recursive spinlocks
On 28.11.2023 14:16, Juergen Gross wrote:
> On 24.11.23 19:59, Alejandro Vallejo wrote:
>> On 20/11/2023 11:38, Juergen Gross wrote:
>>> --- a/xen/include/xen/spinlock.h
>>> +++ b/xen/include/xen/spinlock.h
>> > [snip]
>>> @@ -182,8 +191,10 @@ typedef struct spinlock {
>>> #endif
>>> } spinlock_t;
>>> +typedef spinlock_t rspinlock_t;
>>> #define spin_lock_init(l) (*(l) = (spinlock_t)SPIN_LOCK_UNLOCKED)
>>> +#define rspin_lock_init(l) (*(l) = (rspinlock_t)SPIN_LOCK_UNLOCKED)
>>
>> nit: Both variants of [r]spin_lock_init(l) could be inline functions
>> rather than macros.
>
> I was following the spin_lock_init() example, and I guess that is following
> the Linux kernel example.
>
> I don't mind either way, but maybe other maintainers have a preference?
While switching to inline functions would be nice, the new item should
imo be in line with the existing one (i.e. be a macro as long as that
other one also is a macro). And converting what has been there shouldn't
be a requirement for this series to land.
Jan
[ovmf test] 183890: all pass - PUSHED
flight 183890 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/183890/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf d451bba399687b4102459db5a447fc9abb8fdee1 baseline version: ovmf 0e9ce9146a6dc50a35488e3a4a7a2a4bbaf1eb1c Last test of basis 183879 2023-11-28 03:21:56 Z0 days Testing same since 183890 2023-11-28 10:41:09 Z0 days1 attempts People who touched revisions under test: Michael Kubacki 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 0e9ce9146a..d451bba399 d451bba399687b4102459db5a447fc9abb8fdee1 -> xen-tested-master
Re: [PATCH v4 13/14] xen: ifdef inclusion of in
On Tue, 2023-11-28 at 13:53 +0100, Jan Beulich wrote:
> On 28.11.2023 12:49, Oleksii wrote:
> > On Tue, 2023-11-28 at 10:58 +0100, Jan Beulich wrote:
> > > On 28.11.2023 10:28, Oleksii wrote:
> > > > On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote:
> > > > > On 27.11.2023 20:38, Oleksii wrote:
> > > > > > On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote:
> > > > > > > On 27.11.2023 15:13, Oleksii Kurochko wrote:
> > > > > > > > --- a/xen/arch/ppc/include/asm/grant_table.h
> > > > > > > > +++ /dev/null
> > > > > > > > @@ -1,5 +0,0 @@
> > > > > > > > -/* SPDX-License-Identifier: GPL-2.0-only */
> > > > > > > > -#ifndef __ASM_PPC_GRANT_TABLE_H__
> > > > > > > > -#define __ASM_PPC_GRANT_TABLE_H__
> > > > > > > > -
> > > > > > > > -#endif /* __ASM_PPC_GRANT_TABLE_H__ */
> > > > > > >
> > > > > > > Removing this header would be correct only if GRANT_TABLE
> > > > > > > had
> > > > > > > a
> > > > > > > "depends on
> > > > > > > !PPC", I'm afraid. Recall that the earlier randconfig
> > > > > > > adjustment
> > > > > > > in
> > > > > > > CI was
> > > > > > > actually requested to be undone, at which point what an
> > > > > > > arch's
> > > > > > > defconfig
> > > > > > > says isn't necessarily what a randconfig should use.
> > > > > > We can do depends on !PPC && !RISCV but shouldn't it be
> > > > > > enough
> > > > > > only
> > > > > > to
> > > > > > turn CONFIG_GRANT_TABLE off in defconfig and set
> > > > > > CONFIG_GRANT_TABLE=n
> > > > > > in EXTRA_XEN_CONFIG?
> > > > >
> > > > > That _might_ be sufficient for CI, but we shouldn't take CI
> > > > > as
> > > > > the
> > > > > only
> > > > > thing in the world. Personally I consider any kind of
> > > > > overriding
> > > > > for,
> > > > > in particular, randconfig at bets bogus. Whatever may not be
> > > > > selected
> > > > > (or must be selected) should be arranged for by Kconfig files
> > > > > themselves.
> > > > > That said, there may be _rare_ exceptions. But what PPC's and
> > > > > RISC-
> > > > > V's
> > > > > defconfig-s have right now is more than "rare" imo.
> > > > >
> > > > > > Some time ago I also tried to redefine "Config GRANT_TABLE"
> > > > > > in
> > > > > > arch-
> > > > > > specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it
> > > > > > works
> > > > > > for
> > > > > > me.
> > > > > > Could it be solution instead of "depends on..." ?
> > > > >
> > > > > Why would we want to re-define an setting? There wants to be
> > > > > one
> > > > > single
> > > > > place where a common option is defined. Or maybe I don't
> > > > > understand
> > > > > what you're suggesting ...
> > > > I just thought this change is temporary because grant_table.h
> > > > will
> > > > be
> > > > introduced later or earlier, and it will be needed to remove
> > > > "depends
> > > > on !PPC && !RISCV". And not to litter common KConfig with the
> > > > change
> > > > which will be removed, it will be better to redefine it in
> > > > arch-
> > > > specific Kconfig with default n.
> > >
> > > Right. But if it's indeed temporary, what's the point of this
> > > patch?
> > > The entire series is (supposed to be) to improve code structure
> > > for
> > > longer term purposes. If a non-generic grant_table.h is going to
> > > appear for PPC and RISC-V, I don't see why the present dummy
> > > would
> > > need removing. That's only useful if an arch can be expected to
> > > live
> > > with GRANT_TABLE=n even when otherwise feature-complete, and at
> > > that
> > > point modifying the Kconfig dependencies would (imo) be
> > > appropriate.
> > I agree. Let's proceed by adding the dependency in the KConfig
> > file.
> >
> > So which one option will be better:
> > 1. depends on !PPC && !RISCV
> > 2. depends on ARM || X86
>
> Agreeing and then making this suggestion contradicts one another.
> Unless
> the long-term plan is for PPC and RISC-V to not use grant tables.
On RISC-V side, I can run guests in dom0less and still don't use
grant_tables. And I am not sure when I'll start to implement it. Only
one thing I defined in grant_table.h is:
#define gnttab_dom0_frames()
\
min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext -
_stext))
But I think it can moved somewhere or dropped as it was defined because
of:
void __init create_dom0(void)
{
struct domain *dom0;
struct xen_domctl_createdomain dom0_cfg = {
.flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
.max_evtchn_port = -1,
.max_grant_frames = gnttab_dom0_frames(),
.max_maptrack_frames = -1,
.grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
};
...
Which was copied from Arm.
Taking into account that opt_max_grant_frames is 0 in case when
CONFIG_GRANT_TABLE=n then ".max_grant_frames =" will be 0 in case of
!CONFIG_GRANT_TABLE so for time being the macros gnttab_dom0_frames can
be dropped until CONFIG_GRANT_TABLE will be introduced.
But I am not aware of PPC plans of usage this config.
~ Oleksii
Re: [PATCH v3 06/13] xen/spinlock: add rspin_[un]lock_irq[save|restore]()
On 24.11.23 20:23, Alejandro Vallejo wrote:
On 20/11/2023 11:38, Juergen Gross wrote:
Instead of special casing rspin_lock_irqsave() and
rspin_unlock_irqrestore() for the console lock, add those functions
to spinlock handling and use them where needed.
Signed-off-by: Juergen Gross
---
V2:
- new patch
---
xen/arch/x86/traps.c | 14 --
xen/common/spinlock.c | 16
xen/drivers/char/console.c | 18 +-
xen/include/xen/console.h | 5 +++--
xen/include/xen/spinlock.h | 7 +++
5 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e1356f696a..f72769e79b 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -647,13 +647,15 @@ void show_stack_overflow(unsigned int cpu, const struct
cpu_user_regs *regs)
void show_execution_state(const struct cpu_user_regs *regs)
{
/* Prevent interleaving of output. */
- unsigned long flags = console_lock_recursive_irqsave();
+ unsigned long flags;
+
+ rspin_lock_irqsave(_lock, flags);
This feels a bit weird because rspin_lock_irqsave() being lowercase
hints that it's a either a function or behaves like one. For that it
should take instead.
I don't think so. This is common behavior with the Linux kernel, and I
think we should keep it. Especially as I believe rspin_lock_irqsave()
and spin_lock_irqsave() should have a similar interface. Or would you want
us to change more than 250 call sites of spin_lock_irqsave()?
show_registers(regs);
show_code(regs);
show_stack(regs);
- console_unlock_recursive_irqrestore(flags);
+ rspin_unlock_irqrestore(_lock, flags);
}
void cf_check show_execution_state_nonconst(struct cpu_user_regs *regs)
@@ -663,7 +665,7 @@ void cf_check show_execution_state_nonconst(struct
cpu_user_regs *regs)
void vcpu_show_execution_state(struct vcpu *v)
{
- unsigned long flags = 0;
+ unsigned long flags;
if ( test_bit(_VPF_down, >pause_flags) )
{
@@ -698,7 +700,7 @@ void vcpu_show_execution_state(struct vcpu *v)
#endif
/* Prevent interleaving of output. */
- flags = console_lock_recursive_irqsave();
+ rspin_lock_irqsave(_lock, flags);
vcpu_show_registers(v);
@@ -708,7 +710,7 @@ void vcpu_show_execution_state(struct vcpu *v)
* Stop interleaving prevention: The necessary P2M lookups involve
* locking, which has to occur with IRQs enabled.
*/
- console_unlock_recursive_irqrestore(flags);
+ rspin_unlock_irqrestore(_lock, flags);
show_hvm_stack(v, >arch.user_regs);
}
@@ -717,7 +719,7 @@ void vcpu_show_execution_state(struct vcpu *v)
if ( guest_kernel_mode(v, >arch.user_regs) )
show_guest_stack(v, >arch.user_regs);
- console_unlock_recursive_irqrestore(flags);
+ rspin_unlock_irqrestore(_lock, flags);
}
#ifdef CONFIG_HVM
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 26c667d3cc..17716fc4eb 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -475,6 +475,16 @@ void rspin_lock(rspinlock_t *lock)
lock->recurse_cnt++;
}
+unsigned long __rspin_lock_irqsave(rspinlock_t *lock)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ rspin_lock(lock);
+
+ return flags;
+}
+
void rspin_unlock(rspinlock_t *lock)
{
if ( likely(--lock->recurse_cnt == 0) )
@@ -484,6 +494,12 @@ void rspin_unlock(rspinlock_t *lock)
}
}
+void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags)
+{
+ rspin_unlock(lock);
+ local_irq_restore(flags);
+}
+
#ifdef CONFIG_DEBUG_LOCK_PROFILE
struct lock_profile_anc {
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 369b2f9077..cc743b67ec 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -120,7 +120,7 @@ static int __read_mostly sercon_handle = -1;
int8_t __read_mostly opt_console_xen; /* console=xen */
#endif
-static DEFINE_RSPINLOCK(console_lock);
+DEFINE_RSPINLOCK(console_lock);
/*
* To control the amount of printing, thresholds are added.
@@ -1158,22 +1158,6 @@ void console_end_log_everything(void)
atomic_dec(_everything);
}
-unsigned long console_lock_recursive_irqsave(void)
-{
- unsigned long flags;
-
- local_irq_save(flags);
- rspin_lock(_lock);
-
- return flags;
-}
-
-void console_unlock_recursive_irqrestore(unsigned long flags)
-{
- rspin_unlock(_lock);
- local_irq_restore(flags);
-}
-
void console_force_unlock(void)
{
watchdog_disable();
diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index ab5c30c0da..dff0096b27 100644
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -8,8 +8,11 @@
#define __CONSOLE_H__
#include
+#include
#include
+extern rspinlock_t console_lock;
+
struct xen_sysctl_readconsole;
long read_console_ring(struct xen_sysctl_readconsole *op);
@@ -20,8 +23,6 @@ void
Re: [PATCH v3 04/13] xen/spinlock: introduce new type for recursive spinlocks
On 24.11.23 19:59, Alejandro Vallejo wrote:
On 20/11/2023 11:38, Juergen Gross wrote:
Introduce a new type "rspinlock_t" to be used for recursive spinlocks.
For now it is only an alias of spinlock_t, so both types can still be
used for recursive spinlocks. This will be changed later, though.
Switch all recursive spinlocks to the new type.
Define the initializer helpers and use them where appropriate.
Signed-off-by: Juergen Gross
---
V2:
- carved out from V1 patch
---
xen/arch/x86/include/asm/mm.h | 2 +-
xen/arch/x86/mm/mm-locks.h | 2 +-
xen/common/domain.c | 4 ++--
xen/common/ioreq.c | 2 +-
xen/drivers/char/console.c | 4 ++--
xen/drivers/passthrough/pci.c | 2 +-
xen/include/xen/sched.h | 6 +++---
xen/include/xen/spinlock.h | 19 +++
8 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
index 05dfe35502..8a6e0c283f 100644
--- a/xen/arch/x86/include/asm/mm.h
+++ b/xen/arch/x86/include/asm/mm.h
@@ -596,7 +596,7 @@ unsigned long domain_get_maximum_gpfn(struct domain *d);
/* Definition of an mm lock: spinlock with extra fields for debugging */
typedef struct mm_lock {
- spinlock_t lock;
+ rspinlock_t lock;
Considering it's rspinlock_t rather than spinlock_recursive, do we want
to change also mm_lock_recursive() and paging_lock_recursive() into
mm_rlock() and paging_rlock() ?
I don't think this should be part of this series.
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index bbe1472571..19561d5e61 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
> [snip]
@@ -182,8 +191,10 @@ typedef struct spinlock {
#endif
} spinlock_t;
+typedef spinlock_t rspinlock_t;
#define spin_lock_init(l) (*(l) = (spinlock_t)SPIN_LOCK_UNLOCKED)
+#define rspin_lock_init(l) (*(l) = (rspinlock_t)SPIN_LOCK_UNLOCKED)
nit: Both variants of [r]spin_lock_init(l) could be inline functions
rather than macros.
I was following the spin_lock_init() example, and I guess that is following
the Linux kernel example.
I don't mind either way, but maybe other maintainers have a preference?
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
Re: [XEN PATCH] x86/guest_walk: address violations of MISRA C:2012 Rule 8.3
On 28/11/2023 1:00 pm, Jan Beulich wrote: > On 28.11.2023 10:46, Federico Serafini wrote: >> Uniform declaration and definition of guest_walk_tables() using >> parameter name "pfec_walk": >> this name highlights the connection with PFEC_* constants and it is >> consistent with the use of the parameter within function body. >> No functional change. >> >> Signed-off-by: Federico Serafini > I'm curious what other x86 maintainers think. I for one don't like this, > but not enough to object if others are happy. That said, there was earlier > discussion (and perhaps even a patch), yet without a reference I don't > think I can locate this among all the Misra bits and pieces. I looked at this and wanted a bit of time to think. Sadly, this code is half way through some cleanup, which started before speculation and will continue in my copious free time. It's wrong to be passing PFEC_* constants, and that's why I renamed pfec -> walk the last time I was fixing security bugs here (indeed, passing the wrong constant here *was* the security issue). I missed the prototype while fixing the implementation. At some point, PFEC_* will no longer be passed in. Therefore I'd far rather this was a one-line change for the declaration changing pfec -> walk. As it stands, you're effectively reverting a correction I made. Thanks, ~Andrew
Re: [XEN PATCH] x86/guest_walk: address violations of MISRA C:2012 Rule 8.3
On 28/11/23 14:00, Jan Beulich wrote: On 28.11.2023 10:46, Federico Serafini wrote: Uniform declaration and definition of guest_walk_tables() using parameter name "pfec_walk": this name highlights the connection with PFEC_* constants and it is consistent with the use of the parameter within function body. No functional change. Signed-off-by: Federico Serafini I'm curious what other x86 maintainers think. I for one don't like this, but not enough to object if others are happy. That said, there was earlier discussion (and perhaps even a patch), yet without a reference I don't think I can locate this among all the Misra bits and pieces. The last thread about guest_walk_tables(): https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg02055.html Do you have any suggestions on how to handle this? -- Federico Serafini, M.Sc. Software Engineer, BUGSENG (http://bugseng.com)
Re: [PATCH v3 02/13] xen/spinlock: reduce lock profile ifdefs
On 24.11.23 18:59, Alejandro Vallejo wrote: Hi, On 20/11/2023 11:38, Juergen Gross wrote:> With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs can be dropped from spinlock.c. Signed-off-by: Juergen Gross --- V2: - new patch V3: - add variable name to macros parameter (Jan Beulich) --- xen/common/spinlock.c | 49 +++ 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c index d7194e518c..ce18fbdd8a 100644 --- a/xen/common/spinlock.c +++ b/xen/common/spinlock.c @@ -267,25 +267,28 @@ void spin_debug_disable(void) lock->profile->time_hold += NOW() - lock->profile->time_locked; \ lock->profile->lock_cnt++; \ } -#define LOCK_PROFILE_VAR s_time_t block = 0 -#define LOCK_PROFILE_BLOCK block = block ? : NOW(); -#define LOCK_PROFILE_GOT \ +#define LOCK_PROFILE_VAR(var, val) s_time_t var = (val) +#define LOCK_PROFILE_BLOCK(var ) var = var ? : NOW() nit: spaces before the closing parenthesis Yeah, seen that later. Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [XEN PATCH] x86/guest_walk: address violations of MISRA C:2012 Rule 8.3
On 28.11.2023 10:46, Federico Serafini wrote:
> Uniform declaration and definition of guest_walk_tables() using
> parameter name "pfec_walk":
> this name highlights the connection with PFEC_* constants and it is
> consistent with the use of the parameter within function body.
> No functional change.
>
> Signed-off-by: Federico Serafini
I'm curious what other x86 maintainers think. I for one don't like this,
but not enough to object if others are happy. That said, there was earlier
discussion (and perhaps even a patch), yet without a reference I don't
think I can locate this among all the Misra bits and pieces.
Jan
> --- a/xen/arch/x86/include/asm/guest_pt.h
> +++ b/xen/arch/x86/include/asm/guest_pt.h
> @@ -422,7 +422,7 @@ static inline unsigned int guest_walk_to_page_order(const
> walk_t *gw)
>
> bool
> guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m,
> - unsigned long va, walk_t *gw, uint32_t pfec,
> + unsigned long va, walk_t *gw, uint32_t pfec_walk,
>gfn_t top_gfn, mfn_t top_mfn, void *top_map);
>
> /* Pretty-print the contents of a guest-walk */
> --- a/xen/arch/x86/mm/guest_walk.c
> +++ b/xen/arch/x86/mm/guest_walk.c
> @@ -69,7 +69,7 @@ static bool set_ad_bits(guest_intpte_t *guest_p,
> guest_intpte_t *walk_p,
> */
> bool
> guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m,
> - unsigned long va, walk_t *gw, uint32_t walk,
> + unsigned long va, walk_t *gw, uint32_t pfec_walk,
>gfn_t top_gfn, mfn_t top_mfn, void *top_map)
> {
> struct domain *d = v->domain;
> @@ -100,16 +100,17 @@ guest_walk_tables(const struct vcpu *v, struct
> p2m_domain *p2m,
> * inputs to a guest walk, but a whole load of code currently passes in
> * other PFEC_ constants.
> */
> -walk &= (PFEC_implicit | PFEC_insn_fetch | PFEC_user_mode |
> PFEC_write_access);
> +pfec_walk &= (PFEC_implicit | PFEC_insn_fetch | PFEC_user_mode |
> + PFEC_write_access);
>
> /* Only implicit supervisor data accesses exist. */
> -ASSERT(!(walk & PFEC_implicit) ||
> - !(walk & (PFEC_insn_fetch | PFEC_user_mode)));
> +ASSERT(!(pfec_walk & PFEC_implicit) ||
> + !(pfec_walk & (PFEC_insn_fetch | PFEC_user_mode)));
>
> perfc_incr(guest_walk);
> memset(gw, 0, sizeof(*gw));
> gw->va = va;
> -gw->pfec = walk & (PFEC_user_mode | PFEC_write_access);
> +gw->pfec = pfec_walk & (PFEC_user_mode | PFEC_write_access);
>
> /*
> * PFEC_insn_fetch is only reported if NX or SMEP are enabled. Hardware
> @@ -117,7 +118,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain
> *p2m,
> * rights.
> */
> if ( guest_nx_enabled(v) || guest_smep_enabled(v) )
> -gw->pfec |= (walk & PFEC_insn_fetch);
> +gw->pfec |= (pfec_walk & PFEC_insn_fetch);
>
> #if GUEST_PAGING_LEVELS >= 3 /* PAE or 64... */
> #if GUEST_PAGING_LEVELS >= 4 /* 64-bit only... */
> @@ -399,7 +400,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain
> *p2m,
> * N.B. In the case that the walk ended with a superpage, the fabricated
> * gw->l1e contains the appropriate leaf pkey.
> */
> -if ( !(walk & PFEC_insn_fetch) &&
> +if ( !(pfec_walk & PFEC_insn_fetch) &&
> ((ar & _PAGE_USER) ? guest_pku_enabled(v)
> : guest_pks_enabled(v)) )
> {
> @@ -408,8 +409,8 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain
> *p2m,
> unsigned int pk_ar = (pkr >> (pkey * PKEY_WIDTH)) & (PKEY_AD |
> PKEY_WD);
>
> if ( (pk_ar & PKEY_AD) ||
> - ((walk & PFEC_write_access) && (pk_ar & PKEY_WD) &&
> - ((walk & PFEC_user_mode) || guest_wp_enabled(v))) )
> + ((pfec_walk & PFEC_write_access) && (pk_ar & PKEY_WD) &&
> + ((pfec_walk & PFEC_user_mode) || guest_wp_enabled(v))) )
> {
> gw->pfec |= PFEC_prot_key;
> goto out;
> @@ -417,17 +418,17 @@ guest_walk_tables(const struct vcpu *v, struct
> p2m_domain *p2m,
> }
> #endif
>
> -if ( (walk & PFEC_insn_fetch) && (ar & _PAGE_NX_BIT) )
> +if ( (pfec_walk & PFEC_insn_fetch) && (ar & _PAGE_NX_BIT) )
> /* Requested an instruction fetch and found NX? Fail. */
> goto out;
>
> -if ( walk & PFEC_user_mode ) /* Requested a user acess. */
> +if ( pfec_walk & PFEC_user_mode ) /* Requested a user acess. */
> {
> if ( !(ar & _PAGE_USER) )
> /* Got a supervisor walk? Unconditional fail. */
> goto out;
>
> -if ( (walk & PFEC_write_access) && !(ar & _PAGE_RW) )
> +if ( (pfec_walk & PFEC_write_access) && !(ar & _PAGE_RW) )
> /* Requested a write and only got a read? Fail. */
> goto out;
> }
> @@ -435,18 +436,18 @@ guest_walk_tables(const struct vcpu *v, struct
> p2m_domain
Re: [PATCH v4 13/14] xen: ifdef inclusion of in
On 28.11.2023 12:49, Oleksii wrote: > On Tue, 2023-11-28 at 10:58 +0100, Jan Beulich wrote: >> On 28.11.2023 10:28, Oleksii wrote: >>> On Tue, 2023-11-28 at 08:57 +0100, Jan Beulich wrote: On 27.11.2023 20:38, Oleksii wrote: > On Mon, 2023-11-27 at 15:41 +0100, Jan Beulich wrote: >> On 27.11.2023 15:13, Oleksii Kurochko wrote: >>> --- a/xen/arch/ppc/include/asm/grant_table.h >>> +++ /dev/null >>> @@ -1,5 +0,0 @@ >>> -/* SPDX-License-Identifier: GPL-2.0-only */ >>> -#ifndef __ASM_PPC_GRANT_TABLE_H__ >>> -#define __ASM_PPC_GRANT_TABLE_H__ >>> - >>> -#endif /* __ASM_PPC_GRANT_TABLE_H__ */ >> >> Removing this header would be correct only if GRANT_TABLE had >> a >> "depends on >> !PPC", I'm afraid. Recall that the earlier randconfig >> adjustment >> in >> CI was >> actually requested to be undone, at which point what an >> arch's >> defconfig >> says isn't necessarily what a randconfig should use. > We can do depends on !PPC && !RISCV but shouldn't it be enough > only > to > turn CONFIG_GRANT_TABLE off in defconfig and set > CONFIG_GRANT_TABLE=n > in EXTRA_XEN_CONFIG? That _might_ be sufficient for CI, but we shouldn't take CI as the only thing in the world. Personally I consider any kind of overriding for, in particular, randconfig at bets bogus. Whatever may not be selected (or must be selected) should be arranged for by Kconfig files themselves. That said, there may be _rare_ exceptions. But what PPC's and RISC- V's defconfig-s have right now is more than "rare" imo. > Some time ago I also tried to redefine "Config GRANT_TABLE" in > arch- > specific Kconfig + defconfig + EXTRA_XEN_CONFIG and it works > for > me. > Could it be solution instead of "depends on..." ? Why would we want to re-define an setting? There wants to be one single place where a common option is defined. Or maybe I don't understand what you're suggesting ... >>> I just thought this change is temporary because grant_table.h will >>> be >>> introduced later or earlier, and it will be needed to remove >>> "depends >>> on !PPC && !RISCV". And not to litter common KConfig with the >>> change >>> which will be removed, it will be better to redefine it in arch- >>> specific Kconfig with default n. >> >> Right. But if it's indeed temporary, what's the point of this patch? >> The entire series is (supposed to be) to improve code structure for >> longer term purposes. If a non-generic grant_table.h is going to >> appear for PPC and RISC-V, I don't see why the present dummy would >> need removing. That's only useful if an arch can be expected to live >> with GRANT_TABLE=n even when otherwise feature-complete, and at that >> point modifying the Kconfig dependencies would (imo) be appropriate. > I agree. Let's proceed by adding the dependency in the KConfig file. > > So which one option will be better: > 1. depends on !PPC && !RISCV > 2. depends on ARM || X86 Agreeing and then making this suggestion contradicts one another. Unless the long-term plan is for PPC and RISC-V to not use grant tables. Jan
[PATCH] .gitignore: generalize *.new
It's not only in xen/include/xen/ that we generate (intermediate) *.new files. Signed-off-by: Jan Beulich --- Really I don't think I can spot what *.new we create in that specific directory. xen/include/ certainly has some. --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ *.so.[0-9]* *.bin *.bak +*.new *.tmp *.spot *.spit @@ -277,7 +278,6 @@ xen/include/config/ xen/include/generated/ xen/include/public/public -xen/include/xen/*.new xen/include/xen/acm_policy.h xen/include/xen/compile.h xen/include/xen/hypercall-defs.h
Re: [PATCH v6 0/5] Fine granular configuration
> On 28 Nov 2023, at 12:38, Michal Orzel wrote: > > Hi Luca, > > On 28/11/2023 11:36, Luca Fancellu wrote: >> >> >>> On 24 Nov 2023, at 09:59, Luca Fancellu wrote: >>> >>> + CC Maintainers >>> On 24 Nov 2023, at 09:48, Luca Fancellu wrote: This serie aims to add more modularity to some feature that can be excluded without issues from the build. The first patch is already reviewed. v2 update: So I've tried to see how to put the dom0less code in the common code, but the amount of modifications are not trivial, even putting only the common part and protecting them with ARM, leaving the ARM specific stuff under arch/ like gic etc... will leave a status that is not very nice, so I've decided for now to keep everything on the arm architecture so that who is going to work on unifying the code in common can just take from there and do the proper rework. Luca Fancellu (5): arm/gicv2: make GICv2 driver and vGICv2 optional xen/arm: Add asm/domain.h include to kernel.h arm/dom0less: put dom0less feature code in a separate module xen/arm: Move static memory build code in separate modules arm/dom0less: introduce Kconfig for dom0less feature xen/arch/arm/Kconfig | 27 + xen/arch/arm/Makefile |7 +- xen/arch/arm/arm32/mmu/mm.c |1 + xen/arch/arm/arm64/mmu/mm.c |1 + xen/arch/arm/bootfdt.c| 161 +- xen/arch/arm/dom0less-build.c | 1018 ++ xen/arch/arm/domain_build.c | 3591 ++--- xen/arch/arm/efi/efi-boot.h |4 + xen/arch/arm/gic-v3.c |4 + xen/arch/arm/include/asm/dom0less-build.h | 30 + xen/arch/arm/include/asm/domain_build.h | 34 + xen/arch/arm/include/asm/kernel.h |1 + xen/arch/arm/include/asm/setup.h |2 - xen/arch/arm/include/asm/static-memory.h | 45 + xen/arch/arm/include/asm/static-shmem.h | 66 + xen/arch/arm/setup.c | 57 +- xen/arch/arm/static-memory.c | 287 ++ xen/arch/arm/static-shmem.c | 547 xen/arch/arm/vgic.c |2 + xen/arch/arm/vgic/Makefile|4 +- xen/common/Kconfig|2 +- 21 files changed, 3058 insertions(+), 2833 deletions(-) create mode 100644 xen/arch/arm/dom0less-build.c create mode 100644 xen/arch/arm/include/asm/dom0less-build.h create mode 100644 xen/arch/arm/include/asm/static-memory.h create mode 100644 xen/arch/arm/include/asm/static-shmem.h create mode 100644 xen/arch/arm/static-memory.c create mode 100644 xen/arch/arm/static-shmem.c >>> >>> I sent this serie forgetting about adding the maintainers, CC them >>> now. >> >> Hi all, >> >> I think all the patches here are Ack-ed by a maintainer, is there any issue >> to >> merge them? > We discussed this on Matrix and Julien will take care of committing this week. Ok understood, thanks > > ~Michal >
