Hi
Here I'm sending some ia64 patches to make it work in the ski emulator.
This has been broken for a long time.
Mikulas
The ski emulator can't spawn any process with larger stack than
0x7ffff00000000.
Signed-off-by: Mikulas Patocka <[email protected]>
---
arch/ia64/include/asm/resource.h | 6 ++++++
1 file changed, 6 insertions(+)
Index: linux-2.6-ia64/arch/ia64/include/asm/resource.h
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/include/uapi/asm/resource.h 2013-04-20 19:14:30.000000000 +0200
+++ linux-2.6-ia64/arch/ia64/include/uapi/asm/resource.h 2013-04-20 20:59:00.000000000 +0200
@@ -2,6 +2,12 @@
#define _ASM_IA64_RESOURCE_H
#include <asm/ustack.h>
+
+#if defined(__KERNEL__) && defined(CONFIG_IA64_HP_SIM)
+/* The ski simulator can't spawn any process with larger stack size */
+#define _STK_LIM_MAX 0x7ffff00000000UL
+#endif
+
#include <asm-generic/resource.h>
#endif /* _ASM_IA64_RESOURCE_H */
This patch fixes the following bugs:
* Lockup when out of memory: If we are out of memory, we must actually
read the data and drop it. If we don't read the data, the interrupt is
still pending and the data are still in the emulator's queue. The guest
system locks up, processing the interrupt forever and trying to allocate
a sk_buff.
* Fix a memory leak. If the emulator has no more packets for us, it
returns zero. We need to free the allocated skbuf, otherwise it is
leaked.
* There was double warning when out of memory.
* Two nested loops on receive - not a bug, but they are useless.
The upper loop in simeth_interrupt is unconditional, it loops until some
data were read. The lower loop in simeth_rx loops SIMETH_RECV_MAX times.
I replaced them with just one loop in simeth_rx that loops as long as
some data were read.
* Removed some informational printks that happen under normal operation.
Signed-off-by: Mikulas Patocka <[email protected]>
---
arch/ia64/hp/sim/simeth.c | 41 ++++++++++++-----------------------------
1 file changed, 12 insertions(+), 29 deletions(-)
Index: linux-2.6-ia64/arch/ia64/hp/sim/simeth.c
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/hp/sim/simeth.c 2014-01-24 02:02:52.000000000 +0100
+++ linux-2.6-ia64/arch/ia64/hp/sim/simeth.c 2014-01-24 02:32:40.000000000 +0100
@@ -25,8 +25,6 @@
#include "hpsim_ssc.h"
-#define SIMETH_RECV_MAX 10
-
/*
* Maximum possible received frame for Ethernet.
* We preallocate an sk_buff of that size to avoid costly
@@ -48,7 +46,7 @@ static int simeth_probe1(void);
static int simeth_open(struct net_device *dev);
static int simeth_close(struct net_device *dev);
static int simeth_tx(struct sk_buff *skb, struct net_device *dev);
-static int simeth_rx(struct net_device *dev);
+static void simeth_rx(struct net_device *dev);
static struct net_device_stats *simeth_get_stats(struct net_device *dev);
static irqreturn_t simeth_interrupt(int irq, void *dev_id);
static void set_multicast_list(struct net_device *dev);
@@ -299,13 +297,9 @@ simeth_device_event(struct notifier_bloc
if (strcmp(dev->name, ifa->ifa_label) == 0) break;
}
if ( ifa == NULL ) {
- printk(KERN_ERR "simeth_open: can't find device %s's ifa\n", dev->name);
return NOTIFY_DONE;
}
- printk(KERN_INFO "simeth_device_event: %s ipaddr=0x%x\n",
- dev->name, ntohl(ifa->ifa_local));
-
/*
* XXX Fix me
* if the device was up, and we're simply reconfiguring it, not sure
@@ -318,9 +312,6 @@ simeth_device_event(struct notifier_bloc
netdev_attach(local->simfd, dev->irq, ntohl(ifa->ifa_local)):
netdev_detach(local->simfd);
- printk(KERN_INFO "simeth: netdev_attach/detach: event=%s ->%d\n",
- event == NETDEV_UP ? "attach":"detach", r);
-
return NOTIFY_DONE;
}
@@ -408,7 +399,6 @@ make_new_skb(struct net_device *dev)
*/
nskb = dev_alloc_skb(SIMETH_FRAME_SIZE + 2);
if ( nskb == NULL ) {
- printk(KERN_NOTICE "%s: memory squeeze. dropping packet.\n", dev->name);
return NULL;
}
@@ -422,34 +412,30 @@ make_new_skb(struct net_device *dev)
/*
* called from interrupt handler to process a received frame
*/
-static int
+static void
simeth_rx(struct net_device *dev)
{
struct simeth_local *local;
struct sk_buff *skb;
int len;
- int rcv_count = SIMETH_RECV_MAX;
+ static u8 oom_sink[SIMETH_FRAME_SIZE];
local = netdev_priv(dev);
- /*
- * the loop concept has been borrowed from other drivers
- * looks to me like it's a throttling thing to avoid pushing to many
- * packets at one time into the stack. Making sure we can process them
- * upstream and make forward progress overall
- */
- do {
+ while (1) {
if ( (skb=make_new_skb(dev)) == NULL ) {
printk(KERN_NOTICE "%s: memory squeeze. dropping packet.\n", dev->name);
- local->stats.rx_dropped++;
- return 0;
+ while (netdev_read(local->simfd, oom_sink, SIMETH_FRAME_SIZE))
+ local->stats.rx_dropped++;
+ return;
}
/*
* Read only one frame at a time
*/
len = netdev_read(local->simfd, skb->data, SIMETH_FRAME_SIZE);
if ( len == 0 ) {
- if ( simeth_debug > 0 ) printk(KERN_WARNING "%s: count=%d netdev_read=0\n",
- dev->name, SIMETH_RECV_MAX-rcv_count);
+ kfree_skb(skb);
+ if ( simeth_debug > 0 ) printk(KERN_WARNING "%s: netdev_read=0\n",
+ dev->name);
break;
}
#if 0
@@ -471,9 +457,7 @@ simeth_rx(struct net_device *dev)
local->stats.rx_packets++;
local->stats.rx_bytes += len;
- } while ( --rcv_count );
-
- return len; /* 0 = nothing left to read, otherwise, we can try again */
+ }
}
/*
@@ -487,7 +471,7 @@ simeth_interrupt(int irq, void *dev_id)
/*
* very simple loop because we get interrupts only when receiving
*/
- while (simeth_rx(dev));
+ simeth_rx(dev);
return IRQ_HANDLED;
}
@@ -503,7 +487,6 @@ simeth_get_stats(struct net_device *dev)
static void
set_multicast_list(struct net_device *dev)
{
- printk(KERN_WARNING "%s: set_multicast_list called\n", dev->name);
}
__initcall(simeth_probe);
Fix sleeping with interrupts disabled warning in simserial.c:
BUG: sleeping function called from invalid context at mm/slub.c:925
in_atomic(): 0, irqs_disabled(): 1, pid: 1, name: init
Call Trace:
[<a000000100011180>] show_stack+0x40/0x90
sp=e0000000070b7b40 bsp=e0000000070b12c8
[<a000000100563e20>] dump_stack+0x20/0x40
sp=e0000000070b7d10 bsp=e0000000070b12b0
[<a000000100090220>] __might_sleep+0x1e0/0x200
sp=e0000000070b7d10 bsp=e0000000070b1280
[<a00000010012f060>] kmem_cache_alloc+0x70/0x430
sp=e0000000070b7d10 bsp=e0000000070b1230
[<a0000001000af8c0>] request_threaded_irq+0x150/0x370
sp=e0000000070b7d20 bsp=e0000000070b11c0
[<a0000001003c3b30>] activate+0x180/0x350
sp=e0000000070b7d20 bsp=e0000000070b1188
[<a00000010033d2b0>] tty_port_open+0x1b0/0x280
sp=e0000000070b7d20 bsp=e0000000070b1140
[<a0000001003c42b0>] rs_open+0x150/0x1a0
sp=e0000000070b7d20 bsp=e0000000070b1118
[<a00000010032d930>] tty_open+0x790/0xb00
sp=e0000000070b7d20 bsp=e0000000070b1038
[<a000000100144d10>] chrdev_open+0x250/0x2d0
sp=e0000000070b7d30 bsp=e0000000070b0ff0
[<a000000100138b40>] do_dentry_open.isra.14+0x3d0/0x650
sp=e0000000070b7d40 bsp=e0000000070b0f80
[<a00000010013a8e0>] finish_open+0x90/0xc0
sp=e0000000070b7d40 bsp=e0000000070b0f48
[<a0000001001579d0>] do_last.isra.41+0x1490/0x18c0
sp=e0000000070b7d40 bsp=e0000000070b0e60
[<a000000100157f60>] path_openat.isra.42+0x160/0xa00
sp=e0000000070b7d60 bsp=e0000000070b0d58
[<a000000100158830>] do_filp_open+0x30/0xb0
sp=e0000000070b7d90 bsp=e0000000070b0d20
[<a00000010013b190>] do_sys_open+0x200/0x330
sp=e0000000070b7e20 bsp=e0000000070b0cb8
[<a00000010013b310>] sys_open+0x50/0x70
sp=e0000000070b7e30 bsp=e0000000070b0c60
[<a00000010000b340>] ia64_ret_from_syscall+0x0/0x20
sp=e0000000070b7e30 bsp=e0000000070b0c60
[<a000000000040720>] ia64_ivt+0xffffffff00040720/0x400
sp=e0000000070b8000 bsp=e0000000070b0c60
Signed-off-by: Mikulas Patocka <[email protected]>
---
arch/ia64/hp/sim/simserial.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
Index: linux-2.6-ia64/arch/ia64/hp/sim/simserial.c
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/hp/sim/simserial.c 2014-01-23 23:06:12.000000000 +0100
+++ linux-2.6-ia64/arch/ia64/hp/sim/simserial.c 2014-01-23 23:13:44.000000000 +0100
@@ -365,7 +365,6 @@ static int activate(struct tty_port *por
struct serial_state *state = container_of(port, struct serial_state,
port);
unsigned long flags, page;
- int retval = 0;
page = get_zeroed_page(GFP_KERNEL);
if (!page)
@@ -378,13 +377,6 @@ static int activate(struct tty_port *por
else
state->xmit.buf = (unsigned char *) page;
- if (state->irq) {
- retval = request_irq(state->irq, rs_interrupt_single, 0,
- "simserial", state);
- if (retval)
- goto errout;
- }
-
state->xmit.head = state->xmit.tail = 0;
/*
@@ -398,10 +390,16 @@ static int activate(struct tty_port *por
tty->alt_speed = 230400;
if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_WARP)
tty->alt_speed = 460800;
-
-errout:
local_irq_restore(flags);
- return retval;
+
+ if (state->irq) {
+ int retval = request_irq(state->irq, rs_interrupt_single, 0,
+ "simserial", state);
+ if (retval)
+ return retval;
+ }
+
+ return 0;
}
The simscsi driver processes the requests in the request routine and then
offloads the completion callback to a tasklet. This is buggy because there
is parallel unsynchronized access to the completion queue from the request
routine and from the tasklet.
With current SCSI architecture, requests can be completed directly from
the requets routine. So I removed the tasklet code.
Signed-off-by: Mikulas Patocka <[email protected]>
---
arch/ia64/hp/sim/simscsi.c | 34 ++--------------------------------
1 file changed, 2 insertions(+), 32 deletions(-)
Index: linux-2.6-ia64/arch/ia64/hp/sim/simscsi.c
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/hp/sim/simscsi.c 2014-01-24 01:23:08.000000000 +0100
+++ linux-2.6-ia64/arch/ia64/hp/sim/simscsi.c 2014-01-24 01:26:16.000000000 +0100
@@ -47,9 +47,6 @@
static struct Scsi_Host *host;
-static void simscsi_interrupt (unsigned long val);
-static DECLARE_TASKLET(simscsi_tasklet, simscsi_interrupt, 0);
-
struct disk_req {
unsigned long addr;
unsigned len;
@@ -64,13 +61,6 @@ static int desc[16] = {
-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1
};
-static struct queue_entry {
- struct scsi_cmnd *sc;
-} queue[SIMSCSI_REQ_QUEUE_LEN];
-
-static int rd, wr;
-static atomic_t num_reqs = ATOMIC_INIT(0);
-
/* base name for default disks */
static char *simscsi_root = DEFAULT_SIMSCSI_ROOT;
@@ -95,21 +85,6 @@ simscsi_setup (char *s)
__setup("simscsi=", simscsi_setup);
-static void
-simscsi_interrupt (unsigned long val)
-{
- struct scsi_cmnd *sc;
-
- while ((sc = queue[rd].sc) != NULL) {
- atomic_dec(&num_reqs);
- queue[rd].sc = NULL;
- if (DBG)
- printk("simscsi_interrupt: done with %ld\n", sc->serial_number);
- (*sc->scsi_done)(sc);
- rd = (rd + 1) % SIMSCSI_REQ_QUEUE_LEN;
- }
-}
-
static int
simscsi_biosparam (struct scsi_device *sdev, struct block_device *n,
sector_t capacity, int ip[])
@@ -315,14 +290,9 @@ simscsi_queuecommand_lck (struct scsi_cm
sc->sense_buffer[0] = 0x70;
sc->sense_buffer[2] = 0x00;
}
- if (atomic_read(&num_reqs) >= SIMSCSI_REQ_QUEUE_LEN) {
- panic("Attempt to queue command while command is pending!!");
- }
- atomic_inc(&num_reqs);
- queue[wr].sc = sc;
- wr = (wr + 1) % SIMSCSI_REQ_QUEUE_LEN;
- tasklet_schedule(&simscsi_tasklet);
+ (*sc->scsi_done)(sc);
+
return 0;
}
Fix some warnings in ia64:
arch/ia64/kernel/sys_ia64.c:29:20: warning: unused variable 'mm'
[-Wunused-variable]
arch/ia64/sn/kernel/setup.c:582:23: warning: argument to 'sizeof' in
'memset' call is the same pointer type 'struct pda_s *' as the
destination; expected 'struct pda_s' or an explicit length
[-Wsizeof-pointer-memaccess] (this is a real bug!)
arch/ia64/sn/kernel/bte.c:117:22: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
arch/ia64/sn/kernel/bte.c:125:22: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
Signed-off-by: Mikulas Patocka <[email protected]
---
arch/ia64/kernel/sys_ia64.c | 3 +--
arch/ia64/sn/kernel/bte.c | 4 ++--
arch/ia64/sn/kernel/setup.c | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
Index: linux-2.6-ia64/arch/ia64/kernel/sys_ia64.c
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/kernel/sys_ia64.c 2014-01-24 02:20:52.000000000 +0100
+++ linux-2.6-ia64/arch/ia64/kernel/sys_ia64.c 2014-01-24 02:21:16.000000000 +0100
@@ -26,7 +26,6 @@ arch_get_unmapped_area (struct file *fil
{
long map_shared = (flags & MAP_SHARED);
unsigned long align_mask = 0;
- struct mm_struct *mm = current->mm;
struct vm_unmapped_area_info info;
if (len > RGN_MAP_LIMIT)
@@ -34,7 +33,7 @@ arch_get_unmapped_area (struct file *fil
/* handle fixed mapping: prevent overlap with huge pages */
if (flags & MAP_FIXED) {
- if (is_hugepage_only_range(mm, addr, len))
+ if (is_hugepage_only_range(current->mm, addr, len))
return -EINVAL;
return addr;
}
Index: linux-2.6-ia64/arch/ia64/sn/kernel/bte.c
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/sn/kernel/bte.c 2014-01-24 02:20:43.000000000 +0100
+++ linux-2.6-ia64/arch/ia64/sn/kernel/bte.c 2014-01-24 02:21:16.000000000 +0100
@@ -114,7 +114,7 @@ bte_result_t bte_copy(u64 src, u64 dest,
if (mode & BTE_USE_ANY) {
nasid_to_try[1] = my_nasid;
} else {
- nasid_to_try[1] = (int)NULL;
+ nasid_to_try[1] = 0;
}
} else {
/* try local then remote */
@@ -122,7 +122,7 @@ bte_result_t bte_copy(u64 src, u64 dest,
if (mode & BTE_USE_ANY) {
nasid_to_try[1] = NASID_GET(dest);
} else {
- nasid_to_try[1] = (int)NULL;
+ nasid_to_try[1] = 0;
}
}
Index: linux-2.6-ia64/arch/ia64/sn/kernel/setup.c
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/sn/kernel/setup.c 2014-01-24 02:20:43.000000000 +0100
+++ linux-2.6-ia64/arch/ia64/sn/kernel/setup.c 2014-01-24 02:21:16.000000000 +0100
@@ -579,7 +579,7 @@ void sn_cpu_init(void)
(sn_prom_type == 1) ? "real" : "fake");
}
- memset(pda, 0, sizeof(pda));
+ memset(pda, 0, sizeof(*pda));
if (ia64_sn_get_sn_info(0, &sn_hub_info->shub2,
&sn_hub_info->nasid_bitmask,
&sn_hub_info->nasid_shift,
ia64: validate user arguments in csum_partial_copy_from_user
csum_partial_copy_from_user needs to validate that the argument points to
userspace and not kernelspace (see for example commit
3ddc5b46a8e90f3c9251338b60191d0a804b0d92). Consequently, we need to use
copy_from_user instead of __copy_from_user.
We also need to change csum_partial_copy_nocheck - this function is called
with src pointing to kernel space, so we call set_fs(KERNEL_DS) to prevent
copy_from_user from failing.
Signed-off-by: Mikulas Patocka <[email protected]>
Cc: [email protected]
---
arch/ia64/lib/csum_partial_copy.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
Index: linux-2.6-ia64/arch/ia64/lib/csum_partial_copy.c
===================================================================
--- linux-2.6-ia64.orig/arch/ia64/lib/csum_partial_copy.c 2014-01-24 02:40:10.000000000 +0100
+++ linux-2.6-ia64/arch/ia64/lib/csum_partial_copy.c 2014-01-24 03:05:26.000000000 +0100
@@ -116,8 +116,12 @@ csum_partial_copy_from_user(const void _
* scared.
*/
- if (__copy_from_user(dst, src, len) != 0 && errp)
- *errp = -EFAULT;
+ if (copy_from_user(dst, src, len) != 0) {
+ if (*errp)
+ *errp = -EFAULT;
+ memset(dst, 0, len);
+ return psum;
+ }
result = do_csum(dst, len);
@@ -133,8 +137,13 @@ EXPORT_SYMBOL(csum_partial_copy_from_use
__wsum
csum_partial_copy_nocheck(const void *src, void *dst, int len, __wsum sum)
{
- return csum_partial_copy_from_user((__force const void __user *)src,
- dst, len, sum, NULL);
+ __wsum checksum;
+ mm_segment_t oldfs = get_fs();
+ set_fs(KERNEL_DS);
+ checksum = csum_partial_copy_from_user((__force const void __user *)src,
+ dst, len, sum, NULL);
+ set_fs(oldfs);
+ return checksum;
}
EXPORT_SYMBOL(csum_partial_copy_nocheck);
Mikulas Patocka writes:
> Hi
>
> Here I'm sending some ia64 patches to make it work in the ski emulator.
> This has been broken for a long time.
Thanks. I've recently started running 3.x kernels on ia64 via ski,
but I'm getting random kernel crashes with 3.13. I'll give your
patches a try shortly.
I've written a few patches to improve other aspects of running the
kernel on ski:
- ski patch to use tun/tap networking (no need to run ski as root)
- ski patch to implement a fixed-frequency ITC (the ITC is currently
highly variable, completely breaking basic timekeeping)
- kernel patch to turn PAL_HALT_LIGHT into a new SSC_HALT_LIGHT,
and a corresponing ski patch to pause() on SSC_HALT_LIGHT; this
together with the fixed-frequency ITC patch allows ski to idle
with very low host CPU overhead when the guest kernel idles
- kernel patch to bump the RAM size from 130MB to 2GB
I'd be happy to share these patches if there's interest in them.
/Mikael
On Fri, 24 Jan 2014, Mikael Pettersson wrote:
> Mikulas Patocka writes:
> > Hi
> >
> > Here I'm sending some ia64 patches to make it work in the ski emulator.
> > This has been broken for a long time.
>
> Thanks. I've recently started running 3.x kernels on ia64 via ski,
> but I'm getting random kernel crashes with 3.13. I'll give your
> patches a try shortly.
I also had some random page-table corruption when running recent kernels
in ski. The problems occured when upgrading the whole Debian distribution.
Kernel 2.6.8 was solid, new kernels caused problems, I don't know why.
> I've written a few patches to improve other aspects of running the
> kernel on ski:
> - ski patch to use tun/tap networking (no need to run ski as root)
> - ski patch to implement a fixed-frequency ITC (the ITC is currently
> highly variable, completely breaking basic timekeeping)
> - kernel patch to turn PAL_HALT_LIGHT into a new SSC_HALT_LIGHT,
> and a corresponing ski patch to pause() on SSC_HALT_LIGHT; this
> together with the fixed-frequency ITC patch allows ski to idle
> with very low host CPU overhead when the guest kernel idles
> - kernel patch to bump the RAM size from 130MB to 2GB
>
> I'd be happy to share these patches if there's interest in them.
>
> /Mikael
I would be interested in them. I also patched that timekeeping issue in
ski.
Mikulas
On Fri, 24 Jan 2014, Mikael Pettersson wrote:
> Mikulas Patocka writes:
> > Hi
> >
> > Here I'm sending some ia64 patches to make it work in the ski emulator.
> > This has been broken for a long time.
>
> Thanks. I've recently started running 3.x kernels on ia64 via ski,
> but I'm getting random kernel crashes with 3.13. I'll give your
> patches a try shortly.
BTW. does the kernel boot for you without that _STK_LIM_MAX change? For
me, _STK_LIM_MAX was a showstopper, it wasn't able to spawn any userspace
process without this patch.
Mikulkas
Mikulas:
>> Here I'm sending some ia64 patches to make it work in the ski emulator.
>> This has been broken for a long time.
Thanks - There are questions from time to time on how to test ia64
for those people who do not have hardware.
Mikael:
> Thanks. I've recently started running 3.x kernels on ia64 via ski,
> but I'm getting random kernel crashes with 3.13. I'll give your
> patches a try shortly.
Let me know how that goes - I haven't used ski in a decade and
have quite forgotten how to set it up.
> I've written a few patches to improve other aspects of running the
> kernel on ski:
> - kernel patch to turn PAL_HALT_LIGHT into a new SSC_HALT_LIGHT,
> and a corresponing ski patch to pause() on SSC_HALT_LIGHT; this
> together with the fixed-frequency ITC patch allows ski to idle
> with very low host CPU overhead when the guest kernel idles
> - kernel patch to bump the RAM size from 130MB to 2GB
>
> I'd be happy to share these patches if there's interest in them.
It seems that there are at least two of you out there - so I'm happy
to take kernel patches that make things better. Not sure where the
ski patches go - is someone maintaining that?
-Tony
Mikulas Patocka writes:
>
>
> On Fri, 24 Jan 2014, Mikael Pettersson wrote:
>
> > Mikulas Patocka writes:
> > > Hi
> > >
> > > Here I'm sending some ia64 patches to make it work in the ski emulator.
> > > This has been broken for a long time.
> >
> > Thanks. I've recently started running 3.x kernels on ia64 via ski,
> > but I'm getting random kernel crashes with 3.13. I'll give your
> > patches a try shortly.
>
> BTW. does the kernel boot for you without that _STK_LIM_MAX change? For
> me, _STK_LIM_MAX was a showstopper, it wasn't able to spawn any userspace
> process without this patch.
Yes, everything works fine for me without your _STK_LIM_MAX change.
My ski VMs currently run 3.12 kernels with Fedora 9 user-space.
/Mikael
Mikulas Patocka writes:
>
>
> On Fri, 24 Jan 2014, Mikael Pettersson wrote:
>
> > Mikulas Patocka writes:
> > > Hi
> > >
> > > Here I'm sending some ia64 patches to make it work in the ski emulator.
> > > This has been broken for a long time.
> >
> > Thanks. I've recently started running 3.x kernels on ia64 via ski,
> > but I'm getting random kernel crashes with 3.13. I'll give your
> > patches a try shortly.
>
> I also had some random page-table corruption when running recent kernels
> in ski. The problems occured when upgrading the whole Debian distribution.
> Kernel 2.6.8 was solid, new kernels caused problems, I don't know why.
What I've seen so far seems to indicate that gcc-4.8.2 miscompiles ski,
resulting in kernel oopses, and _possibly_ that gcc-4.7.3 miscompiles
the kernel. Ski compiled by gcc-4.7.3 (on x86_64) running a kernel
compiled by gcc-4.3.6 seems to be a solid combination for me.
> > I've written a few patches to improve other aspects of running the
> > kernel on ski:
> > - ski patch to use tun/tap networking (no need to run ski as root)
> > - ski patch to implement a fixed-frequency ITC (the ITC is currently
> > highly variable, completely breaking basic timekeeping)
> > - kernel patch to turn PAL_HALT_LIGHT into a new SSC_HALT_LIGHT,
> > and a corresponing ski patch to pause() on SSC_HALT_LIGHT; this
> > together with the fixed-frequency ITC patch allows ski to idle
> > with very low host CPU overhead when the guest kernel idles
> > - kernel patch to bump the RAM size from 130MB to 2GB
> >
> > I'd be happy to share these patches if there's interest in them.
> >
> > /Mikael
>
> I would be interested in them. I also patched that timekeeping issue in
> ski.
My ski patches are in <http://user.it.uu.se/~mikpe/linux/patches/ia64/ski-1.3.2/>
for now. I'll post the kernel patches to linux-ia64 @ vger in a few minutes.
/Mikael
Luck, Tony writes:
> Mikulas:
> >> Here I'm sending some ia64 patches to make it work in the ski emulator.
> >> This has been broken for a long time.
>
> Thanks - There are questions from time to time on how to test ia64
> for those people who do not have hardware.
>
> Mikael:
> > Thanks. I've recently started running 3.x kernels on ia64 via ski,
> > but I'm getting random kernel crashes with 3.13. I'll give your
> > patches a try shortly.
>
> Let me know how that goes - I haven't used ski in a decade and
> have quite forgotten how to set it up.
It seems my kernel crashes were due to using gcc-4.8.2 to compile ski,
going back to gcc-4.7.3 for ski, and gcc-4.3.6 for cross-compiling
the kernel, gives me a solid emulated system.
> > I've written a few patches to improve other aspects of running the
> > kernel on ski:
> > - kernel patch to turn PAL_HALT_LIGHT into a new SSC_HALT_LIGHT,
> > and a corresponing ski patch to pause() on SSC_HALT_LIGHT; this
> > together with the fixed-frequency ITC patch allows ski to idle
> > with very low host CPU overhead when the guest kernel idles
> > - kernel patch to bump the RAM size from 130MB to 2GB
> >
> > I'd be happy to share these patches if there's interest in them.
>
> It seems that there are at least two of you out there - so I'm happy
> to take kernel patches that make things better. Not sure where the
> ski patches go - is someone maintaining that?
The ski project on sourceforge seems dead, nothing have been posted
on its mailing list since 2008, and the web interface for browsing
the source repo is broken. Red Hat, Gentoo, and Debian package ski
with a few common build fixes, but that's it.
/Mikael
On Sat, 25 Jan 2014, Mikael Pettersson wrote:
> My ski patches are in <http://user.it.uu.se/~mikpe/linux/patches/ia64/ski-1.3.2/>
> for now. I'll post the kernel patches to linux-ia64 @ vger in a few minutes.
>
> /Mikael
Thanks for the patches.
Isn't this subject to races? - could it lock up if the signal happens just
before the pause syscall?
+ case SSC_HALT_LIGHT:
+ /* Sleep until SIGIO or SIGALRM is received; this relies on
+ keyboard/ethernet input being detected via SIGIO, and the
+ ITC now being emulated via setitimer() and SIGALRM. */
+ pause ();
+ break;
+
Mikulas
Mikulas Patocka writes:
>
>
> On Sat, 25 Jan 2014, Mikael Pettersson wrote:
>
> > My ski patches are in <http://user.it.uu.se/~mikpe/linux/patches/ia64/ski-1.3.2/>
> > for now. I'll post the kernel patches to linux-ia64 @ vger in a few minutes.
> >
> > /Mikael
>
> Thanks for the patches.
>
> Isn't this subject to races? - could it lock up if the signal happens just
> before the pause syscall?
>
> + case SSC_HALT_LIGHT:
> + /* Sleep until SIGIO or SIGALRM is received; this relies on
> + keyboard/ethernet input being detected via SIGIO, and the
> + ITC now being emulated via setitimer() and SIGALRM. */
> + pause ();
> + break;
> +
Thanks for the review. You're right, the pause mustn't happen if
itc_itimer_fired == 1. Let me ponder this for a while...
/Mikael
Mikael Pettersson writes:
> Mikulas Patocka writes:
> >
> >
> > On Sat, 25 Jan 2014, Mikael Pettersson wrote:
> >
> > > My ski patches are in <http://user.it.uu.se/~mikpe/linux/patches/ia64/ski-1.3.2/>
> > > for now. I'll post the kernel patches to linux-ia64 @ vger in a few minutes.
> > >
> > > /Mikael
> >
> > Thanks for the patches.
> >
> > Isn't this subject to races? - could it lock up if the signal happens just
> > before the pause syscall?
> >
> > + case SSC_HALT_LIGHT:
> > + /* Sleep until SIGIO or SIGALRM is received; this relies on
> > + keyboard/ethernet input being detected via SIGIO, and the
> > + ITC now being emulated via setitimer() and SIGALRM. */
> > + pause ();
> > + break;
> > +
>
> Thanks for the review. You're right, the pause mustn't happen if
> itc_itimer_fired == 1. Let me ponder this for a while...
Ok, I've fixed this in two different ways: one patch which uses pselect,
and one patch which uses plain select + the self-pipe trick. Both work
in limited testing, but the pselect one is much nicer and appears to have
a little less host CPU overhead, so that's the one I'm stress-testing now.
Both patches have been uploaded to the same place as before.
/Mikael
It actually turns out that the check in csum_partial_copy_from_user is
pointless (the check is already done in csum_and_copy_from_user that is
the only caller of csum_partial_copy_from_user).
So, please ignore this patch - we need to clean up alpha and x86
implementation of csum_partial_copy_from_user and not add this pointless
check to ia64.
Mikulas
On Thu, 23 Jan 2014, Mikulas Patocka wrote:
> ia64: validate user arguments in csum_partial_copy_from_user
>
> csum_partial_copy_from_user needs to validate that the argument points to
> userspace and not kernelspace (see for example commit
> 3ddc5b46a8e90f3c9251338b60191d0a804b0d92). Consequently, we need to use
> copy_from_user instead of __copy_from_user.
>
> We also need to change csum_partial_copy_nocheck - this function is called
> with src pointing to kernel space, so we call set_fs(KERNEL_DS) to prevent
> copy_from_user from failing.
>
> Signed-off-by: Mikulas Patocka <[email protected]>
> Cc: [email protected]
>
> ---
> arch/ia64/lib/csum_partial_copy.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> Index: linux-2.6-ia64/arch/ia64/lib/csum_partial_copy.c
> ===================================================================
> --- linux-2.6-ia64.orig/arch/ia64/lib/csum_partial_copy.c 2014-01-24 02:40:10.000000000 +0100
> +++ linux-2.6-ia64/arch/ia64/lib/csum_partial_copy.c 2014-01-24 03:05:26.000000000 +0100
> @@ -116,8 +116,12 @@ csum_partial_copy_from_user(const void _
> * scared.
> */
>
> - if (__copy_from_user(dst, src, len) != 0 && errp)
> - *errp = -EFAULT;
> + if (copy_from_user(dst, src, len) != 0) {
> + if (*errp)
> + *errp = -EFAULT;
> + memset(dst, 0, len);
> + return psum;
> + }
>
> result = do_csum(dst, len);
>
> @@ -133,8 +137,13 @@ EXPORT_SYMBOL(csum_partial_copy_from_use
> __wsum
> csum_partial_copy_nocheck(const void *src, void *dst, int len, __wsum sum)
> {
> - return csum_partial_copy_from_user((__force const void __user *)src,
> - dst, len, sum, NULL);
> + __wsum checksum;
> + mm_segment_t oldfs = get_fs();
> + set_fs(KERNEL_DS);
> + checksum = csum_partial_copy_from_user((__force const void __user *)src,
> + dst, len, sum, NULL);
> + set_fs(oldfs);
> + return checksum;
> }
>
> EXPORT_SYMBOL(csum_partial_copy_nocheck);
>