RE: [PATCH v5 1/3] Drivers: hv: vmbus: Add vmbus_requestor data structure for VMBus hardening

From: Andres Beltran
Date: Wed Jul 22 2020 - 15:55:54 EST


> From: Michael Kelley <mikelley@xxxxxxxxxxxxx> Sent: Wednesday, July 22, 2020 3:25 PM
> I don't think the above does what you want. The allocated
> array ends up as follows:
>
> Slot 0 contains "2"
> Slot 1 contains "3"
> ...
> Slot size-2 contains size
> Slot size-1 contains U64_MAX
>
> This means that allocating the next-to-last entry will go
> awry. I think the previous version of the slot initialization
> code will actually work just fine.
>

vmbus_next_request_id() and vmbus_request_addr() check that
the id > size, and then the array index becomes id - 1 (or size - 1 for
the next-to-last entry, which is the last slot) , so I think this works fine.
But as you suggested below, returning current_id + 1 and decrementing
trans_id seems cleaner to me.

> The overall scheme you are using to handle the 0 transactionID is
> a good one. Basically the slot array is still tracking values 0 thru
> size-1, but what is presented to the calling VMbus driver is values
> in the range 1 thru size. That way you can recognize 0 as a special case.
> So take this implementation approach:
> * Start with the previous version of the vmbus_next_request_id()
> and vmbus_request_addr() code.
> * In vmbus_next_request_id(), just return current_id+1 instead of
> current_id.
> * In vmbus_request_addr(), add the new code that checks trans_id
> for 0 and returns immediately. Otherwise, decrement trans_id by 1
> and proceed with the existing code.
>
> With this approach, none of the initialization code needs to change.
> Everything uses values in the range 0 to size-1, except that what is
> presented to the VMbus drivers is shifted higher by 1.

Yes, I'll do this instead.

Andres.