Re: [PATCH 1/4] apic: add support for x2APIC mode

[Bui Quang Minh]

On 2/27/23 23:07, Igor Mammedov wrote:
> On Sat, 25 Feb 2023 17:15:17 +0700
> Bui Quang Minh <minhquangbui99@gmail.com> wrote:
>
> > On 2/24/23 21:29, Igor Mammedov wrote:
> > > On Tue, 21 Feb 2023 23:04:57 +0700
> > > Bui Quang Minh <minhquangbui99@gmail.com> wrote:
> > >    
> > > > This commit refactors APIC registers read/write function to support both
> > > > MMIO read/write in xAPIC mode and MSR read/write in x2APIC mode. Also,
> > > > support larger APIC ID, self IPI, new IPI destination determination in
> > > > x2APIC mode.
> > > >
> > > > Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> > > > ---
> > > >    hw/intc/apic.c                  | 211 +++++++++++++++++++++++++-------
> > > >    hw/intc/apic_common.c           |   2 +-
> > > >    include/hw/i386/apic.h          |   5 +-
> > > >    include/hw/i386/apic_internal.h |   2 +-
> > > >    4 files changed, 172 insertions(+), 48 deletions(-)
> > > >
> > > > diff --git a/hw/intc/apic.c b/hw/intc/apic.c
> > > > index 2d3e55f4e2..205d5923ec 100644
> > > > --- a/hw/intc/apic.c
> > > > +++ b/hw/intc/apic.c
> > > > @@ -30,6 +30,7 @@
> > > >    #include "hw/i386/apic-msidef.h"
> > > >    #include "qapi/error.h"
> > > >    #include "qom/object.h"
> > > > +#include "tcg/helper-tcg.h"
> > > >    
> > > >    #define MAX_APICS 255
> > >
> > > I'm curious how does it work without increasing ^^^?
> >
> > Hmm, my commit message is not entirely correct. In this series, some
> > operations (send IPI, IPI destination determination) have been updated
> > to support x2APIC mode. However, the emulated APIC still doesn't support
> > APIC ID larger than 255 because currently, we use a fixed length (255 +
> > 1) array to manage local APICs. So to support larger APIC ID, I think we
> > need to find any way to manage those, as the possible allocated APIC ID
> > range is large and maybe the allocated APIC ID is sparse which makes
> > fixed length array so wasteful.
> how much sparse it is?

As far as I know, QEMU allows to set CPU's APIC ID, so user can pass a 
very sparse APIC ID array.

> benefits of simple static array is simplicity in management and O(1) access 
> time.
> QEMU does know in advance max apic id so we can size array by dynamically
> allocating it when 1st apic is created. Or if IDs are too sparse
> switch to another structure to keep mapping.

I totally agree with this.

I admit that my main focus on this series is to make x2APIC mode 
function correctly with TCG accelerator, so I skip the part of extending 
the support for higher APIC ID.

Thanks,
Quang Minh.