Re: [Minios-devel] [UNIKRAFT] Xen PVH platform

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Apr 10, 2019 at 09:52:43AM +0000, Simon Kuenzer wrote:
>Hi Marek,
>On 18.03.19, 14:14, "Marek Marczykowski-Górecki" 
><marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>On Mon, Mar 18, 2019 at 08:57:31AM +0000, Simon Kuenzer wrote:
>> Hey Marek,
>> On 12.03.19, 17:03, "Minios-devel on behalf of Marek Marczykowski-Górecki" 
>> <minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx on behalf of 
>> marmarek@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>> On Tue, Mar 12, 2019 at 08:52:11AM +0000, Simon Kuenzer wrote:
>> > The most natural way for Unikraft would be to build two Xen binaries: One 
>> > for PV and another one for PVH. The idea is that you get the most 
>> > optimized image for your execution environment. This way you would avoid 
>> > impacts because of the two implementations. You also would not require a 
>> > detection at runtime.
>> 
>> I see, that makes sense. On the other hand, passing CONFIG_PARAVIRT
>> through multiple layers may be problematic, so I think it may be worth
>> having some universal binary. But I'll leave it for some undefined
>> future.
>> 
>> Which layers do you mean? Unikraft? Or the whole Unikernel that you build 
>> and that includes MirageOS?
> 
> The later one.
> 
> Okay, got it. Is this also the actual reason why you prefer having just one 
> image to cover PV and PVH?

Yes, exactly.

>> Anyway, even without having PV+PVH binary, I have one important question:
>> how start_info_t and HYPERVISOR_start_info should looks like? Right now
>> I've made it an union (with "pv" under CONFIG_PARAVIRT and "hvm" under
>> CONFIG_XEN_HVMLITE). This require all the places using this symbol to be
>> changed. Some alternative would be to have a separate symbol like
>> HYPERVISOR_start_info_hvm for the other start info structure and keep
>> HYPERVISOR_start_info unchanged (or under CONFIG_PARAVIRT). While
>> changing everything using HYPERVISOR_start_info is some pain, IMO it's
>> better than forgetting something and erroneously using (uninitialized)
>> PV version on PVH...
>> 
>> What do you prefer?
>>     
> Hum... good question. I would actually check how much differences you see in 
> the PV and PVH code, as well as these structs. How many fields are similar 
> and how much of the existing code could be re-used? 
>     
>     The structs are very different. I think the only common part is command
>     line and modules (which are ignored by Unikraft anyway). PVH start info
>     is much smaller, a lot of things you get from start info on PV have
>     different discovery mechanisms on PVH (hypercall page use cpuid + msr,
>     memory map, shared info page, xenstore page and console rings use
>     hypercalls).
>     As for the code, besides different initialization of things that are in
>     PV start info struct, the code is quite similar. I think the most
>     notable difference is memory management - besides different way of
>     obtaining memory map, page tables are constructed slightly differently:
>      - there is no need to translate pfn->mfn
>      - there is no need for mmu_update hypercall (you can update page tables
>        directly, and also write to CR3 directly)
>      - page tables are constructed using 2MB pages (this is inherited from
>        Mini-OS, but I think it's good idea for unikernels)
> 
> Hum, depends. Yes, many unikernels could be already happy with two pages. I 
> am not sure what 2MB pages mean for Xen devices and grants. Or would you 
> still manage those regions with 4K pages?

I think those areas are still 4K pages.

>     > Since I assume building different binaries for PVH and PV, you are able 
> to do the build with a different set of sources. 
>     
>     Yes. I think the most notable difference I use #ifdef-like solution,
>     that would require significantly more duplication with runtime detection
>     is pfn<->mfn translation. If using different builds, I simply define
>     pfn_to_mfn/mfn_to_pfn as 1:1.
> 
> I am sorry that I still do not see an advantage of an image that auto-detects 
> PV/PVH ;-) . I see it a bit contrary to the Unikraft philosophy where we want 
> to specialize before runtime: towards the application and towards the 
> execution platform.
> I think this is fine because a user or toolstack anyways needs to specify the 
> VM mode with a domain configuration. Why couldn’t there the fitting image be 
> specified?

This is mostly about building and distributing binaries. It's easier
(both for users and developers) to distribute just one binary that will
work with both configurations, instead of two and making sure you use
the right one depending on environment.
That said, I hope I won't be needing PV here very soon, so this point is
void.

> And because the build system can output you multiple images with a single 
> build the additional costs are minimal.

Can it? While it's probably true for multiple platforms, I'm not so sure
about different configurations of the same platform...

> Now, if we assume that we compile multiple images, the #ifdefs is probably 
> good enough. You can compile a source files multiple times with different 
> compiler flags: e.g., one build for each target platform (PV, PVH).
> 
>     > For instance, entry64.S for PVH can be a complete different one than 
> the one for PV. 
>     
>     Actually, entry64.S is very similar. On PVH I only need to add a little
>     prologue to entry point.
> 
> Sounds like another #ifdef ;-)

Not really - since ELF header specify different entry points for PV and
PVH, I can point PVH entry point at this prologue.

>     > Some other files will probably have minor cases. I would use an 
> approach that looks minimal while keeping readability.
>     > Maybe you provide alternative struct definitions depending on the 
> configuration?
>     
>     I was thinking about this too, but it would definitely not work with
>     runtime detection (which I would still leave as a possibility, even if
>     not implemented right now).
>     There is also another issue to consider here - MirageOS currently access
>     start info structure too, so having different definitions here means
>     MirageOS must be compiled differently for PV and PVH.
>     Mindy, can you list things that MirageOS really needs from the start
>     info structure? Maybe some abstract interface could be added here, that
>     would hide this difference?
> 
> Hum... The ideal case would be if there is not such a dependency between 
> mirage and a specific platform. This would make mirage run on KVM and 
> possibly other platforms out-of-the box. But I understand that you want to 
> run the drivers in ocaml and need to know something about the actual platform 
> you run on. The solution would be to introduce a kind of generic platform 
> interfaces (`ukplat_*()`) that mirage could use to detect if it runs on KVM, 
> Xen, etc. I am not sure yet how this would look like. What does Mirage 
> require from the platform lib when run on Xen? Did you ever test also a 
> different platform, like KVM, to understand what you need there?

I think Mirage on KVM doesn't use Unikraft at all, so it's only about
Xen. And yes, it's about drivers in ocaml. It definitely need xenstore
page address, but not sure about other info.
Mindy?
 
>     > Btw, CONFIG_PARAVIRT and CONFIG_HVMLITE are left-overs from our Mini-OS 
> port. I think it makes sense to clean this up and replace the definitions.
>     
>     CONFIG_XEN_PV, CONFIG_XEN_PVH ?
> 
> Yes.
>     
>     Is unikraft targeting also full Xen HVM?
> 
> Not for now. I am also not sure if we there is a gain for Unikraft HVM on 
> Xen, assuming we would have PVH.

Ok, that's good.

-- 
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: signature.asc
Description: PGP signature

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel