VMA merging updateds?

VMA merging updateds?

[Jarkko Sakkinen]

Hi

I started to look into this old issue with mm subsystem and SGX, i.e.
can we make SGX VMA's to merge together?

This demonstrates the problem pretty well:

https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/

It was result of brk() syscall being applied a few times.

BR, Jarkko

Re: VMA merging updateds?

[Jarkko Sakkinen]

s/VMA merging updateds/SGX VMA merging updates/

Pressed S by mistake in aerc.


On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> Hi
>
> I started to look into this old issue with mm subsystem and SGX, i.e.
> can we make SGX VMA's to merge together?
>
> This demonstrates the problem pretty well:
>
> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
>
> It was result of brk() syscall being applied a few times.

BR, Jarkko

Re: VMA merging updateds?

[Jarkko Sakkinen]

> On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> > Hi
> >
> > I started to look into this old issue with mm subsystem and SGX, i.e.
> > can we make SGX VMA's to merge together?
> >
> > This demonstrates the problem pretty well:
> >
> > https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> >
> > It was result of brk() syscall being applied a few times.

Briging some context here. This can be fixed in the run-time by book
keeping the ranges and doing unmapping/mapping. I guess this goes
beyond what mm should support?

I thought to plain check this as it has been two years since my last
query on topic (if we could improve either the driver or mm somehow).

BR, Jarkko

Re: VMA merging updateds?

[Jarkko Sakkinen]

On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> > On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> > > Hi
> > >
> > > I started to look into this old issue with mm subsystem and SGX, i.e.
> > > can we make SGX VMA's to merge together?
> > >
> > > This demonstrates the problem pretty well:
> > >
> > > https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> > >
> > > It was result of brk() syscall being applied a few times.
>
> Briging some context here. This can be fixed in the run-time by book
> keeping the ranges and doing unmapping/mapping. I guess this goes
> beyond what mm should support?
>
> I thought to plain check this as it has been two years since my last
> query on topic (if we could improve either the driver or mm somehow).

In the past I've substituted kernel's mm merge code with user space
replacement:

https://github.com/enarx/mmledger/blob/main/src/lib.rs

It's essentially a reimplementation of al stuff that goes into 
mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
which map over existing ranges were working correctly, i.e. was
the issue only concerning adjacent VMA's.

What I'm looking here is that can we make some cosntraints that
if satisfied by the pfnmap code, it could leverage the code from
vma_merge(). Perhaps by making explicit call to vma_merge()?
I get that implicit use moves too much responsibility to the mm
subsystem.

BR, Jarkko

Re: VMA merging updateds?

[Huang, Kai]

On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
>>> On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
>>>> Hi
>>>>
>>>> I started to look into this old issue with mm subsystem and SGX, i.e.
>>>> can we make SGX VMA's to merge together?
>>>>
>>>> This demonstrates the problem pretty well:
>>>>
>>>> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
>>>>
>>>> It was result of brk() syscall being applied a few times.
>>
>> Briging some context here. This can be fixed in the run-time by book
>> keeping the ranges and doing unmapping/mapping. I guess this goes
>> beyond what mm should support?
>>
>> I thought to plain check this as it has been two years since my last
>> query on topic (if we could improve either the driver or mm somehow).
> 
> In the past I've substituted kernel's mm merge code with user space
> replacement:
> 
> https://github.com/enarx/mmledger/blob/main/src/lib.rs
> 
> It's essentially a reimplementation of al stuff that goes into
> mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> which map over existing ranges were working correctly, i.e. was
> the issue only concerning adjacent VMA's.
> 
> What I'm looking here is that can we make some cosntraints that
> if satisfied by the pfnmap code, it could leverage the code from
> vma_merge(). Perhaps by making explicit call to vma_merge()?
> I get that implicit use moves too much responsibility to the mm
> subsystem.
> 

Hi Jarkko,

Just want to understand more on the background:

Are you seeing any real problem due to needing a lot of mmap()s to the 
same enclave, or it is just a problem that doesn't look nice and you 
want to resolve?

I mean, this problem doesn't seem to be SGX-specific but a common one 
for VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random 
device drivers with mmap() support.  We will need a good justification 
if we want to make any core-mm change, if any, for this.

Re: VMA merging updateds?

[Jarkko Sakkinen]

On Thu Sep 26, 2024 at 3:07 AM EEST, Kai Huang wrote:
>
>
> On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> > On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> >>> On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> >>>> Hi
> >>>>
> >>>> I started to look into this old issue with mm subsystem and SGX, i.e.
> >>>> can we make SGX VMA's to merge together?
> >>>>
> >>>> This demonstrates the problem pretty well:
> >>>>
> >>>> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> >>>>
> >>>> It was result of brk() syscall being applied a few times.
> >>
> >> Briging some context here. This can be fixed in the run-time by book
> >> keeping the ranges and doing unmapping/mapping. I guess this goes
> >> beyond what mm should support?
> >>
> >> I thought to plain check this as it has been two years since my last
> >> query on topic (if we could improve either the driver or mm somehow).
> > 
> > In the past I've substituted kernel's mm merge code with user space
> > replacement:
> > 
> > https://github.com/enarx/mmledger/blob/main/src/lib.rs
> > 
> > It's essentially a reimplementation of al stuff that goes into
> > mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> > which map over existing ranges were working correctly, i.e. was
> > the issue only concerning adjacent VMA's.
> > 
> > What I'm looking here is that can we make some cosntraints that
> > if satisfied by the pfnmap code, it could leverage the code from
> > vma_merge(). Perhaps by making explicit call to vma_merge()?
> > I get that implicit use moves too much responsibility to the mm
> > subsystem.
> > 
>
> Hi Jarkko,
>
> Just want to understand more on the background:
>
> Are you seeing any real problem due to needing a lot of mmap()s to the 
> same enclave, or it is just a problem that doesn't look nice and you 
> want to resolve?
>
> I mean, this problem doesn't seem to be SGX-specific but a common one 
> for VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random 
> device drivers with mmap() support.  We will need a good justification 
> if we want to make any core-mm change, if any, for this.

It requires essentially replicating core mm in user space.

It's a manageable problem but feels silly since logic in merging
is mostly 1:1. It's not a problem for me personally as I'm not
making any money from SGX (more so to Intel).

BR, Jarkko

Re: VMA merging updateds?

[Jarkko Sakkinen]

qn Thu Sep 26, 2024 at 3:33 AM EEST, Jarkko Sakkinen wrote:
> On Thu Sep 26, 2024 at 3:07 AM EEST, Kai Huang wrote:
> >
> >
> > On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> > > On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> > >>> On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> > >>>> Hi
> > >>>>
> > >>>> I started to look into this old issue with mm subsystem and SGX, i.e.
> > >>>> can we make SGX VMA's to merge together?
> > >>>>
> > >>>> This demonstrates the problem pretty well:
> > >>>>
> > >>>> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> > >>>>
> > >>>> It was result of brk() syscall being applied a few times.
> > >>
> > >> Briging some context here. This can be fixed in the run-time by book
> > >> keeping the ranges and doing unmapping/mapping. I guess this goes
> > >> beyond what mm should support?
> > >>
> > >> I thought to plain check this as it has been two years since my last
> > >> query on topic (if we could improve either the driver or mm somehow).
> > > 
> > > In the past I've substituted kernel's mm merge code with user space
> > > replacement:
> > > 
> > > https://github.com/enarx/mmledger/blob/main/src/lib.rs
> > > 
> > > It's essentially a reimplementation of al stuff that goes into
> > > mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> > > which map over existing ranges were working correctly, i.e. was
> > > the issue only concerning adjacent VMA's.
> > > 
> > > What I'm looking here is that can we make some cosntraints that
> > > if satisfied by the pfnmap code, it could leverage the code from
> > > vma_merge(). Perhaps by making explicit call to vma_merge()?
> > > I get that implicit use moves too much responsibility to the mm
> > > subsystem.
> > > 
> >
> > Hi Jarkko,
> >
> > Just want to understand more on the background:
> >
> > Are you seeing any real problem due to needing a lot of mmap()s to the 
> > same enclave, or it is just a problem that doesn't look nice and you 
> > want to resolve?
> >
> > I mean, this problem doesn't seem to be SGX-specific but a common one 
> > for VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random 
> > device drivers with mmap() support.  We will need a good justification 
> > if we want to make any core-mm change, if any, for this.
>
> It requires essentially replicating core mm in user space.
>
> It's a manageable problem but feels silly since logic in merging
> is mostly 1:1. It's not a problem for me personally as I'm not
> making any money from SGX (more so to Intel).

I.e. in the case when you want do a syscall shim. You fix it up by
maintaining reflected version of the VMA database in the user space
and remapping everything based on that for every possible mm-call.

I've implemented such feature in the past for Enarx so it is entirely
possible.

BR, Jarkko

Re: VMA merging updateds?

[Huang, Kai]

On 26/09/2024 12:33 pm, Jarkko Sakkinen wrote:
> On Thu Sep 26, 2024 at 3:07 AM EEST, Kai Huang wrote:
>>
>>
>> On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
>>> On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
>>>>> On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
>>>>>> Hi
>>>>>>
>>>>>> I started to look into this old issue with mm subsystem and SGX, i.e.
>>>>>> can we make SGX VMA's to merge together?
>>>>>>
>>>>>> This demonstrates the problem pretty well:
>>>>>>
>>>>>> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
>>>>>>
>>>>>> It was result of brk() syscall being applied a few times.
>>>>
>>>> Briging some context here. This can be fixed in the run-time by book
>>>> keeping the ranges and doing unmapping/mapping. I guess this goes
>>>> beyond what mm should support?
>>>>
>>>> I thought to plain check this as it has been two years since my last
>>>> query on topic (if we could improve either the driver or mm somehow).
>>>
>>> In the past I've substituted kernel's mm merge code with user space
>>> replacement:
>>>
>>> https://github.com/enarx/mmledger/blob/main/src/lib.rs
>>>
>>> It's essentially a reimplementation of al stuff that goes into
>>> mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
>>> which map over existing ranges were working correctly, i.e. was
>>> the issue only concerning adjacent VMA's.
>>>
>>> What I'm looking here is that can we make some cosntraints that
>>> if satisfied by the pfnmap code, it could leverage the code from
>>> vma_merge(). Perhaps by making explicit call to vma_merge()?
>>> I get that implicit use moves too much responsibility to the mm
>>> subsystem.
>>>
>>
>> Hi Jarkko,
>>
>> Just want to understand more on the background:
>>
>> Are you seeing any real problem due to needing a lot of mmap()s to the
>> same enclave, or it is just a problem that doesn't look nice and you
>> want to resolve?
>>
>> I mean, this problem doesn't seem to be SGX-specific but a common one
>> for VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random
>> device drivers with mmap() support.  We will need a good justification
>> if we want to make any core-mm change, if any, for this.
> 
> It requires essentially replicating core mm in user space.
> 

Is there any real problem if we don't do this at all (neither in core mm 
nor in userspace)?

Re: VMA merging updateds?

[Huang, Kai]

>>>>>>> I started to look into this old issue with mm subsystem and SGX, i.e.
>>>>>>> can we make SGX VMA's to merge together?
>>>>>>>
>>>>>>> This demonstrates the problem pretty well:
>>>>>>>
>>>>>>> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
>>>>>>>
>>>>>>> It was result of brk() syscall being applied a few times.
>>>>>
>>>>> Briging some context here. This can be fixed in the run-time by book
>>>>> keeping the ranges and doing unmapping/mapping. I guess this goes
>>>>> beyond what mm should support?
>>>>>
>>>>> I thought to plain check this as it has been two years since my last
>>>>> query on topic (if we could improve either the driver or mm somehow).
>>>>
>>>> In the past I've substituted kernel's mm merge code with user space
>>>> replacement:
>>>>
>>>> https://github.com/enarx/mmledger/blob/main/src/lib.rs
>>>>
>>>> It's essentially a reimplementation of al stuff that goes into
>>>> mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
>>>> which map over existing ranges were working correctly, i.e. was
>>>> the issue only concerning adjacent VMA's.
>>>>
>>>> What I'm looking here is that can we make some cosntraints that
>>>> if satisfied by the pfnmap code, it could leverage the code from
>>>> vma_merge(). Perhaps by making explicit call to vma_merge()?
>>>> I get that implicit use moves too much responsibility to the mm
>>>> subsystem.
>>>>
>>>
>>> Hi Jarkko,
>>>
>>> Just want to understand more on the background:
>>>
>>> Are you seeing any real problem due to needing a lot of mmap()s to the
>>> same enclave, or it is just a problem that doesn't look nice and you
>>> want to resolve?
>>>
>>> I mean, this problem doesn't seem to be SGX-specific but a common one
>>> for VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random
>>> device drivers with mmap() support.  We will need a good justification
>>> if we want to make any core-mm change, if any, for this.
>>
>> It requires essentially replicating core mm in user space.
>>
>> It's a manageable problem but feels silly since logic in merging
>> is mostly 1:1. It's not a problem for me personally as I'm not
>> making any money from SGX (more so to Intel).
> 
> I.e. in the case when you want do a syscall shim. You fix it up by
> maintaining reflected version of the VMA database in the user space
> and remapping everything based on that for every possible mm-call.
> 
> I've implemented such feature in the past for Enarx so it is entirely
> possible.
> 

Just want to understand the problem/use case better:

I _think_ I got what the "syscall shim" is.  But can you elaborate why 
is this "syscall shim" related to the "kernel unable to merge contiguous 
VMAs for the same enclave"?  Assuming the kernel can actually merge 
enclave VMAs, how does it help on the "syscall shim"?

Re: VMA merging updateds?

[Jarkko Sakkinen]

On Thu Sep 26, 2024 at 3:38 AM EEST, Kai Huang wrote:
>
>
> On 26/09/2024 12:33 pm, Jarkko Sakkinen wrote:
> > On Thu Sep 26, 2024 at 3:07 AM EEST, Kai Huang wrote:
> >>
> >>
> >> On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> >>> On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> >>>>> On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> >>>>>> Hi
> >>>>>>
> >>>>>> I started to look into this old issue with mm subsystem and SGX, i.e.
> >>>>>> can we make SGX VMA's to merge together?
> >>>>>>
> >>>>>> This demonstrates the problem pretty well:
> >>>>>>
> >>>>>> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> >>>>>>
> >>>>>> It was result of brk() syscall being applied a few times.
> >>>>
> >>>> Briging some context here. This can be fixed in the run-time by book
> >>>> keeping the ranges and doing unmapping/mapping. I guess this goes
> >>>> beyond what mm should support?
> >>>>
> >>>> I thought to plain check this as it has been two years since my last
> >>>> query on topic (if we could improve either the driver or mm somehow).
> >>>
> >>> In the past I've substituted kernel's mm merge code with user space
> >>> replacement:
> >>>
> >>> https://github.com/enarx/mmledger/blob/main/src/lib.rs
> >>>
> >>> It's essentially a reimplementation of al stuff that goes into
> >>> mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> >>> which map over existing ranges were working correctly, i.e. was
> >>> the issue only concerning adjacent VMA's.
> >>>
> >>> What I'm looking here is that can we make some cosntraints that
> >>> if satisfied by the pfnmap code, it could leverage the code from
> >>> vma_merge(). Perhaps by making explicit call to vma_merge()?
> >>> I get that implicit use moves too much responsibility to the mm
> >>> subsystem.
> >>>
> >>
> >> Hi Jarkko,
> >>
> >> Just want to understand more on the background:
> >>
> >> Are you seeing any real problem due to needing a lot of mmap()s to the
> >> same enclave, or it is just a problem that doesn't look nice and you
> >> want to resolve?
> >>
> >> I mean, this problem doesn't seem to be SGX-specific but a common one
> >> for VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random
> >> device drivers with mmap() support.  We will need a good justification
> >> if we want to make any core-mm change, if any, for this.
> > 
> > It requires essentially replicating core mm in user space.
> > 
>
> Is there any real problem if we don't do this at all (neither in core mm 
> nor in userspace)?

7f8f00000000-7f8f00009000 r--s 00000000 00:05 84                         /dev/sgx_enclave
7f8f00009000-7f8f00034000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f00034000-7f8f000ff000 r-xs 00000000 00:05 84                         /dev/sgx_enclave
7f8f000ff000-7f8f00201000 ---p 00000000 00:00 0
7f8f00201000-7f8f003fc000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f003fc000-7f8f003fd000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f003fd000-7f8f00400000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f00400000-7f8f00643000 r--s 00000000 00:05 84                         /dev/sgx_enclave
7f8f00643000-7f8f0197b000 r-xs 00000000 00:05 84                         /dev/sgx_enclave
7f8f0197b000-7f8f01bab000 r--s 00000000 00:05 84                         /dev/sgx_enclave
7f8f01bab000-7f8f01fa7000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f01fa7000-7f8f08000000 ---p 00000000 00:00 0
7f8f08000000-7f8f08001000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08001000-7f8f08003000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08003000-7f8f08006000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08006000-7f8f0800b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0800b000-7f8f08014000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08014000-7f8f08025000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08025000-7f8f08046000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08046000-7f8f0804a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0804a000-7f8f0804b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0804b000-7f8f0804c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0804c000-7f8f0804d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0804d000-7f8f0804e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0804e000-7f8f0804f000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0804f000-7f8f08050000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08050000-7f8f08051000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08051000-7f8f08052000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08052000-7f8f08053000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08053000-7f8f08054000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08054000-7f8f08055000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08055000-7f8f08056000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08056000-7f8f08057000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08057000-7f8f08058000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08058000-7f8f08059000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08059000-7f8f0805a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0805a000-7f8f0805b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0805b000-7f8f0805c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0805c000-7f8f0805d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0805d000-7f8f0805e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0805e000-7f8f0805f000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0805f000-7f8f08060000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08060000-7f8f08062000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08062000-7f8f08063000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08063000-7f8f08064000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08064000-7f8f08065000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08065000-7f8f08066000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08066000-7f8f08067000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08067000-7f8f08068000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08068000-7f8f08069000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08069000-7f8f0806a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0806a000-7f8f0806b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0806b000-7f8f0806c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0806c000-7f8f0806d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0806d000-7f8f0806e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0806e000-7f8f0806f000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0806f000-7f8f08070000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08070000-7f8f08071000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08071000-7f8f08072000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08072000-7f8f08081000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08081000-7f8f08082000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08082000-7f8f08083000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08083000-7f8f08084000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08084000-7f8f08085000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08085000-7f8f08086000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08086000-7f8f08087000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08087000-7f8f08088000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08088000-7f8f08089000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08089000-7f8f0808a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0808a000-7f8f0808b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0808b000-7f8f0809a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0809a000-7f8f0809c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0809c000-7f8f0809d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0809d000-7f8f0809e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0809e000-7f8f080a0000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080a0000-7f8f080a3000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080a3000-7f8f080a9000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080a9000-7f8f080b5000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080b5000-7f8f080c0000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080c0000-7f8f080c6000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080c6000-7f8f080d1000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080d1000-7f8f080dd000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080dd000-7f8f080f4000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f080f4000-7f8f08121000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08121000-7f8f0814a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f0814a000-7f8f08162000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08162000-7f8f08177000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08177000-7f8f081a0000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f081a0000-7f8f081c1000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f081c1000-7f8f081d6000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f081d6000-7f8f081ff000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f081ff000-7f8f08228000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
7f8f08228000-7f8ffffff000 ---p 00000000 00:00 0
7f8ffffff000-7f9000000000 rw-s 00000000 00:05 84                         /dev/sgx_enclave

BR, Jarkko

Re: VMA merging updateds?

[Jarkko Sakkinen]

On Thu Sep 26, 2024 at 4:47 AM EEST, Jarkko Sakkinen wrote:
> On Thu Sep 26, 2024 at 3:38 AM EEST, Kai Huang wrote:
> >
> >
> > On 26/09/2024 12:33 pm, Jarkko Sakkinen wrote:
> > > On Thu Sep 26, 2024 at 3:07 AM EEST, Kai Huang wrote:
> > >>
> > >>
> > >> On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> > >>> On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> > >>>>> On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> > >>>>>> Hi
> > >>>>>>
> > >>>>>> I started to look into this old issue with mm subsystem and SGX, i.e.
> > >>>>>> can we make SGX VMA's to merge together?
> > >>>>>>
> > >>>>>> This demonstrates the problem pretty well:
> > >>>>>>
> > >>>>>> https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> > >>>>>>
> > >>>>>> It was result of brk() syscall being applied a few times.
> > >>>>
> > >>>> Briging some context here. This can be fixed in the run-time by book
> > >>>> keeping the ranges and doing unmapping/mapping. I guess this goes
> > >>>> beyond what mm should support?
> > >>>>
> > >>>> I thought to plain check this as it has been two years since my last
> > >>>> query on topic (if we could improve either the driver or mm somehow).
> > >>>
> > >>> In the past I've substituted kernel's mm merge code with user space
> > >>> replacement:
> > >>>
> > >>> https://github.com/enarx/mmledger/blob/main/src/lib.rs
> > >>>
> > >>> It's essentially a reimplementation of al stuff that goes into
> > >>> mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> > >>> which map over existing ranges were working correctly, i.e. was
> > >>> the issue only concerning adjacent VMA's.
> > >>>
> > >>> What I'm looking here is that can we make some cosntraints that
> > >>> if satisfied by the pfnmap code, it could leverage the code from
> > >>> vma_merge(). Perhaps by making explicit call to vma_merge()?
> > >>> I get that implicit use moves too much responsibility to the mm
> > >>> subsystem.
> > >>>
> > >>
> > >> Hi Jarkko,
> > >>
> > >> Just want to understand more on the background:
> > >>
> > >> Are you seeing any real problem due to needing a lot of mmap()s to the
> > >> same enclave, or it is just a problem that doesn't look nice and you
> > >> want to resolve?
> > >>
> > >> I mean, this problem doesn't seem to be SGX-specific but a common one
> > >> for VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random
> > >> device drivers with mmap() support.  We will need a good justification
> > >> if we want to make any core-mm change, if any, for this.
> > > 
> > > It requires essentially replicating core mm in user space.
> > > 
> >
> > Is there any real problem if we don't do this at all (neither in core mm 
> > nor in userspace)?
>
> 7f8f00000000-7f8f00009000 r--s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f00009000-7f8f00034000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f00034000-7f8f000ff000 r-xs 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f000ff000-7f8f00201000 ---p 00000000 00:00 0
> 7f8f00201000-7f8f003fc000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f003fc000-7f8f003fd000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f003fd000-7f8f00400000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f00400000-7f8f00643000 r--s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f00643000-7f8f0197b000 r-xs 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0197b000-7f8f01bab000 r--s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f01bab000-7f8f01fa7000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f01fa7000-7f8f08000000 ---p 00000000 00:00 0
> 7f8f08000000-7f8f08001000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08001000-7f8f08003000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08003000-7f8f08006000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08006000-7f8f0800b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0800b000-7f8f08014000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08014000-7f8f08025000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08025000-7f8f08046000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08046000-7f8f0804a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0804a000-7f8f0804b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0804b000-7f8f0804c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0804c000-7f8f0804d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0804d000-7f8f0804e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0804e000-7f8f0804f000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0804f000-7f8f08050000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08050000-7f8f08051000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08051000-7f8f08052000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08052000-7f8f08053000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08053000-7f8f08054000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08054000-7f8f08055000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08055000-7f8f08056000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08056000-7f8f08057000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08057000-7f8f08058000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08058000-7f8f08059000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08059000-7f8f0805a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0805a000-7f8f0805b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0805b000-7f8f0805c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0805c000-7f8f0805d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0805d000-7f8f0805e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0805e000-7f8f0805f000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0805f000-7f8f08060000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08060000-7f8f08062000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08062000-7f8f08063000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08063000-7f8f08064000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08064000-7f8f08065000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08065000-7f8f08066000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08066000-7f8f08067000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08067000-7f8f08068000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08068000-7f8f08069000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08069000-7f8f0806a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0806a000-7f8f0806b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0806b000-7f8f0806c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0806c000-7f8f0806d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0806d000-7f8f0806e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0806e000-7f8f0806f000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0806f000-7f8f08070000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08070000-7f8f08071000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08071000-7f8f08072000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08072000-7f8f08081000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08081000-7f8f08082000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08082000-7f8f08083000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08083000-7f8f08084000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08084000-7f8f08085000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08085000-7f8f08086000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08086000-7f8f08087000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08087000-7f8f08088000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08088000-7f8f08089000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08089000-7f8f0808a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0808a000-7f8f0808b000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0808b000-7f8f0809a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0809a000-7f8f0809c000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0809c000-7f8f0809d000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0809d000-7f8f0809e000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0809e000-7f8f080a0000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080a0000-7f8f080a3000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080a3000-7f8f080a9000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080a9000-7f8f080b5000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080b5000-7f8f080c0000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080c0000-7f8f080c6000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080c6000-7f8f080d1000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080d1000-7f8f080dd000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080dd000-7f8f080f4000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f080f4000-7f8f08121000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08121000-7f8f0814a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f0814a000-7f8f08162000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08162000-7f8f08177000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08177000-7f8f081a0000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f081a0000-7f8f081c1000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f081c1000-7f8f081d6000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f081d6000-7f8f081ff000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f081ff000-7f8f08228000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> 7f8f08228000-7f8ffffff000 ---p 00000000 00:00 0
> 7f8ffffff000-7f9000000000 rw-s 00000000 00:05 84                         /dev/sgx_enclave

Just giving ridiculous answer to a ridiculous question.

You clearly started commenting w/o reading the original thread.

BR, Jarkko

Re: VMA merging updateds?

[Jarkko Sakkinen]

On Thu Sep 26, 2024 at 4:48 AM EEST, Jarkko Sakkinen wrote:
> > 7f8f08121000-7f8f0814a000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f0814a000-7f8f08162000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f08162000-7f8f08177000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f08177000-7f8f081a0000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f081a0000-7f8f081c1000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f081c1000-7f8f081d6000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f081d6000-7f8f081ff000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f081ff000-7f8f08228000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
> > 7f8f08228000-7f8ffffff000 ---p 00000000 00:00 0
> > 7f8ffffff000-7f9000000000 rw-s 00000000 00:05 84                         /dev/sgx_enclave
>
> Just giving ridiculous answer to a ridiculous question.
>
> You clearly started commenting w/o reading the original thread.

It is two years since I did my own merging algorithm in user space [1].

If I recall correctly, since SGX driver does not have vm_close() by
mapping over in brk() shim you can fixup that. Obviously this needs [1]
so that you can check up from somewhere that you're doing adjacent map
with matching perms.

If nothing is done in user space, then VMA space can literally blow up
depending on the memory access pattern of the payload (in the case Enarx
it is an arbitrary program compiled to wasm, the enclave includes WASM
JIT as static payload).

I totally get if this absolute NO for core mm. Just thinking that is
SGX really the only existing location in kernel where you have:

1. pfnmap
2. bunch of regions
3. regions have varying permissions

And could there be some minimal weaker set of constraints that would
allow merges. Obviously it cannot be "any pfnmap" will go. If not,
**** it, I don't care, that's just life ;-) Stronger than pfnmap,
weaker than "struct page".

[1] https://github.com/enarx/mmledger/blob/main/src/lib.rs

BR, Jarkko

Re: VMA merging updateds?

[Lorenzo Stoakes]

Jumping into this thread mid-way to give my point of view.

On Thu, Sep 26, 2024 at 12:07:02PM GMT, Huang, Kai wrote:
>
>
> On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> > On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> > > > On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> > > > > Hi
> > > > >
> > > > > I started to look into this old issue with mm subsystem and SGX, i.e.
> > > > > can we make SGX VMA's to merge together?
> > > > >
> > > > > This demonstrates the problem pretty well:
> > > > >
> > > > > https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> > > > >
> > > > > It was result of brk() syscall being applied a few times.
> > >
> > > Briging some context here. This can be fixed in the run-time by book
> > > keeping the ranges and doing unmapping/mapping. I guess this goes
> > > beyond what mm should support?

The reason you're seeing this is that the ranges are VM_PFNMAP (as well as
VM_IO, VM_DONTEXPAND) , which are part of the VM_SPECIAL bitmask and
therefore explicitly not permitted to merge.

VMA merging occurs not when VMAs are merely adjacent to one another, but
when they are adjacent to one another AND share the same attributes.

Obviously for most VMA attributes this is critically important - you can't
have a read-only range merge with a r/w range, it is the VMAs that are
telling us this, equally so if different files/non-adjacent
offsets/etc. etc.

For these 'special' mappings each individual VMA may have distinct private
state and may be tracked/managed by the driver, and indeed I see that
vma->vm_private_data is used.

Also SGX utilises custom VMA handling operations for fault handling and
obviously does its own thing.

Because of this, there's just no way mm can know whether it's ok to merge
or not. Some of the attributes become in effect 'hidden'. You could map
anything in these ranges and track that with any kind of state.

So it's absolutely correct that we do not merge in these cases, as things
currently stand.

> > >
> > > I thought to plain check this as it has been two years since my last
> > > query on topic (if we could improve either the driver or mm somehow).
> >
> > In the past I've substituted kernel's mm merge code with user space
> > replacement:
> >
> > https://github.com/enarx/mmledger/blob/main/src/lib.rs
> >
> > It's essentially a reimplementation of al stuff that goes into
> > mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> > which map over existing ranges were working correctly, i.e. was
> > the issue only concerning adjacent VMA's.

mm/vma.c's vma_merge_existing_range() and vma_merge_new_range() now :) I
have completely rewritten this code from 6.12 onwards.

> >
> > What I'm looking here is that can we make some cosntraints that
> > if satisfied by the pfnmap code, it could leverage the code from
> > vma_merge(). Perhaps by making explicit call to vma_merge()?
> > I get that implicit use moves too much responsibility to the mm
> > subsystem.

Merging/splitting behaviour is an implementation detail and absolutely
cannot be exposed like this (and would be dangerous to do so).

However, I think the only sensible way we could proceed with something like
this is to add a new vma operation explicitly for pfnmap mappings which are
_otherwise mergeable_ like:

	vm_ops->may_merge_pfnmap(struct vm_area_struct *,
				 struct vm_area_struct *)

Which the driver could implement to check internal state matches between
the two VMAs.

It's nicely opt-in as we'd not merge if this were not set, and it defers
the decision as to 'hidden attributes' to the driver. Also we could ensure
all other merge characteristics were satisfied first to avoid any invalid
merging between VMAs - in fact this function would only be called if:

1. Both VMAs are VM_PFNMAP.
2. Both VMAs are otherwise compatible.
3. Both VMAs implement vm_ops->may_merge_pfnmap() (should be implied by 2,
   as vm_ops equality implies file equality).
4. vm_ops->may_merge_pfnmap(vma1, vma2) returns true.

At which point we could merge. Note that the existence of .close() could
cause issues here, though the existing merging rules should handle this
correctly.

I _suspect_ from a brief reading of the SGX stuff you only really need the
enclave value to be the same?

So it could be something like:

	static bool sgx_may_merge_pfnmap(struct vma_area_struct *vma1,
					 struct vma_area_struct *vma2)
	{
		/* Merge if enclaves match. */
		return vma1->vm_private_data == vma2->vm_private_data;
	}

Since you seem to be doing mappings explicitly based on virtual addresses
in these ranges?

I could look into RFC'ing something for you guys to test when I get a
chance?

> >
>
> Hi Jarkko,
>
> Just want to understand more on the background:
>
> Are you seeing any real problem due to needing a lot of mmap()s to the same
> enclave, or it is just a problem that doesn't look nice and you want to
> resolve?
>
> I mean, this problem doesn't seem to be SGX-specific but a common one for
> VMAs with VM_PFNMAP (any bit in VM_SPECIAL), e.g., from random device
> drivers with mmap() support.  We will need a good justification if we want
> to make any core-mm change, if any, for this.

I'm guessing the problem is a concern that a map limit will be hit or VMA
memory usage will be a problem? If not it may simply seem ugly...

Re: VMA merging updateds?

[Jarkko Sakkinen]

On Fri Sep 27, 2024 at 8:39 PM EEST, Lorenzo Stoakes wrote:
> Jumping into this thread mid-way to give my point of view.
>
> On Thu, Sep 26, 2024 at 12:07:02PM GMT, Huang, Kai wrote:
> >
> >
> > On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> > > On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> > > > > On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> > > > > > Hi
> > > > > >
> > > > > > I started to look into this old issue with mm subsystem and SGX, i.e.
> > > > > > can we make SGX VMA's to merge together?
> > > > > >
> > > > > > This demonstrates the problem pretty well:
> > > > > >
> > > > > > https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> > > > > >
> > > > > > It was result of brk() syscall being applied a few times.
> > > >
> > > > Briging some context here. This can be fixed in the run-time by book
> > > > keeping the ranges and doing unmapping/mapping. I guess this goes
> > > > beyond what mm should support?
>
> The reason you're seeing this is that the ranges are VM_PFNMAP (as well as
> VM_IO, VM_DONTEXPAND) , which are part of the VM_SPECIAL bitmask and
> therefore explicitly not permitted to merge.
>
> VMA merging occurs not when VMAs are merely adjacent to one another, but
> when they are adjacent to one another AND share the same attributes.
>
> Obviously for most VMA attributes this is critically important - you can't
> have a read-only range merge with a r/w range, it is the VMAs that are
> telling us this, equally so if different files/non-adjacent
> offsets/etc. etc.
>
> For these 'special' mappings each individual VMA may have distinct private
> state and may be tracked/managed by the driver, and indeed I see that
> vma->vm_private_data is used.
>
> Also SGX utilises custom VMA handling operations for fault handling and
> obviously does its own thing.
>
> Because of this, there's just no way mm can know whether it's ok to merge
> or not. Some of the attributes become in effect 'hidden'. You could map
> anything in these ranges and track that with any kind of state.
>
> So it's absolutely correct that we do not merge in these cases, as things
> currently stand.
>
> > > >
> > > > I thought to plain check this as it has been two years since my last
> > > > query on topic (if we could improve either the driver or mm somehow).
> > >
> > > In the past I've substituted kernel's mm merge code with user space
> > > replacement:
> > >
> > > https://github.com/enarx/mmledger/blob/main/src/lib.rs
> > >
> > > It's essentially a reimplementation of al stuff that goes into
> > > mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> > > which map over existing ranges were working correctly, i.e. was
> > > the issue only concerning adjacent VMA's.
>
> mm/vma.c's vma_merge_existing_range() and vma_merge_new_range() now :) I
> have completely rewritten this code from 6.12 onwards.
>
> > >
> > > What I'm looking here is that can we make some cosntraints that
> > > if satisfied by the pfnmap code, it could leverage the code from
> > > vma_merge(). Perhaps by making explicit call to vma_merge()?
> > > I get that implicit use moves too much responsibility to the mm
> > > subsystem.
>
> Merging/splitting behaviour is an implementation detail and absolutely
> cannot be exposed like this (and would be dangerous to do so).

I can believe that (not unexpected)

I've been doing some initial research for possibility to integrate Enarx
(LF governed confidential computing run-time) with 
https://github.com/rust-vmm/vm-memory, which is pretty solid mmap
abstraction, for which at least Paolo Bonzini is active contributor.

Enarx is multi-backend supporting also VM based private memory in
addition SGX. It has some a bit degraded parts at least SNP interfacing
code that need to be fixed. So I'm now just scoping what needs to be
done make it fresh again. It is just a leisure time sudoku just to
recover a rusty codebase whenever have some idle time.

>
> However, I think the only sensible way we could proceed with something like
> this is to add a new vma operation explicitly for pfnmap mappings which are
> _otherwise mergeable_ like:
>
> 	vm_ops->may_merge_pfnmap(struct vm_area_struct *,
> 				 struct vm_area_struct *)
>
> Which the driver could implement to check internal state matches between
> the two VMAs.
>
> It's nicely opt-in as we'd not merge if this were not set, and it defers
> the decision as to 'hidden attributes' to the driver. Also we could ensure
> all other merge characteristics were satisfied first to avoid any invalid
> merging between VMAs - in fact this function would only be called if:
>
> 1. Both VMAs are VM_PFNMAP.
> 2. Both VMAs are otherwise compatible.
> 3. Both VMAs implement vm_ops->may_merge_pfnmap() (should be implied by 2,
>    as vm_ops equality implies file equality).
> 4. vm_ops->may_merge_pfnmap(vma1, vma2) returns true.

OK strong maybe see what I wrote below.

>
> At which point we could merge. Note that the existence of .close() could
> cause issues here, though the existing merging rules should handle this
> correctly.
>
> I _suspect_ from a brief reading of the SGX stuff you only really need the
> enclave value to be the same?
>
> So it could be something like:
>
> 	static bool sgx_may_merge_pfnmap(struct vma_area_struct *vma1,
> 					 struct vma_area_struct *vma2)
> 	{
> 		/* Merge if enclaves match. */
> 		return vma1->vm_private_data == vma2->vm_private_data;
> 	}
>
> Since you seem to be doing mappings explicitly based on virtual addresses
> in these ranges?

Yep, so this is how it works now that I revisited the cod ea bit:

1. Enclave is a naturally aligned static address range of pow2 size.
   That is mapped first and everything happens inside. It is mapped
   with none permissions. With opcode ENCLS[ECREATE] this range is
   connected to a singe enclave.
2. With enclave or actually even SNP VM then internally has small
   engine and database that implements mmap() call and calls back
   to the outside world with a MAP_FIXED range. So actually in all
   cases my mmledger book keeper is in all cases needed. And it
   is needed to define what is expected from run-time.

   It must happen this way because of a distrust model. Without
   going too much details enclaves can set requirements for page
   permissions cryptographically and have structure (enclave
   page cache map) for this. Thus enclave (or SNP VM) leads,
   run-time follows that by delegating (fixed) mmap() calls,

   In practice the enclave will #GP if memory access is made
   with unexpected permission, even if page tables would allow
   it. In theory you could even just plain rwx map the range
   and base only on EPCM but in practice matching the page
   tables is useful and adds a bit defense in depth.

With SNP things obviously work correctly because it is just a
fancy VM with weird extra shenanigans.

With SGX things are fixed up by allocating a static heap and
never translating brk/sbrk to any syscall. Then for regular
mmaps() it just I guess hopes that user space software uses
it wisely :-)

>
> I could look into RFC'ing something for you guys to test when I get a
> chance?

So I got some ideas I could try out after reading your proposal and
restudying work I did over two years ago :-) I think it makes sense
for me to try to see how much I can improve first.

With pfnmap what did happen when you have a fixed map let's say
from A to B and you map from A to C where C > B? Cannot recall does
this success. Right, and as said vm_close() does not exist.

That could be used based mmledger to overwrite the old VMA. Or I
could do pro-active unmapping.

mmledger could tell "what changed" i.e.  provide syscall parameters for
0-2 munmaps and mmap(MAP_FIXED)  and ask run-time to execute those
instead of just mmap(MAP_FIXED)

I need to improve that crate anyway because I realized that you need
something like that to any trusted execution environment because they
reason what they want from memory :-)

So I get on this first and come back to this lore thread later when
I have some more experience on the topic.

> I'm guessing the problem is a concern that a map limit will be hit or VMA
> memory usage will be a problem? If not it may simply seem ugly...

Yea, so Enarx can take any software compiled to static wasm program and
put into a protected environment. It has a syscall shim and wasm
run-time and loader when it launches. If the application uses mm
syscalls like a good citizen it should not blow up VMA list but
that ofc is based on good faith ;-)

[1] Only for completeness:
    https://github.com/enarx/enarx/blob/main/crates/shim-sgx/src/heap.rs

BR, Jarkko

Re: VMA merging updateds?

[Lorenzo Stoakes]

On Mon, Sep 30, 2024 at 01:36:06AM GMT, Jarkko Sakkinen wrote:
> On Fri Sep 27, 2024 at 8:39 PM EEST, Lorenzo Stoakes wrote:
> > Jumping into this thread mid-way to give my point of view.
> >
> > On Thu, Sep 26, 2024 at 12:07:02PM GMT, Huang, Kai wrote:
> > >
> > >
> > > On 23/09/2024 7:48 pm, Jarkko Sakkinen wrote:
> > > > On Sun Sep 22, 2024 at 7:57 PM EEST, Jarkko Sakkinen wrote:
> > > > > > On Sun Sep 22, 2024 at 7:27 PM EEST, Jarkko Sakkinen wrote:
> > > > > > > Hi
> > > > > > >
> > > > > > > I started to look into this old issue with mm subsystem and SGX, i.e.
> > > > > > > can we make SGX VMA's to merge together?
> > > > > > >
> > > > > > > This demonstrates the problem pretty well:
> > > > > > >
> > > > > > > https://lore.kernel.org/linux-sgx/884c7ea454cf2eb0ba2e95f7c25bd42018824f97.camel@kernel.org/
> > > > > > >
> > > > > > > It was result of brk() syscall being applied a few times.
> > > > >
> > > > > Briging some context here. This can be fixed in the run-time by book
> > > > > keeping the ranges and doing unmapping/mapping. I guess this goes
> > > > > beyond what mm should support?
> >
> > The reason you're seeing this is that the ranges are VM_PFNMAP (as well as
> > VM_IO, VM_DONTEXPAND) , which are part of the VM_SPECIAL bitmask and
> > therefore explicitly not permitted to merge.
> >
> > VMA merging occurs not when VMAs are merely adjacent to one another, but
> > when they are adjacent to one another AND share the same attributes.
> >
> > Obviously for most VMA attributes this is critically important - you can't
> > have a read-only range merge with a r/w range, it is the VMAs that are
> > telling us this, equally so if different files/non-adjacent
> > offsets/etc. etc.
> >
> > For these 'special' mappings each individual VMA may have distinct private
> > state and may be tracked/managed by the driver, and indeed I see that
> > vma->vm_private_data is used.
> >
> > Also SGX utilises custom VMA handling operations for fault handling and
> > obviously does its own thing.
> >
> > Because of this, there's just no way mm can know whether it's ok to merge
> > or not. Some of the attributes become in effect 'hidden'. You could map
> > anything in these ranges and track that with any kind of state.
> >
> > So it's absolutely correct that we do not merge in these cases, as things
> > currently stand.
> >
> > > > >
> > > > > I thought to plain check this as it has been two years since my last
> > > > > query on topic (if we could improve either the driver or mm somehow).
> > > >
> > > > In the past I've substituted kernel's mm merge code with user space
> > > > replacement:
> > > >
> > > > https://github.com/enarx/mmledger/blob/main/src/lib.rs
> > > >
> > > > It's essentially a reimplementation of al stuff that goes into
> > > > mm/mmap.c's vma_merge(). I cannot recall anymore whether merges
> > > > which map over existing ranges were working correctly, i.e. was
> > > > the issue only concerning adjacent VMA's.
> >
> > mm/vma.c's vma_merge_existing_range() and vma_merge_new_range() now :) I
> > have completely rewritten this code from 6.12 onwards.
> >
> > > >
> > > > What I'm looking here is that can we make some cosntraints that
> > > > if satisfied by the pfnmap code, it could leverage the code from
> > > > vma_merge(). Perhaps by making explicit call to vma_merge()?
> > > > I get that implicit use moves too much responsibility to the mm
> > > > subsystem.
> >
> > Merging/splitting behaviour is an implementation detail and absolutely
> > cannot be exposed like this (and would be dangerous to do so).
>
> I can believe that (not unexpected)
>
> I've been doing some initial research for possibility to integrate Enarx
> (LF governed confidential computing run-time) with
> https://github.com/rust-vmm/vm-memory, which is pretty solid mmap
> abstraction, for which at least Paolo Bonzini is active contributor.
>
> Enarx is multi-backend supporting also VM based private memory in
> addition SGX. It has some a bit degraded parts at least SNP interfacing
> code that need to be fixed. So I'm now just scoping what needs to be
> done make it fresh again. It is just a leisure time sudoku just to
> recover a rusty codebase whenever have some idle time.
>
> >
> > However, I think the only sensible way we could proceed with something like
> > this is to add a new vma operation explicitly for pfnmap mappings which are
> > _otherwise mergeable_ like:
> >
> > 	vm_ops->may_merge_pfnmap(struct vm_area_struct *,
> > 				 struct vm_area_struct *)
> >
> > Which the driver could implement to check internal state matches between
> > the two VMAs.
> >
> > It's nicely opt-in as we'd not merge if this were not set, and it defers
> > the decision as to 'hidden attributes' to the driver. Also we could ensure
> > all other merge characteristics were satisfied first to avoid any invalid
> > merging between VMAs - in fact this function would only be called if:
> >
> > 1. Both VMAs are VM_PFNMAP.
> > 2. Both VMAs are otherwise compatible.
> > 3. Both VMAs implement vm_ops->may_merge_pfnmap() (should be implied by 2,
> >    as vm_ops equality implies file equality).
> > 4. vm_ops->may_merge_pfnmap(vma1, vma2) returns true.
>
> OK strong maybe see what I wrote below.
>
> >
> > At which point we could merge. Note that the existence of .close() could
> > cause issues here, though the existing merging rules should handle this
> > correctly.
> >
> > I _suspect_ from a brief reading of the SGX stuff you only really need the
> > enclave value to be the same?
> >
> > So it could be something like:
> >
> > 	static bool sgx_may_merge_pfnmap(struct vma_area_struct *vma1,
> > 					 struct vma_area_struct *vma2)
> > 	{
> > 		/* Merge if enclaves match. */
> > 		return vma1->vm_private_data == vma2->vm_private_data;
> > 	}
> >
> > Since you seem to be doing mappings explicitly based on virtual addresses
> > in these ranges?
>
> Yep, so this is how it works now that I revisited the cod ea bit:
>
> 1. Enclave is a naturally aligned static address range of pow2 size.
>    That is mapped first and everything happens inside. It is mapped
>    with none permissions. With opcode ENCLS[ECREATE] this range is
>    connected to a singe enclave.
> 2. With enclave or actually even SNP VM then internally has small
>    engine and database that implements mmap() call and calls back
>    to the outside world with a MAP_FIXED range. So actually in all
>    cases my mmledger book keeper is in all cases needed. And it
>    is needed to define what is expected from run-time.
>
>    It must happen this way because of a distrust model. Without
>    going too much details enclaves can set requirements for page
>    permissions cryptographically and have structure (enclave
>    page cache map) for this. Thus enclave (or SNP VM) leads,
>    run-time follows that by delegating (fixed) mmap() calls,
>
>    In practice the enclave will #GP if memory access is made
>    with unexpected permission, even if page tables would allow
>    it. In theory you could even just plain rwx map the range
>    and base only on EPCM but in practice matching the page
>    tables is useful and adds a bit defense in depth.
>
> With SNP things obviously work correctly because it is just a
> fancy VM with weird extra shenanigans.
>
> With SGX things are fixed up by allocating a static heap and
> never translating brk/sbrk to any syscall. Then for regular
> mmaps() it just I guess hopes that user space software uses
> it wisely :-)
>
> >
> > I could look into RFC'ing something for you guys to test when I get a
> > chance?
>
> So I got some ideas I could try out after reading your proposal and
> restudying work I did over two years ago :-) I think it makes sense
> for me to try to see how much I can improve first.
>
> With pfnmap what did happen when you have a fixed map let's say
> from A to B and you map from A to C where C > B? Cannot recall does
> this success. Right, and as said vm_close() does not exist.
>
> That could be used based mmledger to overwrite the old VMA. Or I
> could do pro-active unmapping.
>
> mmledger could tell "what changed" i.e.  provide syscall parameters for
> 0-2 munmaps and mmap(MAP_FIXED)  and ask run-time to execute those
> instead of just mmap(MAP_FIXED)
>
> I need to improve that crate anyway because I realized that you need
> something like that to any trusted execution environment because they
> reason what they want from memory :-)
>
> So I get on this first and come back to this lore thread later when
> I have some more experience on the topic.

OK cool let's revisit this when you've done that.

I think if you did want assistance from VMA merging what I delineated above
is the only sensible way forward, afaict.

>
> > I'm guessing the problem is a concern that a map limit will be hit or VMA
> > memory usage will be a problem? If not it may simply seem ugly...
>
> Yea, so Enarx can take any software compiled to static wasm program and
> put into a protected environment. It has a syscall shim and wasm
> run-time and loader when it launches. If the application uses mm
> syscalls like a good citizen it should not blow up VMA list but
> that ofc is based on good faith ;-)
>
> [1] Only for completeness:
>     https://github.com/enarx/enarx/blob/main/crates/shim-sgx/src/heap.rs
>
> BR, Jarkko

Re: VMA merging updateds?

[Jarkko Sakkinen]

On Mon, 2024-09-30 at 09:05 +0100, Lorenzo Stoakes wrote:
> > So I get on this first and come back to this lore thread later when
> > I have some more experience on the topic.
> 
> OK cool let's revisit this when you've done that.
> 
> I think if you did want assistance from VMA merging what I delineated
> above
> is the only sensible way forward, afaict.

So I did run some tests and since a TEE (trusted execution environment)
needs in all cases idea what it expects kernel to do all TEE's need a
memory manager. Enarx is multi-arch, and in the case of SGX there is
permissions bits per page owned by the enclave, on VM based confidential
computing (such as AMD SEN-SNP) there's some other weird shenanigans but
across all arhitectures there exists a constraint, which causes the
need. I.e. whatever happens in mm in kernel side cannot simplify the
implementation.

The current implementation of Enarx does not merge but I did some
experimental change doing "munmap and mmap" dance and I don't see
any mentionable change in the benchmarks.

So I came from the wrong early assumptions and made wrong conclusions
in the first place, i.e. somehow this mmledger crate could rendered
out. Had not looked at the code base either for 1.5 years but should
have started what I've now done first in order to recall all the
consraints.

BR, Jarkko