On Fri, 1 Nov 2024 at 01:40, Jarkko Sakkinen <jarkko@xxxxxxxxxx> wrote: > > On Fri Nov 1, 2024 at 2:33 AM EET, Jarkko Sakkinen wrote: > > On Fri Nov 1, 2024 at 1:08 AM EET, Thomas Gleixner wrote: > > > On Fri, Nov 01 2024 at 00:37, Jarkko Sakkinen wrote: > > > > On Thu Oct 31, 2024 at 9:25 PM EET, Thomas Gleixner wrote: > > > >> So this looks pretty reasonable to me by now and I'm inclined to take it > > > >> through the tip x86 tree, but that needs reviewed/acked-by's from the > > > >> crypto and TPM folks. EFI has been reviewed already. > > > >> > > > >> Can we make progress on this please? > > > > > > > > So TPM patches do have bunch of glitches: > > > > > > > > - 15/20: I don't get this. There is nothing to report unless tree > > > > is falling. The reported-by tag literally meaningless. Maybe this > > > > is something that makes sense with this feature. Explain from that > > > > angle. > > > > - 16/20: Is this actually a bug fix? If it is should be before 15/20. > > > > - 17/20: the commit message could do a better job explaining how the > > > > locality can vary. I'm not sure how this will be used by rest of > > > > the patch set. > > > > - 18/20: I'm not confident we want to give privilege to set locality > > > > to the user space. The commit message neither makes a case of this. > > > > Has this been tested to together with bus encryption (just checking)? > > > > > > Can you please explicitely voice your detailed technical concerns in > > > replies to the actual patches? > > > > - 15/20 looks like a rigged patch. I don't really know why it is done > > so it is hard to either suggest how "resolve it". > > - 16/20 probably makes sense but if it is a bug fix or part of it is, > > the bug fix should have relevant fixes etc tags so that it can be > > picked up to stable kernels. > > - 17-18/20: I'd speak about this as the "one whole" i.e. here the > > privilege to be able change locality during run-time is really > > concerning. Could the locality be figured out for the kernel > > command-line instead? The sysfs attribute can exist as read-only. > > > > So yeah, the way I see it 15-16 are the more trivial issue to sort > > out (probably) but with 17-18 we have an actual architectural concern > > for kernel overall. > > Further: > > 15/20: I can accept this without reported-by tag (or changed as > suggested-by). It does not harm. > 16/20: I'll re-review this with time. I'll try to get this done > latest next week. > > So let's put focus only on 17 and 18. Can this problem be sorted out > by kernel command-line parameter? In the case of locality we want to > keep regular "chain of trust" i.e. boot-loader makes the decision, > *even* in the case of DRTM. I would call this almost as constraint > that would be wise to set. > Please don't add a kernel command line parameter for this - the code running in the decompressor will be the one setting it and there are better ways to pass information between these components (and the slaunch stack is already doing that in any case) Also, let's have this discussion in the appropriate place, i.e., on the thread for each respective patch.
