[PATCH 11/22] platform: generic: mips p8700: restrict memory region to physical memory as in DTS

[Vladimir Kondratiev]

> Hi Vladimir,

>>        dom = sbi_domain_thishart_ptr();
>>        reg = find_last_memregion(dom);
>> -     sbi_printf("Fix region[%ld]: base 0x%lx order %ld flags 0x%lx\n",
>> -                reg - dom->regions, reg->base, reg->order, reg->flags);
>> -     reg->order = sbi_fls(end_of_dram);

> I have some doubts on this. On the code level, `reg` comes from
> find_last_memregion, which exposes internal memory region data structure
> from `domain`. If modifying the base/order/flags directly, that bypasses
> the sanitation logic in root_add_memregion, which could invalidate the
> assumption in lib/ code. In your subsequent patch, there's also direct
> access to dom->regions (again internal structures). IMO, we shouldn't do
> that in platform code. Perhaps Anup has better thoughts.

Thanks for input. Fixed this, I will modify only "flags" for the last entry,
that should be OK. For the DRAM, will add new entry using API

>> -     sbi_printf("Modified order: %ld to match end of DRAM 0x%lx\n",
>> -                reg->order, end_of_dram);
>> +     ret = fixup_dram_region(dom, reg);

> Based on your description above, you are trying to block speculative
> access above the physical memory region. I encountered similar problem
> when coding up the EIC7700 platform. I also need to block access to some
> regions to avoid speculative access triggering bus errors. However I
> realized that just doing what you did here is not enough. For M mode, if
> there's no matching PMP (traditional) entries, then by default the access
> is granted. Hence, I don't see how you prevent speculation from M mode to
> system NOC.

Good catch. On my platform it worked by coincident, you're right - it is
safer to make last entry totally prohibitively, covering whole address range.
Did just this. Will upload v2 shortly, after running tests

Thanks, Vladimir