On Sun, Jun 16, 2019 at 4:37 PM Waldek Kozaczuk <[email protected]>
wrote:

>
> +    # virtual address space 1 GiB at a time
>>>>> +    # The very 1st entry maps 1st GiB 1:1 by pointing to ident_pt_l2
>>>>> table
>>>>> +    # that specifies addresses of every one of 512 2MiB slots of
>>>>> physical memory
>>>>> +    .quad ident_pt_l2 + 0x67 - OSV_KERNEL_VM_SHIFT
>>>>> +    # The 2nd entry maps 2nd GiB to the same 1st GiB of physical
>>>>> memory by pointing
>>>>> +    # to the same ident_pt_l2 table as the 1st entry above
>>>>> +    # This way we effectively provide correct mapping for the kernel
>>>>> linked
>>>>> +    # to start at 1 GiB + 2 MiB (0x40200000) in virtual memory and
>>>>> point to
>>>>> +    # 2 MiB address (0x200000) where it starts in physical memory
>>>>> +    .quad ident_pt_l2 + 0x67 - OSV_KERNEL_VM_SHIFT
>>>>>
>>>>
>>>> Oh, but doesn't this mean that this only works correctly when
>>>> OSV_KERNEL_VM_SHIFT is *exactly* 1 GB?
>>>> I.e., the reason why you want the mapping of the second gigabyte to be
>>>> identical to the first gigabyte is
>>>> just because the shift is exactly 1GB?
>>>>
>>> That is correct. The general scheme (which I am planning to make part of
>>> the next patch at some time) should be this:
>>> OSV_KERNEL_VM_SHIFT = 1 GiB + N * 2MiB where 0 =< N < 500 (more less as
>>> last 24MB of the 2nd GB should be enough for the kernel).
>>> But then instead of re-using and pointing to the ident_pt_l2 table I
>>> will have to define extra instance of ident_pt_l2-equivalent-table where
>>> the first N entries will be zero.
>>>
>>
>> So although in all other places in the code, OSV_KERNEL_VM_SHIFT can be
>> anything, in this specific
>> place you really do assume that OSV_KERNEL_VM_SHIFT= 1GB and nothing else
>> will work? If this
>> is the case, I think you should add a static assertion (if it's possible
>> in the assembly code?) or
>> equivalent, to fail the compilation if OSV_KERNEL_VM_SHIFT != 1 GB.
>>
> Indeed at least with this patch (until we have more dynamic formula) it
> all comes down to a single constraint where we reuse the pl3 table and the
> shift has to be be 1GB. How can we assert it with the compiler ?
>

Since  OSV_KERNEL_VM_SHIFT is a preprocessor macro, maybe we can check it
with the preprocessor - even in the assembler code, something like:

#if OSV_KERNEL_VM_SHIFT != 0x40000000 && OSV_KERNEL_VM_SHIFT != 0
#error This code only works correctly for OSV_KERNEL_VM_SHIFT = 0x40000000
or 0
#endif

Put it right next to the code which only works for these cases (correct me
if I'm wrong but
I think the same code works for either 0 and 0x40000000, but not anything
else?) and check that you get a compilation error for OSV_KERNEL_VM_SHIFT =
0x12345.

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CANEVyjupu8dZ4JtVW7bOuucOJSo-47Yo-BBQfDOBHy01eY-UkA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to