Re: [PATCH 14/16] hw/i3c: remote_i3c: Add model

[Jeremy Kerr <jk@codeconstruct.com.au>]

Hi Joe,

> > 1) Is this something that qemu upstream would accept? Do we need a
> > formal description of the guest-to-host interface somewhere? Or is there
> > a more standard way of exposing busses like this?
> > 
> Not sure! I'm open to ideas.
> I think the most controversial portion of the remote target like this
> might be doing socket transfers in an MMIO context.
> i.e. driver does a write to I3C controller -> triggers a transaction
> to remote target -> remote target sends/reads data over socket.
> 
> Because of that, we might need to add a way to do these transactions
> asynchronously.

OK, that plus the general concept of having a socket interface to the i3c
bus might need a review from someone above my qemu pay-grade.

but, on the assumption that those are acceptable in general:

> > Assuming we do adopt your approach though, I think the protocol
> > description needs some work. There seems to be other messages not listed
> > in your protocol comments, and the direction of some seems to be
> > reversed. I'm happy to contribute to that documentation if you like.
> 
> Oops. I'll reread and revise in v2. If you have anything else you want
> to add too, let me know and I'll add them as well.

So I implemented a little daemon for the other side of the socket
interface. I have a few thoughts on the protocol structure:

 * can we change the target -> controller read response messages (data +
   len) into a normal message type (ie, assign an opcode and use that in
   the header)? We may want to use separate opcodes for each response
   type.

 * I would suggest expanding the start behaviour a little: the message
   could contain the target address, and the target responds with an
   ACK/NACK event. The model would need to block on the response in
   order to return the correct ACK/NACK value (and pass to the
   hardware), but this means we can either implement the protocol at the
   bus level, or at the individual-device level.

   (with the current standalone NACK event, I don't see how a model
   could reliably handle that)

 * I'm not clear on why the RemoteI3CRXEvents are defined separately.
   Can these just be normal messages with an opcode (and no payload)?

I've yet to implement IBIs though, that's next on my list. Happy to chat
separately if this gets off-topic for the qemu general discussion.

Overall though, this is great work! Thanks for the contributions.

Cheers,


Jeremy