libxl__build_hvm type confusion

libxl__build_hvm type confusion

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 527 bytes --]

Hi,

libxl__domain_build calls libxl__build_hvm for both
LIBXL_DOMAIN_TYPE_HVM and LIBXL_DOMAIN_TYPE_PVH, but libxl__build_hvm
uses fields from b_info->u.hvm, which looks like invalid thing to do.
Should those field be moved out of that union?
Additionally I think some asserts in every function using b_info->u
would be a good idea.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: libxl__build_hvm type confusion

[Roger Pau Monné]

On Sat, Aug 04, 2018 at 08:25:18PM +0200, Marek Marczykowski-Górecki wrote:
> Hi,
> 
> libxl__domain_build calls libxl__build_hvm for both
> LIBXL_DOMAIN_TYPE_HVM and LIBXL_DOMAIN_TYPE_PVH, but libxl__build_hvm
> uses fields from b_info->u.hvm, which looks like invalid thing to do.
> Should those field be moved out of that union?

Depends on the fields and whether they are relevant for PVH or not.
Either they are moved out of the HVM struct and shared between PVH and
HVM if they are relevant, or it's usage it's fully limited to domain
type HVM.

I guess this mostly works because those HVM fields are not aliased to
any of the fields of the PVH struct and they are all 0 which happens
to be the correct value for PVH.

> Additionally I think some asserts in every function using b_info->u
> would be a good idea.

Sure, I'm all in for adding more checks!

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: libxl__build_hvm type confusion

[Marek Marczykowski-Górecki]

[-- Attachment #1.1: Type: text/plain, Size: 1392 bytes --]

On Mon, Aug 06, 2018 at 11:16:49AM +0200, Roger Pau Monné wrote:
> On Sat, Aug 04, 2018 at 08:25:18PM +0200, Marek Marczykowski-Górecki wrote:
> > Hi,
> > 
> > libxl__domain_build calls libxl__build_hvm for both
> > LIBXL_DOMAIN_TYPE_HVM and LIBXL_DOMAIN_TYPE_PVH, but libxl__build_hvm
> > uses fields from b_info->u.hvm, which looks like invalid thing to do.
> > Should those field be moved out of that union?
> 
> Depends on the fields and whether they are relevant for PVH or not.
> Either they are moved out of the HVM struct and shared between PVH and
> HVM if they are relevant, or it's usage it's fully limited to domain
> type HVM.

In this particular case it is about:
 - u.hvm.mmio_hole_memkb
 - u.hvm.rdm_mem_boundary_memkb

Not sure if those are relevant for PVH

> I guess this mostly works because those HVM fields are not aliased to
> any of the fields of the PVH struct and they are all 0 which happens
> to be the correct value for PVH.

Since u.pvh is very small, that's probably the case.

> > Additionally I think some asserts in every function using b_info->u
> > would be a good idea.
> 
> Sure, I'm all in for adding more checks!
> 
> Thanks, Roger.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel