Re: [PATCH v4] docs/designs: re-work the xenstore migration document...

[Julien Grall <julien@xen.org>]

Hi Juergen,

On 28/04/2020 11:10, Jürgen Groß wrote:
> On 28.04.20 11:05, Julien Grall wrote:
>>> -where tx_id is the non-zero identifier values of an open transaction.
>>> +| Field     | Description                                       |
>>> +|-----------|---------------------------------------------------|
>>> +| `domid`   | The domain-id that owns the shared page           |
>>> +|           |                                                   |
>>> +| `tdomid`  | The domain-id that `domid` acts on behalf of if   |
>>> +|           | it has been subject to an SET_TARGET              |
>>> +|           | operation [2] or DOMID_INVALID [3] otherwise      |
>>> +|           |                                                   |
>>> +| `flags`   | Must be zero                                      |
>>> +|           |                                                   |
>>> +| `evtchn`  | The port number of the interdomain channel used   |
>>> +|           | by `domid` to communicate with xenstored          |
>>> +|           |                                                   |
>>> +| `mfn`     | The MFN of the shared page for a live update or   |
>>> +|           | ~0 (i.e. all bits set) otherwise                  |
>>> -### Protocol Extension
>>> +Since the ABI guarantees that entry 1 in `domid`'s grant table will 
>>> always
>>> +contain the GFN of the shared page, so for a live update `mfn` can 
>>> be used to
>>> +give confidence that `domid` has not been re-cycled during the update.
>>
>> I am confused, there is no way a XenStored running in an Arm domain 
>> can map the MFN of the shared page. So how is this going to work out?
> 
> I guess this will be a MFN for PV guests and a guest PFN else.

If we use Xen terminology (xen/include/xen/mm.h), this would be a GFN.

> 
>>
>> [...]
>>
>>> -START_DOMAIN_TRANSACTION    <domid>|<transid>|
>>> +    0       1       2       3    octet
>>> ++-------+-------+-------+-------+
>>> +| conn-id                       |
>>> ++-------------------------------+
>>> +| tx-id                         |
>>> ++---------------+---------------+
>>> +| path-len      | value-len     |
>>> ++---------------+---------------+
>>> +| access        | perm-count    |
>>> ++---------------+---------------+
>>> +| perm1                         |
>>> ++-------------------------------+
>>> +...
>>> ++-------------------------------+
>>> +| permN                         |
>>> ++---------------+---------------+
>>> +| path
>>> +...
>>> +| value
>>> +...
>>> +```
>>> +
>>> +
>>> +| Field        | Description                                    |
>>> +|--------------|------------------------------------------------|
>>> +| `conn-id`    | If this value is non-zero then this record     |
>>> +|              | related to a pending transaction               |
>>
>> If conn-id is 0, how do you know the owner of the node?
> 
> The first permission entry's domid is the owner.

I think this should be spell out in the specification.

> 
>>
>>> +|              |                                                |
>>> +| `tx-id`      | This value should be ignored if `conn-id` is   |
>>> +|              | zero. Otherwise it specifies the id of the     |
>>> +|              | pending transaction                            |
>>> +|              |                                                |
>>> +| `path-len`   | The length (in octets) of `path` including the |
>>> +|              | NUL terminator                                 |
>>> +|              |                                                |
>>> +| `value-len`  | The length (in octets) of `value` (which will  |
>>> +|              | be zero for a deleted node)                    |
>>> +|              |                                                |
>>> +| `access`     | This value should be ignored if this record    |
>>> +|              | does not relate to a pending transaction,      |
>>> +|              | otherwise it specifies the accesses made to    |
>>> +|              | the node and hence is a bitwise OR of:         |
>>> +|              |                                                |
>>> +|              | 0x0001: read                                   |
>>> +|              | 0x0002: written                                |
>>> +|              |                                                |
>>> +|              | The value will be zero for a deleted node      |
>>> +|              |                                                |
>>> +| `perm-count` | The number (N) of node permission specifiers   |
>>> +|              | (which will be 0 for a node deleted in a       |
>>> +|              | pending transaction)                           |
>>> +|              |                                                |
>>> +| `perm1..N`   | A list of zero or more node permission         |
>>> +|              | specifiers (see below)                         |
>>
>> This is a bit odd to start at one. Does it mean perm0 exists and not 
>> preserved?
> 
> Why? perm0 does not exist. If you have N permissions 1..N is just fine.
> If you have no permissions (N=0) you won't have any permX entry.
> 
> In theory you could say perm0..N-1, but this would result in perm0..-1
> for N=0 which would be really odd.

As it is odd to me to start at 1 (I am used to index starting at 0) ;). 
The more it wasn't clear how you would specify the owner. So I thought 
perm0 had a specific meaning.

If you clarify perm1 is the owner, then it may make easier to figure out 
that perm0 doesn't exist.

Cheers,

-- 
Julien Grall