On Wed, Mar 26, 2025 at 11:36:09AM +0800, Xiaoyao Li wrote:
> On 3/26/2025 2:46 AM, Daniel P. Berrangé wrote:
> > On Fri, Jan 24, 2025 at 08:20:48AM -0500, Xiaoyao Li wrote:
> > > Add docs/system/i386/tdx.rst for TDX support, and add tdx in
> > > confidential-guest-support.rst
> > >
> > > Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>
> > > ---
> >
> > > ---
> > > docs/system/confidential-guest-support.rst | 1 +
> > > docs/system/i386/tdx.rst | 156 +++++++++++++++++++++
> > > docs/system/target-i386.rst | 1 +
> > > 3 files changed, 158 insertions(+)
> > > create mode 100644 docs/system/i386/tdx.rst
> >
> >
> > > +Launching a TD (TDX VM)
> > > +-----------------------
> > > +
> > > +To launch a TD, the necessary command line options are tdx-guest object
> > > and
> > > +split kernel-irqchip, as below:
> > > +
> > > +.. parsed-literal::
> > > +
> > > + |qemu_system_x86| \\
> > > + -object tdx-guest,id=tdx0 \\
> > > + -machine
> > > ...,kernel-irqchip=split,confidential-guest-support=tdx0 \\
> > > + -bios OVMF.fd \\
> > > +
> > > +Restrictions
> > > +------------
> > > +
> > > + - kernel-irqchip must be split;
> >
> > Is there a reason why we don't make QEMU set kernel-irqchip=split
> > automatically when tdx-guest is enabled ?
> >
> > It feels silly to default to a configuration that is known to be
> > broken with TDX. I thought about making libvirt automatically
> > set kernel-irqchip=split, or even above that making virt-install
> > automatically set it. Addressing it in QEMU would seem the most
> > appropriate place though.
>
> For x86, if not with machine older than machine-4.0, the default
> kernel_irqchip is set to split when users don't set a value explicitly:
I think you may have mis-read the code. *ONLY* pc-q35-4.0 uses
the split IRQ chip. Everything both older and newer than that
uses kernel IRQ chip by default. So our default machine type
settings are incompatible with TDX, except for pc-q35-4.0
We initially tried to use the split IRQ chip by default for 4.0:
commit b2fc91db84470a78f8e93f5b5f913c17188792c8
Author: Peter Xu <pet...@redhat.com>
Date: Thu Dec 20 13:40:35 2018 +0800
q35: set split kernel irqchip as default
Starting from QEMU 4.0, let's specify "split" as the default value for
kernel-irqchip.
but had to revert this in the very next release
commit c87759ce876a7a0b17c2bf4f0b964bd51f0ee871
Author: Alex Williamson <alex.william...@redhat.com>
Date: Tue May 14 14:14:41 2019 -0600
q35: Revert to kernel irqchip
Commit b2fc91db8447 ("q35: set split kernel irqchip as default") changed
the default for the pc-q35-4.0 machine type to use split irqchip, which
turned out to have disasterous effects on vfio-pci INTx support.
>
> if (s->kernel_irqchip_split == ON_OFF_AUTO_AUTO) {
> s->kernel_irqchip_split = mc->default_kernel_irqchip_split ?
> ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
> }
>
>
> I think QEMU should only set it to split automatically for TDX guest when
> users don't provide a explicit value. And current code just works as
> expected.
>
> Further, I think we can at least add the check in tdx_kvm_init() like this
>
> if (kvm_state->kernel_irqchip_split != ON_OFF_AUTO_ON) {
> error_setg(errp, "TDX VM requires kernel_irqchip to be split");
> return -EINVAL;
> }
>
> Are you OK with it?
IMHO we need to modify the current check for
"kernel_irqchip_split == ON_OFF_AUTO_AUTO"
so that it sets to 'ON_OFF_AUTO_ON', if TDX is enabled.
ie something more like
if (s->kernel_irqchip_split == ON_OFF_AUTO_AUTO) {
if (...tdx...) {
s->kernel_irqchip_split = ON_OFF_AUTO_ON;
} else {
s->kernel_irqchip_split = mc->default_kernel_irqchip_split ?
> ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
}
}
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
