On Fri, 2025-02-14 at 13:11 +0000, Fuad Tabba wrote: > Hi Patrick, > > On Fri, 14 Feb 2025 at 12:37, Patrick Roy <roypat@xxxxxxxxxxxx> wrote: >> >> >> >> On Fri, 2025-02-14 at 11:33 +0000, Fuad Tabba wrote: >>> Hi Quentin, >>> >>> On Fri, 14 Feb 2025 at 11:13, Quentin Perret <qperret@xxxxxxxxxx> wrote: >>>> >>>> On Tuesday 11 Feb 2025 at 17:09:20 (+0000), Quentin Perret wrote: >>>>> Hi Patrick, >>>>> >>>>> On Tuesday 11 Feb 2025 at 16:32:31 (+0000), Patrick Roy wrote: >>>>>> I was hoping that SW_PROTECTED_VM will be the VM type that something >>>>>> like Firecracker could use, e.g. an interface to guest_memfd specifically >>>>>> _without_ pKVM, as Fuad was saying. >>>>> >>>>> I had, probably incorrectly, assumed that we'd eventually want to allow >>>>> gmem for all VMs, including traditional KVM VMs that don't have anything >>>>> special. Perhaps the gmem support could be exposed via a KVM_CAP in this >>>>> case? >>>>> >>>>> Anyway, no objection to the proposed approach in this patch assuming we >>>>> will eventually have HW_PROTECTED_VM for pKVM VMs, and that _that_ can be >>>>> bit 31 :). >>>> >>>> Thinking about this a bit deeper, I am still wondering what this new >>>> SW_PROTECTED VM type is buying us? Given that SW_PROTECTED VMs accept >>>> both guest-memfd backed memslots and traditional HVA-backed memslots, we >>>> could just make normal KVM guests accept guest-memfd memslots and get >>>> the same thing? Is there any reason not to do that instead? Even though >>>> SW_PROTECTED VMs are documented as 'unstable', the reality is this is >>>> UAPI and you can bet it will end up being relied upon, so I would prefer >>>> to have a solid reason for introducing this new VM type. >>> >>> The more I think about it, I agree with you. I think that reasonable >>> behavior (for kvm/arm64) would be to allow using guest_memfd with all >>> VM types. If the VM type is a non-protected type, then its memory is >>> considered shared by default and is mappable --- as long as the >>> kconfig option is enabled. If VM is protected then the memory is not >>> shared by default. >>> >>> What do you think Patrick? Do you need an explicit VM type? >> >> Mhh, no, if "normal" VMs support guest_memfd, then that works too. I >> suggested the VM type because that's how x86 works >> (KVM_X86_SW_PROTECTED_VM), but never actually stopped to think about >> whether it makes sense for ARM. Maybe Sean knows something we're missing? >> >> I wonder whether having the "default sharedness" depend on the vm type >> works out though - whether a range of gmem is shared or private is a >> property of the guest_memfd instance, not the VM it's attached to, so I >> guess the default behavior needs to be based solely on the guest_memfd >> as well (and then if someone tries to attach a gmem to a VM whose desire >> of protection doesnt match the guest_memfd's configuration, that >> operation would fail)? > > Each guest_memfd is associated with a KVM instance. Although it could > migrate, it would be weird for a guest_memfd instance to migrate > between different types of VM, or at least, migrate between VMs that > have different confidentiality requirements. Ahh, right, I keep forgetting that CREATE_GUEST_MEMFD() is a vm ioctl. My bad, sorry! >> Tangentially related, does KVM_GMEM_SHARED to you mean "guest_memfd also >> supports shared sections", or "guest_memfd does not support private >> memory anymore"? (the difference being that in the former, then >> KVM_GMEM_SHARED would later get the ability to convert ranges private, >> and the EOPNOSUPP is just a transient state until conversion support is >> merged) - doesnt matter for my usecase, but I got curious as some other >> threads implied the second option to me and I ended up wondering why. > > My thinking (and implementation in the other patch series) is that > KVM_GMEM_SHARED (back then called KVM_GMEM_MAPPABLE) allows sharing in > place/mapping, without adding restrictions. That makes sense to me, thanks for the explanation! > Cheers, > /fuad > >> Best, >> Patrick >> >>> Cheers, >>> /fuad >>> >>>> Cheers, >>>> Quentin
