Re: [PATCH v6 00/17] x86: Secure Encrypted Virtualization (AMD)

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-05-29 14:59:46, Brijesh Singh wrote:
> 
> 
> On 5/29/17 3:38 PM, Jordan Justen wrote:
> > On 2017-05-29 04:16:15, Laszlo Ersek wrote:
> >> (looks like I was the one to comment as second reviewer after all :) )
> >>
> >> On 05/26/17 23:05, Jordan Justen wrote:
> >>> On 2017-05-26 07:43:48, Brijesh Singh wrote:
> >>>> Changes since v4:
> >>>>  - decouple IoMmu protocol implementation from AmdSevDxe into a seperate
> >>>>    IoMmuDxe driver. And introduce a placeholder protocol to provide the
> >>>>    dependency support for the dependent modules.
> >>> I think you split IoMmuDxe out from AmdSevDxe based on my feedback
> >>> regarding APRIORI, but I don't think this helped.
> >>>
> >>> Ideally I would like to see one driver named IoMmuDxe that is *not* in
> >>> APRIORI.
> >> There are two separate goals here:
> >>
> >> (1) Make sure that any driver that adds MMIO ranges will automatically
> >> add those ranges with the C bit cleared in the PTEs, without actually
> >> knowing about SEV.
> > Ok, this sounds reasonable.
> >
> > The APRIORI method looks like a hack. Why is this not being handled at
> > the time the page tables are being built, in DxeIpl? Couldn't we
> > define a platform Page Tables library to allow a platform to somehow
> > modify the page tables as they are built? Or, maybe just after? This
> > would also make sure it happens before DXE runs.
> 
> Before introducing  AmdSevDxe driver, we did proposed patches to clear
> the C-bit during the page table creation time. In the first patch [1],
> Leo tried to  teach gcd.c to clear the C-bit from MMIO. IIRC, the main
> concern was -- typically Dxecore does not do any CPU specific thing
> hence we should try to find some alternative approach.

DxeCore doesn't build the page tables. DxeIpl builds them. I agree
that DxeCore is not the right place to handle this. In
https://lists.01.org/pipermail/edk2-devel/2017-March/008987.html
Jiewen suggested that DxeIpl could be updated during page table
creation time.

In https://lists.01.org/pipermail/edk2-devel/2017-April/009883.html
Leo said that DxeIpl won't work because new I/O ranges might be added.
I don't understand this, because isn't DxeIpl and an early APRIORI
entry are roughly equivalent in the boot sequence?

-Jordan

> In second patch
> [2], Leo tried to introduce a new notify protocol to get MMIO add/remove
> events. During discussion Jiewen suggested to look into adding a new
> platform driver into APRIORI to avoid the need for any modifications
> inside the Gcdcore - this seems workable solution which did not require
> adding any CPU specific code inside the Gcd.
> 
> [1] https://lists.01.org/pipermail/edk2-devel/2017-March/008974.html
> [2] https://lists.01.org/pipermail/edk2-devel/2017-April/009852.html
>