xen/include: public: Document the padding in struct xen_hvm_param

Commit Message

Julien Grall Feb. 6, 2020, 3:41 p.m. UTC

From: Julien Grall <[e-mail]>

There is an implicit padding of 2 bytes in struct xen_hvm_param between
the field domid and index. Make it explicit by introduce a padding
field. This can also serve as documentation.

Note that I don't think we can mandate it to be zero because a guest may
not have initialized the padding.

Signed-off-by: Julien Grall <[e-mail]>
---
 xen/include/public/hvm/hvm_op.h | 1 +
 1 file changed, 1 insertion(+)

Comments

Jan Beulich Feb. 6, 2020, 3:52 p.m. UTC | #1

On 06.02.2020 16:41, Julien Grall wrote:
> From: Julien Grall <[e-mail]>
> 
> There is an implicit padding of 2 bytes in struct xen_hvm_param between
> the field domid and index. Make it explicit by introduce a padding
> field. This can also serve as documentation.
> 
> Note that I don't think we can mandate it to be zero because a guest may
> not have initialized the padding.

Agreed - we can mandate such only at introduction, not at any
later time.

> Signed-off-by: Julien Grall <[e-mail]>

Acked-by: Jan Beulich <[e-mail]>

(I doubt this is the only missing field to make padding explicit.)

Jan

Julien Grall Feb. 6, 2020, 3:55 p.m. UTC | #2

Hi Jan,

On 06/02/2020 15:52, Jan Beulich wrote:
> On 06.02.2020 16:41, Julien Grall wrote:
>> From: Julien Grall <[e-mail]>
>>
>> There is an implicit padding of 2 bytes in struct xen_hvm_param between
>> the field domid and index. Make it explicit by introduce a padding
>> field. This can also serve as documentation.
>>
>> Note that I don't think we can mandate it to be zero because a guest may
>> not have initialized the padding.
> 
> Agreed - we can mandate such only at introduction, not at any
> later time.
> 
>> Signed-off-by: Julien Grall <[e-mail]>
> 
> Acked-by: Jan Beulich <[e-mail]>

Thank you!

> (I doubt this is the only missing field to make padding explicit.)

Probably not, sadly. I stumbled accross the HVM param implicit padding 
while looking at liveupdating the HVM param.

I guess I could use pahole to check the paddings are explicit for all 
the public structure.

Cheers,

Wei Liu Feb. 6, 2020, 4:19 p.m. UTC | #3

On Thu, Feb 06, 2020 at 03:41:18PM +0000, Julien Grall wrote:
> From: Julien Grall <[e-mail]>
> 
> There is an implicit padding of 2 bytes in struct xen_hvm_param between
> the field domid and index. Make it explicit by introduce a padding
> field. This can also serve as documentation.
> 
> Note that I don't think we can mandate it to be zero because a guest may
> not have initialized the padding.
> 
> Signed-off-by: Julien Grall <[e-mail]>

Acked-by: Wei Liu <[e-mail]>

Patch

diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
index 610e020a62..b599d3cbd0 100644
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -32,6 +32,7 @@ 
 #define HVMOP_get_param           1
 struct xen_hvm_param {
     domid_t  domid;    /* IN */
+    uint16_t pad;
     uint32_t index;    /* IN */
     uint64_t value;    /* IN/OUT */
 };