[PATCH] - hurd pci-arbiter: remove embedded pciaccess code

[Damien Zammit]

Hi all,

This patch (see attached) removes all embedded pciaccess code from the
arbiter, and adds config.h which is currently missing from master.

NB: I have another patch for libpciaccess to add the hurd method and add
Joan's tweaks to the raw x86 access method, but is not strictly
essential to upstream that to have the arbiter in a working state.

The arbiter works ok with just this patch on top of master since
libpciaccess uses the raw x86 method for hurd currently.

However, to get regionX access and rom access, as well as the "real"
hurdish pci access method into pciaccess, I will need to upstream the
pciaccess stuff.

Also, to make everything work smoothly in the future, I have a patch for
gnumach to restrict the number of processes that can simultaneously
access pci io cfg range of ports down to 1 as per discussion on the xorg
mailing list. I will make a new post with the gnumach patch.

Cheers,
Damien

[Samuel Thibault]

Hello,

Thanks for this :)

@@ -35,6 +35,8 @@ include ../Makeconf
CFLAGS += -I$(PORTDIR)/include
+CPPFLAGS += -imacros $(srcdir)/config.h

Why is this needed?

(that's the reason why you have to add a config.h file, but its content
is completely useless).

@@ -85,10 +86,10 @@ io_config_file (struct pci_device * dev, off_t offset, size_t * len,
assert_backtrace (dev != 0);
/* Don't exceed the config space size */
- if (offset > dev->config_size)
+ if (offset > PCI_CONFIG_SIZE)
return EINVAL;
--- a/pci-arbiter/pcifs.h
+++ b/pci-arbiter/pcifs.h
@@ -27,8 +27,11 @@
+// FIXME: Hardcoded PCI config size
+#define PCI_CONFIG_SIZE 256

So it seems that libpciaccess doesn't provide an interface for this.
Please leave as TODO in config_block_op to just return what could be
read once op() returns EIO, so that we do get the short reads
implemented by just leaving it up to libpciaccess. Limiting to 256 bytes
will be fine for now.

@@ -85,7 +85,7 @@ S_pci_conf_read (struct protid * master, int reg, char **data,
error_t err;
pthread_mutex_t *lock;
struct pcifs_dirent *e;
-
+
if (!master)
return EOPNOTSUPP;

Please avoid such unneeded hunk :)

@@ -266,7 +276,7 @@ create_fs_tree (struct pcifs * fs, struct pci_system * pci_sys)
e_stat = func_parent->stat;
e_stat.st_mode &= ~(S_IFDIR | S_IXUSR | S_IXGRP);
e_stat.st_mode |= S_IFREG | S_IWUSR | S_IWGRP;
- e_stat.st_size = device->config_size;
+ e_stat.st_size = PCI_CONFIG_SIZE; // FIXME: Hardcoded

Mmm. This will also be a TODO. I'd say on the long run libpciaccess
should really provide a way to get the config size instead of having to
just get EIO.

Samuel

[Damien Zammit]

Post by Samuel Thibault

+CPPFLAGS += -imacros $(srcdir)/config.h

Why is this needed?

I have now removed config.h and this line.

Post by Samuel Thibault
So it seems that libpciaccess doesn't provide an interface for this.
Please leave as TODO in config_block_op to just return what could be
read once op() returns EIO, so that we do get the short reads
implemented by just leaving it up to libpciaccess. Limiting to 256 bytes
will be fine for now.

I added this in the TODO file

Post by Samuel Thibault

@@ -85,7 +85,7 @@ S_pci_conf_read (struct protid * master, int reg, char **data,
-
+

Please avoid such unneeded hunk :)

Fixed

Post by Samuel Thibault

@@ -266,7 +276,7 @@ create_fs_tree (struct pcifs * fs, struct pci_system * pci_sys)
- e_stat.st_size = device->config_size;
+ e_stat.st_size = PCI_CONFIG_SIZE; // FIXME: Hardcoded

Mmm. This will also be a TODO. I'd say on the long run libpciaccess
should really provide a way to get the config size instead of having to
just get EIO.

Yes, it would be nice if libpciaccess provided the size of the config
space. I have just left this as is for now.

See attached for revised patch.

Damien

[Samuel Thibault]

Hello,

Thanks for the revised patch :)

Do you know how your copyright assignment is going, so I can commit it?

Samuel

[Damien Zammit]

Post by Samuel Thibault
Do you know how your copyright assignment is going, so I can commit it?

Haven't heard anything since I sent through my request.

Damien

[Thomas Schwinge]

Hi!

Post by Damien Zammit

Post by Samuel Thibault
Do you know how your copyright assignment is going, so I can commit it?

Haven't heard anything since I sent through my request.

This has yesterday been reported as completed in email with "Subject:
[gnu.org #1332514] Damien Zammit HURD MACH LIBC GRUB".

Thanks!


Grüße
Thomas