From: Nathan Lynch <nathanl@linux•ibm.com>
To: Michael Ellerman <mpe@ellerman•id.au>,
Alexey Kardashevskiy <aik@ozlabs•ru>,
linuxppc-dev@lists•ozlabs.org
Cc: tyreld@linux•ibm.com, brking@linux•ibm.com, ajd@linux•ibm.com,
aneesh.kumar@linux•ibm.com
Subject: Re: [PATCH 6/6] powerpc/rtas: constrain user region allocation to RMA
Date: Thu, 21 Jan 2021 09:27:18 -0600 [thread overview]
Message-ID: <87pn1ywbs9.fsf@linux.ibm.com> (raw)
In-Reply-To: <87ft2vrew6.fsf@mpe.ellerman.id.au>
Michael Ellerman <mpe@ellerman•id.au> writes:
> Nathan Lynch <nathanl@linux•ibm.com> writes:
>> Alexey Kardashevskiy <aik@ozlabs•ru> writes:
>>> On 16/01/2021 02:38, Nathan Lynch wrote:
>>>> Alexey Kardashevskiy <aik@ozlabs•ru> writes:
>>>>> On 15/01/2021 09:00, Nathan Lynch wrote:
>>>>>> Memory locations passed as arguments from the OS to RTAS usually need
>>>>>> to be addressable in 32-bit mode and must reside in the Real Mode
>>>>>> Area. On PAPR guests, the RMA starts at logical address 0 and is the
>>>>>> first logical memory block reported in the LPAR’s device tree.
>>>>>>
>>>>>> On powerpc targets with RTAS, Linux makes available to user space a
>>>>>> region of memory suitable for arguments to be passed to RTAS via
>>>>>> sys_rtas(). This region (rtas_rmo_buf) is allocated via the memblock
>>>>>> API during boot in order to ensure that it satisfies the requirements
>>>>>> described above.
>>>>>>
>>>>>> With radix MMU, the upper limit supplied to the memblock allocation
>>>>>> can exceed the bounds of the first logical memory block, since
>>>>>> ppc64_rma_size is ULONG_MAX and RTAS_INSTANTIATE_MAX is 1GB. (512MB is
>>>>>> a common size of the first memory block according to a small sample of
>>>>>> LPARs I have checked.) This leads to failures when user space invokes
>>>>>> an RTAS function that uses a work area, such as
>>>>>> ibm,configure-connector.
>>>>>>
>>>>>> Alter the determination of the upper limit for rtas_rmo_buf's
>>>>>> allocation to consult the device tree directly, ensuring placement
>>>>>> within the RMA regardless of the MMU in use.
>>>>>
>>>>> Can we tie this with RTAS (which also needs to be in RMA) and simply add
>>>>> extra 64K in prom_instantiate_rtas() and advertise this address
>>>>> (ALIGH_UP(rtas-base + rtas-size, PAGE_SIZE)) to the user space? We do
>>>>> not need this RMO area before that point.
>>>>
>>>> Can you explain more about what advantage that would bring? I'm not
>>>> seeing it. It's a more significant change than what I've written
>>>> here.
>>>
>>>
>>> We already allocate space for RTAS and (like RMO) it needs to be in RMA,
>>> and RMO is useless without RTAS. We can reuse RTAS allocation code for
>>> RMO like this:
>>
>> When you say RMO I assume you are referring to rtas_rmo_buf? (I don't
>> think it is well-named.)
> ...
>
> RMO (Real mode offset) is the old term we used to use to refer to what
> is now called the RMA (Real mode area). There are still many references
> to RMO in Linux, but they almost certainly all refer to what we now call
> the RMA.
Yes... but I think in this discussion Alexey was using RMO to stand in
for rtas_rmo_buf, which was what I was trying to clarify.
>>> May be store in the FDT as "linux,rmo-base" next to "linux,rtas-base",
>>> for clarity, as sharing symbols between prom and main kernel is a bit
>>> tricky.
>>>
>>> The benefit is that we do not do the same thing (== find 64K in RMA)
>>> in 2 different ways and if the RMO allocated my way is broken - we'll
>>> know it much sooner as RTAS itself will break too.
>>
>> Implementation details aside... I'll grant that combining the
>> allocations into one in prom_init reduces some duplication in the sense
>> that both are subject to the same constraints (mostly - the RTAS data
>> area must not cross a 256MB boundary, while the user region may). But
>> they really are distinct concerns. The RTAS private data area is
>> specified in the platform architecture, the OS is obligated to allocate
>> it and pass it to instantiate-rtas, etc etc. However the user region
>> (rtas_rmo_buf) is purely a Linux construct which is there to support
>> sys_rtas.
>>
>> Now, there are multiple sites in the kernel proper that must allocate
>> memory suitable for passing to RTAS. Obviously there is value in
>> consolidating the logic for that purpose in one place, so I'll work on
>> adding that in v2. OK?
>
> I don't think we want to move any allocations into prom_init.c unless we
> have to.
>
> It's best thought of as a trampoline, that runs before the kernel
> proper, to transition from live OF to a flat DT environment. One thing
> that must be done as part of that is instantiating RTAS, because it's
> basically a runtime copy of the live OF. But any other allocs are for
> Linux to handle later, IMHO.
Agreed.
next prev parent reply other threads:[~2021-01-21 15:29 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-14 21:59 [PATCH 0/6] powerpc/rtas: miscellaneous cleanups, user region allocation Nathan Lynch
2021-01-14 21:59 ` [PATCH 1/6] powerpc/rtas: improve ppc_rtas_rmo_buf_show documentation Nathan Lynch
2021-01-15 4:38 ` Alexey Kardashevskiy
2021-01-15 5:50 ` Andrew Donnellan
2021-01-14 22:00 ` [PATCH 2/6] powerpc/rtas-proc: remove unused RMO_READ_BUF_MAX Nathan Lynch
2021-01-15 4:38 ` Alexey Kardashevskiy
2021-01-15 5:52 ` Andrew Donnellan
2021-01-14 22:00 ` [PATCH 3/6] powerpc/rtas: remove ibm_suspend_me_token Nathan Lynch
2021-01-15 4:38 ` Alexey Kardashevskiy
2021-01-15 5:52 ` Andrew Donnellan
2021-01-14 22:00 ` [PATCH 4/6] powerpc/rtas: move syscall filter setup into separate function Nathan Lynch
2021-01-15 4:39 ` Alexey Kardashevskiy
2021-01-15 16:04 ` Nathan Lynch
2021-01-15 5:49 ` Andrew Donnellan
2021-01-14 22:00 ` [PATCH 5/6] powerpc/rtas: rename RTAS_RMOBUF_MAX to RTAS_USER_REGION_SIZE Nathan Lynch
2021-01-15 4:38 ` Alexey Kardashevskiy
2021-01-15 15:56 ` Nathan Lynch
2021-01-18 4:15 ` Alexey Kardashevskiy
2021-01-20 1:17 ` Nathan Lynch
2021-01-20 5:05 ` Alexey Kardashevskiy
2021-01-21 15:17 ` Nathan Lynch
2021-01-15 6:10 ` Andrew Donnellan
2021-01-15 12:04 ` kernel test robot
2021-01-14 22:00 ` [PATCH 6/6] powerpc/rtas: constrain user region allocation to RMA Nathan Lynch
2021-01-15 4:38 ` Alexey Kardashevskiy
2021-01-15 15:38 ` Nathan Lynch
2021-01-18 4:12 ` Alexey Kardashevskiy
2021-01-20 0:39 ` Nathan Lynch
2021-01-20 4:49 ` Alexey Kardashevskiy
2021-01-20 12:06 ` Michael Ellerman
2021-01-21 15:27 ` Nathan Lynch [this message]
2021-01-23 1:54 ` Alexey Kardashevskiy
2021-01-19 9:00 ` Michael Ellerman
2021-01-19 21:00 ` Nathan Lynch
2021-01-20 12:13 ` Michael Ellerman
2021-01-21 0:26 ` Nathan Lynch
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pn1ywbs9.fsf@linux.ibm.com \
--to=nathanl@linux$(echo .)ibm.com \
--cc=aik@ozlabs$(echo .)ru \
--cc=ajd@linux$(echo .)ibm.com \
--cc=aneesh.kumar@linux$(echo .)ibm.com \
--cc=brking@linux$(echo .)ibm.com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=mpe@ellerman$(echo .)id.au \
--cc=tyreld@linux$(echo .)ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox