On 03/02/2018 12:11 AM, Konrad Rzeszutek Wilk wrote:
- +----------------+----------------+----------------+----------------+
- | gref_directory | 24
- +----------------+----------------+----------------+----------------+
- | reserved | 28
- +----------------+----------------+----------------+----------------+
- |//////////////////////////////////|
- | period_sz | 28
- +----------------+----------------+----------------+----------------+
- | reserved | 32
- +----------------+----------------+----------------+----------------+
@@ -578,6 +616,14 @@
- pcm_channels - uint8_t, number of channels of this stream,
- [channels-min; channels-max]
- buffer_sz - uint32_t, buffer size to be allocated, octets
- period_sz - uint32_t, recommended event period size, octets
- This is the recommended (hint) value of the period at which frontend would
- like to receive XENSND_EVT_CUR_POS notifications from the backend when
- stream position advances during playback/capture.
- It shows how many octets are expected to be played/captured before
- sending such an event.
- If set to 0 no XENSND_EVT_CUR_POS events are sent by the backend.
I would gate this based on the version. That is if version 0 then this field does not exist.
Well, by default we have all unused fields set to 0: [1] And for version 1 (or 0, initial) of the protocol it means that period_sz falls into reserved region [2]. With the comment above for version 2 it means that old behavior is in use, e.g. no XENSND_EVT_CUR_POS events are sent by the backend, so we are backward compatible in this case
- gref_directory - grant_ref_t, a reference to the first shared page
- describing shared buffer references. At least one page exists. If shared
- buffer size (buffer_sz) exceeds what can be addressed by this single page,
@@ -592,6 +638,7 @@ struct xensnd_open_req { uint16_t reserved; uint32_t buffer_sz; grant_ref_t gref_directory;
- uint32_t period_sz;
The same here. Just put a comment mentioning the version part.
So, if you still want to gate it, how would you like it? XENSND_PROTOCOL_VERSION was defined as a string "1"/"2" and preprocessor won't allow comparing strings, e.g. you can't do #if XENSND_PROTOCOL_VERSION == "1" So, we might want re-defining XENSND_PROTOCOL_VERSION as an integer in the first patch, so it can be used here. Then, if it is an integer: 1. #if XENSND_PROTOCOL_VERSION != 1 uint32_t period_sz; #endif 2. #if XENSND_PROTOCOL_VERSION > 1 uint32_t period_sz; #endif 3. #if XENSND_PROTOCOL_VERSION == 2 uint32_t period_sz; #endif
Please let me know whether we still want gating and if so in which way.
Thank you, Oleksandr [1] https://elixir.bootlin.com/linux/v4.16-rc3/source/include/xen/interface/io/s... [2] https://elixir.bootlin.com/linux/v4.16-rc3/source/include/xen/interface/io/s...