swung0x48/MobileGL
1
0
Fork 0
mirror of https://github.com/MobileGL-Dev/MobileGL synced 2026-09-17 00:28:31 +09:00
Code Issues Packages Projects Releases Wiki Activity

[Fix] (MG_Pipe): give the four acceptance calls the kReplySlot their applier entry points have always answered through, so the flags table is the single thing that sizes the reply pool and the decoder posts against

Browse Source
This commit is contained in:
swung0x48
2026-09-11 15:44:24 -04:00
parent ff2994d9a8
commit d55e37337b
5 changed files with 106 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
MobileGL/MG_Pipe/PipeCalls.def
+11 -4
View File
@@ -90,7 +90,14 @@
/* property of the payload, so its absence was simply wrong. Adding a flag does not */ \
/* move an opcode: the opcode is this line's 1-based position. */ \
X(GetCaps, MGPCaps, kScreen, kReplySlot|kHasBlob) \
X(ResourceCreate, MGPResourceDesc, kScreen, kNone) \
/* THE FOUR ACCEPTANCE ROWS (P5 R-5, R-16). These four and map_persistent are the only */ \
/* applier entry points that RETURN something the client acts on destructively - clearing */ \
/* per-level dirty flags, latching parameters, advancing its own bookkeeping - so the */ \
/* answer has to come back, and kReplySlot is how a record says so. They carried no flag */ \
/* until P5, because in monolith the answer is the return value of a direct call and there */ \
/* was nothing to declare. kMGPipeCallFlags is now what sizes the reply pool and what the */ \
/* decoder posts against, so a row that answers without the flag is a waiter that hangs. */ \
X(ResourceCreate, MGPResourceDesc, kScreen, kReplySlot) \
/* NO kHasBlob, BY RULING (P5 R-13.3; CONTRACT-P5.md table 1 row 19). MGPResourceDesc */ \
/* owns no MGPBlobRef and gains none: under split `initialBytes` is always nullptr and */ \
/* the initial content of a glBufferData(size, data) / glTexImage*(..., data) arrives */ \
@@ -99,7 +106,7 @@
/* cost is one extra record. NOTE the SECOND uncarried companion on this call, the */ \
/* MGPRespecifiedLevel* scope (PipeApply.h:792) - it is NOT bytes and NOT covered by */ \
/* this ruling; table 1 row 19 gives it the two pad fields of MGPResourceDesc. */ \
X(ResourceRespecify, MGPResourceDesc, kScreen, kNeedsAck) \
X(ResourceRespecify, MGPResourceDesc, kScreen, kNeedsAck|kReplySlot) \
X(ResourceDestroy, MGPHandleOnly, kScreen, kNone) \
X(MapPersistent, MGPHandleOnly, kScreen, kReplySlot|kOptional) \
X(UnmapPersistent, MGPHandleOnly, kScreen, kOptional) \
@@ -154,8 +161,8 @@
/* final step asserts sizeof(ResidualValueBlock) == 0 (plan 6.3). */ \
X(SetResidualValueState, MGPResidualValueState, kCtxState, kHasBlob) \
/* ---- context: per-object state and transfer (plan 4.4.3 set_texture_params, 4.4.4) ---- */ \
X(SetTextureParams, MGPTextureParams, kCtxObject, kNone) \
X(ResourceSubData, MGPSubData, kCtxObject, kHasBlob|kVarTail) \
X(SetTextureParams, MGPTextureParams, kCtxObject, kReplySlot) \
X(ResourceSubData, MGPSubData, kCtxObject, kHasBlob|kVarTail|kReplySlot) \
X(BufferSubDataResident, MGPSubData, kCtxObject, kHasBlob|kOptional) \
X(ResourceSubDataComplete, MGPSubDataComplete, kCtxObject, kNone) \
/* NO kHasBlob AND NO MGPBlobRef, BY RULING (P5 R-13.2; CONTRACT-P5.md table 1 row 21). */ \
MobileGL/MG_Pipe/generated/PipeTables.inc
+4 -4
View File
@@ -16,8 +16,8 @@
// call and the frontend keeps its own path (plan B section 4.1).
struct MGPipeScreen {
void (*GetCaps)(const MGPCaps* payload, MGPReplySlot* reply);
void (*ResourceCreate)(const MGPResourceDesc* payload);
void (*ResourceRespecify)(const MGPResourceDesc* payload);
void (*ResourceCreate)(const MGPResourceDesc* payload, MGPReplySlot* reply);
void (*ResourceRespecify)(const MGPResourceDesc* payload, MGPReplySlot* reply);
void (*ResourceDestroy)(const MGPHandleOnly* payload);
void (*MapPersistent)(const MGPHandleOnly* payload, MGPReplySlot* reply);
void (*UnmapPersistent)(const MGPHandleOnly* payload);
@@ -67,8 +67,8 @@ struct MGPipeContext {
void (*SetDrawProgram)(const MGPHandleOnly* payload);
void (*SetDispatchProgram)(const MGPHandleOnly* payload);
void (*SetResidualValueState)(const MGPResidualValueState* payload);
void (*SetTextureParams)(const MGPTextureParams* payload);
void (*ResourceSubData)(const MGPSubData* payload, const void* varTail, Uint32 varTailCount);
void (*SetTextureParams)(const MGPTextureParams* payload, MGPReplySlot* reply);
void (*ResourceSubData)(const MGPSubData* payload, const void* varTail, Uint32 varTailCount, MGPReplySlot* reply);
void (*BufferSubDataResident)(const MGPSubData* payload);
void (*ResourceSubDataComplete)(const MGPSubDataComplete* payload);
void (*ResourceFlushRange)(const MGPFlushRange* payload);
MobileGL/MG_Pipe/generated/PipeThunks.inc
+8 -8
View File
@@ -21,12 +21,12 @@ inline void MGP_GetCaps(const MGPCaps* payload, MGPReplySlot* reply) {
gMGPipeScreen.GetCaps(payload, reply);
}
inline void MGP_ResourceCreate(const MGPResourceDesc* payload) {
gMGPipeScreen.ResourceCreate(payload);
inline void MGP_ResourceCreate(const MGPResourceDesc* payload, MGPReplySlot* reply) {
gMGPipeScreen.ResourceCreate(payload, reply);
}
inline void MGP_ResourceRespecify(const MGPResourceDesc* payload) {
gMGPipeScreen.ResourceRespecify(payload);
inline void MGP_ResourceRespecify(const MGPResourceDesc* payload, MGPReplySlot* reply) {
gMGPipeScreen.ResourceRespecify(payload, reply);
}
inline void MGP_ResourceDestroy(const MGPHandleOnly* payload) {
@@ -201,12 +201,12 @@ inline void MGP_SetResidualValueState(const MGPResidualValueState* payload) {
gMGPipeContext.SetResidualValueState(payload);
}
inline void MGP_SetTextureParams(const MGPTextureParams* payload) {
gMGPipeContext.SetTextureParams(payload);
inline void MGP_SetTextureParams(const MGPTextureParams* payload, MGPReplySlot* reply) {
gMGPipeContext.SetTextureParams(payload, reply);
}
inline void MGP_ResourceSubData(const MGPSubData* payload, const void* varTail, Uint32 varTailCount) {
gMGPipeContext.ResourceSubData(payload, varTail, varTailCount);
inline void MGP_ResourceSubData(const MGPSubData* payload, const void* varTail, Uint32 varTailCount, MGPReplySlot* reply) {
gMGPipeContext.ResourceSubData(payload, varTail, varTailCount, reply);
}
inline void MGP_BufferSubDataResident(const MGPSubData* payload) {
MobileGL/MG_Pipe/generated/PipeWire.inc
+53 -5
View File
@@ -34,7 +34,33 @@
struct MGPWireRecHeader {
Uint16 Op; // MGPWireOp
Uint16 Flags; // MGPipeCallFlags of the call, for asserts and tracing
// RING FRAMING FLAGS - MG_Remote::Transport::RingRecordFlags - NOT MGPipeCallFlags.
//
// This field IS RingRecordHeader::flags (Ring.h): the two structs are the same eight bytes
// and the ring writes this one. The two flag spaces OVERLAP AND DISAGREE, so stamping
// MGPipeCallFlagsFor(op) in here is a silent, data-dependent corruption:
//
// bit 0 kNeedsAck == kRecNeedsAck agree
// bit 1 kHasBlob == kRecHasBlob agree
// bit 2 kVarTail vs kRecPad DISAGREE
// bit 3 kHostSpan vs kRecBorrowSlot DISAGREE
// bit 4 kReplySlot vs kRecVarTail DISAGREE
// bit 5 kOptional vs (nothing) no ring counterpart
//
// The first two agreeing is exactly what makes this dangerous: a naive stamp looks right
// in a debugger. It is not. RingProducer::Reserve masks kRecPad out of a caller's flags,
// so bit 2 is defended for anyone who goes through it - and ONLY bit 2, and only on that
// path. A producer that writes this header itself (these are the same eight bytes, so a
// codec with its own header struct does exactly that) loses even that: Pop treats kRecPad
// as a WRAP FILLER AND SKIPS THE RECORD. Bits 3 and 4 are undefended everywhere - a
// stamped kHostSpan tells the consumer this record borrowed a slot into the GPU timeline
// and must retire late, and a stamped kReplySlot claims a tail that is not there.
//
// THE TWO SPACES ARE DISJOINT BY TRANSLATION, NOT SHARED. The encoder maps one to the
// other explicitly (MG_Remote/Wire/PipeWireCodec.cpp) and nothing else may write this
// field. The CALL's flags are not on the wire at all and do not need to be: the opcode is,
// and MGPipeCallFlagsFor(op) recovers them exactly on either side.
Uint16 Flags;
Uint32 Size; // bytes of this record including the header and the variable tail
};
static_assert(sizeof(MGPWireRecHeader) == 8, "the wire header is 8 bytes");
@@ -138,8 +164,8 @@ enum class MGPWireOp : Uint16 {
inline constexpr Uint32 kMGPipeCallFlags[static_cast<SizeT>(MGPWireOp::kOpCount)] = {
/* 0 kInvalid */ static_cast<Uint32>(kNone),
/* 1 GetCaps */ static_cast<Uint32>(kReplySlot | kHasBlob),
/* 2 ResourceCreate */ static_cast<Uint32>(kNone),
/* 3 ResourceRespecify */ static_cast<Uint32>(kNeedsAck),
/* 2 ResourceCreate */ static_cast<Uint32>(kReplySlot),
/* 3 ResourceRespecify */ static_cast<Uint32>(kNeedsAck | kReplySlot),
/* 4 ResourceDestroy */ static_cast<Uint32>(kNone),
/* 5 MapPersistent */ static_cast<Uint32>(kReplySlot | kOptional),
/* 6 UnmapPersistent */ static_cast<Uint32>(kOptional),
@@ -183,8 +209,8 @@ inline constexpr Uint32 kMGPipeCallFlags[static_cast<SizeT>(MGPWireOp::kOpCount)
/* 44 SetDrawProgram */ static_cast<Uint32>(kNone),
/* 45 SetDispatchProgram */ static_cast<Uint32>(kNone),
/* 46 SetResidualValueState */ static_cast<Uint32>(kHasBlob),
/* 47 SetTextureParams */ static_cast<Uint32>(kNone),
/* 48 ResourceSubData */ static_cast<Uint32>(kHasBlob | kVarTail),
/* 47 SetTextureParams */ static_cast<Uint32>(kReplySlot),
/* 48 ResourceSubData */ static_cast<Uint32>(kHasBlob | kVarTail | kReplySlot),
/* 49 BufferSubDataResident */ static_cast<Uint32>(kHasBlob | kOptional),
/* 50 ResourceSubDataComplete */ static_cast<Uint32>(kNone),
/* 51 ResourceFlushRange */ static_cast<Uint32>(kNone),
@@ -239,6 +265,28 @@ static_assert((MGPipeCallFlagsFor(MGPWireOp::DrawVbo) &
static_cast<Uint32>(kHostSpan | kVarTail)) == static_cast<Uint32>(kHostSpan | kVarTail),
"draw_vbo is the conditional-tail plus host-span shape the codec is measured on");
// The bit layout of MGPipeCallFlags, read out of MGPipe.h by the generator.
// MG_Test/Wire/RingTest.cpp holds the other half: what each of these bits means
// in MG_Remote::Transport::RingRecordFlags, which is NOT the same thing.
static_assert(static_cast<Uint32>(kNeedsAck) == (1u << 0),
"MGPipeCallFlags::kNeedsAck moved bit; MG_Remote's translation is keyed on it");
static_assert(static_cast<Uint32>(kHasBlob) == (1u << 1),
"MGPipeCallFlags::kHasBlob moved bit; MG_Remote's translation is keyed on it");
static_assert(static_cast<Uint32>(kVarTail) == (1u << 2),
"MGPipeCallFlags::kVarTail moved bit; MG_Remote's translation is keyed on it");
static_assert(static_cast<Uint32>(kHostSpan) == (1u << 3),
"MGPipeCallFlags::kHostSpan moved bit; MG_Remote's translation is keyed on it");
static_assert(static_cast<Uint32>(kReplySlot) == (1u << 4),
"MGPipeCallFlags::kReplySlot moved bit; MG_Remote's translation is keyed on it");
static_assert(static_cast<Uint32>(kOptional) == (1u << 5),
"MGPipeCallFlags::kOptional moved bit; MG_Remote's translation is keyed on it");
// ... and this is ALL of them. A seventh flag changes this number, which is the
// build break that sends its author to the ring's flag space before it ships.
inline constexpr Uint32 kMGPipeCallFlagsAllBits = static_cast<Uint32>(kNeedsAck | kHasBlob | kVarTail | kHostSpan | kReplySlot | kOptional);
static_assert(kMGPipeCallFlagsAllBits == 0x3Fu,
"the MGPipeCallFlags bit set changed; see RingTest's cross-enum table");
struct alignas(8) MGPWireRec_GetCaps {
MGPWireRecHeader Header;
MGPCaps Payload;
MobileGL/MG_Test/Pipe/PipeCatalogueTest.cpp
+30
View File
@@ -850,6 +850,36 @@ TEST(PipeCatalogue, ResourceRespecifyAcksOnlyImmutableStorage) {
#undef MGP_COUNT_ACKING_CALLS
EXPECT_EQ(ackingCalls, 1u);
// P5 R-16: the same shape over kReplySlot, which had no count pin at all until the flag
// became load-bearing. It is what sizes the reply pool and what the decoder posts against,
// so the number is now a protocol quantity rather than a documentation one.
//
// FOURTEEN. Ten answers that were always declared - get_caps, map_persistent, the two fence
// reads, the three query reads, the two readbacks and read_pixels - plus the FOUR ACCEPTANCE
// ROWS, whose applier entry points return a Bool the client acts on destructively and which
// carried no flag because in monolith that answer is a direct call's return value.
Uint32 replySlotCalls = 0;
#define MGP_COUNT_REPLY_SLOT_CALLS(Name, Payload, Class, Flags) \
if ((static_cast<Uint32>(Flags) & static_cast<Uint32>(kReplySlot)) != 0) ++replySlotCalls;
MGP_CALL_LIST(MGP_COUNT_REPLY_SLOT_CALLS)
#undef MGP_COUNT_REPLY_SLOT_CALLS
EXPECT_EQ(replySlotCalls, 14u);
// And the four by name, because a count alone would let a row lose the flag while another
// gained one. These are exactly the MGPipeApply* entry points that return Bool
// (PipeApply.h:820, :868, :897, :1023); map_persistent's void* is the fifth answer and was
// already declared.
Uint32 acceptanceWithSlot = 0;
#define MGP_COUNT_ACCEPTANCE_ROWS(Name, Payload, Class, Flags) \
if ((std::strcmp(#Name, "ResourceCreate") == 0 || std::strcmp(#Name, "ResourceRespecify") == 0 || \
std::strcmp(#Name, "ResourceSubData") == 0 || std::strcmp(#Name, "SetTextureParams") == 0) && \
(static_cast<Uint32>(Flags) & static_cast<Uint32>(kReplySlot)) != 0) { \
++acceptanceWithSlot; \
}
MGP_CALL_LIST(MGP_COUNT_ACCEPTANCE_ROWS)
#undef MGP_COUNT_ACCEPTANCE_ROWS
EXPECT_EQ(acceptanceWithSlot, 4u);
// glBufferStorage: an immutable store, and the one entry point allowed a synchronous ack.
MGPResourceDesc immutable{};
immutable.Immutable = 1;