|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 1/3] xen/riscv: always preset A/D bits in G-stage PTEs
On 10/7/26 1:32 PM, Oleksii Kurochko wrote: On 10/7/26 12:44 PM, Jan Beulich wrote:On 07.10.2026 12:03, Oleksii Kurochko wrote:On 10/6/26 5:20 PM, Jan Beulich wrote:On 05.10.2026 18:00, Baptiste Le Duc wrote:A RISC-V implementation can manage the PTE A/D bits in one of two ways: 1) Update the 'A' and 'D' PTE bits in hardware (ratified as Svadu). 2) Generate a page fault when 'A' and/or 'D' is clear, so that softwarecan set them (ratified as Svade).p2m_set_pte_flags() presets A/D in G-stage PTEs only when the device treeadvertises Svade.According to the code it seems like it is setting A/D bits always independently on what is DT file. Baptiste, am I missing something? Platforms that use scheme (2) without advertising Svade,such as the HiFive Premier P550, then hit guest-page faults that Xen doesnot handle.Is that a quirk then (to avoid the word "bug")? Since it's all DT that conveys available ISA information, why can't the DT for those systems simply be fixed? Depending on the answer here I might question the presence of the Fixes: tag below.I think it can at some point. Let me clarify why "at some point"... So how the things are working now: OpenSBI detects the available ISA extensions in two steps: 1. It parses the CPU nodes in the DTB. If a node has the riscv,isa-extensions property, OpenSBI uses it; otherwise it falls back to riscv,isa. riscv,isa-extensions describes the same extensions as riscv,isa, but as a list of strings, which makes it easier to parse. From this, OpenSBI builds a per-hart bitmap of available extensions (basically what we do in Xen). (Platform-specific code can also add extensions to the bitmap at this step.) 2. It then probes a small, fixed set of extensions (Sstc, Smaia, Sscofpmf, Smstateen, Ssstateen, Smcntrpmf, Sdtrig) by reading a CSR that the extension introduces and checking whether the access traps. This is similar to what we do for Sstc in Xen. Step 2 only adds extensions to the bitmap. It does not check or clear the extensions that came from the DT; the only exception is Zicntr, which is detected purely by probing. So for extensions such asSvade/Svadu, which have no CSR to probe, the DT is the only source of truth.Now, what OpenSBI does with Svade/Svadu specifically. First, it disables hardware A/D updating by default, i.e. it clears menvcfg.ADUE: Now, what OpenSBI does with Svade/Svadu specifically. First, it disables hardware A/D updating by default, i.e. it clears menvcfg.ADUE: /* Disable HW A/D updating by default */ menvcfg_val &= ~ENVCFG_ADUE; Then it sets ADUE again, depending on which extensions are listed in the DTB: /* * Assume only Svadu is supported when it is the only extension * present in the ISA string. Svade is assumed when neither are * present. When both are present we must default to Svade (see * the zero reset value of FWFT.PTE_AD_HW_UPDATING). */ if (!sbi_hart_has_extension(scratch, SBI_HART_EXT_SVADE)) __set_menvcfg_ext(SBI_HART_EXT_SVADU, ENVCFG_ADUE); So all OpenSBI does is set or clear menvcfg.ADUE based on what is mentined in DTB. So, to answer your original question: yes, this can be fixed just by updating the DTS. OpenSBI will then configure menvcfg.ADUE according to the extensions listed there. Note that the logic above is from OpenSBI v1.9. Versions v1.5–v1.8 behave the same when the DT lists only svadu, only svade, or both. When neither is listed, they leave ADUE unchanged instead of clearing it. Versions before v1.5 don't handle Svade/Svadu at all. But IIRC such approach was suggested in one of vX and it was rejected because we can't in Xen be sure that passed DTB is correct and corresponds to how real h/w is configured as specifically with ADUE bit in menvcfg we can't read it in HS-mode as an exception will occur as it is M-mode register. So we have to trust that OpenSBI (or other firmwarewill configure the A/D update scheme according to what is mentioned in DTS).So then basically the question is do we want to rely on that how OpenSBI configures h/w? It seems like it could be fine. For example, without support for the FWFT extension, we can't access or change the menvcfg bit anyway. The FWFT extension was ratified in 2025, so we have to trust OpenSBI anyway (or introduce our own extension or port the FWFT extension to an earlier OpenSBI version) before FWFT is started to be available. With this explanation would you (Jan) be okay to go with just providing proper DTS?I can't answer this, as I lack details on where the DTS is coming from. So far I thought this was a blob handed to Xen (as kind of a module) at boot. What you say above makes me uncertain about that. If it is a blob as I thought, how difficult is it for people to edit a broken blob, for it to correctly reflect their hardware's capabilities?It depends...So usually the things are going in this way products like U-boot, Linux has dts in its code source base and then these DTSes are compiled by build system to DTB, and then this DTB is put to the place where U-boot, Linux can reach it, or U-boot provides through a register DTB to Linux. Sometimes it also could be a case when what is U-boot/Linux source code base is taken as a base and then a new dts is provided based on them, for example, to choose what h/w should be enabled/disabled or probably for some specific board some device is connected to a different pin and so DTS will be needed to be updated, and so on...From one side, it could be super easy as DTB (blob) could be taken by anyone then just decompile (it using a default dtc compiler) to DTS, update the necessary properties and then updated DTS (using dtc compiler again) compile back to DTB.But on the other side it could be some kind of secure boot where all the binaries are signed and then it will be pretty hard to decompile and compile DTB with the same sign....If that's not "very simple", or if there's concern that people may object to doing such editing, then I think we could (should) go without that. All I then would ask is that the description be clarified a little further (clarifying that absence of both extensions is a valid, albeit historical situation).Also I am looking at henvcfg register which we could access in HS-mode and it also has ADUE bit and the description for it is the following:```If the Svadu extension is implemented, the ADUE bit controls whether hardware updating of PTE A/D bits is enabled for VS-stage address translation. When ADUE=1, hardware updating of PTE A/D bits is enabled during VS-stage address translation, and the implementation behaves as though the Svade extension were not implemented for VS-mode address translation. When ADUE=0, the implementation behaves as though Svade were implemented for VS-stage address translation. If Svadu is not implemented, ADUE is read-only zero```So, at some point, we could verify how OpenSBI configured the hardware and compare it with what is specified in the DTS file.The only problem is how to interpret this part: “If Svadu is not implemented, ADUE is read-only zero.” The spec does not explicitly state that Svade will be used in this case, but logically, it seems that if Svadu is not implemented, the implementation has no other choice but to provide Svade and so basically we could detect if Svade is used based only on ADUE bit.If we accept this assumption, we could fully verify, in HS-mode, both what is specified in the DTS and how OpenSBI configured the hardware. This would allow us to require users to provide a proper DTS with the A/ D bits update scheme correctly specified. Probably we could do such assumption as I found the following in the spec:The Svade extension requires page-fault exceptions be raised when PTE A/D bits need be set, hence Svade is implemented when ADUE=0. ~ Oleksii
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |