Re: [PATCH 2/8] x86/paging: fold most HAP and shadow final teardown
On 21.12.2022 18:16, Andrew Cooper wrote:
> On 21/12/2022 1:25 pm, Jan Beulich wrote:
>> --- a/xen/arch/x86/mm/paging.c
>> +++ b/xen/arch/x86/mm/paging.c
>> @@ -842,10 +842,46 @@ int paging_teardown(struct domain *d)
>> /* Call once all of the references to the domain have gone away */
>> void paging_final_teardown(struct domain *d)
>> {
>> -if ( hap_enabled(d) )
>> +bool hap = hap_enabled(d);
>> +
>> +PAGING_PRINTK("%pd final teardown starts. Pages total = %u, free = %u,
>> p2m = %u\n",
>
> PAGING_PRINTK() already includes __func__, so just "%pd start: total %u,
> free %u, p2m %u\n" which is shorter.
Hmm, yes, can do.
>> + d, d->arch.paging.total_pages,
>> + d->arch.paging.free_pages, d->arch.paging.p2m_pages);
>> +
>> +if ( hap )
>> hap_final_teardown(d);
>> +
>> +/*
>> + * Double-check that the domain didn't have any paging memory.
>> + * It is possible for a domain that never got domain_kill()ed
>> + * to get here with its paging allocation intact.
>
> I know you're mostly just moving this comment, but it's misleading.
>
> This path is used for the domain_create() error path, and there will be
> a nonzero allocation for HVM guests.
>
> I think we do want to rework this eventually - we will simplify things
> massively by splitting the things can can only happen for a domain which
> has run into relinquish_resources.
>
> At a minimum, I'd suggest dropping the first sentence. "double check"
> implies it's an extraordinary case, which isn't warranted here IMO.
Instead of dropping I think I would prefer to make it start "Make sure
...".
>> + */
>> +if ( d->arch.paging.total_pages )
>> +{
>> +if ( hap )
>> +hap_teardown(d, NULL);
>> +else
>> +shadow_teardown(d, NULL);
>> +}
>> +
>> +/* It is now safe to pull down the p2m map. */
>> +p2m_teardown(p2m_get_hostp2m(d), true, NULL);
>> +
>> +/* Free any paging memory that the p2m teardown released. */
>
> I don't think this isn't true any more. 410 also made HAP/shadow free
> pages fully for a dying domain, so p2m_teardown() at this point won't
> add to the free memory pool.
>
> I think the subsequent *_set_allocation() can be dropped, and the
> assertions left.
I agree, but if anything this will want to be a separate patch then.
Jan
[xen-4.13-testing test] 175440: FAIL
flight 175440 xen-4.13-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/175440/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-livepatchbroken in 175426 test-amd64-i386-xl-shadowbroken in 175426 test-amd64-i386-xl-xsm broken in 175426 test-amd64-i386-xl-qemut-debianhvm-i386-xsm broken in 175426 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm broken in 175426 Tests which are failing intermittently (not blocking): test-amd64-i386-xl-xsm 5 host-install(5) broken in 175426 pass in 175440 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 5 host-install(5) broken in 175426 pass in 175440 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 5 host-install(5) broken in 175426 pass in 175440 test-amd64-i386-livepatch5 host-install(5) broken in 175426 pass in 175440 test-amd64-i386-xl-shadow5 host-install(5) broken in 175426 pass in 175440 test-amd64-amd64-xl-shadow 18 guest-localmigrate fail in 175426 pass in 175440 test-amd64-i386-xl-qemuu-debianhvm-amd64 18 guest-localmigrate/x10 fail in 175426 pass in 175440 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail pass in 175426 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174675 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174675 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 174675 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174675 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 174675 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 174675 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174675 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174675 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174675 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174675 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174675 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 174675 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-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-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-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-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfai
[ovmf test] 175448: all pass - PUSHED
flight 175448 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/175448/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 129404f6e4395008ac0045e7e627edbba2a1e064 baseline version: ovmf 62031335bdbacc68253d43481477b9a468e0644e Last test of basis 175444 2022-12-21 16:12:30 Z0 days Testing same since 175448 2022-12-22 01:42:28 Z0 days1 attempts People who touched revisions under test: Gerd Hoffmann Liming Gao 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 62031335bd..129404f6e4 129404f6e4395008ac0045e7e627edbba2a1e064 -> xen-tested-master
[linux-linus test] 175439: regressions - trouble: broken/fail/pass
flight 175439 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/175439/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-shadow broken test-amd64-amd64-xl-qemuu-win7-amd64 broken test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm broken test-amd64-amd64-xl-qemut-debianhvm-i386-xsmbroken test-amd64-amd64-xl-qemut-debianhvm-amd64 broken test-amd64-amd64-xl-credit2 broken test-amd64-amd64-libvirt-pair broken test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm broken test-amd64-amd64-pairbroken test-amd64-amd64-xl-qemuu-win7-amd64 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-xl-credit2 5 host-install(5)broken REGR. vs. 173462 test-amd64-amd64-xl-qemut-debianhvm-amd64 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-pair 7 host-install/dst_host(7) broken REGR. vs. 173462 test-amd64-amd64-xl-shadow5 host-install(5)broken REGR. vs. 173462 test-amd64-amd64-libvirt-pair 7 host-install/dst_host(7) broken REGR. vs. 173462 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-xl-qemuu-ws16-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-coresched-amd64-xl 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-xl-qemut-win7-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start fail REGR. vs. 173462 test-amd64-amd64-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-qemuu-nested-amd 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-qemuu-nested-intel 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-pvhv2-amd 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-xl-qemut-ws16-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-freebsd12-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-freebsd11-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-pvhv2-intel 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-multivcpu 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-libvirt-qcow2 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-libvirt 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-pvshim8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-examine 8 reboot fail REGR. vs. 173462 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 173462 test-arm64-arm64-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-xsm 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-libvirt-xsm 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-ovmf-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-debianhvm-amd64 8 xen-bootfail REGR. vs. 173462 test-armhf-armhf-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-examine 8 reboot fail REGR. vs. 173462 test-amd64-amd64-xl 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-examine-uefi 8 reboot fail REGR. vs. 173462 test-amd64-amd64-examine-bios 8 reboot fail REGR. vs. 173462 test-amd64-amd64-examine 8 reboot fail REGR. vs. 173462 test-amd64-amd64-pygrub 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-seattle 8 xen-boot fail REGR. vs. 173462 test-arm6
[xen-unstable-smoke test] 175449: tolerable all pass - PUSHED
flight 175449 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/175449/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: 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 test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass version targeted for testing: xen 9c57a297378932249c3edefa5065c838f47cb3fb baseline version: xen dc380df12acfe53ccdcbeecaaee3510a3b0e374e Last test of basis 175431 2022-12-20 21:03:41 Z1 days Testing same since 175449 2022-12-22 02:00:30 Z0 days1 attempts People who touched revisions under test: Michal Orzel jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git dc380df12a..9c57a29737 9c57a297378932249c3edefa5065c838f47cb3fb -> smoke
Re: [RFC 0/4] Adding Virtual Memory Fuses to Xen
On Wed, Dec 21, 2022 at 04:53:46PM -0800, Stefano Stabellini wrote: > On Tue, 20 Dec 2022, Demi Marie Obenour wrote: > > On Tue, Dec 20, 2022 at 10:17:24PM +, Smith, Jackson wrote: > > > > Hi Stefano, > > > > > > > > On 16/12/2022 01:46, Stefano Stabellini wrote: > > > > > On Thu, 15 Dec 2022, Julien Grall wrote: > > > > On 13/12/2022 19:48, Smith, Jackson wrote: > > > > >>> Yes, we are familiar with the "secret-free hypervisor" work. As > > > you > > > > >>> point out, both our work and the secret-free hypervisor remove the > > > > >>> directmap region to mitigate the risk of leaking sensitive guest > > > > >>> secrets. However, our work is slightly different because it > > > > >>> additionally prevents attackers from tricking Xen into remapping a > > > > guest. > > > > >> > > > > >> I understand your goal, but I don't think this is achieved (see > > > > >> above). You would need an entity to prevent write to TTBR0_EL2 in > > > > >> order to fully protect it. > > > > > > > > > > Without a way to stop Xen from reading/writing TTBR0_EL2, we > > > > cannot > > > > > claim that the guest's secrets are 100% safe. > > > > > > > > > > But the attacker would have to follow the sequence you outlines > > > > above > > > > > to change Xen's pagetables and remap guest memory before > > > > accessing it. > > > > > It is an additional obstacle for attackers that want to steal other > > > > guests' > > > > > secrets. The size of the code that the attacker would need to inject > > > > > in Xen would need to be bigger and more complex. > > > > > > > > Right, that's why I wrote with a bit more work. However, the nuance > > > > you mention doesn't seem to be present in the cover letter: > > > > > > > > "This creates what we call "Software Enclaves", ensuring that an > > > > adversary with arbitrary code execution in the hypervisor STILL cannot > > > > read/write guest memory." > > > > > > > > So if the end goal if really to protect against *all* sort of > > > arbitrary > > > > code, > > > > then I think we should have a rough idea how this will look like in > > > Xen. > > > > > > > > From a brief look, it doesn't look like it would be possible to > > > prevent > > > > modification to TTBR0_EL2 (even from EL3). We would need to > > > > investigate if there are other bits in the architecture to help us. > > > > > > > > > > > > > > Every little helps :-) > > > > > > > > I can see how making the life of the attacker more difficult is > > > > appealing. > > > > Yet, the goal needs to be clarified and the risk with the approach > > > > acknowledged (see above). > > > > > > > > > > You're right, we should have mentioned this weakness in our first email. > > > Sorry about the oversight! This is definitely still a limitation that we > > > have not yet overcome. However, we do think that the increase in > > > attacker workload that you and Stefano are discussing could still be > > > valuable to security conscious Xen users. > > > > > > It would nice to find additional architecture features that we can use > > > to close this hole on arm, but there aren't any that stand out to me > > > either. > > > > > > With this limitation in mind, what are the next steps we should take to > > > support this feature for the xen community? Is this increase in attacker > > > workload meaningful enough to justify the inclusion of VMF in Xen? > > > > Personally, I don’t think so. The kinds of workloads VMF is usable > > for (no hypercalls) are likely easily portable to other hypervisors, > > including formally verified microkernels such as seL4 that provide... > > What other hypervisors might or might not do should not be a factor in > this discussion and it would be best to leave it aside. Indeed so, sorry. > From an AMD/Xilinx point of view, most of our customers using Xen in > productions today don't use any hypercalls in one or more of their VMs. > Xen is great for these use-cases and it is rather common in embedded. > It is certainly a different configuration from what most are come to > expect from Xen on the server/desktop x86 side. There is no question > that guests without hypercalls are important for Xen on ARM. I was completely unaware of this. > As a Xen community we have a long history and strong interest in making > Xen more secure and also, more recently, safer (in the ISO 26262 > safety-certification sense). The VMF work is very well aligned with both > of these efforts and any additional burder to attackers is certainly > good for Xen. That it is. > Now the question is what changes are necessary and how to make them to > the codebase. And if it turns out that some of the changes are not > applicable or too complex to accept, the decision will be made purely > from a code maintenance point of view and will have nothing to do with > VMs making no hypercalls being unimportant (i.e. if we don't accept one > or more patches is not going to have anything to do with the use-case > being unimportant or what other hyper
Re: [RFC PATCH 01/18] arm: cppcheck: misra rule 20.7 deviations for alternative.h
On Tue, 20 Dec 2022, Julien Grall wrote:
> Hi Luca,
>
> On 20/12/2022 08:50, Luca Fancellu wrote:
> > Cppcheck reports violations of rule 20.7 in two macros of
> > alternative.h.
> >
> > The first one is in the __ALT_PTR macro where cppcheck wants to have
> > parentheses on the second operand of a member access operator, which
> > is not allowed from the c language syntax, furthermore, the macro
> > parameter is never used as an expression, hence we can suppress the
> > finding.
>
> Looking at the Misra Rule 20.7 examples [1], this looks similar to
> GET_MEMBER(). So I don't understand why cppcheck is complaining.
>
> >
> > The second finding is in the __ALTERNATIVE_CFG macro, where cppcheck
> > wants to have parentheses on the macro arguments, but the macro
> > parameters are never used as expressions and are used only for text
> > substitution, so cppcheck is not taking into account the context of
> > use of the macro arguments and adding parenthesis would break the
> > code, so we can suppress the finding.
>
> This reads like you want to report a bug against cppcheck.
+1
> > No error was shown by eclair and coverity for those lines.
> >
> > Signed-off-by: Luca Fancellu
> > ---
> > docs/misra/false-positive-cppcheck.json | 14 ++
> > xen/arch/arm/include/asm/alternative.h | 2 ++
>
> This file is meant to be close to Linux. From my understanding of the
> discussion yesterday, we didn't want to add deviation there.
Yeah. We are still finalizing the list of Linux files in Xen
(https://marc.info/?l=xen-devel&m=166859007703945) so we don't have a
programmatic way to check if something should be scanned or not. At the
moment it is easy to make mistakes. I hope will get it committed soon,
then we can blacklist anything listed under
docs/misra/external-files.txt (automatically ask cppcheck to avoid
scanning files listed in docs/misra/external-files.txt).
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/docs/misra/false-positive-cppcheck.json
> > b/docs/misra/false-positive-cppcheck.json
> > index 5d4da2ce6170..5e7d9377f60b 100644
> > --- a/docs/misra/false-positive-cppcheck.json
> > +++ b/docs/misra/false-positive-cppcheck.json
> > @@ -3,6 +3,20 @@
> > "content": [
> > {
> > "id": "SAF-0-false-positive-cppcheck",
> > +"violation-id": "misra-c2012-20.7",
> > +"tool-version": "2.7",
> > +"name": "R20.7 second operand of member-access operator",
> > +"text": "The second operand of a member access operator shall
> > be a name of a member of the type pointed to, so in this particular case it
> > is wrong to use parentheses on the macro parameter."
> > +},
> > +{
> > +"id": "SAF-1-false-positive-cppcheck",
> > +"violation-id": "misra-c2012-20.7",
> > +"tool-version": "2.7",
> > +"name": "R20.7 C macro parameters used only for text
> > substitution",
> > +"text": "The macro parameters used in this case are not part of
> > an expression, they are used for text substitution."
> > +},
> In both cases the constructs are commonly used in Xen to generate code. So I
> am a bit concerned to have to add deviation everywhere cppcheck is wrong.
>
> More generally, we are still at the beginning of MISRA in Xen and I don't
> think we should start with adding deviations from bugs in the tools. Instead,
> we should report those bugs and hopefully by the time Xen is mostly MISRA
> complaint the tools will not report the false positive.
>
> If they are still then we can decide to add deviations.
I also think we should report cppcheck bugs.
That said, I think the reason why Luca submitted this series is that we
are actually not far from having cppcheck running automatically as part
of gitlab-ci to scan staging and new patches.
Enabling automatic cppcheck scans is something within our reach, I'd say
it is only few weeks away. That's actually going to start giving some
concrete benefits for our work on MISRA C. We'll get automatic bug
reports and quality reports, which is pretty cool.
Aside from the list of Linux files, we also need to deal with false
positives otherwise the reports will be too "noisy". If we correctly
silence the outstanding false-positive cppcheck reports, we can actually
have a clean "zero issues" report by cppcheck on staging with the
current list of MISRA C rules supported by docs/misra/rules.rst.
And there are only few cppcheck false positives to deal with. Most of
them are for Rule 20.7. During the last FuSa call, I actually suggested
to skip Rule 20.7 when it comes to scanning for issues (i.e. blacklist
Rule 20.7).
Luca suggested that before doing that we could at least try to
fix/deviate the violations reported by cppcheck (they are far fewer than
the ones reported by Eclair). So here we are :-)
So in short:
- This is not the first of many series flooding xen-devel adding
deviations; this is the
Re: [RFC 0/7] Proposal to make x86 IOMMU driver support configurable
On Tue, 20 Dec 2022, Xenia Ragiadakou wrote: > > We need to decide whether it is sensible to allow users to turn off > > IOMMU support. It probably is, because you could trivially do this by > > selecting CONFIG_INTEL only, and booting the result on an AMD system. > > > > I cannot understand. I guess that if the code for the target system is > disabled, Xen won't boot ... hopefully :). > > If users are not allowed to turn off the IOMMU support, it can be always > enabled unconditionally via Kconfig select based on the virtualization > technology or other. Just wanted to say that it should be fine either way. If we don't want to provide an option to disable the IOMMU, then it could be handled at the kconfig level.
Re: [PATCH v2] xen/arm: Allow to set grant table related limits for dom0less domUs
On Mon, 19 Dec 2022, Michal Orzel wrote:
> At the moment, for dom0less domUs, we do not have a way to specify
> per domain grant table related limits (unlike when using xl), namely
> max version, max number of grant frames, max number of maptrack frames.
> This means that such domains always use the values specified by the Xen
> command line parameters or their default values if unspecified.
>
> In order to have more control over dom0less domUs, introduce the
> following device-tree properties that can be set under domUs nodes:
> - max_grant_version to set the maximum grant table version the domain
>is allowed to use,
> - max_grant_frames to set the maximum number of grant frames the domain
>is allowed to have,
> - max_maptrack_frames to set the maximum number of grant maptrack frames
>the domain is allowed to have.
>
> Update documentation accordingly.
>
> Note that the values obtained from device tree are of type uint32_t,
> whereas the d_cfg.max_{grant_frames,maptrack_frames} are of type int32_t.
> Call panic in case of overflow. Other sanity checks are already there in
> grant_table_init() resulting in panic in case of errors, therefore no
> need to repeat them in create_domUs().
>
> Signed-off-by: Michal Orzel
Reviewed-by: Stefano Stabellini
> ---
> Changes in v2:
> - call panic in case of int32_t overflow
> ---
> docs/misc/arm/device-tree/booting.txt | 21 +
> xen/arch/arm/domain_build.c | 18 ++
> 2 files changed, 39 insertions(+)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt
> b/docs/misc/arm/device-tree/booting.txt
> index 87eaa3e25491..3879340b5e0a 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -223,6 +223,27 @@ with the following properties:
> the default size of domain P2M pool, i.e. 1MB per guest vCPU plus 4KB
> per MB of guest RAM plus 512KB for guest extended regions.
>
> +- max_grant_version
> +
> +Optional. A 32-bit integer specifying the maximum grant table version
> +the domain is allowed to use (valid values are 1 or 2). If this property
> +is missing, the value specified by Xen command line parameter
> gnttab=max-ver
> +(or its default value if unspecified, i.e. 1) is used.
> +
> +- max_grant_frames
> +
> +Optional. A 32-bit integer specifying the maximum number of grant frames
> +the domain is allowed to have. If this property is missing, the value
> +specified by Xen command line parameter gnttab_max_frames (or its default
> +value if unspecified, i.e. 64) is used.
> +
> +- max_maptrack_frames
> +
> +Optional. A 32-bit integer specifying the maximum number of grant
> maptrack
> +frames the domain is allowed to have. If this property is missing, the
> +value specified by Xen command line parameter gnttab_max_maptrack_frames
> +(or its default value if unspecified, i.e. 1024) is used.
> +
> Under the "xen,domain" compatible node, one or more sub-nodes are present
> for the DomU kernel and ramdisk.
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index bef5e905a73c..829cea8de84f 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3872,6 +3872,7 @@ void __init create_domUs(void)
> .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
> };
> unsigned int flags = 0U;
> +uint32_t val;
>
> if ( !dt_device_is_compatible(node, "xen,domain") )
> continue;
> @@ -3940,6 +3941,23 @@ void __init create_domUs(void)
> d_cfg.cpupool_id = pool_id;
> }
>
> +if ( dt_property_read_u32(node, "max_grant_version", &val) )
> +d_cfg.grant_opts = XEN_DOMCTL_GRANT_version(val);
> +
> +if ( dt_property_read_u32(node, "max_grant_frames", &val) )
> +{
> +if ( val > INT32_MAX )
> +panic("max_grant_frames (%"PRIu32") overflow\n", val);
> +d_cfg.max_grant_frames = val;
> +}
> +
> +if ( dt_property_read_u32(node, "max_maptrack_frames", &val) )
> +{
> +if ( val > INT32_MAX )
> +panic("max_maptrack_frames (%"PRIu32") overflow\n", val);
> +d_cfg.max_maptrack_frames = val;
> +}
> +
> /*
> * The variable max_init_domid is initialized with zero, so here it's
> * very important to use the pre-increment operator to call
> --
> 2.25.1
>
Re: [RFC 0/4] Adding Virtual Memory Fuses to Xen
On Tue, 20 Dec 2022, Demi Marie Obenour wrote: > On Tue, Dec 20, 2022 at 10:17:24PM +, Smith, Jackson wrote: > > > Hi Stefano, > > > > > > On 16/12/2022 01:46, Stefano Stabellini wrote: > > > > On Thu, 15 Dec 2022, Julien Grall wrote: > > > On 13/12/2022 19:48, Smith, Jackson wrote: > > > >>> Yes, we are familiar with the "secret-free hypervisor" work. As > > you > > > >>> point out, both our work and the secret-free hypervisor remove the > > > >>> directmap region to mitigate the risk of leaking sensitive guest > > > >>> secrets. However, our work is slightly different because it > > > >>> additionally prevents attackers from tricking Xen into remapping a > > > guest. > > > >> > > > >> I understand your goal, but I don't think this is achieved (see > > > >> above). You would need an entity to prevent write to TTBR0_EL2 in > > > >> order to fully protect it. > > > > > > > > Without a way to stop Xen from reading/writing TTBR0_EL2, we > > > cannot > > > > claim that the guest's secrets are 100% safe. > > > > > > > > But the attacker would have to follow the sequence you outlines > > > above > > > > to change Xen's pagetables and remap guest memory before > > > accessing it. > > > > It is an additional obstacle for attackers that want to steal other > > > guests' > > > > secrets. The size of the code that the attacker would need to inject > > > > in Xen would need to be bigger and more complex. > > > > > > Right, that's why I wrote with a bit more work. However, the nuance > > > you mention doesn't seem to be present in the cover letter: > > > > > > "This creates what we call "Software Enclaves", ensuring that an > > > adversary with arbitrary code execution in the hypervisor STILL cannot > > > read/write guest memory." > > > > > > So if the end goal if really to protect against *all* sort of > > arbitrary > > > code, > > > then I think we should have a rough idea how this will look like in > > Xen. > > > > > > From a brief look, it doesn't look like it would be possible to > > prevent > > > modification to TTBR0_EL2 (even from EL3). We would need to > > > investigate if there are other bits in the architecture to help us. > > > > > > > > > > > Every little helps :-) > > > > > > I can see how making the life of the attacker more difficult is > > > appealing. > > > Yet, the goal needs to be clarified and the risk with the approach > > > acknowledged (see above). > > > > > > > You're right, we should have mentioned this weakness in our first email. > > Sorry about the oversight! This is definitely still a limitation that we > > have not yet overcome. However, we do think that the increase in > > attacker workload that you and Stefano are discussing could still be > > valuable to security conscious Xen users. > > > > It would nice to find additional architecture features that we can use > > to close this hole on arm, but there aren't any that stand out to me > > either. > > > > With this limitation in mind, what are the next steps we should take to > > support this feature for the xen community? Is this increase in attacker > > workload meaningful enough to justify the inclusion of VMF in Xen? > > Personally, I don’t think so. The kinds of workloads VMF is usable > for (no hypercalls) are likely easily portable to other hypervisors, > including formally verified microkernels such as seL4 that provide... What other hypervisors might or might not do should not be a factor in this discussion and it would be best to leave it aside. >From an AMD/Xilinx point of view, most of our customers using Xen in productions today don't use any hypercalls in one or more of their VMs. Xen is great for these use-cases and it is rather common in embedded. It is certainly a different configuration from what most are come to expect from Xen on the server/desktop x86 side. There is no question that guests without hypercalls are important for Xen on ARM. As a Xen community we have a long history and strong interest in making Xen more secure and also, more recently, safer (in the ISO 26262 safety-certification sense). The VMF work is very well aligned with both of these efforts and any additional burder to attackers is certainly good for Xen. Now the question is what changes are necessary and how to make them to the codebase. And if it turns out that some of the changes are not applicable or too complex to accept, the decision will be made purely from a code maintenance point of view and will have nothing to do with VMs making no hypercalls being unimportant (i.e. if we don't accept one or more patches is not going to have anything to do with the use-case being unimportant or what other hypervisors might or might not do).
RE: [RFC 0/4] Adding Virtual Memory Fuses to Xen
On Tue, 20 Dec 2022, Smith, Jackson wrote: > > Hi Stefano, > > > > On 16/12/2022 01:46, Stefano Stabellini wrote: > > > On Thu, 15 Dec 2022, Julien Grall wrote: > > On 13/12/2022 19:48, Smith, Jackson wrote: > > >>> Yes, we are familiar with the "secret-free hypervisor" work. As > you > > >>> point out, both our work and the secret-free hypervisor remove the > > >>> directmap region to mitigate the risk of leaking sensitive guest > > >>> secrets. However, our work is slightly different because it > > >>> additionally prevents attackers from tricking Xen into remapping a > > guest. > > >> > > >> I understand your goal, but I don't think this is achieved (see > > >> above). You would need an entity to prevent write to TTBR0_EL2 in > > >> order to fully protect it. > > > > > > Without a way to stop Xen from reading/writing TTBR0_EL2, we > > cannot > > > claim that the guest's secrets are 100% safe. > > > > > > But the attacker would have to follow the sequence you outlines > > above > > > to change Xen's pagetables and remap guest memory before > > accessing it. > > > It is an additional obstacle for attackers that want to steal other > > guests' > > > secrets. The size of the code that the attacker would need to inject > > > in Xen would need to be bigger and more complex. > > > > Right, that's why I wrote with a bit more work. However, the nuance > > you mention doesn't seem to be present in the cover letter: > > > > "This creates what we call "Software Enclaves", ensuring that an > > adversary with arbitrary code execution in the hypervisor STILL cannot > > read/write guest memory." > > > > So if the end goal if really to protect against *all* sort of > arbitrary > > code, > > then I think we should have a rough idea how this will look like in > Xen. > > > > From a brief look, it doesn't look like it would be possible to > prevent > > modification to TTBR0_EL2 (even from EL3). We would need to > > investigate if there are other bits in the architecture to help us. > > > > > > > > Every little helps :-) > > > > I can see how making the life of the attacker more difficult is > > appealing. > > Yet, the goal needs to be clarified and the risk with the approach > > acknowledged (see above). > > > > You're right, we should have mentioned this weakness in our first email. > Sorry about the oversight! This is definitely still a limitation that we > have not yet overcome. However, we do think that the increase in > attacker workload that you and Stefano are discussing could still be > valuable to security conscious Xen users. > > It would nice to find additional architecture features that we can use > to close this hole on arm, but there aren't any that stand out to me > either. > > With this limitation in mind, what are the next steps we should take to > support this feature for the xen community? Is this increase in attacker > workload meaningful enough to justify the inclusion of VMF in Xen? I think it could be valuable as an additional obstacle for the attacker to overcome. The next step would be to port your series on top of Julien's "Remove the directmap" patch series https://marc.info/?l=xen-devel&m=167119090721116 Julien, what do you think?
[xen-4.17-testing test] 175437: regressions - trouble: broken/fail/pass
flight 175437 xen-4.17-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/175437/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-livepatchbroken test-amd64-i386-xl-qemuu-debianhvm-i386-xsm broken test-amd64-amd64-xl-qemut-debianhvm-amd64 broken test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 5 host-install(5) broken REGR. vs. 175096 test-amd64-amd64-xl-qemut-debianhvm-amd64 5 host-install(5) broken REGR. vs. 175096 test-amd64-i386-livepatch 5 host-install(5)broken REGR. vs. 175096 test-amd64-i386-migrupgrade 26 guest-migrate/src_host/dst_host fail REGR. vs. 175096 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175096 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 175096 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175096 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 175096 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175096 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175096 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 175096 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 175096 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175096 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175096 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175096 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175096 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt 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-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-r
[libvirt test] 175436: tolerable all pass - PUSHED
flight 175436 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/175436/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175306 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175306 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175306 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-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-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 15 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-libvirt-qcow2 14 migrate-support-checkfail never pass version targeted for testing: libvirt 4102acc608819b15658ca3e37ceca7c89efae16b baseline version: libvirt b271d6f3b0d70e8e7be22252bf25deebe8536d39 Last test of basis 175306 2022-12-16 04:21:45 Z5 days Testing same since 175419 2022-12-20 04:18:52 Z1 days2 attempts People who touched revisions under test: Ján Tomko 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 gene
[ovmf test] 175444: all pass - PUSHED
flight 175444 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/175444/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 62031335bdbacc68253d43481477b9a468e0644e baseline version: ovmf 0b633b14944903c32aa061befaf38bd8d994cf13 Last test of basis 175441 2022-12-21 11:13:47 Z0 days Testing same since 175444 2022-12-21 16:12:30 Z0 days1 attempts People who touched revisions under test: Dun Tan duntan Jian J Wang Judah Vang Nishant C Mistry Tan, Dun 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 0b633b1494..62031335bd 62031335bdbacc68253d43481477b9a468e0644e -> xen-tested-master
[xen-unstable test] 175434: regressions - trouble: broken/fail/pass
flight 175434 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/175434/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-libvirt-pair broken test-amd64-i386-migrupgrade broken test-amd64-i386-xl-pvshimbroken test-amd64-i386-xl-qemut-debianhvm-i386-xsm broken test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsmbroken test-amd64-i386-xl-pvshim 5 host-install(5)broken REGR. vs. 175399 test-amd64-i386-libvirt-pair 6 host-install/src_host(6) broken REGR. vs. 175399 test-amd64-i386-migrupgrade 6 host-install/src_host(6) broken REGR. vs. 175399 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 5 host-install(5) broken REGR. vs. 175399 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 5 host-install(5) broken REGR. vs. 175399 test-amd64-amd64-examine-uefi 5 host-install broken REGR. vs. 175399 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 175399 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-examine-uefi 4 memdisk-try-append fail REGR. vs. 175399 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 175399 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175399 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175399 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175399 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 175399 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 175399 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175399 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175399 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 175399 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175399 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175399 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175399 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-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-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-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-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-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-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-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-
[linux-5.4 test] 175433: trouble: broken/fail/pass
flight 175433 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/175433/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-pair broken test-amd64-amd64-xl-qemuu-ovmf-amd64 broken test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadowbroken test-amd64-amd64-xl-qemuu-debianhvm-amd64 broken test-amd64-amd64-xl broken test-amd64-i386-xl-qemuu-ws16-amd64 broken test-amd64-i386-xl-qemuu-ws16-amd64 5 host-install(5) broken REGR. vs. 175197 test-amd64-amd64-xl-credit2 broken in 175407 test-amd64-i386-qemuu-rhel6hvm-intel broken in 175407 test-amd64-i386-xl-qemuu-debianhvm-amd64 broken in 175418 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm broken in 175418 test-amd64-i386-xl-qemut-debianhvm-amd64 broken in 175418 test-amd64-amd64-xl-qemut-debianhvm-amd64 broken in 175418 test-amd64-amd64-dom0pvh-xl-intel broken in 175418 Tests which are failing intermittently (not blocking): test-amd64-i386-qemuu-rhel6hvm-intel 5 host-install(5) broken in 175407 pass in 175433 test-amd64-amd64-xl-credit2 5 host-install(5) broken in 175407 pass in 175433 test-amd64-i386-xl-qemuu-debianhvm-amd64 5 host-install(5) broken in 175418 pass in 175433 test-amd64-i386-xl-qemut-debianhvm-amd64 5 host-install(5) broken in 175418 pass in 175433 test-amd64-amd64-dom0pvh-xl-intel 5 host-install(5) broken in 175418 pass in 175433 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 5 host-install(5) broken in 175418 pass in 175433 test-amd64-amd64-xl-qemut-debianhvm-amd64 5 host-install(5) broken in 175418 pass in 175433 test-amd64-i386-examine 5 host-install broken pass in 175407 test-amd64-amd64-xl-qemuu-debianhvm-amd64 5 host-install(5) broken pass in 175418 test-amd64-amd64-xl 5 host-install(5) broken pass in 175418 test-amd64-amd64-xl-qemuu-ovmf-amd64 5 host-install(5) broken pass in 175418 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 5 host-install(5) broken pass in 175418 test-amd64-i386-pair 7 host-install/dst_host(7) broken pass in 175418 test-armhf-armhf-xl-multivcpu 14 guest-start fail in 175418 pass in 175433 test-arm64-arm64-libvirt-raw 18 guest-start.2fail in 175418 pass in 175433 test-armhf-armhf-xl-vhd 13 guest-start fail in 175418 pass in 175433 test-amd64-i386-xl-vhd 21 guest-start/debian.repeat fail in 175418 pass in 175433 test-armhf-armhf-xl-credit1 18 guest-start/debian.repeat fail pass in 175407 test-armhf-armhf-xl-rtds 14 guest-startfail pass in 175418 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail blocked in 175197 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail in 175407 like 175197 test-armhf-armhf-xl-rtds 18 guest-start/debian.repeat fail in 175418 like 175197 test-armhf-armhf-xl-rtds15 migrate-support-check fail in 175418 never pass test-armhf-armhf-xl-rtds 16 saverestore-support-check fail in 175418 never pass test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175197 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 175197 test-armhf-armhf-xl-multivcpu 18 guest-start/debian.repeatfail like 175197 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175197 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 175197 test-armhf-armhf-xl-credit2 18 guest-start/debian.repeatfail like 175197 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175197 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175197 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175197 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 175197 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 175197 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175197 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-arm64-ar
Re: [PATCH 8/8] x86/shadow: drop zero initialization from shadow_domain_init()
On 21/12/2022 1:28 pm, Jan Beulich wrote: > There's no need for this as struct domain starts out zero-filled. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper
Re: [PATCH 7/8] x86/shadow: don't open-code copy_domain_page()
On 21/12/2022 1:27 pm, Jan Beulich wrote: > Let's use the library-like function that we have. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper
Re: [PATCH 6/8] x86/shadow: adjust and move sh_type_to_size[]
On 21/12/2022 1:27 pm, Jan Beulich wrote:
> Drop the SH_type_none entry - there are no allocation attempts with
> this type, and there also shouldn't be any. Adjust the shadow_size()
> alternative path to match that change. Also generalize two related
> assertions.
>
> While there move the entire table and the respective part of the comment
> there to hvm.c, resulting in one less #ifdef. In the course of the
> movement switch to using designated initializers.
>
> Signed-off-by: Jan Beulich
Reviewed-by: Andrew Cooper , although ...
> --- a/xen/arch/x86/mm/shadow/hvm.c
> +++ b/xen/arch/x86/mm/shadow/hvm.c
> @@ -33,6 +33,37 @@
>
> #include "private.h"
>
> +/*
> + * This table shows the allocation behaviour of the different modes:
> + *
> + * Xen paging 64b 64b 64b
> + * Guest paging32b pae 64b
> + * PV or HVM HVM HVM *
> + * Shadow paging pae pae 64b
> + *
> + * sl1 size 8k 4k 4k
> + * sl2 size16k 4k 4k
> + * sl3 size --4k
> + * sl4 size --4k
> + */
> +const uint8_t sh_type_to_size[] = {
> +[SH_type_l1_32_shadow] = 2,
> +[SH_type_fl1_32_shadow] = 2,
> +[SH_type_l2_32_shadow] = 4,
> +[SH_type_l1_pae_shadow] = 1,
> +[SH_type_fl1_pae_shadow] = 1,
> +[SH_type_l2_pae_shadow] = 1,
> +[SH_type_l1_64_shadow] = 1,
> +[SH_type_fl1_64_shadow] = 1,
> +[SH_type_l2_64_shadow] = 1,
> +[SH_type_l2h_64_shadow] = 1,
> +[SH_type_l3_64_shadow] = 1,
> +[SH_type_l4_64_shadow] = 1,
> +[SH_type_p2m_table] = 1,
> +[SH_type_monitor_table] = 1,
> +[SH_type_oos_snapshot] = 1,
... this feels like it's wanting to turn into a (1 + ...) expression. I
can't see anything that prevents us from reordering the SH_type
constants, but
1 + (idx == 1 /* l1 */ || idx == /* fl1 */) + 2 * (idx == 3 /* l2 */);
doesn't obviously simplify further.
~Andrew
[XEN v4] xen/arm: Probe the load/entry point address of an uImage correctly
Currently, kernel_uimage_probe() does not read the load/entry point address
set in the uImge header. Thus, info->zimage.start is 0 (default value). This
causes, kernel_zimage_place() to treat the binary (contained within uImage)
as position independent executable. Thus, it loads it at an incorrect
address.
The correct approach would be to read "uimage.load" and set
info->zimage.start. This will ensure that the binary is loaded at the
correct address. Also, read "uimage.ep" and set info->entry (ie kernel entry
address).
If user provides load address (ie "uimage.load") as 0x0, then the image is
treated as position independent executable. Xen can load such an image at
any address it considers appropriate. A position independent executable
cannot have a fixed entry point address.
This behavior is applicable for both arm32 and arm64 platforms.
Earlier for arm32 and arm64 platforms, Xen was ignoring the load and entry
point address set in the uImage header. With this commit, Xen will use them.
This makes the behavior of Xen consistent with uboot for uimage headers.
Users who want to use Xen with statically partitioned domains, can provide
non zero load address and entry address for the dom0/domU kernel. It is
required that the load and entry address provided must be within the memory
region allocated by Xen.
A deviation from uboot behaviour is that we consider load address == 0x0,
to denote that the image supports position independent execution. This
is to make the behavior consistent across uImage and zImage.
Signed-off-by: Ayan Kumar Halder
---
Changes from v1 :-
1. Added a check to ensure load address and entry address are the same.
2. Considered load address == 0x0 as position independent execution.
3. Ensured that the uImage header interpretation is consistent across
arm32 and arm64.
v2 :-
1. Mentioned the change in existing behavior in booting.txt.
2. Updated booting.txt with a new section to document "Booting Guests".
v3 :-
1. Removed the constraint that the entry point should be same as the load
address. Thus, Xen uses both the load address and entry point to determine
where the image is to be copied and the start address.
2. Updated documentation to denote that load address and start address
should be within the memory region allocated by Xen.
3. Added constraint that user cannot provide entry point for a position
independent executable (PIE) image.
docs/misc/arm/booting.txt | 26 ++
xen/arch/arm/include/asm/kernel.h | 2 +-
xen/arch/arm/kernel.c | 45 ---
3 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/docs/misc/arm/booting.txt b/docs/misc/arm/booting.txt
index 3e0c03e065..12339dfecb 100644
--- a/docs/misc/arm/booting.txt
+++ b/docs/misc/arm/booting.txt
@@ -23,6 +23,28 @@ The exceptions to this on 32-bit ARM are as follows:
There are no exception on 64-bit ARM.
+Booting Guests
+--
+
+Xen supports the legacy image header[3], zImage protocol for 32-bit
+ARM Linux[1] and Image protocol defined for ARM64[2].
+
+Earlier for legacy image protocol, Xen ignored the load address and
+entry point specified in the header. This has now changed.
+
+Now, it loads the image at the load address provided in the header.
+And the entry point is used as the kernel start address.
+
+A deviation from uboot is that, Xen treats "load address == 0x0" as
+position independent execution (PIE). Thus, Xen will load such an image
+at an address it considers appropriate. Also, user cannot specify the
+entry point of a PIE image since the start address cennot be
+predetermined.
+
+Users who want to use Xen with statically partitioned domains, can provide
+the fixed non zero load address and start address for the dom0/domU kernel.
+The load address and start address specified by the user in the header must
+be within the memory region allocated by Xen.
Firmware/bootloader requirements
@@ -39,3 +61,7 @@ Latest version:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/t
[2] linux/Documentation/arm64/booting.rst
Latest version:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.rst
+
+[3] legacy format header
+Latest version:
https://source.denx.de/u-boot/u-boot/-/blob/master/include/image.h#L315
+https://linux.die.net/man/1/mkimage
diff --git a/xen/arch/arm/include/asm/kernel.h
b/xen/arch/arm/include/asm/kernel.h
index 5bb30c3f2f..4617cdc83b 100644
--- a/xen/arch/arm/include/asm/kernel.h
+++ b/xen/arch/arm/include/asm/kernel.h
@@ -72,7 +72,7 @@ struct kernel_info {
#ifdef CONFIG_ARM_64
paddr_t text_offset; /* 64-bit Image only */
#endif
-paddr_t start; /* 32-bit zImage only */
+paddr_t start; /* Must be 0 for 64-bit Image */
} zimage;
};
};
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 23b840ea9e..58b3db0aa4 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/a
Re: [PATCH 5/8] x86/PV: drop dead paging_update_paging_modes() call during Dom0 construction
On 21/12/2022 1:27 pm, Jan Beulich wrote: > The function won't ever be invoked, as paging_mode_enabled() always > returns false here due to the immediately preceding clearing of > d->arch.paging.mode. While compilers recognize this and eliminate the > call, make this explicit in the source (which likely 9a28170f2da2 ["pvh > dom0: construct_dom0 changes"] should have done right away, albeit even > before that the call looks to have been pointless - shadow mode enabling > has occurred later virtually forever). > > While there also update an adjacent partly stale comment. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper
Re: [PATCH 4/8] x86/paging: move and conditionalize flush_tlb() hook
On 21/12/2022 1:26 pm, Jan Beulich wrote:
> The hook isn't mode dependent, hence it's misplaced in struct
> paging_mode. (Or alternatively I see no reason why the alloc_page() and
> free_page() hooks don't also live there.) Move it to struct
> paging_domain.
How you flush the TLBs has absolutely nothing to do with what mode the
guest is in.
But this hook too confuses p2m flushes with vcpu flushes.
> The hook also is used for HVM guests only, so make respective pieces
> conditional upon CONFIG_HVM.
>
> While there also add __must_check to the hook declaration, as it's
> imperative that callers deal with getting back "false".
>
> While moving the shadow implementation, introduce a "curr" local
> variable.
>
> Signed-off-by: Jan Beulich
Reviewed-by: Andrew Cooper , with two
observations.
> --- a/xen/arch/x86/include/asm/paging.h
> +++ b/xen/arch/x86/include/asm/paging.h
> @@ -300,6 +299,12 @@ static inline unsigned long paging_ga_to
> page_order);
> }
>
> +/* Flush selected vCPUs TLBs. NULL for all. */
> +static inline bool paging_flush_tlb(const unsigned long *vcpu_bitmap)
> +{
> +return current->domain->arch.paging.flush_tlb(vcpu_bitmap);
Not for this patch, but for cases like this, we should probably drop the
function pointer.
There are only two options, and they're invariant for the context, so
if ( hap )
hap_flush_tlb(...);
else
shadow_flush_tlb(...);
will almost certainly be faster on any CPU that Xen is liable to run
on. Especially as HAP is probably ~100% common case.
> --- a/xen/arch/x86/mm/shadow/hvm.c
> +++ b/xen/arch/x86/mm/shadow/hvm.c
> @@ -688,6 +688,66 @@ static void sh_emulate_unmap_dest(struct
> atomic_inc(&v->domain->arch.paging.shadow.gtable_dirty_version);
> }
>
> +static bool flush_vcpu(const struct vcpu *v, const unsigned long
> *vcpu_bitmap)
> +{
> +return !vcpu_bitmap || test_bit(v->vcpu_id, vcpu_bitmap);
> +}
> +
> +/* Flush TLB of selected vCPUs. NULL for all. */
> +bool cf_check shadow_flush_tlb(const unsigned long *vcpu_bitmap)
> +{
> +static DEFINE_PER_CPU(cpumask_t, flush_cpumask);
The hap and shadow variants both have a static percpu mask like this.
However, this is an irqs-on region with no nested locking, so I suspect
this path can share one of the scheduler percpu variables too.
~Andrew
Re: [PATCH 3/8] x86/paging: move update_paging_modes() hook
On 21/12/2022 1:25 pm, Jan Beulich wrote: > The hook isn't mode dependent, hence it's misplaced in struct > paging_mode. (Or alternatively I see no reason why the alloc_page() and > free_page() hooks don't also live there.) Move it to struct > paging_domain. > > While there rename the hook and HAP's as well as shadow's hook functions > to use singular; I never understood why plural was used. (Renaming in > particular the wrapper would be touching quite a lot of other code.) There are always two modes; Xen's, and the guest's. This was probably more visible back in the 32-bit days, but remnants of it are still around with the fact that struct vcpu needs to be below the 4G boundary for the PDPTRs for when the guest isn't in Long Mode. HAP also hides it fairly well given the uniformity of EPT/NPT (and always 4 levels in a 64-bit Xen), but I suspect it will become more visible again when we start supporting LA57. All of that said, I have debated the utility of this hook. It mixes up various concepts including the singleton construction of monitor tables, and TLB flushing (which should not happen for a mov cr3 with NOFLUSH set), and with those concepts abstracted properly, the only remaining action is caching the right ops pointer. I'm not convinced it will survive a concerned attempt to fix the HVM p2m vs guest tlb flushing confusion. > --- a/xen/arch/x86/mm/shadow/none.c > +++ b/xen/arch/x86/mm/shadow/none.c > @@ -27,6 +32,9 @@ int shadow_domain_init(struct domain *d) > }; > > paging_log_dirty_init(d, &sh_none_ops); > + > +d->arch.paging.update_paging_mode = _update_paging_mode; > + > return is_hvm_domain(d) ? -EOPNOTSUPP : 0; I know you haven't changed the logic here, but this hook looks broken. Why do we fail it right at the end for HVM domains? ~Andrew
Re: [PATCH 2/8] x86/paging: fold most HAP and shadow final teardown
On 21/12/2022 1:25 pm, Jan Beulich wrote:
> --- a/xen/arch/x86/mm/paging.c
> +++ b/xen/arch/x86/mm/paging.c
> @@ -842,10 +842,46 @@ int paging_teardown(struct domain *d)
> /* Call once all of the references to the domain have gone away */
> void paging_final_teardown(struct domain *d)
> {
> -if ( hap_enabled(d) )
> +bool hap = hap_enabled(d);
> +
> +PAGING_PRINTK("%pd final teardown starts. Pages total = %u, free = %u,
> p2m = %u\n",
PAGING_PRINTK() already includes __func__, so just "%pd start: total %u,
free %u, p2m %u\n" which is shorter.
> + d, d->arch.paging.total_pages,
> + d->arch.paging.free_pages, d->arch.paging.p2m_pages);
> +
> +if ( hap )
> hap_final_teardown(d);
> +
> +/*
> + * Double-check that the domain didn't have any paging memory.
> + * It is possible for a domain that never got domain_kill()ed
> + * to get here with its paging allocation intact.
I know you're mostly just moving this comment, but it's misleading.
This path is used for the domain_create() error path, and there will be
a nonzero allocation for HVM guests.
I think we do want to rework this eventually - we will simplify things
massively by splitting the things can can only happen for a domain which
has run into relinquish_resources.
At a minimum, I'd suggest dropping the first sentence. "double check"
implies it's an extraordinary case, which isn't warranted here IMO.
> + */
> +if ( d->arch.paging.total_pages )
> +{
> +if ( hap )
> +hap_teardown(d, NULL);
> +else
> +shadow_teardown(d, NULL);
> +}
> +
> +/* It is now safe to pull down the p2m map. */
> +p2m_teardown(p2m_get_hostp2m(d), true, NULL);
> +
> +/* Free any paging memory that the p2m teardown released. */
I don't think this isn't true any more. 410 also made HAP/shadow free
pages fully for a dying domain, so p2m_teardown() at this point won't
add to the free memory pool.
I think the subsequent *_set_allocation() can be dropped, and the
assertions left.
~Andrew
[ovmf test] 175441: all pass - PUSHED
flight 175441 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/175441/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 0b633b14944903c32aa061befaf38bd8d994cf13 baseline version: ovmf d103840cfb559c28831c2635b916d60118f671cc Last test of basis 175202 2022-12-14 13:42:59 Z7 days Failing since175214 2022-12-14 18:42:16 Z6 days 41 attempts Testing same since 175441 2022-12-21 11:13:47 Z0 days1 attempts People who touched revisions under test: Abner Chang Adam Dunlap Anthony PERARD Ard Biesheuvel Chun-Yi Lee Chun-Yi Lee de...@edk2.groups.io Dov Murik Gerd Hoffmann jdzhang Jeff Brasen Jeshua Smith Jian J Wang Jiaqi Gao Jiewen Yao Judah Vang Kavya Kuo, Ted MarsX Lin Matt DeVillier Michael D Kinney Michael Kubacki Min M Xu Min Xu Nishant C Mistry Ray Ni Rebecca Cran Rebecca Cran Sean Rhodes Sebastien Boeuf Ted Kuo Tom Lendacky Xie, Yuanhao Yuanhao Xie 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 d103840cfb..0b633b1494 0b633b14944903c32aa061befaf38bd8d994cf13 -> xen-tested-master
Re: [RFC 5/7] x86/iommu: the code addressing CVE-2011-1898 is VT-d specific
On 12/20/22 13:09, Andrew Cooper wrote: On 19/12/2022 6:34 am, Xenia Ragiadakou wrote: The variable untrusted_msi indicates whether the system is vulnerable to CVE-2011-1898. This vulnerablity is VT-d specific. Place the code that addresses the issue under CONFIG_INTEL_VTD. No functional change intended. Signed-off-by: Xenia Ragiadakou Actually, this variable is pretty bogus. I think I'd like to delete it entirely. Nevertheless, I don't think that it would be appropriate to be done as part of this series. There are systems with no IOMMU at all, and we certainly used to let PV Passthrough go ahead. (Not sure we do any more.) There are systems with DMA remapping only, but no interrupt remapping. These are known insecure. I'm honestly not convinced that an ISR read and crash is useful when the user has already constructed an known-unsafe configuration, because a malicious guest in that case can still fully mess with dom0 by sending vectors other than 0x80 and 0x82. In particular, this option does not get activated on AMD when the user elects to disable interrupt remapping, and I'm disinclined to wire it up in that case too. ~Andrew P.S. It occurs to me that FRED obsoletes the need for this anyway, seeing as it does properly distinguish the source of an event. -- Xenia
Re: [PATCH 1/8] x86/paging: fold HAP and shadow memory alloc related fields
On 21.12.2022 14:49, Andrew Cooper wrote:
> On 21/12/2022 1:24 pm, Jan Beulich wrote:
>> Especially with struct shadow_domain and struct hap_domain not living in
>> a union inside struct paging_domain, let's avoid the duplication: The
>> fields are named and used in identical ways, and only one of HAP or
>> shadow can be in use for a domain. This then also renders involved
>> expressions slightly more legible.
>>
>> Signed-off-by: Jan Beulich
>
> Reviewed-by: Andrew Cooper , with two minor
> suggestions.
Thanks.
>> --- a/xen/arch/x86/include/asm/domain.h
>> +++ b/xen/arch/x86/include/asm/domain.h
>> @@ -179,10 +173,6 @@ struct shadow_vcpu {
>> /*hardware assisted paging */
>> //
>> struct hap_domain {
>> -struct page_list_head freelist;
>> -unsigned int total_pages; /* number of pages allocated */
>> -unsigned int free_pages; /* number of pages on freelists */
>> -unsigned int p2m_pages;/* number of pages allocated to p2m */
>> };
>
> Do we want to drop hap_domain? I can't forsee needing to put anything
> back into it.
I would prefer to keep it, even if it's unclear what may want putting
there. It's not obvious to me at all that nothing ever will.
>> --- a/xen/arch/x86/mm/paging.c
>> +++ b/xen/arch/x86/mm/paging.c
>> @@ -979,17 +980,17 @@ int __init paging_set_allocation(struct
>>
>> int arch_get_paging_mempool_size(struct domain *d, uint64_t *size)
>> {
>> -int rc;
>> +unsigned long pages;
>>
>> if ( is_pv_domain(d) ) /* TODO: Relax in due course */
>> return -EOPNOTSUPP;
>>
>> -if ( hap_enabled(d) )
>> -rc = hap_get_allocation_bytes(d, size);
>> -else
>> -rc = shadow_get_allocation_bytes(d, size);
>> +pages = d->arch.paging.total_pages;
>> +pages += d->arch.paging.p2m_pages;
>
> Any chance I can talk you into having a second space before the =, so we
> get:
>
> pages = d->arch.paging.total_pages;
> pages += d->arch.paging.p2m_pages;
>
> nicely aligned vertically?
Sure, I can do that. I recall I was actually pondering whether to ...
Jan
Re: [PATCH 1/8] x86/paging: fold HAP and shadow memory alloc related fields
On 21/12/2022 1:24 pm, Jan Beulich wrote:
> Especially with struct shadow_domain and struct hap_domain not living in
> a union inside struct paging_domain, let's avoid the duplication: The
> fields are named and used in identical ways, and only one of HAP or
> shadow can be in use for a domain. This then also renders involved
> expressions slightly more legible.
>
> Signed-off-by: Jan Beulich
Reviewed-by: Andrew Cooper , with two minor
suggestions.
> ---
> Quite likely more folding of code is possible with this. For example
> {hap,shadow}_set_allocation() are now yet more similar than they already
> were.
TBH, I think it wants to go further still. paging_mempool was
intentionally common because all HAP architectures need it, and I really
don't want to be playing XSA-410 with every new architecture.
But this is fine to defer until further work.
> --- a/xen/arch/x86/include/asm/domain.h
> +++ b/xen/arch/x86/include/asm/domain.h
> @@ -179,10 +173,6 @@ struct shadow_vcpu {
> /*hardware assisted paging */
> //
> struct hap_domain {
> -struct page_list_head freelist;
> -unsigned int total_pages; /* number of pages allocated */
> -unsigned int free_pages; /* number of pages on freelists */
> -unsigned int p2m_pages;/* number of pages allocated to p2m */
> };
Do we want to drop hap_domain? I can't forsee needing to put anything
back into it.
> --- a/xen/arch/x86/mm/paging.c
> +++ b/xen/arch/x86/mm/paging.c
> @@ -979,17 +980,17 @@ int __init paging_set_allocation(struct
>
> int arch_get_paging_mempool_size(struct domain *d, uint64_t *size)
> {
> -int rc;
> +unsigned long pages;
>
> if ( is_pv_domain(d) ) /* TODO: Relax in due course */
> return -EOPNOTSUPP;
>
> -if ( hap_enabled(d) )
> -rc = hap_get_allocation_bytes(d, size);
> -else
> -rc = shadow_get_allocation_bytes(d, size);
> +pages = d->arch.paging.total_pages;
> +pages += d->arch.paging.p2m_pages;
Any chance I can talk you into having a second space before the =, so we
get:
pages = d->arch.paging.total_pages;
pages += d->arch.paging.p2m_pages;
nicely aligned vertically?
~Andrew
[PATCH 5/8] x86/PV: drop dead paging_update_paging_modes() call during Dom0 construction
The function won't ever be invoked, as paging_mode_enabled() always returns false here due to the immediately preceding clearing of d->arch.paging.mode. While compilers recognize this and eliminate the call, make this explicit in the source (which likely 9a28170f2da2 ["pvh dom0: construct_dom0 changes"] should have done right away, albeit even before that the call looks to have been pointless - shadow mode enabling has occurred later virtually forever). While there also update an adjacent partly stale comment. Signed-off-by: Jan Beulich --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -804,11 +803,8 @@ int __init dom0_construct_pv(struct doma d->arch.paging.mode = 0; -/* Set up CR3 value for write_ptbase */ -if ( paging_mode_enabled(d) ) -paging_update_paging_modes(v); -else -update_cr3(v); +/* Set up CR3 value for switch_cr3_cr4(). */ +update_cr3(v); /* We run on dom0's page tables for the final part of the build process. */ switch_cr3_cr4(cr3_pa(v->arch.cr3), read_cr4());
[PATCH 8/8] x86/shadow: drop zero initialization from shadow_domain_init()
There's no need for this as struct domain starts out zero-filled. Signed-off-by: Jan Beulich --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -64,12 +64,8 @@ int shadow_domain_init(struct domain *d) d->arch.paging.update_paging_mode = shadow_update_paging_mode; -#if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC) -d->arch.paging.shadow.oos_active = 0; -#endif #ifdef CONFIG_HVM d->arch.paging.flush_tlb = shadow_flush_tlb; -d->arch.paging.shadow.pagetable_dying_op = 0; #endif return 0;
[PATCH 7/8] x86/shadow: don't open-code copy_domain_page()
Let's use the library-like function that we have.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -455,7 +455,6 @@ static void _sh_resync(struct vcpu *v, m
static void oos_hash_add(struct vcpu *v, mfn_t gmfn)
{
int i, idx, oidx, swap = 0;
-void *gptr, *gsnpptr;
mfn_t *oos = v->arch.paging.shadow.oos;
mfn_t *oos_snapshot = v->arch.paging.shadow.oos_snapshot;
struct oos_fixup *oos_fixup = v->arch.paging.shadow.oos_fixup;
@@ -488,11 +487,7 @@ static void oos_hash_add(struct vcpu *v,
if ( swap )
SWAP(oos_snapshot[idx], oos_snapshot[oidx]);
-gptr = map_domain_page(oos[oidx]);
-gsnpptr = map_domain_page(oos_snapshot[oidx]);
-memcpy(gsnpptr, gptr, PAGE_SIZE);
-unmap_domain_page(gptr);
-unmap_domain_page(gsnpptr);
+copy_domain_page(oos_snapshot[oidx], oos[oidx]);
}
/* Remove an MFN from the list of out-of-sync guest pagetables */
[PATCH 6/8] x86/shadow: adjust and move sh_type_to_size[]
Drop the SH_type_none entry - there are no allocation attempts with
this type, and there also shouldn't be any. Adjust the shadow_size()
alternative path to match that change. Also generalize two related
assertions.
While there move the entire table and the respective part of the comment
there to hvm.c, resulting in one less #ifdef. In the course of the
movement switch to using designated initializers.
Signed-off-by: Jan Beulich
---
In principle the SH_type_l2h_64_shadow entry could be dropped here as
well (for being a PV32-only thing), but I wasn't sure whether that would
be deemed okay right here.
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -838,44 +838,11 @@ sh_validate_guest_entry(struct vcpu *v,
* not contiguous in memory; functions for handling offsets into them are
* defined in shadow/multi.c (shadow_l1_index() etc.)
*
- * This table shows the allocation behaviour of the different modes:
- *
- * Xen paging 64b 64b 64b
- * Guest paging32b pae 64b
- * PV or HVM HVM HVM *
- * Shadow paging pae pae 64b
- *
- * sl1 size 8k 4k 4k
- * sl2 size16k 4k 4k
- * sl3 size --4k
- * sl4 size --4k
- *
* In HVM guests, the p2m table is built out of shadow pages, and we provide
* a function for the p2m management to steal pages, in max-order chunks, from
* the free pool.
*/
-#ifdef CONFIG_HVM
-const u8 sh_type_to_size[] = {
-1, /* SH_type_none */
-2, /* SH_type_l1_32_shadow */
-2, /* SH_type_fl1_32_shadow */
-4, /* SH_type_l2_32_shadow */
-1, /* SH_type_l1_pae_shadow */
-1, /* SH_type_fl1_pae_shadow */
-1, /* SH_type_l2_pae_shadow */
-1, /* SH_type_l1_64_shadow */
-1, /* SH_type_fl1_64_shadow */
-1, /* SH_type_l2_64_shadow */
-1, /* SH_type_l2h_64_shadow */
-1, /* SH_type_l3_64_shadow */
-1, /* SH_type_l4_64_shadow */
-1, /* SH_type_p2m_table */
-1, /* SH_type_monitor_table */
-1 /* SH_type_oos_snapshot */
-};
-#endif
-
/*
* Figure out the least acceptable quantity of shadow memory.
* The minimum memory requirement for always being able to free up a
@@ -1121,7 +1088,7 @@ mfn_t shadow_alloc(struct domain *d,
unsigned int i;
ASSERT(paging_locked_by_me(d));
-ASSERT(shadow_type != SH_type_none);
+ASSERT(pages);
perfc_incr(shadow_alloc);
if ( d->arch.paging.free_pages < pages )
@@ -1201,9 +1168,9 @@ void shadow_free(struct domain *d, mfn_t
perfc_incr(shadow_free);
shadow_type = sp->u.sh.type;
-ASSERT(shadow_type != SH_type_none);
ASSERT(sp->u.sh.head || (shadow_type > SH_type_max_shadow));
pages = shadow_size(shadow_type);
+ASSERT(pages);
pin_list = &d->arch.paging.shadow.pinned_shadows;
for ( i = 0; i < pages; i++ )
--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -33,6 +33,37 @@
#include "private.h"
+/*
+ * This table shows the allocation behaviour of the different modes:
+ *
+ * Xen paging 64b 64b 64b
+ * Guest paging32b pae 64b
+ * PV or HVM HVM HVM *
+ * Shadow paging pae pae 64b
+ *
+ * sl1 size 8k 4k 4k
+ * sl2 size16k 4k 4k
+ * sl3 size --4k
+ * sl4 size --4k
+ */
+const uint8_t sh_type_to_size[] = {
+[SH_type_l1_32_shadow] = 2,
+[SH_type_fl1_32_shadow] = 2,
+[SH_type_l2_32_shadow] = 4,
+[SH_type_l1_pae_shadow] = 1,
+[SH_type_fl1_pae_shadow] = 1,
+[SH_type_l2_pae_shadow] = 1,
+[SH_type_l1_64_shadow] = 1,
+[SH_type_fl1_64_shadow] = 1,
+[SH_type_l2_64_shadow] = 1,
+[SH_type_l2h_64_shadow] = 1,
+[SH_type_l3_64_shadow] = 1,
+[SH_type_l4_64_shadow] = 1,
+[SH_type_p2m_table] = 1,
+[SH_type_monitor_table] = 1,
+[SH_type_oos_snapshot] = 1,
+};
+
/**/
/* x86 emulator support for the shadow code
*/
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -363,7 +363,7 @@ shadow_size(unsigned int shadow_type)
return sh_type_to_size[shadow_type];
#else
ASSERT(shadow_type < SH_type_unused);
-return 1;
+return shadow_type != SH_type_none;
#endif
}
[PATCH 4/8] x86/paging: move and conditionalize flush_tlb() hook
The hook isn't mode dependent, hence it's misplaced in struct
paging_mode. (Or alternatively I see no reason why the alloc_page() and
free_page() hooks don't also live there.) Move it to struct
paging_domain.
The hook also is used for HVM guests only, so make respective pieces
conditional upon CONFIG_HVM.
While there also add __must_check to the hook declaration, as it's
imperative that callers deal with getting back "false".
While moving the shadow implementation, introduce a "curr" local
variable.
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -237,6 +237,11 @@ struct paging_domain {
void (*free_page)(struct domain *d, struct page_info *pg);
void (*update_paging_mode)(struct vcpu *v);
+
+#ifdef CONFIG_HVM
+/* Flush selected vCPUs TLBs. NULL for all. */
+bool __must_check (*flush_tlb)(const unsigned long *vcpu_bitmap);
+#endif
};
struct paging_vcpu {
--- a/xen/arch/x86/include/asm/paging.h
+++ b/xen/arch/x86/include/asm/paging.h
@@ -140,7 +140,6 @@ struct paging_mode {
#endif
void (*update_cr3)(struct vcpu *v, int do_locking,
bool noflush);
-bool (*flush_tlb )(const unsigned long *vcpu_bitmap);
unsigned int guest_levels;
@@ -300,6 +299,12 @@ static inline unsigned long paging_ga_to
page_order);
}
+/* Flush selected vCPUs TLBs. NULL for all. */
+static inline bool paging_flush_tlb(const unsigned long *vcpu_bitmap)
+{
+return current->domain->arch.paging.flush_tlb(vcpu_bitmap);
+}
+
#endif /* CONFIG_HVM */
/* Update all the things that are derived from the guest's CR3.
@@ -408,12 +413,6 @@ static always_inline unsigned int paging
return bits;
}
-/* Flush selected vCPUs TLBs. NULL for all. */
-static inline bool paging_flush_tlb(const unsigned long *vcpu_bitmap)
-{
-return paging_get_hostmode(current)->flush_tlb(vcpu_bitmap);
-}
-
#endif /* XEN_PAGING_H */
/*
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -445,6 +445,7 @@ static void hap_destroy_monitor_table(st
//
static void cf_check hap_update_paging_mode(struct vcpu *v);
+static bool cf_check flush_tlb(const unsigned long *vcpu_bitmap);
void hap_domain_init(struct domain *d)
{
@@ -458,6 +459,7 @@ void hap_domain_init(struct domain *d)
paging_log_dirty_init(d, &hap_ops);
d->arch.paging.update_paging_mode = hap_update_paging_mode;
+d->arch.paging.flush_tlb = flush_tlb;
}
/* return 0 for success, -errno for failure */
@@ -847,7 +849,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_real_mode,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_real_mode,
.update_cr3 = hap_update_cr3,
-.flush_tlb = flush_tlb,
.guest_levels = 1
};
@@ -857,7 +858,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_2_levels,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_2_levels,
.update_cr3 = hap_update_cr3,
-.flush_tlb = flush_tlb,
.guest_levels = 2
};
@@ -867,7 +867,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_3_levels,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_3_levels,
.update_cr3 = hap_update_cr3,
-.flush_tlb = flush_tlb,
.guest_levels = 3
};
@@ -877,7 +876,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_4_levels,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_4_levels,
.update_cr3 = hap_update_cr3,
-.flush_tlb = flush_tlb,
.guest_levels = 4
};
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -68,6 +68,7 @@ int shadow_domain_init(struct domain *d)
d->arch.paging.shadow.oos_active = 0;
#endif
#ifdef CONFIG_HVM
+d->arch.paging.flush_tlb = shadow_flush_tlb;
d->arch.paging.shadow.pagetable_dying_op = 0;
#endif
@@ -3134,66 +3135,6 @@ static void cf_check sh_clean_dirty_bitm
paging_unlock(d);
}
-
-static bool flush_vcpu(const struct vcpu *v, const unsigned long *vcpu_bitmap)
-{
-return !vcpu_bitmap || test_bit(v->vcpu_id, vcpu_bitmap);
-}
-
-/* Flush TLB of selected vCPUs. NULL for all. */
-bool cf_check shadow_flush_tlb(const unsigned long *vcpu_bitmap)
-{
-static DEFINE_PER_CPU(cpumask_t, flush_cpumask);
-cpumask_t *mask = &this_cpu(flush_cpumask);
-struct domain *d = current->domain;
-struct vcpu *v;
-
-/* Avoid deadlock if more than one vcpu tries this at the same time. */
-if ( !spin_trylock(&d->hypercall_deadlock_mutex) )
-return false;
-
-/* Pause all other vcpus. */
-for_each_vcpu ( d, v )
-if ( v != current && flush_vcpu(v, vcpu_bitmap) )
-
[PATCH 3/8] x86/paging: move update_paging_modes() hook
The hook isn't mode dependent, hence it's misplaced in struct
paging_mode. (Or alternatively I see no reason why the alloc_page() and
free_page() hooks don't also live there.) Move it to struct
paging_domain.
While there rename the hook and HAP's as well as shadow's hook functions
to use singular; I never understood why plural was used. (Renaming in
particular the wrapper would be touching quite a lot of other code.)
Signed-off-by: Jan Beulich
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -235,6 +235,8 @@ struct paging_domain {
* (used by p2m and log-dirty code for their tries) */
struct page_info * (*alloc_page)(struct domain *d);
void (*free_page)(struct domain *d, struct page_info *pg);
+
+void (*update_paging_mode)(struct vcpu *v);
};
struct paging_vcpu {
--- a/xen/arch/x86/include/asm/paging.h
+++ b/xen/arch/x86/include/asm/paging.h
@@ -140,7 +140,6 @@ struct paging_mode {
#endif
void (*update_cr3)(struct vcpu *v, int do_locking,
bool noflush);
-void (*update_paging_modes )(struct vcpu *v);
bool (*flush_tlb )(const unsigned long *vcpu_bitmap);
unsigned int guest_levels;
@@ -316,7 +315,7 @@ static inline void paging_update_cr3(str
* has changed, and when bringing up a VCPU for the first time. */
static inline void paging_update_paging_modes(struct vcpu *v)
{
-paging_get_hostmode(v)->update_paging_modes(v);
+v->domain->arch.paging.update_paging_mode(v);
}
#ifdef CONFIG_PV
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -443,6 +443,9 @@ static void hap_destroy_monitor_table(st
//
/* HAP DOMAIN LEVEL FUNCTIONS */
//
+
+static void cf_check hap_update_paging_mode(struct vcpu *v);
+
void hap_domain_init(struct domain *d)
{
static const struct log_dirty_ops hap_ops = {
@@ -453,6 +456,8 @@ void hap_domain_init(struct domain *d)
/* Use HAP logdirty mechanism. */
paging_log_dirty_init(d, &hap_ops);
+
+d->arch.paging.update_paging_mode = hap_update_paging_mode;
}
/* return 0 for success, -errno for failure */
@@ -772,7 +777,7 @@ hap_paging_get_mode(struct vcpu *v)
&hap_paging_protected_mode);
}
-static void cf_check hap_update_paging_modes(struct vcpu *v)
+static void cf_check hap_update_paging_mode(struct vcpu *v)
{
struct domain *d = v->domain;
unsigned long cr3_gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT;
@@ -842,7 +847,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_real_mode,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_real_mode,
.update_cr3 = hap_update_cr3,
-.update_paging_modes= hap_update_paging_modes,
.flush_tlb = flush_tlb,
.guest_levels = 1
};
@@ -853,7 +857,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_2_levels,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_2_levels,
.update_cr3 = hap_update_cr3,
-.update_paging_modes= hap_update_paging_modes,
.flush_tlb = flush_tlb,
.guest_levels = 2
};
@@ -864,7 +867,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_3_levels,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_3_levels,
.update_cr3 = hap_update_cr3,
-.update_paging_modes= hap_update_paging_modes,
.flush_tlb = flush_tlb,
.guest_levels = 3
};
@@ -875,7 +877,6 @@ static const struct paging_mode hap_pagi
.gva_to_gfn = hap_gva_to_gfn_4_levels,
.p2m_ga_to_gfn = hap_p2m_ga_to_gfn_4_levels,
.update_cr3 = hap_update_cr3,
-.update_paging_modes= hap_update_paging_modes,
.flush_tlb = flush_tlb,
.guest_levels = 4
};
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -45,6 +45,8 @@ static int cf_check sh_enable_log_dirty(
static int cf_check sh_disable_log_dirty(struct domain *);
static void cf_check sh_clean_dirty_bitmap(struct domain *);
+static void cf_check shadow_update_paging_mode(struct vcpu *);
+
/* Set up the shadow-specific parts of a domain struct at start of day.
* Called for every domain from arch_domain_create() */
int shadow_domain_init(struct domain *d)
@@ -60,6 +62,8 @@ int shadow_domain_init(struct domain *d)
/* Use shadow pagetables for log-dirty support */
paging_log_dirty_init(d, &sh_ops);
+d->arch.paging.update_paging_mode = shadow_update_paging_mode;
+
#if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)
d->arch.paging.shadow.oos_active = 0;
#endif
@@ -2558,7 +2562,12 @@ static void sh_update_paging_modes(struc
v->arch.paging.
[PATCH 2/8] x86/paging: fold most HAP and shadow final teardown
HAP does a few things beyond what's common, which are left there at
least for now. Common operations, however, are moved to
paging_final_teardown(), allowing shadow_final_teardown() to go away.
While moving (and hence generalizing) the respective SHADOW_PRINTK()
drop the logging of total_pages from the 2nd instance - the value is
necessarily zero after {hap,shadow}_set_allocation().
Signed-off-by: Jan Beulich
---
The remaining parts of hap_final_teardown() could be moved as well, at
the price of a CONFIG_HVM conditional. I wasn't sure whether that was
deemed reasonable.
--- a/xen/arch/x86/include/asm/shadow.h
+++ b/xen/arch/x86/include/asm/shadow.h
@@ -78,9 +78,6 @@ int shadow_domctl(struct domain *d,
void shadow_vcpu_teardown(struct vcpu *v);
void shadow_teardown(struct domain *d, bool *preempted);
-/* Call once all of the references to the domain have gone away */
-void shadow_final_teardown(struct domain *d);
-
void sh_remove_shadows(struct domain *d, mfn_t gmfn, int fast, int all);
/* Adjust shadows ready for a guest page to change its type. */
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -268,8 +268,8 @@ static void hap_free(struct domain *d, m
/*
* For dying domains, actually free the memory here. This way less work is
- * left to hap_final_teardown(), which cannot easily have preemption checks
- * added.
+ * left to paging_final_teardown(), which cannot easily have preemption
+ * checks added.
*/
if ( unlikely(d->is_dying) )
{
@@ -552,18 +552,6 @@ void hap_final_teardown(struct domain *d
for (i = 0; i < MAX_NESTEDP2M; i++) {
p2m_teardown(d->arch.nested_p2m[i], true, NULL);
}
-
-if ( d->arch.paging.total_pages != 0 )
-hap_teardown(d, NULL);
-
-p2m_teardown(p2m_get_hostp2m(d), true, NULL);
-/* Free any memory that the p2m teardown released */
-paging_lock(d);
-hap_set_allocation(d, 0, NULL);
-ASSERT(d->arch.paging.p2m_pages == 0);
-ASSERT(d->arch.paging.free_pages == 0);
-ASSERT(d->arch.paging.total_pages == 0);
-paging_unlock(d);
}
void hap_vcpu_teardown(struct vcpu *v)
--- a/xen/arch/x86/mm/paging.c
+++ b/xen/arch/x86/mm/paging.c
@@ -842,10 +842,46 @@ int paging_teardown(struct domain *d)
/* Call once all of the references to the domain have gone away */
void paging_final_teardown(struct domain *d)
{
-if ( hap_enabled(d) )
+bool hap = hap_enabled(d);
+
+PAGING_PRINTK("%pd final teardown starts. Pages total = %u, free = %u,
p2m = %u\n",
+ d, d->arch.paging.total_pages,
+ d->arch.paging.free_pages, d->arch.paging.p2m_pages);
+
+if ( hap )
hap_final_teardown(d);
+
+/*
+ * Double-check that the domain didn't have any paging memory.
+ * It is possible for a domain that never got domain_kill()ed
+ * to get here with its paging allocation intact.
+ */
+if ( d->arch.paging.total_pages )
+{
+if ( hap )
+hap_teardown(d, NULL);
+else
+shadow_teardown(d, NULL);
+}
+
+/* It is now safe to pull down the p2m map. */
+p2m_teardown(p2m_get_hostp2m(d), true, NULL);
+
+/* Free any paging memory that the p2m teardown released. */
+paging_lock(d);
+
+if ( hap )
+hap_set_allocation(d, 0, NULL);
else
-shadow_final_teardown(d);
+shadow_set_allocation(d, 0, NULL);
+
+PAGING_PRINTK("%pd final teardown done. Pages free = %u, p2m = %u\n",
+ d, d->arch.paging.free_pages, d->arch.paging.p2m_pages);
+ASSERT(!d->arch.paging.p2m_pages);
+ASSERT(!d->arch.paging.free_pages);
+ASSERT(!d->arch.paging.total_pages);
+
+paging_unlock(d);
p2m_final_teardown(d);
}
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1232,7 +1232,7 @@ void shadow_free(struct domain *d, mfn_t
/*
* For dying domains, actually free the memory here. This way less
- * work is left to shadow_final_teardown(), which cannot easily have
+ * work is left to paging_final_teardown(), which cannot easily have
* preemption checks added.
*/
if ( unlikely(dying) )
@@ -2940,35 +2940,6 @@ out:
}
}
-void shadow_final_teardown(struct domain *d)
-/* Called by arch_domain_destroy(), when it's safe to pull down the p2m map. */
-{
-SHADOW_PRINTK("dom %u final teardown starts."
- " Shadow pages total = %u, free = %u, p2m=%u\n",
- d->domain_id, d->arch.paging.total_pages,
- d->arch.paging.free_pages, d->arch.paging.p2m_pages);
-
-/* Double-check that the domain didn't have any shadow memory.
- * It is possible for a domain that never got domain_kill()ed
- * to get here with its shadow allocation intact. */
-if ( d->arch.paging.total_pages != 0 )
-shadow_teardown(d, NULL);
-
-/* It is now safe to pull down the p2m map. */
-p2m
[PATCH 1/8] x86/paging: fold HAP and shadow memory alloc related fields
Especially with struct shadow_domain and struct hap_domain not living in
a union inside struct paging_domain, let's avoid the duplication: The
fields are named and used in identical ways, and only one of HAP or
shadow can be in use for a domain. This then also renders involved
expressions slightly more legible.
Signed-off-by: Jan Beulich
---
Quite likely more folding of code is possible with this. For example
{hap,shadow}_set_allocation() are now yet more similar than they already
were.
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -99,12 +99,6 @@ struct shadow_domain {
unsigned int opt_flags;/* runtime tunable optimizations on/off */
struct page_list_head pinned_shadows;
-/* Memory allocation */
-struct page_list_head freelist;
-unsigned int total_pages; /* number of pages allocated */
-unsigned int free_pages; /* number of pages on freelists */
-unsigned int p2m_pages;/* number of pages allocated to p2m */
-
/* 1-to-1 map for use when HVM vcpus have paging disabled */
pagetable_t unpaged_pagetable;
@@ -179,10 +173,6 @@ struct shadow_vcpu {
/*hardware assisted paging */
//
struct hap_domain {
-struct page_list_head freelist;
-unsigned int total_pages; /* number of pages allocated */
-unsigned int free_pages; /* number of pages on freelists */
-unsigned int p2m_pages;/* number of pages allocated to p2m */
};
//
@@ -218,6 +208,13 @@ struct paging_domain {
struct shadow_domainshadow;
/* extension for hardware-assited paging */
struct hap_domain hap;
+
+/* Memory allocation (common to shadow and HAP) */
+struct page_list_head freelist;
+unsigned inttotal_pages; /* number of pages allocated */
+unsigned intfree_pages; /* number of pages on freelists */
+unsigned intp2m_pages;/* number of pages allocated to p2m
*/
+
/* log dirty support */
struct log_dirty_domain log_dirty;
--- a/xen/arch/x86/include/asm/hap.h
+++ b/xen/arch/x86/include/asm/hap.h
@@ -47,7 +47,6 @@ int hap_track_dirty_vram(struct domain
extern const struct paging_mode *hap_paging_get_mode(struct vcpu *);
int hap_set_allocation(struct domain *d, unsigned int pages, bool *preempted);
unsigned int hap_get_allocation(struct domain *d);
-int hap_get_allocation_bytes(struct domain *d, uint64_t *size);
#endif /* XEN_HAP_H */
--- a/xen/arch/x86/include/asm/shadow.h
+++ b/xen/arch/x86/include/asm/shadow.h
@@ -97,8 +97,6 @@ void shadow_blow_tables_per_domain(struc
int shadow_set_allocation(struct domain *d, unsigned int pages,
bool *preempted);
-int shadow_get_allocation_bytes(struct domain *d, uint64_t *size);
-
#else /* !CONFIG_SHADOW_PAGING */
#define shadow_vcpu_teardown(v) ASSERT(is_pv_vcpu(v))
@@ -110,8 +108,6 @@ int shadow_get_allocation_bytes(struct d
({ ASSERT_UNREACHABLE(); -EOPNOTSUPP; })
#define shadow_set_allocation(d, pages, preempted) \
({ ASSERT_UNREACHABLE(); -EOPNOTSUPP; })
-#define shadow_get_allocation_bytes(d, size) \
-({ ASSERT_UNREACHABLE(); -EOPNOTSUPP; })
static inline void sh_remove_shadows(struct domain *d, mfn_t gmfn,
int fast, int all) {}
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -249,11 +249,11 @@ static struct page_info *hap_alloc(struc
if ( unlikely(d->is_dying) )
return NULL;
-pg = page_list_remove_head(&d->arch.paging.hap.freelist);
+pg = page_list_remove_head(&d->arch.paging.freelist);
if ( unlikely(!pg) )
return NULL;
-d->arch.paging.hap.free_pages--;
+d->arch.paging.free_pages--;
clear_domain_page(page_to_mfn(pg));
@@ -274,12 +274,12 @@ static void hap_free(struct domain *d, m
if ( unlikely(d->is_dying) )
{
free_domheap_page(pg);
-d->arch.paging.hap.total_pages--;
+d->arch.paging.total_pages--;
return;
}
-d->arch.paging.hap.free_pages++;
-page_list_add_tail(pg, &d->arch.paging.hap.freelist);
+d->arch.paging.free_pages++;
+page_list_add_tail(pg, &d->arch.paging.freelist);
}
static struct page_info *cf_check hap_alloc_p2m_page(struct domain *d)
@@ -293,8 +293,8 @@ static struct page_info *cf_check hap_al
if ( likely(pg != NULL) )
{
-d->arch.paging.hap.total_pages--;
-d->arch.paging.hap.p2m_pages++;
+d->arch.paging.total_pages--;
+d->arch.paging.p2m_pages++;
ASSERT(!page_get_owner(pg) && !(pg->count_info & PGC_count_mask));
}
else if ( !d->arch.paging.p2m_alloc_failed && !d->is_dying )
@@ -328,8 +328,8 @@ static void cf_check hap_free_p2m_page(s
pg->count_info &= ~PGC_count_mask;
page_set_owner(pg, NULL);
}
-d->a
[PATCH 0/8] x86/mm: address aspects noticed during XSA-410 work
Various tidying changes accumulated during that effort. They're all functionally independent afaict, but there may be contextual interactions between some of them. 1: paging: fold HAP and shadow memory alloc related fields 2: paging: fold most HAP and shadow final teardown 3: paging: move update_paging_modes() hook 4: paging: move and conditionalize flush_tlb() hook 5: PV: drop dead paging_update_paging_modes() call during Dom0 construction 6: shadow: adjust and move sh_type_to_size[] 7: shadow: don't open-code copy_domain_page() 8: shadow: drop zero initialization from shadow_domain_init() Jan
[qemu-mainline test] 175430: regressions - trouble: broken/fail/pass
flight 175430 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/175430/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-xsm broken test-amd64-amd64-xl-qemuu-ovmf-amd64 broken test-amd64-amd64-xl-qemuu-debianhvm-i386-xsmbroken test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadowbroken test-amd64-amd64-xl-qemuu-debianhvm-amd64 broken test-amd64-amd64-xl broken test-amd64-amd64-xl 5 host-install(5)broken REGR. vs. 175394 test-amd64-amd64-xl-qemuu-debianhvm-amd64 5 host-install(5) broken REGR. vs. 175394 test-amd64-amd64-xl-qemuu-ovmf-amd64 5 host-install(5) broken REGR. vs. 175394 test-amd64-amd64-xl-xsm 5 host-install(5)broken REGR. vs. 175394 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 5 host-install(5) broken REGR. vs. 175394 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 5 host-install(5) broken REGR. vs. 175394 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 175394 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175394 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175394 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175394 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175394 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175394 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175394 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175394 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175394 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-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-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail
[xen-4.16-testing test] 175427: regressions - trouble: broken/fail/pass
flight 175427 xen-4.16-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/175427/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemut-debianhvm-i386-xsm broken test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsmbroken test-amd64-i386-xl-vhd broken test-amd64-i386-xl-xsm broken test-amd64-i386-xl-qemuu-ovmf-amd64 broken test-amd64-i386-freebsd10-amd64 broken test-amd64-i386-xl-vhd5 host-install(5)broken REGR. vs. 175155 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 5 host-install(5) broken REGR. vs. 175155 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 5 host-install(5) broken REGR. vs. 175155 test-amd64-i386-xl-xsm5 host-install(5)broken REGR. vs. 175155 test-amd64-i386-freebsd10-amd64 5 host-install(5) broken REGR. vs. 175155 test-amd64-i386-xl-qemuu-ovmf-amd64 5 host-install(5) broken REGR. vs. 175155 test-amd64-i386-livepatch 7 xen-install fail REGR. vs. 175155 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 175155 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 175155 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 175155 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 175155 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 175155 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 175155 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 175155 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 175155 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 175155 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 175155 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 175155 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 175155 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-raw 14 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail never pass test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pa
Re: [RFC 7/7] x86/dpci: move hvm_dpci_isairq_eoi() to generic HVM code
On 21.12.2022 12:13, Xenia Ragiadakou wrote: > > On 12/21/22 12:19, Jan Beulich wrote: >> On 19.12.2022 07:34, Xenia Ragiadakou wrote: >>> The function hvm_dpci_isairq_eoi() has no dependencies on VT-d driver code >>> and can be moved from xen/drivers/passthrough/vtd/x86/hvm.c to >>> xen/drivers/passthrough/x86/hvm.c. >>> >>> Remove the now empty xen/drivers/passthrough/vtd/x86/hvm.c. >>> >>> Since the funcion is hvm specific, move its declaration from xen/iommu.h >>> to asm/hvm/io.h. >> >> While everything else looks good here, I question this part: The function >> is both HVM- and IOMMU-specific. However, by moving the definition ... >> >>> No functional change intended. >>> >>> Signed-off-by: Xenia Ragiadakou >>> --- >>> >>> I was not sure how to handle the copyright. I assume that I have to >>> retain the copyright of Weidong Han , right? >>> >>> xen/arch/x86/include/asm/hvm/io.h| 1 + >>> xen/drivers/passthrough/vtd/x86/Makefile | 1 - >>> xen/drivers/passthrough/vtd/x86/hvm.c| 64 >>> xen/drivers/passthrough/x86/hvm.c| 42 >> >> ... here, you don't need a declaration anymore anyway - the function can >> simply become static then, as its only caller lives in this very file. > > I will change it to static. > > Regarding the copyright, shall I add the copyright of Weidong Han > , that was in the deleted file? Strictly speaking you probably ought to, yes. Jan
Re: [RFC 6/7] x86/iommu: call pi_update_irte through an hvm_function callback
On 21.12.2022 12:09, Xenia Ragiadakou wrote: > > On 12/21/22 12:13, Jan Beulich wrote: >> On 19.12.2022 07:34, Xenia Ragiadakou wrote: >>> @@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v, >>> alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs); >>> } >>> >>> +static inline int hvm_pi_update_irte(const struct vcpu *v, >>> + const struct pirq *pirq, uint8_t gvec) >> >> Why "int" as return type when both call sites ignore the return value? > > Because the original function returned int. Hmm, indeed - looking more closely there can actually be errors, and those shouldn't really be ignored in all cases. At the very least an assertion would seem on order. > I 'm not sure though why the returned value is ignored. Kevin, thoughts? Jan
Re: [RFC 7/7] x86/dpci: move hvm_dpci_isairq_eoi() to generic HVM code
On 12/21/22 12:19, Jan Beulich wrote: On 19.12.2022 07:34, Xenia Ragiadakou wrote: The function hvm_dpci_isairq_eoi() has no dependencies on VT-d driver code and can be moved from xen/drivers/passthrough/vtd/x86/hvm.c to xen/drivers/passthrough/x86/hvm.c. Remove the now empty xen/drivers/passthrough/vtd/x86/hvm.c. Since the funcion is hvm specific, move its declaration from xen/iommu.h to asm/hvm/io.h. While everything else looks good here, I question this part: The function is both HVM- and IOMMU-specific. However, by moving the definition ... No functional change intended. Signed-off-by: Xenia Ragiadakou --- I was not sure how to handle the copyright. I assume that I have to retain the copyright of Weidong Han , right? xen/arch/x86/include/asm/hvm/io.h| 1 + xen/drivers/passthrough/vtd/x86/Makefile | 1 - xen/drivers/passthrough/vtd/x86/hvm.c| 64 xen/drivers/passthrough/x86/hvm.c| 42 ... here, you don't need a declaration anymore anyway - the function can simply become static then, as its only caller lives in this very file. I will change it to static. Regarding the copyright, shall I add the copyright of Weidong Han , that was in the deleted file? Jan -- Xenia
Re: [RFC 6/7] x86/iommu: call pi_update_irte through an hvm_function callback
On 12/21/22 12:13, Jan Beulich wrote:
On 19.12.2022 07:34, Xenia Ragiadakou wrote:
@@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v,
alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs);
}
+static inline int hvm_pi_update_irte(const struct vcpu *v,
+ const struct pirq *pirq, uint8_t gvec)
Why "int" as return type when both call sites ignore the return value?
Because the original function returned int. I 'm not sure though why the
returned value is ignored.
@@ -893,6 +908,13 @@ static inline void hvm_set_reg(struct vcpu *v, unsigned
int reg, uint64_t val)
ASSERT_UNREACHABLE();
}
+static inline int hvm_pi_update_irte(const struct vcpu *v,
+ const struct pirq *pirq, uint8_t gvec)
+{
+ASSERT_UNREACHABLE();
+return -EOPNOTSUPP;
+}
I don't think you need this stub - both callers live in a file which
is built only for HVM=y anyway.
That's true. I will remove it.
Jan
--
Xenia
[ovmf test] 175438: regressions - FAIL
flight 175438 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/175438/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 175202 test-amd64-amd64-xl-qemuu-ovmf-amd64 12 debian-hvm-install fail REGR. vs. 175202 version targeted for testing: ovmf 451521ccbcaa45de27fbcd2565cb363fd05e3661 baseline version: ovmf d103840cfb559c28831c2635b916d60118f671cc Last test of basis 175202 2022-12-14 13:42:59 Z6 days Failing since175214 2022-12-14 18:42:16 Z6 days 40 attempts Testing same since 175438 2022-12-21 08:14:00 Z0 days1 attempts People who touched revisions under test: Abner Chang Adam Dunlap Ard Biesheuvel Chun-Yi Lee Chun-Yi Lee de...@edk2.groups.io Dov Murik Gerd Hoffmann jdzhang Jeff Brasen Jeshua Smith Jian J Wang Jiaqi Gao Jiewen Yao Judah Vang Kavya Kuo, Ted MarsX Lin Matt DeVillier Michael D Kinney Michael Kubacki Min M Xu Min Xu Nishant C Mistry Ray Ni Rebecca Cran Rebecca Cran Sean Rhodes Sebastien Boeuf Ted Kuo Tom Lendacky Xie, Yuanhao Yuanhao Xie 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 fail test-amd64-i386-xl-qemuu-ovmf-amd64 fail 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 1162 lines long.)
Re: [RFC 1/7] x86/iommu: make AMD-Vi and Intel VT-d support configurable
On 12/21/22 09:51, Jan Beulich wrote: On 19.12.2022 07:34, Xenia Ragiadakou wrote: --- a/xen/drivers/passthrough/Kconfig +++ b/xen/drivers/passthrough/Kconfig @@ -37,6 +37,22 @@ config IPMMU_VMSA endif +config AMD_IOMMU + bool "AMD IOMMU" + depends on X86 + default y + ---help--- + Enables I/O virtualization on platforms that implement the + AMD I/O Virtualization Technology (IOMMU). + +config INTEL_VTD + bool "Intel VT-d" + depends on X86 + default y + ---help--- + Enables I/O virtualization on platforms that implement the + Intel Virtualization Technology for Directed I/O (Intel VT-d). One more thing Andrew and I have been talking about: As he has mentioned elsewhere, IOMMU support is needed to boot systems with more than 254 CPUs (depending on APIC ID layout the boundary may actually be lower). Hence it needs to at least be considered to make the prompts here (to be precise: in the much later patch adding the prompts) dependent on EXPERT, to prevent people from unknowingly building a non-functioning (on some systems) hypervisor. I will mention it in help as Andrew suggested and I will make it visible only if EXPERT. Jan -- Xenia
Re: [PATCH 03/22] acpi: vmap pages in acpi_os_alloc_memory
Hi Jan,
On 20/12/2022 15:15, Jan Beulich wrote:
On 16.12.2022 12:48, Julien Grall wrote:
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -244,6 +244,11 @@ void *vmap(const mfn_t *mfn, unsigned int nr)
return __vmap(mfn, 1, nr, 1, PAGE_HYPERVISOR, VMAP_DEFAULT);
}
+void *vmap_contig_pages(mfn_t mfn, unsigned int nr_pages)
I don't think the _pages suffix buys us much here. I also think parameter
names would better be consistent with other functions here, in particular
with vmap() (i.e. s/nr_pages/nr/).
I will do the renaming.
--- a/xen/drivers/acpi/osl.c
+++ b/xen/drivers/acpi/osl.c
@@ -221,7 +221,11 @@ void *__init acpi_os_alloc_memory(size_t sz)
void *ptr;
if (system_state == SYS_STATE_early_boot)
- return mfn_to_virt(mfn_x(alloc_boot_pages(PFN_UP(sz), 1)));
+ {
+ mfn_t mfn = alloc_boot_pages(PFN_UP(sz), 1);
+
+ return vmap_contig_pages(mfn, PFN_UP(sz));
+ }
Multiple pages may be allocated here, yet ...
@@ -246,5 +250,10 @@ void __init acpi_os_free_memory(void *ptr)
if (is_xmalloc_memory(ptr))
xfree(ptr);
else if (ptr && system_state == SYS_STATE_early_boot)
- init_boot_pages(__pa(ptr), __pa(ptr) + PAGE_SIZE);
+ {
+ paddr_t addr = mfn_to_maddr(vmap_to_mfn(ptr));
+
+ vunmap(ptr);
+ init_boot_pages(addr, addr + PAGE_SIZE);
+ }
... (as before) only one page would be freed here. With the move to
vmap() it ought to be possible to do better now. (If you want to
defer this to a later patch, please at least mention the aspect in
the description.)
Good point, I will have a look to solve it in this patch.
Cheers,
--
Julien Grall
Re: [PATCH 02/22] x86/setup: move vm_init() before acpi calls
On 21.12.2022 11:18, Julien Grall wrote:
> On 20/12/2022 15:08, Jan Beulich wrote:
>> On 16.12.2022 12:48, Julien Grall wrote:
>>> --- a/xen/common/vmap.c
>>> +++ b/xen/common/vmap.c
>>> @@ -34,9 +34,20 @@ void __init vm_init_type(enum vmap_region type, void
>>> *start, void *end)
>>>
>>> for ( i = 0, va = (unsigned long)vm_bitmap(type); i < nr; ++i, va +=
>>> PAGE_SIZE )
>>> {
>>> -struct page_info *pg = alloc_domheap_page(NULL, 0);
>>> +mfn_t mfn;
>>> +int rc;
>>>
>>> -map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
>>> +if ( system_state == SYS_STATE_early_boot )
>>> +mfn = alloc_boot_pages(1, 1);
>>> +else
>>> +{
>>> +struct page_info *pg = alloc_domheap_page(NULL, 0);
>>> +
>>> +BUG_ON(!pg);
>>> +mfn = page_to_mfn(pg);
>>> +}
>>> +rc = map_pages_to_xen(va, mfn, 1, PAGE_HYPERVISOR);
>>> +BUG_ON(rc);
>>
>> The adding of a return value check is unrelated and not overly useful:
>>
>>> clear_page((void *)va);
>>
>> This will fault anyway if the mapping attempt failed.
>
> Not always. At least on Arm, map_pages_to_xen() could fail if the VA was
> mapped to another physical address.
Oh, okay.
> This seems unlikely, yet I think that relying on clear_page() to always
> fail when map_pages_to_xen() return an error is bogus.
Fair enough, but then please at least call out the change (and the reason)
in the description. Even better might be to make this a separate change,
but I wouldn't insist (quite likely I wouldn't have made this a separate
change either).
Jan
[xen-4.13-testing test] 175426: trouble: broken/fail/pass
flight 175426 xen-4.13-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/175426/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-livepatchbroken test-amd64-i386-xl-qemut-debianhvm-i386-xsm broken test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsmbroken test-amd64-i386-xl-shadowbroken test-amd64-i386-xl-xsm broken Tests which are failing intermittently (not blocking): test-amd64-i386-xl-qemut-debianhvm-i386-xsm 5 host-install(5) broken pass in 175412 test-amd64-i386-xl-xsm5 host-install(5) broken pass in 175412 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 5 host-install(5) broken pass in 175412 test-amd64-i386-livepatch 5 host-install(5) broken pass in 175412 test-amd64-i386-xl-shadow 5 host-install(5) broken pass in 175412 test-armhf-armhf-libvirt-qcow2 20 leak-check/check fail in 175412 pass in 175426 test-amd64-amd64-xl-shadow 18 guest-localmigrate fail pass in 175412 test-amd64-i386-xl-qemuu-debianhvm-amd64 18 guest-localmigrate/x10 fail pass in 175412 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 174675 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 174675 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 174675 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 174675 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 174675 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 174675 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check fail like 174675 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 174675 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 174675 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 174675 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 174675 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 174675 test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-arm64-arm64-xl 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-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-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-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-i386-libvirt-raw 14 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-raw 14 migrate-support-check
Re: [RFC 7/7] x86/dpci: move hvm_dpci_isairq_eoi() to generic HVM code
On 19.12.2022 07:34, Xenia Ragiadakou wrote: > The function hvm_dpci_isairq_eoi() has no dependencies on VT-d driver code > and can be moved from xen/drivers/passthrough/vtd/x86/hvm.c to > xen/drivers/passthrough/x86/hvm.c. > > Remove the now empty xen/drivers/passthrough/vtd/x86/hvm.c. > > Since the funcion is hvm specific, move its declaration from xen/iommu.h > to asm/hvm/io.h. While everything else looks good here, I question this part: The function is both HVM- and IOMMU-specific. However, by moving the definition ... > No functional change intended. > > Signed-off-by: Xenia Ragiadakou > --- > > I was not sure how to handle the copyright. I assume that I have to > retain the copyright of Weidong Han , right? > > xen/arch/x86/include/asm/hvm/io.h| 1 + > xen/drivers/passthrough/vtd/x86/Makefile | 1 - > xen/drivers/passthrough/vtd/x86/hvm.c| 64 > xen/drivers/passthrough/x86/hvm.c| 42 ... here, you don't need a declaration anymore anyway - the function can simply become static then, as its only caller lives in this very file. Jan
Re: [PATCH 02/22] x86/setup: move vm_init() before acpi calls
Hi Jan,
On 20/12/2022 15:08, Jan Beulich wrote:
On 16.12.2022 12:48, Julien Grall wrote:
From: Wei Liu
After the direct map removal, pages from the boot allocator are not
mapped at all in the direct map. Although we have map_domain_page, they
Nit: "will not be mapped" or "are not going to be mapped", or else this
sounds like there's a bug somewhere.
I will update.
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -870,6 +870,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
unsigned long eb_start, eb_end;
bool acpi_boot_table_init_done = false, relocated = false;
int ret;
+bool vm_init_done = false;
Can this please be grouped with the other bool-s (even visible in context)?
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -34,9 +34,20 @@ void __init vm_init_type(enum vmap_region type, void *start,
void *end)
for ( i = 0, va = (unsigned long)vm_bitmap(type); i < nr; ++i, va += PAGE_SIZE )
{
-struct page_info *pg = alloc_domheap_page(NULL, 0);
+mfn_t mfn;
+int rc;
-map_pages_to_xen(va, page_to_mfn(pg), 1, PAGE_HYPERVISOR);
+if ( system_state == SYS_STATE_early_boot )
+mfn = alloc_boot_pages(1, 1);
+else
+{
+struct page_info *pg = alloc_domheap_page(NULL, 0);
+
+BUG_ON(!pg);
+mfn = page_to_mfn(pg);
+}
+rc = map_pages_to_xen(va, mfn, 1, PAGE_HYPERVISOR);
+BUG_ON(rc);
The adding of a return value check is unrelated and not overly useful:
clear_page((void *)va);
This will fault anyway if the mapping attempt failed.
Not always. At least on Arm, map_pages_to_xen() could fail if the VA was
mapped to another physical address.
This seems unlikely, yet I think that relying on clear_page() to always
fail when map_pages_to_xen() return an error is bogus.
Cheers,
--
Julien Grall
Re: [RFC 6/7] x86/iommu: call pi_update_irte through an hvm_function callback
On 19.12.2022 07:34, Xenia Ragiadakou wrote:
> @@ -774,6 +779,16 @@ static inline void hvm_set_nonreg_state(struct vcpu *v,
> alternative_vcall(hvm_funcs.set_nonreg_state, v, nrs);
> }
>
> +static inline int hvm_pi_update_irte(const struct vcpu *v,
> + const struct pirq *pirq, uint8_t gvec)
Why "int" as return type when both call sites ignore the return value?
> @@ -893,6 +908,13 @@ static inline void hvm_set_reg(struct vcpu *v, unsigned
> int reg, uint64_t val)
> ASSERT_UNREACHABLE();
> }
>
> +static inline int hvm_pi_update_irte(const struct vcpu *v,
> + const struct pirq *pirq, uint8_t gvec)
> +{
> +ASSERT_UNREACHABLE();
> +return -EOPNOTSUPP;
> +}
I don't think you need this stub - both callers live in a file which
is built only for HVM=y anyway.
Jan
Re: [RFC 4/7] x86/acpi: separate AMD-Vi and VT-d specific functions
On 19.12.2022 07:34, Xenia Ragiadakou wrote: > The functions acpi_dmar_init() and acpi_dmar_zap/reinstate() are > VT-d specific while the function acpi_ivrs_init() is AMD-Vi specific. > To eliminate dead code, they need to be guarded under CONFIG_INTEL_VTD > and CONFIG_AMD_IOMMU, respectively. > > Instead of adding #ifdef guards around the function calls, implement them > as empty static inline functions. > > Take the opportunity to move the declarations of acpi_dmar_zap/reinstate() to > the arch specific header. > > No functional change intended. > > Signed-off-by: Xenia Ragiadakou Reviewed-by: Jan Beulich
[PATCH 2/2] public: misra rule 20.7 fix on memory.h
Cppcheck has found a violation of rule 20.7 for the macro XENMEM_SHARING_OP_FIELD_MAKE_GREF, the argument "val" is used in an expression, hence add parenthesis to the argument "val" to fix the violation. Signed-off-by: Luca Fancellu Acked-by: Jan Beulich --- xen/include/public/memory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index 29cf5c823902..c5f0d31e235d 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -485,7 +485,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_access_op_t); #define XENMEM_SHARING_OP_FIELD_IS_GREF_FLAG (xen_mk_ullong(1) << 62) #define XENMEM_SHARING_OP_FIELD_MAKE_GREF(field, val) \ -(field) = (XENMEM_SHARING_OP_FIELD_IS_GREF_FLAG | val) +(field) = (XENMEM_SHARING_OP_FIELD_IS_GREF_FLAG | (val)) #define XENMEM_SHARING_OP_FIELD_IS_GREF(field) \ ((field) & XENMEM_SHARING_OP_FIELD_IS_GREF_FLAG) #define XENMEM_SHARING_OP_FIELD_GET_GREF(field)\ -- 2.17.1
[PATCH 1/2] public: misra rule 20.7 fix on errno.h
Cppcheck has found a violation of rule 20.7 for the macro XEN_ERRNO,
while the macro parameter is never used as an expression, it doesn't
harm the code or the readability to add parenthesis, so add them.
This finding is reported also by eclair and coverity.
Signed-off-by: Luca Fancellu
Acked-by: Jan Beulich
---
xen/include/public/errno.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index 6bdc8c507990..5a78a7607c0d 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -31,7 +31,7 @@
#ifndef __ASSEMBLY__
-#define XEN_ERRNO(name, value) XEN_##name = value,
+#define XEN_ERRNO(name, value) XEN_##name = (value),
enum xen_errno {
#elif __XEN_INTERFACE_VERSION__ < 0x00040700
--
2.17.1
[PATCH 0/2] cppcheck rule 20.7 fixes
In this serie there are some fixes for the rule 20.7. The analysed build is arm64, to reproduce the reports here the command: ./xen/scripts/xen-analysis.py --cppcheck-misra --run-cppcheck -- CROSS_COMPILE="aarch64-linux-gnu-" XEN_TARGET_ARCH="arm64" O=/path/to/artifacts_folder Luca Fancellu (2): public: misra rule 20.7 fix on errno.h public: misra rule 20.7 fix on memory.h xen/include/public/errno.h | 2 +- xen/include/public/memory.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.17.1
Re: [RFC PATCH 09/18] xen: cppcheck: misra rule 20.7 deviation on kconfig.h
> On 20 Dec 2022, at 09:48, Jan Beulich wrote:
>
> On 20.12.2022 09:50, Luca Fancellu wrote:
>> Cppcheck has found a violation of rule 20.7 for the macro
>> __config_enabled but the preprocessor branch where this macro is
>> defined should not be analysed by cppcheck when CPPCHECK macro is
>> defined, hence this is a false positive of the tool and we can
>> safely suppress the finding.
>
> So what was commit 43aa3f6e72d3's ("xen/build: Add cppcheck and
> cppcheck-html make rules") adjustment to the file about then?
Yes the commit is right, cppcheck itself needs that, the problem here
comes from the misra add-on (the cppcheck component that does the
MISRA analysis) that is wrong.
Anyway I will drop this patch together with all the false-positive on the
Tool for now.
>
> Jan
>
[linux-linus test] 175424: regressions - trouble: broken/fail/pass
flight 175424 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/175424/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-shadow broken test-amd64-amd64-xl-qemuu-win7-amd64 broken test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm broken test-amd64-amd64-xl-qemut-debianhvm-i386-xsmbroken test-amd64-amd64-xl-qemut-debianhvm-amd64 broken test-amd64-amd64-xl-credit2 broken test-amd64-amd64-libvirt-pair broken test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm broken test-amd64-amd64-pairbroken test-amd64-amd64-xl-qemuu-win7-amd64 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-xl-credit2 5 host-install(5)broken REGR. vs. 173462 test-amd64-amd64-xl-qemut-debianhvm-amd64 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-pair 7 host-install/dst_host(7) broken REGR. vs. 173462 test-amd64-amd64-xl-shadow5 host-install(5)broken REGR. vs. 173462 test-amd64-amd64-libvirt-pair 7 host-install/dst_host(7) broken REGR. vs. 173462 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 5 host-install(5) broken REGR. vs. 173462 test-amd64-amd64-xl-qemuu-ws16-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-coresched-amd64-xl 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-xl-qemut-win7-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-qemuu-nested-amd 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-qemuu-nested-intel 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-pvhv2-amd 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-xl-qemut-ws16-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-debianhvm-amd64 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-freebsd11-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-freebsd12-amd64 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-multivcpu 8 xen-bootfail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-pvhv2-intel 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-pvshim8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-libvirt 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-libvirt-qcow2 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-vhd 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-dom0pvh-xl-amd 14 guest-start fail REGR. vs. 173462 test-arm64-arm64-examine 8 reboot fail REGR. vs. 173462 test-amd64-amd64-dom0pvh-xl-intel 14 guest-start fail REGR. vs. 173462 test-arm64-arm64-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-xsm 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-libvirt-xsm 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-xl-qemuu-ovmf-amd64 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl-credit2 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-xl 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-examine 8 reboot fail REGR. vs. 173462 test-amd64-amd64-xl-credit1 8 xen-boot fail REGR. vs. 173462 test-armhf-armhf-libvirt 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-examine-uefi 8 reboot fail REGR. vs. 173462 test-amd64-amd64-examine-bios 8 reboot fail REGR. vs. 173462 test-amd64-amd64-examine 8 reboot fail REGR. vs. 173462 test-amd64-amd64-pygrub 8 xen-boot fail REGR. vs. 173462 test-amd64-amd64-libvirt-raw 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-xsm 8 xen-boot fail REGR. vs. 173462 test-arm64-arm64-xl-seattle 8 xen-boot fail REGR. vs. 173462 test-arm6
Re: [RFC PATCH 17/18] public: misra rule 20.7 deviation on errno.h
On 21.12.2022 09:46, Luca Fancellu wrote: >> On 20 Dec 2022, at 09:51, Jan Beulich wrote: >> On 20.12.2022 09:50, Luca Fancellu wrote: >>> Cppcheck has found a violation of rule 20.7 for the macro XEN_ERRNO, >>> while the macro parameter is never used as an expression, it doesn't >>> harm the code or the readability to add parenthesis, so add them. >>> >>> This finding is reported also by eclair and coverity. >>> >>> Signed-off-by: Luca Fancellu >> >> Acked-by: Jan Beulich >> >> But with the title adjusted - this isn't about a deviation, but actually >> addressing a finding. > > Is it ok this title: > > public: misra rule 20.7 fix on errno.h Sure. Jan
Re: [RFC PATCH 00/18] cppcheck rule 20.7 fixes
> On 20 Dec 2022, at 09:55, Jan Beulich wrote: > > On 20.12.2022 09:50, Luca Fancellu wrote: >> In this serie there are some fixes for the rule 20.7, mainly violation found >> by >> cppcheck, most of them are false positive but some of them can be fixed. >> >> The analysed build is arm64, to reproduce the reports here the command: >> >> ./xen/scripts/xen-analysis.py --cppcheck-misra --run-cppcheck -- >> CROSS_COMPILE="aarch64-linux-gnu-" XEN_TARGET_ARCH="arm64" >> O=/path/to/artifacts_folder >> >> Luca Fancellu (18): >> arm: cppcheck: misra rule 20.7 deviations for alternative.h >> arm: cppcheck: misra rule 20.7 deviation on processor.h >> arm: cppcheck: misra rule 20.7 deviation on asm_defns.h >> arm: cppcheck: misra rule 20.7 deviation on config.h >> arm: cppcheck: fix misra rule 20.7 on arm/include/asm/string.h >> public: cppcheck: misra rule 20.7 on public/arch-arm.h >> xen: cppcheck: misra rule 20.7 deviation on compiler.h >> xen: cppcheck: misra rule 20.7 deviation on init.h >> xen: cppcheck: misra rule 20.7 deviation on kconfig.h >> xen: cppcheck: misra rule 20.7 deviation on types.h >> xen: cppcheck: misra rule 20.7 deviation on xmalloc.h >> arm: cppcheck: misra rule 20.7 deviation on asm/arm64/sysregs.h >> public/x86: cppcheck: misra rule 20.7 deviation on hvm/save.h >> public/x86: cppcheck: misra rule 20.7 deviation on xen-x86_32.h >> public/x86: cppcheck: misra rule 20.7 deviation on xen-x86_64.h >> public/x86: cppcheck: misra rule 20.7 deviation on arch-x86/xen.h >> public: misra rule 20.7 deviation on errno.h >> public: misra rule 20.7 deviation on memory.h > > Like Julien I object to the massive addition of false positive markers > just because of very basic shortcomings in cppcheck. I find this > particularly bad in public headers - imo no such annotations should > appear there at all. I would suggest that you split off the actual > code changes, which are likely going to be less controversial. Yes I will send the patches with your review and drop the others with False-positive or fixes that are not agreed. > > Jan
Re: [RFC PATCH 17/18] public: misra rule 20.7 deviation on errno.h
> On 20 Dec 2022, at 09:51, Jan Beulich wrote: > > On 20.12.2022 09:50, Luca Fancellu wrote: >> Cppcheck has found a violation of rule 20.7 for the macro XEN_ERRNO, >> while the macro parameter is never used as an expression, it doesn't >> harm the code or the readability to add parenthesis, so add them. >> >> This finding is reported also by eclair and coverity. >> >> Signed-off-by: Luca Fancellu > > Acked-by: Jan Beulich > > But with the title adjusted - this isn't about a deviation, but actually > addressing a finding. Is it ok this title: public: misra rule 20.7 fix on errno.h > > Jan
