Re: [Qemu-devel] Qemu-devel Digest, Vol 112, Issue 520

[Paulo Arcinas]

On Jul 23, 2012 4:59 PM, <address@hidden> wrote:
>
> Send Qemu-devel mailing list submissions to
>         address@hidden
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         https://lists.nongnu.org/mailman/listinfo/qemu-devel
> or, via email, send a message with subject or body 'help' to
>         address@hidden
>
> You can reach the person managing the list at
>         address@hidden
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Qemu-devel digest..."
>
>
> Today's Topics:
>
>    1. Re: [PATCH v2] MP initialization protocol differs between cpu
>       families, and for P6 and onward models it is up to CPU to decide
>       if it will be BSP using this protocol, so try to model this.
>       However there is no point in implementing MP initialization
>       protocol in qemu. Thus first CPU is always marked as BSP.
>       (Igor Mammedov)
>    2. Re: [PATCH 0/4 v2] target-i386: move tcg intialization inside
>       CPU object (Igor Mammedov)
>    3. Re: [PATCH v2] MP initialization protocol differs between cpu
>       families, and for P6 and onward models it is up to CPU to decide
>       if it will be BSP using this protocol, so try to model this.
>       However there is no point in implementing MP initialization
>       protocol in qemu. Thus first CPU is always marked as BSP.
>       (Gleb Natapov)
>    4. Re: [RFC PATCH 2/2] block: gluster as block backend
>       (Bharata B Rao)
>    5. Re: [RFC PATCH 0/2] GlusterFS support in QEMU - v2 (Bharata B Rao)
>    6. Re: [RFC seabios PATCH] enumerate APIC IDs directly       from CPUs
>       (Gleb Natapov)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Mon, 23 Jul 2012 09:44:05 +0200
> From: Igor Mammedov <address@hidden>
> To: address@hidden
> Cc: address@hidden, address@hidden, address@hidden,
>         address@hidden, address@hidden,
>         address@hidden, address@hidden, address@hidden,
>         address@hidden, address@hidden
> Subject: Re: [Qemu-devel] [PATCH v2] MP initialization protocol
>         differs between cpu families, and for P6 and onward models it is up to
>         CPU to decide if it will be BSP using this protocol, so try to model
>         this. However there is no point in implementing MP initialization
>         protocol in qemu. Thus first CPU is always marked as BSP.
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> Hello Gleb,
>
> Is this v2 patch more acceptable then v1?
>
>
>
> ------------------------------
>
> Message: 2
> Date: Mon, 23 Jul 2012 09:46:56 +0200
> From: Igor Mammedov <address@hidden>
> To: address@hidden
> Cc: Anthony Liguori <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH 0/4 v2] target-i386: move tcg
>         intialization inside CPU object
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=UTF-8; format=flowed
>
> ping.
>
> On 06/25/2012 03:55 PM, Igor Mammedov wrote:
> > v2:
> >    - drop usage of prev_debug_excp_handler consistently in all users
> >    - split from reset patches to avoid confusion of inter-dependency
> >
> > Compile & Run tested:
> >    target-i386: tcg and kvm mode
> >    i386-linux-user: running of /bin/ls
> > Compile tested:
> >    xtensa-softmmu && xtensaeb-softmmu
> >
> > git tree for testing:
> >    https://github.com/imammedo/qemu/tree/x86cpu_qom_tcg_v2
> >
> >
> > Igor Mammedov (4):
> >    target-i386: drop usage of prev_debug_excp_handler
> >    target-xtensa: drop usage of prev_debug_excp_handler
> >    cleanup cpu_set_debug_excp_handler
> >    target-i386: move tcg initialization into x86_cpu_initfn()
> >
> >   cpu-exec.c             |    5 +----
> >   exec-all.h             |    2 +-
> >   target-i386/cpu.c      |   10 ++++++++++
> >   target-i386/cpu.h      |    1 +
> >   target-i386/helper.c   |   16 +---------------
> >   target-xtensa/helper.c |    8 +-------
> >   6 files changed, 15 insertions(+), 27 deletions(-)
> >
>
> --
> -----
>   Igor
>
>
>
>
>
> ------------------------------
>
> Message: 3
> Date: Mon, 23 Jul 2012 11:06:48 +0300
> From: Gleb Natapov <address@hidden>
> To: Igor Mammedov <address@hidden>
> Cc: address@hidden, address@hidden, address@hidden,
>         address@hidden, address@hidden, address@hidden,
>         address@hidden, address@hidden,   address@hidden,
>         address@hidden
> Subject: Re: [Qemu-devel] [PATCH v2] MP initialization protocol
>         differs between cpu families, and for P6 and onward models it is up to
>         CPU to decide if it will be BSP using this protocol, so try to model
>         this. However there is no point in implementing MP initialization
>         protocol in qemu. Thus first CPU is always marked as BSP.
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=us-ascii
>
> On Mon, Jul 23, 2012 at 09:44:05AM +0200, Igor Mammedov wrote:
> > Hello Gleb,
> >
> > Is this v2 patch more acceptable then v1?
> Yes. Sorry for not being explicit about it :)
>
> --
>                         Gleb.
>
>
>
> ------------------------------
>
> Message: 4
> Date: Mon, 23 Jul 2012 14:02:57 +0530
> From: Bharata B Rao <address@hidden>
> To: Stefan Hajnoczi <address@hidden>
> Cc: Amar Tumballi <address@hidden>, Anand Avati
>         <address@hidden>,    address@hidden, Vijay Bellur
>         <address@hidden>
> Subject: Re: [Qemu-devel] [RFC PATCH 2/2] block: gluster as block
>         backend
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=us-ascii
>
> On Sun, Jul 22, 2012 at 04:38:00PM +0100, Stefan Hajnoczi wrote:
> > On Sat, Jul 21, 2012 at 9:31 AM, Bharata B Rao
> > <address@hidden> wrote:
> > > +typedef struct GlusterAIOCB {
> > > +    BlockDriverAIOCB common;
> > > +    QEMUIOVector *qiov;
> >
> > The qiov field is unused.
> >
> > > +    char *bounce;
> >
> > Unused.
>
> Yes, removed these two.
>
> >
> > > +    struct BDRVGlusterState *s;
> >
> > You can get this through common.bs->opaque, but if you like having a
> > shortcut, that's fine.
> >
> > > +    int cancelled;
> >
> > bool
>
> Ok.
>
> >
> > > +} GlusterAIOCB;
> > > +
> > > +typedef struct GlusterCBKData {
> > > +    GlusterAIOCB *acb;
> > > +    struct BDRVGlusterState *s;
> > > +    int64_t size;
> > > +    int ret;
> > > +} GlusterCBKData;
> >
> > I think GlusterCBKData could just be part of GlusterAIOCB.  That would
> > simplify the code a little and avoid some malloc/free.
>
> Are you suggesting to put a field
>
> GlusterCBKData gcbk;
>
> inside GlusterAIOCB and use gcbk from there or
>
> Are you suggesting that I make the fields of GlusterCBKData part of
> GlusterAIOCB and get rid of GlusterCBKData altogether ? This means I would
> have to pass the GlusterAIOCB to gluster async calls and update its fields from
> gluster callback routine. I can do this, but I am not sure if you can touch
> the fields of GlusterAIOCB in non-QEMU threads (gluster callback thread).
>
> >
> > > +
> > > +typedef struct BDRVGlusterState {
> > > +    struct glfs *glfs;
> > > +    int fds[2];
> > > +    int open_flags;
> > > +    struct glfs_fd *fd;
> > > +    int qemu_aio_count;
> > > +    int event_reader_pos;
> > > +    GlusterCBKData *event_gcbk;
> > > +} BDRVGlusterState;
> > > +
> > > +#define GLUSTER_FD_READ 0
> > > +#define GLUSTER_FD_WRITE 1
> > > +
> > > +static void qemu_gluster_complete_aio(GlusterCBKData *gcbk)
> > > +{
> > > +    GlusterAIOCB *acb = gcbk->acb;
> > > +    int ret;
> > > +
> > > +    if (acb->cancelled) {
> >
> > Where does cancelled get set?
>
> I realised that I am not supporting bdrv_aio_cancel(). I guess I will have
> to add support for this in next version.
>
> >
> > > +        qemu_aio_release(acb);
> > > +        goto done;
> > > +    }
> > > +
> > > +    if (gcbk->ret == gcbk->size) {
> > > +        ret = 0; /* Success */
> > > +    } else if (gcbk->ret < 0) {
> > > +        ret = gcbk->ret; /* Read/Write failed */
> > > +    } else {
> > > +        ret = -EINVAL; /* Partial read/write - fail it */
> >
> > EINVAL is for invalid arguments.  EIO would be better.
>
> Ok.
>
> >
> > > +/*
> > > + * file=protocol:address@hidden:volname:image
> > > + */
> > > +static int qemu_gluster_parsename(GlusterConf *c, const char *filename)
> > > +{
> > > +    char *file = g_strdup(filename);
> > > +    char *token, *next_token, *saveptr;
> > > +    char *token_s, *next_token_s, *saveptr_s;
> > > +    int ret = -EINVAL;
> > > +
> > > +    /* Discard the protocol */
> > > +    token = strtok_r(file, ":", &saveptr);
> > > +    if (!token) {
> > > +        goto out;
> > > +    }
> > > +
> > > +    /* address@hidden */
> > > +    next_token = strtok_r(NULL, ":", &saveptr);
> > > +    if (!next_token) {
> > > +        goto out;
> > > +    }
> > > +    if (strchr(next_token, '@')) {
> > > +        token_s = strtok_r(next_token, "@", &saveptr_s);
> > > +        if (!token_s) {
> > > +            goto out;
> > > +        }
> > > +        strncpy(c->server, token_s, HOST_NAME_MAX);
> >
> > strncpy(3) will not NUL-terminate when token_s is HOST_NAME_MAX
> > characters long.  QEMU has cutils.c:pstrcpy().
>
> Will use pstrcpy.
>
> >
> > When the argument is too long we should probably report an error
> > instead of truncating.
>
> Or should we let gluster APIs to flag an error with truncated
> server and volume names ?
>
> >
> > Same below.
> >
> > > +        next_token_s = strtok_r(NULL, "@", &saveptr_s);
> > > +        if (!next_token_s) {
> > > +            goto out;
> > > +        }
> > > +        c->port = atoi(next_token_s);
> >
> > No error checking.  If the input is invalid an error message would
> > help the user here.
>
> Fixed.
>
> >
> > > +static struct glfs *qemu_gluster_init(GlusterConf *c, const char *filename)
> > > +{
> > > +    struct glfs *glfs = NULL;
> > > +    int ret;
> > > +
> > > +    ret = qemu_gluster_parsename(c, filename);
> > > +    if (ret < 0) {
> > > +        errno = -ret;
> > > +        goto out;
> > > +    }
> > > +
> > > +    glfs = glfs_new(c->volname);
> > > +    if (!glfs) {
> > > +        goto out;
> > > +    }
> > > +
> > > +    ret = glfs_set_volfile_server(glfs, "socket", c->server, c->port);
> > > +    if (ret < 0) {
> > > +        goto out;
> > > +    }
> > > +
> > > +    /*
> > > +     * TODO: Logging is not necessary but instead nice to have.
> > > +     * Can QEMU optionally log into a standard place ?
> >
> > QEMU prints to stderr, can you do that here too?  The global log file
> > is not okay, especially when multiple QEMU instances are running.
>
> Ok, I can do glfs_set_logging(glfs, "/dev/stderr", loglevel);
>
> >
> > > +     * Need to use defines like gf_loglevel_t:GF_LOG_INFO instead of
> > > +     * hard coded values like 7 here.
> > > +     */
> > > +    ret = glfs_set_logging(glfs, "/tmp/qemu-gluster.log", 7);
> > > +    if (ret < 0) {
> > > +        goto out;
> > > +    }
> > > +
> > > +    ret = glfs_init(glfs);
> > > +    if (ret < 0) {
> > > +        goto out;
> > > +    }
> > > +    return glfs;
> > > +
> > > +out:
> > > +    if (glfs) {
> > > +        (void)glfs_fini(glfs);
> > > +    }
> > > +    return NULL;
> > > +}
> > > +
> > > +static int qemu_gluster_open(BlockDriverState *bs, const char *filename,
> > > +    int bdrv_flags)
> > > +{
> > > +    BDRVGlusterState *s = bs->opaque;
> > > +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
> >
> > Can this be allocated on the stack?
>
> It consists of PATH_MAX(4096), HOST_NAME_MAX(255) and GLUSTERD_MAX_VOLUME_NAME
> (1000). A bit heavy to be on stack ?
>
> >
> > > +    int ret;
> > > +
> > > +    s->glfs = qemu_gluster_init(c, filename);
> > > +    if (!s->glfs) {
> > > +        ret = -errno;
> > > +        goto out;
> > > +    }
> > > +
> > > +    s->open_flags |=  O_BINARY;
> >
> > Can open_flags be a local variable?
>
> Yes, fixed.
>
> >
> > > +static int qemu_gluster_create(const char *filename,
> > > +        QEMUOptionParameter *options)
> > > +{
> > > +    struct glfs *glfs;
> > > +    struct glfs_fd *fd;
> > > +    GlusterConf *c = g_malloc(sizeof(GlusterConf));
> > > +    int ret = 0;
> > > +    int64_t total_size = 0;
> > > +
> > > +    glfs = qemu_gluster_init(c, filename);
> > > +    if (!glfs) {
> > > +        ret = -errno;
> > > +        goto out;
> > > +    }
> > > +
> > > +    /* Read out options */
> > > +    while (options && options->name) {
> > > +        if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> > > +            total_size = options->value.n / BDRV_SECTOR_SIZE;
> > > +        }
> > > +        options++;
> > > +    }
> > > +
> > > +    fd = glfs_creat(glfs, c->image, O_WRONLY|O_CREAT|O_TRUNC|O_BINARY, S_IRWXU);
> >
> > Why set the execute permission bit?
>
> Changed to read and write only.
>
> >
> > > +static void qemu_gluster_close(BlockDriverState *bs)
> > > +{
> > > +    BDRVGlusterState *s = bs->opaque;
> > > +
> > > +    if (s->fd) {
> > > +        glfs_close(s->fd);
> > > +        s->fd = NULL;
> > > +    }
> >
> > Why not call glfs_fini() here?
>
> Missed that, fixed now.
>
> Thanks for your comments.
>
> Regards,
> Bharata.
>
>
>
>
> ------------------------------
>
> Message: 5
> Date: Mon, 23 Jul 2012 14:20:31 +0530
> From: Bharata B Rao <address@hidden>
> To: Stefan Hajnoczi <address@hidden>
> Cc: Amar Tumballi <address@hidden>, Anand Avati
>         <address@hidden>,    address@hidden, Vijay Bellur
>         <address@hidden>
> Subject: Re: [Qemu-devel] [RFC PATCH 0/2] GlusterFS support in QEMU -
>         v2
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=us-ascii
>
> On Sun, Jul 22, 2012 at 03:42:28PM +0100, Stefan Hajnoczi wrote:
> > On Sat, Jul 21, 2012 at 9:29 AM, Bharata B Rao
> > <address@hidden> wrote:
> > > -drive file=gluster:address@hidden:volname:image
> > >
> > > - Here 'gluster' is the protocol.
> > > - 'address@hidden' specifies the server where the volume file specification for
> > >   the given volume resides. 'port' is the port number on which gluster
> > >   management daemon (glusterd) is listening. This is optional and if not
> > >   specified, QEMU will send 0 which will make libgfapi to use the default
> > >   port.
> >
> > 'address@hidden' is weird notation.  Normally it is 'server:port' (e.g.
> > URLs).  Can you change it?
>
> I don't like but, but settled for it since port was optional and : was
> being used as separator here.
>
> >
> > What about the other transports supported by libgfapi: UNIX domain
> > sockets and RDMA?  My reading of glfs.h is that there are 3 connection
> > options:
> > 1. 'transport': 'socket' (default), 'unix', 'rdma'
> > 2. 'host': server hostname for 'socket', path to UNIX domain socket
> > for 'unix', or something else for 'rdma'
> > 3. 'port': TCP port when 'socket' is used.  Ignored otherwise.
> >
> > Unfortunately QEMU block drivers cannot take custom options yet.  That
> > would make it possible to cleanly map these connection options and
> > save you from inventing syntax which doesn't expose all options.
> >
> > In the meantime it would be nice if the syntax exposed all options.
>
> So without the capability to pass custom options to block drivers, am I forced
> to keep extending the file= with more and more options ?
>
> file=gluster:transport:server:port:volname:image ?
>
> Looks ugly and not easy to make any particular option optional. If needed I can
> support this from GlusterFS backend.
>
> >
> > > Note that we are no longer using volfiles directly and use volume names
> > > instead. For this to work, gluster management daemon (glusterd) needs to
> > > be running on the QEMU node. This limits the QEMU user to access the volumes by
> > > the default volfiles that are generated by gluster CLI. This should be
> > > fine as long as gluster CLI provides the capability to generate or regenerate
> > > volume files for a given volume with the xlator set that QEMU user is
> > > interested in. GlusterFS developers tell me that this can be provided with
> > > some enhancements to Gluster CLI/glusterd. Note that the custom volume files
> > > is typically needed when GlusterFS server is co-located with QEMU in
> > > which case it would  be beneficial to get rid of client-server overhead and
> > > RPC communication overhead.
> >
> > My knowledge of GlusterFS is limited.  Here is what I am thinking:
> >
> > 1. The user cannot specify a local configuration file, you require
> > that there is a glusterd running which provides configuration
> > information.
>
> Yes. User only specifies a volume name and glusterd is used to fetch
> the right volume file for that volume name.
>
> > 2. It is currently not possible to bypass RPC because the glusterd
> > managed configuration file doesn't support that.
>
> It is possible. Gluster already supports custom extensions
> to volume names and it is possible to use the required volfile by specifying
> this custom volname extension.
>
> For eg, if I have a volume named test, by default the volfile used for
> it will be test-fuse.vol. Currently I can put my own custom volfile into
> the standard location and get glusterd pick that up. I can specify
> test.rpcbypass as volname and glusterd will pick test.rpcbypass.vol.
>
> What is currently not supported is the ability to create test.rpcbypass.vol
> from gluster CLI. I believe that gluster developers are ok with enhancing
> gluster CLI to support generating/regenerating volfiles for a given volume
> with custom translator set.
>
> >
> > I'm not sure if these statements are true?
> >
> > Would you support local volfiles in the future again?  Why force users
> > to run glusterd?
>
> I will let gluster folks on CC to answer this and let us know the benefits
> of always depending on glusterd.
>
> I guess running glusterd would be beneficial when supporting migration. QEMU
> working from a local volume (with volname=test.rpcbypass) can be easily
> restarted on a different node by just changing volname to test. glusterd will
> take care of fetching the right volfile automatically for us.
>
> >
> > > - As mentioned above, the VM image on gluster volume can be specified like
> > >   this:
> > >         -drive file=gluster:localhost:testvol:/F17,format=gluster
> > >
> > >   Note that format=gluster is not needed ideally and its a work around I have
> > >   until libgfapi provides a working connection cleanup routine (glfs_fini()).
> > >   When the format isn't specified, QEMU figures out the format by doing
> > >   find_image_format that results in one open and close before opening the
> > >   image file long term for standard read and write. Gluster connection
> > >   initialization is done from open and connection termination is done from
> > >   close. But since glfs_fini() isn't working yet, I am bypassing
> > >   find_image_format by specifying format=gluster directly which results in
> > >   just one open and hence I am not limited by glfs_fini().
> >
> > Has libgfapi been released yet?
>
> Its part of gluster mainline now.
>
> > Does it have versioning which will
> > allow the QEMU GlusterFS block driver to build against different
> > versions?  I'm just wondering how the pieces will fit together once
> > distros start shipping them.
>
> I request gluster folks on CC to comment about version and shipping
> information.
>
> Regards,
> Bharata.
>
>
>
>
> ------------------------------
>
> Message: 6
> Date: Mon, 23 Jul 2012 11:58:59 +0300
> From: Gleb Natapov <address@hidden>
> To: Eduardo Habkost <address@hidden>
> Cc: Igor Mammedov <address@hidden>, address@hidden,
>         address@hidden
> Subject: Re: [Qemu-devel] [RFC seabios PATCH] enumerate APIC IDs
>         directly        from CPUs
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=cp1255
>
> On Thu, Jul 19, 2012 at 04:46:35PM -0300, Eduardo Habkost wrote:
> > On Thu, Jul 19, 2012 at 11:28:54AM -0300, Eduardo Habkost wrote:
> > > On Thu, Jul 19, 2012 at 12:58:46PM +0300, Gleb Natapov wrote:
> > > > On Tue, Jul 17, 2012 at 06:56:30PM -0300, Eduardo Habkost wrote:
> > > > > This patch is an attempt to fix the non-continguous-APIC-ID problem without the
> > > > > FW_CFG_LAPIC_INFO approach I have sent proposed last week.
> > > > >
> > > > > Basically, this changes Seabios to probe for APIC IDs directly from the
> > > > > CPUs on boot, instead of getting it using fw_cfg, store the found APIC
> > > > > IDs on a bitmap, and use that information whe building the MADT, SRAT,
> > > > > and SSDT ACPI tables.
> > > > >
> > > > > To do this properly, we have to decide the meaning of "CPU IDs" in the
> > > > > QEMU<->Seabios interfaces, too. I see two possible approaches:
> > > > >
> > > > > 1) Have Seabios and QEMU agree on a a "CPU identifier", that is
> > > > >    independent from the APIC ID.
> > > > > 2) Always use the Initial APIC ID on all communication between QEMU and
> > > > >    Seabios.
> > > > >
> > > > We need to be prepared to support more than 255 cpus. With > 255 cpus
> > > > comes x2apic and x2apic has 32bit apic ids. HW does not have to support
> > > > all of the bits though, but potentially all the bitmasks can grow
> > > > prohibitedly large.
> > >
> > > I see only two solutions:
> > >
> > > - Specify an interface/convention for QEMU and Seabios agree upon a "CPU
> > >   identifier" <=> x2apic <=> LAPIC ID mapping for all CPUs.
> > > - Specify new NUMA-information and CPU hotplug interfaces (or extend the
> > >   existing ones) based on x2apic ID, when Seabios start supporting
> > >   x2apic.
> > >
> > > I am not particularly inclined towards any of those two solutions. I
> > > dislike them equally.  :-)
> >
> > Oh, it is simpler than I have expected. x2APIC specification:
> >
> > "The local APIC ID is initialized by hardware with a 32 bit ID (x2APIC
> > ID). The lowest 8 bits of the x2APIC ID is the legacy local xAPIC ID,
> > and is stored in the upper 8 bits of the APIC register for access in
> > xAPIC mode."
> >
> > And the ACPI specification:
> >
> > "Logical processors with APIC ID values of 255 and greater are required
> > to have a Processor Device object and must convey the processor?s APIC
> > information to OSPM using the Processor Local X2APIC structure. Logical
> > processors with APIC ID values less than 255 must use the Processor
> > Local APIC structure to convey their APIC information to OSPM."
> >
> > That means the x2APIC ID and the xAPIC ID are interchangeable, for
> > values <= 255. That means the QEMU<=>Seabios communication can be safely
> > based on "APIC IDs" without any ambiguity.
> >
> Yes for <= 255 they interchangeable. That's why we can add +x2apic to
> our cpu models without changes to the BIOS.
>
> > The CPU hotplug interface is a bit of a problem because it is based on a
> > 256-bit bitmap. But on the day it gets extended to support more than 256
> > CPUs, it can safely be based on APIC IDs and still keep compatibility
> > with systems without x2APIC.
> The bitmap will have to be extended if we will go beyond 256 cpus. Using
> apic-id to index the bitmap means that the size of the bitmap is a
> function of max apic-id we want to support, not max number of cpus.
>
> >
> > So, now I am strongly inclined towards the second option from the list
> > above: just use APIC IDs everywhere to identify CPUs when QEMU and
> > Seabios communicate with each other, and QEMU can completely ignore the
> > "Processor ID" used by Seabios.
> I agree with making "Processor ID" Seabios internal thing.
>
> >
> > >
> > > Note that I am more worried about the QEMU<->Seabios interfaces. The
> > > APIC ID bitmap on smp.c, for example, is just an implementation detail:
> > > if we make Seabios support x2apic, that code can be changed to use a
> > > different data structure instead.
> > >
> > [...]
> > > > > To try to make things less likely to break on the common, non-hotplug
> > > > > case, this patch makes the Processor IDs be chosen this way:
> > > > >
> > > > > - The CPUs present on boot get contiguous Processor IDs (even if the
> > > > >   APIC IDs are not contiguous);
> > > > > - The remaining Processor declarations are going to associated to the
> > > > >   remaining APIC IDs (immediately after the last present APIC ID),
> > > > >   sequentially. This means that hotplugged CPUs may not get contiguous
> > > > >   Processor declarations if their APIC IDs are not contiguous.
> > > > >
> > > > I am curious what will happen if cpu will be hot plugged, than hibernate
> > > > and resume is done. After resume hot plugged cpu will have different
> > > > Processor ID in ACPI. This may or may not be a problem.
> > >
> > > True. Keeping those tables stable after hotplug and hibernate may be a
> > > challenge.
> > >
> > > Maybe it would be easier to just leave holes on the MADT and SSDT tables
> > > (making APIC ID and Processor ID always match), and hope no OS will be
> > > confused by the holes.
> >
> > I am inclined to try this approach first (keep APIC ID == ACPI Processor
> > ID), to keep things simple in Seabios. I am hoping no OS will have
> > problems with the holes in the list of enabled Processor IDs.
> >
> They shouldn't.
>
> --
>                         Gleb.
>
>
>
> ------------------------------
>
> _______________________________________________
> Qemu-devel mailing list
> address@hidden
> https://lists.nongnu.org/mailman/listinfo/qemu-devel
>
>
> End of Qemu-devel Digest, Vol 112, Issue 520
> ********************************************