Messages in this thread | | | Subject | Re: [PATCH v1 01/10] powerpc/pseries/iommu: Replace hard-coded page shift | From | Alexey Kardashevskiy <> | Date | Thu, 3 Sep 2020 14:26:12 +1000 |
| |
On 02/09/2020 07:38, Leonardo Bras wrote: > On Mon, 2020-08-31 at 13:48 +1000, Alexey Kardashevskiy wrote: >>>>> Well, I created this TCE_RPN_BITS = 52 because the previous mask was a >>>>> hardcoded 40-bit mask (0xfffffffffful), for hard-coded 12-bit (4k) >>>>> pagesize, and on PAPR+/LoPAR also defines TCE as having bits 0-51 >>>>> described as RPN, as described before. >>>>> >>>>> IODA3 Revision 3.0_prd1 (OpenPowerFoundation), Figure 3.4 and 3.5. >>>>> shows system memory mapping into a TCE, and the TCE also has bits 0-51 >>>>> for the RPN (52 bits). "Table 3.6. TCE Definition" also shows it. >>>>> In fact, by the looks of those figures, the RPN_MASK should always be a >>>>> 52-bit mask, and RPN = (page >> tceshift) & RPN_MASK. >>>> >>>> I suspect the mask is there in the first place for extra protection >>>> against too big addresses going to the TCE table (or/and for virtial vs >>>> physical addresses). Using 52bit mask makes no sense for anything, you >>>> could just drop the mask and let c compiler deal with 64bit "uint" as it >>>> is basically a 4K page address anywhere in the 64bit space. Thanks, >>> >>> Assuming 4K pages you need 52 RPN bits to cover the whole 64bit >>> physical address space. The IODA3 spec does explicitly say the upper >>> bits are optional and the implementation only needs to support enough >>> to cover up to the physical address limit, which is 56bits of P9 / >>> PHB4. If you want to validate that the address will fit inside of >>> MAX_PHYSMEM_BITS then fine, but I think that should be done as a >>> WARN_ON or similar rather than just silently masking off the bits. >> >> We can do this and probably should anyway but I am also pretty sure we >> can just ditch the mask and have the hypervisor return an error which >> will show up in dmesg. > > Ok then, ditching the mask.
Well, you could run a little experiment and set some bits above that old mask and see how phyp reacts :)
> Thanks! >
-- Alexey
| |