Re: [PATCH v2] QEMU fw_cfg DMA interface documentation
On Mon, Sep 07, 2015 at 01:08:29PM +0200, Gerd Hoffmann wrote: > > It's just simplicity. If you want to read a few times from the same > > field (like in ACPI tables, read the data size and then the data), you > > need a way to enable and disable the selector and manage the current > > offset for that entry. This is already provided with the "old" > > interface. > > Could be handled with a 'select' control bit. Only when set select > entry and reset offset to zero. I think two features would help "round off" the new fw_cfg DMA proposal: add a select bit as you describe (that uses the 16 most significant bits of the "control" field for the "select entry" when the bit is set), and define a static signature (eg, "QEMU CFG") when reading the 64bit MMIO dma register. Both are optional features that don't change the fundamental interface; I was thinking of sending them as two patches on top of Marc's next version of his patch series (if no one else gets to it first). -Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: QEMU fw_cfg DMA interface
On Thu, Aug 06, 2015 at 02:37:45PM +0200, Marc Marí wrote: > On Thu, 6 Aug 2015 13:27:16 +0100 > Stefan Hajnoczi wrote: > > > On Thu, Aug 6, 2015 at 12:00 PM, Marc Marí wrote: > > > When running a Linux guest on top of QEMU, using the -kernel > > > options, this is the timing improvement for x86: > > > > > > QEMU commit 2be4f242b50a8 and SeaBIOS commit 908a58c1d5ff > > > QEMU startup time: .078 > > > BIOS startup time: .060 > > > Kernel setup time: .578 > > > Total time: .716 > > > > > > QEMU with this patch series and SeaBIOS with this patch series > > > QEMU startup time: .080 > > > BIOS startup time: .039 > > > Kernel setup time: .002 > > > Total time: .121 > > > > Impressive results! > > > > Is this a fully-featured QEMU build or did you disable things? > > > > Is this the default SeaBIOS build or did you disable things? > > > > This is the default QEMU configuration I get for my system. It's not a > fully-featured QEMU, but it has a lot of things enabled. SeaBIOS > has a default configuration (with debugging disabled). Thanks! What qemu command-line did you use during testing? Also, do you have a quick primer on how to use the kvm trace stuff to obtain timings? -Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 02/12] syslets: add syslet.h include file, user API/ABI definitions
On Wed, Feb 28, 2007 at 10:41:17PM +0100, Ingo Molnar wrote:
> From: Ingo Molnar <[EMAIL PROTECTED]>
>
> add include/linux/syslet.h which contains the user-space API/ABI
> declarations. Add the new header to include/linux/Kbuild as well.
Hi Ingo,
I'd like to propose a simpler userspace API for syslets. I believe
this revised API is just as capable as yours (anything done purely in
kernel space with the existing API can also be done with this one).
An "atom" would look like:
struct syslet_uatom {
u32 nr;
u64 ret_ptr;
u64 next;
u64 arg_nr;
u64 args[6];
};
The sys_nr, ret_ptr, and next fields would be unchanged. The args
array would directly store the arguments to the system call. To
optimize the case where only a few arguments are necessary, an
explicit argument count would be set in the arg_nr field.
The above is very similar to what Linus and Davide described as a
"single submission" syslet interface - it differs only with the
addition of the next parameter. As with your API, a null next field
would immediately stop the syslet. So, a user wishing to run a single
system call asynchronously could use the above interface with the next
field set to null.
Of course, the above lacks the syscall return testing capabilities in
your atoms. To obtain that capability, one could add a new syscall:
long sys_syslet_helper(long flags, long *ptr, long inc, u64 new_next)
The above is effectively a combination of sys_umem_add and the "flags"
field from the existing syslet_uatom. The system call would only be
available from syslets. It would add "inc" to the integer stored in
"ptr" and return the result. The "flags" field could optionally
contain one of:
SYSLET_BRANCH_ON_NONZERO
SYSLET_BRANCH_ON_ZERO
SYSLET_BRANCH_ON_NEGATIVE
SYSLET_BRANCH_ON_NON_POSITIVE
If the flag were set and the return result of the syscall met the
specified condition, then the code would arrange for the calling
syslet to branch to "new_next" instead of the normal "next".
I would also change the event ring notification system. Instead of
building that support into all syslets, one could introduce an "add to
head" syscall specifically for that purpose. If done this way,
userspace could arrange for this new sys_addtoring call to always be
the last uatom executed in a syslet. This would make the support
optional - those userspace applications that prefer to use a futex or
signal as an event system could arrange to have those system calls as
the last one in the chain instead. With this change, the
sys_async_exec would simplify to:
long sys_async_exec(struct syslet_uatom *uatom);
As above, I believe this API has as much power as the existing system.
The general idea is to make the system call / atoms simpler and use
more atoms when building complex chains.
For example, the open & stat case could be done with a chain like the
following:
atom1: &atom3->args[1] = sys_open(...)
atom2: sys_syslet_helper(SYSLET_BRANCH_ON_NON_POSITIVE,
&atom3->args[1], 0, atom4)
atom3: sys_stat([arg1 filled above], ...)
atom4: sys_futex(...) // alert parent of completion
It is also possible to use sys_syslet_helper to push a return value to
multiple syslet parameters (for example, propagating an fd from open
to multiple reads). For example:
atom1: &atom3->args[1] = sys_open(...)
atom2: &atom4->args[1] = sys_syslet_helper(0, &atom3->args[1], 0, 0)
atom3: sys_read([arg1 filled in atom1], ...)
atom4: sys_read([arg1 filled in atom2], ...)
...
Although this is a bit ugly, I must wonder how many times one would
build chains complex enough to require it.
Cheers,
-Kevin
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 0/6] arch-neutral GPIO calls
Hi Dave, On Mon, Jan 01, 2007 at 12:06:19PM -0800, David Brownell wrote: > > The concern I have with your current implementation is that I don't > > see a way to flexibly add in support for additional gpio pins on a > > machine by machine basis. The code does do a good job of abstracting > > gpios based on the primary architecture (eg, PXA vs OMAP), but not on > > a chip basis (eg, PXA vs ASIC3). > > You used the key word: implementation. The interface allows it, > but such board-specific extensions haven't yet been needed; so > they've not yet been implemented. > > See the appended for a patch roughly along the lines of what has > previously been discussed here. No interface change, just updated > implementation code. And the additional implementation logic won't > kick on boards that don't need it. Thanks for enlightening me on previous discussions. The interface in the "gpiolib" patch is along the lines of what I'm looking for. However, I still feel this approach is backwards. Going from a 'struct gpio' to an 'int gpio' is easy. Attempting to go from an 'int gpio' to a 'struct gpio' is hard, as the code you provide demonstrates. Can you help explain what the gain to using an integer over a 'struct gpio' is? I can't help but feel that fundamentally gpios are not integers and that it is a mistake to code an API around that concept. > Note that the current implementations are a win even without yet > being able to handle the board-specific external GPIO controllers. > The API is clearer than chip-specific register access, and since > it's arch-neutral it already solves integration problems (like > having one SPI controller driver work on both AT91 and AVR). I agree that the code you provide is a big step up from the existing code. We also both seem to agree that more than just an arch abstraction is necessary (the cansleep stuff seems to be an acknowledgment of this). However, I don't understand the downside to making the API sane from the start and avoiding the artificial integer namespace problems. > Note that I see that kind of update as happening after the first round > of patches go upstream: accept the interface first, then update boards > to support it ... including later the cansleep calls, on some boards. Once many of the problems are fixed, I fear fixing the remaining will be a harder sell. Due to the difficulty of implementing kernel wide APIs, I suspect some maintainers will just stick to cramming features in the arch directory (as your patch encourages) which can lead to duplicated code and fragmented implementations. Thanks for working on this. -Kevin - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.20-rc1 0/6] arch-neutral GPIO calls
> Based on earlier discussion, I'm sending a refresh of the generic GPIO
> patch, with several (ARM based) implementations in separate patches:
Hi Dave,
I'm very interested in seeing an abstraction for gpios. Over the last
several months, I've been working on getting Linux running on my phone
- see www.handhelds.org for more information. These types of ARM
based PDAs and phones are riddled with GPIOs, and there is frequent
code duplication when two machines have to do similar functionality
with different gpios.
Unfortunately, I fear the implementation you propose is not robust
enough to handle the cases I need to handle. In particular, I think a
"struct gpio" instead of "int gpio" is needed to describe a gpio pin.
Some background - on the phone I use, it has the standard PXA27x
gpios, a gpio extender chip (egpio) which is connected via the memory
bus, and a micro-controller attached via an i2c like bus. Other
phones will commonly have the PXA27x, "egpio", and an "asic3" chip.
With the sale of the PXA line, it seems Samsung and TI based ARM chips
(and their corresponding GPIOs) will be on the rise.
As you know, basic functionality like setting a LED, detecting a
button press, requesting an irq, etc. are common with GPIOs regardless
of which platform, or which gpio chip, is in use.
The concern I have with your current implementation is that I don't
see a way to flexibly add in support for additional gpio pins on a
machine by machine basis. The code does do a good job of abstracting
gpios based on the primary architecture (eg, PXA vs OMAP), but not on
a chip basis (eg, PXA vs ASIC3).
Specifically, once the code pulls in "" it will get the
PXA gpio code, but this will only allow access to the arch gpios, not
the machine specific gpios. Also, one of the goals of the developers
at handhelds.org is to have one kernel for many different phones --
from a userspace point of view they don't generally differ very much.
As such, this isn't a matter of just having each "machine" override
the gpio.h file at compile time; it really needs to be done at
runtime.
I understand that the existing code works entirely on integers.
However, I fear this is at attribute of the problem, not of the
solution.
> - Core patch, doc + +
> - OMAP implementation
> - AT91 implementation
> - PXA implementation
> - SA1100 implementation
> - S3C2410 implementation
Your patch clearly shows that the existing implementations are using
integers. I think there is a simple way to reuse the existing
implementations. For example on pxa one could do something like:
= include/linux/gpio.h
struct gpio_ops {
int (*gpio_direction_input)(struct *gpio);
...
};
struct gpio {
struct gpio_ops *ops;
};
= arch/arm/mach-pxa/gpio.c
struct gpio pxa_gpios[120] = {
{.ops = pxa_gpio_ops}, ...
};
int pxa_gpio_direction_input(struct *gpio) {
int pxa_gpio = gpio - pxa_gpios;
pxa_gpio_mode(pxa_gpio | GPIO_IN);
return 0;
}
...
> Other than clarifications, the main change in the doc is defining
> new calls safe for use with GPIOs on things like pcf8574 I2C gpio
> expanders; those new calls can sleep, but are otherwise the same as
> the spinlock-safe versions. The implementations above implement that
> as a wrapper (the asm-generic header) around the spinlock-safe calls.
As above, I'm confused how these expanders would work in practice.
The expanders would be present on a machine by machine basis but the
code seems to be implemented on an arch by arch basis. Perhaps an
example would help me.
Please CC me on replies.
Cheers,
-Kevin
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
