> > > From an interface cleanliness point of view I'd rather see > > > all the optional fields as optional. That's done by marking them > > > with a * so > > > '*channel': 'int16' > > > > > > Then the signature of the related qmp_cxl_inject_gen_media_event > > > gains a boolean has_channel parameter (before channel) > > > > > > Those booleans can be used to set the flags etc. > > > > Ah! Ok cool. Yes this would make a lot more sense. I did not realize QMP > > did > > this optional thing. That makes scripting this easier as well! > > > > Did you put all this on a branch or not? I did not see anything new at: > > > > https://gitlab.com/jic23/qemu.git > > > > I can definitely respin but it sounds like you were going to make some > > changes. > > I got side tracked and reworked a few more things. Hopefully in have a branch > up in a day or two.
Now pushed out as https://gitlab.com/jic23/qemu/-/tree/cxl-2023-01-11 My plan is to send the first 8 patches to Michael targetting upstream which includes your uuid related patches from the start of this series. The changes to RAS error injection since previous tree are substantial to support multiple header logging. I've made a few tweaks to your other patches on that branch including the optional parameters stuff and some reworks I mentioned in this thread. Note there is a dirty hack on top of that tree to deal with a build issue on my arch-linux test box that I foolishly upgraded this morning. Might break things on other distros (version issue with curl). Jonathan > > Jonathan > > > > > Ira > > > > > > > > Very lightly tested: > > > > > > diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c > > > index 4660a44ef8..cb9bb0b166 100644 > > > --- a/hw/mem/cxl_type3.c > > > +++ b/hw/mem/cxl_type3.c > > > @@ -1203,8 +1203,9 @@ void qmp_cxl_inject_gen_media_event(const char > > > *path, uint8_t log, > > > uint8_t flags, uint64_t physaddr, > > > uint8_t descriptor, uint8_t type, > > > uint8_t transaction_type, > > > - int16_t channel, int16_t rank, > > > - int32_t device, > > > + bool has_channel, uint8_t channel, > > > + bool has_rank, uint8_t rank, > > > + bool has_device, uint32_t device, > > > const char *component_id, > > > Error **errp) > > > { > > > @@ -1238,22 +1239,22 @@ void qmp_cxl_inject_gen_media_event(const char > > > *path, uint8_t log, > > > gem.type = type; > > > gem.transaction_type = transaction_type; > > > > > > - if (0 <= channel && channel <= 0xFF) { > > > + if (has_channel) { > > > gem.channel = channel; > > > valid_flags |= CXL_GMER_VALID_CHANNEL; > > > } > > > > > > - if (0 <= rank && rank <= 0xFF) { > > > + if (has_rank) { > > > gem.rank = rank; > > > valid_flags |= CXL_GMER_VALID_RANK; > > > } > > > > > > - if (0 <= device && device <= 0xFFFFFF) { > > > + if (has_device) { > > > st24_le_p(gem.device, device); > > > valid_flags |= CXL_GMER_VALID_DEVICE; > > > } > > > > > > - if (component_id && strcmp(component_id, "")) { > > > + if (component_id) { > > > strncpy((char *)gem.component_id, component_id, > > > sizeof(gem.component_id) - 1); > > > valid_flags |= CXL_GMER_VALID_COMPONENT; > > > diff --git a/qapi/cxl.json b/qapi/cxl.json > > > index 56e85a28d7..085f82e7da 100644 > > > --- a/qapi/cxl.json > > > +++ b/qapi/cxl.json > > > @@ -26,8 +26,8 @@ > > > 'data': { 'path': 'str', 'log': 'uint8', 'flags': 'uint8', > > > 'physaddr': 'uint64', 'descriptor': 'uint8', > > > 'type': 'uint8', 'transactiontype': 'uint8', > > > - 'channel': 'int16', 'rank': 'int16', > > > - 'device': 'int32', 'componentid': 'str' > > > + '*channel': 'uint8', '*rank': 'uint8', > > > + '*device': 'uint32', '*componentid': 'str' > > > }} > > > > > > ## > > > > > > > + > > > > ## > > > > # @cxl-inject-poison: > > > > # > > > > > > > > >
