On 09/08/2017 03:40 AM, Borislav Petkov wrote:
On Thu, Sep 07, 2017 at 05:19:32PM -0500, Brijesh Singh wrote:At high level, AMD-SP (AMD Secure Processor) (i.e CCP driver) will provide the support for CCP, SEV and TEE FW commands. +--- CCP | AMD-SP --| | +--- SEV | | +---- PSP ---* | +---- TEEI still don't see the need for such finegrained separation, though. There's no "this is a separate compilation unit because... ". At least the PSP branch could be a single driver without the interface. For example, psp_request_sev_irq() is called only by sev_dev_init(). So why is sev-dev a separate compilation unit? Is anything else going to use the PSP interface?
I don't know anything about the TEE support hence I don't have very strong reason for finegrained separation -- I just wanted to ensure that the SEV enablement does not interfere with TEE support in the future.
If not, just put it all in a psp-dev file and that's it. We have a gazillion config options and having two more just because, is not a good reason. You can always carve it out later if there's real need. But if the SEV thing can't function without the PSP thing, then you can just as well put it inside it. This way you can save yourself a bunch of exported functions and the like. Another example for not optimal design is psp_request_tee_irq() - it doesn't really request an irq by calling into the IRQ core but simply assigns a handler. Which looks to me like you're simulating an interface where one is not really needed. Ditto for the sev_irq version, btw.
It's possible that both TEE and SEV share the same interrupt but their interrupt handling approach could be totally different hence I tried to abstract it. I am making several assumption on TEE side without knowing in detail ;) I can go with your recommendation -- we can always crave it out later once the TEE support is visible. -Brijesh
