From: Hector Martin <marcan@marcan.st>
Apparently nobody can figure out where the old logic came from, but it
seems like it has never been actually used on any supported firmware to
this day. OSLog buffers were apparently never requested.
But starting with 13.3, we actually need this implemented properly for
MTP (and later AOP) to work, so let's actually do that.
Signed-off-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
drivers/soc/apple/rtkit-internal.h | 1 +
drivers/soc/apple/rtkit.c | 55 +++++++++++++++++++++++---------------
2 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/drivers/soc/apple/rtkit-internal.h b/drivers/soc/apple/rtkit-internal.h
index 27c9fa745fd5..b8d5244678f0 100644
--- a/drivers/soc/apple/rtkit-internal.h
+++ b/drivers/soc/apple/rtkit-internal.h
@@ -44,6 +44,7 @@ struct apple_rtkit {
struct apple_rtkit_shmem ioreport_buffer;
struct apple_rtkit_shmem crashlog_buffer;
+ struct apple_rtkit_shmem oslog_buffer;
struct apple_rtkit_shmem syslog_buffer;
char *syslog_msg_buffer;
diff --git a/drivers/soc/apple/rtkit.c b/drivers/soc/apple/rtkit.c
index be0d08861168..35734ae8c9ce 100644
--- a/drivers/soc/apple/rtkit.c
+++ b/drivers/soc/apple/rtkit.c
@@ -67,8 +67,9 @@ enum {
#define APPLE_RTKIT_SYSLOG_MSG_SIZE GENMASK_ULL(31, 24)
#define APPLE_RTKIT_OSLOG_TYPE GENMASK_ULL(63, 56)
-#define APPLE_RTKIT_OSLOG_INIT 1
-#define APPLE_RTKIT_OSLOG_ACK 3
+#define APPLE_RTKIT_OSLOG_BUFFER_REQUEST 1
+#define APPLE_RTKIT_OSLOG_SIZE GENMASK_ULL(55, 36)
+#define APPLE_RTKIT_OSLOG_IOVA GENMASK_ULL(35, 0)
#define APPLE_RTKIT_MIN_SUPPORTED_VERSION 11
#define APPLE_RTKIT_MAX_SUPPORTED_VERSION 12
@@ -259,15 +260,20 @@ static int apple_rtkit_common_rx_get_buffer(struct apple_rtkit *rtk,
struct apple_rtkit_shmem *buffer,
u8 ep, u64 msg)
{
- size_t n_4kpages = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_SIZE, msg);
u64 reply;
int err;
+ if (ep == APPLE_RTKIT_EP_OSLOG) {
+ buffer->size = FIELD_GET(APPLE_RTKIT_OSLOG_SIZE, msg);
+ buffer->iova = FIELD_GET(APPLE_RTKIT_OSLOG_IOVA, msg) << 12;
+ } else {
+ buffer->size = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_SIZE, msg) << 12;
+ buffer->iova = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_IOVA, msg);
+ }
+
buffer->buffer = NULL;
buffer->iomem = NULL;
buffer->is_mapped = false;
- buffer->iova = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_IOVA, msg);
- buffer->size = n_4kpages << 12;
dev_dbg(rtk->dev, "RTKit: buffer request for 0x%zx bytes at %pad\n",
buffer->size, &buffer->iova);
@@ -292,11 +298,21 @@ static int apple_rtkit_common_rx_get_buffer(struct apple_rtkit *rtk,
}
if (!buffer->is_mapped) {
- reply = FIELD_PREP(APPLE_RTKIT_SYSLOG_TYPE,
- APPLE_RTKIT_BUFFER_REQUEST);
- reply |= FIELD_PREP(APPLE_RTKIT_BUFFER_REQUEST_SIZE, n_4kpages);
- reply |= FIELD_PREP(APPLE_RTKIT_BUFFER_REQUEST_IOVA,
- buffer->iova);
+ /* oslog uses different fields */
+ if (ep == APPLE_RTKIT_EP_OSLOG) {
+ reply = FIELD_PREP(APPLE_RTKIT_OSLOG_TYPE,
+ APPLE_RTKIT_OSLOG_BUFFER_REQUEST);
+ reply |= FIELD_PREP(APPLE_RTKIT_OSLOG_SIZE, buffer->size);
+ reply |= FIELD_PREP(APPLE_RTKIT_OSLOG_IOVA,
+ buffer->iova >> 12);
+ } else {
+ reply = FIELD_PREP(APPLE_RTKIT_SYSLOG_TYPE,
+ APPLE_RTKIT_BUFFER_REQUEST);
+ reply |= FIELD_PREP(APPLE_RTKIT_BUFFER_REQUEST_SIZE,
+ buffer->size >> 12);
+ reply |= FIELD_PREP(APPLE_RTKIT_BUFFER_REQUEST_IOVA,
+ buffer->iova);
+ }
apple_rtkit_send_message(rtk, ep, reply, NULL, false);
}
@@ -494,25 +510,18 @@ static void apple_rtkit_syslog_rx(struct apple_rtkit *rtk, u64 msg)
}
}
-static void apple_rtkit_oslog_rx_init(struct apple_rtkit *rtk, u64 msg)
-{
- u64 ack;
-
- dev_dbg(rtk->dev, "RTKit: oslog init: msg: 0x%llx\n", msg);
- ack = FIELD_PREP(APPLE_RTKIT_OSLOG_TYPE, APPLE_RTKIT_OSLOG_ACK);
- apple_rtkit_send_message(rtk, APPLE_RTKIT_EP_OSLOG, ack, NULL, false);
-}
-
static void apple_rtkit_oslog_rx(struct apple_rtkit *rtk, u64 msg)
{
u8 type = FIELD_GET(APPLE_RTKIT_OSLOG_TYPE, msg);
switch (type) {
- case APPLE_RTKIT_OSLOG_INIT:
- apple_rtkit_oslog_rx_init(rtk, msg);
+ case APPLE_RTKIT_OSLOG_BUFFER_REQUEST:
+ apple_rtkit_common_rx_get_buffer(rtk, &rtk->oslog_buffer,
+ APPLE_RTKIT_EP_OSLOG, msg);
break;
default:
- dev_warn(rtk->dev, "RTKit: Unknown oslog message: %llx\n", msg);
+ dev_warn(rtk->dev, "RTKit: Unknown oslog message: %llx\n",
+ msg);
}
}
@@ -729,6 +738,7 @@ int apple_rtkit_reinit(struct apple_rtkit *rtk)
apple_rtkit_free_buffer(rtk, &rtk->ioreport_buffer);
apple_rtkit_free_buffer(rtk, &rtk->crashlog_buffer);
+ apple_rtkit_free_buffer(rtk, &rtk->oslog_buffer);
apple_rtkit_free_buffer(rtk, &rtk->syslog_buffer);
kfree(rtk->syslog_msg_buffer);
@@ -916,6 +926,7 @@ void apple_rtkit_free(struct apple_rtkit *rtk)
apple_rtkit_free_buffer(rtk, &rtk->ioreport_buffer);
apple_rtkit_free_buffer(rtk, &rtk->crashlog_buffer);
+ apple_rtkit_free_buffer(rtk, &rtk->oslog_buffer);
apple_rtkit_free_buffer(rtk, &rtk->syslog_buffer);
kfree(rtk->syslog_msg_buffer);
--
2.34.1> + if (ep == APPLE_RTKIT_EP_OSLOG) {
> + buffer->size = FIELD_GET(APPLE_RTKIT_OSLOG_SIZE, msg);
> + buffer->iova = FIELD_GET(APPLE_RTKIT_OSLOG_IOVA, msg) << 12;
> + } else {
> + buffer->size = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_SIZE, msg) << 12;
> + buffer->iova = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_IOVA, msg);
> + }
The shifts are suspiciously asymmetric. Are we really sure this is
correct? My guess is that both size & iova for both oslog & buffer need
to be page-aligned, so all 4 lines should be shifted, and the bit
offsets should be adjusted in turn, and the lower 12-bits in oslog_size
and buffer_iova are reserved. But that's just a guess.
Anyway if this logic is really what we want it deserves a comment
because it looks like a typo.
(Likewise later in the patch)
On Mon, Feb 24, 2025, at 19:09, Alyssa Rosenzweig wrote:
>> + if (ep == APPLE_RTKIT_EP_OSLOG) {
>> + buffer->size = FIELD_GET(APPLE_RTKIT_OSLOG_SIZE, msg);
>> + buffer->iova = FIELD_GET(APPLE_RTKIT_OSLOG_IOVA, msg) << 12;
>> + } else {
>> + buffer->size = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_SIZE, msg) << 12;
>> + buffer->iova = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_IOVA, msg);
>> + }
>
> The shifts are suspiciously asymmetric. Are we really sure this is
> correct? My guess is that both size & iova for both oslog & buffer need
> to be page-aligned, so all 4 lines should be shifted, and the bit
> offsets should be adjusted in turn, and the lower 12-bits in oslog_size
> and buffer_iova are reserved. But that's just a guess.
>
> Anyway if this logic is really what we want it deserves a comment
> because it looks like a typo.
That guess can't be true for syslog since there's no change in behavior here
and the syslog endpoint has been working fine so far. This common code is
also used for other endpoints that request buffers and there haven't been
any issues there either. The size is just passed in "number of 4k chunks"
and the IOVA needs no additional fixups.
The entire reason for this commit is because this common logic just didn't
work for oslog. Our u-boot fork uses the same logic as used here [1]. We're stealing
a chunk of MTP's SRAM to make hand-off to Linux easier there. If either size or
IOVA was off by a factor 0x4000 this would've never worked in the first
place.
I'll add a comment that this is really what we want even though it looks odd.
Sven
[1] https://github.com/AsahiLinux/u-boot/blob/582f851413c3fd1fcd60d701ed54fff2e840b9cf/arch/arm/mach-apple/rtkit.c#L144
> >> + if (ep == APPLE_RTKIT_EP_OSLOG) {
> >> + buffer->size = FIELD_GET(APPLE_RTKIT_OSLOG_SIZE, msg);
> >> + buffer->iova = FIELD_GET(APPLE_RTKIT_OSLOG_IOVA, msg) << 12;
> >> + } else {
> >> + buffer->size = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_SIZE, msg) << 12;
> >> + buffer->iova = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_IOVA, msg);
> >> + }
> >
> > The shifts are suspiciously asymmetric. Are we really sure this is
> > correct? My guess is that both size & iova for both oslog & buffer need
> > to be page-aligned, so all 4 lines should be shifted, and the bit
> > offsets should be adjusted in turn, and the lower 12-bits in oslog_size
> > and buffer_iova are reserved. But that's just a guess.
> >
> > Anyway if this logic is really what we want it deserves a comment
> > because it looks like a typo.
>
> That guess can't be true for syslog since there's no change in behavior here
> and the syslog endpoint has been working fine so far. This common code is
> also used for other endpoints that request buffers and there haven't been
> any issues there either. The size is just passed in "number of 4k chunks"
> and the IOVA needs no additional fixups.
>
>
> The entire reason for this commit is because this common logic just didn't
> work for oslog. Our u-boot fork uses the same logic as used here [1]. We're stealing
> a chunk of MTP's SRAM to make hand-off to Linux easier there. If either size or
> IOVA was off by a factor 0x4000 this would've never worked in the first
> place.
I'm not suggesting things are off by a factor of 4k. Rather I'm
questioning what the behaviour is when we're not 4k aligned. (I.e.
the syslog or oslog buffer does not both start and end at 4k
boundaries.)
If we're aligned, all our bottom bits are 0, and hypothetically we're
putting 0 into "reserved-must be zero" bits.
I guess it's inconsequential if everything is 4k aligned in practice.
But .. is it? I don't know.
On Wed, Feb 26, 2025, at 18:05, Alyssa Rosenzweig wrote:
>> >> + if (ep == APPLE_RTKIT_EP_OSLOG) {
>> >> + buffer->size = FIELD_GET(APPLE_RTKIT_OSLOG_SIZE, msg);
>> >> + buffer->iova = FIELD_GET(APPLE_RTKIT_OSLOG_IOVA, msg) << 12;
>> >> + } else {
>> >> + buffer->size = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_SIZE, msg) << 12;
>> >> + buffer->iova = FIELD_GET(APPLE_RTKIT_BUFFER_REQUEST_IOVA, msg);
>> >> + }
>> >
>> > The shifts are suspiciously asymmetric. Are we really sure this is
>> > correct? My guess is that both size & iova for both oslog & buffer need
>> > to be page-aligned, so all 4 lines should be shifted, and the bit
>> > offsets should be adjusted in turn, and the lower 12-bits in oslog_size
>> > and buffer_iova are reserved. But that's just a guess.
>> >
>> > Anyway if this logic is really what we want it deserves a comment
>> > because it looks like a typo.
>>
>> That guess can't be true for syslog since there's no change in behavior here
>> and the syslog endpoint has been working fine so far. This common code is
>> also used for other endpoints that request buffers and there haven't been
>> any issues there either. The size is just passed in "number of 4k chunks"
>> and the IOVA needs no additional fixups.
>>
>>
>> The entire reason for this commit is because this common logic just didn't
>> work for oslog. Our u-boot fork uses the same logic as used here [1]. We're stealing
>> a chunk of MTP's SRAM to make hand-off to Linux easier there. If either size or
>> IOVA was off by a factor 0x4000 this would've never worked in the first
>> place.
>
> I'm not suggesting things are off by a factor of 4k. Rather I'm
> questioning what the behaviour is when we're not 4k aligned. (I.e.
> the syslog or oslog buffer does not both start and end at 4k
> boundaries.)
>
> If we're aligned, all our bottom bits are 0, and hypothetically we're
> putting 0 into "reserved-must be zero" bits.
>
> I guess it's inconsequential if everything is 4k aligned in practice.
> But .. is it? I don't know.
For the common buffers at least we sometimes _get_ the address from the
co-processor when it e.g. points to its internal SRAM. We could access any
buffer aligned to a 4 byte boundary in that case because it's just MMIO
access then and I'm not even sure how strongly the buffer passed to us will be aligned.
I'd rather not zero out bits there because we just cannot be sure those really
are reserved.
If the co-processor only asks for a buffer we'll grab it using dma_alloc_coherent
and it's going to be aligned anyway then.
For oslog there aren't even any "reserved lower bits" in the message. It really
expects the down-shifted address on the wire starting at bit 0.
Sven
© 2016 - 2026 Red Hat, Inc.