On 2/28/23 23:39, Igor Mammedov wrote:
On Tue, 28 Feb 2023 21:34:33 +0700
Bui Quang Minh <minhquangbu...@gmail.com> wrote:

On 2/27/23 23:07, Igor Mammedov wrote:
On Sat, 25 Feb 2023 17:15:17 +0700
Bui Quang Minh <minhquangbu...@gmail.com> wrote:
On 2/24/23 21:29, Igor Mammedov wrote:
On Tue, 21 Feb 2023 23:04:57 +0700
Bui Quang Minh <minhquangbu...@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 <minhquangbu...@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.

I don't think that it does permit this (if it does it's a bug that should be 
fixed).

As far as I'm aware QEMU derives apic_id from '-smp' and possibly cpu type
(there was some differences between Intel and AMD in how apic id was encoded
notably AMD having threads or cores that lead to sparse apic id), though I don't
remember current state of affairs in x86 cpu topo code.

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.
the tricky part in such half approach is making sure that the code is
'correct' and won't lead to exploits.
It would be easier to review if it was completed solution instead of partial.

I looked around and found the way to dynamically allocate local_apics array

        void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
        {
                if (!kvm_irqchip_in_kernel()) {
                        apic_set_max_apic_id(x86ms->apic_id_limit);
                }

        }

We already calculated apic_id_limit before creating CPU and local APIC so we can use that number to dynamically allocated local_apics.

However, there are still problems while trying to extending support to APIC ID larger than 255 because there are many places assume APIC ID is 8-bit long. One of that is interrupt remapping which returns 32-bit destination ID but uses MSI (which has 8-bit destination) to send to APIC. I will look more into this.

Thanks,
Quang Minh.

Reply via email to