[Xen-devel] [PATCH v3 09/28] xen/arm32: head: Mark the end of subroutines with ENDPROC
putn() and puts() are two subroutines. Add ENDPROC for the benefits of static analysis tools and the reader. Signed-off-by: Julien Grall Acked-by: Stefano Stabellini --- Changes in v3: - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 99f4af18d8..8b4c8a4714 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -518,6 +518,7 @@ puts: moveq pc, lr early_uart_transmit r11, r1 b puts +ENDPROC(puts) /* * Print a 32-bit number in hex. Specific to the PL011 UART. @@ -537,6 +538,7 @@ putn: subs r3, r3, #1 bne 1b mov pc, lr +ENDPROC(putn) hex:.ascii "0123456789abcdef" .align 2 -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/spec-ctrl: Scrub stale segment registers on leaky hardware
On 12/08/2019 09:00, Jan Beulich wrote: > On 09.08.2019 19:16, Andrew Cooper wrote: >> --- a/docs/misc/xen-command-line.pandoc >> +++ b/docs/misc/xen-command-line.pandoc >> @@ -1914,7 +1914,7 @@ By default SSBD will be mitigated at runtime >> (i.e `ssbd=runtime`). >> ### spec-ctrl (x86) >> > `= List of [ , xen=, >> {pv,hvm,msr-sc,rsb,md-clear}=, >> > bti-thunk=retpoline|lfence|jmp, >> {ibrs,ibpb,ssbd,eager-fpu, >> -> l1d-flush,l1tf-barrier}= ]` >> +> l1d-flush,stale-seg-clear,l1tf-barrier}= ]` >> Controls for speculative execution sidechannel mitigations. By >> default, Xen >> will pick the most appropriate mitigations based on compiled in >> support, >> @@ -1986,6 +1986,12 @@ Irrespective of Xen's setting, the feature is >> virtualised for HVM guests to >> use. By default, Xen will enable this mitigation on hardware >> believed to be >> vulnerable to L1TF. >> +On all hardware, the `stale-seg-clear=` option can be used to >> force or prevent >> +Xen from clearing the microarchitectural segment register copies on >> context >> +switch. By default, Xen will choose to use stale segment clearing >> on affected >> +hardware. The clearing logic is tuned to microarchitectural details >> of the >> +affected CPUs. >> + >> On hardware vulnerable to L1TF, the `l1tf-barrier=` option can be >> used to force >> or prevent Xen from protecting evaluations inside the hypervisor >> with a barrier >> instruction to not load potentially secret information into L1 >> cache. By > > Purely out of curiosity: Is there a reason both insertions happen between > two pre-existing sub-options, not after all of them? Easier backportability. That and l1tf-barrier is not going to say in this form by the time 4.13 gets released. > >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -1328,6 +1328,29 @@ static void load_segments(struct vcpu *n) >> dirty_segment_mask = per_cpu(dirty_segment_mask, cpu); >> per_cpu(dirty_segment_mask, cpu) = 0; >> + /* >> + * CPUs which suffer from stale segment register leakage have >> two copies >> + * of each segment register [Correct at the time of writing - >> Aug 2019]. >> + * >> + * We must write to both of them to scrub state from the >> previous vcpu. >> + * However, two writes in quick succession stall the pipeline, >> as only one >> + * write per segment can be speculatively outstanding. >> + * >> + * Split the two sets of writes to each register to maximise the >> chance >> + * that these writes have retired before the second set starts, >> thus >> + * reducing the chance of stalling. >> + */ >> + if ( opt_stale_seg_clear ) >> + { >> + asm volatile ( "mov %[sel], %%ds;" >> + "mov %[sel], %%es;" >> + "mov %[sel], %%fs;" >> + "mov %[sel], %%gs;" >> + :: [sel] "r" (0) ); >> + /* Force a reload of all segments to be the second scrubbing >> write. */ >> + dirty_segment_mask = ~0; >> + } > > Having the command line option, do we care about people actually using > it on AMD hardware? I care that it can be forced on to test. Beyond that, I expect the overhead will be large on Atom and AMD, which is why the command like doc was worded a bit more forcefully than other options, and is hinting at "don't try using this on non-affected hardware". > For %ds and %es this would now lead to up to three > selector register loads each, with the one above being completely > pointless (due to not clearing the segment bases anyway). Question of > course is whether adding yet another conditional (to the added code > above or to preload_segment()) would be any better than having this > extra selector register write. preload_segment() needs to change anyway given Rome's behaviour, but I need to wait for feedback from AMD as to whether they are happy with my CPUID bit and erratum proposal. > > Furthermore, if we cared, shouldn't SVM code also honor the flag and > gain extra loads, albeit perhaps with unlikely() used in the if()? If you observe, svm_ctxt_switch_to() already has this unconditionally. This was added in f9caf5ffaba but I'm not convinced it can be correct. I don't see equivalent logic in KVM, or any description to this effect in the APM. It might be an errata on an early SVM-capable processor. > > As to your choice of loading a nul selector: For the VERW change iirc > we've been told that using a nul selector is a bad choice from > performance pov. Are nul selectors being treated better for selector > register writes? VERW and segment loads are very different. VERW is microcoded, unconditionally references memory, and doesn't have a plausible usecase in the 286+ where you would call it with a NUL selector (until the MDS side effect came along). As a result, handling NUL is the slowpath in microcode. Loading a segment selector with NUL has
Re: [Xen-devel] [RFC] Code of Conduct
On 8/9/19 6:48 PM, Lars Kurth wrote: > Hi all, > > Following the discussion we had at the Developer Summit (see > https://wiki.xenproject.org/wiki/Design_Sessions_2019#Community_Issues_.2F_Improvements_-_Communication.2C_Code_of_Conduct.2C_etc. > for notes) I put together a draft for the Code of Conduct which can be found > here as well as inlined below > https://docs.google.com/document/d/1NnWdU_VnC1N_ZzxQG6jU9fnY2GPVCcfPJT5KY61WXJM/edit?usp=sharing > > > It is based on the LF Events CoC as we agreed on (the diff is attached). I > took the scope and enforcement sections from > https://www.contributor-covenant.org/version/1/4/code-of-conduct.html and > simplified it rather than inventing something new. > > You can provide feedback by commenting on the google doc or by replying to > the in-lined version below. > I expect it will some more discussion to get consensus. > > Note that I am not very attached to some of the terms, such as "Xen Project > CoC Team" and in some cases "participant" should probably be replaced by > community > members. > > But I felt, we should have something more concrete to discuss compared to > previous discussions. > > A Code of Conduct is a project wide policy change: thus, all subprojects > lists are CC'ed Thanks for doing this Lars. I think this is a step forward. I have a couple of comments, but only on the wording. > > Regards > Lars > > Here is the actual text > --- > # Our Pledge > In the interest of fostering an open and welcoming environment, we as > community > members of the Xen Project pledge to making participation in our project and > our > community a harassment-free experience for everyone. To me "pledge" means "promise"; and I don't think we can promise anyone that they'll have a harassment-free experience. I might say, "we ... are committed to making participation ... a harassment-free experience"; or "we ... pledge to maintain a harassment-free experience" or something like that. > # Unacceptable Behavior > Harassment will not be tolerated in the Xen Project Community in any form, > including but not limited to harassment based on gender, gender identity and > expression, sexual orientation, disability, physical appearance, body size, > race, > age, religion, ethnicity, nationality, level of experience, education, or > socio-economic status or any other status protected by laws in jurisdictions > in > which community members are based. > Harassment includes the use of abusive, > offensive or degrading language, intimidation, stalking, harassing > photography > or recording, inappropriate physical contact, sexual imagery and unwelcome > sexual advances, requests for sexual favors, publishing others' private > information such as a physical or electronic address without explicit > permission > and other conduct which could reasonably be considered inappropriate in a > professional setting. Should we put "such as physical or electronic address[es]" in parentheses? Also, I'm in favor of the Oxford Comma (so a comma after 'permission'). I might say "or any other conduct"; for some reason it sounds more natural to me. > Any report of harassment within the Xen Project community will be addressed > swiftly. Participants asked to stop any harassing behavior are expected to > comply immediately. Anyone who witnesses or is subjected to unacceptable > behavior should notify the Xen Project’s CoC team via cond...@xenproject.org. > > # Consequences of Unacceptable Behavior > If a participant engages in harassing behavior, the Xen Project’s CoC team > may > take any action it deems appropriate, ranging from issuance of a warning to > the > offending individual to expulsion from the Xen Project community. I realize by saying "range" you probably meant to include this, but I think spelling out "temporary suspension" as a possible consequence. E.g.: "If a participant engages in harassing behavior, the Xen Project's CoC team will investigate and take an action it deems appropriate against the offending individual. This may include issuing a warning, temporary suspension from mailing lists or commit rights, or expulsion from the XenProject community." That's all I had; thanks again, Lars. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ANNOUNCE] Xen Project Developer Summit 2019: Slides, Recordings and Design Session notes are available
Dear Community member, if you didn’t make it to the developer summit, you can find material related to the summit in the following locations Slides: https://www.slideshare.net/search/slideshow?searchfrom=header&q=XPDDS19 YouTube playlist of recordings https://www.youtube.com/user/XenProjectSoftware/playlists Design Session descriptions and notes (some are still missing) https://wiki.xenproject.org/wiki/Design_Sessions_2019 Best Regards Lars ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/AMD-Vi: Fold exit paths of {enable, disable}_iommu()
... to avoid having multiple spin_unlock_irqrestore() calls. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Boris Ostrovsky CC: Suravee Suthikulpanit CC: Brian Woods Interestingly GCC 6.3 managed to fold disable_iommu() automatically. There is some partial folding for enable_iommu() (insofar as there is only a single call to _spin_unlock_irqrestore emitted), but this delta yeilds add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20 (-20) Function old new delta enable_iommu18441824 -20 Total: Before=3340299, After=3340279, chg -0.00% which means that something wasn't done automatically. Noticed while investigating the S3 regression. --- xen/drivers/passthrough/amd/iommu_init.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c index bb9f33e264..bb5a3e57c9 100644 --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -899,11 +899,8 @@ static void enable_iommu(struct amd_iommu *iommu) spin_lock_irqsave(&iommu->lock, flags); -if ( iommu->enabled ) -{ -spin_unlock_irqrestore(&iommu->lock, flags); -return; -} +if ( unlikely(iommu->enabled) ) +goto out; amd_iommu_erratum_746_workaround(iommu); @@ -957,6 +954,8 @@ static void enable_iommu(struct amd_iommu *iommu) amd_iommu_flush_all_caches(iommu); iommu->enabled = 1; + + out: spin_unlock_irqrestore(&iommu->lock, flags); } @@ -966,11 +965,8 @@ static void disable_iommu(struct amd_iommu *iommu) spin_lock_irqsave(&iommu->lock, flags); -if ( !iommu->enabled ) -{ -spin_unlock_irqrestore(&iommu->lock, flags); -return; -} +if ( unlikely(!iommu->enabled) ) +goto out; if ( !iommu->ctrl.int_cap_xt_en ) amd_iommu_msi_enable(iommu, IOMMU_CONTROL_DISABLED); @@ -988,6 +984,7 @@ static void disable_iommu(struct amd_iommu *iommu) iommu->enabled = 0; + out: spin_unlock_irqrestore(&iommu->lock, flags); } -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.4 test] 139968: regressions - FAIL
flight 139968 linux-4.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/139968/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64 6 xen-buildfail REGR. vs. 139698 Tests which did not succeed, but are not blocking: test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked n/a test-amd64-amd64-examine 1 build-check(1) blocked n/a test-amd64-amd64-pygrub 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-amd64-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1)blocked n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked n/a build-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit1 7 xen-boot fail never pas
[Xen-devel] [xen-unstable-smoke test] 139992: tolerable all pass - PUSHED
flight 139992 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/139992/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen b6b5608b6027fd62ce565ecd72a3422c1223beaf baseline version: xen 60685089cb0ce4f3e1873c5e81f6ed3b77a492b2 Last test of basis 139980 2019-08-12 08:03:37 Z0 days Testing same since 139992 2019-08-12 11:00:43 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich 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 60685089cb..b6b5608b60 b6b5608b6027fd62ce565ecd72a3422c1223beaf -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 19/28] xen/arm32: head: Don't setup the fixmap on secondary CPUs
setup_fixmap() will setup the fixmap in the boot page tables in order to use earlyprintk and also update the register r11 holding the address to the UART. However, secondary CPUs are not using earlyprintk between turning the MMU on and switching to the runtime page table. So setting up the fixmap in the boot pages table is pointless. This means most of setup_fixmap() is not necessary for the secondary CPUs. The update of UART address is now moved out of setup_fixmap() and duplicated in the CPU boot and secondary CPUs boot. Additionally, the call to setup_fixmap() is removed from secondary CPUs boot. Lastly, take the opportunity to replace load from literal pool with the new macro mov_w. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's reviewed-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 0c95d1c432..8f945d318a 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -159,6 +159,10 @@ past_zImage: mov pc, r0 primary_switched: blsetup_fixmap +#ifdef CONFIG_EARLY_PRINTK +/* Use a virtual address to access the UART. */ +mov_w r11, EARLY_UART_VIRTUAL_ADDRESS +#endif b launch ENDPROC(start) @@ -201,8 +205,6 @@ GLOBAL(init_secondary) ldr r0, =secondary_switched mov pc, r0 secondary_switched: -blsetup_fixmap - /* * Non-boot CPUs need to move on to the proper pagetables, which were * setup in init_secondary_pagetables. @@ -221,6 +223,10 @@ secondary_switched: dsb /* Ensure completion of TLB+BP flush */ isb +#ifdef CONFIG_EARLY_PRINTK +/* Use a virtual address to access the UART. */ +mov_w r11, EARLY_UART_VIRTUAL_ADDRESS +#endif b launch ENDPROC(init_secondary) @@ -475,13 +481,6 @@ setup_fixmap: */ dsb #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ -/* - * Non-boot CPUs don't need to rebuild the fixmap itself, just - * the mapping from boot_second to xen_fixmap - */ -teq r12, #0 -bne 1f - /* Add UART to the fixmap table */ ldr r1, =xen_fixmap/* r1 := vaddr (xen_fixmap) */ lsr r2, r11, #THIRD_SHIFT @@ -502,9 +501,6 @@ setup_fixmap: mov r4, r4, lsr #(SECOND_SHIFT - 3) /* r4 := Slot for FIXMAP(0) */ mov r3, #0x0 strd r2, r3, [r1, r4] /* Map it in the fixmap's slot */ - -/* Use a virtual address to access the UART. */ -ldr r11, =EARLY_UART_VIRTUAL_ADDRESS #endif /* -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Terminology for "guest" - Was: [PATCH] docs/sphinx: Introduction
On 8/8/19 10:13 AM, Julien Grall wrote: > Hi Jan, > > On 08/08/2019 10:04, Jan Beulich wrote: >> On 08.08.2019 10:43, Andrew Cooper wrote: >>> On 08/08/2019 07:22, Jan Beulich wrote: On 07.08.2019 21:41, Andrew Cooper wrote: > --- /dev/null > +++ b/docs/glossary.rst > @@ -0,0 +1,37 @@ > +Glossary > + > + > +.. Terms should appear in alphabetical order > + > +.. glossary:: > + > + control domain > + A :term:`domain`, commonly dom0, with the permission and > responsibility > + to create and manage other domains on the system. > + > + domain > + A domain is Xen's unit of resource ownership, and generally has > at the > + minimum some RAM and virtual CPUs. > + > + The terms :term:`domain` and :term:`guest` are commonly used > + interchangeably, but they mean subtly different things. > + > + A guest is a single virtual machine. > + > + Consider the case of live migration where, for a period of > time, one > + guest will be comprised of two domains, while it is in transit. > + > + domid > + The numeric identifier of a running :term:`domain`. It is > unique to a > + single instance of Xen, used as the identifier in various APIs, > and is > + typically allocated sequentially from 0. > + > + guest > + See :term:`domain` I think you want to mention the usual distinction here: Dom0 is, while a domain, commonly not considered a guest. >>> >>> To be honest, I had totally forgotten about that. I guess now is the >>> proper time to rehash it in public. >>> >>> I don't think the way it currently gets used has a clear or coherent set >>> of rules, because I can't think of any to describe how it does get used. >>> >>> Either there are a clear and coherent (and simple!) set of rules for >>> what we mean by "guest", at which point they can live here in the >>> glossary, or the fuzzy way it is current used should cease. >> >> What's fuzzy about Dom0 not being a guest (due to being a part of the >> host instead)? > Dom0 is not part of the host if you are using an hardware domain. I > actually thought we renamed everything in Xen from Dom0 to hwdom to > avoid the confusion. > > I also would rather avoid to treat "dom0" as not a guest. In normal > setup this is a more privilege guest, in other setup this may just be a > normal guest (think about partitioning). A literal guest is someone who doesn't live in the building (or work in the buliding, if you're in a hotel). The fact that the staff cleaning rooms are restricted in their privileges doesn't make them guests of the hotel. The toolstack domain, the hardware domain, the driver domain, the xenstore domain, and so on are all part of the host system, designed to allow you to use Xen to do the thing you actually want to do: Run guests. And it's important that we have a word that distinguishes "domains that we only care about because they make it possible to run other domains", and "domains that we actually want to run". "guest / host" is a natural terminology for these. We already have the word "domain", which includes dom0, driver domains, toolstack domains, hardware domains, as well as guest domains. We don't need "guest" to be a synonym for "domain". -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 4/6] x86/boot: Rename trampoline_{start, end} to boot_trampoline_{start, end}
On 09.08.2019 17:02, David Woodhouse wrote: From: David Woodhouse In preparation for splitting the boot and permanent trampolines from each other. Some of these will change back, but most are boot so do the plain search/replace that way first, then a subsequent patch will extract the permanent trampoline code. To be honest I don't view it as helpful to do things in this order. If you first re-arranged the ordering of items within the trampoline, we'd then not end up with an intermediate state where the labels are misleading. Is there a reason things can't sensibly be done the other way around? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/iommu: remove usage of {set/clear}_identity_p2m_entry against PV domains
On 8/12/19 3:55 PM, Roger Pau Monné wrote: > On Mon, Aug 12, 2019 at 03:24:36PM +0100, George Dunlap wrote: >> On 8/12/19 2:56 PM, Roger Pau Monné wrote: >>> On Mon, Aug 12, 2019 at 02:17:53PM +0100, George Dunlap wrote: On 8/2/19 10:22 AM, Roger Pau Monne wrote: > Switch rmrr_identity_mapping to use iommu_{un}map in order to > establish RMRR mappings for PV domains, like it's done in > arch_iommu_hwdom_init. This solves the issue of a PV hardware domain > not getting RMRR mappings because {set/clear}_identity_p2m_entry was > not properly updating the iommu page tables. Sorry, I think this description is somewhat backwards: you're saying what you're doing first, and then saying what the problematic behavior was, but not actually saying what was causing the problematic behavior. Why was {set,clear}_identity_p2m not updating the page tables? I agree with Jan, that figuring that out is a prerequisite for any kind of fix: if `need_iommu_pt_sync()` is false at that point for the hardware domain, then there's a bigger problem than RMRRs not being adjusted properly. >>> >>> need_iommu_pt_sync is indeed false for a PV hardware domain not >>> running in strict mode, see: >>> >>> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/passthrough/iommu.c;h=f8c3bf53bd1793df93d7ddea6564dc929f40c156;hb=HEAD#l192 >>> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html >>> >>> This is fine for a non-strict PV hardware domain, since it has all the >>> host memory (minus memory used by Xen) mapped in the iommu page tables >>> except for RMRR regions not marked as reserved in the e820 memory map, >>> which are added using set_identity_p2m_entry. >>> >>> The issue here is that this patch alone doesn't fix the issue for the >>> reporter, and there seems to be an additional flush required in order >>> to get the issue solved on his end: >>> >>> https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00526.html >>> >>> My theory is that the system Roman is using is booting with DMA >>> remapping enabled in the iommu, and somehow the call to >>> iommu_flush_all in intel_iommu_hwdom_init doesn't seem to work >>> properly, while calling iommu_iotlb_flush_all does seem to do the >>> right thing. I'm still waiting for Roman to come back with the result >>> of my last debug patches in order to figure out whether my hypothesis >>> above is true. >>> >>> This however won't still explain the weird behaviour of >>> iommu_flush_all, and further debugging will still be required. >> >> OK; so this patch has four effects, it looks like: >> >> 1. iommu_legacy_[un]map -> iommu_[un]map >> 2. Move iommu ops out of {set,clear}_identity_p2m for PV guests; >> open-code them in rmrr_identity_mapping >> 3. For non-translated guests, do the operation unconditionally >> 4. Add a flush > > There's already a flush in iommu_legacy_[un]map, so the flush is also > done for both patches, just in different places. The legacy interface > does the flush on every call, while the new interface allows to > postpone the flush until all iommu page-table operations are done. > >> >> Regarding #3, the previous patch changed it from "if >> iommu_needs_pt_sync" to "if has_iommu_pt"; this one changes it to >> "always". Is that what you intended? > > Well, #3 is not actually correct. The code in intel_iommu_hwdom_init > and hence setup_hwdom_rmrr will only be called if the domain has an > iommu, so has_iommu_pt will always be true when adding RMRR regions. > Note rmrr_identity_mapping is the only caller of > {set,clear}_identity_p2m against PV guests. But if iommu is set the same in both cases, and if the flush happens in both cases, then why did this patch work and the previous patch didn't? >> I don't really see the point of #2: from the device's perspective, the >> "physmap" is the IOMMU for PV guests, and IOMMU+p2m for HVM guests; it >> makes sense to have a single place to call for either type of guest, >> rather than open-coding every location. > > OK, that's fine, but note that {set/clear}_identity_p2m_entry is > AFAICT the only p2m function allowed against PV guests, the rest will > return some kind of error, which IMO makes it the outlier, hence my > proposal to make it available to translated guests only, like the rest > of the p2m functions in that file. I just find it confusing that some > p2m functions can be used against PV guests, but not others. Right, but that's because set_identity_p2m_entry() isn't really about the p2m. It's about making sure that both devices and SMI handlers (or whatever) can access certain bits of memory. Lots of functions in p2m.c "tolerate" calls for PV guests; and guest_physmap_* calls used to actually to IOMMU operations on PV guests before XSA 288. We could change that to set_identity_physmap_entry() or something if you prefer. -George ___ Xen-devel mail
Re: [Xen-devel] [PATCH 3/6] remove late (on-demand) construction of IOMMU page tables
> -Original Message- [snip] > > > > On 30.07.2019 15:44, Paul Durrant wrote: > > > NOTE: This patch will cause a small amount of extra resource to be used > > >to accommodate IOMMU page tables that may never be used, since the > > >per-domain IOMMU flag enable flag is currently set to the value > > >of the global iommu_enable flag. A subsequent patch will add an > > >option to the toolstack to allow it to be turned off if there is > > >no intention to assign passthrough hardware to the domain. > > > > In particular if the default of this is going to be "true" (I > > didn't look at that patch yet, but the wording above makes me > > assume so), in auto-ballooning mode without shared page tables > > more memory should imo be ballooned out of Dom0 now. It has > > always been a bug that IOMMU page tables weren't accounted for, > > but it would become even more prominent then. > > Ultimately, once the whole series is applied, then nothing much should change > for those specifying > passthrough h/w in an xl.cfg. The main difference will be that h/w cannot be > passed through to a > domain that was not originally created with IOMMU pagetables. > With patches applied up to this point then, yes, every domain will get IOMMU > page tables. I guess I'll > take a look at the auto-ballooning code and see what needs to be done. > Ok, I've had a look... I could make a rough calculation in libxl_domain_need_memory() based on the domain's max_memkb and an assumption of a 4 level translation with 512 PTEs per level, or would prefer such guestimation to be overridable using an xl.cfg parameter in a broadly similar way to shadow_memkb? Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 25/28] xen/arm64: head: Introduce macros to create table and mapping entry
At the moment, any update to the boot-pages are open-coded. This is making more difficult to understand the logic of a function as each update roughly requires 6 instructions. To ease the readability, two new macros are introduced: - create_table_entry: Create a page-table entry in a given table. This can work at any level. - create_mapping_entry: Create a mapping entry in a given table. None of the users will require to map at any other level than 3rd (i.e page granularity). So the macro is supporting support 3rd level mapping. Furthermore, the two macros are capable to work independently of the state of the MMU. Lastly, take the opportunity to replace open-coded version in setup_fixmap() by the two new macros. The ones in create_page_tables() will be replaced in a follow-up patch. Signed-off-by: Julien Grall --- Changes in v3: - Patch added --- xen/arch/arm/arm64/head.S | 83 ++- 1 file changed, 67 insertions(+), 16 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index f2a0e1d3b0..f4177dbba1 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -492,6 +492,68 @@ cpu_init: ENDPROC(cpu_init) /* + * Macro to create a page table entry in \ptbl to \tbl + * + * ptbl:table symbol where the entry will be created + * tbl: table symbol to point to + * virt:virtual address + * shift: #imm page table shift + * tmp1:scratch register + * tmp2:scratch register + * tmp3:scratch register + * + * Preserves \virt + * Clobbers \tmp1, \tmp2, \tmp3 + * + * Also use x20 for the phys offset. + * + * Note that all parameters using registers should be distinct. + */ +.macro create_table_entry, ptbl, tbl, virt, shift, tmp1, tmp2, tmp3 +lsr \tmp1, \virt, #\shift +and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ + +load_paddr \tmp2, \tbl +mov \tmp3, #PT_PT /* \tmp3 := right for linear PT */ +orr \tmp3, \tmp3, \tmp2 /* + \tlb paddr */ + +adr_l \tmp2, \ptbl + +str \tmp3, [\tmp2, \tmp1, lsl #3] +.endm + +/* + * Macro to create a mapping entry in \tbl to \phys. Only mapping in 3rd + * level table (i.e page granularity) is supported. + * + * tbl: table symbol where the entry will be created + * virt:virtual address + * phys:physical address (should be page aligned) + * tmp1:scratch register + * tmp2:scratch register + * tmp3:scratch register + * type:mapping type. If not specified it will be normal memory (PT_MEM_L3) + * + * Preserves \virt, \phys + * Clobbers \tmp1, \tmp2, \tmp3 + * + * Note that all parameters using registers should be distinct. + */ +.macro create_mapping_entry, tbl, virt, phys, tmp1, tmp2, tmp3, type=PT_MEM_L3 +and \tmp3, \phys, #THIRD_MASK /* \tmp3 := PAGE_ALIGNED(phys) */ + +lsr \tmp1, \virt, #THIRD_SHIFT +and \tmp1, \tmp1, #LPAE_ENTRY_MASK/* \tmp1 := slot in \tlb */ + +mov \tmp2, #\type /* \tmp2 := right for section PT */ +orr \tmp2, \tmp2, \tmp3 /* + PAGE_ALIGNED(phys) */ + +adr_l \tmp3, \tbl + +str \tmp2, [\tmp3, \tmp1, lsl #3] +.endm + +/* * Rebuild the boot pagetable's first-level entries. The structure * is described in mm.c. * @@ -735,28 +797,17 @@ ENDPROC(remove_identity_mapping) * x20: Physical offset * x23: Early UART base physical address * - * Clobbers x1 - x4 + * Clobbers x0 - x3 */ setup_fixmap: #ifdef CONFIG_EARLY_PRINTK /* Add UART to the fixmap table */ -ldr x1, =xen_fixmap/* x1 := vaddr (xen_fixmap) */ -lsr x2, x23, #THIRD_SHIFT -lsl x2, x2, #THIRD_SHIFT /* 4K aligned paddr of UART */ -mov x3, #PT_DEV_L3 -orr x2, x2, x3 /* x2 := 4K dev map including UART */ -str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ +ldr x0, =EARLY_UART_VIRTUAL_ADDRESS +create_mapping_entry xen_fixmap, x0, x23, x1, x2, x3, type=PT_DEV_L3 #endif - /* Map fixmap into boot_second */ -ldr x4, =boot_second /* x4 := vaddr (boot_second) */ -load_paddr x2, xen_fixmap -mov x3, #PT_PT -orr x2, x2, x3 /* x2 := table map of xen_fixmap */ -ldr x1, =FIXMAP_ADDR(0) -lsr x1, x1, #(SECOND_SHIFT - 3) /* x1 := Slot for FIXMAP(0) */ -str x2, [x4, x1] /* Map it in the fixmap's slot */ - +ldr x0, =FIXMAP_ADDR(0) +create_table_entry boot_second, xen_fixmap, x0, SECOND_SHIFT, x1, x2, x3 /* Ensure any page table updates made above have occurred. */ dsb nshst -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/blkback: fix memory leaks
On 8/11/19 1:23 PM, Wenwen Wang wrote: > In read_per_ring_refs(), after 'req' and related memory regions are > allocated, xen_blkif_map() is invoked to map the shared frame, irq, and > etc. However, if this mapping process fails, no cleanup is performed, > leading to memory leaks. To fix this issue, invoke the cleanup before > returning the error. Reviewed-by: Boris Ostrovsky ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 6/7] xen/arm: don't iomem_permit_access for reserved-memory regions
Don't allow reserved-memory regions to be remapped into any unprivileged guests, until reserved-memory regions are properly supported in Xen. For now, do not call iomem_permit_access on them, because giving iomem_permit_access to dom0 means that the toolstack will be able to assign the region to a domU. Signed-off-by: Stefano Stabellini --- Changes in v5: - fix check condition - use strnicmp - return error - improve commit message Changes in v4: - compare the parent name with reserved-memory - use dt_node_cmp Changes in v3: - new patch --- xen/arch/arm/domain_build.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 4c8404155a..e0c0c01c88 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -1155,15 +1155,23 @@ static int __init map_range_to_domain(const struct dt_device_node *dev, bool need_mapping = !dt_device_for_passthrough(dev); int res; -res = iomem_permit_access(d, paddr_to_pfn(addr), - paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); -if ( res ) +/* + * Don't give iomem permissions for reserved-memory ranges to domUs + * until reserved-memory support is complete. + */ +if ( strnicmp(dt_node_full_name(dev), "/reserved-memory", + strlen("/reserved-memory")) != 0 ) { -printk(XENLOG_ERR "Unable to permit to dom%d access to" - " 0x%"PRIx64" - 0x%"PRIx64"\n", - d->domain_id, - addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); -return res; +res = iomem_permit_access(d, paddr_to_pfn(addr), +paddr_to_pfn(PAGE_ALIGN(addr + len - 1))); +if ( res ) +{ +printk(XENLOG_ERR "Unable to permit to dom%d access to" +" 0x%"PRIx64" - 0x%"PRIx64"\n", +d->domain_id, +addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); +return res; +} } if ( need_mapping ) -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] docs/sphinx: Introduction
On 8/7/19 8:41 PM, Andrew Cooper wrote: > Put together an introduction page for the Sphinx/RST docs, along with a > glossary which will accumulate over time. > > Signed-off-by: Andrew Cooper > --- > CC: George Dunlap > CC: Ian Jackson > CC: Jan Beulich > CC: Stefano Stabellini > CC: Wei Liu > CC: Julien Grall > CC: Roger Pau Monné > CC: Lars Kurth > > A rendered version is visible here: > > > https://andrewcoop-xen.readthedocs.io/en/docs-devel/admin-guide/introduction.html > > The image is created with https://www.draw.io/ and has the embedded source. > This means that anyone can open the SVG in Draw.IO, either online or with the > desktop editor, and modify the image in its original form. The only > modification I've made to the SVG as written is to xmllint it. > > RFC to get some feedback on level of technical detail (specifically, it must > not assume any Xen knowledge at all, but also shouldn't be an overload of > information), as well as views on the drawing itself. > --- > docs/admin-guide/index.rst | 1 + > docs/admin-guide/introduction.rst| 38 + > docs/admin-guide/xen-overview.drawio.svg | 97 > > docs/glossary.rst| 37 > docs/index.rst | 12 > 5 files changed, 185 insertions(+) > create mode 100644 docs/admin-guide/introduction.rst > create mode 100644 docs/admin-guide/xen-overview.drawio.svg > create mode 100644 docs/glossary.rst > > diff --git a/docs/admin-guide/index.rst b/docs/admin-guide/index.rst > index 6907d58829..fb5fa363d3 100644 > --- a/docs/admin-guide/index.rst > +++ b/docs/admin-guide/index.rst > @@ -2,4 +2,5 @@ Admin Guide > === > > .. toctree:: > + introduction > microcode-loading > diff --git a/docs/admin-guide/introduction.rst > b/docs/admin-guide/introduction.rst > new file mode 100644 > index 00..ea960308ab > --- /dev/null > +++ b/docs/admin-guide/introduction.rst > @@ -0,0 +1,38 @@ > +Introduction > + > + > +Xen is an open source, bare metal hypervisor. It runs as the most privileged > +piece of software, and shares the resources of the hardware between virtual > +machines. > + > +In Xen terminology, there are :term:`domains`, commonly abbreviated > to > +dom, which are identified by their numeric :term:`domid`. > + > +When Xen boots, dom0 is automatically started as well. Dom0 is a virtual > +machine which, by default, is granted full permissions [1]_. A typical setup > +might be: > + > +.. image:: xen-overview.drawio.svg > + > +Dom0 takes the role of :term:`control domain`, responsible for creating and > +managing other virtual machines, and the role of :term:`hardware domain`, > +responsible for hardware and marshalling guest I/O. > + > +Xen is deliberately minimal, and has no device drivers [2]_. Xen manages > RAM, > +schedules virtual CPUs on the available physical CPUs, and marshals > +interrupts. > + > +Xen also provides a hypercall interface to guests, including event channels > +(virtual interrupts), grant tables (shared memory), on which a lot of higher > +level functionality is built. > + > +.. rubric:: Footnotes > + > +.. [1] A common misconception with Xen's architecture is that dom0 is somehow > + different to other guests. The choice of id 0 is not an accident, and > + follows in UNIX heritage. > + > +.. [2] This definition might be fuzzy. Xen can talk to common serial UARTs, > + and knows how to drive various CPU internal devices such as IOMMUs, > but > + has no knowledge of network cards, disks, etc. All of that is the > + hardware domains responsibility. > diff --git a/docs/admin-guide/xen-overview.drawio.svg > b/docs/admin-guide/xen-overview.drawio.svg > new file mode 100644 > index 00..f120cdf77a > --- /dev/null > +++ b/docs/admin-guide/xen-overview.drawio.svg > @@ -0,0 +1,97 @@ > + > + "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd";> > +http://www.w3.org/2000/svg"; > xmlns:xlink="http://www.w3.org/1999/xlink"; version="1.1" width="701px" > height="461px" viewBox="-0.5 -0.5 701 461" content="modified="2019-08-04T17:05:55.267Z" host="" > agent="Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like > Gecko) draw.io/11.1.1 Chrome/76.0.3809.88 Electron/6.0.0 Safari/537.36" > etag="M7ISh4Ny83I7m1UfK1F2" version="11.1.1" > type="device"> name="Page-1">7Zpbd5s4EMc/jR/Tw/3y6FuadNu0jde72X3pwSAua4w4smzjfPoVRhiD5DvGbM42D0WDkMRv5q8ZDB25P0s+Iyv2v0EHhB1JcJKOPOhIkigKEvkvtawziyGamcFDgUM7FYZR8A6oUaDWReCAeakjhjDEQVw22jCKgI1LNgshuCp3c2FYnjW2PMAYRrYVstY/Awf79C5UobA/gcDz8faG6ZmZlXemhrlvOXC1Y5KHHbmPIMTZ0SzpgzCFl3PJrnvcc3a7MAQifMoFySP67j/+MuJ+8nUIfo3D/it4oN5ZWuGC3vAbiOh68TqHgOAickA6jtCRe24Qhn0YQrQ5KbuGDWyb2OcYwSnYOTMxVEXdXAEjTF0rpe0lQDggjLth4EXEOAscJ52rZ1EDyoj2fIiCd3KxFRKjmM4RW3YQea+UuFqYaIulQkGlc4Jkx0QpfQZwBjBaky40RGWZemxVOFzP3ervOFujNovGmLcdqnADOaCeOMMrCuOVAZwJ57nFUY
Re: [Xen-devel] [PATCH v5] x86/xpti: Don't leak TSS-adjacent percpu data via Meltdown
On 12.08.2019 12:29, Andrew Cooper wrote: On 12/08/2019 08:23, Jan Beulich wrote: @@ -747,16 +747,10 @@ void load_system_tables(void) .bitmap = IOBMP_INVALID_OFFSET, }; - _set_tssldt_desc( - gdt + TSS_ENTRY, - (unsigned long)tss, - offsetof(struct tss_struct, __cacheline_filler) - 1, - SYS_DESC_tss_avail); - _set_tssldt_desc( - compat_gdt + TSS_ENTRY, - (unsigned long)tss, - offsetof(struct tss_struct, __cacheline_filler) - 1, - SYS_DESC_tss_busy); + _set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss, + sizeof(*tss) - 1, SYS_DESC_tss_avail); + _set_tssldt_desc(compat_gdt + TSS_ENTRY, (unsigned long)tss, + sizeof(*tss) - 1, SYS_DESC_tss_busy); Do you think it is worth having a BUILD_BUG_ON(sizeof(*tss) < 0x67), just to confirm that the load wont fault? Not sure - it feels like going a little overboard with checks. Feel free to add one though if you're really convinced it helps, but then please with 0x68 in place of 0x67. (I'm about to leave now, so if you want me to add anything and/or commit it, it would have to wait two weeks.) Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/arm: setup: Add Xen as boot module before printing all boot modules
On Mon, 12 Aug 2019, Julien Grall wrote: > Since commit f60658c6ae "xen/arm: Stop relocating Xen", the position of > Xen in memory is not printed anymore. This can make difficult to debug > early code. > > As Xen is not relocated anymore, we can add Xen as boot module before > calling boot_fdt_info(). With that, the function will print Xen module > information along with all the other modules. > > Signed-off-by: Julien Grall Acked-by: Stefano Stabellini > --- > xen/arch/arm/setup.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index d047ff8e31..7509d76dd4 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -779,18 +779,18 @@ void __init start_xen(unsigned long boot_phys_offset, >"Please check your bootloader.\n", >fdt_paddr); > > -fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr); > - > -cmdline = boot_fdt_cmdline(device_tree_flattened); > -printk("Command line: %s\n", cmdline); > -cmdline_parse(cmdline); > - > /* Register Xen's load address as a boot module. */ > xen_bootmodule = add_boot_module(BOOTMOD_XEN, > (paddr_t)(uintptr_t)(_start + boot_phys_offset), > (paddr_t)(uintptr_t)(_end - _start + 1), false); > BUG_ON(!xen_bootmodule); > > +fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr); > + > +cmdline = boot_fdt_cmdline(device_tree_flattened); > +printk("Command line: %s\n", cmdline); > +cmdline_parse(cmdline); > + > setup_mm(fdt_paddr, fdt_size); > > /* Parse the ACPI tables for possible boot-time configuration */ > -- > 2.11.0 > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 03/28] xen/arm64: head: Rework and document setup_fixmap()
At the moment, the fixmap table is only hooked when earlyprintk is used. This is fine today because in C land, the fixmap is not used by anyone until the the boot CPU is switching to the runtime page-tables. In the future, the boot CPU will not switch between page-tables to avoid TLB incoherency. Thus, the fixmap table will need to be always hooked before any use. Let's start doing it now in setup_fixmap(). Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall Acked-by: Stefano Stabellini --- Changes in v3: - Fix typo in the commit message - Add Stefano's Acked-by Changes in v2: - Update the comment to reflect that we clobbers x1 - x4 and not x0 - x1. - Add the list of input registers - s/ID map/1:1 mapping/ - Reword the commit message --- xen/arch/arm/arm64/head.S | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index ec138aae3e..7b6b820726 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -702,8 +702,21 @@ identity_mapping_removed: ret ENDPROC(remove_identity_mapping) +/* + * Map the UART in the fixmap (when earlyprintk is used) and hook the + * fixmap table in the page tables. + * + * The fixmap cannot be mapped in create_page_tables because it may + * clash with the 1:1 mapping. + * + * Inputs: + * x20: Physical offset + * x23: Early UART base physical address + * + * Clobbers x1 - x4 + */ setup_fixmap: -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ +#ifdef CONFIG_EARLY_PRINTK /* Add UART to the fixmap table */ ldr x1, =xen_fixmap/* x1 := vaddr (xen_fixmap) */ lsr x2, x23, #THIRD_SHIFT @@ -711,6 +724,7 @@ setup_fixmap: mov x3, #PT_DEV_L3 orr x2, x2, x3 /* x2 := 4K dev map including UART */ str x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ +#endif /* Map fixmap into boot_second */ ldr x4, =boot_second /* x4 := vaddr (boot_second) */ @@ -723,7 +737,7 @@ setup_fixmap: /* Ensure any page table updates made above have occurred. */ dsb nshst -#endif + ret ENDPROC(setup_fixmap) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 07/28] xen/arm64: head: Fix typo in the documentation on top of init_uart()
Signed-off-by: Julien Grall --- Changes in v3: - Patch added --- xen/arch/arm/arm64/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index cd03101196..a6f3aa4ee5 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -825,7 +825,7 @@ ENTRY(switch_ttbr) /* * Initialize the UART. Should only be called on the boot CPU. * - * Ouput: + * Output: * x23: Early UART base physical address * * Clobbers x0 - x1 -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/iommu: remove usage of {set/clear}_identity_p2m_entry against PV domains
> -Original Message- > From: Roger Pau Monne > Sent: 02 August 2019 10:22 > To: xen-devel@lists.xenproject.org > Cc: Roger Pau Monne ; Roman Shaposhnik > ; George Dunlap > ; Jan Beulich ; Andrew Cooper > ; Wei Liu ; Kevin Tian > ; Paul Durrant > > Subject: [PATCH] x86/iommu: remove usage of {set/clear}_identity_p2m_entry > against PV domains > > Switch rmrr_identity_mapping to use iommu_{un}map in order to > establish RMRR mappings for PV domains, like it's done in > arch_iommu_hwdom_init. This solves the issue of a PV hardware domain > not getting RMRR mappings because {set/clear}_identity_p2m_entry was > not properly updating the iommu page tables. > I'm not sure this is the right approach. TBH it's not clear to me exactly what role the P2M code should play for a PV domain (I'm guessing it's probably there so that shadowing can be turned on) but it would make more sense to me if fewer paths went round the side of it and the internals handled all of the PV/HVM differences. Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 0/7] reserved-memory in dom0
Hi all, This patch series introduces partial reserved-memory support for dom0 only (no domU support for reserved-memory yet.) The following changes since commit 762b9a2d990bba1f3aefe660cff0c37ad2e375bc: xen/page_alloc: Keep away MFN 0 from the buddy allocator (2019-08-09 11:12:55 -0700) are available in the Git repository at: http://xenbits.xenproject.org/git-http/people/sstabellini/xen-unstable.git reserved-mem-4 for you to fetch changes up to 74d2088dbd793409587a20db124d004e944d23e8: xen/arm: add reserved-memory regions to the dom0 memory node (2019-08-12 12:25:45 -0700) Stefano Stabellini (7): xen/arm: pass node to device_tree_for_each_node xen/arm: make process_memory_node a device_tree_node_func xen/arm: keep track of reserved-memory regions xen/arm: early_print_info print reserved_mem xen/arm: handle reserved-memory in consider_modules and dt_unreserved_regions xen/arm: don't iomem_permit_access for reserved-memory regions xen/arm: add reserved-memory regions to the dom0 memory node xen/arch/arm/acpi/boot.c | 8 ++-- xen/arch/arm/bootfdt.c| 96 ++- xen/arch/arm/domain_build.c | 46 ++--- xen/arch/arm/setup.c | 53 ++-- xen/include/asm-arm/setup.h | 1 + xen/include/xen/device_tree.h | 6 +-- 6 files changed, 157 insertions(+), 53 deletions(-) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] remove late (on-demand) construction of IOMMU page tables
> -Original Message- > From: Jan Beulich > Sent: 07 August 2019 11:32 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Julien Grall ; > Alexandru Isaila > ; Petre Pircalabu ; > Razvan Cojocaru > ; Andrew Cooper ; Roger > Pau Monne > ; VolodymyrBabchuk ; George > Dunlap > ; Ian Jackson ; Stefano > Stabellini > ; Konrad Rzeszutek Wilk ; > Tamas K Lengyel > ; Tim (Xen.org) ; Wei Liu > Subject: Re: [Xen-devel] [PATCH 3/6] remove late (on-demand) construction of > IOMMU page tables > > On 30.07.2019 15:44, Paul Durrant wrote: > > NOTE: This patch will cause a small amount of extra resource to be used > >to accommodate IOMMU page tables that may never be used, since the > >per-domain IOMMU flag enable flag is currently set to the value > >of the global iommu_enable flag. A subsequent patch will add an > >option to the toolstack to allow it to be turned off if there is > >no intention to assign passthrough hardware to the domain. > > In particular if the default of this is going to be "true" (I > didn't look at that patch yet, but the wording above makes me > assume so), in auto-ballooning mode without shared page tables > more memory should imo be ballooned out of Dom0 now. It has > always been a bug that IOMMU page tables weren't accounted for, > but it would become even more prominent then. Ultimately, once the whole series is applied, then nothing much should change for those specifying passthrough h/w in an xl.cfg. The main difference will be that h/w cannot be passed through to a domain that was not originally created with IOMMU pagetables. With patches applied up to this point then, yes, every domain will get IOMMU page tables. I guess I'll take a look at the auto-ballooning code and see what needs to be done. > > > --- a/xen/arch/x86/hvm/mtrr.c > > +++ b/xen/arch/x86/hvm/mtrr.c > > @@ -783,7 +783,8 @@ HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr, > > hvm_load_mtrr_msr, 1, > > > > void memory_type_changed(struct domain *d) > > { > > -if ( (has_iommu_pt(d) || cache_flush_permitted(d)) && d->vcpu && > > d->vcpu[0] ) > > +if ( (is_iommu_enabled(d) || cache_flush_permitted(d)) && d->vcpu && > > + d->vcpu[0] ) > > As a really minor comment - I think it wouldn't be bad for both > d->vcpu references to end up on the same line. Ok. > > > @@ -625,8 +548,7 @@ static void iommu_dump_p2m_table(unsigned char key) > > ops = iommu_get_ops(); > > for_each_domain(d) > > { > > -if ( is_hardware_domain(d) || > > - dom_iommu(d)->status < IOMMU_STATUS_initialized ) > > +if ( !is_iommu_enabled(d) ) > > continue; > > Why do you drop the hwdom check here? Because is_iommu_enabled() for the h/w domain will always be true if iommu_enabled is true, so no need for a special case. > > > --- a/xen/include/asm-arm/iommu.h > > +++ b/xen/include/asm-arm/iommu.h > > @@ -21,7 +21,7 @@ struct arch_iommu > > }; > > > > /* Always share P2M Table between the CPU and the IOMMU */ > > -#define iommu_use_hap_pt(d) (has_iommu_pt(d)) > > +#define iommu_use_hap_pt(d) (is_iommu_enabled(d)) > > I'd suggest dropping the stray outer pair of parentheses at the > same time. Ok, will do. Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 01/28] linkage: new macros for assembler symbols
Hi, this time a more detailed look. :) > Subject: Re: [PATCH v8 01/28] linkage: new macros for assembler symbols Patch subject needs a verb, like, for example: "linkage: Introduce new macros for assembler symbols" On Thu, Aug 08, 2019 at 12:38:27PM +0200, Jiri Slaby wrote: > Introduce new C macros for annotations of functions and data in > assembly. There is a long-standing mess in macros like ENTRY, END, > ENDPROC and similar. They are used in different manners and sometimes > incorrectly. > > So introduce macros with clear use to annotate assembly as follows: > > a) Support macros for the ones below >SYM_T_FUNC -- type used by assembler to mark functions >SYM_T_OBJECT -- type used by assembler to mark data >SYM_T_NONE -- type used by assembler to mark entries of unknown type > >They are defined as STT_FUNC, STT_OBJECT, and STT_NOTYPE >respectively. According to the gas manual, this is the most portable >way. I am not sure about other assemblers, so we can switch this back >to %function and %object if this turns into a problem. Architectures >can also override them by something like ", @function" if they need. > >SYM_A_ALIGN, SYM_A_NONE -- align the symbol? >SYM_L_GLOBAL, SYM_L_WEAK, SYM_L_LOCAL -- linkage of symbols > > b) Mostly internal annotations, used by the ones below >SYM_ENTRY -- use only if you have to (for non-paired symbols) >SYM_START -- use only if you have to (for paired symbols) >SYM_END -- use only if you have to (for paired symbols) > > c) Annotations for code >SYM_INNER_LABEL_ALIGN -- only for labels in the middle of code >SYM_INNER_LABEL -- only for labels in the middle of code > >SYM_FUNC_START_LOCAL_ALIAS -- use where there are two local names for > one function >SYM_FUNC_START_ALIAS -- use where there are two global names for one > function >SYM_FUNC_END_ALIAS -- the end of LOCAL_ALIASed or ALIASed function > >SYM_FUNC_START -- use for global functions >SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment >SYM_FUNC_START_LOCAL -- use for local functions >SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o > alignment >SYM_FUNC_START_WEAK -- use for weak functions >SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment >SYM_FUNC_END -- the end of SYM_FUNC_START_LOCAL, SYM_FUNC_START, > SYM_FUNC_START_WEAK, ... > >For functions with special (non-C) calling conventions: >SYM_CODE_START -- use for non-C (special) functions >SYM_CODE_START_NOALIGN -- use for non-C (special) functions, w/o > alignment >SYM_CODE_START_LOCAL -- use for local non-C (special) functions >SYM_CODE_START_LOCAL_NOALIGN -- use for local non-C (special) > functions, w/o alignment >SYM_CODE_END -- the end of SYM_CODE_START_LOCAL or SYM_CODE_START > > d) For data >SYM_DATA_START -- global data symbol >SYM_DATA_START_LOCAL -- local data symbol >SYM_DATA_END -- the end of the SYM_DATA_START symbol >SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol >SYM_DATA -- start+end wrapper around simple global data >SYM_DATA_LOCAL -- start+end wrapper around simple local data > > == > > The macros allow to pair starts and ends of functions and mark functions > correctly in the output ELF objects. > > All users of the old macros in x86 are converted to use these in further > patches. > > [v2] > * use SYM_ prefix and sane names > * add SYM_START and SYM_END and parametrize all the macros > > [v3] > * add SYM_DATA, SYM_DATA_LOCAL, and SYM_DATA_END_LABEL > > [v4] > * add _NOALIGN versions of some macros > * add _CODE_ derivates of _FUNC_ macros > > [v5] > * drop "SIMPLE" from data annotations > * switch NOALIGN and ALIGN variants of inner labels > * s/visibility/linkage/; s@SYM_V_@SYM_L_@ > * add Documentation > > [v6] > * fixed typos found by Randy Dunlap > * remove doubled INNER_LABEL macros, one pair was unused > > [v8] > * use lkml.kernel.org for links > * link the docs from index.rst (by bpetkov) > * fixed typos on the docs Patch version history which doesn't belong in the commit message goes... > Signed-off-by: Jiri Slaby > Cc: Andrew Morton > Cc: Boris Ostrovsky > Cc: h...@zytor.com > Cc: Ingo Molnar > Cc: jpoim...@redhat.com > Cc: Juergen Gross > Cc: Len Brown > Cc: Linus Torvalds > Cc: linux-ker...@vger.kernel.org > Cc: linux...@vger.kernel.org > Cc: mi...@redhat.com > Cc: Pavel Machek > Cc: Peter Zijlstra > Cc: "Rafael J. Wysocki" > Cc: Thomas Gleixner > Cc: xen-devel@lists.xenproject.org > Cc: x...@kernel.org > --- ... here, under the "---" so that git can ignore it when applying. > Documentation/asm-annotations.rst | 217 ++ > Documentation/index.rst | 8 + > arch/x86/include/asm/linkage.h| 10 +- > include/linux/linkage.h | 245 +- > 4 files changed, 469 insertions(+),
[Xen-devel] [PATCH v3 18/28] xen/arm32: head: Move assembly switch to the runtime PT in secondary CPUs path
The assembly switch to the runtime PT is only necessary for the secondary CPUs. So move the code in the secondary CPUs path. While this is definitely not compliant with the Arm Arm as we are switching between two differents set of page-tables without turning off the MMU. Turning off the MMU is impossible here as the ID map may clash with other mappings in the runtime page-tables. This will require more rework to avoid the problem. So for now add a TODO in the code. Finally, the code is currently assume that r5 will be properly set to 0 before hand. This is done by create_page_tables() which is called quite early in the boot process. There are a risk this may be oversight in the future and therefore breaking secondary CPUs boot. Instead, set r5 to 0 just before using it. Signed-off-by: Julien Grall --- Changes in v3: - There is no need to zero r5 Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 41 +++-- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index f8603051e4..0c95d1c432 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -202,6 +202,25 @@ GLOBAL(init_secondary) mov pc, r0 secondary_switched: blsetup_fixmap + +/* + * Non-boot CPUs need to move on to the proper pagetables, which were + * setup in init_secondary_pagetables. + * + * XXX: This is not compliant with the Arm Arm. + */ +ldr r4, =init_ttbr /* VA of HTTBR value stashed by CPU 0 */ +ldrd r4, r5, [r4] /* Actual value */ +dsb +mcrr CP64(r4, r5, HTTBR) +dsb +isb +mcr CP32(r0, TLBIALLH) /* Flush hypervisor TLB */ +mcr CP32(r0, ICIALLU) /* Flush I-cache */ +mcr CP32(r0, BPIALL) /* Flush branch predictor */ +dsb /* Ensure completion of TLB+BP flush */ +isb + b launch ENDPROC(init_secondary) @@ -505,28 +524,6 @@ ENDPROC(setup_fixmap) launch: PRINT("- Ready -\r\n") -/* The boot CPU should go straight into C now */ -teq r12, #0 -beq 1f - -/* - * Non-boot CPUs need to move on to the proper pagetables, which were - * setup in init_secondary_pagetables. - */ - -ldr r4, =init_ttbr /* VA of HTTBR value stashed by CPU 0 */ -ldrd r4, r5, [r4] /* Actual value */ -dsb -mcrr CP64(r4, r5, HTTBR) -dsb -isb -mcr CP32(r0, TLBIALLH) /* Flush hypervisor TLB */ -mcr CP32(r0, ICIALLU) /* Flush I-cache */ -mcr CP32(r0, BPIALL) /* Flush branch predictor */ -dsb /* Ensure completion of TLB+BP flush */ -isb - -1: ldr r0, =init_data add r0, #INITINFO_stack/* Find the boot-time stack */ ldr sp, [r0] -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/suspend: Sanity check more properties in enter_state()
The logic depends on being run on CPU0, and in IDLE context. Having this explicitly identified allows for simplification of the whole S3 path. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné --- xen/arch/x86/acpi/power.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c index aecc754fdb..d83e8cdd52 100644 --- a/xen/arch/x86/acpi/power.c +++ b/xen/arch/x86/acpi/power.c @@ -174,6 +174,8 @@ static int enter_state(u32 state) return -EBUSY; BUG_ON(system_state != SYS_STATE_active); +BUG_ON(!is_idle_vcpu(current)); +BUG_ON(smp_processor_id() != 0); system_state = SYS_STATE_suspend; printk(XENLOG_INFO "Preparing system for ACPI S%d state.\n", state); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 6/7] xen/arm: don't iomem_permit_access for reserved-memory regions
On Mon, 12 Aug 2019, Julien Grall wrote: > On 09/08/2019 23:56, Stefano Stabellini wrote: > > On Thu, 8 Aug 2019, Volodymyr Babchuk wrote: > > > Hi Stefano, > > > > > > Stefano Stabellini writes: > > > > > > > Don't allow reserved-memory regions to be remapped into any guests, > > > > until reserved-memory regions are properly supported in Xen. For now, > > > > do not call iomem_permit_access for them. > > > > > > > > Signed-off-by: Stefano Stabellini > > > > --- > > > > > > > > Changes in v4: > > > > - compare the parent name with reserved-memory > > > > - use dt_node_cmp > > > > > > > > Changes in v3: > > > > - new patch > > > > --- > > > > xen/arch/arm/domain_build.c | 24 > > > > 1 file changed, 16 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > > > index 4c8404155a..267e0549e2 100644 > > > > --- a/xen/arch/arm/domain_build.c > > > > +++ b/xen/arch/arm/domain_build.c > > > > @@ -1153,17 +1153,25 @@ static int __init map_range_to_domain(const > > > > struct dt_device_node *dev, > > > > struct map_range_data *mr_data = data; > > > > struct domain *d = mr_data->d; > > > > bool need_mapping = !dt_device_for_passthrough(dev); > > > > +const struct dt_device_node *parent = dt_get_parent(dev); > > > > int res; > > > > > > > > -res = iomem_permit_access(d, paddr_to_pfn(addr), > > > > - paddr_to_pfn(PAGE_ALIGN(addr + len - > > > > 1))); > > > > -if ( res ) > > > > +/* > > > > + * Don't give iomem permissions for reserved-memory ranges until > > > > + * reserved-memory support is complete. > > > > + */ > > > > +if ( dt_node_cmp(dt_node_name(parent), "reserved-memory") == 0 ) > > > Am I missing something, or you are permitting access only if it from a > > > "reserved-memory" node? This contradicts with patch description. > > > > Well spotted! I inverted the condition by mistake. > > > > > > > > { > > > > -printk(XENLOG_ERR "Unable to permit to dom%d access to" > > > > - " 0x%"PRIx64" - 0x%"PRIx64"\n", > > > > - d->domain_id, > > > > - addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); > > > > -return res; > > > > +res = iomem_permit_access(d, paddr_to_pfn(addr), > > > > + paddr_to_pfn(PAGE_ALIGN(addr + len - > > > > 1))); > > > > +if ( res ) > > > > +{ > > > > +printk(XENLOG_ERR "Unable to permit to dom%d access to" > > > > + " 0x%"PRIx64" - 0x%"PRIx64"\n", > > > > + d->domain_id, > > > > + addr & PAGE_MASK, PAGE_ALIGN(addr + len) - 1); > > > > +return res; > > > > +} > > > > } > > > > > > > > if ( need_mapping ) > > > So, this region cold be mapped, but without the access? > > IOMEM access and mapping are two different things. The former gives a domain > control over managing the region (i.e mapping, unmapping, giving access to > another domain). The latter will map the region in the P2M so the domain can > read/write into it. > > > > > I'll change it to return early from the function for reserved-memory > > regions. > > I am not sure to understand you suggestion here... You still need to have > reserved-regions mapped into the hardware domain. The only thing we want to > prevent is the domain to manage the region. I forgot that giving iomem permission to dom0 automatically means that the toolstack can give iomem permission to a domU for the same region. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [edk2-devel] [PATCH v4 00/35] Specific platform to run OVMF in Xen PVH and HVM guests
On Tue, Jul 30, 2019 at 03:10:13PM +0200, Laszlo Ersek wrote: > Hi Anthony, > > On 07/29/19 17:39, Anthony PERARD wrote: > > Patch series available in this git branch: > > https://xenbits.xen.org/git-http/people/aperard/ovmf.git > > br.platform-xen-pvh-v4 > > > > Changes in v4: > > - patch "OvmfPkg/XenPlatformPei: Reserve hvmloader's memory only when it has > > run" was removed, and instead a different change is done in > > "OvmfPkg/XenPlatformPei: Rework memory detection" > > - other changes detailed in the notes of each patch > > I've gone through the v4 series. If reviewers on the xen-devel list > think v4 is okay to merge, I can do that (with the small fixups I > offered here and there). I suggest that we wait a few days -- please > ping me when you believe the review on xen-devel has concluded. > > If you prefer to post v5, that works as well of course. There's a few more small fixup proposed by Roger, should I post a v5 for them? (and maybe only CC you and the lists.) Otherwise, I've pushed the branch br.platform-xen-pvh-v4.1 to my repo [1] where I believe I've collected all the small fixups. [1] https://xenbits.xen.org/git-http/people/aperard/ovmf.git br.platform-xen-pvh-v4.1 Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/arm: setup: Add Xen as boot module before printing all boot modules
Since commit f60658c6ae "xen/arm: Stop relocating Xen", the position of Xen in memory is not printed anymore. This can make difficult to debug early code. As Xen is not relocated anymore, we can add Xen as boot module before calling boot_fdt_info(). With that, the function will print Xen module information along with all the other modules. Signed-off-by: Julien Grall --- xen/arch/arm/setup.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index d047ff8e31..7509d76dd4 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -779,18 +779,18 @@ void __init start_xen(unsigned long boot_phys_offset, "Please check your bootloader.\n", fdt_paddr); -fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr); - -cmdline = boot_fdt_cmdline(device_tree_flattened); -printk("Command line: %s\n", cmdline); -cmdline_parse(cmdline); - /* Register Xen's load address as a boot module. */ xen_bootmodule = add_boot_module(BOOTMOD_XEN, (paddr_t)(uintptr_t)(_start + boot_phys_offset), (paddr_t)(uintptr_t)(_end - _start + 1), false); BUG_ON(!xen_bootmodule); +fdt_size = boot_fdt_info(device_tree_flattened, fdt_paddr); + +cmdline = boot_fdt_cmdline(device_tree_flattened); +printk("Command line: %s\n", cmdline); +cmdline_parse(cmdline); + setup_mm(fdt_paddr, fdt_size); /* Parse the ACPI tables for possible boot-time configuration */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 139952: regressions - FAIL
flight 139952 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/139952/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 139876 Tests which are failing intermittently (not blocking): test-amd64-i386-pair10 xen-boot/src_host fail in 139915 pass in 139952 test-amd64-i386-pair11 xen-boot/dst_host fail in 139915 pass in 139952 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 guest-start/debianhvm.repeat fail in 139915 pass in 139952 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 guest-start/debianhvm.repeat fail in 139915 pass in 139952 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail pass in 139915 test-amd64-amd64-xl-pvshim 16 guest-localmigrate fail pass in 139915 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 13 migrate-support-check fail in 139915 never pass test-amd64-i386-libvirt-xsm 13 migrate-support-check fail in 139915 never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail in 139915 never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail in 139915 never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 139876 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 139876 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139876 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 139876 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139876 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 139876 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 139876 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 139876 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 139876 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf
Re: [Xen-devel] [PATCH v4 3/7] xen/arm: keep track of reserved-memory regions
On Sat, 10 Aug 2019, Julien Grall wrote: > On Fri, 9 Aug 2019, 23:21 Stefano Stabellini, wrote: > On Wed, 7 Aug 2019, Julien Grall wrote: > > Hi Stefano, > > > > On 06/08/2019 22:49, Stefano Stabellini wrote: > > > As we parse the device tree in Xen, keep track of the > reserved-memory > > > regions as they need special treatment (follow-up patches will make > use > > > of the stored information.) > > > > > > Reuse process_memory_node to add reserved-memory regions to the > > > bootinfo.reserved_mem array. > > > > > > Refuse to continue once we reach the max number of reserved memory > > > regions to avoid accidentally mapping any portions of them into a > VM. > > > > > > Signed-off-by: Stefano Stabellini > > > > > > --- > > > Changes in v4: > > > - depth + 1 in process_reserved_memory_node > > > > Ah, you fixed it in this patch. But then, this does not match the > > documentation in patch #1. > > Yes good point, see below > > > > > - pass address_cells and size_cells to device_tree_for_each_node > > > - pass struct meminfo * instead of a boolean to process_memory_node > > > - improve in-code comment > > > > I can't see any comment, is that an improvement? :) > > It got lost with the refactoring of the code, but I don't think we need > it anymore > > > > > - use a separate process_reserved_memory_node (separate from > > > process_memory_node) function wrapper to have different error > handling > > > > > > Changes in v3: > > > - match only /reserved-memory > > > - put the warning back in place for reg not present on a normal > memory > > > region > > > - refuse to continue once we reach the max number of reserved memory > > > regions > > > > > > Changes in v2: > > > - call process_memory_node from process_reserved_memory_node to > avoid > > > duplication > > > --- > > > xen/arch/arm/bootfdt.c | 43 > +++-- > > > xen/include/asm-arm/setup.h | 1 + > > > 2 files changed, 38 insertions(+), 6 deletions(-) > > > > > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > > > index c22d57cd72..3e6fd63b16 100644 > > > --- a/xen/arch/arm/bootfdt.c > > > +++ b/xen/arch/arm/bootfdt.c > > > @@ -144,6 +144,7 @@ static int __init process_memory_node(const > void *fdt, > > > int node, > > > const __be32 *cell; > > > paddr_t start, size; > > > u32 reg_cells = address_cells + size_cells; > > > + struct meminfo *mem = (struct meminfo *)data; > > > > The cast is unnecessary. > > > > The rest of the code looks good. Pending the discussion about > > device_tree_for_each_node: > > > > Acked-by: Julien Grall > > Thank you. I removed the cast. Also, I think that it makes more sense to > do the depth increase (depth + 1) inside the implementation of > device_tree_for_each_node instead of at the caller site, like it is done > in this patch. This would match the documentation better and is cleaner > from an interface point of view. So I'll remove the depth increase from > this patch and move it to the first patch (min_depth = depth + 1). > > > Well, you don't need to pass the depth at all. It is just an artificial > number for libfdt to know were to stop. > > We also don't need the absolute depth in any of the early FDT. The relative > one is sufficient. Yes, you are right, good suggestion___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 139980: tolerable all pass - PUSHED
flight 139980 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/139980/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 60685089cb0ce4f3e1873c5e81f6ed3b77a492b2 baseline version: xen 762b9a2d990bba1f3aefe660cff0c37ad2e375bc Last test of basis 139879 2019-08-09 19:11:46 Z2 days Testing same since 139980 2019-08-12 08:03:37 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich 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 762b9a2d99..60685089cb 60685089cb0ce4f3e1873c5e81f6ed3b77a492b2 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 06/28] xen/arm64: head: Introduce a macro to get a PC-relative address of a symbol
Arm64 provides instructions to load a PC-relative address, but with some limitations: - adr is enable to cope with +/-1MB - adrp is enale to cope with +/-4GB but relative to a 4KB page address Because of that, the code requires to use 2 instructions to load any Xen symbol. To make the code more obvious, introducing a new macro adr_l is introduced. The new macro is used to replace a couple of open-coded use in efi_xen_start. The macro is copied from Linux 5.2-rc4. Signed-off-by: Julien Grall Acked-by: Stefano Stabellini --- Changes in v3: - Fix typo in my e-mail address - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm64/head.S | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 9b703e79ce..cd03101196 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -117,6 +117,18 @@ #endif /* !CONFIG_EARLY_PRINTK */ +/* + * Pseudo-op for PC relative adr , where is + * within the range +/- 4GB of the PC. + * + * @dst: destination register (64 bit wide) + * @sym: name of the symbol + */ +.macro adr_l, dst, sym +adrp \dst, \sym +add \dst, \dst, :lo12:\sym +.endm + /* Load the physical address of a symbol into xb */ .macro load_paddr xb, sym ldr \xb, =\sym @@ -895,11 +907,9 @@ ENTRY(efi_xen_start) * Flush dcache covering current runtime addresses * of xen text/data. Then flush all of icache. */ -adrp x1, _start -add x1, x1, #:lo12:_start +adr_l x1, _start mov x0, x1 -adrp x2, _end -add x2, x2, #:lo12:_end +adr_l x2, _end sub x1, x2, x1 bl__flush_dcache_area -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 4/7] xen/arm: early_print_info print reserved_mem
Improve early_print_info to also print the banks saved in bootinfo.reserved_mem. Print them right after RESVD, increasing the same index. Since we are at it, also switch the existing RESVD print to use unsigned int. Signed-off-by: Stefano Stabellini --- Changes in v5: - switch to unsigned Changes in v4: - new patch --- xen/arch/arm/bootfdt.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 0b0e22a3d0..32153e6207 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -337,9 +337,10 @@ static int __init early_scan_node(const void *fdt, static void __init early_print_info(void) { struct meminfo *mi = &bootinfo.mem; +struct meminfo *mem_resv = &bootinfo.reserved_mem; struct bootmodules *mods = &bootinfo.modules; struct bootcmdlines *cmds = &bootinfo.cmdlines; -int i, nr_rsvd; +unsigned int i, j, nr_rsvd; for ( i = 0; i < mi->nr_banks; i++ ) printk("RAM: %"PRIpaddr" - %"PRIpaddr"\n", @@ -361,9 +362,15 @@ static void __init early_print_info(void) continue; /* fdt_get_mem_rsv returns length */ e += s; -printk(" RESVD[%d]: %"PRIpaddr" - %"PRIpaddr"\n", +printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, s, e); } +for ( j = 0; j < mem_resv->nr_banks; j++, i++ ) +{ +printk(" RESVD[%u]: %"PRIpaddr" - %"PRIpaddr"\n", i, + mem_resv->bank[j].start, + mem_resv->bank[j].start + mem_resv->bank[j].size - 1); +} printk("\n"); for ( i = 0 ; i < cmds->nr_mods; i++ ) printk("CMDLINE[%"PRIpaddr"]:%s %s\n", cmds->cmdline[i].start, -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] domain: introduce XEN_DOMCTL_CDF_iommu
> -Original Message- > From: Jan Beulich > Sent: 07 August 2019 10:22 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Julien Grall ; > Andrew Cooper > ; Anthony Perard ; > Roger Pau Monne > ; Volodymyr Babchuk ; > George Dunlap > ; Ian Jackson ; Stefano > Stabellini > ; Konrad Rzeszutek Wilk ; Tim > (Xen.org) ; > Wei Liu > Subject: Re: [PATCH 1/6] domain: introduce XEN_DOMCTL_CDF_iommu > > On 30.07.2019 15:44, Paul Durrant wrote: > > --- a/xen/arch/arm/domain.c > > +++ b/xen/arch/arm/domain.c > > @@ -673,8 +673,7 @@ int arch_domain_create(struct domain *d, > > > > ASSERT(config != NULL); > > > > -/* p2m_init relies on some value initialized by the IOMMU subsystem */ > > -if ( (rc = iommu_domain_init(d)) != 0 ) > > +if ( is_iommu_enabled(d) && (rc = iommu_domain_init(d)) != 0 ) > > goto fail; > > Instead of this and ... > > > --- a/xen/arch/x86/domain.c > > +++ b/xen/arch/x86/domain.c > > @@ -604,7 +604,7 @@ int arch_domain_create(struct domain *d, > > if ( (rc = init_domain_irq_mapping(d)) != 0 ) > > goto fail; > > > > -if ( (rc = iommu_domain_init(d)) != 0 ) > > +if ( is_iommu_enabled(d) && (rc = iommu_domain_init(d)) != 0 ) > > goto fail; > > ... this (and any further copies in future ports), wouldn't it > be better to centrally do this in iommu_domain_init() itself? Ok... it kind of seemed more logical to avoid the call if the flag is not present... but there's no real consistency on this kind of thing in the Xen codebase so I'll change it to shorten the patch. > > > --- a/xen/common/domain.c > > +++ b/xen/common/domain.c > > @@ -301,7 +301,8 @@ static int sanitise_domain_config(struct > > xen_domctl_createdomain *config) > > XEN_DOMCTL_CDF_hap | > > XEN_DOMCTL_CDF_s3_integrity | > > XEN_DOMCTL_CDF_oos_off | > > - XEN_DOMCTL_CDF_xs_domain) ) > > + XEN_DOMCTL_CDF_xs_domain | > > + XEN_DOMCTL_CDF_iommu) ) > > { > > dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags); > > return -EINVAL; > > Also refuse XEN_DOMCTL_CDF_iommu when !iommu_enabled? Yes, that would be reasonable. > > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -981,6 +981,11 @@ static inline bool is_xenstore_domain(const struct > > domain *d) > > return d->options & XEN_DOMCTL_CDF_xs_domain; > > } > > > > +static inline bool is_iommu_enabled(const struct domain *d) > > +{ > > +return d->options & XEN_DOMCTL_CDF_iommu; > > +} > > Perhaps wrap in evaluate_nospec()? > Given the codepaths that it will eventually get, yes it probably should have the guard. Paul > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 24/28] xen/arm: Zero BSS after the MMU and D-cache is turned on
At the moment BSS is zeroed before the MMU and D-Cache is turned on. In other words, the cache will be bypassed when zeroing the BSS section. On Arm64, per the Image protocol [1], the state of the cache for BSS region is not known because it is not part of the "loaded kernel image". On Arm32, the boot protocol [2] does not mention anything about the state of the cache. Therefore, it should be assumed that it is not known for BSS region. This means that the cache will need to be invalidated twice for the BSS region: 1) Before zeroing to remove any dirty cache line. Otherwise they may get evicted while zeroing and therefore overriding the value. 2) After zeroing to remove any cache line that may have been speculated. Otherwise when turning on MMU and D-Cache, the CPU may see old values. At the moment, the only reason to have BSS zeroed early is because the boot page tables are part of it. To avoid the two cache invalidations, it would be better if the boot page tables are part of the "loaded kernel image" and therefore be zeroed when loading the image into memory. A good candidate is the section .data.page_aligned. A new macro DEFINE_BOOT_PAGE_TABLE is introduced to create and mark page-tables used before BSS is zeroed. This includes all boot_* but also xen_fixmap as zero_bss() will print a message when earlyprintk is enabled. [1] linux/Documentation/arm64/booting.txt [2] linux/Documentation/arm/Booting Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's reviewed-by Changes in v2: - Add missing signed-off - Clarify commit message - Add arm32 parts --- xen/arch/arm/arm32/head.S | 11 +++ xen/arch/arm/arm64/head.S | 7 +++ xen/arch/arm/mm.c | 23 +-- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 2317fb8855..e86a9f95e7 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -153,7 +153,6 @@ past_zImage: mov r12, #0/* r12 := is_secondary_cpu */ blcheck_cpu_mode -blzero_bss blcpu_init blcreate_page_tables blenable_mmu @@ -174,6 +173,7 @@ primary_switched: /* Use a virtual address to access the UART. */ mov_w r11, EARLY_UART_VIRTUAL_ADDRESS #endif +blzero_bss PRINT("- Ready -\r\n") /* Setup the arguments for start_xen and jump to C world */ mov r0, r10/* r0 := Physical offset */ @@ -284,17 +284,12 @@ ENDPROC(check_cpu_mode) /* * Zero BSS * - * Inputs: - * r10: Physical offset - * * Clobbers r0 - r3 */ zero_bss: PRINT("- Zero BSS -\r\n") -ldr r0, =__bss_start /* Load start & end of bss */ -ldr r1, =__bss_end -add r0, r0, r10/* Apply physical offset */ -add r1, r1, r10 +ldr r0, =__bss_start /* r0 := vaddr(__bss_start) */ +ldr r1, =__bss_end /* r1 := vaddr(__bss_start) */ mov r2, #0 1: str r2, [r0], #4 diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index a6f3aa4ee5..f2a0e1d3b0 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -309,7 +309,6 @@ real_start_efi: mov x22, #0/* x22 := is_secondary_cpu */ blcheck_cpu_mode -blzero_bss blcpu_init blcreate_page_tables blenable_mmu @@ -330,6 +329,7 @@ primary_switched: /* Use a virtual address to access the UART. */ ldr x23, =EARLY_UART_VIRTUAL_ADDRESS #endif +blzero_bss PRINT("- Ready -\r\n") /* Setup the arguments for start_xen and jump to C world */ mov x0, x20/* x0 := Physical offset */ @@ -432,7 +432,6 @@ ENDPROC(check_cpu_mode) * Zero BSS * * Inputs: - * x20: Physical offset * x26: Do we need to zero BSS? * * Clobbers x0 - x3 @@ -442,8 +441,8 @@ zero_bss: cbnz x26, skip_bss PRINT("- Zero BSS -\r\n") -load_paddr x0, __bss_start/* Load paddr of start & end of bss */ -load_paddr x1, __bss_end +ldr x0, =__bss_start /* x0 := vaddr(__bss_start) */ +ldr x1, =__bss_end /* x1 := vaddr(__bss_start) */ 1: str xzr, [x0], #8 cmp x0, x1 diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index e1cdeaaf2f..65552da4ba 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -62,6 +62,17 @@ mm_printk(const char *fmt, ...) {} } while (0); #endif +/* + * Macros to define page-tables: + * - DEFINE_BOOT_PAGE_TABLE is used to define page-table that are used + * in assembly code before BSS is zeroed. + * - DEFINE_PAGE_TABLE{,S} are used to define one or multiple + * page-tables to be used after BSS is zeroed (typical
Re: [Xen-devel] [ANNOUNCE] Xen 4.13 Development Update
> -Original Message- > > === x86 === > > * PV-IOMMU (v7) > - Paul Durrant > Realistically I'm unlikely to get back to the hypercall interface any time soon. There's just too much groundwork to lay, so probably better stop tracking this for now. Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 3/7] xen/arm: keep track of reserved-memory regions
As we parse the device tree in Xen, keep track of the reserved-memory regions as they need special treatment (follow-up patches will make use of the stored information.) Reuse process_memory_node to add reserved-memory regions to the bootinfo.reserved_mem array. Refuse to continue once we reach the max number of reserved memory regions to avoid accidentally mapping any portions of them into a VM. Signed-off-by: Stefano Stabellini --- Changes in v5: - remove unneeded cast - remove unneeded strlen check - don't pass address_cells, size_cells, depth to device_tree_for_each_node Changes in v4: - depth + 1 in process_reserved_memory_node - pass address_cells and size_cells to device_tree_for_each_node - pass struct meminfo * instead of a boolean to process_memory_node - improve in-code comment - use a separate process_reserved_memory_node (separate from process_memory_node) function wrapper to have different error handling Changes in v3: - match only /reserved-memory - put the warning back in place for reg not present on a normal memory region - refuse to continue once we reach the max number of reserved memory regions Changes in v2: - call process_memory_node from process_reserved_memory_node to avoid duplication --- xen/arch/arm/bootfdt.c | 41 +++-- xen/include/asm-arm/setup.h | 1 + 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 590b14304c..0b0e22a3d0 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -136,6 +136,7 @@ static int __init process_memory_node(const void *fdt, int node, const __be32 *cell; paddr_t start, size; u32 reg_cells = address_cells + size_cells; +struct meminfo *mem = data; if ( address_cells < 1 || size_cells < 1 ) return -ENOENT; @@ -147,21 +148,46 @@ static int __init process_memory_node(const void *fdt, int node, cell = (const __be32 *)prop->data; banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32)); -for ( i = 0; i < banks && bootinfo.mem.nr_banks < NR_MEM_BANKS; i++ ) +for ( i = 0; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ ) { device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); if ( !size ) continue; -bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start; -bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size; -bootinfo.mem.nr_banks++; +mem->bank[mem->nr_banks].start = start; +mem->bank[mem->nr_banks].size = size; +mem->nr_banks++; } -if ( bootinfo.mem.nr_banks == NR_MEM_BANKS ) +if ( mem->nr_banks == NR_MEM_BANKS ) return -ENOSPC; return 0; } +static int __init process_reserved_memory_node(const void *fdt, int node, + const char *name, int depth, + u32 address_cells, + u32 size_cells, + void *data) +{ +int rc = process_memory_node(fdt, node, name, depth, address_cells, + size_cells, data); + +if ( rc == -ENOSPC ) +panic("Max number of supported reserved-memory regions reached."); +else if ( rc != -ENOENT ) +return rc; +return 0; +} + +static int __init process_reserved_memory(const void *fdt, int node, + const char *name, int depth, + u32 address_cells, u32 size_cells) +{ +return device_tree_for_each_node(fdt, node, + process_reserved_memory_node, + &bootinfo.reserved_mem); +} + static void __init process_multiboot_node(const void *fdt, int node, const char *name, u32 address_cells, u32 size_cells) @@ -295,7 +321,10 @@ static int __init early_scan_node(const void *fdt, if ( device_tree_node_matches(fdt, node, "memory") ) rc = process_memory_node(fdt, node, name, depth, - address_cells, size_cells, NULL); + address_cells, size_cells, &bootinfo.mem); +else if ( depth == 1 && !strcmp(name, "reserved-memory") ) +rc = process_reserved_memory(fdt, node, name, depth, + address_cells, size_cells); else if ( depth <= 3 && (device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) || device_tree_node_compatible(fdt, node, "multiboot,module" ))) process_multiboot_node(fdt, node, name, address_cells, size_cells); diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index 8bf3d5910a..efcba545c2 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -66,6 +66,7 @@ struc
[Xen-devel] [PATCH v3 15/28] xen/arm32: head: Rework and document zero_bss()
On secondary CPUs, zero_bss() will be a NOP because BSS only need to be zeroed once at boot. So the call in the secondary CPUs path can be removed. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's reviewed-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index c7b4fe4cd4..1189ed6c47 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -192,7 +192,6 @@ GLOBAL(init_secondary) PRINT(" booting -\r\n") #endif blcheck_cpu_mode -blzero_bss blcpu_init blcreate_page_tables blenable_mmu @@ -238,11 +237,15 @@ check_cpu_mode: b fail ENDPROC(check_cpu_mode) +/* + * Zero BSS + * + * Inputs: + * r10: Physical offset + * + * Clobbers r0 - r3 + */ zero_bss: -/* Zero BSS On the boot CPU to avoid nasty surprises */ -teq r12, #0 -bne skip_bss - PRINT("- Zero BSS -\r\n") ldr r0, =__bss_start /* Load start & end of bss */ ldr r1, =__bss_end @@ -254,7 +257,6 @@ zero_bss: cmp r0, r1 blo 1b -skip_bss: mov pc, lr ENDPROC(zero_bss) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] drm/xen-front: Make structure fb_funcs constant
Static structure fb_funcs, of type drm_framebuffer_funcs, is used only when it is passed to drm_gem_fb_create_with_funcs() as its last argument. drm_gem_fb_create_with_funcs does not modify its lst argument (fb_funcs) and hence fb_funcs is never modified. Therefore make fb_funcs constant to protect it from further modification. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta --- drivers/gpu/drm/xen/xen_drm_front_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c index c2955d375394..4a984f4e 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_kms.c +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c @@ -45,7 +45,7 @@ static void fb_destroy(struct drm_framebuffer *fb) drm_gem_fb_destroy(fb); } -static struct drm_framebuffer_funcs fb_funcs = { +static const struct drm_framebuffer_funcs fb_funcs = { .destroy = fb_destroy, }; -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 3/3] xen/blkback: Use refcount_t for refcount
Reference counters are preferred to use refcount_t instead of atomic_t. This is because the implementation of refcount_t can prevent overflows and detect possible use-after-free. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan --- Changes in v2: - Also convert pending_req::pendcnt to refcount_t. drivers/block/xen-blkback/blkback.c | 6 +++--- drivers/block/xen-blkback/common.h | 9 + drivers/block/xen-blkback/xenbus.c | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index fd1e19f1a49f..b24bb0aea35f 100644 --- a/drivers/block/xen-blkback/blkback.c +++ b/drivers/block/xen-blkback/blkback.c @@ -1098,7 +1098,7 @@ static void __end_block_io_op(struct pending_req *pending_req, * the grant references associated with 'request' and provide * the proper response on the ring. */ - if (atomic_dec_and_test(&pending_req->pendcnt)) + if (refcount_dec_and_test(&pending_req->pendcnt)) xen_blkbk_unmap_and_respond(pending_req); } @@ -1395,7 +1395,7 @@ static int dispatch_rw_block_io(struct xen_blkif_ring *ring, bio_set_op_attrs(bio, operation, operation_flags); } - atomic_set(&pending_req->pendcnt, nbio); + refcount_set(&pending_req->pendcnt, nbio); blk_start_plug(&plug); for (i = 0; i < nbio; i++) @@ -1424,7 +1424,7 @@ static int dispatch_rw_block_io(struct xen_blkif_ring *ring, fail_put_bio: for (i = 0; i < nbio; i++) bio_put(biolist[i]); - atomic_set(&pending_req->pendcnt, 1); + refcount_set(&pending_req->pendcnt, 1); __end_block_io_op(pending_req, BLK_STS_RESOURCE); msleep(1); /* back off a bit */ return -EIO; diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h index 1d3002d773f7..824d64a8339b 100644 --- a/drivers/block/xen-blkback/common.h +++ b/drivers/block/xen-blkback/common.h @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -309,7 +310,7 @@ struct xen_blkif { struct xen_vbd vbd; /* Back pointer to the backend_info. */ struct backend_info *be; - atomic_trefcnt; + refcount_t refcnt; /* for barrier (drain) requests */ struct completion drain_complete; atomic_tdrain; @@ -343,7 +344,7 @@ struct pending_req { struct xen_blkif_ring *ring; u64 id; int nr_segs; - atomic_tpendcnt; + refcount_t pendcnt; unsigned short operation; int status; struct list_headfree_list; @@ -362,10 +363,10 @@ struct pending_req { (_v)->bdev->bd_part->nr_sects : \ get_capacity((_v)->bdev->bd_disk)) -#define xen_blkif_get(_b) (atomic_inc(&(_b)->refcnt)) +#define xen_blkif_get(_b) (refcount_inc(&(_b)->refcnt)) #define xen_blkif_put(_b) \ do {\ - if (atomic_dec_and_test(&(_b)->refcnt)) \ + if (refcount_dec_and_test(&(_b)->refcnt)) \ schedule_work(&(_b)->free_work);\ } while (0) diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index 3ac6a5d18071..ecc5f9c5bf3f 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -169,7 +169,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid) return ERR_PTR(-ENOMEM); blkif->domid = domid; - atomic_set(&blkif->refcnt, 1); + refcount_set(&blkif->refcnt, 1); init_completion(&blkif->drain_complete); INIT_WORK(&blkif->free_work, xen_blkif_deferred_free); -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/3] xen/blkback: Use refcount_t for refcount
On Thu, Aug 8, 2019 at 9:35 PM Roger Pau Monné wrote: > > On Thu, Aug 08, 2019 at 09:11:00PM +0800, Chuhong Yuan wrote: > > Reference counters are preferred to use refcount_t instead of > > atomic_t. > > This is because the implementation of refcount_t can prevent > > overflows and detect possible use-after-free. > > So convert atomic_t ref counters to refcount_t. > > Thanks! > > I think there are more reference counters in blkback than > the one you fixed. There's also an inflight field in xen_blkif_ring, > and a pendcnt in pending_req which look like possible candidates to > switch to use refcount_t, have you looked into switching those two > also? > It seems that xen_blkif_ring::inflight is 0-based and cannot be directly converted to refcount_t. This is because the implementation of refcount_t will warn on increasing a 0 ref count. Therefore I only convert pending_req::pendcnt in v2. > Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 140007: regressions - FAIL
flight 140007 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/140007/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-livepatch 7 xen-boot fail REGR. vs. 139876 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 139876 test-amd64-amd64-xl-pvshim 18 guest-localmigrate/x10 fail REGR. vs. 139876 build-amd64-xsm 6 xen-buildfail REGR. vs. 139876 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-arm64-arm64-examine 11 examine-serial/bootloaderfail like 139876 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 139876 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 139876 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 139876 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 139876 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 139876 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 139876 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 139876 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 139876 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 139876 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 save
Re: [Xen-devel] [PATCH v2 15/34] staging/vc04_services: convert put_page() to put_user_page*()
On 05.08.19 00:48, john.hubb...@gmail.com wrote: > From: John Hubbard > > For pages that were retained via get_user_pages*(), release those pages > via the new put_user_page*() routines, instead of via put_page() or > release_pages(). > > This is part a tree-wide conversion, as described in commit fc1d8e7cca2d > ("mm: introduce put_user_page*(), placeholder versions"). > > Acked-by: Greg Kroah-Hartman > > Cc: Eric Anholt > Cc: Stefan Wahren > Cc: Greg Kroah-Hartman > Cc: Mihaela Muraru > Cc: Suniel Mahesh > Cc: Al Viro > Cc: Sidong Yang > Cc: Kishore KP > Cc: linux-rpi-ker...@lists.infradead.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: de...@driverdev.osuosl.org > Signed-off-by: John Hubbard Acked-by: Stefan Wahren ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 140011: all pass - PUSHED
flight 140011 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/140011/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf a0792697bc54e5472e2126c6fbff8031868426a9 baseline version: ovmf 4053587347a4c68402c1fc40921b7f1cdaec900e Last test of basis 139967 2019-08-12 01:54:11 Z1 days Testing same since 140011 2019-08-12 16:12:54 Z0 days1 attempts People who touched revisions under test: Bob Feng Feng, Bob C 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 4053587347..a0792697bc a0792697bc54e5472e2126c6fbff8031868426a9 -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Terminology for "guest" - Was: [PATCH] docs/sphinx: Introduction
On 8/12/19 8:01 AM, Andrew Cooper wrote: On 12/08/2019 15:53, George Dunlap wrote: On 8/8/19 10:13 AM, Julien Grall wrote: Hi Jan, On 08/08/2019 10:04, Jan Beulich wrote: On 08.08.2019 10:43, Andrew Cooper wrote: On 08/08/2019 07:22, Jan Beulich wrote: On 07.08.2019 21:41, Andrew Cooper wrote: --- /dev/null +++ b/docs/glossary.rst @@ -0,0 +1,37 @@ +Glossary + + +.. Terms should appear in alphabetical order + +.. glossary:: + + control domain + A :term:`domain`, commonly dom0, with the permission and responsibility + to create and manage other domains on the system. + + domain + A domain is Xen's unit of resource ownership, and generally has at the + minimum some RAM and virtual CPUs. + + The terms :term:`domain` and :term:`guest` are commonly used + interchangeably, but they mean subtly different things. + + A guest is a single virtual machine. + + Consider the case of live migration where, for a period of time, one + guest will be comprised of two domains, while it is in transit. + + domid + The numeric identifier of a running :term:`domain`. It is unique to a + single instance of Xen, used as the identifier in various APIs, and is + typically allocated sequentially from 0. + + guest + See :term:`domain` I think you want to mention the usual distinction here: Dom0 is, while a domain, commonly not considered a guest. To be honest, I had totally forgotten about that. I guess now is the proper time to rehash it in public. I don't think the way it currently gets used has a clear or coherent set of rules, because I can't think of any to describe how it does get used. Either there are a clear and coherent (and simple!) set of rules for what we mean by "guest", at which point they can live here in the glossary, or the fuzzy way it is current used should cease. What's fuzzy about Dom0 not being a guest (due to being a part of the host instead)? Dom0 is not part of the host if you are using an hardware domain. I actually thought we renamed everything in Xen from Dom0 to hwdom to avoid the confusion. I also would rather avoid to treat "dom0" as not a guest. In normal setup this is a more privilege guest, in other setup this may just be a normal guest (think about partitioning). A literal guest is someone who doesn't live in the building (or work in the buliding, if you're in a hotel). The fact that the staff cleaning rooms are restricted in their privileges doesn't make them guests of the hotel. The toolstack domain, the hardware domain, the driver domain, the xenstore domain, and so on are all part of the host system, designed to allow you to use Xen to do the thing you actually want to do: Run guests. And it's important that we have a word that distinguishes "domains that we only care about because they make it possible to run other domains", and "domains that we actually want to run". "guest / host" is a natural terminology for these. We already have the word "domain", which includes dom0, driver domains, toolstack domains, hardware domains, as well as guest domains. We don't need "guest" to be a synonym for "domain". So... Please can someone propose wording simple, clear wording for what "guest" means. A potentially untrusted domain. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt bisection] complete build-arm64-libvirt
branch xen-unstable xenbranch xen-unstable job build-arm64-libvirt testid libvirt-build Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib https://git.savannah.gnu.org/git/gnulib.git/ Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: libvirt git://libvirt.org/libvirt.git Bug introduced: 55c8d1a95f9736641aab15bd7562db7d6890aceb Bug not present: 463629559d4e534793e69ad84065ab4babc9c021 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/140039/ commit 55c8d1a95f9736641aab15bd7562db7d6890aceb Author: Daniel P. Berrangé Date: Tue Jul 23 11:06:27 2019 +0100 remote: handle autoprobing of driver within virtproxyd The virtproxyd daemon is merely responsible for forwarding RPC calls to one of the other per-driver daemons. As such, it does not have any drivers loaded and so regular auto-probing logic will not work. We need it to be able to handle NULL URIs though, so must implement some kind of alternative probing logic. When running as root this is quite crude. If a per-driver daemon is running, its UNIX socket will exist and we can assume it will accept connections. If the per-driver daemon is not running, but socket autostart is enabled, we again just assume it will accept connections. The is not great, however, because a default install may well have all sockets available for activation. IOW, the virtxend socket may exist, despite the fact that the libxl driver will not actually work. When running as non-root this is slightly easier as we only have two drivers, QEMU and VirtualBox. These daemons will likely not be running and socket activation won't be used either, as libvirt spawns the daemon on demand. So we just check whether the daemon actually is installed. Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/libvirt/build-arm64-libvirt.libvirt-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/libvirt/build-arm64-libvirt.libvirt-build --summary-out=tmp/140039.bisection-summary --basis-template=139829 --blessings=real,real-bisect libvirt build-arm64-libvirt libvirt-build Searching for failure / basis pass: 139972 fail [host=laxton1] / 139853 ok. Failure / basis pass flights: 139972 / 139853 Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib https://git.savannah.gnu.org/git/gnulib.git/ Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest e4c05240bf96218a10cd331813327524cb020621 c8e2eee548e6e81f3fccd31cf9f9a825db7fc8a8 6280c94f306df6a20bbc100ba15a5a81af0366e6 1a624dd7cf0db5783b27e18e6c790178d14a1e6a dbf360567a7da50db4d2f9bde4649aba21aa8106 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 6c9639a72f0ca3a9430ef75f375877182281fdef Basis pass 6c62122e0615172068a6286428740024f8084264 c8e2eee548e6e81f3fccd31cf9f9a825db7fc8a8 6280c94f306df6a20bbc100ba15a5a81af0366e6 111bbcf87621953dc6ceff09f6c953d33c2689d6 dbf360567a7da50db4d2f9bde4649aba21aa8106 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 0a6ad045c5fe5e0463fe32fb8d03b433f58d1841 Generating revisions with ./adhoc-revtuple-generator git://libvirt.org/libvirt.git#6c62122e0615172068a6286428740024f8084264-e4c05240bf96218a10cd331813327524cb020621 https://git.savannah.gnu.org/git/gnulib.git/#c8e2eee548e6e81f3fccd31cf9f9a825db7fc8a8-c8e2eee548e6e81f3fccd31cf9f9a825db7fc8a8 https://gitlab.com/keycodemap/keycodemapdb.git#6280c94f306df6a20bbc100ba15a5a81af0366e6-6280c94f306df6a20bbc100ba15a5a81af0366e6 git://xenbits.xen.org/osstest/ovmf.git#111bbcf87621953dc6ceff09f6c953d33c2689d\ 6-1a624dd7cf0db5783b27e18e6c790178d14a1e6a git://xenbits.xen.org/qemu-xen.git#dbf360567a7da50db4d2f9bde4649aba21aa8106-dbf360567a7da50db4d2f9bde4649aba21aa8106 git://xenbits.xen.org/osstest/seabios.git#30f1e41f04fb4c715d27f987f003cfc31c9ff4f3-30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 git://xenbits.xen.org/xen.git#0a6ad045c5fe5e0463fe32fb8d03b433f58d1841-6c9639a72f0ca3a9430ef75f375877182281fdef Loaded 3001 nodes in revision graph Searching for test results: 139853 pass 6c62122e0615172068a6286428740024f8084264 c8e2eee548e6e81f3fccd31cf9f9a825db7fc8a8 6280c94f306df6a20bbc100ba15a5a81af0366e6 111bbcf87621953dc6ceff09f
[Xen-devel] [linux-4.4 test] 140005: regressions - FAIL
flight 140005 linux-4.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/140005/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-pvshim 18 guest-localmigrate/x10 fail REGR. vs. 139698 build-amd64 6 xen-build fail in 139968 REGR. vs. 139698 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-vhd 7 xen-boot fail pass in 139968 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-ws16-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-shadow1 build-check(1) blocked in 139968 n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-credit1 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-qemuu-nested-amd 1 build-check(1) blocked in 139968 n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-raw1 build-check(1) blocked in 139968 n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-qemuu-nested-intel 1 build-check(1)blocked in 139968 n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked in 139968 n/a test-amd64-i386-pair 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1)blocked in 139968 n/a test-amd64-amd64-xl-qemut-win10-i386 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-pvhv2-intel 1 build-check(1)blocked in 139968 n/a build-amd64-libvirt 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-i386-pvgrub 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qemut-win7-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-i386-freebsd10-amd64 1 build-check(1)blocked in 139968 n/a test-amd64-i386-examine 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qemut-debianhvm-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-pvhv2-amd 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1)blocked in 139968 n/a test-amd64-amd64-xl-qemuu-ovmf-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1)blocked in 139968 n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qcow2 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-pygrub 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-libvirt 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-rtds 1 build-check(1) blocked in 139968 n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qemuu-win10-i386 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked in 139968 n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1)blocked in 139968 n/a test-amd64-amd64-pair 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-amd64-pvgrub 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1)blocked in 139968 n/a test-amd64-amd64-xl-qemuu-win7-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qemuu-ws16-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked in 139968 n/a test-amd64-amd64-xl-pvshim1 build-check(1) blocked in 139968 n/a test-amd64-i386-libvirt 1 build-check(1) blocked in 139968 n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1)blocked
[Xen-devel] [linux-linus test] 139996: regressions - FAIL
flight 139996 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/139996/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-examine 8 reboot fail REGR. vs. 133580 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-freebsd10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-freebsd10-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-libvirt 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 133580 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580 test-amd64-amd64-xl-pvshim 16 guest-localmigrate fail REGR. vs. 133580 build-armhf-pvops 6 kernel-build fail REGR. vs. 133580 build-amd64-xsm 6 xen-buildfail REGR. vs. 133580 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 133580 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580 Tests which did not succeed, but are not blocking: test-armhf-armhf-examine 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pas
[Xen-devel] [linux-next test] 139986: regressions - FAIL
flight 139986 linux-next real [real] http://logs.test-lab.xenproject.org/osstest/logs/139986/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl 18 guest-localmigrate/x10 fail REGR. vs. 139832 test-amd64-amd64-libvirt-vhd 19 guest-destroy fail in 139860 REGR. vs. 139832 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-xsm 11 debian-fixup fail in 139860 pass in 139986 test-amd64-amd64-xl 11 debian-fixup fail in 139860 pass in 139986 test-amd64-amd64-xl-multivcpu 11 debian-fixupfail in 139860 pass in 139986 test-amd64-amd64-xl-qemuu-debianhvm-amd64 16 guest-localmigrate/x10 fail in 139860 pass in 139986 test-amd64-amd64-xl-pvshim 20 guest-start/debian.repeat fail in 139860 pass in 139986 test-amd64-amd64-libvirt-xsm 18 guest-start/debian.repeat fail in 139860 pass in 139986 test-arm64-arm64-xl 16 guest-start/debian.repeat fail in 139860 pass in 139986 test-arm64-arm64-xl-credit1 16 guest-start/debian.repeat fail in 139860 pass in 139986 test-arm64-arm64-xl-xsm 16 guest-start/debian.repeat fail in 139860 pass in 139986 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 18 guest-start/debianhvm.repeat fail in 139860 pass in 139986 test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail in 139860 pass in 139986 test-amd64-amd64-xl-shadow9 leak-check/basis(9)fail pass in 139860 test-amd64-amd64-pair 24 guest-migrate/dst_host/src_host/debian.repeat fail pass in 139860 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 15 depriv-audit-qemu/create fail pass in 139860 test-arm64-arm64-xl-credit2 16 guest-start/debian.repeat fail pass in 139860 test-amd64-amd64-libvirt-vhd 17 guest-start/debian.repeat fail pass in 139860 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 16 guest-localmigrate/x10 fail pass in 139860 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-examine 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-arm64-arm64-examine 11 examine-serial/bootloader fail in 139860 like 139832 test-amd64-i386-xl-raw7 xen-boot fail like 139832 test-amd64-i386-examine 8 reboot fail like 139832 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-boot fail like 139832 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail like 139832 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail like 139832 test-amd64-i386-libvirt 7 xen-boot fail like 139832 test-amd64-i386-freebsd10-i386 7 xen-bootfail like 139832 test-amd64-i386-xl-xsm7 xen-boot fail like 139832 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-boot fail like 139832 test-amd64-i386-xl-shadow 7 xen-boot fail like 139832 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail like 139832 test-amd64-i386-pair 10 xen-boot/src_hostfail like 139832 test-amd64-i386-pair 11 xen-boot/dst_hostfail like 139832 test-amd64-i386-xl-qemuu-win10-i386 7 xen-boot fail like 139832 test-amd64-i386-xl7 xen-boot fail like 139832 test-amd64-i386-libvirt-xsm 7 xen-boot fail like 139832 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-boot fail like 139832 test-amd64-i386-xl-qemut-win10-i386 7 xen-boot fail like 139832 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-bootfail like 139832 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail like 139832 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-boot fail like 139832 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-boot fail like 139832 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-boot fail like 139832 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail like 139832 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-boot fail like 139832 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail like 139832 test-amd64-i386-freebsd10
[Xen-devel] [PATCH v5 2/7] xen/arm: make process_memory_node a device_tree_node_func
Change the signature of process_memory_node to match device_tree_node_func. Thanks to this change, the next patch will be able to use device_tree_for_each_node to call process_memory_node on all the children of a provided node. Return error if there is no reg property or if nr_banks is reached. Let the caller deal with the error. Signed-off-by: Stefano Stabellini --- Changes in v5: - return -ENOENT if address_cells or size_cells are not properly set Changes in v4: - return error if there is no reg propery, remove printk - return error if nr_banks is reached Changes in v3: - improve commit message - check return value of process_memory_node Changes in v2: - new --- xen/arch/arm/bootfdt.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index a872ea57d6..590b14304c 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -125,9 +125,10 @@ int __init device_tree_for_each_node(const void *fdt, int node, return 0; } -static void __init process_memory_node(const void *fdt, int node, - const char *name, - u32 address_cells, u32 size_cells) +static int __init process_memory_node(const void *fdt, int node, + const char *name, int depth, + u32 address_cells, u32 size_cells, + void *data) { const struct fdt_property *prop; int i; @@ -137,18 +138,11 @@ static void __init process_memory_node(const void *fdt, int node, u32 reg_cells = address_cells + size_cells; if ( address_cells < 1 || size_cells < 1 ) -{ -printk("fdt: node `%s': invalid #address-cells or #size-cells", - name); -return; -} +return -ENOENT; prop = fdt_get_property(fdt, node, "reg", NULL); if ( !prop ) -{ -printk("fdt: node `%s': missing `reg' property\n", name); -return; -} +return -ENOENT; cell = (const __be32 *)prop->data; banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32)); @@ -162,6 +156,10 @@ static void __init process_memory_node(const void *fdt, int node, bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size; bootinfo.mem.nr_banks++; } + +if ( bootinfo.mem.nr_banks == NR_MEM_BANKS ) +return -ENOSPC; +return 0; } static void __init process_multiboot_node(const void *fdt, int node, @@ -293,15 +291,18 @@ static int __init early_scan_node(const void *fdt, u32 address_cells, u32 size_cells, void *data) { +int rc = 0; + if ( device_tree_node_matches(fdt, node, "memory") ) -process_memory_node(fdt, node, name, address_cells, size_cells); +rc = process_memory_node(fdt, node, name, depth, + address_cells, size_cells, NULL); else if ( depth <= 3 && (device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) || device_tree_node_compatible(fdt, node, "multiboot,module" ))) process_multiboot_node(fdt, node, name, address_cells, size_cells); else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") ) process_chosen_node(fdt, node, name, address_cells, size_cells); -return 0; +return rc; } static void __init early_print_info(void) -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 1/7] xen/arm: pass node to device_tree_for_each_node
Add a new parameter to device_tree_for_each_node: node, the node to start the search from. Passing 0 triggers the old behavior. Set min_depth to depth of the current node + 1 and replace the for loop with a do/while loop to avoid scanning siblings of the initial node passed as an argument. We need this change because in follow-up patches we want to be able to use reuse device_tree_for_each_node to call a function for each children nodes of a provided node and the node itself. Signed-off-by: Stefano Stabellini --- Changes in v5: - go back to v3 - code style improvement in acpi/boot.c - improve comments and commit message - increase min_depth to avoid parsing siblings - replace for with do/while loop and increase min_depth to avoid scanning siblings of the initial node - pass only node, calculate depth Changes in v3: - improve commit message - improve in-code comments - improve code style Changes in v2: - new --- xen/arch/arm/acpi/boot.c | 8 +--- xen/arch/arm/bootfdt.c| 19 ++- xen/include/xen/device_tree.h | 6 +++--- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c index 9b29769a10..d4957cca06 100644 --- a/xen/arch/arm/acpi/boot.c +++ b/xen/arch/arm/acpi/boot.c @@ -246,9 +246,11 @@ int __init acpi_boot_table_init(void) * - the device tree is not empty (it has more than just a /chosen node) * and ACPI has not been force enabled (acpi=force) */ -if ( param_acpi_off || ( !param_acpi_force - && device_tree_for_each_node(device_tree_flattened, - dt_scan_depth1_nodes, NULL))) +if ( param_acpi_off) +goto disable; + if ( !param_acpi_force && +device_tree_for_each_node(device_tree_flattened, 0, + dt_scan_depth1_nodes, NULL) ) goto disable; /* diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index 891b4b66ff..a872ea57d6 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -77,6 +77,7 @@ static u32 __init device_tree_get_u32(const void *fdt, int node, /** * device_tree_for_each_node - iterate over all device tree nodes * @fdt: flat device tree. + * @node: node to start the search from * @func: function to call for each node. * @data: data to pass to @func. * @@ -85,20 +86,17 @@ static u32 __init device_tree_get_u32(const void *fdt, int node, * Returns 0 if all nodes were iterated over successfully. If @func * returns a value different from 0, that value is returned immediately. */ -int __init device_tree_for_each_node(const void *fdt, +int __init device_tree_for_each_node(const void *fdt, int node, device_tree_node_func func, void *data) { -int node; -int depth; +int depth = fdt_node_depth(fdt, node); +int min_depth = depth + 1; u32 address_cells[DEVICE_TREE_MAX_DEPTH]; u32 size_cells[DEVICE_TREE_MAX_DEPTH]; int ret; -for ( node = 0, depth = 0; - node >=0 && depth >= 0; - node = fdt_next_node(fdt, node, &depth) ) -{ +do { const char *name = fdt_get_name(fdt, node, NULL); u32 as, ss; @@ -120,7 +118,10 @@ int __init device_tree_for_each_node(const void *fdt, ret = func(fdt, node, name, depth, as, ss, data); if ( ret != 0 ) return ret; -} + +node = fdt_next_node(fdt, node, &depth); +} while ( node >= 0 && depth >= min_depth ); + return 0; } @@ -357,7 +358,7 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t paddr) add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false); -device_tree_for_each_node((void *)fdt, early_scan_node, NULL); +device_tree_for_each_node((void *)fdt, 0, early_scan_node, NULL); early_print_info(); return fdt_totalsize(fdt); diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index 83156297e2..9a7a8f2dab 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -158,9 +158,9 @@ typedef int (*device_tree_node_func)(const void *fdt, extern const void *device_tree_flattened; -int device_tree_for_each_node(const void *fdt, - device_tree_node_func func, - void *data); +int device_tree_for_each_node(const void *fdt, int node, + device_tree_node_func func, + void *data); /** * dt_unflatten_host_device_tree - Unflatten the host device tree -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 5/7] xen/arm: handle reserved-memory in consider_modules and dt_unreserved_regions
reserved-memory regions overlap with memory nodes. The overlapping memory is reserved-memory and should be handled accordingly: consider_modules and dt_unreserved_regions should skip these regions the same way they are already skipping mem-reserve regions. Signed-off-by: Stefano Stabellini Acked-by: Julien Grall --- Changes in v4: - code style - add acked-by Changes in v3: - coding style - in-code comments Changes in v2: - fix commit message: full overlap - remove check_reserved_memory - extend consider_modules and dt_unreserved_regions --- xen/arch/arm/setup.c | 53 +--- 1 file changed, 50 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 215746a5c3..bc4082296e 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -206,6 +206,28 @@ void __init dt_unreserved_regions(paddr_t s, paddr_t e, } } +/* + * i is the current bootmodule we are evaluating across all possible + * kinds. + * + * When retrieving the corresponding reserved-memory addresses + * below, we need to index the bootinfo.reserved_mem bank starting + * from 0, and only counting the reserved-memory modules. Hence, + * we need to use i - nr. + */ +for ( ; i - nr < bootinfo.reserved_mem.nr_banks; i++ ) +{ +paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start; +paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i - nr].size; + +if ( s < r_e && r_s < e ) +{ +dt_unreserved_regions(r_e, e, cb, i + 1); +dt_unreserved_regions(s, r_s, cb, i + 1); +return; +} +} + cb(s, e); } @@ -392,7 +414,7 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, { const struct bootmodules *mi = &bootinfo.modules; int i; -int nr_rsvd; +int nr; s = (s+align-1) & ~(align-1); e = e & ~(align-1); @@ -418,9 +440,9 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, /* Now check any fdt reserved areas. */ -nr_rsvd = fdt_num_mem_rsv(device_tree_flattened); +nr = fdt_num_mem_rsv(device_tree_flattened); -for ( ; i < mi->nr_mods + nr_rsvd; i++ ) +for ( ; i < mi->nr_mods + nr; i++ ) { paddr_t mod_s, mod_e; @@ -442,6 +464,31 @@ static paddr_t __init consider_modules(paddr_t s, paddr_t e, return consider_modules(s, mod_s, size, align, i+1); } } + +/* + * i is the current bootmodule we are evaluating, across all + * possible kinds of bootmodules. + * + * When retrieving the corresponding reserved-memory addresses, we + * need to index the bootinfo.reserved_mem bank starting from 0, and + * only counting the reserved-memory modules. Hence, we need to use + * i - nr. + */ +nr += mi->nr_mods; +for ( ; i - nr < bootinfo.reserved_mem.nr_banks; i++ ) +{ +paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start; +paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i - nr].size; + +if ( s < r_e && r_s < e ) +{ +r_e = consider_modules(r_e, e, size, align, i + 1); +if ( r_e ) +return r_e; + +return consider_modules(s, r_s, size, align, i + 1); +} +} return e; } #endif -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 7/7] xen/arm: add reserved-memory regions to the dom0 memory node
Reserved memory regions are automatically remapped to dom0. Their device tree nodes are also added to dom0 device tree. However, the dom0 memory node is not currently extended to cover the reserved memory regions ranges as required by the spec. This commit fixes it. Change make_memory_node to take a struct meminfo * instead of a kernel_info. Call it twice for dom0, once to create the first regular memory node, and the second time to create a second memory node with the ranges covering reserved-memory regions. Also, make a small code style fix in make_memory_node. Signed-off-by: Stefano Stabellini Acked-by: Julien Grall --- Changes in v5: - add acked-by Changes in v4: - pass struct meminfo * to make_memory_node - call make_memory_node twice for dom0, once for normal memory, once for reserved-memory regions --- xen/arch/arm/domain_build.c | 22 +++--- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index e0c0c01c88..d11cd40ba1 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -639,11 +639,11 @@ static int __init fdt_property_interrupts(void *fdt, gic_interrupt_t *intr, static int __init make_memory_node(const struct domain *d, void *fdt, int addrcells, int sizecells, - const struct kernel_info *kinfo) + struct meminfo *mem) { int res, i; int reg_size = addrcells + sizecells; -int nr_cells = reg_size*kinfo->mem.nr_banks; +int nr_cells = reg_size * mem->nr_banks; __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */]; __be32 *cells; @@ -662,10 +662,10 @@ static int __init make_memory_node(const struct domain *d, return res; cells = ®[0]; -for ( i = 0 ; i < kinfo->mem.nr_banks; i++ ) +for ( i = 0 ; i < mem->nr_banks; i++ ) { -u64 start = kinfo->mem.bank[i].start; -u64 size = kinfo->mem.bank[i].size; +u64 start = mem->bank[i].start; +u64 size = mem->bank[i].size; dt_dprintk(" Bank %d: %#"PRIx64"->%#"PRIx64"\n", i, start, start + size); @@ -1485,10 +1485,18 @@ static int __init handle_node(struct domain *d, struct kernel_info *kinfo, if ( res ) return res; -res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo); +res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo->mem); if ( res ) return res; +/* + * Create a second memory node to store the ranges covering + * reserved-memory regions. + */ +res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, + &bootinfo.reserved_mem); +if ( res ) +return res; } res = fdt_end_node(kinfo->fdt); @@ -1744,7 +1752,7 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo) if ( ret ) goto err; -ret = make_memory_node(d, kinfo->fdt, addrcells, sizecells, kinfo); +ret = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo->mem); if ( ret ) goto err; -- 2.17.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen/arm: p2m: Free the p2m entry after flushing the IOMMU TLBs
When freeing a p2m entry, all the sub-tree behind it will also be freed. This may include intermediate page-tables or any l3 entry requiring to drop a reference (e.g for foreign pages). As soon as pages are freed, they may be re-used by Xen or another domain. Therefore it is necessary to flush *all* the TLBs beforehand. While CPU TLBs will be flushed before freeing the pages, this is not the case for IOMMU TLBs. This can be solved by moving the IOMMU TLBs flush earlier in the code. This wasn't considered as a security issue as device passthrough on Arm is not security supported. Signed-off-by: Julien Grall --- Cc: olekst...@gmail.com Cc: oleksandr_tyshche...@epam.com I discovered it while looking at the code, so I don't have any reproducer of the issue. There is a small windows where page could be reallocated to Xen or another domain but still present in the IOMMU TLBs. This patch only address the case where the flush succeed. In the unlikely case where it does not succeed, then we will still free the pages. The IOMMU helper will crash domain, but the device may still not be quiescent. So there are a potentially issues do DMA on wrong things. At the moment, none of the Arm IOMMUs drivers (including the IPMMU one under review) are return an error here. Note that flush may still fail (see timeout), but is ignored. This is not great as it means a device may DMA into something that does not belong to the domain. So we probably want to return an error here. Even if an error is returned, there are still potential issues (see above). The fix is not entirely trivial, we would need to keep the page around until the a device is quiescent or the IOMMU is reset. This mostly likely means until the domain is fully destroyed. One of the solution would be to: 1) Have a pool of memory for each domain p2m page-tables. So the domain can only touch itself 2) Defer foreign mapping removal 1) should also solve the case where the P2M is trying to shatter everything and therefore hog the memory. Note that today we don't free empty page-tables. 2) I always felt trying to remove the foreign page reference in the p2m code was wrong. This is done because we currently allow the guest to remove any mapping. So we need to protect ourself against a rogue guest. We could try to restrict what the guest can do on the p2m. --- xen/arch/arm/p2m.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 3c8287a048..963cd1d600 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1048,14 +1048,6 @@ static int __p2m_set_entry(struct p2m_domain *p2m, p2m->lowest_mapped_gfn = gfn_min(p2m->lowest_mapped_gfn, sgfn); } -/* - * Free the entry only if the original pte was valid and the base - * is different (to avoid freeing when permission is changed). - */ -if ( p2m_is_valid(orig_pte) && - !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) ) -p2m_free_entry(p2m, orig_pte, level); - if ( has_iommu_pt(p2m->domain) && (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) ) { @@ -1072,6 +1064,14 @@ static int __p2m_set_entry(struct p2m_domain *p2m, else rc = 0; +/* + * Free the entry only if the original pte was valid and the base + * is different (to avoid freeing when permission is changed). + */ +if ( p2m_is_valid(orig_pte) && + !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) ) +p2m_free_entry(p2m, orig_pte, level); + out: unmap_domain_page(table); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 140013: tolerable all pass - PUSHED
flight 140013 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/140013/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 6a4a62534853b4d20b44990e0d56c665b1ff55ae baseline version: xen 3fda2214f1a7ff972427812e50dc6f1f61cf594f Last test of basis 140001 2019-08-12 14:00:35 Z0 days Testing same since 140013 2019-08-12 17:00:57 Z0 days1 attempts People who touched revisions under test: Andrew Cooper 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 3fda2214f1..6a4a625348 6a4a62534853b4d20b44990e0d56c665b1ff55ae -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2 2/6] iommu/arm: Add ability to handle deferred probing request
Hi Oleksandr, On 8/12/19 1:01 PM, Oleksandr wrote: On 12.08.19 14:11, Julien Grall wrote: On 02/08/2019 17:39, Oleksandr Tyshchenko wrote: From: Oleksandr Tyshchenko This patch adds minimal required support to General IOMMU framework to be able to handle a case when IOMMU driver requesting deferred probing for a device. In order not to pull Linux's error code (-EPROBE_DEFER) to Xen we have chosen -EAGAIN to be used for indicating that device probing is deferred. This is needed for the upcoming IPMMU driver which may request deferred probing depending on what device will be probed the first (there is some dependency between these devices, Root device must be registered before Cache devices. If not the case, driver will deny further Cache device probes until Root device is registered). As we can't guarantee a fixed pre-defined order for the device nodes in DT, we need to be ready for the situation where devices being probed in "any" order. Signed-off-by: Oleksandr Tyshchenko --- xen/common/device_tree.c | 1 + xen/drivers/passthrough/arm/iommu.c | 35 ++- xen/include/asm-arm/device.h | 6 +- xen/include/xen/device_tree.h | 1 + 4 files changed, 41 insertions(+), 2 deletions(-) diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index e107c6f..6f37448 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -1774,6 +1774,7 @@ static unsigned long __init unflatten_dt_node(const void *fdt, /* By default the device is not protected */ np->is_protected = false; INIT_LIST_HEAD(&np->domain_list); + INIT_LIST_HEAD(&np->deferred_probe); I am not entirely happy to add a new list_head field per node just for the benefits of boot code. Could we re-use domain_list (with a comment in the code and appropriate ASSERT)? Agree that only boot code uses deferred_probe field. I will consider re-using domain_list. Could you please clarify regarding ASSERT (where to put and what to check). What I meant is adding an ASSERT to check that np->domain_list is at empty at least before trying to add in the list. This would help to debug any potential issue if we end up to use domain_list earlier in the future. I can't see why it would as iommu is called earlier, but who knows :). if ( new_format ) { diff --git a/xen/drivers/passthrough/arm/iommu.c b/xen/drivers/passthrough/arm/iommu.c index 2135233..3195919 100644 --- a/xen/drivers/passthrough/arm/iommu.c +++ b/xen/drivers/passthrough/arm/iommu.c @@ -20,6 +20,12 @@ #include #include +/* + * Used to keep track of devices for which driver requested deferred probing + * (returns -EAGAIN). + */ +static LIST_HEAD(deferred_probe_list); This wants to be in init section as this is only used at boot. Will do. + static const struct iommu_ops *iommu_ops; const struct iommu_ops *iommu_get_ops(void) @@ -42,7 +48,7 @@ void __init iommu_set_ops(const struct iommu_ops *ops) int __init iommu_hardware_setup(void) { - struct dt_device_node *np; + struct dt_device_node *np, *tmp; int rc; unsigned int num_iommus = 0; @@ -51,6 +57,33 @@ int __init iommu_hardware_setup(void) rc = device_init(np, DEVICE_IOMMU, NULL); if ( !rc ) num_iommus++; + else if (rc == -EAGAIN) + /* + * Driver requested deferred probing, so add this device to + * the deferred list for further processing. + */ + list_add(&np->deferred_probe, &deferred_probe_list); + } + + /* + * Process devices in the deferred list if at least one successfully + * probed device is present. + */ I think this can turn into an infinite loop if all device in deferred_probe_list still return -EDEFER_PROBE and num_iommus is a non-zero. Agree. A better condition would be to check that at least one IOMMU is added at each loop. If not, then we should bail with an error because it likely means something is buggy. Sounds reasonable. Will do. Just to clarify: >>> A better condition would be to check that at least one IOMMU is added at each loop. Maybe, not only added (rc == 0), but driver didn't request deferred probe (rc != -EAGAIN). I think adding an IOMMU is enough. If you return an error other than -EAGAIN here after deferring probing, then you are likely going to fail at the next loop. So better to stop early. I realize this not what the current code is doing (I know I wrote it ;)). But I am not sure it is sane to continue if only part of the IOMMUs are initialized. Most likely you will see an error much later that may be not trivial to find out. Imagine you want to passthrough you network card to a guest but the IOMMU initialization failed... Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.or
[Xen-devel] [libvirt bisection] complete build-i386-libvirt
branch xen-unstable xenbranch xen-unstable job build-i386-libvirt testid libvirt-build Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib https://git.savannah.gnu.org/git/gnulib.git/ Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: libvirt git://libvirt.org/libvirt.git Bug introduced: 55c8d1a95f9736641aab15bd7562db7d6890aceb Bug not present: 463629559d4e534793e69ad84065ab4babc9c021 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/140014/ commit 55c8d1a95f9736641aab15bd7562db7d6890aceb Author: Daniel P. Berrangé Date: Tue Jul 23 11:06:27 2019 +0100 remote: handle autoprobing of driver within virtproxyd The virtproxyd daemon is merely responsible for forwarding RPC calls to one of the other per-driver daemons. As such, it does not have any drivers loaded and so regular auto-probing logic will not work. We need it to be able to handle NULL URIs though, so must implement some kind of alternative probing logic. When running as root this is quite crude. If a per-driver daemon is running, its UNIX socket will exist and we can assume it will accept connections. If the per-driver daemon is not running, but socket autostart is enabled, we again just assume it will accept connections. The is not great, however, because a default install may well have all sockets available for activation. IOW, the virtxend socket may exist, despite the fact that the libxl driver will not actually work. When running as non-root this is slightly easier as we only have two drivers, QEMU and VirtualBox. These daemons will likely not be running and socket activation won't be used either, as libvirt spawns the daemon on demand. So we just check whether the daemon actually is installed. Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrangé For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/libvirt/build-i386-libvirt.libvirt-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/libvirt/build-i386-libvirt.libvirt-build --summary-out=tmp/140014.bisection-summary --basis-template=139829 --blessings=real,real-bisect libvirt build-i386-libvirt libvirt-build Searching for failure / basis pass: 139972 fail [host=albana1] / 139853 [host=baroque1] 139829 [host=chardonnay1] 139790 [host=italia1] 139756 [host=huxelrebe1] 139688 [host=huxelrebe1] 139663 [host=huxelrebe0] 139627 [host=debina0] 139585 ok. Failure / basis pass flights: 139972 / 139585 (tree with no url: minios) Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib https://git.savannah.gnu.org/git/gnulib.git/ Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest e4c05240bf96218a10cd331813327524cb020621 c8e2eee548e6e81f3fccd31cf9f9a825db7fc8a8 6280c94f306df6a20bbc100ba15a5a81af0366e6 1a624dd7cf0db5783b27e18e6c790178d14a1e6a d0d8ad39ecb51cd7497cd524484fe09f50876798 dbf360567a7da50db4d2f9bde4649aba21aa8106 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 6c9639a72f0ca3a9430ef75f375877182281fdef Basis pass b4ab33b90bc2fce87bdcd7aad7669eff3bd79924 8089c00979a5b089cff592c6b91420e595657167 6280c94f306df6a20bbc100ba15a5a81af0366e6 b3d00df69c78fa0e12986a7ff334689a76f4578a d0d8ad39ecb51cd7497cd524484fe09f50876798 1bcf484fa9f451cc8c290fe80fd0e764199ca81c 30f1e41f04fb4c715d27f987f003cfc31c9ff4f3 22ec7474348fea2c4a32b0872dd3385bf3785a26 Generating revisions with ./adhoc-revtuple-generator git://libvirt.org/libvirt.git#b4ab33b90bc2fce87bdcd7aad7669eff3bd79924-e4c05240bf96218a10cd331813327524cb020621 https://git.savannah.gnu.org/git/gnulib.git/#8089c00979a5b089cff592c6b91420e595657167-c8e2eee548e6e81f3fccd31cf9f9a825db7fc8a8 https://gitlab.com/keycodemap/keycodemapdb.git#6280c94f306df6a20bbc100ba15a5a81af0366e6-6280c94f306df6a20bbc100ba15a5a81af0366e6 git://xenbits.xen.org/osstest/ovmf.git#b3d00df69c78fa0e12986a7ff334689a76f4578\ a-1a624dd7cf0db5783b27e18e6c790178d14a1e6a git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#1bcf484fa9f451cc8c290fe80f
[Xen-devel] [PATCH] x86/desc: Move boot_gdtr into .rodata
It is never written to. This was an oversight when it was moved from asm into C. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné --- xen/arch/x86/desc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/x86/desc.c b/xen/arch/x86/desc.c index 42ccdc2f8c..dfeb1beaa8 100644 --- a/xen/arch/x86/desc.c +++ b/xen/arch/x86/desc.c @@ -89,7 +89,7 @@ seg_desc_t boot_compat_gdt[PAGE_SIZE / sizeof(seg_desc_t)] = * References boot_cpu_gdt_table for a short period, until the CPUs switch * onto their per-CPU GDTs. */ -struct desc_ptr boot_gdtr = { +const struct desc_ptr boot_gdtr = { .limit = LAST_RESERVED_GDT_BYTE, .base = (unsigned long)(boot_gdt - FIRST_RESERVED_GDT_ENTRY), }; -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/suspend: Simplify system table handling on resume
load_TR() is used exclusively in the resume path, but jumps through a lot of unnecessary hoops. As suspend/resume is strictly on CPU0 in idle context, the correct GDT to use is boot_gdt, which means it doesn't need saving on suspend. Similarly, the correct IDT to use can be derived, and the LDT is guaranteed to be NUL. The TR is still correct in the GDT, but needs the busy bit clearing before we can reload it. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné A slightly different option would be to call load_system_tables() rather than opencoding part of it in restore_rest_processor_state(). However, that is more setup than is necessary. Thoughts? --- xen/arch/x86/acpi/suspend.c | 14 +- xen/arch/x86/acpi/wakeup_prot.S | 13 + xen/arch/x86/traps.c| 21 - xen/include/asm-x86/desc.h | 2 -- 4 files changed, 14 insertions(+), 36 deletions(-) diff --git a/xen/arch/x86/acpi/suspend.c b/xen/arch/x86/acpi/suspend.c index ba9d2e13a7..a6f2584645 100644 --- a/xen/arch/x86/acpi/suspend.c +++ b/xen/arch/x86/acpi/suspend.c @@ -41,7 +41,19 @@ void save_rest_processor_state(void) void restore_rest_processor_state(void) { -load_TR(); +unsigned int cpu = smp_processor_id(); +seg_desc_t *gdt = per_cpu(gdt, cpu) - FIRST_RESERVED_GDT_ENTRY; +struct tss64 *tss = &per_cpu(tss_page, cpu).tss; +const struct desc_ptr idtr = { +.base = (unsigned long)idt_tables[cpu], +.limit = (IDT_ENTRIES * sizeof(idt_entry_t)) - 1, +}; + +_set_tssldt_desc(gdt + TSS_ENTRY, (unsigned long)tss, + sizeof(*tss) - 1, SYS_DESC_tss_avail); +lidt(&idtr); +ltr(TSS_SELECTOR); +lldt(0); /* Recover syscall MSRs */ wrmsrl(MSR_LSTAR, saved_lstar); diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S index 9e9fcc1ab6..74261cb4f1 100644 --- a/xen/arch/x86/acpi/wakeup_prot.S +++ b/xen/arch/x86/acpi/wakeup_prot.S @@ -34,10 +34,6 @@ ENTRY(do_suspend_lowlevel) mov %ss, REF(saved_ss) -sgdtREF(saved_gdt) -sidtREF(saved_idt) -sldtREF(saved_ldt) - mov %cr0, GREG(ax) mov GREG(ax), REF(saved_cr0) @@ -55,6 +51,7 @@ ENTRY(do_suspend_lowlevel) ENTRY(__ret_point) +lgdtboot_gdtr(%rip) /* mmu_cr4_features contains latest cr4 setting */ mov REF(mmu_cr4_features), GREG(ax) @@ -66,10 +63,6 @@ ENTRY(__ret_point) mov REF(saved_cr0), GREG(ax) mov GREG(ax), %cr0 -lgdtREF(saved_gdt) -lidtREF(saved_idt) -lldtREF(saved_ldt) - mov REF(saved_ss), %ss LOAD_GREG(sp) @@ -129,9 +122,5 @@ DECLARE_GREG(13) DECLARE_GREG(14) DECLARE_GREG(15) -saved_gdt: .quad 0,0 -saved_idt: .quad 0,0 -saved_ldt: .quad 0,0 - saved_cr0: .quad 0 saved_cr3: .quad 0 diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 23069e25ec..b424687f85 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1901,27 +1901,6 @@ static void __init set_intr_gate(unsigned int n, void *addr) __set_intr_gate(n, 0, addr); } -void load_TR(void) -{ -struct tss64 *tss = &this_cpu(tss_page).tss; -struct desc_ptr old_gdt, tss_gdt = { -.base = (long)(this_cpu(gdt) - FIRST_RESERVED_GDT_ENTRY), -.limit = LAST_RESERVED_GDT_BYTE -}; - -_set_tssldt_desc( -this_cpu(gdt) + TSS_ENTRY - FIRST_RESERVED_GDT_ENTRY, -(unsigned long)tss, sizeof(*tss) - 1, SYS_DESC_tss_avail); -_set_tssldt_desc( -this_cpu(compat_gdt) + TSS_ENTRY - FIRST_RESERVED_GDT_ENTRY, -(unsigned long)tss, sizeof(*tss) - 1, SYS_DESC_tss_busy); - -/* Switch to non-compat GDT (which has B bit clear) to execute LTR. */ -asm volatile ( -"sgdt %0; lgdt %2; ltr %w1; lgdt %0" -: "=m" (old_gdt) : "rm" (TSS_SELECTOR), "m" (tss_gdt) : "memory" ); -} - static unsigned int calc_ler_msr(void) { switch ( boot_cpu_data.x86_vendor ) diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h index 603b9a9013..24db3e9510 100644 --- a/xen/include/asm-x86/desc.h +++ b/xen/include/asm-x86/desc.h @@ -216,8 +216,6 @@ DECLARE_PER_CPU(seg_desc_t *, compat_gdt); DECLARE_PER_CPU(l1_pgentry_t, compat_gdt_l1e); DECLARE_PER_CPU(bool, full_gdt_loaded); -extern void load_TR(void); - static inline void lgdt(const struct desc_ptr *gdtr) { __asm__ __volatile__ ( "lgdt %0" :: "m" (*gdtr) : "memory" ); -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/suspend: Simplify resume path
This started off as "get rid of load_TR()" as identified in the TSS cleanup series, and morphed slightly. Andrew Cooper (3): x86/suspend: Sanity check more properties in enter_state() x86/desc: Move boot_gdtr into .rodata x86/suspend: Simplify system table handling on resume xen/arch/x86/acpi/power.c | 2 ++ xen/arch/x86/acpi/suspend.c | 14 +- xen/arch/x86/acpi/wakeup_prot.S | 13 + xen/arch/x86/desc.c | 2 +- xen/arch/x86/traps.c| 21 - xen/include/asm-x86/desc.h | 2 -- 6 files changed, 17 insertions(+), 37 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.19 test] 139966: regressions - FAIL
flight 139966 linux-4.19 real [real] http://logs.test-lab.xenproject.org/osstest/logs/139966/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-xsm 6 xen-buildfail REGR. vs. 129313 build-armhf-pvops 6 kernel-build fail REGR. vs. 129313 Tests which are failing intermittently (not blocking): test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail pass in 139883 test-arm64-arm64-libvirt-xsm 7 xen-boot fail pass in 139922 test-arm64-arm64-examine 11 examine-serial/bootloader fail pass in 139922 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-multivcpu 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-credit1 1 build-check(1) blocked n/a test-armhf-armhf-xl-arndale 1 build-check(1) blocked n/a test-amd64-amd64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-cubietruck 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl-rtds 1 build-check(1) blocked n/a test-armhf-armhf-examine 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 13 migrate-support-check fail in 139922 never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-check fail in 139922 never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail in 139922 never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail in 139922 never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-check fail in 139922 never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-check fail in 139922 never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop
Re: [Xen-devel] [PATCH] x86/AMD-Vi: Fold exit paths of {enable, disable}_iommu()
On Mon, Aug 12, 2019 at 06:52:05PM +0100, Andy Cooper wrote: > ... to avoid having multiple spin_unlock_irqrestore() calls. > > Signed-off-by: Andrew Cooper Acked-by: Brian Woods > --- > CC: Jan Beulich > CC: Wei Liu > CC: Roger Pau Monné > CC: Boris Ostrovsky > CC: Suravee Suthikulpanit > CC: Brian Woods > > Interestingly GCC 6.3 managed to fold disable_iommu() automatically. There is > some partial folding for enable_iommu() (insofar as there is only a single > call to _spin_unlock_irqrestore emitted), but this delta yeilds > > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-20 (-20) > Function old new delta > enable_iommu18441824 -20 > Total: Before=3340299, After=3340279, chg -0.00% > > which means that something wasn't done automatically. > > Noticed while investigating the S3 regression. > --- > xen/drivers/passthrough/amd/iommu_init.c | 17 +++-- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/xen/drivers/passthrough/amd/iommu_init.c > b/xen/drivers/passthrough/amd/iommu_init.c > index bb9f33e264..bb5a3e57c9 100644 > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -899,11 +899,8 @@ static void enable_iommu(struct amd_iommu *iommu) > > spin_lock_irqsave(&iommu->lock, flags); > > -if ( iommu->enabled ) > -{ > -spin_unlock_irqrestore(&iommu->lock, flags); > -return; > -} > +if ( unlikely(iommu->enabled) ) > +goto out; > > amd_iommu_erratum_746_workaround(iommu); > > @@ -957,6 +954,8 @@ static void enable_iommu(struct amd_iommu *iommu) > amd_iommu_flush_all_caches(iommu); > > iommu->enabled = 1; > + > + out: > spin_unlock_irqrestore(&iommu->lock, flags); > } > > @@ -966,11 +965,8 @@ static void disable_iommu(struct amd_iommu *iommu) > > spin_lock_irqsave(&iommu->lock, flags); > > -if ( !iommu->enabled ) > -{ > -spin_unlock_irqrestore(&iommu->lock, flags); > -return; > -} > +if ( unlikely(!iommu->enabled) ) > +goto out; > > if ( !iommu->ctrl.int_cap_xt_en ) > amd_iommu_msi_enable(iommu, IOMMU_CONTROL_DISABLED); > @@ -988,6 +984,7 @@ static void disable_iommu(struct amd_iommu *iommu) > > iommu->enabled = 0; > > + out: > spin_unlock_irqrestore(&iommu->lock, flags); > } > > -- > 2.11.0 > -- Brian Woods ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 09/28] xen/arm32: head: Mark the end of subroutines with ENDPROC
On 12/08/2019 18:30, Julien Grall wrote: putn() and puts() are two subroutines. Add ENDPROC for the benefits of static analysis tools and the reader. Signed-off-by: Julien Grall Acked-by: Stefano Stabellini Hmmm, I made a typo in Stefano's e-mail address. I can fix it on commit message (or next version). Cheers, --- Changes in v3: - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 99f4af18d8..8b4c8a4714 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -518,6 +518,7 @@ puts: moveq pc, lr early_uart_transmit r11, r1 b puts +ENDPROC(puts) /* * Print a 32-bit number in hex. Specific to the PL011 UART. @@ -537,6 +538,7 @@ putn: subs r3, r3, #1 bne 1b mov pc, lr +ENDPROC(putn) hex:.ascii "0123456789abcdef" .align 2 -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 04/28] xen/arm64: head: Rework and document launch()
Boot CPU and secondary CPUs will use different entry point to C code. At the moment, the decision on which entry to use is taken within launch(). In order to avoid a branch for the decision and make the code clearer, launch() is reworked to take in parameters the entry point and its arguments. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Indent all the comments the same way - Add Stefano's reviewed-by Changes in v2: - Use x3 instead of x4 - Add a clobbers section --- xen/arch/arm/arm64/head.S | 43 +++ 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 7b6b820726..925fb93e58 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -318,6 +318,11 @@ primary_switched: /* Use a virtual address to access the UART. */ ldr x23, =EARLY_UART_VIRTUAL_ADDRESS #endif +PRINT("- Ready -\r\n") +/* Setup the arguments for start_xen and jump to C world */ +mov x0, x20/* x0 := Physical offset */ +mov x1, x21/* x1 := paddr(FDT) */ +ldr x2, =start_xen b launch ENDPROC(real_start) @@ -380,6 +385,9 @@ secondary_switched: /* Use a virtual address to access the UART. */ ldr x23, =EARLY_UART_VIRTUAL_ADDRESS #endif +PRINT("- Ready -\r\n") +/* Jump to C world */ +ldr x2, =start_secondary b launch ENDPROC(init_secondary) @@ -741,23 +749,26 @@ setup_fixmap: ret ENDPROC(setup_fixmap) +/* + * Setup the initial stack and jump to the C world + * + * Inputs: + * x0 : Argument 0 of the C function to call + * x1 : Argument 1 of the C function to call + * x2 : C entry point + * + * Clobbers x3 + */ launch: -PRINT("- Ready -\r\n") - -ldr x0, =init_data -add x0, x0, #INITINFO_stack /* Find the boot-time stack */ -ldr x0, [x0] -add x0, x0, #STACK_SIZE/* (which grows down from the top). */ -sub x0, x0, #CPUINFO_sizeof /* Make room for CPU save record */ -mov sp, x0 - -cbnz x22, 1f - -mov x0, x20/* Marshal args: - phys_offset */ -mov x1, x21/* - FDT */ -b start_xen /* and disappear into the land of C */ -1: -b start_secondary/* (to the appropriate entry point) */ +ldr x3, =init_data +add x3, x3, #INITINFO_stack /* Find the boot-time stack */ +ldr x3, [x3] +add x3, x3, #STACK_SIZE /* (which grows down from the top). */ +sub x3, x3, #CPUINFO_sizeof /* Make room for CPU save record */ +mov sp, x3 + +/* Jump to C world */ +brx2 ENDPROC(launch) /* Fail-stop */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 23/28] xen/arm32: head: Setup HTTBR in enable_mmu() and add missing isb
At the moment, HTTBR is setup in create_page_tables(). This is fine as it is called by every CPUs. However, such assumption may not hold in the future. To make change easier, the HTTBR is not setup in enable_mmu(). Take the opportunity to add the missing isb() to ensure the HTTBR is seen before the MMU is turned on. Lastly, the only use of r5 in create_page_tables() is now removed. So the register can be removed from the clobber list of the function. Signed-off-by: Julien Grall --- Changes in v3: - Move the comment in the correct place - r5 is not cloberred anymore Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 3c18037575..2317fb8855 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -359,7 +359,7 @@ ENDPROC(cpu_init) * r9 : paddr(start) * r10: phys offset * - * Clobbers r0 - r6 + * Clobbers r0 - r4, r6 * * Register usage within this function: * r6 : Identity map in place @@ -374,11 +374,8 @@ create_page_tables: moveq r6, #1 /* r6 := identity map now in place */ movne r6, #0 /* r6 := identity map not yet in place */ -/* Write Xen's PT's paddr into the HTTBR */ ldr r4, =boot_pgtable add r4, r4, r10/* r4 := paddr (boot_pagetable) */ -mov r5, #0 /* r4:r5 is paddr (boot_pagetable) */ -mcrr CP64(r4, r5, HTTBR) /* Setup boot_pgtable: */ ldr r1, =boot_second @@ -484,6 +481,13 @@ enable_mmu: mcr CP32(r0, TLBIALLH) /* Flush hypervisor TLBs */ dsb nsh +/* Write Xen's PT's paddr into the HTTBR */ +ldr r0, =boot_pgtable +add r0, r0, r10/* r0 := paddr (boot_pagetable) */ +mov r1, #0 /* r0:r1 is paddr (boot_pagetable) */ +mcrr CP64(r0, r1, HTTBR) +isb + mrc CP32(r0, HSCTLR) /* Enable MMU and D-cache */ orr r0, r0, #(SCTLR_Axx_ELx_M|SCTLR_Axx_ELx_C) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 11/28] xen/arm32: head: Rework UART initialization on boot CPU
Anything executed after the label common_start can be executed on all CPUs. However most of the instructions executed between the label common_start and init_uart are not executed on the boot CPU. The only instructions executed are to lookup the CPUID so it can be printed on the console (if earlyprintk is enabled). Printing the CPUID is not entirely useful to have for the boot CPU and requires a conditional branch to bypass unused instructions. Furthermore, the function init_uart is only called for boot CPU requiring another conditional branch. This makes the code a bit tricky to follow. The UART initialization is now moved before the label common_start. This now requires to have a slightly altered print for the boot CPU and set the early UART base address in each the two path (boot CPU and secondary CPUs). This has the nice effect to remove a couple of conditional branch in the code. After this rework, the CPUID is only used at the very beginning of the secondary CPUs boot path. So there is no need to "reserve" x24 for the CPUID. Lastly, take the opportunity to replace load from literal pool with the new macro mov_w. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - s/Ouput/Output/ - Add Stefano's reviewed-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 28 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index b54331c19d..fd3a273550 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -54,7 +54,7 @@ * r4 - * r5 - * r6 - identity map in place - * r7 - CPUID + * r7 - * r8 - DTB address (boot CPU only) * r9 - paddr(start) * r10 - phys offset @@ -123,6 +123,12 @@ past_zImage: add r8, r10/* r8 := paddr(DTB) */ #endif +/* Initialize the UART if earlyprintk has been enabled. */ +#ifdef CONFIG_EARLY_PRINTK +blinit_uart +#endif +PRINT("- Boot CPU booting -\r\n") + mov r12, #0/* r12 := is_secondary_cpu */ b common_start @@ -137,14 +143,9 @@ GLOBAL(init_secondary) mov r12, #1/* r12 := is_secondary_cpu */ -common_start: mrc CP32(r1, MPIDR) bic r7, r1, #(~MPIDR_HWID_MASK) /* Mask out flags to get CPU ID */ -/* Non-boot CPUs wait here until __cpu_up is ready for them */ -teq r12, #0 -beq 1f - ldr r0, =smp_up_cpu add r0, r0, r10/* Apply physical offset */ dsb @@ -156,15 +157,14 @@ common_start: 1: #ifdef CONFIG_EARLY_PRINTK -ldr r11, =EARLY_UART_BASE_ADDRESS /* r11 := UART base address */ -teq r12, #0/* Boot CPU sets up the UART too */ -bleq init_uart +mov_w r11, EARLY_UART_BASE_ADDRESS /* r11 := UART base address */ PRINT("- CPU ") mov r0, r7 blputn PRINT(" booting -\r\n") #endif +common_start: /* Check that this CPU has Hyp mode */ mrc CP32(r0, ID_PFR1) and r0, r0, #0xf000/* Bits 12-15 define virt extensions */ @@ -497,11 +497,15 @@ ENTRY(switch_ttbr) #ifdef CONFIG_EARLY_PRINTK /* - * Bring up the UART. - * r11: Early UART base address - * Clobbers r0-r2 + * Initialize the UART. Should only be called on the boot CPU. + * + * Output: + * r11: Early UART base physical address + * + * Clobbers r0 - r3 */ init_uart: +mov_w r11, EARLY_UART_BASE_ADDRESS #ifdef EARLY_PRINTK_INIT_UART early_uart_init r11, r1, r2 #endif -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 10/28] xen/arm32: head: Don't clobber r14/lr in the macro PRINT
The current implementation of the macro PRINT will clobber r14/lr. This means the user should save r14 if it cares about it. Follow-up patches will introduce more use of PRINT in places where lr should be preserved. Rather than requiring all the user to preserve lr, the macro PRINT is modified to save and restore it. While the comment state r3 will be clobbered, this is not the case. So PRINT will use r3 to preserve lr. Lastly, take the opportunity to move the comment on top of PRINT and use PRINT in init_uart. Both changes will be helpful in a follow-up patch. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 8b4c8a4714..b54331c19d 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -64,15 +64,20 @@ * r14 - LR * r15 - PC */ -/* Macro to print a string to the UART, if there is one. - * Clobbers r0-r3. */ #ifdef CONFIG_EARLY_PRINTK -#define PRINT(_s) \ -adr r0, 98f ; \ -blputs; \ -b 99f ; \ -98: .asciz _s ; \ -.align 2 ; \ +/* + * Macro to print a string to the UART, if there is one. + * + * Clobbers r0 - r3 + */ +#define PRINT(_s) \ +mov r3, lr ;\ +adr r0, 98f ;\ +blputs ;\ +mov lr, r3 ;\ +b 99f ;\ +98: .asciz _s ;\ +.align 2 ;\ 99: #else /* CONFIG_EARLY_PRINTK */ #define PRINT(s) @@ -500,10 +505,8 @@ init_uart: #ifdef EARLY_PRINTK_INIT_UART early_uart_init r11, r1, r2 #endif -adr r0, 1f -b puts /* Jump to puts */ -1: .asciz "- UART enabled -\r\n" -.align 4 +PRINT("- UART enabled -\r\n") +mov pc, lr /* * Print early debug messages. -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 20/28] xen/arm32: head: Remove 1:1 mapping as soon as it is not used
The 1:1 mapping may clash with other parts of the Xen virtual memory layout. At the moment, Xen is handling the clash by only creating a mapping to the runtime virtual address before enabling the MMU. The rest of the mappings (such as the fixmap) will be mapped after the MMU is enabled. However, the code doing the mapping is not safe as it replace mapping without using the Break-Before-Make sequence. As the 1:1 mapping can be anywhere in the memory, it is easier to remove all the entries added as soon as the 1:1 mapping is not used rather than adding the Break-Before-Make sequence everywhere. It is difficult to track where exactly the 1:1 mapping was created without a full rework of create_page_tables(). Instead, introduce a new function remove_identity_mapping() will look where is the top-level entry for the 1:1 mapping and remove it. The new function is only called for the boot CPU. Secondary CPUs will switch directly to the runtime page-tables so there are no need to remove the 1:1 mapping. Note that this still doesn't make the Secondary CPUs path safe but it is not making it worst. Signed-off-by: Julien Grall --- It is very likely we will need to re-introduce the 1:1 mapping to cater secondary CPUs boot and suspend/resume. For now, the attempt is to make boot CPU path fully Arm Arm compliant. Changes in v3: - Remove unused label - Avoid harcoding slots Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 86 +-- 1 file changed, 69 insertions(+), 17 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 8f945d318a..34fc9ffcee 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -32,6 +32,10 @@ #define PT_UPPER(x) (PT_##x & 0xf00) #define PT_LOWER(x) (PT_##x & 0x0ff) +/* Convenience defines to get slot used by Xen mapping. */ +#define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START) +#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START) + #if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC)) #include EARLY_PRINTK_INC #endif @@ -158,6 +162,13 @@ past_zImage: ldr r0, =primary_switched mov pc, r0 primary_switched: +/* + * The 1:1 map may clash with other parts of the Xen virtual memory + * layout. As it is not used anymore, remove it completely to + * avoid having to worry about replacing existing mapping + * afterwards. + */ +blremove_identity_mapping blsetup_fixmap #ifdef CONFIG_EARLY_PRINTK /* Use a virtual address to access the UART. */ @@ -474,12 +485,63 @@ enable_mmu: mov pc, lr ENDPROC(enable_mmu) -setup_fixmap: +/* + * Remove the 1:1 map for the page-tables. It is not easy to keep track + * where the 1:1 map was mapped, so we will look for the top-level entry + * exclusive to the 1:1 map and remove it. + * + * Inputs: + * r9 : paddr(start) + * + * Clobbers r0 - r3 + */ +remove_identity_mapping: +/* r2:r3 := invalid page-table entry */ +mov r2, #0x0 +mov r3, #0x0 /* - * Now we can install the fixmap and dtb mappings, since we - * don't need the 1:1 map any more + * Find the first slot used. Remove the entry for the first + * table if the slot is not XEN_FIRST_SLOT. For slot XEN_FIRST_SLOT, + * the 1:1 mapping was done in the second table. */ -dsb +lsr r1, r9, #FIRST_SHIFT +mov_w r0, LPAE_ENTRY_MASK +and r1, r1, r0 /* r1 := first slot */ +cmp r1, #XEN_FIRST_SLOT +beq 1f +/* It is not in slot 0, remove the entry */ +ldr r0, =boot_pgtable /* r0 := root table */ +lsl r1, r1, #3 /* r1 := Slot offset */ +strd r2, r3, [r0, r1] +b identity_mapping_removed + +1: +/* + * Find the second slot used. Remove the entry for the first + * table if the slot is not XEN_SECOND_SLOT. For slot XEN_SECOND_SLOT, + * it means the 1:1 mapping was not created. + */ +lsr r1, r9, #SECOND_SHIFT +mov_w r0, LPAE_ENTRY_MASK +and r1, r1, r0 /* r1 := second slot */ +cmp r1, #XEN_SECOND_SLOT +beq identity_mapping_removed +/* It is not in slot 1, remove the entry */ +ldr r0, =boot_second /* r0 := second table */ +lsl r1, r1, #3 /* r1 := Slot offset */ +strd r2, r3, [r0, r1] + +identity_mapping_removed: +/* See asm-arm/arm32/flushtlb.h for the explanation of the sequence. */ +dsb nshst +mcr CP32(r0, TLBIALLH) +dsb nsh +isb + +mov pc, lr +ENDPROC(remove_identity_mapping) + +setup_fixmap: #if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ /* Add UART to the fixmap table *
[Xen-devel] [PATCH v3 00/28] xen/arm: Rework head.S to make it more compliant with the Arm Arm
Hi all, This is part of the boot/memory rework for Xen on Arm, but not sent as MM-PARTx as this is focusing on the boot code. Similar to the memory code, the boot code is not following the Arm Arm and could lead to memory corruption/TLB conflict abort. I am not aware of any platforms where Xen fails to boot, yet it should be fixed sooner rather than later. While making the code more compliant, I have also took the opportunity to simplify the boot and also add more documentation. After this series, the boot CPU and secondary CPUs path is mostly compliant with the Arm Arm. The only non-compliant places I am aware of are: 1) create_page_tables: Some rework is necessary to update the page-tables safely without the MMU on. 2) The switches between boot and runtime page-tables (for both boot CPU and secondary CPUs) are not safe. All will be addressed in follow-up series. The boot code would also benefits another proof read for missing isb()/dsb(). For convenience I provided a branch based on staging: git://xenbits.xen.org/people/julieng/xen-unstable.git branch boot/v3 Cheers, Julien Grall (28): xen/arm: lpae: Allow more LPAE helpers to be used in assembly xen/arm64: head: Remove 1:1 mapping as soon as it is not used xen/arm64: head: Rework and document setup_fixmap() xen/arm64: head: Rework and document launch() xen/arm64: head: Setup TTBR_EL2 in enable_mmu() and add missing isb xen/arm64: head: Introduce a macro to get a PC-relative address of a symbol xen/arm64: head: Fix typo in the documentation on top of init_uart() xen/arm32: head: Add a macro to move an immediate constant into a 32-bit register xen/arm32: head: Mark the end of subroutines with ENDPROC xen/arm32: head: Don't clobber r14/lr in the macro PRINT xen/arm32: head: Rework UART initialization on boot CPU xen/arm32: head: Introduce print_reg xen/arm32: head: Introduce distinct paths for the boot CPU and secondary CPUs xen/arm32: head: Rework and document check_cpu_mode() xen/arm32: head: Rework and document zero_bss() xen/arm32: head: Document create_pages_tables() xen/arm32: head: Document enable_mmu() xen/arm32: head: Move assembly switch to the runtime PT in secondary CPUs path xen/arm32: head: Don't setup the fixmap on secondary CPUs xen/arm32: head: Remove 1:1 mapping as soon as it is not used xen/arm32: head: Rework and document setup_fixmap() xen/arm32: head: Rework and document launch() xen/arm32: head: Setup HTTBR in enable_mmu() and add missing isb xen/arm: Zero BSS after the MMU and D-cache is turned on xen/arm64: head: Introduce macros to create table and mapping entry xen/arm64: head: Use a page mapping for the 1:1 mapping in create_page_tables() xen/arm32: head: Introduce macros to create table and mapping entry xen/arm32: head: Use a page mapping for the 1:1 mapping in create_page_tables() xen/arch/arm/arm32/head.S | 601 ++--- xen/arch/arm/arm64/head.S | 438 + xen/arch/arm/mm.c | 25 +- xen/include/asm-arm/lpae.h | 10 +- 4 files changed, 716 insertions(+), 358 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 02/28] xen/arm64: head: Remove 1:1 mapping as soon as it is not used
The 1:1 mapping may clash with other parts of the Xen virtual memory layout. At the moment, Xen is handling the clash by only creating a mapping to the runtime virtual address before enabling the MMU. The rest of the mappings (such as the fixmap) will be mapped after the MMU is enabled. However, the code doing the mapping is not safe as it replace mapping without using the Break-Before-Make sequence. As the 1:1 mapping can be anywhere in the memory, it is easier to remove all the entries added as soon as the 1:1 mapping is not used rather than adding the Break-Before-Make sequence everywhere. It is difficult to track where exactly the 1:1 mapping was created without a full rework of create_page_tables(). Instead, introduce a new function remove_identity_mapping() will look where is the top-level entry for the 1:1 mapping and remove it. The new function is only called for the boot CPU. Secondary CPUs will switch directly to the runtime page-tables so there are no need to remove the 1:1 mapping. Note that this still doesn't make the Secondary CPUs path safe but it is not making it worst. Signed-off-by: Julien Grall --- It is very likely we will need to re-introduce the 1:1 mapping to cater secondary CPUs boot and suspend/resume. For now, the attempt is to make boot CPU path fully Arm Arm compliant. Changes in v3: - Avoid hardcoding slots Changes in v2: - s/ID map/1:1 mapping/ - Rename remove_id_map() to remove_identity_mapping() - Add missing signed-off-by --- xen/arch/arm/arm64/head.S | 94 +++ 1 file changed, 79 insertions(+), 15 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 50cff08756..ec138aae3e 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -33,6 +33,11 @@ #define PT_DEV0xe71 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=0 P=1 */ #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */ +/* Convenience defines to get slot used by Xen mapping. */ +#define XEN_ZEROETH_SLOTzeroeth_table_offset(XEN_VIRT_START) +#define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START) +#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START) + #define __HEAD_FLAG_PAGE_SIZE ((PAGE_SHIFT - 10) / 2) #define __HEAD_FLAG_PHYS_BASE 1 @@ -301,6 +306,13 @@ real_start_efi: ldr x0, =primary_switched brx0 primary_switched: +/* + * The 1:1 map may clash with other parts of the Xen virtual memory + * layout. As it is not used anymore, remove it completely to + * avoid having to worry about replacing existing mapping + * afterwards. + */ +blremove_identity_mapping blsetup_fixmap #ifdef CONFIG_EARLY_PRINTK /* Use a virtual address to access the UART. */ @@ -626,10 +638,71 @@ enable_mmu: ret ENDPROC(enable_mmu) +/* + * Remove the 1:1 map for the page-tables. It is not easy to keep track + * where the 1:1 map was mapped, so we will look for the top-level entry + * exclusive to the 1:1 map and remove it. + * + * Inputs: + * x19: paddr(start) + * + * Clobbers x0 - x1 + */ +remove_identity_mapping: +/* + * Find the zeroeth slot used. Remove the entry from zeroeth + * table if the slot is not XEN_ZEROETH_SLOT. For slot + * XEN_ZEROETH_SLOT, the 1:1 mapping was either done in first + * or second table. + */ +lsr x1, x19, #ZEROETH_SHIFT /* x1 := zeroeth slot */ +cmp x1, #XEN_ZEROETH_SLOT +beq 1f +/* It is not in slot XEN_ZEROETH_SLOT, remove the entry. */ +ldr x0, =boot_pgtable /* x0 := root table */ +str xzr, [x0, x1, lsl #3] +b identity_mapping_removed + +1: +/* + * Find the first slot used. Remove the entry for the first + * table if the slot is not XEN_FIRST_SLOT. For slot XEN_FIRST_SLOT, + * the 1:1 mapping was done in the second table. + */ +lsr x1, x19, #FIRST_SHIFT +and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ +cmp x1, #XEN_FIRST_SLOT +beq 1f +/* It is not in slot XEN_FIRST_SLOT, remove the entry. */ +ldr x0, =boot_first /* x0 := first table */ +str xzr, [x0, x1, lsl #3] +b identity_mapping_removed + +1: +/* + * Find the second slot used. Remove the entry for the first + * table if the slot is not XEN_SECOND_SLOT. For slot XEN_SECOND_SLOT, + * it means the 1:1 mapping was not created. + */ +lsr x1, x19, #SECOND_SHIFT +and x1, x1, #LPAE_ENTRY_MASK /* x1 := first slot */ +cmp x1, #XEN_SECOND_SLOT +beq identity_mapping_removed +/* It is not in slot 1, remove the entry */ +ldr x0, =boot_second /* x0 := second table */ +str xzr, [x0,
[Xen-devel] [PATCH v3 28/28] xen/arm32: head: Use a page mapping for the 1:1 mapping in create_page_tables()
At the moment the function create_page_tables() will use 1GB/2MB mapping for the identity mapping. As we don't know what is present before and after Xen in memory, we may end up to map device/reserved-memory with cacheable memory. This may result to mismatched attributes as other users may access the same region differently. To prevent any issues, we should only map the strict minimum in the 1:1 mapping. A check in xen.lds.S already guarantees anything necessary for turning on the MMU fits in a page (at the moment 4K). As only one page will be mapped for the 1:1 mapping, it is necessary to pre-allocate a page for the 3rd level table. For simplicity, all the tables that may be necessary for setting up the 1:1 mapping are linked together in advance. They will then be linked to the boot page tables at the correct level. Signed-off-by: Julien Grall --- Changes in v3: - Patch added --- xen/arch/arm/arm32/head.S | 119 ++ xen/arch/arm/mm.c | 2 +- 2 files changed, 48 insertions(+), 73 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 6d03fecaf2..dec6266803 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -444,73 +444,13 @@ ENDPROC(cpu_init) * r6 : Identity map in place */ create_page_tables: -/* - * If Xen is loaded at exactly XEN_VIRT_START then we don't - * need an additional 1:1 mapping, the virtual mapping will - * suffice. - */ -cmp r9, #XEN_VIRT_START -moveq r6, #1 /* r6 := identity map now in place */ -movne r6, #0 /* r6 := identity map not yet in place */ - -ldr r4, =boot_pgtable -add r4, r4, r10/* r4 := paddr (boot_pagetable) */ - -/* Setup boot_pgtable: */ -ldr r1, =boot_second -add r1, r1, r10/* r1 := paddr (boot_second) */ - -/* ... map boot_second in boot_pgtable[0] */ -orr r2, r1, #PT_UPPER(PT) /* r2:r3 := table map of boot_second */ -orr r2, r2, #PT_LOWER(PT) /* (+ rights for linear PT) */ -mov r3, #0x0 -strd r2, r3, [r4, #0] /* Map it in slot 0 */ - -/* ... map of paddr(start) in boot_pgtable */ -lsrs r1, r9, #FIRST_SHIFT /* Offset of base paddr in boot_pgtable */ -beq 1f /* If it is in slot 0 then map in boot_second - * later on */ -lsl r2, r1, #FIRST_SHIFT /* Base address for 1GB mapping */ -orr r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */ -orr r2, r2, #PT_LOWER(MEM) -lsl r1, r1, #3 /* r1 := Slot offset */ -mov r3, #0x0 -strd r2, r3, [r4, r1] /* Mapping of paddr(start) */ -mov r6, #1 /* r6 := identity map now in place */ - -1: /* Setup boot_second: */ -ldr r4, =boot_second -add r4, r4, r10/* r4 := paddr (boot_second) */ - -ldr r1, =boot_third -add r1, r1, r10/* r1 := paddr (boot_third) */ - -/* ... map boot_third in boot_second[1] */ -orr r2, r1, #PT_UPPER(PT) /* r2:r3 := table map of boot_third */ -orr r2, r2, #PT_LOWER(PT) /* (+ rights for linear PT) */ -mov r3, #0x0 -strd r2, r3, [r4, #8] /* Map it in slot 1 */ - -/* ... map of paddr(start) in boot_second */ -cmp r6, #1 /* r6 is set if already created */ -beq 1f -lsr r2, r9, #SECOND_SHIFT /* Offset of base paddr in boot_second */ -ldr r3, =LPAE_ENTRY_MASK -and r1, r2, r3 -cmp r1, #1 -beq virtphys_clash /* It's in slot 1, which we cannot handle */ - -lsl r2, r2, #SECOND_SHIFT /* Base address for 2MB mapping */ -orr r2, r2, #PT_UPPER(MEM) /* r2:r3 := section map */ -orr r2, r2, #PT_LOWER(MEM) -mov r3, #0x0 -lsl r1, r1, #3 /* r1 := Slot offset */ -strd r2, r3, [r4, r1] /* Mapping of paddr(start) */ -mov r6, #1 /* r6 := identity map now in place */ +/* Prepare the page-tables for mapping Xen */ +ldr r0, =XEN_VIRT_START +create_table_entry boot_pgtable, boot_second, r0, FIRST_SHIFT +create_table_entry boot_second, boot_third, r0, SECOND_SHIFT /* Setup boot_third: */ -1: ldr r4, =boot_third -add r4, r4, r10/* r4 := paddr (boot_third) */ +adr_l r4, boot_third, mmu=0 lsr r2, r9, #THIRD_SHIFT /* Base address for 4K mapping */ lsl r2, r2, #THIRD_SHIFT @@ -527,16 +467,51 @@ create_page_tables: blo 1b /* - * Defer fixmap and dtb mapping until after paging enabled, to - * avoid them clashing with the 1:1 mapping. + * If Xen i
[Xen-devel] [PATCH v3 16/28] xen/arm32: head: Document create_pages_tables()
Document the behavior and the main registers usage within the function. Note that r6 is now only used within the function, so it does not need to be part of the common register. Signed-off-by: Julien Grall Acked-by: Stefano Stabellini --- Changes in v3: - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 30 +++--- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 1189ed6c47..83f8774e2a 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -53,7 +53,7 @@ * r3 - * r4 - * r5 - - * r6 - identity map in place + * r6 - * r7 - * r8 - DTB address (boot CPU only) * r9 - paddr(start) @@ -302,18 +302,26 @@ cpu_init_done: mov pc, r5/* Return address is in r5 */ ENDPROC(cpu_init) +/* + * Rebuild the boot pagetable's first-level entries. The structure + * is described in mm.c. + * + * After the CPU enables paging it will add the fixmap mapping + * to these page tables, however this may clash with the 1:1 + * mapping. So each CPU must rebuild the page tables here with + * the 1:1 in place. + * + * Inputs: + * r9 : paddr(start) + * r10: phys offset + * + * Clobbers r0 - r6 + * + * Register usage within this function: + * r6 : Identity map in place + */ create_page_tables: /* - * Rebuild the boot pagetable's first-level entries. The structure - * is described in mm.c. - * - * After the CPU enables paging it will add the fixmap mapping - * to these page tables, however this may clash with the 1:1 - * mapping. So each CPU must rebuild the page tables here with - * the 1:1 in place. - */ - -/* * If Xen is loaded at exactly XEN_VIRT_START then we don't * need an additional 1:1 mapping, the virtual mapping will * suffice. -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 21/28] xen/arm32: head: Rework and document setup_fixmap()
At the moment, the fixmap table is only hooked when earlyprintk is used. This is fine today because in C land, the fixmap is not used by anyone until the the boot CPU is switching to the runtime page-tables. In the future, the boot CPU will not switch between page-tables to avoid TLB incoherency. Thus, the fixmap table will need to be always hooked beofre any use. Let's start doing it now in setup_fixmap(). Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - The unused label is now removed in the previous patch - Add Stefano's reviewed-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 34fc9ffcee..ee5c3d2af8 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -541,8 +541,21 @@ identity_mapping_removed: mov pc, lr ENDPROC(remove_identity_mapping) +/* + * Map the UART in the fixmap (when earlyprintk is used) and hook the + * fixmap table in the page tables. + * + * The fixmap cannot be mapped in create_page_tables because it may + * clash with the 1:1 mapping. + * + * Inputs: + * r10: Physical offset + * r11: Early UART base physical address + * + * Clobbers r1 - r4 + */ setup_fixmap: -#if defined(CONFIG_EARLY_PRINTK) /* Fixmap is only used by early printk */ +#if defined(CONFIG_EARLY_PRINTK) /* Add UART to the fixmap table */ ldr r1, =xen_fixmap/* r1 := vaddr (xen_fixmap) */ lsr r2, r11, #THIRD_SHIFT @@ -551,6 +564,7 @@ setup_fixmap: orr r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART */ mov r3, #0x0 strd r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */ +#endif /* Map fixmap into boot_second */ ldr r1, =boot_second /* r1 := vaddr (boot_second) */ @@ -565,7 +579,7 @@ setup_fixmap: /* Ensure any page table updates made above have occurred. */ dsb nshst -#endif + mov pc, lr ENDPROC(setup_fixmap) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 17/28] xen/arm32: head: Document enable_mmu()
Document the behavior and the main registers usage within enable_mmu(). Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 7 +++ 1 file changed, 7 insertions(+) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 83f8774e2a..f8603051e4 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -423,6 +423,13 @@ virtphys_clash: b fail ENDPROC(create_page_tables) +/* + * Turn on the Data Cache and the MMU. The function will return on the 1:1 + * mapping. In other word, the caller is responsible to switch to the runtime + * mapping. + * + * Clobbers r0 - r3 + */ enable_mmu: PRINT("- Turning on paging -\r\n") -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 14/28] xen/arm32: head: Rework and document check_cpu_mode()
A branch in the success case can be avoided by inverting the branch condition. At the same time, remove a pointless comment as Xen can only run at Hypervisor Mode. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall --- Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 4285f76463..c7b4fe4cd4 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -206,6 +206,16 @@ secondary_switched: b launch ENDPROC(init_secondary) +/* + * Check if the CPU supports virtualization extensions and has been booted + * in Hypervisor mode. + * + * This function will never return when the CPU doesn't support + * virtualization extensions or is booted in another mode than + * Hypervisor mode. + * + * Clobbers r0 - r3 + */ check_cpu_mode: /* Check that this CPU has Hyp mode */ mrc CP32(r0, ID_PFR1) @@ -220,15 +230,12 @@ check_cpu_mode: mrs r0, cpsr and r0, r0, #0x1f /* Mode is in the low 5 bits of CPSR */ teq r0, #0x1a /* Hyp Mode? */ -beq hyp +moveq pc, lr /* Yes, return */ /* OK, we're boned. */ PRINT("- Xen must be entered in NS Hyp mode -\r\n") PRINT("- Please update the bootloader -\r\n") b fail - -hyp:PRINT("- Xen starting in Hyp mode -\r\n") -mov pc, lr ENDPROC(check_cpu_mode) zero_bss: -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 08/28] xen/arm32: head: Add a macro to move an immediate constant into a 32-bit register
The current boot code is using the pattern ldr rX, =... to move an immediate constant into a 32-bit register. This pattern implies to load the immediate constant from a literal pool, meaning a memory access will be performed. The memory access can be avoided by using movw/movt instructions. A new macro is introduced to move an immediate constant into a 32-bit register without a memory load. Follow-up patches will make use of it. Signed-off-by: Julien Grall Acked-by: Stefano Stabellini --- Changes in v3: - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 9 + 1 file changed, 9 insertions(+) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index 18ded49a04..99f4af18d8 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -37,6 +37,15 @@ #endif /* + * Move an immediate constant into a 32-bit register using movw/movt + * instructions. + */ +.macro mov_w reg, word +movw \reg, #:lower16:\word +movt \reg, #:upper16:\word +.endm + +/* * Common register usage in this file: * r0 - * r1 - -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 13/28] xen/arm32: head: Introduce distinct paths for the boot CPU and secondary CPUs
The boot code is currently quite difficult to go through because of the lack of documentation and a number of indirection to avoid executing some path in either the boot CPU or secondary CPUs. In an attempt to make the boot code easier to follow, each parts of the boot are now in separate functions. Furthermore, the paths for the boot CPU and secondary CPUs are now distinct and for now will call each functions. Follow-ups will remove unnecessary calls and do further improvement (such as adding documentation and reshuffling). Note that the switch from using the ID mapping to the runtime mapping is duplicated for each path. This is because in the future we will need to stay longer in the ID mapping for the boot CPU. Lastly, it is now required to save lr in cpu_init() becauswe the function will call other functions and therefore clobber lr. Signed-off-by: Julien Grall --- Changes in v3: - Remove hard tab - s/ID map/1:1 mapping/ Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 65 +++ 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index c4ee06ba93..4285f76463 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -148,7 +148,19 @@ past_zImage: mov r12, #0/* r12 := is_secondary_cpu */ -b common_start +blcheck_cpu_mode +blzero_bss +blcpu_init +blcreate_page_tables +blenable_mmu + +/* We are still in the 1:1 mapping. Jump to the runtime Virtual Address. */ +ldr r0, =primary_switched +mov pc, r0 +primary_switched: +blsetup_fixmap +b launch +ENDPROC(start) GLOBAL(init_secondary) cpsid aif/* Disable all interrupts */ @@ -179,8 +191,22 @@ GLOBAL(init_secondary) print_reg r7 PRINT(" booting -\r\n") #endif - -common_start: +blcheck_cpu_mode +blzero_bss +blcpu_init +blcreate_page_tables +blenable_mmu + + +/* We are still in the 1:1 mapping. Jump to the runtime Virtual Address. */ +ldr r0, =secondary_switched +mov pc, r0 +secondary_switched: +blsetup_fixmap +b launch +ENDPROC(init_secondary) + +check_cpu_mode: /* Check that this CPU has Hyp mode */ mrc CP32(r0, ID_PFR1) and r0, r0, #0xf000/* Bits 12-15 define virt extensions */ @@ -202,7 +228,10 @@ common_start: b fail hyp:PRINT("- Xen starting in Hyp mode -\r\n") +mov pc, lr +ENDPROC(check_cpu_mode) +zero_bss: /* Zero BSS On the boot CPU to avoid nasty surprises */ teq r12, #0 bne skip_bss @@ -219,8 +248,14 @@ hyp:PRINT("- Xen starting in Hyp mode -\r\n") blo 1b skip_bss: +mov pc, lr +ENDPROC(zero_bss) + +cpu_init: PRINT("- Setting up control registers -\r\n") +mov r5, lr /* r5 := return address */ + /* Get processor specific proc info into r1 */ bl__lookup_processor_type teq r1, #0 @@ -231,7 +266,6 @@ skip_bss: PRINT(" -\r\n") b fail 1: - /* Jump to cpu_init */ ldr r1, [r1, #PROCINFO_cpu_init] /* r1 := vaddr(init func) */ adr lr, cpu_init_done /* Save return address */ @@ -256,6 +290,10 @@ cpu_init_done: ldr r0, =HSCTLR_SET mcr CP32(r0, HSCTLR) +mov pc, r5/* Return address is in r5 */ +ENDPROC(cpu_init) + +create_page_tables: /* * Rebuild the boot pagetable's first-level entries. The structure * is described in mm.c. @@ -359,15 +397,16 @@ cpu_init_done: /* boot pagetable setup complete */ cmp r6, #1/* Did we manage to create an identity mapping ? */ -beq 1f +moveq pc, lr PRINT("Unable to build boot page tables - Failed to identity map Xen.\r\n") b fail virtphys_clash: /* Identity map clashes with boot_third, which we cannot handle yet */ PRINT("- Unable to build boot page tables - virt and phys addresses clash. -\r\n") b fail +ENDPROC(create_page_tables) -1: +enable_mmu: PRINT("- Turning on paging -\r\n") /* @@ -377,16 +416,16 @@ virtphys_clash: mcr CP32(r0, TLBIALLH) /* Flush hypervisor TLBs */ dsb nsh -ldr r1, =paging/* Explicit vaddr, not RIP-relative */ mrc CP32(r0, HSCTLR) /* Enable MMU and D-cache */ orr r0, r0, #(SCTLR_Axx_ELx_M|SCTLR_Axx_ELx_C) dsb /* Flush PTE writes and finish reads */ mcr CP32(r0, HSCTLR) /* now paging is enabled */ isb
[Xen-devel] [PATCH v3 22/28] xen/arm32: head: Rework and document launch()
Boot CPU and secondary CPUs will use different entry point to C code. At the moment, the decision on which entry to use is taken within launch(). In order to avoid using conditional instruction and make the call clearer, launch() is reworked to take in parameters the entry point and its arguments. Lastly, document the behavior and the main registers usage within the function. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's reviewed-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 34 -- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index ee5c3d2af8..3c18037575 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -174,6 +174,11 @@ primary_switched: /* Use a virtual address to access the UART. */ mov_w r11, EARLY_UART_VIRTUAL_ADDRESS #endif +PRINT("- Ready -\r\n") +/* Setup the arguments for start_xen and jump to C world */ +mov r0, r10/* r0 := Physical offset */ +mov r1, r8 /* r1 := paddr(FDT) */ +ldr r2, =start_xen b launch ENDPROC(start) @@ -238,6 +243,9 @@ secondary_switched: /* Use a virtual address to access the UART. */ mov_w r11, EARLY_UART_VIRTUAL_ADDRESS #endif +PRINT("- Ready -\r\n") +/* Jump to C world */ +ldr r2, =start_secondary b launch ENDPROC(init_secondary) @@ -583,19 +591,25 @@ setup_fixmap: mov pc, lr ENDPROC(setup_fixmap) +/* + * Setup the initial stack and jump to the C world + * + * Inputs: + * r0 : Argument 0 of the C function to call + * r1 : Argument 1 of the C function to call + * r2 : C entry point + * + * Clobbers r3 + */ launch: -PRINT("- Ready -\r\n") - -ldr r0, =init_data -add r0, #INITINFO_stack/* Find the boot-time stack */ -ldr sp, [r0] +ldr r3, =init_data +add r3, #INITINFO_stack/* Find the boot-time stack */ +ldr sp, [r3] add sp, #STACK_SIZE/* (which grows down from the top). */ sub sp, #CPUINFO_sizeof/* Make room for CPU save record */ -teq r12, #0 -moveq r0, r10/* Marshal args: - phys_offset */ -moveq r1, r8 /* - DTB address */ -beq start_xen /* and disappear into the land of C */ -b start_secondary/* (to the appropriate entry point) */ + +/* Jump to C world */ + bxr2 ENDPROC(launch) /* Fail-stop */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 26/28] xen/arm64: head: Use a page mapping for the 1:1 mapping in create_page_tables()
At the moment the function create_page_tables() will use 1GB/2MB mapping for the identity mapping. As we don't know what is present before and after Xen in memory, we may end up to map device/reserved-memory with cacheable memory. This may result to mismatched attributes as other users may access the same region differently. To prevent any issues, we should only map the strict minimum in the 1:1 mapping. A check in xen.lds.S already guarantees anything necessary for turning on the MMU fits in a page (at the moment 4K). As only one page will be mapped for the 1:1 mapping, it is necessary to pre-allocate a page for the 3rd level table. For simplicity, all the tables that may be necessary for setting up the 1:1 mapping are linked together in advance. They will then be linked to the boot page tables at the correct level. Signed-off-by: Julien Grall --- Changes in v3: - Patch added --- xen/arch/arm/arm64/head.S | 176 -- xen/arch/arm/mm.c | 2 + 2 files changed, 78 insertions(+), 100 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index f4177dbba1..1e5b1035b8 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -566,100 +566,17 @@ ENDPROC(cpu_init) * x19: paddr(start) * x20: phys offset * - * Clobbers x0 - x4, x25 - * - * Register usage within this function: - * x25: Identity map in place + * Clobbers x0 - x4 */ create_page_tables: -/* - * If Xen is loaded at exactly XEN_VIRT_START then we don't - * need an additional 1:1 mapping, the virtual mapping will - * suffice. - */ -cmp x19, #XEN_VIRT_START -cset x25, eq/* x25 := identity map in place, or not */ - -load_paddr x4, boot_pgtable - -/* Setup boot_pgtable: */ -load_paddr x1, boot_first - -/* ... map boot_first in boot_pgtable[0] */ -mov x3, #PT_PT /* x2 := table map of boot_first */ -orr x2, x1, x3 /* + rights for linear PT */ -str x2, [x4, #0] /* Map it in slot 0 */ - -/* ... map of paddr(start) in boot_pgtable+boot_first_id */ -lsr x1, x19, #ZEROETH_SHIFT/* Offset of base paddr in boot_pgtable */ -cbz x1, 1f /* It's in slot 0, map in boot_first - * or boot_second later on */ - -/* - * Level zero does not support superpage mappings, so we have - * to use an extra first level page in which we create a 1GB mapping. - */ -load_paddr x2, boot_first_id - -mov x3, #PT_PT /* x2 := table map of boot_first_id */ -orr x2, x2, x3 /* + rights for linear PT */ -str x2, [x4, x1, lsl #3] - -load_paddr x4, boot_first_id - -lsr x1, x19, #FIRST_SHIFT /* x1 := Offset of base paddr in boot_first_id */ -lsl x2, x1, #FIRST_SHIFT /* x2 := Base address for 1GB mapping */ -mov x3, #PT_MEM/* x2 := Section map */ -orr x2, x2, x3 -and x1, x1, #LPAE_ENTRY_MASK /* x1 := Slot offset */ -str x2, [x4, x1, lsl #3] /* Mapping of paddr(start) */ -mov x25, #1/* x25 := identity map now in place */ - -1: /* Setup boot_first: */ -load_paddr x4, boot_first /* Next level into boot_first */ - -/* ... map boot_second in boot_first[0] */ -load_paddr x1, boot_second -mov x3, #PT_PT /* x2 := table map of boot_second */ -orr x2, x1, x3 /* + rights for linear PT */ -str x2, [x4, #0] /* Map it in slot 0 */ - -/* ... map of paddr(start) in boot_first */ -cbnz x25, 1f/* x25 is set if already created */ -lsr x2, x19, #FIRST_SHIFT /* x2 := Offset of base paddr in boot_first */ -and x1, x2, #LPAE_ENTRY_MASK /* x1 := Slot to use */ -cbz x1, 1f /* It's in slot 0, map in boot_second */ - -lsl x2, x2, #FIRST_SHIFT /* Base address for 1GB mapping */ -mov x3, #PT_MEM/* x2 := Section map */ -orr x2, x2, x3 -str x2, [x4, x1, lsl #3] /* Create mapping of paddr(start)*/ -mov x25, #1/* x25 := identity map now in place */ - -1: /* Setup boot_second: */ -load_paddr x4, boot_second - -/* ... map boot_third in boot_second[1] */ -load_paddr x1, boot_third -mov x3, #PT_PT /* x2 := table map of boot_third */ -orr x2, x1, x3 /* + rights for linear PT */ -str x2, [x4, #8] /* Map it in slot 1 */ - -/* ... map of paddr(start) in boot_second */ -cbnz x25, 1f/* x25 is set if already created */ -lsr x2, x19, #SECOND_SHIFT /* x2 := Offset
[Xen-devel] [PATCH v3 27/28] xen/arm32: head: Introduce macros to create table and mapping entry
At the moment, any update to the boot-pages are open-coded. This is making more difficult to understand the logic of a function as each update roughly requires 6 instructions. To ease the readability, two new macros are introduced: - create_table_entry: Create a page-table entry in a given table. This can work at any level. - create_mapping_entry: Create a mapping entry in a given table. None of the users will require to map at any other level than 3rd (i.e page granularity). So the macro is supporting support 3rd level mapping. Unlike arm64, there are no easy way to have a PC relative address within the range -/+4GB. In order to have the possibility to use the macro in context with MMU on/off, the user needs to tell the state of the MMU. Lastly, take the opportunity to replace open-coded version in setup_fixmap() by the two new macros. The ones in create_page_tables() will be replaced in a follow-up patch. Signed-off-by: Julien Grall --- The adr_l hack is a bit ugly, but I can't find nicer way to avoid code duplication and improve readability. Changes in v3: - Patch added --- xen/arch/arm/arm32/head.S | 108 ++ 1 file changed, 89 insertions(+), 19 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index e86a9f95e7..6d03fecaf2 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -50,6 +50,20 @@ .endm /* + * There are no easy way to have a PC relative address within the range + * +/- 4GB of the PC. + * + * This macro workaround it by asking the user to tell whether the MMU + * has been turned on or not. + */ +.macro adr_l, dst, sym, mmu +ldr \dst, =\sym +.if \mmu == 0 +add \dst, \dst, r10 +.endif +.endm + +/* * Common register usage in this file: * r0 - * r1 - @@ -342,6 +356,76 @@ cpu_init_done: ENDPROC(cpu_init) /* + * Macro to create a page table entry in \ptbl to \tbl + * + * ptbl:table symbol where the entry will be created + * tbl: table symbol to point to + * virt:virtual address + * shift: #imm page table shift + * mmu: Is the MMU turned on/off. If not specified it will be off + * + * Preserves \virt + * Clobbers r1 - r4 + * + * Also use r10 for the phys offset. + * + * Note that \virt should be in a register other than r1 - r4 + */ +.macro create_table_entry, ptbl, tbl, virt, shift, mmu=0 +lsr r1, \virt, #\shift +mov_w r2, LPAE_ENTRY_MASK +and r1, r1, r2 /* r1 := slot in \tlb */ +lsl r1, r1, #3 /* r1 := slot offset in \tlb */ + +ldr r4, =\tbl +add r4, r4, r10/* r4 := paddr(\tlb) */ + +mov r2, #PT_PT /* r2:r3 := right for linear PT */ +orr r2, r2, r4 /* + \tlb paddr */ +mov r3, #0 + +adr_l r4, \ptbl, \mmu + +strd r2, r3, [r4, r1] +.endm + +/* + * Macro to create a mapping entry in \tbl to \paddr. Only mapping in 3rd + * level table (i.e page granularity) is supported. + * + * tbl: table symbol where the entry will be created + * virt:virtual address + * phys:physical address + * type:mapping type. If not specified it will be normal memory (PT_MEM_L3) + * mmu: Is the MMU turned on/off. If not specified it will be off + * + * Preserves \virt, \phys + * Clobbers r1 - r4 + * + * * Also use r10 for the phys offset. + * + * Note that \virt and \paddr should be in other registers than r1 - r4 + * and be distinct. + */ +.macro create_mapping_entry, tbl, virt, phys, type=PT_MEM_L3, mmu=0 +lsr r4, \phys, #THIRD_SHIFT +lsl r4, r4, #THIRD_SHIFT /* r4 := PAGE_ALIGNED(phys) */ + +mov_w r2, LPAE_ENTRY_MASK +lsr r1, \virt, #THIRD_SHIFT +and r1, r1, r2 /* r1 := slot in \tlb */ +lsl r1, r1, #3 /* r1 := slot offset in \tlb */ + +mov r2, #\type /* r2:r3 := right for section PT */ +orr r2, r2, r4 /* + PAGE_ALIGNED(phys) */ +mov r3, #0 + +adr_l r4, \tbl, \mmu + +strd r2, r3, [r4, r1] +.endm + +/* * Rebuild the boot pagetable's first-level entries. The structure * is described in mm.c. * @@ -559,31 +643,17 @@ ENDPROC(remove_identity_mapping) * r10: Physical offset * r11: Early UART base physical address * - * Clobbers r1 - r4 + * Clobbers r0 - r4 */ setup_fixmap: #if defined(CONFIG_EARLY_PRINTK) /* Add UART to the fixmap table */ -ldr r1, =xen_fixmap/* r1 := vaddr (xen_fixmap) */ -lsr r2, r11, #THIRD_SHIFT -lsl r2, r2, #THIRD_SHIFT /* 4K aligned paddr of UART */ -orr r2, r2, #PT_UPPER(DEV_L3) -orr r2, r2, #PT_LOWER(DEV_L3) /* r2:r3 := 4K dev map including UART */ -mov r3, #0x0 -strd r2, r3, [r1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap'
[Xen-devel] [PATCH v3 01/28] xen/arm: lpae: Allow more LPAE helpers to be used in assembly
A follow-up patch will require to use *_table_offset() and *_MASK helpers from assembly. This can be achieved by using _AT() macro to remove the type when called from assembly. Signed-off-by: Julien Grall --- Changes in v3: - Patch added --- xen/include/asm-arm/lpae.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h index c22780f8f3..4797f9cee4 100644 --- a/xen/include/asm-arm/lpae.h +++ b/xen/include/asm-arm/lpae.h @@ -245,19 +245,19 @@ TABLE_OFFSET_HELPERS(64); #define THIRD_SHIFT(PAGE_SHIFT) #define THIRD_ORDER(THIRD_SHIFT - PAGE_SHIFT) -#define THIRD_SIZE ((paddr_t)1 << THIRD_SHIFT) +#define THIRD_SIZE (_AT(paddr_t, 1) << THIRD_SHIFT) #define THIRD_MASK (~(THIRD_SIZE - 1)) #define SECOND_SHIFT (THIRD_SHIFT + LPAE_SHIFT) #define SECOND_ORDER (SECOND_SHIFT - PAGE_SHIFT) -#define SECOND_SIZE((paddr_t)1 << SECOND_SHIFT) +#define SECOND_SIZE(_AT(paddr_t, 1) << SECOND_SHIFT) #define SECOND_MASK(~(SECOND_SIZE - 1)) #define FIRST_SHIFT(SECOND_SHIFT + LPAE_SHIFT) #define FIRST_ORDER(FIRST_SHIFT - PAGE_SHIFT) -#define FIRST_SIZE ((paddr_t)1 << FIRST_SHIFT) +#define FIRST_SIZE (_AT(paddr_t, 1) << FIRST_SHIFT) #define FIRST_MASK (~(FIRST_SIZE - 1)) #define ZEROETH_SHIFT (FIRST_SHIFT + LPAE_SHIFT) #define ZEROETH_ORDER (ZEROETH_SHIFT - PAGE_SHIFT) -#define ZEROETH_SIZE ((paddr_t)1 << ZEROETH_SHIFT) +#define ZEROETH_SIZE (_AT(paddr_t, 1) << ZEROETH_SHIFT) #define ZEROETH_MASK (~(ZEROETH_SIZE - 1)) /* Calculate the offsets into the pagetables for a given VA */ @@ -266,7 +266,7 @@ TABLE_OFFSET_HELPERS(64); #define second_linear_offset(va) ((va) >> SECOND_SHIFT) #define third_linear_offset(va) ((va) >> THIRD_SHIFT) -#define TABLE_OFFSET(offs) ((unsigned int)(offs) & LPAE_ENTRY_MASK) +#define TABLE_OFFSET(offs) (_AT(unsigned int, offs) & LPAE_ENTRY_MASK) #define first_table_offset(va) TABLE_OFFSET(first_linear_offset(va)) #define second_table_offset(va) TABLE_OFFSET(second_linear_offset(va)) #define third_table_offset(va) TABLE_OFFSET(third_linear_offset(va)) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 05/28] xen/arm64: head: Setup TTBR_EL2 in enable_mmu() and add missing isb
At the moment, TTBR_EL2 is setup in create_page_tables(). This is fine as it is called by every CPUs. However, such assumption may not hold in the future. To make change easier, the TTBR_EL2 is not setup in enable_mmu(). Take the opportunity to add the missing isb() to ensure the TTBR_EL2 is seen before the MMU is turned on. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's acked-by Changes in v2: - Patch added --- xen/arch/arm/arm64/head.S | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S index 925fb93e58..9b703e79ce 100644 --- a/xen/arch/arm/arm64/head.S +++ b/xen/arch/arm/arm64/head.S @@ -507,9 +507,7 @@ create_page_tables: cmp x19, #XEN_VIRT_START cset x25, eq/* x25 := identity map in place, or not */ -/* Write Xen's PT's paddr into TTBR0_EL2 */ load_paddr x4, boot_pgtable -msr TTBR0_EL2, x4 /* Setup boot_pgtable: */ load_paddr x1, boot_first @@ -637,6 +635,11 @@ enable_mmu: tlbi alle2 /* Flush hypervisor TLBs */ dsb nsh +/* Write Xen's PT's paddr into TTBR0_EL2 */ +load_paddr x0, boot_pgtable +msr TTBR0_EL2, x0 +isb + mrs x0, SCTLR_EL2 orr x0, x0, #SCTLR_Axx_ELx_M /* Enable MMU */ orr x0, x0, #SCTLR_Axx_ELx_C /* Enable D-cache */ -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 12/28] xen/arm32: head: Introduce print_reg
At the moment, the user should save r14/lr if it cares about it. Follow-up patches will introduce more use of putn in place where lr should be preserved. Furthermore, any user of putn should also move the value to register r0 if it was stored in a different register. For convenience, a new macro is introduced to print a given register. The macro will take care for us to move the value to r0 and also preserve lr. Lastly the new macro is used to replace all the callsite of putn. This will simplify rework/review later on. Signed-off-by: Julien Grall Reviewed-by: Stefano Stabellini --- Changes in v3: - Add Stefano's reviewed-by Changes in v2: - Patch added --- xen/arch/arm/arm32/head.S | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S index fd3a273550..c4ee06ba93 100644 --- a/xen/arch/arm/arm32/head.S +++ b/xen/arch/arm/arm32/head.S @@ -79,8 +79,25 @@ 98: .asciz _s ;\ .align 2 ;\ 99: + +/* + * Macro to print the value of register \rb + * + * Clobbers r0 - r4 + */ +.macro print_reg rb +mov r0, \rb +mov r4, lr +blputn +mov lr, r4 +.endm + #else /* CONFIG_EARLY_PRINTK */ #define PRINT(s) + +.macro print_reg rb +.endm + #endif /* !CONFIG_EARLY_PRINTK */ .arm @@ -159,8 +176,7 @@ GLOBAL(init_secondary) #ifdef CONFIG_EARLY_PRINTK mov_w r11, EARLY_UART_BASE_ADDRESS /* r11 := UART base address */ PRINT("- CPU ") -mov r0, r7 -blputn +print_reg r7 PRINT(" booting -\r\n") #endif @@ -211,8 +227,7 @@ skip_bss: bne 1f mov r4, r0 PRINT("- Missing processor info: ") -mov r0, r4 -blputn +print_reg r4 PRINT(" -\r\n") b fail 1: -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/vtd: Fix S3 resume following c/s 650c31d3af
c/s 650c31d3af "x86/IRQ: fix locking around vector management" adjusted the locking in adjust_irq_affinity(). The S3 path ends up here via iommu_resume() before interrupts are enabled, at which point spin_lock_irq() which fail ASSERT(local_irq_is_enabled()); but with no working console. Use spin_lock_irqsave() instead to cope with interrupts already being disabled. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné CC: Jun Nakajima CC: Kevin Tian I'm fairly confident that the AMD side of things is fine, because enable_iommu() is encompased by a spin_lock_irqsave() block. --- xen/drivers/passthrough/vtd/iommu.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 5d72270c5b..defa74fae3 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -2135,15 +2135,16 @@ static void adjust_irq_affinity(struct acpi_drhd_unit *drhd) : NUMA_NO_NODE; const cpumask_t *cpumask = NULL; struct irq_desc *desc; +unsigned long flags; if ( node < MAX_NUMNODES && node_online(node) && cpumask_intersects(&node_to_cpumask(node), &cpu_online_map) ) cpumask = &node_to_cpumask(node); desc = irq_to_desc(drhd->iommu->msi.irq); -spin_lock_irq(&desc->lock); +spin_lock_irqsave(&desc->lock, flags); dma_msi_set_affinity(desc, cpumask); -spin_unlock_irq(&desc->lock); +spin_unlock_irqrestore(&desc->lock, flags); } static int adjust_vtd_irq_affinities(void) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 140001: tolerable all pass - PUSHED
flight 140001 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/140001/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 3fda2214f1a7ff972427812e50dc6f1f61cf594f baseline version: xen b6b5608b6027fd62ce565ecd72a3422c1223beaf Last test of basis 139992 2019-08-12 11:00:43 Z0 days Testing same since 140001 2019-08-12 14:00:35 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich Julien Grall jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git b6b5608b60..3fda2214f1 3fda2214f1a7ff972427812e50dc6f1f61cf594f -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen / EC2 release criteria proposal
On Sat, 2019-08-10 at 20:12 +0200, Dridi Boukelmoune wrote: > Sorry for the top posting, "smart" phone... > > What about Qubes OS? Isn't their dom0 using xen, based on Fedora? > > Do they use Xen as packaged by Fedora? If not, couldn't they contribute > whatever they do that Fedora doesn't here? > > It might be worth getting in touch with them. They look like a significant > Xen user, on Fedora. I have no idea, but those seem like reasonable questions. I'll see if I can track down a contact point for them. Thanks! -- Adam Williamson Fedora QA Community Monkey IRC: adamw | Twitter: AdamW_Fedora | XMPP: adamw AT happyassassin . net http://www.happyassassin.net ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 139967: all pass - PUSHED
flight 139967 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/139967/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 4053587347a4c68402c1fc40921b7f1cdaec900e baseline version: ovmf 1a624dd7cf0db5783b27e18e6c790178d14a1e6a Last test of basis 139880 2019-08-09 20:40:11 Z2 days Testing same since 139967 2019-08-12 01:54:11 Z0 days1 attempts People who touched revisions under test: Star Zeng Zeng, Star 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 1a624dd7cf..4053587347 4053587347a4c68402c1fc40921b7f1cdaec900e -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/iommu: remove usage of {set/clear}_identity_p2m_entry against PV domains
On Mon, Aug 12, 2019 at 04:15:38PM +0100, George Dunlap wrote: > On 8/12/19 3:55 PM, Roger Pau Monné wrote: > > On Mon, Aug 12, 2019 at 03:24:36PM +0100, George Dunlap wrote: > >> On 8/12/19 2:56 PM, Roger Pau Monné wrote: > >>> On Mon, Aug 12, 2019 at 02:17:53PM +0100, George Dunlap wrote: > On 8/2/19 10:22 AM, Roger Pau Monne wrote: > > Switch rmrr_identity_mapping to use iommu_{un}map in order to > > establish RMRR mappings for PV domains, like it's done in > > arch_iommu_hwdom_init. This solves the issue of a PV hardware domain > > not getting RMRR mappings because {set/clear}_identity_p2m_entry was > > not properly updating the iommu page tables. > > Sorry, I think this description is somewhat backwards: you're saying > what you're doing first, and then saying what the problematic behavior > was, but not actually saying what was causing the problematic behavior. > > Why was {set,clear}_identity_p2m not updating the page tables? > > I agree with Jan, that figuring that out is a prerequisite for any kind > of fix: if `need_iommu_pt_sync()` is false at that point for the > hardware domain, then there's a bigger problem than RMRRs not being > adjusted properly. > >>> > >>> need_iommu_pt_sync is indeed false for a PV hardware domain not > >>> running in strict mode, see: > >>> > >>> http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/passthrough/iommu.c;h=f8c3bf53bd1793df93d7ddea6564dc929f40c156;hb=HEAD#l192 > >>> https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html > >>> > >>> This is fine for a non-strict PV hardware domain, since it has all the > >>> host memory (minus memory used by Xen) mapped in the iommu page tables > >>> except for RMRR regions not marked as reserved in the e820 memory map, > >>> which are added using set_identity_p2m_entry. > >>> > >>> The issue here is that this patch alone doesn't fix the issue for the > >>> reporter, and there seems to be an additional flush required in order > >>> to get the issue solved on his end: > >>> > >>> https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00526.html > >>> > >>> My theory is that the system Roman is using is booting with DMA > >>> remapping enabled in the iommu, and somehow the call to > >>> iommu_flush_all in intel_iommu_hwdom_init doesn't seem to work > >>> properly, while calling iommu_iotlb_flush_all does seem to do the > >>> right thing. I'm still waiting for Roman to come back with the result > >>> of my last debug patches in order to figure out whether my hypothesis > >>> above is true. > >>> > >>> This however won't still explain the weird behaviour of > >>> iommu_flush_all, and further debugging will still be required. > >> > >> OK; so this patch has four effects, it looks like: > >> > >> 1. iommu_legacy_[un]map -> iommu_[un]map > >> 2. Move iommu ops out of {set,clear}_identity_p2m for PV guests; > >> open-code them in rmrr_identity_mapping > >> 3. For non-translated guests, do the operation unconditionally > >> 4. Add a flush > > > > There's already a flush in iommu_legacy_[un]map, so the flush is also > > done for both patches, just in different places. The legacy interface > > does the flush on every call, while the new interface allows to > > postpone the flush until all iommu page-table operations are done. > > > >> > >> Regarding #3, the previous patch changed it from "if > >> iommu_needs_pt_sync" to "if has_iommu_pt"; this one changes it to > >> "always". Is that what you intended? > > > > Well, #3 is not actually correct. The code in intel_iommu_hwdom_init > > and hence setup_hwdom_rmrr will only be called if the domain has an > > iommu, so has_iommu_pt will always be true when adding RMRR regions. > > Note rmrr_identity_mapping is the only caller of > > {set,clear}_identity_p2m against PV guests. > > But if iommu is set the same in both cases, and if the flush happens in > both cases, then why did this patch work and the previous patch didn't? iommu_legacy_[un]map uses iommu_iotlb_flush while my patch uses iommu_iotlb_flush_all. I'm still not sure why this difference in behaviour, similar to what happens with iommu_flush_all. > >> I don't really see the point of #2: from the device's perspective, the > >> "physmap" is the IOMMU for PV guests, and IOMMU+p2m for HVM guests; it > >> makes sense to have a single place to call for either type of guest, > >> rather than open-coding every location. > > > > OK, that's fine, but note that {set/clear}_identity_p2m_entry is > > AFAICT the only p2m function allowed against PV guests, the rest will > > return some kind of error, which IMO makes it the outlier, hence my > > proposal to make it available to translated guests only, like the rest > > of the p2m functions in that file. I just find it confusing that some > > p2m functions can be used against PV guests, but not others. > > Right, but that's because set_identity_p2m_entry() isn't really about
[Xen-devel] [PATCH] x86/boot: Simplify %fs setup in trampoline_setup
mov/shr is easier to follow than shld, and doesn't have a merge dependency on the previous value of %edx. Shorten the rest of the code by streamlining the comments. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné In addition to being clearer to follow, mov/shr is faster than shld to decode and execute. See https://godbolt.org/z/A5kvuC for the latency/throughput/port analysis, the Intel Optimisation guide which classifes them as "Slow Int" instructions, or the AMD Optimisation guide which specifically has a section entitled "Alternatives to SHLD Instruction". --- xen/arch/x86/boot/head.S | 27 ++- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 782deac0d4..26b680521d 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -556,24 +556,17 @@ trampoline_setup: /* * Called on legacy BIOS and EFI platforms. * - * Initialize bits 0-15 of BOOT_FS segment descriptor base address. + * Set the BOOT_FS descriptor base address to %esi. */ -mov %si,BOOT_FS+2+sym_esi(trampoline_gdt) - -/* Initialize bits 16-23 of BOOT_FS segment descriptor base address. */ -shld$16,%esi,%edx -mov %dl,BOOT_FS+4+sym_esi(trampoline_gdt) - -/* Initialize bits 24-31 of BOOT_FS segment descriptor base address. */ -mov %dh,BOOT_FS+7+sym_esi(trampoline_gdt) - -/* - * Initialize %fs and later use it to access Xen data where possible. - * According to Intel 64 and IA-32 Architectures Software Developer's - * Manual it is safe to do that without reloading GDTR before. - */ -mov $BOOT_FS,%edx -mov %edx,%fs +mov %esi, %edx +shr $16, %edx +mov %si, BOOT_FS + 2 + sym_esi(trampoline_gdt) /* Bits 0-15 */ +mov %dl, BOOT_FS + 4 + sym_esi(trampoline_gdt) /* Bits 16-23 */ +mov %dh, BOOT_FS + 7 + sym_esi(trampoline_gdt) /* Bits 24-31 */ + +/* Load %fs to allow for access to Xen data. */ +mov $BOOT_FS, %edx +mov %edx, %fs /* Save Xen image load base address for later use. */ mov %esi,sym_fs(xen_phys_start) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Terminology for "guest" - Was: [PATCH] docs/sphinx: Introduction
On 12/08/2019 15:53, George Dunlap wrote: > On 8/8/19 10:13 AM, Julien Grall wrote: >> Hi Jan, >> >> On 08/08/2019 10:04, Jan Beulich wrote: >>> On 08.08.2019 10:43, Andrew Cooper wrote: On 08/08/2019 07:22, Jan Beulich wrote: > On 07.08.2019 21:41, Andrew Cooper wrote: >> --- /dev/null >> +++ b/docs/glossary.rst >> @@ -0,0 +1,37 @@ >> +Glossary >> + >> + >> +.. Terms should appear in alphabetical order >> + >> +.. glossary:: >> + >> + control domain >> + A :term:`domain`, commonly dom0, with the permission and >> responsibility >> + to create and manage other domains on the system. >> + >> + domain >> + A domain is Xen's unit of resource ownership, and generally has >> at the >> + minimum some RAM and virtual CPUs. >> + >> + The terms :term:`domain` and :term:`guest` are commonly used >> + interchangeably, but they mean subtly different things. >> + >> + A guest is a single virtual machine. >> + >> + Consider the case of live migration where, for a period of >> time, one >> + guest will be comprised of two domains, while it is in transit. >> + >> + domid >> + The numeric identifier of a running :term:`domain`. It is >> unique to a >> + single instance of Xen, used as the identifier in various APIs, >> and is >> + typically allocated sequentially from 0. >> + >> + guest >> + See :term:`domain` > I think you want to mention the usual distinction here: Dom0 is, > while a domain, commonly not considered a guest. To be honest, I had totally forgotten about that. I guess now is the proper time to rehash it in public. I don't think the way it currently gets used has a clear or coherent set of rules, because I can't think of any to describe how it does get used. Either there are a clear and coherent (and simple!) set of rules for what we mean by "guest", at which point they can live here in the glossary, or the fuzzy way it is current used should cease. >>> What's fuzzy about Dom0 not being a guest (due to being a part of the >>> host instead)? >> Dom0 is not part of the host if you are using an hardware domain. I >> actually thought we renamed everything in Xen from Dom0 to hwdom to >> avoid the confusion. >> >> I also would rather avoid to treat "dom0" as not a guest. In normal >> setup this is a more privilege guest, in other setup this may just be a >> normal guest (think about partitioning). > A literal guest is someone who doesn't live in the building (or work in > the buliding, if you're in a hotel). The fact that the staff cleaning > rooms are restricted in their privileges doesn't make them guests of the > hotel. > > The toolstack domain, the hardware domain, the driver domain, the > xenstore domain, and so on are all part of the host system, designed to > allow you to use Xen to do the thing you actually want to do: Run guests. > > And it's important that we have a word that distinguishes "domains that > we only care about because they make it possible to run other domains", > and "domains that we actually want to run". "guest / host" is a natural > terminology for these. > > We already have the word "domain", which includes dom0, driver domains, > toolstack domains, hardware domains, as well as guest domains. We don't > need "guest" to be a synonym for "domain". So... Please can someone propose wording simple, clear wording for what "guest" means. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/iommu: remove usage of {set/clear}_identity_p2m_entry against PV domains
On Mon, Aug 12, 2019 at 03:24:36PM +0100, George Dunlap wrote: > On 8/12/19 2:56 PM, Roger Pau Monné wrote: > > On Mon, Aug 12, 2019 at 02:17:53PM +0100, George Dunlap wrote: > >> On 8/2/19 10:22 AM, Roger Pau Monne wrote: > >>> Switch rmrr_identity_mapping to use iommu_{un}map in order to > >>> establish RMRR mappings for PV domains, like it's done in > >>> arch_iommu_hwdom_init. This solves the issue of a PV hardware domain > >>> not getting RMRR mappings because {set/clear}_identity_p2m_entry was > >>> not properly updating the iommu page tables. > >> > >> Sorry, I think this description is somewhat backwards: you're saying > >> what you're doing first, and then saying what the problematic behavior > >> was, but not actually saying what was causing the problematic behavior. > >> > >> Why was {set,clear}_identity_p2m not updating the page tables? > >> > >> I agree with Jan, that figuring that out is a prerequisite for any kind > >> of fix: if `need_iommu_pt_sync()` is false at that point for the > >> hardware domain, then there's a bigger problem than RMRRs not being > >> adjusted properly. > > > > need_iommu_pt_sync is indeed false for a PV hardware domain not > > running in strict mode, see: > > > > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/passthrough/iommu.c;h=f8c3bf53bd1793df93d7ddea6564dc929f40c156;hb=HEAD#l192 > > https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html > > > > This is fine for a non-strict PV hardware domain, since it has all the > > host memory (minus memory used by Xen) mapped in the iommu page tables > > except for RMRR regions not marked as reserved in the e820 memory map, > > which are added using set_identity_p2m_entry. > > > > The issue here is that this patch alone doesn't fix the issue for the > > reporter, and there seems to be an additional flush required in order > > to get the issue solved on his end: > > > > https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00526.html > > > > My theory is that the system Roman is using is booting with DMA > > remapping enabled in the iommu, and somehow the call to > > iommu_flush_all in intel_iommu_hwdom_init doesn't seem to work > > properly, while calling iommu_iotlb_flush_all does seem to do the > > right thing. I'm still waiting for Roman to come back with the result > > of my last debug patches in order to figure out whether my hypothesis > > above is true. > > > > This however won't still explain the weird behaviour of > > iommu_flush_all, and further debugging will still be required. > > OK; so this patch has four effects, it looks like: > > 1. iommu_legacy_[un]map -> iommu_[un]map > 2. Move iommu ops out of {set,clear}_identity_p2m for PV guests; > open-code them in rmrr_identity_mapping > 3. For non-translated guests, do the operation unconditionally > 4. Add a flush There's already a flush in iommu_legacy_[un]map, so the flush is also done for both patches, just in different places. The legacy interface does the flush on every call, while the new interface allows to postpone the flush until all iommu page-table operations are done. > > Regarding #3, the previous patch changed it from "if > iommu_needs_pt_sync" to "if has_iommu_pt"; this one changes it to > "always". Is that what you intended? Well, #3 is not actually correct. The code in intel_iommu_hwdom_init and hence setup_hwdom_rmrr will only be called if the domain has an iommu, so has_iommu_pt will always be true when adding RMRR regions. Note rmrr_identity_mapping is the only caller of {set,clear}_identity_p2m against PV guests. > I don't really see the point of #2: from the device's perspective, the > "physmap" is the IOMMU for PV guests, and IOMMU+p2m for HVM guests; it > makes sense to have a single place to call for either type of guest, > rather than open-coding every location. OK, that's fine, but note that {set/clear}_identity_p2m_entry is AFAICT the only p2m function allowed against PV guests, the rest will return some kind of error, which IMO makes it the outlier, hence my proposal to make it available to translated guests only, like the rest of the p2m functions in that file. I just find it confusing that some p2m functions can be used against PV guests, but not others. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/6] use is_iommu_enabled() where appropriate...
> -Original Message- > From: Jan Beulich > Sent: 07 August 2019 10:56 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Brian Woods ; > Suravee Suthikulpanit > ; Julien Grall ; Andrew > Cooper > ; Roger Pau Monne ; > VolodymyrBabchuk > ; George Dunlap ; Jun > Nakajima > ; Kevin Tian ; Stefano > Stabellini > ; Wei Liu > Subject: Re: [PATCH 2/6] use is_iommu_enabled() where appropriate... > > On 30.07.2019 15:44, Paul Durrant wrote: > > --- a/xen/arch/arm/p2m.c > > +++ b/xen/arch/arm/p2m.c > > @@ -1531,8 +1531,7 @@ int p2m_init(struct domain *d) > >* shared with the CPU, Xen has to make sure that the PT changes have > >* reached the memory > >*/ > > -p2m->clean_pte = iommu_enabled && > > -!iommu_has_feature(d, IOMMU_FEAT_COHERENT_WALK); > > +p2m->clean_pte = !iommu_has_feature(d, IOMMU_FEAT_COHERENT_WALK); > > I can't tell if the original code was meant to be this way, but I'm > afraid your transformation is not correct: The prior construct, > expanding iommu_has_feature(), was > > iommu_enabled && !(iommu_enabled && test_bit(feature, dom_iommu(d)->features)) > > which transforms through > > iommu_enabled && (!iommu_enabled || !test_bit(feature, > dom_iommu(d)->features)) > > to > > (iommu_enabled && !iommu_enabled) || (iommu_enabled && !test_bit(feature, > dom_iommu(d)->features)) > > and hence > > iommu_enabled && !test_bit(feature, dom_iommu(d)->features) > > whereas the new code simply is > > !(iommu_enabled && test_bit(feature, dom_iommu(d)->features)) > > i.e. > > !iommu_enabled || !test_bit(feature, dom_iommu(d)->features) Yes, somehow I'd read that the iommu_enabled was inverted in the first instance. I'll add a check of is_iommu_enabled() back into p2m_init(). > > > @@ -766,7 +766,7 @@ ept_set_entry(struct p2m_domain *p2m, gfn_t gfn_, mfn_t > > mfn, > > new_entry.sp = !!i; > > new_entry.sa_p2mt = p2mt; > > new_entry.access = p2ma; > > -new_entry.snp = (iommu_enabled && iommu_snoop); > > +new_entry.snp = is_iommu_enabled(p2m->domain) && iommu_snoop; > > Please use d here. > > Seeing that this is the last change in x86/mm/, did you overlook > the use in p2m_pt_set_entry()? Or is this meant to go on top of > Alexandru's "x86/mm: Clean IOMMU flags from p2m-pt code" (which > should then be noted in a post-commit-message remark)? Yes, it needs to go on top of Alexandru's patch. I said the series is based on that patch in the cover letter but I can state it here as well if it helps. > > > @@ -556,7 +556,7 @@ int iommu_do_domctl( > > { > > int ret = -ENODEV; > > > > -if ( !iommu_enabled ) > > +if ( !is_iommu_enabled(d) ) > > return -ENOSYS; > > ENOSYS was wrong here from the beginning, but it certainly gets > worse with this no longer being a system wide property. Please > change to EOPNOTSUPP or some other suitable one. Sure. I'll go with EOPNOTSUPP. > > > --- a/xen/drivers/passthrough/pci.c > > +++ b/xen/drivers/passthrough/pci.c > > @@ -864,7 +864,7 @@ static int pci_clean_dpci_irqs(struct domain *d) > > Above here there's another use in pci_enable_acs(), which should > imo act on pdev->domain. Oh yes, I'll fix that. > > There's another use in flask_iommu_resource_use_perm(). All > callers of the function hold a struct domain * in their hands, > which I think they should pass into this function such that the > conditional can be replaced. I wasn't sure about this one. It looks more like the perm passed back is based on the hardware features available but I guess it will DTRT if the IOMMU is not enabled for the domain. Paul > > Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [edk2-devel] [PATCH v4 29/35] OvmfPkg/OvmfXen: Override PcdFSBClock to Xen vLAPIC timer frequency
On Thu, Aug 08, 2019 at 05:18:15PM +0200, Roger Pau Monné wrote: > On Thu, Aug 08, 2019 at 03:26:41PM +0100, Anthony PERARD wrote: > > So EDKII doesn't have that capability, FSBClock is a build time value > > and can't be changed at run time. But OVMF (on KVM or HVM) doesn't use > > that value at all, it uses the ACPI timer instead. > > But after your series both PVH and HVM will use the lapic timer > instead of the ACPI timer, and thus rely on the value of FSBClock? Short answer, Yes. Longuer answer, after the series is applied, there will be a new platform, "OvmfXen" which will be for both Xen PVH and Xen HVM, but the OVMF that we know (OvmfPkgX64 that xen.git builds) will still be capable of running on Xen HVM for a short while and will still use the ACPI timer. -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC] Code of Conduct
On 8/12/19 3:27 PM, Lars Kurth wrote: > I am wondering how you feel about the usage of "participant". There are > a few instances left in the text. > > "Any report of harassment within the Xen Project community will be addressed > swiftly. Participants asked to stop ..." > > # Consequences of Unacceptable Behavior > If a participant engages in harassing behaviour > > I would probably also want to replace this with "Community member asked ..." > and "If a community member engages in ..." Seems reasonable to me. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RFC] Code of Conduct
Hi George, On 12/08/2019, 12:35, "George Dunlap" wrote: On 8/9/19 6:48 PM, Lars Kurth wrote: > Hi all, > > Following the discussion we had at the Developer Summit (see https://wiki.xenproject.org/wiki/Design_Sessions_2019#Community_Issues_.2F_Improvements_-_Communication.2C_Code_of_Conduct.2C_etc. for notes) I put together a draft for the Code of Conduct which can be found here as well as inlined below > https://docs.google.com/document/d/1NnWdU_VnC1N_ZzxQG6jU9fnY2GPVCcfPJT5KY61WXJM/edit?usp=sharing > > It is based on the LF Events CoC as we agreed on (the diff is attached). I took the scope and enforcement sections from https://www.contributor-covenant.org/version/1/4/code-of-conduct.html and simplified it rather than inventing something new. > > You can provide feedback by commenting on the google doc or by replying to the in-lined version below. > I expect it will some more discussion to get consensus. > > Note that I am not very attached to some of the terms, such as "Xen Project CoC Team" and in some cases "participant" should probably be replaced by community > members. > > But I felt, we should have something more concrete to discuss compared to previous discussions. > > A Code of Conduct is a project wide policy change: thus, all subprojects lists are CC'ed Thanks for doing this Lars. I think this is a step forward. I have a couple of comments, but only on the wording. > > Regards > Lars > > Here is the actual text > --- > # Our Pledge > In the interest of fostering an open and welcoming environment, we as community > members of the Xen Project pledge to making participation in our project and our > community a harassment-free experience for everyone. To me "pledge" means "promise"; and I don't think we can promise anyone that they'll have a harassment-free experience. I might say, "we ... are committed to making participation ... a harassment-free experience"; or "we ... pledge to maintain a harassment-free experience" or something like that. This comes directly from the Contributor Covenant v1.4 But I also like "we ... are committed to making participation ... a harassment-free experience" better then pledge. > # Unacceptable Behavior > Harassment will not be tolerated in the Xen Project Community in any form, > including but not limited to harassment based on gender, gender identity and > expression, sexual orientation, disability, physical appearance, body size, race, > age, religion, ethnicity, nationality, level of experience, education, or > socio-economic status or any other status protected by laws in jurisdictions in > which community members are based. > Harassment includes the use of abusive, > offensive or degrading language, intimidation, stalking, harassing photography > or recording, inappropriate physical contact, sexual imagery and unwelcome > sexual advances, requests for sexual favors, publishing others' private > information such as a physical or electronic address without explicit permission > and other conduct which could reasonably be considered inappropriate in a > professional setting. Should we put "such as physical or electronic address[es]" in parentheses? Fine with me Also, I'm in favor of the Oxford Comma (so a comma after 'permission'). I might say "or any other conduct"; for some reason it sounds more natural to me. Either works > Any report of harassment within the Xen Project community will be addressed > swiftly. Participants asked to stop any harassing behavior are expected to > comply immediately. Anyone who witnesses or is subjected to unacceptable > behavior should notify the Xen Project’s CoC team via cond...@xenproject.org. > > # Consequences of Unacceptable Behavior > If a participant engages in harassing behavior, the Xen Project’s CoC team may > take any action it deems appropriate, ranging from issuance of a warning to the > offending individual to expulsion from the Xen Project community. I realize by saying "range" you probably meant to include this, but I think spelling out "temporary suspension" as a possible consequence. E.g.: "If a participant engages in harassing behavior, the Xen Project's CoC team will investigate and take an action it deems appropriate against the offending individual. This may include issuing a warning, temporary suspension from mailing lists or commit rights, or expulsion from the XenProject community." That looks good That's all I had; thanks again, Lars. I am wondering how you feel about the usage of "participant". There are a few instances left in the text. "Any report of harassment within the Xen Project com
Re: [Xen-devel] [PATCH] x86/iommu: remove usage of {set/clear}_identity_p2m_entry against PV domains
On 8/12/19 2:56 PM, Roger Pau Monné wrote: > On Mon, Aug 12, 2019 at 02:17:53PM +0100, George Dunlap wrote: >> On 8/2/19 10:22 AM, Roger Pau Monne wrote: >>> Switch rmrr_identity_mapping to use iommu_{un}map in order to >>> establish RMRR mappings for PV domains, like it's done in >>> arch_iommu_hwdom_init. This solves the issue of a PV hardware domain >>> not getting RMRR mappings because {set/clear}_identity_p2m_entry was >>> not properly updating the iommu page tables. >> >> Sorry, I think this description is somewhat backwards: you're saying >> what you're doing first, and then saying what the problematic behavior >> was, but not actually saying what was causing the problematic behavior. >> >> Why was {set,clear}_identity_p2m not updating the page tables? >> >> I agree with Jan, that figuring that out is a prerequisite for any kind >> of fix: if `need_iommu_pt_sync()` is false at that point for the >> hardware domain, then there's a bigger problem than RMRRs not being >> adjusted properly. > > need_iommu_pt_sync is indeed false for a PV hardware domain not > running in strict mode, see: > > http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/passthrough/iommu.c;h=f8c3bf53bd1793df93d7ddea6564dc929f40c156;hb=HEAD#l192 > https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html > > This is fine for a non-strict PV hardware domain, since it has all the > host memory (minus memory used by Xen) mapped in the iommu page tables > except for RMRR regions not marked as reserved in the e820 memory map, > which are added using set_identity_p2m_entry. > > The issue here is that this patch alone doesn't fix the issue for the > reporter, and there seems to be an additional flush required in order > to get the issue solved on his end: > > https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00526.html > > My theory is that the system Roman is using is booting with DMA > remapping enabled in the iommu, and somehow the call to > iommu_flush_all in intel_iommu_hwdom_init doesn't seem to work > properly, while calling iommu_iotlb_flush_all does seem to do the > right thing. I'm still waiting for Roman to come back with the result > of my last debug patches in order to figure out whether my hypothesis > above is true. > > This however won't still explain the weird behaviour of > iommu_flush_all, and further debugging will still be required. OK; so this patch has four effects, it looks like: 1. iommu_legacy_[un]map -> iommu_[un]map 2. Move iommu ops out of {set,clear}_identity_p2m for PV guests; open-code them in rmrr_identity_mapping 3. For non-translated guests, do the operation unconditionally 4. Add a flush Regarding #3, the previous patch changed it from "if iommu_needs_pt_sync" to "if has_iommu_pt"; this one changes it to "always". Is that what you intended? I don't really see the point of #2: from the device's perspective, the "physmap" is the IOMMU for PV guests, and IOMMU+p2m for HVM guests; it makes sense to have a single place to call for either type of guest, rather than open-coding every location. Can't comment on the other aspects. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [freebsd-master test] 139987: trouble: blocked/broken
flight 139987 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/139987/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-freebsd broken build-amd64-freebsd 5 host-install(5)broken REGR. vs. 139861 Tests which did not succeed, but are not blocking: build-amd64-freebsd-again 1 build-check(1) blocked n/a build-amd64-xen-freebsd 1 build-check(1) blocked n/a version targeted for testing: freebsd 57f16e900dfc21dc7ed636d71e4e19b0a43da1d0 baseline version: freebsd d3d0bd153cf3e76effd2e9e8c66a847d1c5defe3 Last test of basis 139861 2019-08-09 09:20:02 Z3 days Testing same since 139987 2019-08-12 09:20:31 Z0 days1 attempts People who touched revisions under test: asomers avos bdragon brooks cem cy delphij ian imp jhibbits kib manu ngie tsoome jobs: build-amd64-freebsd-againblocked build-amd64-freebsd broken build-amd64-xen-freebsd blocked sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job build-amd64-freebsd broken broken-step build-amd64-freebsd host-install(5) Not pushing. (No revision log; it would be 346 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/blkback: fix memory leaks
On 8/11/19 10:23 AM, Wenwen Wang wrote: > In read_per_ring_refs(), after 'req' and related memory regions are > allocated, xen_blkif_map() is invoked to map the shared frame, irq, and > etc. However, if this mapping process fails, no cleanup is performed, > leading to memory leaks. To fix this issue, invoke the cleanup before > returning the error. Applied, thanks. -- Jens Axboe ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/iommu: remove usage of {set/clear}_identity_p2m_entry against PV domains
On Mon, Aug 12, 2019 at 02:17:53PM +0100, George Dunlap wrote: > On 8/2/19 10:22 AM, Roger Pau Monne wrote: > > Switch rmrr_identity_mapping to use iommu_{un}map in order to > > establish RMRR mappings for PV domains, like it's done in > > arch_iommu_hwdom_init. This solves the issue of a PV hardware domain > > not getting RMRR mappings because {set/clear}_identity_p2m_entry was > > not properly updating the iommu page tables. > > Sorry, I think this description is somewhat backwards: you're saying > what you're doing first, and then saying what the problematic behavior > was, but not actually saying what was causing the problematic behavior. > > Why was {set,clear}_identity_p2m not updating the page tables? > > I agree with Jan, that figuring that out is a prerequisite for any kind > of fix: if `need_iommu_pt_sync()` is false at that point for the > hardware domain, then there's a bigger problem than RMRRs not being > adjusted properly. need_iommu_pt_sync is indeed false for a PV hardware domain not running in strict mode, see: http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/passthrough/iommu.c;h=f8c3bf53bd1793df93d7ddea6564dc929f40c156;hb=HEAD#l192 https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg01547.html This is fine for a non-strict PV hardware domain, since it has all the host memory (minus memory used by Xen) mapped in the iommu page tables except for RMRR regions not marked as reserved in the e820 memory map, which are added using set_identity_p2m_entry. The issue here is that this patch alone doesn't fix the issue for the reporter, and there seems to be an additional flush required in order to get the issue solved on his end: https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00526.html My theory is that the system Roman is using is booting with DMA remapping enabled in the iommu, and somehow the call to iommu_flush_all in intel_iommu_hwdom_init doesn't seem to work properly, while calling iommu_iotlb_flush_all does seem to do the right thing. I'm still waiting for Roman to come back with the result of my last debug patches in order to figure out whether my hypothesis above is true. This however won't still explain the weird behaviour of iommu_flush_all, and further debugging will still be required. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel