Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Julien Grall]

On 12/09/17 11:58, Roger Pau Monné wrote:

On Tue, Sep 12, 2017 at 11:42:38AM +0100, Julien Grall wrote:

Hi Roger,

On 14/08/17 15:28, Roger Pau Monne wrote:

This functionality is going to reside in vpci.c (and the corresponding
vpci.h header), and should be arch-agnostic. The handlers introduced
in this patch setup the basic functionality required in order to trap
accesses to the PCI config space, and allow decoding the address and
finding the corresponding handler that should handle the access
(although no handlers are implemented).


If I understand correctly this patch, the virtual BDF will always correspond
to the physical BDF. Is that right? If so, would you mind to explain why
such restriction?


Yes, this is not a limitation of this patch, but of the implementation
that follows. Likely this will be expanded when support for DomU is
added, but for Dom0 at least on x86 adding such a translation layer is
not needed, since I see no reason to present a different PCI topology
to Dom0.


I think this will be the same for ARM. Dom0 will always have pBDF == 
vBDF. For guest will likely want pBDF != vBDF.


This answer to my other question regarding the plan to support vBDF != 
pBDF :). Thanks.




Thanks, Roger.



--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Roger Pau Monné]

On Tue, Sep 12, 2017 at 11:42:38AM +0100, Julien Grall wrote:
> Hi Roger,
> 
> On 14/08/17 15:28, Roger Pau Monne wrote:
> > This functionality is going to reside in vpci.c (and the corresponding
> > vpci.h header), and should be arch-agnostic. The handlers introduced
> > in this patch setup the basic functionality required in order to trap
> > accesses to the PCI config space, and allow decoding the address and
> > finding the corresponding handler that should handle the access
> > (although no handlers are implemented).
> 
> If I understand correctly this patch, the virtual BDF will always correspond
> to the physical BDF. Is that right? If so, would you mind to explain why
> such restriction?

Yes, this is not a limitation of this patch, but of the implementation
that follows. Likely this will be expanded when support for DomU is
added, but for Dom0 at least on x86 adding such a translation layer is
not needed, since I see no reason to present a different PCI topology
to Dom0.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Julien Grall]

Hi Roger,

On 14/08/17 15:28, Roger Pau Monne wrote:

This functionality is going to reside in vpci.c (and the corresponding
vpci.h header), and should be arch-agnostic. The handlers introduced
in this patch setup the basic functionality required in order to trap
accesses to the PCI config space, and allow decoding the address and
finding the corresponding handler that should handle the access
(although no handlers are implemented).


If I understand correctly this patch, the virtual BDF will always 
correspond to the physical BDF. Is that right? If so, would you mind to 
explain why such restriction?


Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Jan Beulich]

>>> On 08.09.17 at 16:41,  wrote:
> On Mon, Sep 04, 2017 at 09:38:11AM -0600, Jan Beulich wrote:
>> >>> On 14.08.17 at 16:28,  wrote:
>> > +/*
>> > + * At this point we have the following layout:
>> > + *
>> > + * Note that this refers to the position of the variables,
>> > + * but the value has already changed from the one given at
>> > + * initialization time because write tests have been performed.
>> > + *
>> > + * 322416 8 0
>> > + *  +-+-+-+-+
>> > + *  |  r0   | 0
>> > + *  +-+-+-+-+
>> > + *  | r7  |  r6 |  r5 |/| 32
>> > + *  +-+-+-+-|
>> > + *  |///| 64
>> > + *  +---+---+
>> > + *  |///|r12| 96
>> > + *  +---+---+
>> > + * ...
>> > + *  / = empty.
>> 
>> Maybe better "unwritten"?
> 
> I've been thinking about this, and I'm not sure unwritten is better,
> in fact the test will write to this registers, it's just that there's
> no backing handlers so writes will be discarded and reads will return
> ~0.
> 
> So I think "empty" or maybe "unhandled" is more descriptive.

"unhandled" then please - registers can't possibly be empty imo.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Roger Pau Monné]

On Mon, Sep 04, 2017 at 09:38:11AM -0600, Jan Beulich wrote:
> >>> On 14.08.17 at 16:28,  wrote:
> > +/*
> > + * At this point we have the following layout:
> > + *
> > + * Note that this refers to the position of the variables,
> > + * but the value has already changed from the one given at
> > + * initialization time because write tests have been performed.
> > + *
> > + * 322416 8 0
> > + *  +-+-+-+-+
> > + *  |  r0   | 0
> > + *  +-+-+-+-+
> > + *  | r7  |  r6 |  r5 |/| 32
> > + *  +-+-+-+-|
> > + *  |///| 64
> > + *  +---+---+
> > + *  |///|r12| 96
> > + *  +---+---+
> > + * ...
> > + *  / = empty.
> 
> Maybe better "unwritten"?

I've been thinking about this, and I'm not sure unwritten is better,
in fact the test will write to this registers, it's just that there's
no backing handlers so writes will be discarded and reads will return
~0.

So I think "empty" or maybe "unhandled" is more descriptive.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Jan Beulich]

>>> On 07.09.17 at 13:30,  wrote:
> On Thu, Sep 07, 2017 at 03:06:50AM -0600, Jan Beulich wrote:
>> >>> On 06.09.17 at 17:40,  wrote:
>> > On Mon, Sep 04, 2017 at 09:38:11AM -0600, Jan Beulich wrote:
>> >> >>> On 14.08.17 at 16:28,  wrote:
>> >> > Changes since v4:
>> >> >[...]
>> >> > * Hypervisor code:
>> >> >[...]
>> >> >  - Constify the data opaque parameter of read handlers.
>> >> 
>> >> Is that a good idea? Such callbacks should generally be allowed to
>> >> modify their state even if the operation is just a read - think of a
>> >> private lock or statistics/debugging data to be updated.
>> > 
>> > Right now the consistency of the opaque data is kept by the global
>> > vpci lock, which I like because it makes the code simpler. If the
>> > opaque data is not constified for the read handlers then I would be
>> > against using a read/write lock.
>> > 
>> > Statistic/debug data IMHO should be kept in a separate structure with
>> > it's own lock, that's referenced by the opaque data. This would allow
>> > Xen to not allocate this for non-debug builds, reducing memory
>> > footprint and lock contention in production.
>> 
>> I don't like this, as it makes adding such transiently needlessly
>> hard (as one would need to drop all the const-s or cast them away).
> 
> Hm, I'm not sure I follow. I was thinking of something along the lines
> of:
> 
> struct vpci_msi_stats {...};
> 
> struct vpci_msi {
> [...]
> struct vpci_msi_stats *stats;
> };
> 
> Then you can easily have a handler like:
> 
> static void vpci_msi_reg(..., const void *data)
> {
> const struct vpci_msi *msi = data;
> struct vpci_msi_stats *stats = msi->stats;
> [...]
> }
> 
> That should work AFAICT.

Sure, but the structure needs allocating. Which again is something
I wouldn't want to have to worry about when adding _temporary_
debugging or statistics code. But this is all moot anyway with ...

>> Would it be an
>> alternative to make the (spin) lock per-device rather than per-
>> domain? That might also be a good idea for pass-through (as there
>> Dom0 as well as the owning DomU fundamentally have access to
>> the config space of a device, and they'd better be synchronized).
> 
> That would also work, then I agree it should be a spin lock and the
> const from the read handlers can be dropped. Unless you say otherwise
> I'm going to implement this.

... this.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Roger Pau Monné]

On Thu, Sep 07, 2017 at 03:06:50AM -0600, Jan Beulich wrote:
> >>> On 06.09.17 at 17:40,  wrote:
> > On Mon, Sep 04, 2017 at 09:38:11AM -0600, Jan Beulich wrote:
> >> >>> On 14.08.17 at 16:28,  wrote:
> >> > Changes since v4:
> >> >[...]
> >> > * Hypervisor code:
> >> >[...]
> >> >  - Constify the data opaque parameter of read handlers.
> >> 
> >> Is that a good idea? Such callbacks should generally be allowed to
> >> modify their state even if the operation is just a read - think of a
> >> private lock or statistics/debugging data to be updated.
> > 
> > Right now the consistency of the opaque data is kept by the global
> > vpci lock, which I like because it makes the code simpler. If the
> > opaque data is not constified for the read handlers then I would be
> > against using a read/write lock.
> > 
> > Statistic/debug data IMHO should be kept in a separate structure with
> > it's own lock, that's referenced by the opaque data. This would allow
> > Xen to not allocate this for non-debug builds, reducing memory
> > footprint and lock contention in production.
> 
> I don't like this, as it makes adding such transiently needlessly
> hard (as one would need to drop all the const-s or cast them away).

Hm, I'm not sure I follow. I was thinking of something along the lines
of:

struct vpci_msi_stats {...};

struct vpci_msi {
[...]
struct vpci_msi_stats *stats;
};

Then you can easily have a handler like:

static void vpci_msi_reg(..., const void *data)
{
const struct vpci_msi *msi = data;
struct vpci_msi_stats *stats = msi->stats;
[...]
}

That should work AFAICT.

> What was the reason for switching to the rwlock anyway? Did you
> measure any performance problems? Are there Dom0 kernels not
> serializing PCI config space accesses anyway?

Not that I know of, but the PCIe spec doesn't seem to require non
concurrent accesses. PCI of course must not be concurrent.

> Would it be an
> alternative to make the (spin) lock per-device rather than per-
> domain? That might also be a good idea for pass-through (as there
> Dom0 as well as the owning DomU fundamentally have access to
> the config space of a device, and they'd better be synchronized).

That would also work, then I agree it should be a spin lock and the
const from the read handlers can be dropped. Unless you say otherwise
I'm going to implement this.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Jan Beulich]

>>> On 06.09.17 at 17:40,  wrote:
> On Mon, Sep 04, 2017 at 09:38:11AM -0600, Jan Beulich wrote:
>> >>> On 14.08.17 at 16:28,  wrote:
>> > Changes since v4:
>> >[...]
>> > * Hypervisor code:
>> >[...]
>> >  - Constify the data opaque parameter of read handlers.
>> 
>> Is that a good idea? Such callbacks should generally be allowed to
>> modify their state even if the operation is just a read - think of a
>> private lock or statistics/debugging data to be updated.
> 
> Right now the consistency of the opaque data is kept by the global
> vpci lock, which I like because it makes the code simpler. If the
> opaque data is not constified for the read handlers then I would be
> against using a read/write lock.
> 
> Statistic/debug data IMHO should be kept in a separate structure with
> it's own lock, that's referenced by the opaque data. This would allow
> Xen to not allocate this for non-debug builds, reducing memory
> footprint and lock contention in production.

I don't like this, as it makes adding such transiently needlessly
hard (as one would need to drop all the const-s or cast them away).
What was the reason for switching to the rwlock anyway? Did you
measure any performance problems? Are there Dom0 kernels not
serializing PCI config space accesses anyway? Would it be an
alternative to make the (spin) lock per-device rather than per-
domain? That might also be a good idea for pass-through (as there
Dom0 as well as the owning DomU fundamentally have access to
the config space of a device, and they'd better be synchronized).

>> I also have to admit that I find it strange that
>> you talk of zeroing something here - the net effect of the function
>> is not producing any zeros anywhere afaict. Such a pre-function
>> comment is normally describing the effect of the function as seen
>> to the caller rather than its inner workings.
> 
> OK, would it be better to write it as:
> 
> /*
>  * Merge new data into a partial result.
>  *
>  * Copy the value found in 'new' from [0, size) left shifted by
>  * 'offset' into 'data'.
>  */

Yes.

>> > + */
>> > +typedef uint32_t vpci_read_t(struct pci_dev *pdev, unsigned int reg,
>> > + const void *data);
>> > +
>> > +typedef void vpci_write_t(struct pci_dev *pdev, unsigned int reg,
>> > +  uint32_t val, void *data);
>> 
>> Do these two really need access to the struct pci_dev, rather than
>> just struct vpci? And if they do, then are they really permitted to
>> alter that struct pci_dev?
> 
> I'm leaning towards pdev because it already contains vpci.

Well, I certainly guessed that to be the reason for the way things
are now. But what data in the physical device structure would such
a handler possibly require? Oh, looking at later patches, you take
SBDF from there. That's certainly a good enough reason then.

> AFAICT it should be fine to constify it.

In which case please do.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Roger Pau Monné]

On Mon, Sep 04, 2017 at 09:38:11AM -0600, Jan Beulich wrote:
> >>> On 14.08.17 at 16:28,  wrote:
> > Changes since v4:
> >[...]
> > * Hypervisor code:
> >[...]
> >  - Constify the data opaque parameter of read handlers.
> 
> Is that a good idea? Such callbacks should generally be allowed to
> modify their state even if the operation is just a read - think of a
> private lock or statistics/debugging data to be updated.

Right now the consistency of the opaque data is kept by the global
vpci lock, which I like because it makes the code simpler. If the
opaque data is not constified for the read handlers then I would be
against using a read/write lock.

Statistic/debug data IMHO should be kept in a separate structure with
it's own lock, that's referenced by the opaque data. This would allow
Xen to not allocate this for non-debug builds, reducing memory
footprint and lock contention in production.

> > +/*
> > + * Merge new data into a partial result.
> > + *
> > + * Zero the bytes of 'data' from [offset, offset + size), and
> > + * merge the value found in 'new' from [0, offset) left shifted
> 
> DYM [0, size) here?

Yes, will fix.

> I also have to admit that I find it strange that
> you talk of zeroing something here - the net effect of the function
> is not producing any zeros anywhere afaict. Such a pre-function
> comment is normally describing the effect of the function as seen
> to the caller rather than its inner workings.

OK, would it be better to write it as:

/*
 * Merge new data into a partial result.
 *
 * Copy the value found in 'new' from [0, size) left shifted by
 * 'offset' into 'data'.
 */

> > + */
> > +typedef uint32_t vpci_read_t(struct pci_dev *pdev, unsigned int reg,
> > + const void *data);
> > +
> > +typedef void vpci_write_t(struct pci_dev *pdev, unsigned int reg,
> > +  uint32_t val, void *data);
> 
> Do these two really need access to the struct pci_dev, rather than
> just struct vpci? And if they do, then are they really permitted to
> alter that struct pci_dev?

I'm leaning towards pdev because it already contains vpci. AFAICT it
should be fine to constify it.

Thanks, Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Jan Beulich]

>>> On 14.08.17 at 16:28,  wrote:
> Changes since v4:
>[...]
> * Hypervisor code:
>[...]
>  - Constify the data opaque parameter of read handlers.

Is that a good idea? Such callbacks should generally be allowed to
modify their state even if the operation is just a read - think of a
private lock or statistics/debugging data to be updated.

> --- /dev/null
> +++ b/tools/tests/vpci/main.c
> @@ -0,0 +1,314 @@
> +/*
> + * Unit tests for the generic vPCI handler code.
> + *
> + * Copyright (C) 2017 Citrix Systems R
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms and conditions of the GNU General Public
> + * License, version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this program; If not, see 
> .
> + */
> +
> +#include "emul.h"
> +
> +/* Single vcpu (current), and single domain with a single PCI device. */
> +static struct vpci vpci;
> +
> +static struct domain d = {
> +.lock = false,

UNLOCKED ?

> +int
> +main(int argc, char **argv)
> +{
> +/* Index storage by offset. */
> +uint32_t r0 = 0xdeadbeef;
> +uint8_t r5 = 0xef;
> +uint8_t r6 = 0xbe;
> +uint8_t r7 = 0xef;
> +uint16_t r12 = 0x8696;
> +uint8_t r16[4] = { };
> +uint16_t r20[2] = { };
> +uint32_t r24 = 0;
> +uint8_t r28, r30;
> +unsigned int i;
> +int rc;
> +
> +INIT_LIST_HEAD();
> +
> +VPCI_ADD_REG(vpci_read32, vpci_write32, 0, 4, r0);
> +VPCI_READ_CHECK(0, 4, r0);
> +VPCI_WRITE_CHECK(0, 4, 0xbcbcbcbc);
> +
> +VPCI_ADD_REG(vpci_read8, vpci_write8, 5, 1, r5);
> +VPCI_READ_CHECK(5, 1, r5);
> +VPCI_WRITE_CHECK(5, 1, 0xba);
> +
> +VPCI_ADD_REG(vpci_read8, vpci_write8, 6, 1, r6);
> +VPCI_READ_CHECK(6, 1, r6);
> +VPCI_WRITE_CHECK(6, 1, 0xba);
> +
> +VPCI_ADD_REG(vpci_read8, vpci_write8, 7, 1, r7);
> +VPCI_READ_CHECK(7, 1, r7);
> +VPCI_WRITE_CHECK(7, 1, 0xbd);
> +
> +VPCI_ADD_REG(vpci_read16, vpci_write16, 12, 2, r12);
> +VPCI_READ_CHECK(12, 2, r12);
> +VPCI_READ_CHECK(12, 4, 0x8696);
> +
> +/*
> + * At this point we have the following layout:
> + *
> + * Note that this refers to the position of the variables,
> + * but the value has already changed from the one given at
> + * initialization time because write tests have been performed.
> + *
> + * 322416 8 0
> + *  +-+-+-+-+
> + *  |  r0   | 0
> + *  +-+-+-+-+
> + *  | r7  |  r6 |  r5 |/| 32
> + *  +-+-+-+-|
> + *  |///| 64
> + *  +---+---+
> + *  |///|r12| 96
> + *  +---+---+
> + * ...
> + *  / = empty.

Maybe better "unwritten"?

> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -76,6 +76,11 @@ SECTIONS
>  
>__2M_rodata_start = .;   /* Start of 2M superpages, mapped RO. */
>.rodata : {
> +#if defined(CONFIG_HAS_PCI) && defined(CONFIG_LATE_HWDOM)
> +   __start_vpci_array = .;
> +   *(.rodata.vpci)
> +   __end_vpci_array = .;
> +#endif
> _srodata = .;

Please don't put anything ahead of this label. And I'd prefer if
ordinary .rodata contributions came first, i.e. please don't follow
the bad example ...

> /* Bug frames table */
> __start_bug_frames = .;

... this gives (there are plenty of good examples further down in
this section).

> @@ -167,6 +172,11 @@ SECTIONS
> _einittext = .;
>} :text
>.init.data : {
> +#if defined(CONFIG_HAS_PCI) && !defined(CONFIG_LATE_HWDOM)
> +   __start_vpci_array = .;
> +   *(.init.rodata.vpci)
> +   __end_vpci_array = .;
> +#endif
> *(.init.rodata)

Same here then.

> +int vpci_add_register(const struct pci_dev *pdev, vpci_read_t *read_handler,
> +  vpci_write_t *write_handler, unsigned int offset,
> +  unsigned int size, void *data)
> +{
> +struct list_head *prev;
> +struct vpci_register *r;
> +
> +/* Some sanity checks. */
> +if ( (size != 1 && size != 2 && size != 4) ||
> + offset >= PCI_CFG_SPACE_EXP_SIZE || (offset & (size - 1)) ||
> + (!read_handler && !write_handler) )
> +return -EINVAL;
> +
> +r = xmalloc(struct vpci_register);
> +if ( !r )
> +return -ENOMEM;
> +
> +r->read = read_handler ?: vpci_ignored_read;
> +r->write = write_handler ?: vpci_ignored_write;
> +r->size = size;
> +r->offset = offset;
> +r->private = data;
> +
> +

Re: [Xen-devel] [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to the PCI config space

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monne [mailto:roger@citrix.com]
> Sent: 14 August 2017 15:29
> To: xen-de...@lists.xenproject.org
> Cc: boris.ostrov...@oracle.com; konrad.w...@oracle.com; Roger Pau Monne
> ; Ian Jackson ; Wei Liu
> ; Jan Beulich ; Andrew Cooper
> ; Paul Durrant 
> Subject: [PATCH v5 02/11] vpci: introduce basic handlers to trap accesses to
> the PCI config space
> 
> This functionality is going to reside in vpci.c (and the corresponding
> vpci.h header), and should be arch-agnostic. The handlers introduced
> in this patch setup the basic functionality required in order to trap
> accesses to the PCI config space, and allow decoding the address and
> finding the corresponding handler that should handle the access
> (although no handlers are implemented).
> 
> Note that the traps to the PCI IO ports registers (0xcf8/0xcfc) are
> setup inside of a x86 HVM file, since that's not shared with other
> arches.
> 
> A new XEN_X86_EMU_VPCI x86 domain flag is added in order to signal Xen
> whether a domain should use the newly introduced vPCI handlers, this
> is only enabled for PVH Dom0 at the moment.
> 
> A very simple user-space test is also provided, so that the basic
> functionality of the vPCI traps can be asserted. This has been proven
> quite helpful during development, since the logic to handle partial
> accesses or accesses that expand across multiple registers is not
> trivial.
> 
> The handlers for the registers are added to a linked list that's keep
> sorted at all times. Both the read and write handlers support accesses
> that expand across multiple emulated registers and contain gaps not
> emulated.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Paul Durrant 
> ---
> Changes since v4:
> * User-space test harness:
>  - Do not redirect the output of the test.
>  - Add main.c and emul.h as dependencies of the Makefile target.
>  - Use the same rule to modify the vpci and list headers.
>  - Remove underscores from local macro variables.
>  - Add _check suffix to the test harness multiread function.
>  - Change the value written by every different size in the multiwrite
>test.
>  - Use { } to initialize the r16 and r20 arrays (instead of { 0 }).
>  - Perform some of the read checks with the local variable directly.
>  - Expand some comments.
>  - Implement a dummy rwlock.
> * Hypervisor code:
>  - Guard the linker script changes with CONFIG_HAS_PCI.
>  - Rename vpci_access_check to vpci_access_allowed and make it return
>bool.
>  - Make hvm_pci_decode_addr return the register as return value.
>  - Use ~3 instead of 0xfffc to remove the register offset when
>checking accesses to IO ports.
>  - s/head/prev in vpci_add_register.
>  - Add parentheses around & in vpci_add_register.
>  - Fix register removal.
>  - Change the BUGs in vpci_{read/write}_hw helpers to
>ASSERT_UNREACHABLE.
>  - Make merge_result static and change the computation of the mask to
>avoid using a uint64_t.
>  - Modify vpci_read to only read from hardware the not-emulated gaps.
>  - Remove the vpci_val union and use a uint32_t instead.
>  - Change handler read type to return a uint32_t instead of modifying
>a variable passed by reference.
>  - Constify the data opaque parameter of read handlers.
>  - Change the size parameter of the vpci_{read/write} functions to
>unsigned int.
>  - Place the array of initialization handlers in init.rodata or
>.rodata depending on whether late-hwdom is enabled.
>  - Remove the pci_devs lock, assume the Dom0 is well behaved and won't
>remove the device while trying to access it.
>  - Change the recursive spinlock into a rw lock for performance
>reasons.
> 
> Changes since v3:
> * User-space test harness:
>  - Fix spaces in container_of macro.
>  - Implement a dummy locking functions.
>  - Remove 'current' macro make current a pointer to the statically
>allocated vpcu.
>  - Remove unneeded parentheses in the pci_conf_readX macros.
>  - Fix the name of the write test macro.
>  - Remove the dummy EXPORT_SYMBOL macro (this was needed by the RB
>code only).
>  - Import the max macro.
>  - Test all possible read/write size combinations with all possible
>emulated register sizes.
>  - Introduce a test for register removal.
> * Hypervisor code:
>  - Use a sorted list in order to store the config space handlers.
>  - Remove some unneeded 'else' branches.
>  - Make the IO port handlers always return X86EMUL_OKAY, and set the
>data to all 1's in case of read failure (write are simply ignored).
>  - In hvm_select_ioreq_server reuse local variables when calling
>