Re: [PATCH 14/16] hw/i3c: remote_i3c: Add model
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
Re: [PATCH 14/16] hw/i3c: remote_i3c: Add model
Hi Jeremy, On Sun, Apr 2, 2023 at 11:14 PM Jeremy Kerr wrote: > > Hi Joe, > > > Adds a model to communicate to remote I3C devices over chardev. This > > allows QEMU to communicate to I3C targets that exist outside of QEMU. > > Nice! > > I've been wanting something similar for a while, both for i2c and i3c > busses, to the point of having a similar concept partly implemented. > > A couple of design decisions though: > > 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. > 2) My approach was at the bus level rather than the device level: the > protocol is bidirectional to allow the model to either participate as a > i3c controller or a target. There's quite a bit of mis-fit when applying > that to the qemu device structure though, so your approach is a lot > cleaner. > > I'll have a go at adapting my client to your protocol, and see how the > device interface goes. > > 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. Thanks, Joe > > Cheers, > > > Jeremy
Re: [PATCH 14/16] hw/i3c: remote_i3c: Add model
Hi Joe, > Adds a model to communicate to remote I3C devices over chardev. This > allows QEMU to communicate to I3C targets that exist outside of QEMU. Nice! I've been wanting something similar for a while, both for i2c and i3c busses, to the point of having a similar concept partly implemented. A couple of design decisions though: 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? 2) My approach was at the bus level rather than the device level: the protocol is bidirectional to allow the model to either participate as a i3c controller or a target. There's quite a bit of mis-fit when applying that to the qemu device structure though, so your approach is a lot cleaner. I'll have a go at adapting my client to your protocol, and see how the device interface goes. 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. Cheers, Jeremy
