On 05/02/2016 05:38 PM, Ladi Prosek wrote:
On Mon, May 2, 2016 at 4:21 PM, Marcel Apfelbaum <mar...@redhat.com> wrote:
On 05/02/2016 04:11 PM, Ladi Prosek wrote:
Some of the regions may end up being unmapped, either because
they are optional or because the attempt to map them has failed.
Region types starting at 0 didn't make it easy to test for this
condition.
This commit adds VIRTIO_PCI_REGION_NONE with the value of 0
and bumps the valid region types up by 1.
Signed-off-by: Ladi Prosek <lpro...@redhat.com>
---
src/include/ipxe/virtio-pci.h | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/include/ipxe/virtio-pci.h b/src/include/ipxe/virtio-pci.h
index c7452c8..ec8efaa 100644
--- a/src/include/ipxe/virtio-pci.h
+++ b/src/include/ipxe/virtio-pci.h
@@ -106,12 +106,14 @@ struct virtio_pci_region {
/* How to interpret the base field */
#define VIRTIO_PCI_REGION_TYPE_MASK 0x00000003
+/* The region is unused */
+#define VIRTIO_PCI_REGION_NONE 0x00000000
/* The base field is a memory address */
-#define VIRTIO_PCI_REGION_MEMORY 0x00000000
+#define VIRTIO_PCI_REGION_MEMORY 0x00000001
/* The base field is a port address */
-#define VIRTIO_PCI_REGION_PORT 0x00000001
+#define VIRTIO_PCI_REGION_PORT 0x00000002
/* The base field is an offset within the PCI bar */
-#define VIRTIO_PCI_REGION_PCI_CONFIG 0x00000002
+#define VIRTIO_PCI_REGION_PCI_CONFIG 0x00000003
Is OK if you are not interested to use the flags as bits.
Otherwise you may want to give VIRTIO_PCI_REGION_PCI_CONFIG the value 4.
Yes, I just wanted to reserve 2 bits out of the flags field for this.
The flags are exclusive so no need to combine them and assign one bit
to each.
And I also didn't see you used the new VIRTIO_PCI_REGION_NONE flag.
I suggest to wait with this patch until you will actually use it.
Patch 3/3 has a dependency on the new numbers in
virtnet_uses_region_type as vdev->device may not be mapped and flags
be 0. The old numbers were chosen pretty unfortunately (my fault) so
you couldn't tell an unused region and a memory region apart.
This I understood.
You're right that VIRTIO_PCI_REGION_NONE is not used. I included it
for completeness, to be explicit about what 0 means. I can remove it
if you'd prefer that.
I wouldn't add it if is not used. People searching for it will get
angry/frustrated and git-blame will point to you ... :)
0 as a "disabled" value should be enough until you will actually
use it, then indeed is a good decision.
All of the above IMHO.
Thanks,
Marcel
Thanks,
Ladi
Thanks,
Marcel
unsigned flags;
};
_______________________________________________
ipxe-devel mailing list
ipxe-devel@lists.ipxe.org
https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel
